| Message ID | 20220425125546.4129-3-johnson.wang@mediatek.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce MediaTek CCI devfreq driver | expand |
Hi Johnson,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master v5.18-rc4 next-20220426]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Johnson-Wang/Introduce-MediaTek-CCI-devfreq-driver/20220425-205820
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arc-allyesconfig (https://download.01.org/0day-ci/archive/20220427/202204271534.xh4s4n5E-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/98b34c0587837b0e5b880b11a52433f8f0eee19f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Johnson-Wang/Introduce-MediaTek-CCI-devfreq-driver/20220425-205820
git checkout 98b34c0587837b0e5b880b11a52433f8f0eee19f
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/devfreq/mtk-cci-devfreq.c: In function 'mtk_ccifreq_probe':
>> drivers/devfreq/mtk-cci-devfreq.c:372:21: error: 'struct devfreq_passive_data' has no member named 'parent_type'
372 | passive_data->parent_type = CPUFREQ_PARENT_DEV;
| ^~
>> drivers/devfreq/mtk-cci-devfreq.c:372:37: error: 'CPUFREQ_PARENT_DEV' undeclared (first use in this function)
372 | passive_data->parent_type = CPUFREQ_PARENT_DEV;
| ^~~~~~~~~~~~~~~~~~
drivers/devfreq/mtk-cci-devfreq.c:372:37: note: each undeclared identifier is reported only once for each function it appears in
In file included from include/linux/device.h:15,
from include/linux/devfreq.h:13,
from drivers/devfreq/mtk-cci-devfreq.c:7:
drivers/devfreq/mtk-cci-devfreq.c:378:30: warning: format '%d' expects argument of type 'int', but argument 3 has type 'long int' [-Wformat=]
378 | dev_err(dev, "failed to add devfreq device: %d\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/devfreq/mtk-cci-devfreq.c:378:17: note: in expansion of macro 'dev_err'
378 | dev_err(dev, "failed to add devfreq device: %d\n",
| ^~~~~~~
drivers/devfreq/mtk-cci-devfreq.c:378:62: note: format string is defined here
378 | dev_err(dev, "failed to add devfreq device: %d\n",
| ~^
| |
| int
| %ld
vim +372 drivers/devfreq/mtk-cci-devfreq.c
255
256 static int mtk_ccifreq_probe(struct platform_device *pdev)
257 {
258 struct device *dev = &pdev->dev;
259 struct mtk_ccifreq_drv *drv;
260 struct devfreq_passive_data *passive_data;
261 struct dev_pm_opp *opp;
262 unsigned long rate, opp_volt;
263 int ret;
264
265 drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL);
266 if (!drv)
267 return -ENOMEM;
268
269 drv->dev = dev;
270 drv->soc_data = (const struct mtk_ccifreq_platform_data *)
271 of_device_get_match_data(&pdev->dev);
272 mutex_init(&drv->reg_lock);
273 platform_set_drvdata(pdev, drv);
274
275 drv->cci_clk = devm_clk_get(dev, "cci");
276 if (IS_ERR(drv->cci_clk)) {
277 ret = PTR_ERR(drv->cci_clk);
278 return dev_err_probe(dev, ret,
279 "failed to get cci clk: %d\n", ret);
280 }
281
282 drv->inter_clk = devm_clk_get(dev, "intermediate");
283 if (IS_ERR(drv->inter_clk)) {
284 ret = PTR_ERR(drv->inter_clk);
285 dev_err_probe(dev, ret,
286 "failed to get intermediate clk: %d\n", ret);
287 goto out_free_resources;
288 }
289
290 drv->proc_reg = devm_regulator_get_optional(dev, "proc");
291 if (IS_ERR(drv->proc_reg)) {
292 ret = PTR_ERR(drv->proc_reg);
293 dev_err_probe(dev, ret,
294 "failed to get proc regulator: %d\n", ret);
295 goto out_free_resources;
296 }
297
298 ret = regulator_enable(drv->proc_reg);
299 if (ret) {
300 dev_err(dev, "failed to enable proc regulator\n");
301 goto out_free_resources;
302 }
303
304 drv->sram_reg = regulator_get_optional(dev, "sram");
305 if (IS_ERR(drv->sram_reg))
306 drv->sram_reg = NULL;
307 else {
308 ret = regulator_enable(drv->sram_reg);
309 if (ret) {
310 dev_err(dev, "failed to enable sram regulator\n");
311 goto out_free_resources;
312 }
313 }
314
315 /*
316 * We assume min voltage is 0 and tracking target voltage using
317 * min_volt_shift for each iteration.
318 * The retry_max is 3 times of expeted iteration count.
319 */
320 drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv->soc_data->sram_max_volt,
321 drv->soc_data->proc_max_volt),
322 drv->soc_data->min_volt_shift);
323
324 ret = clk_prepare_enable(drv->cci_clk);
325 if (ret)
326 goto out_free_resources;
327
328 ret = clk_prepare_enable(drv->inter_clk);
329 if (ret)
330 goto out_disable_cci_clk;
331
332 ret = dev_pm_opp_of_add_table(dev);
333 if (ret) {
334 dev_err(dev, "failed to add opp table: %d\n", ret);
335 goto out_disable_inter_clk;
336 }
337
338 rate = clk_get_rate(drv->inter_clk);
339 opp = dev_pm_opp_find_freq_ceil(dev, &rate);
340 if (IS_ERR(opp)) {
341 ret = PTR_ERR(opp);
342 dev_err(dev, "failed to get intermediate opp: %d\n", ret);
343 goto out_remove_opp_table;
344 }
345 drv->inter_voltage = dev_pm_opp_get_voltage(opp);
346 dev_pm_opp_put(opp);
347
348 rate = U32_MAX;
349 opp = dev_pm_opp_find_freq_floor(drv->dev, &rate);
350 if (IS_ERR(opp)) {
351 dev_err(dev, "failed to get opp\n");
352 ret = PTR_ERR(opp);
353 goto out_remove_opp_table;
354 }
355
356 opp_volt = dev_pm_opp_get_voltage(opp);
357 dev_pm_opp_put(opp);
358 ret = mtk_ccifreq_set_voltage(drv, opp_volt);
359 if (ret) {
360 dev_err(dev, "failed to scale to highest voltage %lu in proc_reg\n",
361 opp_volt);
362 goto out_remove_opp_table;
363 }
364
365 passive_data = devm_kzalloc(dev, sizeof(struct devfreq_passive_data),
366 GFP_KERNEL);
367 if (!passive_data) {
368 ret = -ENOMEM;
369 goto out_remove_opp_table;
370 }
371
> 372 passive_data->parent_type = CPUFREQ_PARENT_DEV;
373 drv->devfreq = devm_devfreq_add_device(dev, &mtk_ccifreq_profile,
374 DEVFREQ_GOV_PASSIVE,
375 passive_data);
376 if (IS_ERR(drv->devfreq)) {
377 ret = -EPROBE_DEFER;
378 dev_err(dev, "failed to add devfreq device: %d\n",
379 PTR_ERR(drv->devfreq));
380 goto out_remove_opp_table;
381 }
382
383 drv->opp_nb.notifier_call = mtk_ccifreq_opp_notifier;
384 ret = dev_pm_opp_register_notifier(dev, &drv->opp_nb);
385 if (ret) {
386 dev_err(dev, "failed to register opp notifier: %d\n", ret);
387 goto out_remove_devfreq_device;
388 }
389 return 0;
390
391 out_remove_devfreq_device:
392 devm_devfreq_remove_device(dev, drv->devfreq);
393
394 out_remove_opp_table:
395 dev_pm_opp_of_remove_table(dev);
396
397 out_disable_inter_clk:
398 clk_disable_unprepare(drv->inter_clk);
399
400 out_disable_cci_clk:
401 clk_disable_unprepare(drv->cci_clk);
402
403 out_free_resources:
404 if (regulator_is_enabled(drv->proc_reg))
405 regulator_disable(drv->proc_reg);
406 if (drv->sram_reg && regulator_is_enabled(drv->sram_reg))
407 regulator_disable(drv->sram_reg);
408
409 if (!IS_ERR(drv->proc_reg))
410 regulator_put(drv->proc_reg);
411 if (!IS_ERR(drv->sram_reg))
412 regulator_put(drv->sram_reg);
413 if (!IS_ERR(drv->cci_clk))
414 clk_put(drv->cci_clk);
415 if (!IS_ERR(drv->inter_clk))
416 clk_put(drv->inter_clk);
417
418 return ret;
419 }
420
Hi Johnson,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v5.18-rc4 next-20220427]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Johnson-Wang/Introduce-MediaTek-CCI-devfreq-driver/20220425-205820
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: hexagon-allyesconfig (https://download.01.org/0day-ci/archive/20220427/202204271737.oAuTwqZH-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 1cddcfdc3c683b393df1a5c9063252eb60e52818)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/98b34c0587837b0e5b880b11a52433f8f0eee19f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Johnson-Wang/Introduce-MediaTek-CCI-devfreq-driver/20220425-205820
git checkout 98b34c0587837b0e5b880b11a52433f8f0eee19f
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash block/partitions/ drivers/devfreq/ drivers/iio/imu/ drivers/misc/lkdtm/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/devfreq/mtk-cci-devfreq.c:372:16: error: no member named 'parent_type' in 'struct devfreq_passive_data'
passive_data->parent_type = CPUFREQ_PARENT_DEV;
~~~~~~~~~~~~ ^
drivers/devfreq/mtk-cci-devfreq.c:372:30: error: use of undeclared identifier 'CPUFREQ_PARENT_DEV'
passive_data->parent_type = CPUFREQ_PARENT_DEV;
^
>> drivers/devfreq/mtk-cci-devfreq.c:379:4: warning: format specifies type 'int' but the argument has type 'long' [-Wformat]
PTR_ERR(drv->devfreq));
^~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:144:65: note: expanded from macro 'dev_err'
dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
_p_func(dev, fmt, ##__VA_ARGS__); \
~~~ ^~~~~~~~~~~
1 warning and 2 errors generated.
vim +379 drivers/devfreq/mtk-cci-devfreq.c
255
256 static int mtk_ccifreq_probe(struct platform_device *pdev)
257 {
258 struct device *dev = &pdev->dev;
259 struct mtk_ccifreq_drv *drv;
260 struct devfreq_passive_data *passive_data;
261 struct dev_pm_opp *opp;
262 unsigned long rate, opp_volt;
263 int ret;
264
265 drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL);
266 if (!drv)
267 return -ENOMEM;
268
269 drv->dev = dev;
270 drv->soc_data = (const struct mtk_ccifreq_platform_data *)
271 of_device_get_match_data(&pdev->dev);
272 mutex_init(&drv->reg_lock);
273 platform_set_drvdata(pdev, drv);
274
275 drv->cci_clk = devm_clk_get(dev, "cci");
276 if (IS_ERR(drv->cci_clk)) {
277 ret = PTR_ERR(drv->cci_clk);
278 return dev_err_probe(dev, ret,
279 "failed to get cci clk: %d\n", ret);
280 }
281
282 drv->inter_clk = devm_clk_get(dev, "intermediate");
283 if (IS_ERR(drv->inter_clk)) {
284 ret = PTR_ERR(drv->inter_clk);
285 dev_err_probe(dev, ret,
286 "failed to get intermediate clk: %d\n", ret);
287 goto out_free_resources;
288 }
289
290 drv->proc_reg = devm_regulator_get_optional(dev, "proc");
291 if (IS_ERR(drv->proc_reg)) {
292 ret = PTR_ERR(drv->proc_reg);
293 dev_err_probe(dev, ret,
294 "failed to get proc regulator: %d\n", ret);
295 goto out_free_resources;
296 }
297
298 ret = regulator_enable(drv->proc_reg);
299 if (ret) {
300 dev_err(dev, "failed to enable proc regulator\n");
301 goto out_free_resources;
302 }
303
304 drv->sram_reg = regulator_get_optional(dev, "sram");
305 if (IS_ERR(drv->sram_reg))
306 drv->sram_reg = NULL;
307 else {
308 ret = regulator_enable(drv->sram_reg);
309 if (ret) {
310 dev_err(dev, "failed to enable sram regulator\n");
311 goto out_free_resources;
312 }
313 }
314
315 /*
316 * We assume min voltage is 0 and tracking target voltage using
317 * min_volt_shift for each iteration.
318 * The retry_max is 3 times of expeted iteration count.
319 */
320 drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv->soc_data->sram_max_volt,
321 drv->soc_data->proc_max_volt),
322 drv->soc_data->min_volt_shift);
323
324 ret = clk_prepare_enable(drv->cci_clk);
325 if (ret)
326 goto out_free_resources;
327
328 ret = clk_prepare_enable(drv->inter_clk);
329 if (ret)
330 goto out_disable_cci_clk;
331
332 ret = dev_pm_opp_of_add_table(dev);
333 if (ret) {
334 dev_err(dev, "failed to add opp table: %d\n", ret);
335 goto out_disable_inter_clk;
336 }
337
338 rate = clk_get_rate(drv->inter_clk);
339 opp = dev_pm_opp_find_freq_ceil(dev, &rate);
340 if (IS_ERR(opp)) {
341 ret = PTR_ERR(opp);
342 dev_err(dev, "failed to get intermediate opp: %d\n", ret);
343 goto out_remove_opp_table;
344 }
345 drv->inter_voltage = dev_pm_opp_get_voltage(opp);
346 dev_pm_opp_put(opp);
347
348 rate = U32_MAX;
349 opp = dev_pm_opp_find_freq_floor(drv->dev, &rate);
350 if (IS_ERR(opp)) {
351 dev_err(dev, "failed to get opp\n");
352 ret = PTR_ERR(opp);
353 goto out_remove_opp_table;
354 }
355
356 opp_volt = dev_pm_opp_get_voltage(opp);
357 dev_pm_opp_put(opp);
358 ret = mtk_ccifreq_set_voltage(drv, opp_volt);
359 if (ret) {
360 dev_err(dev, "failed to scale to highest voltage %lu in proc_reg\n",
361 opp_volt);
362 goto out_remove_opp_table;
363 }
364
365 passive_data = devm_kzalloc(dev, sizeof(struct devfreq_passive_data),
366 GFP_KERNEL);
367 if (!passive_data) {
368 ret = -ENOMEM;
369 goto out_remove_opp_table;
370 }
371
372 passive_data->parent_type = CPUFREQ_PARENT_DEV;
373 drv->devfreq = devm_devfreq_add_device(dev, &mtk_ccifreq_profile,
374 DEVFREQ_GOV_PASSIVE,
375 passive_data);
376 if (IS_ERR(drv->devfreq)) {
377 ret = -EPROBE_DEFER;
378 dev_err(dev, "failed to add devfreq device: %d\n",
> 379 PTR_ERR(drv->devfreq));
380 goto out_remove_opp_table;
381 }
382
383 drv->opp_nb.notifier_call = mtk_ccifreq_opp_notifier;
384 ret = dev_pm_opp_register_notifier(dev, &drv->opp_nb);
385 if (ret) {
386 dev_err(dev, "failed to register opp notifier: %d\n", ret);
387 goto out_remove_devfreq_device;
388 }
389 return 0;
390
391 out_remove_devfreq_device:
392 devm_devfreq_remove_device(dev, drv->devfreq);
393
394 out_remove_opp_table:
395 dev_pm_opp_of_remove_table(dev);
396
397 out_disable_inter_clk:
398 clk_disable_unprepare(drv->inter_clk);
399
400 out_disable_cci_clk:
401 clk_disable_unprepare(drv->cci_clk);
402
403 out_free_resources:
404 if (regulator_is_enabled(drv->proc_reg))
405 regulator_disable(drv->proc_reg);
406 if (drv->sram_reg && regulator_is_enabled(drv->sram_reg))
407 regulator_disable(drv->sram_reg);
408
409 if (!IS_ERR(drv->proc_reg))
410 regulator_put(drv->proc_reg);
411 if (!IS_ERR(drv->sram_reg))
412 regulator_put(drv->sram_reg);
413 if (!IS_ERR(drv->cci_clk))
414 clk_put(drv->cci_clk);
415 if (!IS_ERR(drv->inter_clk))
416 clk_put(drv->inter_clk);
417
418 return ret;
419 }
420
On Wed, 2022-04-27 at 17:25 +0800, kernel test robot wrote: > Hi Johnson, > > Thank you for the patch! Perhaps something to improve: > > [auto build test WARNING on robh/for-next] > [also build test WARNING on linus/master v5.18-rc4 next-20220427] > [If your patch is applied to the wrong git tree, kindly drop us a > note. > And when submitting patch, we suggest to use '--base' as documented > in > https://urldefense.com/v3/__https://git-scm.com/docs/git-format-patch__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMS_s9zEZ$ > ] > > url: > https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commits/Johnson-Wang/Introduce-MediaTek-CCI-devfreq-driver/20220425-205820__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMc1U_tqz$ > > base: > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMUzTprof$ > for-next > config: hexagon-allyesconfig ( > https://urldefense.com/v3/__https://download.01.org/0day-ci/archive/20220427/202204271737.oAuTwqZH-lkp@intel.com/config__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMaVRzbSL$ > ) > compiler: clang version 15.0.0 ( > https://urldefense.com/v3/__https://github.com/llvm/llvm-project__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMRqw5IY-$ > $ 1cddcfdc3c683b393df1a5c9063252eb60e52818) > reproduce (this is a W=1 build): > wget > https://urldefense.com/v3/__https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMQLiD-i9$ > -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # > https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commit/98b34c0587837b0e5b880b11a52433f8f0eee19f__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMU5yd7Y2$ > > git remote add linux-review > https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMW4ldtnH$ > > git fetch --no-tags linux-review Johnson-Wang/Introduce- > MediaTek-CCI-devfreq-driver/20220425-205820 > git checkout 98b34c0587837b0e5b880b11a52433f8f0eee19f > # save the config file > mkdir build_dir && cp config build_dir/.config > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross > W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash block/partitions/ > drivers/devfreq/ drivers/iio/imu/ drivers/misc/lkdtm/ > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> > > All warnings (new ones prefixed by >>): > > drivers/devfreq/mtk-cci-devfreq.c:372:16: error: no member named > 'parent_type' in 'struct devfreq_passive_data' > passive_data->parent_type = CPUFREQ_PARENT_DEV; > ~~~~~~~~~~~~ ^ > drivers/devfreq/mtk-cci-devfreq.c:372:30: error: use of undeclared > identifier 'CPUFREQ_PARENT_DEV' > passive_data->parent_type = CPUFREQ_PARENT_DEV; > ^ > > > drivers/devfreq/mtk-cci-devfreq.c:379:4: warning: format > > > specifies type 'int' but the argument has type 'long' [-Wformat] > > PTR_ERR(drv->devfreq)); > ^~~~~~~~~~~~~~~~~~~~~ > include/linux/dev_printk.h:144:65: note: expanded from macro > 'dev_err' > dev_printk_index_wrap(_dev_err, KERN_ERR, dev, > dev_fmt(fmt), ##__VA_ARGS__) > ~~~ > ^~~~~~~~~~~ > include/linux/dev_printk.h:110:23: note: expanded from macro > 'dev_printk_index_wrap' > _p_func(dev, fmt, > ##__VA_ARGS__); \ > ~~~ ^~~~~~~~~~~ > 1 warning and 2 errors generated. > > > vim +379 drivers/devfreq/mtk-cci-devfreq.c > > 255 > 256 static int mtk_ccifreq_probe(struct platform_device > *pdev) > 257 { > 258 struct device *dev = &pdev->dev; > 259 struct mtk_ccifreq_drv *drv; > 260 struct devfreq_passive_data *passive_data; > 261 struct dev_pm_opp *opp; > 262 unsigned long rate, opp_volt; > 263 int ret; > 264 > 265 drv = devm_kzalloc(dev, sizeof(*drv), > GFP_KERNEL); > 266 if (!drv) > 267 return -ENOMEM; > 268 > 269 drv->dev = dev; > 270 drv->soc_data = (const struct > mtk_ccifreq_platform_data *) > 271 of_device_get_match_dat > a(&pdev->dev); > 272 mutex_init(&drv->reg_lock); > 273 platform_set_drvdata(pdev, drv); > 274 > 275 drv->cci_clk = devm_clk_get(dev, "cci"); > 276 if (IS_ERR(drv->cci_clk)) { > 277 ret = PTR_ERR(drv->cci_clk); > 278 return dev_err_probe(dev, ret, > 279 "failed to get cci > clk: %d\n", ret); > 280 } > 281 > 282 drv->inter_clk = devm_clk_get(dev, > "intermediate"); > 283 if (IS_ERR(drv->inter_clk)) { > 284 ret = PTR_ERR(drv->inter_clk); > 285 dev_err_probe(dev, ret, > 286 "failed to get > intermediate clk: %d\n", ret); > 287 goto out_free_resources; > 288 } > 289 > 290 drv->proc_reg = > devm_regulator_get_optional(dev, "proc"); > 291 if (IS_ERR(drv->proc_reg)) { > 292 ret = PTR_ERR(drv->proc_reg); > 293 dev_err_probe(dev, ret, > 294 "failed to get proc > regulator: %d\n", ret); > 295 goto out_free_resources; > 296 } > 297 > 298 ret = regulator_enable(drv->proc_reg); > 299 if (ret) { > 300 dev_err(dev, "failed to enable proc > regulator\n"); > 301 goto out_free_resources; > 302 } > 303 > 304 drv->sram_reg = regulator_get_optional(dev, > "sram"); > 305 if (IS_ERR(drv->sram_reg)) > 306 drv->sram_reg = NULL; > 307 else { > 308 ret = regulator_enable(drv->sram_reg); > 309 if (ret) { > 310 dev_err(dev, "failed to enable > sram regulator\n"); > 311 goto out_free_resources; > 312 } > 313 } > 314 > 315 /* > 316 * We assume min voltage is 0 and tracking > target voltage using > 317 * min_volt_shift for each iteration. > 318 * The retry_max is 3 times of expeted > iteration count. > 319 */ > 320 drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv- > >soc_data->sram_max_volt, > 321 drv- > >soc_data->proc_max_volt), > 322 drv- > >soc_data->min_volt_shift); > 323 > 324 ret = clk_prepare_enable(drv->cci_clk); > 325 if (ret) > 326 goto out_free_resources; > 327 > 328 ret = clk_prepare_enable(drv->inter_clk); > 329 if (ret) > 330 goto out_disable_cci_clk; > 331 > 332 ret = dev_pm_opp_of_add_table(dev); > 333 if (ret) { > 334 dev_err(dev, "failed to add opp table: > %d\n", ret); > 335 goto out_disable_inter_clk; > 336 } > 337 > 338 rate = clk_get_rate(drv->inter_clk); > 339 opp = dev_pm_opp_find_freq_ceil(dev, &rate); > 340 if (IS_ERR(opp)) { > 341 ret = PTR_ERR(opp); > 342 dev_err(dev, "failed to get > intermediate opp: %d\n", ret); > 343 goto out_remove_opp_table; > 344 } > 345 drv->inter_voltage = > dev_pm_opp_get_voltage(opp); > 346 dev_pm_opp_put(opp); > 347 > 348 rate = U32_MAX; > 349 opp = dev_pm_opp_find_freq_floor(drv->dev, > &rate); > 350 if (IS_ERR(opp)) { > 351 dev_err(dev, "failed to get opp\n"); > 352 ret = PTR_ERR(opp); > 353 goto out_remove_opp_table; > 354 } > 355 > 356 opp_volt = dev_pm_opp_get_voltage(opp); > 357 dev_pm_opp_put(opp); > 358 ret = mtk_ccifreq_set_voltage(drv, opp_volt); > 359 if (ret) { > 360 dev_err(dev, "failed to scale to > highest voltage %lu in proc_reg\n", > 361 opp_volt); > 362 goto out_remove_opp_table; > 363 } > 364 > 365 passive_data = devm_kzalloc(dev, sizeof(struct > devfreq_passive_data), > 366 GFP_KERNEL); > 367 if (!passive_data) { > 368 ret = -ENOMEM; > 369 goto out_remove_opp_table; > 370 } > 371 > 372 passive_data->parent_type = CPUFREQ_PARENT_DEV; > 373 drv->devfreq = devm_devfreq_add_device(dev, > &mtk_ccifreq_profile, > 374 DEVFREQ_ > GOV_PASSIVE, > 375 passive_ > data); > 376 if (IS_ERR(drv->devfreq)) { > 377 ret = -EPROBE_DEFER; > 378 dev_err(dev, "failed to add devfreq > device: %d\n", > > 379 PTR_ERR(drv->devfreq)); > 380 goto out_remove_opp_table; > 381 } > 382 > 383 drv->opp_nb.notifier_call = > mtk_ccifreq_opp_notifier; > 384 ret = dev_pm_opp_register_notifier(dev, &drv- > >opp_nb); > 385 if (ret) { > 386 dev_err(dev, "failed to register opp > notifier: %d\n", ret); > 387 goto out_remove_devfreq_device; > 388 } > 389 return 0; > 390 > 391 out_remove_devfreq_device: > 392 devm_devfreq_remove_device(dev, drv->devfreq); > 393 > 394 out_remove_opp_table: > 395 dev_pm_opp_of_remove_table(dev); > 396 > 397 out_disable_inter_clk: > 398 clk_disable_unprepare(drv->inter_clk); > 399 > 400 out_disable_cci_clk: > 401 clk_disable_unprepare(drv->cci_clk); > 402 > 403 out_free_resources: > 404 if (regulator_is_enabled(drv->proc_reg)) > 405 regulator_disable(drv->proc_reg); > 406 if (drv->sram_reg && regulator_is_enabled(drv- > >sram_reg)) > 407 regulator_disable(drv->sram_reg); > 408 > 409 if (!IS_ERR(drv->proc_reg)) > 410 regulator_put(drv->proc_reg); > 411 if (!IS_ERR(drv->sram_reg)) > 412 regulator_put(drv->sram_reg); > 413 if (!IS_ERR(drv->cci_clk)) > 414 clk_put(drv->cci_clk); > 415 if (!IS_ERR(drv->inter_clk)) > 416 clk_put(drv->inter_clk); > 417 > 418 return ret; > 419 } > 420 > Hi "kernel test robot", Thanks for your review. This patch is based on chanwoo/devfreq-testing[1] [1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing I will follow your suggestion to use '--base' in the next version. BRs, Johnson Wang
On 4/27/2022 6:11 PM, Johnson Wang wrote: > On Wed, 2022-04-27 at 17:25 +0800, kernel test robot wrote: >> Hi Johnson, >> >> Thank you for the patch! Perhaps something to improve: >> >> [auto build test WARNING on robh/for-next] >> [also build test WARNING on linus/master v5.18-rc4 next-20220427] >> [If your patch is applied to the wrong git tree, kindly drop us a >> note. >> And when submitting patch, we suggest to use '--base' as documented >> in >> > https://urldefense.com/v3/__https://git-scm.com/docs/git-format-patch__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMS_s9zEZ$ >> ] >> >> url: >> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commits/Johnson-Wang/Introduce-MediaTek-CCI-devfreq-driver/20220425-205820__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMc1U_tqz$ >> >> base: >> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMUzTprof$ >> for-next >> config: hexagon-allyesconfig ( >> https://urldefense.com/v3/__https://download.01.org/0day-ci/archive/20220427/202204271737.oAuTwqZH-lkp@intel.com/config__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMaVRzbSL$ >> ) >> compiler: clang version 15.0.0 ( >> https://urldefense.com/v3/__https://github.com/llvm/llvm-project__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMRqw5IY-$ >> $ 1cddcfdc3c683b393df1a5c9063252eb60e52818) >> reproduce (this is a W=1 build): >> wget >> https://urldefense.com/v3/__https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMQLiD-i9$ >> -O ~/bin/make.cross >> chmod +x ~/bin/make.cross >> # >> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commit/98b34c0587837b0e5b880b11a52433f8f0eee19f__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMU5yd7Y2$ >> >> git remote add linux-review >> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMW4ldtnH$ >> >> git fetch --no-tags linux-review Johnson-Wang/Introduce- >> MediaTek-CCI-devfreq-driver/20220425-205820 >> git checkout 98b34c0587837b0e5b880b11a52433f8f0eee19f >> # save the config file >> mkdir build_dir && cp config build_dir/.config >> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross >> W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash block/partitions/ >> drivers/devfreq/ drivers/iio/imu/ drivers/misc/lkdtm/ >> >> If you fix the issue, kindly add following tag as appropriate >> Reported-by: kernel test robot <lkp@intel.com> >> >> All warnings (new ones prefixed by >>): >> >> drivers/devfreq/mtk-cci-devfreq.c:372:16: error: no member named >> 'parent_type' in 'struct devfreq_passive_data' >> passive_data->parent_type = CPUFREQ_PARENT_DEV; >> ~~~~~~~~~~~~ ^ >> drivers/devfreq/mtk-cci-devfreq.c:372:30: error: use of undeclared >> identifier 'CPUFREQ_PARENT_DEV' >> passive_data->parent_type = CPUFREQ_PARENT_DEV; >> ^ >>>> drivers/devfreq/mtk-cci-devfreq.c:379:4: warning: format >>>> specifies type 'int' but the argument has type 'long' [-Wformat] >> >> PTR_ERR(drv->devfreq)); >> ^~~~~~~~~~~~~~~~~~~~~ >> include/linux/dev_printk.h:144:65: note: expanded from macro >> 'dev_err' >> dev_printk_index_wrap(_dev_err, KERN_ERR, dev, >> dev_fmt(fmt), ##__VA_ARGS__) >> ~~~ >> ^~~~~~~~~~~ >> include/linux/dev_printk.h:110:23: note: expanded from macro >> 'dev_printk_index_wrap' >> _p_func(dev, fmt, >> ##__VA_ARGS__); \ >> ~~~ ^~~~~~~~~~~ >> 1 warning and 2 errors generated. >> >> >> vim +379 drivers/devfreq/mtk-cci-devfreq.c >> >> 255 >> 256 static int mtk_ccifreq_probe(struct platform_device >> *pdev) >> 257 { >> 258 struct device *dev = &pdev->dev; >> 259 struct mtk_ccifreq_drv *drv; >> 260 struct devfreq_passive_data *passive_data; >> 261 struct dev_pm_opp *opp; >> 262 unsigned long rate, opp_volt; >> 263 int ret; >> 264 >> 265 drv = devm_kzalloc(dev, sizeof(*drv), >> GFP_KERNEL); >> 266 if (!drv) >> 267 return -ENOMEM; >> 268 >> 269 drv->dev = dev; >> 270 drv->soc_data = (const struct >> mtk_ccifreq_platform_data *) >> 271 of_device_get_match_dat >> a(&pdev->dev); >> 272 mutex_init(&drv->reg_lock); >> 273 platform_set_drvdata(pdev, drv); >> 274 >> 275 drv->cci_clk = devm_clk_get(dev, "cci"); >> 276 if (IS_ERR(drv->cci_clk)) { >> 277 ret = PTR_ERR(drv->cci_clk); >> 278 return dev_err_probe(dev, ret, >> 279 "failed to get cci >> clk: %d\n", ret); >> 280 } >> 281 >> 282 drv->inter_clk = devm_clk_get(dev, >> "intermediate"); >> 283 if (IS_ERR(drv->inter_clk)) { >> 284 ret = PTR_ERR(drv->inter_clk); >> 285 dev_err_probe(dev, ret, >> 286 "failed to get >> intermediate clk: %d\n", ret); >> 287 goto out_free_resources; >> 288 } >> 289 >> 290 drv->proc_reg = >> devm_regulator_get_optional(dev, "proc"); >> 291 if (IS_ERR(drv->proc_reg)) { >> 292 ret = PTR_ERR(drv->proc_reg); >> 293 dev_err_probe(dev, ret, >> 294 "failed to get proc >> regulator: %d\n", ret); >> 295 goto out_free_resources; >> 296 } >> 297 >> 298 ret = regulator_enable(drv->proc_reg); >> 299 if (ret) { >> 300 dev_err(dev, "failed to enable proc >> regulator\n"); >> 301 goto out_free_resources; >> 302 } >> 303 >> 304 drv->sram_reg = regulator_get_optional(dev, >> "sram"); >> 305 if (IS_ERR(drv->sram_reg)) >> 306 drv->sram_reg = NULL; >> 307 else { >> 308 ret = regulator_enable(drv->sram_reg); >> 309 if (ret) { >> 310 dev_err(dev, "failed to enable >> sram regulator\n"); >> 311 goto out_free_resources; >> 312 } >> 313 } >> 314 >> 315 /* >> 316 * We assume min voltage is 0 and tracking >> target voltage using >> 317 * min_volt_shift for each iteration. >> 318 * The retry_max is 3 times of expeted >> iteration count. >> 319 */ >> 320 drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv- >>> soc_data->sram_max_volt, >> 321 drv- >>> soc_data->proc_max_volt), >> 322 drv- >>> soc_data->min_volt_shift); >> 323 >> 324 ret = clk_prepare_enable(drv->cci_clk); >> 325 if (ret) >> 326 goto out_free_resources; >> 327 >> 328 ret = clk_prepare_enable(drv->inter_clk); >> 329 if (ret) >> 330 goto out_disable_cci_clk; >> 331 >> 332 ret = dev_pm_opp_of_add_table(dev); >> 333 if (ret) { >> 334 dev_err(dev, "failed to add opp table: >> %d\n", ret); >> 335 goto out_disable_inter_clk; >> 336 } >> 337 >> 338 rate = clk_get_rate(drv->inter_clk); >> 339 opp = dev_pm_opp_find_freq_ceil(dev, &rate); >> 340 if (IS_ERR(opp)) { >> 341 ret = PTR_ERR(opp); >> 342 dev_err(dev, "failed to get >> intermediate opp: %d\n", ret); >> 343 goto out_remove_opp_table; >> 344 } >> 345 drv->inter_voltage = >> dev_pm_opp_get_voltage(opp); >> 346 dev_pm_opp_put(opp); >> 347 >> 348 rate = U32_MAX; >> 349 opp = dev_pm_opp_find_freq_floor(drv->dev, >> &rate); >> 350 if (IS_ERR(opp)) { >> 351 dev_err(dev, "failed to get opp\n"); >> 352 ret = PTR_ERR(opp); >> 353 goto out_remove_opp_table; >> 354 } >> 355 >> 356 opp_volt = dev_pm_opp_get_voltage(opp); >> 357 dev_pm_opp_put(opp); >> 358 ret = mtk_ccifreq_set_voltage(drv, opp_volt); >> 359 if (ret) { >> 360 dev_err(dev, "failed to scale to >> highest voltage %lu in proc_reg\n", >> 361 opp_volt); >> 362 goto out_remove_opp_table; >> 363 } >> 364 >> 365 passive_data = devm_kzalloc(dev, sizeof(struct >> devfreq_passive_data), >> 366 GFP_KERNEL); >> 367 if (!passive_data) { >> 368 ret = -ENOMEM; >> 369 goto out_remove_opp_table; >> 370 } >> 371 >> 372 passive_data->parent_type = CPUFREQ_PARENT_DEV; >> 373 drv->devfreq = devm_devfreq_add_device(dev, >> &mtk_ccifreq_profile, >> 374 DEVFREQ_ >> GOV_PASSIVE, >> 375 passive_ >> data); >> 376 if (IS_ERR(drv->devfreq)) { >> 377 ret = -EPROBE_DEFER; >> 378 dev_err(dev, "failed to add devfreq >> device: %d\n", >> > 379 PTR_ERR(drv->devfreq)); >> 380 goto out_remove_opp_table; >> 381 } >> 382 >> 383 drv->opp_nb.notifier_call = >> mtk_ccifreq_opp_notifier; >> 384 ret = dev_pm_opp_register_notifier(dev, &drv- >>> opp_nb); >> 385 if (ret) { >> 386 dev_err(dev, "failed to register opp >> notifier: %d\n", ret); >> 387 goto out_remove_devfreq_device; >> 388 } >> 389 return 0; >> 390 >> 391 out_remove_devfreq_device: >> 392 devm_devfreq_remove_device(dev, drv->devfreq); >> 393 >> 394 out_remove_opp_table: >> 395 dev_pm_opp_of_remove_table(dev); >> 396 >> 397 out_disable_inter_clk: >> 398 clk_disable_unprepare(drv->inter_clk); >> 399 >> 400 out_disable_cci_clk: >> 401 clk_disable_unprepare(drv->cci_clk); >> 402 >> 403 out_free_resources: >> 404 if (regulator_is_enabled(drv->proc_reg)) >> 405 regulator_disable(drv->proc_reg); >> 406 if (drv->sram_reg && regulator_is_enabled(drv- >>> sram_reg)) >> 407 regulator_disable(drv->sram_reg); >> 408 >> 409 if (!IS_ERR(drv->proc_reg)) >> 410 regulator_put(drv->proc_reg); >> 411 if (!IS_ERR(drv->sram_reg)) >> 412 regulator_put(drv->sram_reg); >> 413 if (!IS_ERR(drv->cci_clk)) >> 414 clk_put(drv->cci_clk); >> 415 if (!IS_ERR(drv->inter_clk)) >> 416 clk_put(drv->inter_clk); >> 417 >> 418 return ret; >> 419 } >> 420 >> > > Hi "kernel test robot", > > Thanks for your review. > > This patch is based on chanwoo/devfreq-testing[1] > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing Hi Johnson, Thanks for the feedback, we'll take a look too. Best Regards, Rong Chen > > I will follow your suggestion to use '--base' in the next version. > > BRs, > Johnson Wang > _______________________________________________ > kbuild-all mailing list -- kbuild-all@lists.01.org > To unsubscribe send an email to kbuild-all-leave@lists.01.org >
On Mon, 2022-04-25 at 20:55 +0800, Johnson Wang wrote: > We introduce a devfreq driver for the MediaTek Cache Coherent > Interconnect > (CCI) used by some MediaTek SoCs. > > In this driver, we use the passive devfreq driver to get target > frequencies > and adjust voltages accordingly. In MT8183 and MT8186, the MediaTek > CCI > is supplied by the same regulators with the little core CPUs. > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > --- > This patch depends on "devfreq-testing"[1]. > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing > --- > drivers/devfreq/Kconfig | 10 + > drivers/devfreq/Makefile | 1 + > drivers/devfreq/mtk-cci-devfreq.c | 474 > ++++++++++++++++++++++++++++++ > 3 files changed, 485 insertions(+) > create mode 100644 drivers/devfreq/mtk-cci-devfreq.c > > diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig > index 87eb2b837e68..9754d8b31621 100644 > --- a/drivers/devfreq/Kconfig > +++ b/drivers/devfreq/Kconfig > @@ -120,6 +120,16 @@ config ARM_TEGRA_DEVFREQ > It reads ACTMON counters of memory controllers and adjusts > the > operating frequencies and voltages with OPP support. > > +config ARM_MEDIATEK_CCI_DEVFREQ > + tristate "MEDIATEK CCI DEVFREQ Driver" > + depends on ARM_MEDIATEK_CPUFREQ || COMPILE_TEST > + select DEVFREQ_GOV_PASSIVE > + help > + This adds a devfreq driver for MediaTek Cache Coherent > Interconnect > + which is shared the same regulators with the cpu cluster. It > can track > + buck voltages and update a proper CCI frequency. Use the > notification > + to get the regulator status. > + > config ARM_RK3399_DMC_DEVFREQ > tristate "ARM RK3399 DMC DEVFREQ Driver" > depends on (ARCH_ROCKCHIP && HAVE_ARM_SMCCC) || \ > diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile > index 0b6be92a25d9..bf40d04928d0 100644 > --- a/drivers/devfreq/Makefile > +++ b/drivers/devfreq/Makefile > @@ -11,6 +11,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += > governor_passive.o > obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o > obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ) += imx-bus.o > obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ) += imx8m-ddrc.o > +obj-$(CONFIG_ARM_MEDIATEK_CCI_DEVFREQ) += mtk-cci-devfreq.o > obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o > obj-$(CONFIG_ARM_SUN8I_A33_MBUS_DEVFREQ) += sun8i-a33-mbus.o > obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30-devfreq.o > diff --git a/drivers/devfreq/mtk-cci-devfreq.c b/drivers/devfreq/mtk- > cci-devfreq.c > new file mode 100644 > index 000000000000..b3e31c45a57c > --- /dev/null > +++ b/drivers/devfreq/mtk-cci-devfreq.c > @@ -0,0 +1,474 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2022 MediaTek Inc. > + */ > + > +#include <linux/clk.h> > +#include <linux/devfreq.h> > +#include <linux/minmax.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/platform_device.h> > +#include <linux/pm_opp.h> > +#include <linux/regulator/consumer.h> > + > +struct mtk_ccifreq_platform_data { > + int min_volt_shift; > + int max_volt_shift; > + int proc_max_volt; > + int sram_min_volt; > + int sram_max_volt; > +}; > + > +struct mtk_ccifreq_drv { > + struct device *dev; > + struct devfreq *devfreq; > + struct regulator *proc_reg; > + struct regulator *sram_reg; > + struct clk *cci_clk; > + struct clk *inter_clk; > + int inter_voltage; > + int pre_voltage; > + unsigned long pre_freq; > + /* Avoid race condition for regulators between notify and > policy */ > + struct mutex reg_lock; > + struct notifier_block opp_nb; > + const struct mtk_ccifreq_platform_data *soc_data; > + int vtrack_max; > +}; > + > +static int mtk_ccifreq_set_voltage(struct mtk_ccifreq_drv *drv, int > new_voltage) > +{ > + const struct mtk_ccifreq_platform_data *soc_data = drv- > >soc_data; > + struct device *dev = drv->dev; > + int pre_voltage, pre_vsram, new_vsram, vsram, voltage, ret; > + int retry_max = drv->vtrack_max; > + > + if (!drv->sram_reg) { > + ret = regulator_set_voltage(drv->proc_reg, new_voltage, > + drv->soc_data- > >proc_max_volt); > + goto out_set_voltage; > + } > + > + pre_voltage = regulator_get_voltage(drv->proc_reg); > + if (pre_voltage < 0) { > + dev_err(dev, "invalid vproc value: %d\n", pre_voltage); > + return pre_voltage; > + } > + > + pre_vsram = regulator_get_voltage(drv->sram_reg); > + if (pre_vsram < 0) { > + dev_err(dev, "invalid vsram value: %d\n", pre_vsram); > + return pre_vsram; > + } > + > + new_vsram = clamp(new_voltage + soc_data->min_volt_shift, > + soc_data->sram_min_volt, soc_data- > >sram_max_volt); > + > + do { > + if (pre_voltage <= new_voltage) { > + vsram = clamp(pre_voltage + soc_data- > >max_volt_shift, > + soc_data->sram_min_volt, > new_vsram); > + ret = regulator_set_voltage(drv->sram_reg, > vsram, > + soc_data- > >sram_max_volt); > + if (ret) > + return ret; > + > + if (vsram == soc_data->sram_max_volt || > + new_vsram == soc_data->sram_min_volt) > + voltage = new_voltage; > + else > + voltage = vsram - soc_data- > >min_volt_shift; > + > + ret = regulator_set_voltage(drv->proc_reg, > voltage, > + soc_data- > >proc_max_volt); > + if (ret) { > + regulator_set_voltage(drv->sram_reg, > pre_vsram, > + soc_data- > >sram_max_volt); > + return ret; > + } > + } else if (pre_voltage > new_voltage) { > + voltage = max(new_voltage, > + pre_vsram - soc_data- > >max_volt_shift); > + ret = regulator_set_voltage(drv->proc_reg, > voltage, > + soc_data- > >proc_max_volt); > + if (ret) > + return ret; > + > + if (voltage == new_voltage) > + vsram = new_vsram; > + else > + vsram = max(new_vsram, > + voltage + soc_data- > >min_volt_shift); > + > + ret = regulator_set_voltage(drv->sram_reg, > vsram, > + soc_data- > >sram_max_volt); > + if (ret) { > + regulator_set_voltage(drv->proc_reg, > pre_voltage, > + soc_data- > >proc_max_volt); > + return ret; > + } > + } > + > + pre_voltage = voltage; > + pre_vsram = vsram; > + > + if (--retry_max < 0) { > + dev_err(dev, > + "over loop count, failed to set > voltage\n"); > + return -EINVAL; > + } > + } while (voltage != new_voltage || vsram != new_vsram); > + > +out_set_voltage: > + if (!ret) > + drv->pre_voltage = new_voltage; > + > + return ret; > +} > + > +static int mtk_ccifreq_target(struct device *dev, unsigned long > *freq, > + u32 flags) > +{ > + struct mtk_ccifreq_drv *drv = dev_get_drvdata(dev); > + struct clk *cci_pll = clk_get_parent(drv->cci_clk); > + struct dev_pm_opp *opp; > + unsigned long opp_rate; > + int voltage, pre_voltage, inter_voltage, target_voltage, ret; > + > + if (!drv) > + return -EINVAL; > + > + if (drv->pre_freq == *freq) > + return 0; > + > + inter_voltage = drv->inter_voltage; > + > + opp_rate = *freq; > + opp = devfreq_recommended_opp(dev, &opp_rate, 1); > + if (IS_ERR(opp)) { > + dev_err(dev, "failed to find opp for freq: %ld\n", > opp_rate); > + return PTR_ERR(opp); > + } > + > + mutex_lock(&drv->reg_lock); > + > + voltage = dev_pm_opp_get_voltage(opp); > + dev_pm_opp_put(opp); > + > + if (unlikely(drv->pre_voltage <= 0)) > + pre_voltage = regulator_get_voltage(drv->proc_reg); > + else > + pre_voltage = drv->pre_voltage; > + > + if (pre_voltage < 0) { > + dev_err(dev, "invalid vproc value: %d\n", pre_voltage); > + return pre_voltage; > + } > + > + /* scale up: set voltage first then freq. */ > + target_voltage = max(inter_voltage, voltage); > + if (pre_voltage <= target_voltage) { > + ret = mtk_ccifreq_set_voltage(drv, target_voltage); > + if (ret) { > + dev_err(dev, "failed to scale up voltage\n"); > + goto out_restore_voltage; > + } > + } > + > + /* switch the cci clock to intermediate clock source. */ > + ret = clk_set_parent(drv->cci_clk, drv->inter_clk); > + if (ret) { > + dev_err(dev, "failed to re-parent cci clock\n"); > + goto out_restore_voltage; > + } > + > + /* set the original clock to target rate. */ > + ret = clk_set_rate(cci_pll, *freq); > + if (ret) { > + dev_err(dev, "failed to set cci pll rate: %d\n", ret); > + clk_set_parent(drv->cci_clk, cci_pll); > + goto out_restore_voltage; > + } > + > + /* switch the cci clock back to the original clock source. */ > + ret = clk_set_parent(drv->cci_clk, cci_pll); > + if (ret) { > + dev_err(dev, "failed to re-parent cci clock\n"); > + mtk_ccifreq_set_voltage(drv, inter_voltage); > + goto out_unlock; > + } > + > + /* > + * If the new voltage is lower than the intermediate voltage or > the > + * original voltage, scale down to the new voltage. > + */ > + if (voltage < inter_voltage || voltage < pre_voltage) { > + ret = mtk_ccifreq_set_voltage(drv, voltage); > + if (ret) { > + dev_err(dev, "failed to scale down voltage\n"); > + goto out_unlock; > + } > + } > + > + drv->pre_freq = *freq; > + mutex_unlock(&drv->reg_lock); > + > + return 0; > + > +out_restore_voltage: > + mtk_ccifreq_set_voltage(drv, pre_voltage); > + > +out_unlock: > + mutex_unlock(&drv->reg_lock); > + return ret; > +} > + > +static int mtk_ccifreq_opp_notifier(struct notifier_block *nb, > + unsigned long event, void *data) > +{ > + struct dev_pm_opp *opp = data; > + struct mtk_ccifreq_drv *drv; > + unsigned long freq, volt; > + > + drv = container_of(nb, struct mtk_ccifreq_drv, opp_nb); > + > + if (event == OPP_EVENT_ADJUST_VOLTAGE) { > + freq = dev_pm_opp_get_freq(opp); > + > + mutex_lock(&drv->reg_lock); > + /* current opp item is changed */ > + if (freq == drv->pre_freq) { > + volt = dev_pm_opp_get_voltage(opp); > + mtk_ccifreq_set_voltage(drv, volt); > + } > + mutex_unlock(&drv->reg_lock); > + } > + > + return 0; > +} > + > +static struct devfreq_dev_profile mtk_ccifreq_profile = { > + .target = mtk_ccifreq_target, > +}; > + > +static int mtk_ccifreq_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct mtk_ccifreq_drv *drv; > + struct devfreq_passive_data *passive_data; > + struct dev_pm_opp *opp; > + unsigned long rate, opp_volt; > + int ret; > + > + drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL); > + if (!drv) > + return -ENOMEM; > + > + drv->dev = dev; > + drv->soc_data = (const struct mtk_ccifreq_platform_data *) > + of_device_get_match_data(&pdev->dev); > + mutex_init(&drv->reg_lock); > + platform_set_drvdata(pdev, drv); > + > + drv->cci_clk = devm_clk_get(dev, "cci"); > + if (IS_ERR(drv->cci_clk)) { > + ret = PTR_ERR(drv->cci_clk); > + return dev_err_probe(dev, ret, > + "failed to get cci clk: %d\n", > ret); > + } > + > + drv->inter_clk = devm_clk_get(dev, "intermediate"); > + if (IS_ERR(drv->inter_clk)) { > + ret = PTR_ERR(drv->inter_clk); > + dev_err_probe(dev, ret, > + "failed to get intermediate clk: %d\n", > ret); > + goto out_free_resources; > + } > + > + drv->proc_reg = devm_regulator_get_optional(dev, "proc"); > + if (IS_ERR(drv->proc_reg)) { > + ret = PTR_ERR(drv->proc_reg); > + dev_err_probe(dev, ret, > + "failed to get proc regulator: %d\n", > ret); > + goto out_free_resources; > + } > + > + ret = regulator_enable(drv->proc_reg); > + if (ret) { > + dev_err(dev, "failed to enable proc regulator\n"); > + goto out_free_resources; > + } > + > + drv->sram_reg = regulator_get_optional(dev, "sram"); > + if (IS_ERR(drv->sram_reg)) > + drv->sram_reg = NULL; > + else { > + ret = regulator_enable(drv->sram_reg); > + if (ret) { > + dev_err(dev, "failed to enable sram > regulator\n"); > + goto out_free_resources; > + } > + } > + > + /* > + * We assume min voltage is 0 and tracking target voltage using > + * min_volt_shift for each iteration. > + * The retry_max is 3 times of expeted iteration count. > + */ > + drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv->soc_data- > >sram_max_volt, > + drv->soc_data- > >proc_max_volt), > + drv->soc_data- > >min_volt_shift); > + > + ret = clk_prepare_enable(drv->cci_clk); > + if (ret) > + goto out_free_resources; > + > + ret = clk_prepare_enable(drv->inter_clk); > + if (ret) > + goto out_disable_cci_clk; > + > + ret = dev_pm_opp_of_add_table(dev); > + if (ret) { > + dev_err(dev, "failed to add opp table: %d\n", ret); > + goto out_disable_inter_clk; > + } > + > + rate = clk_get_rate(drv->inter_clk); > + opp = dev_pm_opp_find_freq_ceil(dev, &rate); > + if (IS_ERR(opp)) { > + ret = PTR_ERR(opp); > + dev_err(dev, "failed to get intermediate opp: %d\n", > ret); > + goto out_remove_opp_table; > + } > + drv->inter_voltage = dev_pm_opp_get_voltage(opp); > + dev_pm_opp_put(opp); > + > + rate = U32_MAX; > + opp = dev_pm_opp_find_freq_floor(drv->dev, &rate); > + if (IS_ERR(opp)) { > + dev_err(dev, "failed to get opp\n"); > + ret = PTR_ERR(opp); > + goto out_remove_opp_table; > + } > + > + opp_volt = dev_pm_opp_get_voltage(opp); > + dev_pm_opp_put(opp); > + ret = mtk_ccifreq_set_voltage(drv, opp_volt); > + if (ret) { > + dev_err(dev, "failed to scale to highest voltage %lu in > proc_reg\n", > + opp_volt); > + goto out_remove_opp_table; > + } > + > + passive_data = devm_kzalloc(dev, sizeof(struct > devfreq_passive_data), > + GFP_KERNEL); > + if (!passive_data) { > + ret = -ENOMEM; > + goto out_remove_opp_table; > + } > + > + passive_data->parent_type = CPUFREQ_PARENT_DEV; > + drv->devfreq = devm_devfreq_add_device(dev, > &mtk_ccifreq_profile, > + DEVFREQ_GOV_PASSIVE, > + passive_data); > + if (IS_ERR(drv->devfreq)) { > + ret = -EPROBE_DEFER; > + dev_err(dev, "failed to add devfreq device: %d\n", > + PTR_ERR(drv->devfreq)); > + goto out_remove_opp_table; > + } > + > + drv->opp_nb.notifier_call = mtk_ccifreq_opp_notifier; > + ret = dev_pm_opp_register_notifier(dev, &drv->opp_nb); > + if (ret) { > + dev_err(dev, "failed to register opp notifier: %d\n", > ret); > + goto out_remove_devfreq_device; > + } > + return 0; > + > +out_remove_devfreq_device: > + devm_devfreq_remove_device(dev, drv->devfreq); > + > +out_remove_opp_table: > + dev_pm_opp_of_remove_table(dev); > + > +out_disable_inter_clk: > + clk_disable_unprepare(drv->inter_clk); > + > +out_disable_cci_clk: > + clk_disable_unprepare(drv->cci_clk); > + > +out_free_resources: > + if (regulator_is_enabled(drv->proc_reg)) > + regulator_disable(drv->proc_reg); > + if (drv->sram_reg && regulator_is_enabled(drv->sram_reg)) > + regulator_disable(drv->sram_reg); > + > + if (!IS_ERR(drv->proc_reg)) > + regulator_put(drv->proc_reg); > + if (!IS_ERR(drv->sram_reg)) > + regulator_put(drv->sram_reg); > + if (!IS_ERR(drv->cci_clk)) > + clk_put(drv->cci_clk); > + if (!IS_ERR(drv->inter_clk)) > + clk_put(drv->inter_clk); > + > + return ret; > +} > + > +static int mtk_ccifreq_remove(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct mtk_ccifreq_drv *drv; > + > + drv = platform_get_drvdata(pdev); > + > + dev_pm_opp_unregister_notifier(dev, &drv->opp_nb); > + dev_pm_opp_of_remove_table(dev); > + clk_disable_unprepare(drv->inter_clk); > + clk_disable_unprepare(drv->cci_clk); > + regulator_disable(drv->proc_reg); > + if (drv->sram_reg) > + regulator_disable(drv->sram_reg); > + > + return 0; > +} > + > +static const struct mtk_ccifreq_platform_data mt8183_platform_data = > { > + .min_volt_shift = 100000, > + .max_volt_shift = 200000, > + .proc_max_volt = 1150000, > + .sram_min_volt = 0, > + .sram_max_volt = 1150000, > +}; > + > +static const struct mtk_ccifreq_platform_data mt8186_platform_data = > { > + .min_volt_shift = 100000, > + .max_volt_shift = 250000, > + .proc_max_volt = 1118750, > + .sram_min_volt = 850000, > + .sram_max_volt = 1118750, > +}; > + > +static const struct of_device_id mtk_ccifreq_machines[] = { > + { .compatible = "mediatek,mt8183-cci", .data = > &mt8183_platform_data }, > + { .compatible = "mediatek,mt8186-cci", .data = > &mt8186_platform_data }, > + { }, > +}; > +MODULE_DEVICE_TABLE(of, mtk_ccifreq_machines); > + > +static struct platform_driver mtk_ccifreq_platdrv = { > + .probe = mtk_ccifreq_probe, > + .remove = mtk_ccifreq_remove, > + .driver = { > + .name = "mtk-ccifreq", > + .of_match_table = mtk_ccifreq_machines, > + }, > +}; > +module_platform_driver(mtk_ccifreq_platdrv); > + > +MODULE_DESCRIPTION("MediaTek CCI devfreq driver"); > +MODULE_AUTHOR("Jia-Wei Chang <jia-wei.chang@mediatek.com>"); > +MODULE_LICENSE("GPL v2"); Hi Chanwoo, Just a kindly ping. Could you please give me some suggestion on this patch? Thanks! BRs, Johnson Wang
On Thu, Apr 28, 2022 at 7:39 PM Chen, Rong A <rong.a.chen@intel.com> wrote: > On 4/27/2022 6:11 PM, Johnson Wang wrote: > > On Wed, 2022-04-27 at 17:25 +0800, kernel test robot wrote: > >> Hi Johnson, > >> > >> Thank you for the patch! Perhaps something to improve: > >> > >> [auto build test WARNING on robh/for-next] > >> [also build test WARNING on linus/master v5.18-rc4 next-20220427] > >> [If your patch is applied to the wrong git tree, kindly drop us a > >> note. > >> And when submitting patch, we suggest to use '--base' as documented > >> in > >> > > https://urldefense.com/v3/__https://git-scm.com/docs/git-format-patch__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMS_s9zEZ$ > >> ] > >> > >> url: > >> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commits/Johnson-Wang/Introduce-MediaTek-CCI-devfreq-driver/20220425-205820__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMc1U_tqz$ > >> > >> base: > >> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMUzTprof$ > >> for-next > >> config: hexagon-allyesconfig ( > >> https://urldefense.com/v3/__https://download.01.org/0day-ci/archive/20220427/202204271737.oAuTwqZH-lkp@intel.com/config__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMaVRzbSL$ > >> ) > >> compiler: clang version 15.0.0 ( > >> https://urldefense.com/v3/__https://github.com/llvm/llvm-project__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMRqw5IY-$ > >> $ 1cddcfdc3c683b393df1a5c9063252eb60e52818) > >> reproduce (this is a W=1 build): > >> wget > >> https://urldefense.com/v3/__https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMQLiD-i9$ > >> -O ~/bin/make.cross > >> chmod +x ~/bin/make.cross > >> # > >> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commit/98b34c0587837b0e5b880b11a52433f8f0eee19f__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMU5yd7Y2$ > >> > >> git remote add linux-review > >> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMW4ldtnH$ > >> > >> git fetch --no-tags linux-review Johnson-Wang/Introduce- > >> MediaTek-CCI-devfreq-driver/20220425-205820 > >> git checkout 98b34c0587837b0e5b880b11a52433f8f0eee19f > >> # save the config file > >> mkdir build_dir && cp config build_dir/.config > >> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross > >> W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash block/partitions/ > >> drivers/devfreq/ drivers/iio/imu/ drivers/misc/lkdtm/ > >> > >> If you fix the issue, kindly add following tag as appropriate > >> Reported-by: kernel test robot <lkp@intel.com> > >> > >> All warnings (new ones prefixed by >>): > >> > >> drivers/devfreq/mtk-cci-devfreq.c:372:16: error: no member named > >> 'parent_type' in 'struct devfreq_passive_data' > >> passive_data->parent_type = CPUFREQ_PARENT_DEV; > >> ~~~~~~~~~~~~ ^ > >> drivers/devfreq/mtk-cci-devfreq.c:372:30: error: use of undeclared > >> identifier 'CPUFREQ_PARENT_DEV' > >> passive_data->parent_type = CPUFREQ_PARENT_DEV; > >> ^ > >>>> drivers/devfreq/mtk-cci-devfreq.c:379:4: warning: format > >>>> specifies type 'int' but the argument has type 'long' [-Wformat] > >> > >> PTR_ERR(drv->devfreq)); > >> ^~~~~~~~~~~~~~~~~~~~~ > >> include/linux/dev_printk.h:144:65: note: expanded from macro > >> 'dev_err' > >> dev_printk_index_wrap(_dev_err, KERN_ERR, dev, > >> dev_fmt(fmt), ##__VA_ARGS__) > >> ~~~ > >> ^~~~~~~~~~~ > >> include/linux/dev_printk.h:110:23: note: expanded from macro > >> 'dev_printk_index_wrap' > >> _p_func(dev, fmt, > >> ##__VA_ARGS__); \ > >> ~~~ ^~~~~~~~~~~ > >> 1 warning and 2 errors generated. > >> > >> > >> vim +379 drivers/devfreq/mtk-cci-devfreq.c > >> > >> 255 > >> 256 static int mtk_ccifreq_probe(struct platform_device > >> *pdev) > >> 257 { > >> 258 struct device *dev = &pdev->dev; > >> 259 struct mtk_ccifreq_drv *drv; > >> 260 struct devfreq_passive_data *passive_data; > >> 261 struct dev_pm_opp *opp; > >> 262 unsigned long rate, opp_volt; > >> 263 int ret; > >> 264 > >> 265 drv = devm_kzalloc(dev, sizeof(*drv), > >> GFP_KERNEL); > >> 266 if (!drv) > >> 267 return -ENOMEM; > >> 268 > >> 269 drv->dev = dev; > >> 270 drv->soc_data = (const struct > >> mtk_ccifreq_platform_data *) > >> 271 of_device_get_match_dat > >> a(&pdev->dev); > >> 272 mutex_init(&drv->reg_lock); > >> 273 platform_set_drvdata(pdev, drv); > >> 274 > >> 275 drv->cci_clk = devm_clk_get(dev, "cci"); > >> 276 if (IS_ERR(drv->cci_clk)) { > >> 277 ret = PTR_ERR(drv->cci_clk); > >> 278 return dev_err_probe(dev, ret, > >> 279 "failed to get cci > >> clk: %d\n", ret); > >> 280 } > >> 281 > >> 282 drv->inter_clk = devm_clk_get(dev, > >> "intermediate"); > >> 283 if (IS_ERR(drv->inter_clk)) { > >> 284 ret = PTR_ERR(drv->inter_clk); > >> 285 dev_err_probe(dev, ret, > >> 286 "failed to get > >> intermediate clk: %d\n", ret); > >> 287 goto out_free_resources; > >> 288 } > >> 289 > >> 290 drv->proc_reg = > >> devm_regulator_get_optional(dev, "proc"); > >> 291 if (IS_ERR(drv->proc_reg)) { > >> 292 ret = PTR_ERR(drv->proc_reg); > >> 293 dev_err_probe(dev, ret, > >> 294 "failed to get proc > >> regulator: %d\n", ret); > >> 295 goto out_free_resources; > >> 296 } > >> 297 > >> 298 ret = regulator_enable(drv->proc_reg); > >> 299 if (ret) { > >> 300 dev_err(dev, "failed to enable proc > >> regulator\n"); > >> 301 goto out_free_resources; > >> 302 } > >> 303 > >> 304 drv->sram_reg = regulator_get_optional(dev, > >> "sram"); > >> 305 if (IS_ERR(drv->sram_reg)) > >> 306 drv->sram_reg = NULL; > >> 307 else { > >> 308 ret = regulator_enable(drv->sram_reg); > >> 309 if (ret) { > >> 310 dev_err(dev, "failed to enable > >> sram regulator\n"); > >> 311 goto out_free_resources; > >> 312 } > >> 313 } > >> 314 > >> 315 /* > >> 316 * We assume min voltage is 0 and tracking > >> target voltage using > >> 317 * min_volt_shift for each iteration. > >> 318 * The retry_max is 3 times of expeted > >> iteration count. > >> 319 */ > >> 320 drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv- > >>> soc_data->sram_max_volt, > >> 321 drv- > >>> soc_data->proc_max_volt), > >> 322 drv- > >>> soc_data->min_volt_shift); > >> 323 > >> 324 ret = clk_prepare_enable(drv->cci_clk); > >> 325 if (ret) > >> 326 goto out_free_resources; > >> 327 > >> 328 ret = clk_prepare_enable(drv->inter_clk); > >> 329 if (ret) > >> 330 goto out_disable_cci_clk; > >> 331 > >> 332 ret = dev_pm_opp_of_add_table(dev); > >> 333 if (ret) { > >> 334 dev_err(dev, "failed to add opp table: > >> %d\n", ret); > >> 335 goto out_disable_inter_clk; > >> 336 } > >> 337 > >> 338 rate = clk_get_rate(drv->inter_clk); > >> 339 opp = dev_pm_opp_find_freq_ceil(dev, &rate); > >> 340 if (IS_ERR(opp)) { > >> 341 ret = PTR_ERR(opp); > >> 342 dev_err(dev, "failed to get > >> intermediate opp: %d\n", ret); > >> 343 goto out_remove_opp_table; > >> 344 } > >> 345 drv->inter_voltage = > >> dev_pm_opp_get_voltage(opp); > >> 346 dev_pm_opp_put(opp); > >> 347 > >> 348 rate = U32_MAX; > >> 349 opp = dev_pm_opp_find_freq_floor(drv->dev, > >> &rate); > >> 350 if (IS_ERR(opp)) { > >> 351 dev_err(dev, "failed to get opp\n"); > >> 352 ret = PTR_ERR(opp); > >> 353 goto out_remove_opp_table; > >> 354 } > >> 355 > >> 356 opp_volt = dev_pm_opp_get_voltage(opp); > >> 357 dev_pm_opp_put(opp); > >> 358 ret = mtk_ccifreq_set_voltage(drv, opp_volt); > >> 359 if (ret) { > >> 360 dev_err(dev, "failed to scale to > >> highest voltage %lu in proc_reg\n", > >> 361 opp_volt); > >> 362 goto out_remove_opp_table; > >> 363 } > >> 364 > >> 365 passive_data = devm_kzalloc(dev, sizeof(struct > >> devfreq_passive_data), > >> 366 GFP_KERNEL); > >> 367 if (!passive_data) { > >> 368 ret = -ENOMEM; > >> 369 goto out_remove_opp_table; > >> 370 } > >> 371 > >> 372 passive_data->parent_type = CPUFREQ_PARENT_DEV; > >> 373 drv->devfreq = devm_devfreq_add_device(dev, > >> &mtk_ccifreq_profile, > >> 374 DEVFREQ_ > >> GOV_PASSIVE, > >> 375 passive_ > >> data); > >> 376 if (IS_ERR(drv->devfreq)) { > >> 377 ret = -EPROBE_DEFER; > >> 378 dev_err(dev, "failed to add devfreq > >> device: %d\n", > >> > 379 PTR_ERR(drv->devfreq)); > >> 380 goto out_remove_opp_table; > >> 381 } > >> 382 > >> 383 drv->opp_nb.notifier_call = > >> mtk_ccifreq_opp_notifier; > >> 384 ret = dev_pm_opp_register_notifier(dev, &drv- > >>> opp_nb); > >> 385 if (ret) { > >> 386 dev_err(dev, "failed to register opp > >> notifier: %d\n", ret); > >> 387 goto out_remove_devfreq_device; > >> 388 } > >> 389 return 0; > >> 390 > >> 391 out_remove_devfreq_device: > >> 392 devm_devfreq_remove_device(dev, drv->devfreq); > >> 393 > >> 394 out_remove_opp_table: > >> 395 dev_pm_opp_of_remove_table(dev); > >> 396 > >> 397 out_disable_inter_clk: > >> 398 clk_disable_unprepare(drv->inter_clk); > >> 399 > >> 400 out_disable_cci_clk: > >> 401 clk_disable_unprepare(drv->cci_clk); > >> 402 > >> 403 out_free_resources: > >> 404 if (regulator_is_enabled(drv->proc_reg)) > >> 405 regulator_disable(drv->proc_reg); > >> 406 if (drv->sram_reg && regulator_is_enabled(drv- > >>> sram_reg)) > >> 407 regulator_disable(drv->sram_reg); > >> 408 > >> 409 if (!IS_ERR(drv->proc_reg)) > >> 410 regulator_put(drv->proc_reg); > >> 411 if (!IS_ERR(drv->sram_reg)) > >> 412 regulator_put(drv->sram_reg); > >> 413 if (!IS_ERR(drv->cci_clk)) > >> 414 clk_put(drv->cci_clk); > >> 415 if (!IS_ERR(drv->inter_clk)) > >> 416 clk_put(drv->inter_clk); > >> 417 > >> 418 return ret; > >> 419 } > >> 420 > >> > > > > Hi "kernel test robot", > > > > Thanks for your review. > > > > This patch is based on chanwoo/devfreq-testing[1] > > [1] > > https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing > > Hi Johnson, > > Thanks for the feedback, we'll take a look too. I think the last patch on that branch might be broken. ChenYu
Hi Johnson, On 22. 4. 25. 21:55, Johnson Wang wrote: > We introduce a devfreq driver for the MediaTek Cache Coherent Interconnect > (CCI) used by some MediaTek SoCs. > > In this driver, we use the passive devfreq driver to get target frequencies > and adjust voltages accordingly. In MT8183 and MT8186, the MediaTek CCI > is supplied by the same regulators with the little core CPUs. > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > --- > This patch depends on "devfreq-testing"[1]. > [1]https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing > --- > drivers/devfreq/Kconfig | 10 + > drivers/devfreq/Makefile | 1 + > drivers/devfreq/mtk-cci-devfreq.c | 474 ++++++++++++++++++++++++++++++ > 3 files changed, 485 insertions(+) > create mode 100644 drivers/devfreq/mtk-cci-devfreq.c Acked-by: Chanwoo Choi <cw00.choi@samsung.com> I updated the passive governor on devfreq-testing[1] branch [1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing Could you please test your patches based on devfreq-testing[1] branch?
On 22. 5. 6. 20:38, Johnson Wang wrote: > On Mon, 2022-04-25 at 20:55 +0800, Johnson Wang wrote: >> We introduce a devfreq driver for the MediaTek Cache Coherent >> Interconnect >> (CCI) used by some MediaTek SoCs. >> >> In this driver, we use the passive devfreq driver to get target >> frequencies >> and adjust voltages accordingly. In MT8183 and MT8186, the MediaTek >> CCI >> is supplied by the same regulators with the little core CPUs. >> >> Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> >> Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> >> --- >> This patch depends on "devfreq-testing"[1]. >> [1] >> https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing >> --- >> drivers/devfreq/Kconfig | 10 + >> drivers/devfreq/Makefile | 1 + >> drivers/devfreq/mtk-cci-devfreq.c | 474 >> ++++++++++++++++++++++++++++++ >> 3 files changed, 485 insertions(+) >> create mode 100644 drivers/devfreq/mtk-cci-devfreq.c >> >> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig >> index 87eb2b837e68..9754d8b31621 100644 >> --- a/drivers/devfreq/Kconfig >> +++ b/drivers/devfreq/Kconfig >> @@ -120,6 +120,16 @@ config ARM_TEGRA_DEVFREQ >> It reads ACTMON counters of memory controllers and adjusts >> the >> operating frequencies and voltages with OPP support. >> >> +config ARM_MEDIATEK_CCI_DEVFREQ >> + tristate "MEDIATEK CCI DEVFREQ Driver" >> + depends on ARM_MEDIATEK_CPUFREQ || COMPILE_TEST >> + select DEVFREQ_GOV_PASSIVE >> + help >> + This adds a devfreq driver for MediaTek Cache Coherent >> Interconnect >> + which is shared the same regulators with the cpu cluster. It >> can track >> + buck voltages and update a proper CCI frequency. Use the >> notification >> + to get the regulator status. >> + >> config ARM_RK3399_DMC_DEVFREQ >> tristate "ARM RK3399 DMC DEVFREQ Driver" >> depends on (ARCH_ROCKCHIP && HAVE_ARM_SMCCC) || \ >> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile >> index 0b6be92a25d9..bf40d04928d0 100644 >> --- a/drivers/devfreq/Makefile >> +++ b/drivers/devfreq/Makefile >> @@ -11,6 +11,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += >> governor_passive.o >> obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o >> obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ) += imx-bus.o >> obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ) += imx8m-ddrc.o >> +obj-$(CONFIG_ARM_MEDIATEK_CCI_DEVFREQ) += mtk-cci-devfreq.o >> obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o >> obj-$(CONFIG_ARM_SUN8I_A33_MBUS_DEVFREQ) += sun8i-a33-mbus.o >> obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30-devfreq.o >> diff --git a/drivers/devfreq/mtk-cci-devfreq.c b/drivers/devfreq/mtk- >> cci-devfreq.c >> new file mode 100644 >> index 000000000000..b3e31c45a57c >> --- /dev/null >> +++ b/drivers/devfreq/mtk-cci-devfreq.c >> @@ -0,0 +1,474 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/* >> + * Copyright (C) 2022 MediaTek Inc. >> + */ >> + >> +#include <linux/clk.h> >> +#include <linux/devfreq.h> >> +#include <linux/minmax.h> >> +#include <linux/module.h> >> +#include <linux/of.h> >> +#include <linux/of_device.h> >> +#include <linux/platform_device.h> >> +#include <linux/pm_opp.h> >> +#include <linux/regulator/consumer.h> >> + >> +struct mtk_ccifreq_platform_data { >> + int min_volt_shift; >> + int max_volt_shift; >> + int proc_max_volt; >> + int sram_min_volt; >> + int sram_max_volt; >> +}; >> + >> +struct mtk_ccifreq_drv { >> + struct device *dev; >> + struct devfreq *devfreq; >> + struct regulator *proc_reg; >> + struct regulator *sram_reg; >> + struct clk *cci_clk; >> + struct clk *inter_clk; >> + int inter_voltage; >> + int pre_voltage; >> + unsigned long pre_freq; >> + /* Avoid race condition for regulators between notify and >> policy */ >> + struct mutex reg_lock; >> + struct notifier_block opp_nb; >> + const struct mtk_ccifreq_platform_data *soc_data; >> + int vtrack_max; >> +}; >> + >> +static int mtk_ccifreq_set_voltage(struct mtk_ccifreq_drv *drv, int >> new_voltage) >> +{ >> + const struct mtk_ccifreq_platform_data *soc_data = drv- >>> soc_data; >> + struct device *dev = drv->dev; >> + int pre_voltage, pre_vsram, new_vsram, vsram, voltage, ret; >> + int retry_max = drv->vtrack_max; >> + >> + if (!drv->sram_reg) { >> + ret = regulator_set_voltage(drv->proc_reg, new_voltage, >> + drv->soc_data- >>> proc_max_volt); >> + goto out_set_voltage; >> + } >> + >> + pre_voltage = regulator_get_voltage(drv->proc_reg); >> + if (pre_voltage < 0) { >> + dev_err(dev, "invalid vproc value: %d\n", pre_voltage); >> + return pre_voltage; >> + } >> + >> + pre_vsram = regulator_get_voltage(drv->sram_reg); >> + if (pre_vsram < 0) { >> + dev_err(dev, "invalid vsram value: %d\n", pre_vsram); >> + return pre_vsram; >> + } >> + >> + new_vsram = clamp(new_voltage + soc_data->min_volt_shift, >> + soc_data->sram_min_volt, soc_data- >>> sram_max_volt); >> + >> + do { >> + if (pre_voltage <= new_voltage) { >> + vsram = clamp(pre_voltage + soc_data- >>> max_volt_shift, >> + soc_data->sram_min_volt, >> new_vsram); >> + ret = regulator_set_voltage(drv->sram_reg, >> vsram, >> + soc_data- >>> sram_max_volt); >> + if (ret) >> + return ret; >> + >> + if (vsram == soc_data->sram_max_volt || >> + new_vsram == soc_data->sram_min_volt) >> + voltage = new_voltage; >> + else >> + voltage = vsram - soc_data- >>> min_volt_shift; >> + >> + ret = regulator_set_voltage(drv->proc_reg, >> voltage, >> + soc_data- >>> proc_max_volt); >> + if (ret) { >> + regulator_set_voltage(drv->sram_reg, >> pre_vsram, >> + soc_data- >>> sram_max_volt); >> + return ret; >> + } >> + } else if (pre_voltage > new_voltage) { >> + voltage = max(new_voltage, >> + pre_vsram - soc_data- >>> max_volt_shift); >> + ret = regulator_set_voltage(drv->proc_reg, >> voltage, >> + soc_data- >>> proc_max_volt); >> + if (ret) >> + return ret; >> + >> + if (voltage == new_voltage) >> + vsram = new_vsram; >> + else >> + vsram = max(new_vsram, >> + voltage + soc_data- >>> min_volt_shift); >> + >> + ret = regulator_set_voltage(drv->sram_reg, >> vsram, >> + soc_data- >>> sram_max_volt); >> + if (ret) { >> + regulator_set_voltage(drv->proc_reg, >> pre_voltage, >> + soc_data- >>> proc_max_volt); >> + return ret; >> + } >> + } >> + >> + pre_voltage = voltage; >> + pre_vsram = vsram; >> + >> + if (--retry_max < 0) { >> + dev_err(dev, >> + "over loop count, failed to set >> voltage\n"); >> + return -EINVAL; >> + } >> + } while (voltage != new_voltage || vsram != new_vsram); >> + >> +out_set_voltage: >> + if (!ret) >> + drv->pre_voltage = new_voltage; >> + >> + return ret; >> +} >> + >> +static int mtk_ccifreq_target(struct device *dev, unsigned long >> *freq, >> + u32 flags) >> +{ >> + struct mtk_ccifreq_drv *drv = dev_get_drvdata(dev); >> + struct clk *cci_pll = clk_get_parent(drv->cci_clk); >> + struct dev_pm_opp *opp; >> + unsigned long opp_rate; >> + int voltage, pre_voltage, inter_voltage, target_voltage, ret; >> + >> + if (!drv) >> + return -EINVAL; >> + >> + if (drv->pre_freq == *freq) >> + return 0; >> + >> + inter_voltage = drv->inter_voltage; >> + >> + opp_rate = *freq; >> + opp = devfreq_recommended_opp(dev, &opp_rate, 1); >> + if (IS_ERR(opp)) { >> + dev_err(dev, "failed to find opp for freq: %ld\n", >> opp_rate); >> + return PTR_ERR(opp); >> + } >> + >> + mutex_lock(&drv->reg_lock); >> + >> + voltage = dev_pm_opp_get_voltage(opp); >> + dev_pm_opp_put(opp); >> + >> + if (unlikely(drv->pre_voltage <= 0)) >> + pre_voltage = regulator_get_voltage(drv->proc_reg); >> + else >> + pre_voltage = drv->pre_voltage; >> + >> + if (pre_voltage < 0) { >> + dev_err(dev, "invalid vproc value: %d\n", pre_voltage); >> + return pre_voltage; >> + } >> + >> + /* scale up: set voltage first then freq. */ >> + target_voltage = max(inter_voltage, voltage); >> + if (pre_voltage <= target_voltage) { >> + ret = mtk_ccifreq_set_voltage(drv, target_voltage); >> + if (ret) { >> + dev_err(dev, "failed to scale up voltage\n"); >> + goto out_restore_voltage; >> + } >> + } >> + >> + /* switch the cci clock to intermediate clock source. */ >> + ret = clk_set_parent(drv->cci_clk, drv->inter_clk); >> + if (ret) { >> + dev_err(dev, "failed to re-parent cci clock\n"); >> + goto out_restore_voltage; >> + } >> + >> + /* set the original clock to target rate. */ >> + ret = clk_set_rate(cci_pll, *freq); >> + if (ret) { >> + dev_err(dev, "failed to set cci pll rate: %d\n", ret); >> + clk_set_parent(drv->cci_clk, cci_pll); >> + goto out_restore_voltage; >> + } >> + >> + /* switch the cci clock back to the original clock source. */ >> + ret = clk_set_parent(drv->cci_clk, cci_pll); >> + if (ret) { >> + dev_err(dev, "failed to re-parent cci clock\n"); >> + mtk_ccifreq_set_voltage(drv, inter_voltage); >> + goto out_unlock; >> + } >> + >> + /* >> + * If the new voltage is lower than the intermediate voltage or >> the >> + * original voltage, scale down to the new voltage. >> + */ >> + if (voltage < inter_voltage || voltage < pre_voltage) { >> + ret = mtk_ccifreq_set_voltage(drv, voltage); >> + if (ret) { >> + dev_err(dev, "failed to scale down voltage\n"); >> + goto out_unlock; >> + } >> + } >> + >> + drv->pre_freq = *freq; >> + mutex_unlock(&drv->reg_lock); >> + >> + return 0; >> + >> +out_restore_voltage: >> + mtk_ccifreq_set_voltage(drv, pre_voltage); >> + >> +out_unlock: >> + mutex_unlock(&drv->reg_lock); >> + return ret; >> +} >> + >> +static int mtk_ccifreq_opp_notifier(struct notifier_block *nb, >> + unsigned long event, void *data) >> +{ >> + struct dev_pm_opp *opp = data; >> + struct mtk_ccifreq_drv *drv; >> + unsigned long freq, volt; >> + >> + drv = container_of(nb, struct mtk_ccifreq_drv, opp_nb); >> + >> + if (event == OPP_EVENT_ADJUST_VOLTAGE) { >> + freq = dev_pm_opp_get_freq(opp); >> + >> + mutex_lock(&drv->reg_lock); >> + /* current opp item is changed */ >> + if (freq == drv->pre_freq) { >> + volt = dev_pm_opp_get_voltage(opp); >> + mtk_ccifreq_set_voltage(drv, volt); >> + } >> + mutex_unlock(&drv->reg_lock); >> + } >> + >> + return 0; >> +} >> + >> +static struct devfreq_dev_profile mtk_ccifreq_profile = { >> + .target = mtk_ccifreq_target, >> +}; >> + >> +static int mtk_ccifreq_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct mtk_ccifreq_drv *drv; >> + struct devfreq_passive_data *passive_data; >> + struct dev_pm_opp *opp; >> + unsigned long rate, opp_volt; >> + int ret; >> + >> + drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL); >> + if (!drv) >> + return -ENOMEM; >> + >> + drv->dev = dev; >> + drv->soc_data = (const struct mtk_ccifreq_platform_data *) >> + of_device_get_match_data(&pdev->dev); >> + mutex_init(&drv->reg_lock); >> + platform_set_drvdata(pdev, drv); >> + >> + drv->cci_clk = devm_clk_get(dev, "cci"); >> + if (IS_ERR(drv->cci_clk)) { >> + ret = PTR_ERR(drv->cci_clk); >> + return dev_err_probe(dev, ret, >> + "failed to get cci clk: %d\n", >> ret); >> + } >> + >> + drv->inter_clk = devm_clk_get(dev, "intermediate"); >> + if (IS_ERR(drv->inter_clk)) { >> + ret = PTR_ERR(drv->inter_clk); >> + dev_err_probe(dev, ret, >> + "failed to get intermediate clk: %d\n", >> ret); >> + goto out_free_resources; >> + } >> + >> + drv->proc_reg = devm_regulator_get_optional(dev, "proc"); >> + if (IS_ERR(drv->proc_reg)) { >> + ret = PTR_ERR(drv->proc_reg); >> + dev_err_probe(dev, ret, >> + "failed to get proc regulator: %d\n", >> ret); >> + goto out_free_resources; >> + } >> + >> + ret = regulator_enable(drv->proc_reg); >> + if (ret) { >> + dev_err(dev, "failed to enable proc regulator\n"); >> + goto out_free_resources; >> + } >> + >> + drv->sram_reg = regulator_get_optional(dev, "sram"); >> + if (IS_ERR(drv->sram_reg)) >> + drv->sram_reg = NULL; >> + else { >> + ret = regulator_enable(drv->sram_reg); >> + if (ret) { >> + dev_err(dev, "failed to enable sram >> regulator\n"); >> + goto out_free_resources; >> + } >> + } >> + >> + /* >> + * We assume min voltage is 0 and tracking target voltage using >> + * min_volt_shift for each iteration. >> + * The retry_max is 3 times of expeted iteration count. >> + */ >> + drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv->soc_data- >>> sram_max_volt, >> + drv->soc_data- >>> proc_max_volt), >> + drv->soc_data- >>> min_volt_shift); >> + >> + ret = clk_prepare_enable(drv->cci_clk); >> + if (ret) >> + goto out_free_resources; >> + >> + ret = clk_prepare_enable(drv->inter_clk); >> + if (ret) >> + goto out_disable_cci_clk; >> + >> + ret = dev_pm_opp_of_add_table(dev); >> + if (ret) { >> + dev_err(dev, "failed to add opp table: %d\n", ret); >> + goto out_disable_inter_clk; >> + } >> + >> + rate = clk_get_rate(drv->inter_clk); >> + opp = dev_pm_opp_find_freq_ceil(dev, &rate); >> + if (IS_ERR(opp)) { >> + ret = PTR_ERR(opp); >> + dev_err(dev, "failed to get intermediate opp: %d\n", >> ret); >> + goto out_remove_opp_table; >> + } >> + drv->inter_voltage = dev_pm_opp_get_voltage(opp); >> + dev_pm_opp_put(opp); >> + >> + rate = U32_MAX; >> + opp = dev_pm_opp_find_freq_floor(drv->dev, &rate); >> + if (IS_ERR(opp)) { >> + dev_err(dev, "failed to get opp\n"); >> + ret = PTR_ERR(opp); >> + goto out_remove_opp_table; >> + } >> + >> + opp_volt = dev_pm_opp_get_voltage(opp); >> + dev_pm_opp_put(opp); >> + ret = mtk_ccifreq_set_voltage(drv, opp_volt); >> + if (ret) { >> + dev_err(dev, "failed to scale to highest voltage %lu in >> proc_reg\n", >> + opp_volt); >> + goto out_remove_opp_table; >> + } >> + >> + passive_data = devm_kzalloc(dev, sizeof(struct >> devfreq_passive_data), >> + GFP_KERNEL); >> + if (!passive_data) { >> + ret = -ENOMEM; >> + goto out_remove_opp_table; >> + } >> + >> + passive_data->parent_type = CPUFREQ_PARENT_DEV; >> + drv->devfreq = devm_devfreq_add_device(dev, >> &mtk_ccifreq_profile, >> + DEVFREQ_GOV_PASSIVE, >> + passive_data); >> + if (IS_ERR(drv->devfreq)) { >> + ret = -EPROBE_DEFER; >> + dev_err(dev, "failed to add devfreq device: %d\n", >> + PTR_ERR(drv->devfreq)); >> + goto out_remove_opp_table; >> + } >> + >> + drv->opp_nb.notifier_call = mtk_ccifreq_opp_notifier; >> + ret = dev_pm_opp_register_notifier(dev, &drv->opp_nb); >> + if (ret) { >> + dev_err(dev, "failed to register opp notifier: %d\n", >> ret); >> + goto out_remove_devfreq_device; >> + } >> + return 0; >> + >> +out_remove_devfreq_device: >> + devm_devfreq_remove_device(dev, drv->devfreq); >> + >> +out_remove_opp_table: >> + dev_pm_opp_of_remove_table(dev); >> + >> +out_disable_inter_clk: >> + clk_disable_unprepare(drv->inter_clk); >> + >> +out_disable_cci_clk: >> + clk_disable_unprepare(drv->cci_clk); >> + >> +out_free_resources: >> + if (regulator_is_enabled(drv->proc_reg)) >> + regulator_disable(drv->proc_reg); >> + if (drv->sram_reg && regulator_is_enabled(drv->sram_reg)) >> + regulator_disable(drv->sram_reg); >> + >> + if (!IS_ERR(drv->proc_reg)) >> + regulator_put(drv->proc_reg); >> + if (!IS_ERR(drv->sram_reg)) >> + regulator_put(drv->sram_reg); >> + if (!IS_ERR(drv->cci_clk)) >> + clk_put(drv->cci_clk); >> + if (!IS_ERR(drv->inter_clk)) >> + clk_put(drv->inter_clk); >> + >> + return ret; >> +} >> + >> +static int mtk_ccifreq_remove(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct mtk_ccifreq_drv *drv; >> + >> + drv = platform_get_drvdata(pdev); >> + >> + dev_pm_opp_unregister_notifier(dev, &drv->opp_nb); >> + dev_pm_opp_of_remove_table(dev); >> + clk_disable_unprepare(drv->inter_clk); >> + clk_disable_unprepare(drv->cci_clk); >> + regulator_disable(drv->proc_reg); >> + if (drv->sram_reg) >> + regulator_disable(drv->sram_reg); >> + >> + return 0; >> +} >> + >> +static const struct mtk_ccifreq_platform_data mt8183_platform_data = >> { >> + .min_volt_shift = 100000, >> + .max_volt_shift = 200000, >> + .proc_max_volt = 1150000, >> + .sram_min_volt = 0, >> + .sram_max_volt = 1150000, >> +}; >> + >> +static const struct mtk_ccifreq_platform_data mt8186_platform_data = >> { >> + .min_volt_shift = 100000, >> + .max_volt_shift = 250000, >> + .proc_max_volt = 1118750, >> + .sram_min_volt = 850000, >> + .sram_max_volt = 1118750, >> +}; >> + >> +static const struct of_device_id mtk_ccifreq_machines[] = { >> + { .compatible = "mediatek,mt8183-cci", .data = >> &mt8183_platform_data }, >> + { .compatible = "mediatek,mt8186-cci", .data = >> &mt8186_platform_data }, >> + { }, >> +}; >> +MODULE_DEVICE_TABLE(of, mtk_ccifreq_machines); >> + >> +static struct platform_driver mtk_ccifreq_platdrv = { >> + .probe = mtk_ccifreq_probe, >> + .remove = mtk_ccifreq_remove, >> + .driver = { >> + .name = "mtk-ccifreq", >> + .of_match_table = mtk_ccifreq_machines, >> + }, >> +}; >> +module_platform_driver(mtk_ccifreq_platdrv); >> + >> +MODULE_DESCRIPTION("MediaTek CCI devfreq driver"); >> +MODULE_AUTHOR("Jia-Wei Chang <jia-wei.chang@mediatek.com>"); >> +MODULE_LICENSE("GPL v2"); > > Hi Chanwoo, > > Just a kindly ping. > Could you please give me some suggestion on this patch? > Thanks! > > BRs, > Johnson Wang > Hi Johnson, Sorry for late reply.But I replied it yesterday as following: Maybe, this reply[1] has not yet arrrived at your mail box. [1] https://lore.kernel.org/lkml/0846a062-2ea2-e6d4-03c8-992c2f2e24a0@gmail.com/T/#m3bc609d805d6e74ec7a105be0511926b4afbbaef As I described on reply[1], I updated the patches on devfreq-testing branch. Could you please test your patches based on devfreq-testing branch?
Hi Chen-Yu, On Mon, May 9, 2022 at 10:53 AM Chen-Yu Tsai <wenst@chromium.org> wrote: > > On Thu, Apr 28, 2022 at 7:39 PM Chen, Rong A <rong.a.chen@intel.com> wrote: > > On 4/27/2022 6:11 PM, Johnson Wang wrote: > > > On Wed, 2022-04-27 at 17:25 +0800, kernel test robot wrote: > > >> Hi Johnson, > > >> > > >> Thank you for the patch! Perhaps something to improve: > > >> > > >> [auto build test WARNING on robh/for-next] > > >> [also build test WARNING on linus/master v5.18-rc4 next-20220427] > > >> [If your patch is applied to the wrong git tree, kindly drop us a > > >> note. > > >> And when submitting patch, we suggest to use '--base' as documented > > >> in > > >> > > > https://urldefense.com/v3/__https://git-scm.com/docs/git-format-patch__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMS_s9zEZ$ > > >> ] > > >> > > >> url: > > >> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commits/Johnson-Wang/Introduce-MediaTek-CCI-devfreq-driver/20220425-205820__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMc1U_tqz$ > > >> > > >> base: > > >> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMUzTprof$ > > >> for-next > > >> config: hexagon-allyesconfig ( > > >> https://urldefense.com/v3/__https://download.01.org/0day-ci/archive/20220427/202204271737.oAuTwqZH-lkp@intel.com/config__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMaVRzbSL$ > > >> ) > > >> compiler: clang version 15.0.0 ( > > >> https://urldefense.com/v3/__https://github.com/llvm/llvm-project__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMRqw5IY-$ > > >> $ 1cddcfdc3c683b393df1a5c9063252eb60e52818) > > >> reproduce (this is a W=1 build): > > >> wget > > >> https://urldefense.com/v3/__https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMQLiD-i9$ > > >> -O ~/bin/make.cross > > >> chmod +x ~/bin/make.cross > > >> # > > >> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux/commit/98b34c0587837b0e5b880b11a52433f8f0eee19f__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMU5yd7Y2$ > > >> > > >> git remote add linux-review > > >> https://urldefense.com/v3/__https://github.com/intel-lab-lkp/linux__;!!CTRNKA9wMg0ARbw!wdyoWXNLBcYM97vMuNFQXZ9BaajEp-Kmh5-xrvU2Rlmb0o-b9tRvCD0cPzbLMW4ldtnH$ > > >> > > >> git fetch --no-tags linux-review Johnson-Wang/Introduce- > > >> MediaTek-CCI-devfreq-driver/20220425-205820 > > >> git checkout 98b34c0587837b0e5b880b11a52433f8f0eee19f > > >> # save the config file > > >> mkdir build_dir && cp config build_dir/.config > > >> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross > > >> W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash block/partitions/ > > >> drivers/devfreq/ drivers/iio/imu/ drivers/misc/lkdtm/ > > >> > > >> If you fix the issue, kindly add following tag as appropriate > > >> Reported-by: kernel test robot <lkp@intel.com> > > >> > > >> All warnings (new ones prefixed by >>): > > >> > > >> drivers/devfreq/mtk-cci-devfreq.c:372:16: error: no member named > > >> 'parent_type' in 'struct devfreq_passive_data' > > >> passive_data->parent_type = CPUFREQ_PARENT_DEV; > > >> ~~~~~~~~~~~~ ^ > > >> drivers/devfreq/mtk-cci-devfreq.c:372:30: error: use of undeclared > > >> identifier 'CPUFREQ_PARENT_DEV' > > >> passive_data->parent_type = CPUFREQ_PARENT_DEV; > > >> ^ > > >>>> drivers/devfreq/mtk-cci-devfreq.c:379:4: warning: format > > >>>> specifies type 'int' but the argument has type 'long' [-Wformat] > > >> > > >> PTR_ERR(drv->devfreq)); > > >> ^~~~~~~~~~~~~~~~~~~~~ > > >> include/linux/dev_printk.h:144:65: note: expanded from macro > > >> 'dev_err' > > >> dev_printk_index_wrap(_dev_err, KERN_ERR, dev, > > >> dev_fmt(fmt), ##__VA_ARGS__) > > >> ~~~ > > >> ^~~~~~~~~~~ > > >> include/linux/dev_printk.h:110:23: note: expanded from macro > > >> 'dev_printk_index_wrap' > > >> _p_func(dev, fmt, > > >> ##__VA_ARGS__); \ > > >> ~~~ ^~~~~~~~~~~ > > >> 1 warning and 2 errors generated. > > >> > > >> > > >> vim +379 drivers/devfreq/mtk-cci-devfreq.c > > >> > > >> 255 > > >> 256 static int mtk_ccifreq_probe(struct platform_device > > >> *pdev) > > >> 257 { > > >> 258 struct device *dev = &pdev->dev; > > >> 259 struct mtk_ccifreq_drv *drv; > > >> 260 struct devfreq_passive_data *passive_data; > > >> 261 struct dev_pm_opp *opp; > > >> 262 unsigned long rate, opp_volt; > > >> 263 int ret; > > >> 264 > > >> 265 drv = devm_kzalloc(dev, sizeof(*drv), > > >> GFP_KERNEL); > > >> 266 if (!drv) > > >> 267 return -ENOMEM; > > >> 268 > > >> 269 drv->dev = dev; > > >> 270 drv->soc_data = (const struct > > >> mtk_ccifreq_platform_data *) > > >> 271 of_device_get_match_dat > > >> a(&pdev->dev); > > >> 272 mutex_init(&drv->reg_lock); > > >> 273 platform_set_drvdata(pdev, drv); > > >> 274 > > >> 275 drv->cci_clk = devm_clk_get(dev, "cci"); > > >> 276 if (IS_ERR(drv->cci_clk)) { > > >> 277 ret = PTR_ERR(drv->cci_clk); > > >> 278 return dev_err_probe(dev, ret, > > >> 279 "failed to get cci > > >> clk: %d\n", ret); > > >> 280 } > > >> 281 > > >> 282 drv->inter_clk = devm_clk_get(dev, > > >> "intermediate"); > > >> 283 if (IS_ERR(drv->inter_clk)) { > > >> 284 ret = PTR_ERR(drv->inter_clk); > > >> 285 dev_err_probe(dev, ret, > > >> 286 "failed to get > > >> intermediate clk: %d\n", ret); > > >> 287 goto out_free_resources; > > >> 288 } > > >> 289 > > >> 290 drv->proc_reg = > > >> devm_regulator_get_optional(dev, "proc"); > > >> 291 if (IS_ERR(drv->proc_reg)) { > > >> 292 ret = PTR_ERR(drv->proc_reg); > > >> 293 dev_err_probe(dev, ret, > > >> 294 "failed to get proc > > >> regulator: %d\n", ret); > > >> 295 goto out_free_resources; > > >> 296 } > > >> 297 > > >> 298 ret = regulator_enable(drv->proc_reg); > > >> 299 if (ret) { > > >> 300 dev_err(dev, "failed to enable proc > > >> regulator\n"); > > >> 301 goto out_free_resources; > > >> 302 } > > >> 303 > > >> 304 drv->sram_reg = regulator_get_optional(dev, > > >> "sram"); > > >> 305 if (IS_ERR(drv->sram_reg)) > > >> 306 drv->sram_reg = NULL; > > >> 307 else { > > >> 308 ret = regulator_enable(drv->sram_reg); > > >> 309 if (ret) { > > >> 310 dev_err(dev, "failed to enable > > >> sram regulator\n"); > > >> 311 goto out_free_resources; > > >> 312 } > > >> 313 } > > >> 314 > > >> 315 /* > > >> 316 * We assume min voltage is 0 and tracking > > >> target voltage using > > >> 317 * min_volt_shift for each iteration. > > >> 318 * The retry_max is 3 times of expeted > > >> iteration count. > > >> 319 */ > > >> 320 drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv- > > >>> soc_data->sram_max_volt, > > >> 321 drv- > > >>> soc_data->proc_max_volt), > > >> 322 drv- > > >>> soc_data->min_volt_shift); > > >> 323 > > >> 324 ret = clk_prepare_enable(drv->cci_clk); > > >> 325 if (ret) > > >> 326 goto out_free_resources; > > >> 327 > > >> 328 ret = clk_prepare_enable(drv->inter_clk); > > >> 329 if (ret) > > >> 330 goto out_disable_cci_clk; > > >> 331 > > >> 332 ret = dev_pm_opp_of_add_table(dev); > > >> 333 if (ret) { > > >> 334 dev_err(dev, "failed to add opp table: > > >> %d\n", ret); > > >> 335 goto out_disable_inter_clk; > > >> 336 } > > >> 337 > > >> 338 rate = clk_get_rate(drv->inter_clk); > > >> 339 opp = dev_pm_opp_find_freq_ceil(dev, &rate); > > >> 340 if (IS_ERR(opp)) { > > >> 341 ret = PTR_ERR(opp); > > >> 342 dev_err(dev, "failed to get > > >> intermediate opp: %d\n", ret); > > >> 343 goto out_remove_opp_table; > > >> 344 } > > >> 345 drv->inter_voltage = > > >> dev_pm_opp_get_voltage(opp); > > >> 346 dev_pm_opp_put(opp); > > >> 347 > > >> 348 rate = U32_MAX; > > >> 349 opp = dev_pm_opp_find_freq_floor(drv->dev, > > >> &rate); > > >> 350 if (IS_ERR(opp)) { > > >> 351 dev_err(dev, "failed to get opp\n"); > > >> 352 ret = PTR_ERR(opp); > > >> 353 goto out_remove_opp_table; > > >> 354 } > > >> 355 > > >> 356 opp_volt = dev_pm_opp_get_voltage(opp); > > >> 357 dev_pm_opp_put(opp); > > >> 358 ret = mtk_ccifreq_set_voltage(drv, opp_volt); > > >> 359 if (ret) { > > >> 360 dev_err(dev, "failed to scale to > > >> highest voltage %lu in proc_reg\n", > > >> 361 opp_volt); > > >> 362 goto out_remove_opp_table; > > >> 363 } > > >> 364 > > >> 365 passive_data = devm_kzalloc(dev, sizeof(struct > > >> devfreq_passive_data), > > >> 366 GFP_KERNEL); > > >> 367 if (!passive_data) { > > >> 368 ret = -ENOMEM; > > >> 369 goto out_remove_opp_table; > > >> 370 } > > >> 371 > > >> 372 passive_data->parent_type = CPUFREQ_PARENT_DEV; > > >> 373 drv->devfreq = devm_devfreq_add_device(dev, > > >> &mtk_ccifreq_profile, > > >> 374 DEVFREQ_ > > >> GOV_PASSIVE, > > >> 375 passive_ > > >> data); > > >> 376 if (IS_ERR(drv->devfreq)) { > > >> 377 ret = -EPROBE_DEFER; > > >> 378 dev_err(dev, "failed to add devfreq > > >> device: %d\n", > > >> > 379 PTR_ERR(drv->devfreq)); > > >> 380 goto out_remove_opp_table; > > >> 381 } > > >> 382 > > >> 383 drv->opp_nb.notifier_call = > > >> mtk_ccifreq_opp_notifier; > > >> 384 ret = dev_pm_opp_register_notifier(dev, &drv- > > >>> opp_nb); > > >> 385 if (ret) { > > >> 386 dev_err(dev, "failed to register opp > > >> notifier: %d\n", ret); > > >> 387 goto out_remove_devfreq_device; > > >> 388 } > > >> 389 return 0; > > >> 390 > > >> 391 out_remove_devfreq_device: > > >> 392 devm_devfreq_remove_device(dev, drv->devfreq); > > >> 393 > > >> 394 out_remove_opp_table: > > >> 395 dev_pm_opp_of_remove_table(dev); > > >> 396 > > >> 397 out_disable_inter_clk: > > >> 398 clk_disable_unprepare(drv->inter_clk); > > >> 399 > > >> 400 out_disable_cci_clk: > > >> 401 clk_disable_unprepare(drv->cci_clk); > > >> 402 > > >> 403 out_free_resources: > > >> 404 if (regulator_is_enabled(drv->proc_reg)) > > >> 405 regulator_disable(drv->proc_reg); > > >> 406 if (drv->sram_reg && regulator_is_enabled(drv- > > >>> sram_reg)) > > >> 407 regulator_disable(drv->sram_reg); > > >> 408 > > >> 409 if (!IS_ERR(drv->proc_reg)) > > >> 410 regulator_put(drv->proc_reg); > > >> 411 if (!IS_ERR(drv->sram_reg)) > > >> 412 regulator_put(drv->sram_reg); > > >> 413 if (!IS_ERR(drv->cci_clk)) > > >> 414 clk_put(drv->cci_clk); > > >> 415 if (!IS_ERR(drv->inter_clk)) > > >> 416 clk_put(drv->inter_clk); > > >> 417 > > >> 418 return ret; > > >> 419 } > > >> 420 > > >> > > > > > > Hi "kernel test robot", > > > > > > Thanks for your review. > > > > > > This patch is based on chanwoo/devfreq-testing[1] > > > [1] > > > https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing > > > > Hi Johnson, > > > > Thanks for the feedback, we'll take a look too. > > I think the last patch on that branch might be broken. You mean the patch[1]. Without this patch[1], there are no problems? [1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing&id=ea1011fba665b95fc28f682c9b131799a88b11ae When you tested these patches with patchset[2] without last patch[1] if there are no problems, please reply to patchset[2] with your Tested-by tag. [2] https://patchwork.kernel.org/project/linux-pm/cover/20220507150145.531864-1-cw00.choi@samsung.com/ -- Best Regards, Chanwoo Choi
On Sat, 2022-05-07 at 22:53 +0900, Chanwoo Choi wrote: > On 22. 5. 6. 20:38, Johnson Wang wrote: > > On Mon, 2022-04-25 at 20:55 +0800, Johnson Wang wrote: > > > We introduce a devfreq driver for the MediaTek Cache Coherent > > > Interconnect > > > (CCI) used by some MediaTek SoCs. > > > > > > In this driver, we use the passive devfreq driver to get target > > > frequencies > > > and adjust voltages accordingly. In MT8183 and MT8186, the > > > MediaTek > > > CCI > > > is supplied by the same regulators with the little core CPUs. > > > > > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > > > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > > > --- > > > This patch depends on "devfreq-testing"[1]. > > > [1] > > > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing__;!!CTRNKA9wMg0ARbw!wnTkI_sh3TsliBPFP70DFwtSZGtKyPinFu9h2BwULWX-xrWF0C21rLV-n1HhstfmLw42$ > > > > > > --- > > > drivers/devfreq/Kconfig | 10 + > > > drivers/devfreq/Makefile | 1 + > > > drivers/devfreq/mtk-cci-devfreq.c | 474 > > > ++++++++++++++++++++++++++++++ > > > 3 files changed, 485 insertions(+) > > > create mode 100644 drivers/devfreq/mtk-cci-devfreq.c > > > > > > diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig > > > index 87eb2b837e68..9754d8b31621 100644 > > > --- a/drivers/devfreq/Kconfig > > > +++ b/drivers/devfreq/Kconfig > > > @@ -120,6 +120,16 @@ config ARM_TEGRA_DEVFREQ > > > It reads ACTMON counters of memory controllers and > > > adjusts > > > the > > > operating frequencies and voltages with OPP support. > > > > > > +config ARM_MEDIATEK_CCI_DEVFREQ > > > + tristate "MEDIATEK CCI DEVFREQ Driver" > > > + depends on ARM_MEDIATEK_CPUFREQ || COMPILE_TEST > > > + select DEVFREQ_GOV_PASSIVE > > > + help > > > + This adds a devfreq driver for MediaTek Cache Coherent > > > Interconnect > > > + which is shared the same regulators with the cpu cluster. It > > > can track > > > + buck voltages and update a proper CCI frequency. Use the > > > notification > > > + to get the regulator status. > > > + > > > config ARM_RK3399_DMC_DEVFREQ > > > tristate "ARM RK3399 DMC DEVFREQ Driver" > > > depends on (ARCH_ROCKCHIP && HAVE_ARM_SMCCC) || \ > > > diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile > > > index 0b6be92a25d9..bf40d04928d0 100644 > > > --- a/drivers/devfreq/Makefile > > > +++ b/drivers/devfreq/Makefile > > > @@ -11,6 +11,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += > > > governor_passive.o > > > obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o > > > obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ) += imx-bus.o > > > obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ) += imx8m-ddrc.o > > > +obj-$(CONFIG_ARM_MEDIATEK_CCI_DEVFREQ) += mtk-cci-devfreq.o > > > obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o > > > obj-$(CONFIG_ARM_SUN8I_A33_MBUS_DEVFREQ) += sun8i-a33- > > > mbus.o > > > obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30-devfreq.o > > > diff --git a/drivers/devfreq/mtk-cci-devfreq.c > > > b/drivers/devfreq/mtk- > > > cci-devfreq.c > > > new file mode 100644 > > > index 000000000000..b3e31c45a57c > > > --- /dev/null > > > +++ b/drivers/devfreq/mtk-cci-devfreq.c > > > @@ -0,0 +1,474 @@ > > > +// SPDX-License-Identifier: GPL-2.0-only > > > +/* > > > + * Copyright (C) 2022 MediaTek Inc. > > > + */ > > > + > > > +#include <linux/clk.h> > > > +#include <linux/devfreq.h> > > > +#include <linux/minmax.h> > > > +#include <linux/module.h> > > > +#include <linux/of.h> > > > +#include <linux/of_device.h> > > > +#include <linux/platform_device.h> > > > +#include <linux/pm_opp.h> > > > +#include <linux/regulator/consumer.h> > > > + > > > +struct mtk_ccifreq_platform_data { > > > + int min_volt_shift; > > > + int max_volt_shift; > > > + int proc_max_volt; > > > + int sram_min_volt; > > > + int sram_max_volt; > > > +}; > > > + > > > +struct mtk_ccifreq_drv { > > > + struct device *dev; > > > + struct devfreq *devfreq; > > > + struct regulator *proc_reg; > > > + struct regulator *sram_reg; > > > + struct clk *cci_clk; > > > + struct clk *inter_clk; > > > + int inter_voltage; > > > + int pre_voltage; > > > + unsigned long pre_freq; > > > + /* Avoid race condition for regulators between notify and > > > policy */ > > > + struct mutex reg_lock; > > > + struct notifier_block opp_nb; > > > + const struct mtk_ccifreq_platform_data *soc_data; > > > + int vtrack_max; > > > +}; > > > + > > > +static int mtk_ccifreq_set_voltage(struct mtk_ccifreq_drv *drv, > > > int > > > new_voltage) > > > +{ > > > + const struct mtk_ccifreq_platform_data *soc_data = drv- > > > > soc_data; > > > > > > + struct device *dev = drv->dev; > > > + int pre_voltage, pre_vsram, new_vsram, vsram, voltage, ret; > > > + int retry_max = drv->vtrack_max; > > > + > > > + if (!drv->sram_reg) { > > > + ret = regulator_set_voltage(drv->proc_reg, new_voltage, > > > + drv->soc_data- > > > > proc_max_volt); > > > > > > + goto out_set_voltage; > > > + } > > > + > > > + pre_voltage = regulator_get_voltage(drv->proc_reg); > > > + if (pre_voltage < 0) { > > > + dev_err(dev, "invalid vproc value: %d\n", pre_voltage); > > > + return pre_voltage; > > > + } > > > + > > > + pre_vsram = regulator_get_voltage(drv->sram_reg); > > > + if (pre_vsram < 0) { > > > + dev_err(dev, "invalid vsram value: %d\n", pre_vsram); > > > + return pre_vsram; > > > + } > > > + > > > + new_vsram = clamp(new_voltage + soc_data->min_volt_shift, > > > + soc_data->sram_min_volt, soc_data- > > > > sram_max_volt); > > > > > > + > > > + do { > > > + if (pre_voltage <= new_voltage) { > > > + vsram = clamp(pre_voltage + soc_data- > > > > max_volt_shift, > > > > > > + soc_data->sram_min_volt, > > > new_vsram); > > > + ret = regulator_set_voltage(drv->sram_reg, > > > vsram, > > > + soc_data- > > > > sram_max_volt); > > > > > > + if (ret) > > > + return ret; > > > + > > > + if (vsram == soc_data->sram_max_volt || > > > + new_vsram == soc_data->sram_min_volt) > > > + voltage = new_voltage; > > > + else > > > + voltage = vsram - soc_data- > > > > min_volt_shift; > > > > > > + > > > + ret = regulator_set_voltage(drv->proc_reg, > > > voltage, > > > + soc_data- > > > > proc_max_volt); > > > > > > + if (ret) { > > > + regulator_set_voltage(drv->sram_reg, > > > pre_vsram, > > > + soc_data- > > > > sram_max_volt); > > > > > > + return ret; > > > + } > > > + } else if (pre_voltage > new_voltage) { > > > + voltage = max(new_voltage, > > > + pre_vsram - soc_data- > > > > max_volt_shift); > > > > > > + ret = regulator_set_voltage(drv->proc_reg, > > > voltage, > > > + soc_data- > > > > proc_max_volt); > > > > > > + if (ret) > > > + return ret; > > > + > > > + if (voltage == new_voltage) > > > + vsram = new_vsram; > > > + else > > > + vsram = max(new_vsram, > > > + voltage + soc_data- > > > > min_volt_shift); > > > > > > + > > > + ret = regulator_set_voltage(drv->sram_reg, > > > vsram, > > > + soc_data- > > > > sram_max_volt); > > > > > > + if (ret) { > > > + regulator_set_voltage(drv->proc_reg, > > > pre_voltage, > > > + soc_data- > > > > proc_max_volt); > > > > > > + return ret; > > > + } > > > + } > > > + > > > + pre_voltage = voltage; > > > + pre_vsram = vsram; > > > + > > > + if (--retry_max < 0) { > > > + dev_err(dev, > > > + "over loop count, failed to set > > > voltage\n"); > > > + return -EINVAL; > > > + } > > > + } while (voltage != new_voltage || vsram != new_vsram); > > > + > > > +out_set_voltage: > > > + if (!ret) > > > + drv->pre_voltage = new_voltage; > > > + > > > + return ret; > > > +} > > > + > > > +static int mtk_ccifreq_target(struct device *dev, unsigned long > > > *freq, > > > + u32 flags) > > > +{ > > > + struct mtk_ccifreq_drv *drv = dev_get_drvdata(dev); > > > + struct clk *cci_pll = clk_get_parent(drv->cci_clk); > > > + struct dev_pm_opp *opp; > > > + unsigned long opp_rate; > > > + int voltage, pre_voltage, inter_voltage, target_voltage, ret; > > > + > > > + if (!drv) > > > + return -EINVAL; > > > + > > > + if (drv->pre_freq == *freq) > > > + return 0; > > > + > > > + inter_voltage = drv->inter_voltage; > > > + > > > + opp_rate = *freq; > > > + opp = devfreq_recommended_opp(dev, &opp_rate, 1); > > > + if (IS_ERR(opp)) { > > > + dev_err(dev, "failed to find opp for freq: %ld\n", > > > opp_rate); > > > + return PTR_ERR(opp); > > > + } > > > + > > > + mutex_lock(&drv->reg_lock); > > > + > > > + voltage = dev_pm_opp_get_voltage(opp); > > > + dev_pm_opp_put(opp); > > > + > > > + if (unlikely(drv->pre_voltage <= 0)) > > > + pre_voltage = regulator_get_voltage(drv->proc_reg); > > > + else > > > + pre_voltage = drv->pre_voltage; > > > + > > > + if (pre_voltage < 0) { > > > + dev_err(dev, "invalid vproc value: %d\n", pre_voltage); > > > + return pre_voltage; > > > + } > > > + > > > + /* scale up: set voltage first then freq. */ > > > + target_voltage = max(inter_voltage, voltage); > > > + if (pre_voltage <= target_voltage) { > > > + ret = mtk_ccifreq_set_voltage(drv, target_voltage); > > > + if (ret) { > > > + dev_err(dev, "failed to scale up voltage\n"); > > > + goto out_restore_voltage; > > > + } > > > + } > > > + > > > + /* switch the cci clock to intermediate clock source. */ > > > + ret = clk_set_parent(drv->cci_clk, drv->inter_clk); > > > + if (ret) { > > > + dev_err(dev, "failed to re-parent cci clock\n"); > > > + goto out_restore_voltage; > > > + } > > > + > > > + /* set the original clock to target rate. */ > > > + ret = clk_set_rate(cci_pll, *freq); > > > + if (ret) { > > > + dev_err(dev, "failed to set cci pll rate: %d\n", ret); > > > + clk_set_parent(drv->cci_clk, cci_pll); > > > + goto out_restore_voltage; > > > + } > > > + > > > + /* switch the cci clock back to the original clock source. */ > > > + ret = clk_set_parent(drv->cci_clk, cci_pll); > > > + if (ret) { > > > + dev_err(dev, "failed to re-parent cci clock\n"); > > > + mtk_ccifreq_set_voltage(drv, inter_voltage); > > > + goto out_unlock; > > > + } > > > + > > > + /* > > > + * If the new voltage is lower than the intermediate voltage or > > > the > > > + * original voltage, scale down to the new voltage. > > > + */ > > > + if (voltage < inter_voltage || voltage < pre_voltage) { > > > + ret = mtk_ccifreq_set_voltage(drv, voltage); > > > + if (ret) { > > > + dev_err(dev, "failed to scale down voltage\n"); > > > + goto out_unlock; > > > + } > > > + } > > > + > > > + drv->pre_freq = *freq; > > > + mutex_unlock(&drv->reg_lock); > > > + > > > + return 0; > > > + > > > +out_restore_voltage: > > > + mtk_ccifreq_set_voltage(drv, pre_voltage); > > > + > > > +out_unlock: > > > + mutex_unlock(&drv->reg_lock); > > > + return ret; > > > +} > > > + > > > +static int mtk_ccifreq_opp_notifier(struct notifier_block *nb, > > > + unsigned long event, void *data) > > > +{ > > > + struct dev_pm_opp *opp = data; > > > + struct mtk_ccifreq_drv *drv; > > > + unsigned long freq, volt; > > > + > > > + drv = container_of(nb, struct mtk_ccifreq_drv, opp_nb); > > > + > > > + if (event == OPP_EVENT_ADJUST_VOLTAGE) { > > > + freq = dev_pm_opp_get_freq(opp); > > > + > > > + mutex_lock(&drv->reg_lock); > > > + /* current opp item is changed */ > > > + if (freq == drv->pre_freq) { > > > + volt = dev_pm_opp_get_voltage(opp); > > > + mtk_ccifreq_set_voltage(drv, volt); > > > + } > > > + mutex_unlock(&drv->reg_lock); > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +static struct devfreq_dev_profile mtk_ccifreq_profile = { > > > + .target = mtk_ccifreq_target, > > > +}; > > > + > > > +static int mtk_ccifreq_probe(struct platform_device *pdev) > > > +{ > > > + struct device *dev = &pdev->dev; > > > + struct mtk_ccifreq_drv *drv; > > > + struct devfreq_passive_data *passive_data; > > > + struct dev_pm_opp *opp; > > > + unsigned long rate, opp_volt; > > > + int ret; > > > + > > > + drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL); > > > + if (!drv) > > > + return -ENOMEM; > > > + > > > + drv->dev = dev; > > > + drv->soc_data = (const struct mtk_ccifreq_platform_data *) > > > + of_device_get_match_data(&pdev->dev); > > > + mutex_init(&drv->reg_lock); > > > + platform_set_drvdata(pdev, drv); > > > + > > > + drv->cci_clk = devm_clk_get(dev, "cci"); > > > + if (IS_ERR(drv->cci_clk)) { > > > + ret = PTR_ERR(drv->cci_clk); > > > + return dev_err_probe(dev, ret, > > > + "failed to get cci clk: %d\n", > > > ret); > > > + } > > > + > > > + drv->inter_clk = devm_clk_get(dev, "intermediate"); > > > + if (IS_ERR(drv->inter_clk)) { > > > + ret = PTR_ERR(drv->inter_clk); > > > + dev_err_probe(dev, ret, > > > + "failed to get intermediate clk: %d\n", > > > ret); > > > + goto out_free_resources; > > > + } > > > + > > > + drv->proc_reg = devm_regulator_get_optional(dev, "proc"); > > > + if (IS_ERR(drv->proc_reg)) { > > > + ret = PTR_ERR(drv->proc_reg); > > > + dev_err_probe(dev, ret, > > > + "failed to get proc regulator: %d\n", > > > ret); > > > + goto out_free_resources; > > > + } > > > + > > > + ret = regulator_enable(drv->proc_reg); > > > + if (ret) { > > > + dev_err(dev, "failed to enable proc regulator\n"); > > > + goto out_free_resources; > > > + } > > > + > > > + drv->sram_reg = regulator_get_optional(dev, "sram"); > > > + if (IS_ERR(drv->sram_reg)) > > > + drv->sram_reg = NULL; > > > + else { > > > + ret = regulator_enable(drv->sram_reg); > > > + if (ret) { > > > + dev_err(dev, "failed to enable sram > > > regulator\n"); > > > + goto out_free_resources; > > > + } > > > + } > > > + > > > + /* > > > + * We assume min voltage is 0 and tracking target voltage using > > > + * min_volt_shift for each iteration. > > > + * The retry_max is 3 times of expeted iteration count. > > > + */ > > > + drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv->soc_data- > > > > sram_max_volt, > > > > > > + drv->soc_data- > > > > proc_max_volt), > > > > > > + drv->soc_data- > > > > min_volt_shift); > > > > > > + > > > + ret = clk_prepare_enable(drv->cci_clk); > > > + if (ret) > > > + goto out_free_resources; > > > + > > > + ret = clk_prepare_enable(drv->inter_clk); > > > + if (ret) > > > + goto out_disable_cci_clk; > > > + > > > + ret = dev_pm_opp_of_add_table(dev); > > > + if (ret) { > > > + dev_err(dev, "failed to add opp table: %d\n", ret); > > > + goto out_disable_inter_clk; > > > + } > > > + > > > + rate = clk_get_rate(drv->inter_clk); > > > + opp = dev_pm_opp_find_freq_ceil(dev, &rate); > > > + if (IS_ERR(opp)) { > > > + ret = PTR_ERR(opp); > > > + dev_err(dev, "failed to get intermediate opp: %d\n", > > > ret); > > > + goto out_remove_opp_table; > > > + } > > > + drv->inter_voltage = dev_pm_opp_get_voltage(opp); > > > + dev_pm_opp_put(opp); > > > + > > > + rate = U32_MAX; > > > + opp = dev_pm_opp_find_freq_floor(drv->dev, &rate); > > > + if (IS_ERR(opp)) { > > > + dev_err(dev, "failed to get opp\n"); > > > + ret = PTR_ERR(opp); > > > + goto out_remove_opp_table; > > > + } > > > + > > > + opp_volt = dev_pm_opp_get_voltage(opp); > > > + dev_pm_opp_put(opp); > > > + ret = mtk_ccifreq_set_voltage(drv, opp_volt); > > > + if (ret) { > > > + dev_err(dev, "failed to scale to highest voltage %lu in > > > proc_reg\n", > > > + opp_volt); > > > + goto out_remove_opp_table; > > > + } > > > + > > > + passive_data = devm_kzalloc(dev, sizeof(struct > > > devfreq_passive_data), > > > + GFP_KERNEL); > > > + if (!passive_data) { > > > + ret = -ENOMEM; > > > + goto out_remove_opp_table; > > > + } > > > + > > > + passive_data->parent_type = CPUFREQ_PARENT_DEV; > > > + drv->devfreq = devm_devfreq_add_device(dev, > > > &mtk_ccifreq_profile, > > > + DEVFREQ_GOV_PASSIVE, > > > + passive_data); > > > + if (IS_ERR(drv->devfreq)) { > > > + ret = -EPROBE_DEFER; > > > + dev_err(dev, "failed to add devfreq device: %d\n", > > > + PTR_ERR(drv->devfreq)); > > > + goto out_remove_opp_table; > > > + } > > > + > > > + drv->opp_nb.notifier_call = mtk_ccifreq_opp_notifier; > > > + ret = dev_pm_opp_register_notifier(dev, &drv->opp_nb); > > > + if (ret) { > > > + dev_err(dev, "failed to register opp notifier: %d\n", > > > ret); > > > + goto out_remove_devfreq_device; > > > + } > > > + return 0; > > > + > > > +out_remove_devfreq_device: > > > + devm_devfreq_remove_device(dev, drv->devfreq); > > > + > > > +out_remove_opp_table: > > > + dev_pm_opp_of_remove_table(dev); > > > + > > > +out_disable_inter_clk: > > > + clk_disable_unprepare(drv->inter_clk); > > > + > > > +out_disable_cci_clk: > > > + clk_disable_unprepare(drv->cci_clk); > > > + > > > +out_free_resources: > > > + if (regulator_is_enabled(drv->proc_reg)) > > > + regulator_disable(drv->proc_reg); > > > + if (drv->sram_reg && regulator_is_enabled(drv->sram_reg)) > > > + regulator_disable(drv->sram_reg); > > > + > > > + if (!IS_ERR(drv->proc_reg)) > > > + regulator_put(drv->proc_reg); > > > + if (!IS_ERR(drv->sram_reg)) > > > + regulator_put(drv->sram_reg); > > > + if (!IS_ERR(drv->cci_clk)) > > > + clk_put(drv->cci_clk); > > > + if (!IS_ERR(drv->inter_clk)) > > > + clk_put(drv->inter_clk); > > > + > > > + return ret; > > > +} > > > + > > > +static int mtk_ccifreq_remove(struct platform_device *pdev) > > > +{ > > > + struct device *dev = &pdev->dev; > > > + struct mtk_ccifreq_drv *drv; > > > + > > > + drv = platform_get_drvdata(pdev); > > > + > > > + dev_pm_opp_unregister_notifier(dev, &drv->opp_nb); > > > + dev_pm_opp_of_remove_table(dev); > > > + clk_disable_unprepare(drv->inter_clk); > > > + clk_disable_unprepare(drv->cci_clk); > > > + regulator_disable(drv->proc_reg); > > > + if (drv->sram_reg) > > > + regulator_disable(drv->sram_reg); > > > + > > > + return 0; > > > +} > > > + > > > +static const struct mtk_ccifreq_platform_data > > > mt8183_platform_data = > > > { > > > + .min_volt_shift = 100000, > > > + .max_volt_shift = 200000, > > > + .proc_max_volt = 1150000, > > > + .sram_min_volt = 0, > > > + .sram_max_volt = 1150000, > > > +}; > > > + > > > +static const struct mtk_ccifreq_platform_data > > > mt8186_platform_data = > > > { > > > + .min_volt_shift = 100000, > > > + .max_volt_shift = 250000, > > > + .proc_max_volt = 1118750, > > > + .sram_min_volt = 850000, > > > + .sram_max_volt = 1118750, > > > +}; > > > + > > > +static const struct of_device_id mtk_ccifreq_machines[] = { > > > + { .compatible = "mediatek,mt8183-cci", .data = > > > &mt8183_platform_data }, > > > + { .compatible = "mediatek,mt8186-cci", .data = > > > &mt8186_platform_data }, > > > + { }, > > > +}; > > > +MODULE_DEVICE_TABLE(of, mtk_ccifreq_machines); > > > + > > > +static struct platform_driver mtk_ccifreq_platdrv = { > > > + .probe = mtk_ccifreq_probe, > > > + .remove = mtk_ccifreq_remove, > > > + .driver = { > > > + .name = "mtk-ccifreq", > > > + .of_match_table = mtk_ccifreq_machines, > > > + }, > > > +}; > > > +module_platform_driver(mtk_ccifreq_platdrv); > > > + > > > +MODULE_DESCRIPTION("MediaTek CCI devfreq driver"); > > > +MODULE_AUTHOR("Jia-Wei Chang <jia-wei.chang@mediatek.com>"); > > > +MODULE_LICENSE("GPL v2"); > > > > Hi Chanwoo, > > > > Just a kindly ping. > > Could you please give me some suggestion on this patch? > > Thanks! > > > > BRs, > > Johnson Wang > > > > Hi Johnson, > > Sorry for late reply.But I replied it yesterday as following: > Maybe, this reply[1] has not yet arrrived at your mail box. > [1] > https://lore.kernel.org/lkml/0846a062-2ea2-e6d4-03c8-992c2f2e24a0@gmail.com/T/#m3bc609d805d6e74ec7a105be0511926b4afbbaef > > As I described on reply[1], I updated the patches on devfreq-testing > branch. Could you please test your patches based on devfreq-testing > branch? > Hi Chanwoo, Thanks for your information. I've tested this patch based on the latest devfreq-testing branch. It encounters the same crash as Chen-Yu mentioned[1]. [1]https://lore.kernel.org/linux-pm/YniX1w+oI1eOCmCx@google.com/T/#u BRs, Johnson Wang
On 22. 5. 9. 14:57, Johnson Wang wrote: > On Sat, 2022-05-07 at 22:53 +0900, Chanwoo Choi wrote: >> On 22. 5. 6. 20:38, Johnson Wang wrote: >>> On Mon, 2022-04-25 at 20:55 +0800, Johnson Wang wrote: >>>> We introduce a devfreq driver for the MediaTek Cache Coherent >>>> Interconnect >>>> (CCI) used by some MediaTek SoCs. >>>> >>>> In this driver, we use the passive devfreq driver to get target >>>> frequencies >>>> and adjust voltages accordingly. In MT8183 and MT8186, the >>>> MediaTek >>>> CCI >>>> is supplied by the same regulators with the little core CPUs. >>>> >>>> Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> >>>> Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> >>>> --- >>>> This patch depends on "devfreq-testing"[1]. >>>> [1] >>>> > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing__;!!CTRNKA9wMg0ARbw!wnTkI_sh3TsliBPFP70DFwtSZGtKyPinFu9h2BwULWX-xrWF0C21rLV-n1HhstfmLw42$ >>>> >>>> --- >>>> drivers/devfreq/Kconfig | 10 + >>>> drivers/devfreq/Makefile | 1 + >>>> drivers/devfreq/mtk-cci-devfreq.c | 474 >>>> ++++++++++++++++++++++++++++++ >>>> 3 files changed, 485 insertions(+) >>>> create mode 100644 drivers/devfreq/mtk-cci-devfreq.c >>>> >>>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig >>>> index 87eb2b837e68..9754d8b31621 100644 >>>> --- a/drivers/devfreq/Kconfig >>>> +++ b/drivers/devfreq/Kconfig >>>> @@ -120,6 +120,16 @@ config ARM_TEGRA_DEVFREQ >>>> It reads ACTMON counters of memory controllers and >>>> adjusts >>>> the >>>> operating frequencies and voltages with OPP support. >>>> >>>> +config ARM_MEDIATEK_CCI_DEVFREQ >>>> + tristate "MEDIATEK CCI DEVFREQ Driver" >>>> + depends on ARM_MEDIATEK_CPUFREQ || COMPILE_TEST >>>> + select DEVFREQ_GOV_PASSIVE >>>> + help >>>> + This adds a devfreq driver for MediaTek Cache Coherent >>>> Interconnect >>>> + which is shared the same regulators with the cpu cluster. It >>>> can track >>>> + buck voltages and update a proper CCI frequency. Use the >>>> notification >>>> + to get the regulator status. >>>> + >>>> config ARM_RK3399_DMC_DEVFREQ >>>> tristate "ARM RK3399 DMC DEVFREQ Driver" >>>> depends on (ARCH_ROCKCHIP && HAVE_ARM_SMCCC) || \ >>>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile >>>> index 0b6be92a25d9..bf40d04928d0 100644 >>>> --- a/drivers/devfreq/Makefile >>>> +++ b/drivers/devfreq/Makefile >>>> @@ -11,6 +11,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += >>>> governor_passive.o >>>> obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o >>>> obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ) += imx-bus.o >>>> obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ) += imx8m-ddrc.o >>>> +obj-$(CONFIG_ARM_MEDIATEK_CCI_DEVFREQ) += mtk-cci-devfreq.o >>>> obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o >>>> obj-$(CONFIG_ARM_SUN8I_A33_MBUS_DEVFREQ) += sun8i-a33- >>>> mbus.o >>>> obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30-devfreq.o >>>> diff --git a/drivers/devfreq/mtk-cci-devfreq.c >>>> b/drivers/devfreq/mtk- >>>> cci-devfreq.c >>>> new file mode 100644 >>>> index 000000000000..b3e31c45a57c >>>> --- /dev/null >>>> +++ b/drivers/devfreq/mtk-cci-devfreq.c >>>> @@ -0,0 +1,474 @@ >>>> +// SPDX-License-Identifier: GPL-2.0-only >>>> +/* >>>> + * Copyright (C) 2022 MediaTek Inc. >>>> + */ >>>> + >>>> +#include <linux/clk.h> >>>> +#include <linux/devfreq.h> >>>> +#include <linux/minmax.h> >>>> +#include <linux/module.h> >>>> +#include <linux/of.h> >>>> +#include <linux/of_device.h> >>>> +#include <linux/platform_device.h> >>>> +#include <linux/pm_opp.h> >>>> +#include <linux/regulator/consumer.h> >>>> + >>>> +struct mtk_ccifreq_platform_data { >>>> + int min_volt_shift; >>>> + int max_volt_shift; >>>> + int proc_max_volt; >>>> + int sram_min_volt; >>>> + int sram_max_volt; >>>> +}; >>>> + >>>> +struct mtk_ccifreq_drv { >>>> + struct device *dev; >>>> + struct devfreq *devfreq; >>>> + struct regulator *proc_reg; >>>> + struct regulator *sram_reg; >>>> + struct clk *cci_clk; >>>> + struct clk *inter_clk; >>>> + int inter_voltage; >>>> + int pre_voltage; >>>> + unsigned long pre_freq; >>>> + /* Avoid race condition for regulators between notify and >>>> policy */ >>>> + struct mutex reg_lock; >>>> + struct notifier_block opp_nb; >>>> + const struct mtk_ccifreq_platform_data *soc_data; >>>> + int vtrack_max; >>>> +}; >>>> + >>>> +static int mtk_ccifreq_set_voltage(struct mtk_ccifreq_drv *drv, >>>> int >>>> new_voltage) >>>> +{ >>>> + const struct mtk_ccifreq_platform_data *soc_data = drv- >>>>> soc_data; >>>> >>>> + struct device *dev = drv->dev; >>>> + int pre_voltage, pre_vsram, new_vsram, vsram, voltage, ret; >>>> + int retry_max = drv->vtrack_max; >>>> + >>>> + if (!drv->sram_reg) { >>>> + ret = regulator_set_voltage(drv->proc_reg, new_voltage, >>>> + drv->soc_data- >>>>> proc_max_volt); >>>> >>>> + goto out_set_voltage; >>>> + } >>>> + >>>> + pre_voltage = regulator_get_voltage(drv->proc_reg); >>>> + if (pre_voltage < 0) { >>>> + dev_err(dev, "invalid vproc value: %d\n", pre_voltage); >>>> + return pre_voltage; >>>> + } >>>> + >>>> + pre_vsram = regulator_get_voltage(drv->sram_reg); >>>> + if (pre_vsram < 0) { >>>> + dev_err(dev, "invalid vsram value: %d\n", pre_vsram); >>>> + return pre_vsram; >>>> + } >>>> + >>>> + new_vsram = clamp(new_voltage + soc_data->min_volt_shift, >>>> + soc_data->sram_min_volt, soc_data- >>>>> sram_max_volt); >>>> >>>> + >>>> + do { >>>> + if (pre_voltage <= new_voltage) { >>>> + vsram = clamp(pre_voltage + soc_data- >>>>> max_volt_shift, >>>> >>>> + soc_data->sram_min_volt, >>>> new_vsram); >>>> + ret = regulator_set_voltage(drv->sram_reg, >>>> vsram, >>>> + soc_data- >>>>> sram_max_volt); >>>> >>>> + if (ret) >>>> + return ret; >>>> + >>>> + if (vsram == soc_data->sram_max_volt || >>>> + new_vsram == soc_data->sram_min_volt) >>>> + voltage = new_voltage; >>>> + else >>>> + voltage = vsram - soc_data- >>>>> min_volt_shift; >>>> >>>> + >>>> + ret = regulator_set_voltage(drv->proc_reg, >>>> voltage, >>>> + soc_data- >>>>> proc_max_volt); >>>> >>>> + if (ret) { >>>> + regulator_set_voltage(drv->sram_reg, >>>> pre_vsram, >>>> + soc_data- >>>>> sram_max_volt); >>>> >>>> + return ret; >>>> + } >>>> + } else if (pre_voltage > new_voltage) { >>>> + voltage = max(new_voltage, >>>> + pre_vsram - soc_data- >>>>> max_volt_shift); >>>> >>>> + ret = regulator_set_voltage(drv->proc_reg, >>>> voltage, >>>> + soc_data- >>>>> proc_max_volt); >>>> >>>> + if (ret) >>>> + return ret; >>>> + >>>> + if (voltage == new_voltage) >>>> + vsram = new_vsram; >>>> + else >>>> + vsram = max(new_vsram, >>>> + voltage + soc_data- >>>>> min_volt_shift); >>>> >>>> + >>>> + ret = regulator_set_voltage(drv->sram_reg, >>>> vsram, >>>> + soc_data- >>>>> sram_max_volt); >>>> >>>> + if (ret) { >>>> + regulator_set_voltage(drv->proc_reg, >>>> pre_voltage, >>>> + soc_data- >>>>> proc_max_volt); >>>> >>>> + return ret; >>>> + } >>>> + } >>>> + >>>> + pre_voltage = voltage; >>>> + pre_vsram = vsram; >>>> + >>>> + if (--retry_max < 0) { >>>> + dev_err(dev, >>>> + "over loop count, failed to set >>>> voltage\n"); >>>> + return -EINVAL; >>>> + } >>>> + } while (voltage != new_voltage || vsram != new_vsram); >>>> + >>>> +out_set_voltage: >>>> + if (!ret) >>>> + drv->pre_voltage = new_voltage; >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +static int mtk_ccifreq_target(struct device *dev, unsigned long >>>> *freq, >>>> + u32 flags) >>>> +{ >>>> + struct mtk_ccifreq_drv *drv = dev_get_drvdata(dev); >>>> + struct clk *cci_pll = clk_get_parent(drv->cci_clk); >>>> + struct dev_pm_opp *opp; >>>> + unsigned long opp_rate; >>>> + int voltage, pre_voltage, inter_voltage, target_voltage, ret; >>>> + >>>> + if (!drv) >>>> + return -EINVAL; >>>> + >>>> + if (drv->pre_freq == *freq) >>>> + return 0; >>>> + >>>> + inter_voltage = drv->inter_voltage; >>>> + >>>> + opp_rate = *freq; >>>> + opp = devfreq_recommended_opp(dev, &opp_rate, 1); >>>> + if (IS_ERR(opp)) { >>>> + dev_err(dev, "failed to find opp for freq: %ld\n", >>>> opp_rate); >>>> + return PTR_ERR(opp); >>>> + } >>>> + >>>> + mutex_lock(&drv->reg_lock); >>>> + >>>> + voltage = dev_pm_opp_get_voltage(opp); >>>> + dev_pm_opp_put(opp); >>>> + >>>> + if (unlikely(drv->pre_voltage <= 0)) >>>> + pre_voltage = regulator_get_voltage(drv->proc_reg); >>>> + else >>>> + pre_voltage = drv->pre_voltage; >>>> + >>>> + if (pre_voltage < 0) { >>>> + dev_err(dev, "invalid vproc value: %d\n", pre_voltage); >>>> + return pre_voltage; >>>> + } >>>> + >>>> + /* scale up: set voltage first then freq. */ >>>> + target_voltage = max(inter_voltage, voltage); >>>> + if (pre_voltage <= target_voltage) { >>>> + ret = mtk_ccifreq_set_voltage(drv, target_voltage); >>>> + if (ret) { >>>> + dev_err(dev, "failed to scale up voltage\n"); >>>> + goto out_restore_voltage; >>>> + } >>>> + } >>>> + >>>> + /* switch the cci clock to intermediate clock source. */ >>>> + ret = clk_set_parent(drv->cci_clk, drv->inter_clk); >>>> + if (ret) { >>>> + dev_err(dev, "failed to re-parent cci clock\n"); >>>> + goto out_restore_voltage; >>>> + } >>>> + >>>> + /* set the original clock to target rate. */ >>>> + ret = clk_set_rate(cci_pll, *freq); >>>> + if (ret) { >>>> + dev_err(dev, "failed to set cci pll rate: %d\n", ret); >>>> + clk_set_parent(drv->cci_clk, cci_pll); >>>> + goto out_restore_voltage; >>>> + } >>>> + >>>> + /* switch the cci clock back to the original clock source. */ >>>> + ret = clk_set_parent(drv->cci_clk, cci_pll); >>>> + if (ret) { >>>> + dev_err(dev, "failed to re-parent cci clock\n"); >>>> + mtk_ccifreq_set_voltage(drv, inter_voltage); >>>> + goto out_unlock; >>>> + } >>>> + >>>> + /* >>>> + * If the new voltage is lower than the intermediate voltage or >>>> the >>>> + * original voltage, scale down to the new voltage. >>>> + */ >>>> + if (voltage < inter_voltage || voltage < pre_voltage) { >>>> + ret = mtk_ccifreq_set_voltage(drv, voltage); >>>> + if (ret) { >>>> + dev_err(dev, "failed to scale down voltage\n"); >>>> + goto out_unlock; >>>> + } >>>> + } >>>> + >>>> + drv->pre_freq = *freq; >>>> + mutex_unlock(&drv->reg_lock); >>>> + >>>> + return 0; >>>> + >>>> +out_restore_voltage: >>>> + mtk_ccifreq_set_voltage(drv, pre_voltage); >>>> + >>>> +out_unlock: >>>> + mutex_unlock(&drv->reg_lock); >>>> + return ret; >>>> +} >>>> + >>>> +static int mtk_ccifreq_opp_notifier(struct notifier_block *nb, >>>> + unsigned long event, void *data) >>>> +{ >>>> + struct dev_pm_opp *opp = data; >>>> + struct mtk_ccifreq_drv *drv; >>>> + unsigned long freq, volt; >>>> + >>>> + drv = container_of(nb, struct mtk_ccifreq_drv, opp_nb); >>>> + >>>> + if (event == OPP_EVENT_ADJUST_VOLTAGE) { >>>> + freq = dev_pm_opp_get_freq(opp); >>>> + >>>> + mutex_lock(&drv->reg_lock); >>>> + /* current opp item is changed */ >>>> + if (freq == drv->pre_freq) { >>>> + volt = dev_pm_opp_get_voltage(opp); >>>> + mtk_ccifreq_set_voltage(drv, volt); >>>> + } >>>> + mutex_unlock(&drv->reg_lock); >>>> + } >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static struct devfreq_dev_profile mtk_ccifreq_profile = { >>>> + .target = mtk_ccifreq_target, >>>> +}; >>>> + >>>> +static int mtk_ccifreq_probe(struct platform_device *pdev) >>>> +{ >>>> + struct device *dev = &pdev->dev; >>>> + struct mtk_ccifreq_drv *drv; >>>> + struct devfreq_passive_data *passive_data; >>>> + struct dev_pm_opp *opp; >>>> + unsigned long rate, opp_volt; >>>> + int ret; >>>> + >>>> + drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL); >>>> + if (!drv) >>>> + return -ENOMEM; >>>> + >>>> + drv->dev = dev; >>>> + drv->soc_data = (const struct mtk_ccifreq_platform_data *) >>>> + of_device_get_match_data(&pdev->dev); >>>> + mutex_init(&drv->reg_lock); >>>> + platform_set_drvdata(pdev, drv); >>>> + >>>> + drv->cci_clk = devm_clk_get(dev, "cci"); >>>> + if (IS_ERR(drv->cci_clk)) { >>>> + ret = PTR_ERR(drv->cci_clk); >>>> + return dev_err_probe(dev, ret, >>>> + "failed to get cci clk: %d\n", >>>> ret); >>>> + } >>>> + >>>> + drv->inter_clk = devm_clk_get(dev, "intermediate"); >>>> + if (IS_ERR(drv->inter_clk)) { >>>> + ret = PTR_ERR(drv->inter_clk); >>>> + dev_err_probe(dev, ret, >>>> + "failed to get intermediate clk: %d\n", >>>> ret); >>>> + goto out_free_resources; >>>> + } >>>> + >>>> + drv->proc_reg = devm_regulator_get_optional(dev, "proc"); >>>> + if (IS_ERR(drv->proc_reg)) { >>>> + ret = PTR_ERR(drv->proc_reg); >>>> + dev_err_probe(dev, ret, >>>> + "failed to get proc regulator: %d\n", >>>> ret); >>>> + goto out_free_resources; >>>> + } >>>> + >>>> + ret = regulator_enable(drv->proc_reg); >>>> + if (ret) { >>>> + dev_err(dev, "failed to enable proc regulator\n"); >>>> + goto out_free_resources; >>>> + } >>>> + >>>> + drv->sram_reg = regulator_get_optional(dev, "sram"); >>>> + if (IS_ERR(drv->sram_reg)) >>>> + drv->sram_reg = NULL; >>>> + else { >>>> + ret = regulator_enable(drv->sram_reg); >>>> + if (ret) { >>>> + dev_err(dev, "failed to enable sram >>>> regulator\n"); >>>> + goto out_free_resources; >>>> + } >>>> + } >>>> + >>>> + /* >>>> + * We assume min voltage is 0 and tracking target voltage using >>>> + * min_volt_shift for each iteration. >>>> + * The retry_max is 3 times of expeted iteration count. >>>> + */ >>>> + drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv->soc_data- >>>>> sram_max_volt, >>>> >>>> + drv->soc_data- >>>>> proc_max_volt), >>>> >>>> + drv->soc_data- >>>>> min_volt_shift); >>>> >>>> + >>>> + ret = clk_prepare_enable(drv->cci_clk); >>>> + if (ret) >>>> + goto out_free_resources; >>>> + >>>> + ret = clk_prepare_enable(drv->inter_clk); >>>> + if (ret) >>>> + goto out_disable_cci_clk; >>>> + >>>> + ret = dev_pm_opp_of_add_table(dev); >>>> + if (ret) { >>>> + dev_err(dev, "failed to add opp table: %d\n", ret); >>>> + goto out_disable_inter_clk; >>>> + } >>>> + >>>> + rate = clk_get_rate(drv->inter_clk); >>>> + opp = dev_pm_opp_find_freq_ceil(dev, &rate); >>>> + if (IS_ERR(opp)) { >>>> + ret = PTR_ERR(opp); >>>> + dev_err(dev, "failed to get intermediate opp: %d\n", >>>> ret); >>>> + goto out_remove_opp_table; >>>> + } >>>> + drv->inter_voltage = dev_pm_opp_get_voltage(opp); >>>> + dev_pm_opp_put(opp); >>>> + >>>> + rate = U32_MAX; >>>> + opp = dev_pm_opp_find_freq_floor(drv->dev, &rate); >>>> + if (IS_ERR(opp)) { >>>> + dev_err(dev, "failed to get opp\n"); >>>> + ret = PTR_ERR(opp); >>>> + goto out_remove_opp_table; >>>> + } >>>> + >>>> + opp_volt = dev_pm_opp_get_voltage(opp); >>>> + dev_pm_opp_put(opp); >>>> + ret = mtk_ccifreq_set_voltage(drv, opp_volt); >>>> + if (ret) { >>>> + dev_err(dev, "failed to scale to highest voltage %lu in >>>> proc_reg\n", >>>> + opp_volt); >>>> + goto out_remove_opp_table; >>>> + } >>>> + >>>> + passive_data = devm_kzalloc(dev, sizeof(struct >>>> devfreq_passive_data), >>>> + GFP_KERNEL); >>>> + if (!passive_data) { >>>> + ret = -ENOMEM; >>>> + goto out_remove_opp_table; >>>> + } >>>> + >>>> + passive_data->parent_type = CPUFREQ_PARENT_DEV; >>>> + drv->devfreq = devm_devfreq_add_device(dev, >>>> &mtk_ccifreq_profile, >>>> + DEVFREQ_GOV_PASSIVE, >>>> + passive_data); >>>> + if (IS_ERR(drv->devfreq)) { >>>> + ret = -EPROBE_DEFER; >>>> + dev_err(dev, "failed to add devfreq device: %d\n", >>>> + PTR_ERR(drv->devfreq)); >>>> + goto out_remove_opp_table; >>>> + } >>>> + >>>> + drv->opp_nb.notifier_call = mtk_ccifreq_opp_notifier; >>>> + ret = dev_pm_opp_register_notifier(dev, &drv->opp_nb); >>>> + if (ret) { >>>> + dev_err(dev, "failed to register opp notifier: %d\n", >>>> ret); >>>> + goto out_remove_devfreq_device; >>>> + } >>>> + return 0; >>>> + >>>> +out_remove_devfreq_device: >>>> + devm_devfreq_remove_device(dev, drv->devfreq); >>>> + >>>> +out_remove_opp_table: >>>> + dev_pm_opp_of_remove_table(dev); >>>> + >>>> +out_disable_inter_clk: >>>> + clk_disable_unprepare(drv->inter_clk); >>>> + >>>> +out_disable_cci_clk: >>>> + clk_disable_unprepare(drv->cci_clk); >>>> + >>>> +out_free_resources: >>>> + if (regulator_is_enabled(drv->proc_reg)) >>>> + regulator_disable(drv->proc_reg); >>>> + if (drv->sram_reg && regulator_is_enabled(drv->sram_reg)) >>>> + regulator_disable(drv->sram_reg); >>>> + >>>> + if (!IS_ERR(drv->proc_reg)) >>>> + regulator_put(drv->proc_reg); >>>> + if (!IS_ERR(drv->sram_reg)) >>>> + regulator_put(drv->sram_reg); >>>> + if (!IS_ERR(drv->cci_clk)) >>>> + clk_put(drv->cci_clk); >>>> + if (!IS_ERR(drv->inter_clk)) >>>> + clk_put(drv->inter_clk); >>>> + >>>> + return ret; >>>> +} >>>> + >>>> +static int mtk_ccifreq_remove(struct platform_device *pdev) >>>> +{ >>>> + struct device *dev = &pdev->dev; >>>> + struct mtk_ccifreq_drv *drv; >>>> + >>>> + drv = platform_get_drvdata(pdev); >>>> + >>>> + dev_pm_opp_unregister_notifier(dev, &drv->opp_nb); >>>> + dev_pm_opp_of_remove_table(dev); >>>> + clk_disable_unprepare(drv->inter_clk); >>>> + clk_disable_unprepare(drv->cci_clk); >>>> + regulator_disable(drv->proc_reg); >>>> + if (drv->sram_reg) >>>> + regulator_disable(drv->sram_reg); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +static const struct mtk_ccifreq_platform_data >>>> mt8183_platform_data = >>>> { >>>> + .min_volt_shift = 100000, >>>> + .max_volt_shift = 200000, >>>> + .proc_max_volt = 1150000, >>>> + .sram_min_volt = 0, >>>> + .sram_max_volt = 1150000, >>>> +}; >>>> + >>>> +static const struct mtk_ccifreq_platform_data >>>> mt8186_platform_data = >>>> { >>>> + .min_volt_shift = 100000, >>>> + .max_volt_shift = 250000, >>>> + .proc_max_volt = 1118750, >>>> + .sram_min_volt = 850000, >>>> + .sram_max_volt = 1118750, >>>> +}; >>>> + >>>> +static const struct of_device_id mtk_ccifreq_machines[] = { >>>> + { .compatible = "mediatek,mt8183-cci", .data = >>>> &mt8183_platform_data }, >>>> + { .compatible = "mediatek,mt8186-cci", .data = >>>> &mt8186_platform_data }, >>>> + { }, >>>> +}; >>>> +MODULE_DEVICE_TABLE(of, mtk_ccifreq_machines); >>>> + >>>> +static struct platform_driver mtk_ccifreq_platdrv = { >>>> + .probe = mtk_ccifreq_probe, >>>> + .remove = mtk_ccifreq_remove, >>>> + .driver = { >>>> + .name = "mtk-ccifreq", >>>> + .of_match_table = mtk_ccifreq_machines, >>>> + }, >>>> +}; >>>> +module_platform_driver(mtk_ccifreq_platdrv); >>>> + >>>> +MODULE_DESCRIPTION("MediaTek CCI devfreq driver"); >>>> +MODULE_AUTHOR("Jia-Wei Chang <jia-wei.chang@mediatek.com>"); >>>> +MODULE_LICENSE("GPL v2"); >>> >>> Hi Chanwoo, >>> >>> Just a kindly ping. >>> Could you please give me some suggestion on this patch? >>> Thanks! >>> >>> BRs, >>> Johnson Wang >>> >> >> Hi Johnson, >> >> Sorry for late reply.But I replied it yesterday as following: >> Maybe, this reply[1] has not yet arrrived at your mail box. >> [1] >> > https://lore.kernel.org/lkml/0846a062-2ea2-e6d4-03c8-992c2f2e24a0@gmail.com/T/#m3bc609d805d6e74ec7a105be0511926b4afbbaef >> >> As I described on reply[1], I updated the patches on devfreq-testing >> branch. Could you please test your patches based on devfreq-testing >> branch? >> > > Hi Chanwoo, > > Thanks for your information. > I've tested this patch based on the latest devfreq-testing branch. > It encounters the same crash as Chen-Yu mentioned[1]. > > [1]https://lore.kernel.org/linux-pm/YniX1w+oI1eOCmCx@google.com/T/#u Hi Johnson, Thanks for the test. I'll drop the last patch caused of crash. And I'll send v3 patchset right now.
On Mon, 2022-05-09 at 20:51 +0900, Chanwoo Choi wrote: > On 22. 5. 9. 14:57, Johnson Wang wrote: > > On Sat, 2022-05-07 at 22:53 +0900, Chanwoo Choi wrote: > > > On 22. 5. 6. 20:38, Johnson Wang wrote: > > > > On Mon, 2022-04-25 at 20:55 +0800, Johnson Wang wrote: > > > > > We introduce a devfreq driver for the MediaTek Cache Coherent > > > > > Interconnect > > > > > (CCI) used by some MediaTek SoCs. > > > > > > > > > > In this driver, we use the passive devfreq driver to get > > > > > target > > > > > frequencies > > > > > and adjust voltages accordingly. In MT8183 and MT8186, the > > > > > MediaTek > > > > > CCI > > > > > is supplied by the same regulators with the little core CPUs. > > > > > > > > > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > > > > > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > > > > > --- > > > > > This patch depends on "devfreq-testing"[1]. > > > > > [1] > > > > > > > > > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing__;!!CTRNKA9wMg0ARbw!wnTkI_sh3TsliBPFP70DFwtSZGtKyPinFu9h2BwULWX-xrWF0C21rLV-n1HhstfmLw42$ > > > > > > > > > > --- > > > > > drivers/devfreq/Kconfig | 10 + > > > > > drivers/devfreq/Makefile | 1 + > > > > > drivers/devfreq/mtk-cci-devfreq.c | 474 > > > > > ++++++++++++++++++++++++++++++ > > > > > 3 files changed, 485 insertions(+) > > > > > create mode 100644 drivers/devfreq/mtk-cci-devfreq.c > > > > > > > > > > diff --git a/drivers/devfreq/Kconfig > > > > > b/drivers/devfreq/Kconfig > > > > > index 87eb2b837e68..9754d8b31621 100644 > > > > > --- a/drivers/devfreq/Kconfig > > > > > +++ b/drivers/devfreq/Kconfig > > > > > @@ -120,6 +120,16 @@ config ARM_TEGRA_DEVFREQ > > > > > It reads ACTMON counters of memory controllers and > > > > > adjusts > > > > > the > > > > > operating frequencies and voltages with OPP support. > > > > > > > > > > +config ARM_MEDIATEK_CCI_DEVFREQ > > > > > + tristate "MEDIATEK CCI DEVFREQ Driver" > > > > > + depends on ARM_MEDIATEK_CPUFREQ || COMPILE_TEST > > > > > + select DEVFREQ_GOV_PASSIVE > > > > > + help > > > > > + This adds a devfreq driver for MediaTek Cache > > > > > Coherent > > > > > Interconnect > > > > > + which is shared the same regulators with the cpu > > > > > cluster. It > > > > > can track > > > > > + buck voltages and update a proper CCI frequency. Use > > > > > the > > > > > notification > > > > > + to get the regulator status. > > > > > + > > > > > config ARM_RK3399_DMC_DEVFREQ > > > > > tristate "ARM RK3399 DMC DEVFREQ Driver" > > > > > depends on (ARCH_ROCKCHIP && HAVE_ARM_SMCCC) || \ > > > > > diff --git a/drivers/devfreq/Makefile > > > > > b/drivers/devfreq/Makefile > > > > > index 0b6be92a25d9..bf40d04928d0 100644 > > > > > --- a/drivers/devfreq/Makefile > > > > > +++ b/drivers/devfreq/Makefile > > > > > @@ -11,6 +11,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += > > > > > governor_passive.o > > > > > obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o > > > > > obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ) += imx-bus.o > > > > > obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ) += imx8m-ddrc.o > > > > > +obj-$(CONFIG_ARM_MEDIATEK_CCI_DEVFREQ) += mtk-cci- > > > > > devfreq.o > > > > > obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o > > > > > obj-$(CONFIG_ARM_SUN8I_A33_MBUS_DEVFREQ) += sun8i-a33- > > > > > mbus.o > > > > > obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30- > > > > > devfreq.o > > > > > diff --git a/drivers/devfreq/mtk-cci-devfreq.c > > > > > b/drivers/devfreq/mtk- > > > > > cci-devfreq.c > > > > > new file mode 100644 > > > > > index 000000000000..b3e31c45a57c > > > > > --- /dev/null > > > > > +++ b/drivers/devfreq/mtk-cci-devfreq.c > > > > > @@ -0,0 +1,474 @@ > > > > > +// SPDX-License-Identifier: GPL-2.0-only > > > > > +/* > > > > > + * Copyright (C) 2022 MediaTek Inc. > > > > > + */ > > > > > + > > > > > +#include <linux/clk.h> > > > > > +#include <linux/devfreq.h> > > > > > +#include <linux/minmax.h> > > > > > +#include <linux/module.h> > > > > > +#include <linux/of.h> > > > > > +#include <linux/of_device.h> > > > > > +#include <linux/platform_device.h> > > > > > +#include <linux/pm_opp.h> > > > > > +#include <linux/regulator/consumer.h> > > > > > + > > > > > +struct mtk_ccifreq_platform_data { > > > > > + int min_volt_shift; > > > > > + int max_volt_shift; > > > > > + int proc_max_volt; > > > > > + int sram_min_volt; > > > > > + int sram_max_volt; > > > > > +}; > > > > > + > > > > > +struct mtk_ccifreq_drv { > > > > > + struct device *dev; > > > > > + struct devfreq *devfreq; > > > > > + struct regulator *proc_reg; > > > > > + struct regulator *sram_reg; > > > > > + struct clk *cci_clk; > > > > > + struct clk *inter_clk; > > > > > + int inter_voltage; > > > > > + int pre_voltage; > > > > > + unsigned long pre_freq; > > > > > + /* Avoid race condition for regulators between notify > > > > > and > > > > > policy */ > > > > > + struct mutex reg_lock; > > > > > + struct notifier_block opp_nb; > > > > > + const struct mtk_ccifreq_platform_data *soc_data; > > > > > + int vtrack_max; > > > > > +}; > > > > > + > > > > > +static int mtk_ccifreq_set_voltage(struct mtk_ccifreq_drv > > > > > *drv, > > > > > int > > > > > new_voltage) > > > > > +{ > > > > > + const struct mtk_ccifreq_platform_data *soc_data = drv- > > > > > > soc_data; > > > > > > > > > > + struct device *dev = drv->dev; > > > > > + int pre_voltage, pre_vsram, new_vsram, vsram, voltage, > > > > > ret; > > > > > + int retry_max = drv->vtrack_max; > > > > > + > > > > > + if (!drv->sram_reg) { > > > > > + ret = regulator_set_voltage(drv->proc_reg, > > > > > new_voltage, > > > > > + drv->soc_data- > > > > > > proc_max_volt); > > > > > > > > > > + goto out_set_voltage; > > > > > + } > > > > > + > > > > > + pre_voltage = regulator_get_voltage(drv->proc_reg); > > > > > + if (pre_voltage < 0) { > > > > > + dev_err(dev, "invalid vproc value: %d\n", > > > > > pre_voltage); > > > > > + return pre_voltage; > > > > > + } > > > > > + > > > > > + pre_vsram = regulator_get_voltage(drv->sram_reg); > > > > > + if (pre_vsram < 0) { > > > > > + dev_err(dev, "invalid vsram value: %d\n", > > > > > pre_vsram); > > > > > + return pre_vsram; > > > > > + } > > > > > + > > > > > + new_vsram = clamp(new_voltage + soc_data- > > > > > >min_volt_shift, > > > > > + soc_data->sram_min_volt, soc_data- > > > > > > sram_max_volt); > > > > > > > > > > + > > > > > + do { > > > > > + if (pre_voltage <= new_voltage) { > > > > > + vsram = clamp(pre_voltage + soc_data- > > > > > > max_volt_shift, > > > > > > > > > > + soc_data->sram_min_volt, > > > > > new_vsram); > > > > > + ret = regulator_set_voltage(drv- > > > > > >sram_reg, > > > > > vsram, > > > > > + soc_data- > > > > > > sram_max_volt); > > > > > > > > > > + if (ret) > > > > > + return ret; > > > > > + > > > > > + if (vsram == soc_data->sram_max_volt || > > > > > + new_vsram == soc_data- > > > > > >sram_min_volt) > > > > > + voltage = new_voltage; > > > > > + else > > > > > + voltage = vsram - soc_data- > > > > > > min_volt_shift; > > > > > > > > > > + > > > > > + ret = regulator_set_voltage(drv- > > > > > >proc_reg, > > > > > voltage, > > > > > + soc_data- > > > > > > proc_max_volt); > > > > > > > > > > + if (ret) { > > > > > + regulator_set_voltage(drv- > > > > > >sram_reg, > > > > > pre_vsram, > > > > > + soc_data- > > > > > > sram_max_volt); > > > > > > > > > > + return ret; > > > > > + } > > > > > + } else if (pre_voltage > new_voltage) { > > > > > + voltage = max(new_voltage, > > > > > + pre_vsram - soc_data- > > > > > > max_volt_shift); > > > > > > > > > > + ret = regulator_set_voltage(drv- > > > > > >proc_reg, > > > > > voltage, > > > > > + soc_data- > > > > > > proc_max_volt); > > > > > > > > > > + if (ret) > > > > > + return ret; > > > > > + > > > > > + if (voltage == new_voltage) > > > > > + vsram = new_vsram; > > > > > + else > > > > > + vsram = max(new_vsram, > > > > > + voltage + soc_data- > > > > > > min_volt_shift); > > > > > > > > > > + > > > > > + ret = regulator_set_voltage(drv- > > > > > >sram_reg, > > > > > vsram, > > > > > + soc_data- > > > > > > sram_max_volt); > > > > > > > > > > + if (ret) { > > > > > + regulator_set_voltage(drv- > > > > > >proc_reg, > > > > > pre_voltage, > > > > > + soc_data- > > > > > > proc_max_volt); > > > > > > > > > > + return ret; > > > > > + } > > > > > + } > > > > > + > > > > > + pre_voltage = voltage; > > > > > + pre_vsram = vsram; > > > > > + > > > > > + if (--retry_max < 0) { > > > > > + dev_err(dev, > > > > > + "over loop count, failed to set > > > > > voltage\n"); > > > > > + return -EINVAL; > > > > > + } > > > > > + } while (voltage != new_voltage || vsram != new_vsram); > > > > > + > > > > > +out_set_voltage: > > > > > + if (!ret) > > > > > + drv->pre_voltage = new_voltage; > > > > > + > > > > > + return ret; > > > > > +} > > > > > + > > > > > +static int mtk_ccifreq_target(struct device *dev, unsigned > > > > > long > > > > > *freq, > > > > > + u32 flags) > > > > > +{ > > > > > + struct mtk_ccifreq_drv *drv = dev_get_drvdata(dev); > > > > > + struct clk *cci_pll = clk_get_parent(drv->cci_clk); > > > > > + struct dev_pm_opp *opp; > > > > > + unsigned long opp_rate; > > > > > + int voltage, pre_voltage, inter_voltage, > > > > > target_voltage, ret; > > > > > + > > > > > + if (!drv) > > > > > + return -EINVAL; > > > > > + > > > > > + if (drv->pre_freq == *freq) > > > > > + return 0; > > > > > + > > > > > + inter_voltage = drv->inter_voltage; > > > > > + > > > > > + opp_rate = *freq; > > > > > + opp = devfreq_recommended_opp(dev, &opp_rate, 1); > > > > > + if (IS_ERR(opp)) { > > > > > + dev_err(dev, "failed to find opp for freq: > > > > > %ld\n", > > > > > opp_rate); > > > > > + return PTR_ERR(opp); > > > > > + } > > > > > + > > > > > + mutex_lock(&drv->reg_lock); > > > > > + > > > > > + voltage = dev_pm_opp_get_voltage(opp); > > > > > + dev_pm_opp_put(opp); > > > > > + > > > > > + if (unlikely(drv->pre_voltage <= 0)) > > > > > + pre_voltage = regulator_get_voltage(drv- > > > > > >proc_reg); > > > > > + else > > > > > + pre_voltage = drv->pre_voltage; > > > > > + > > > > > + if (pre_voltage < 0) { > > > > > + dev_err(dev, "invalid vproc value: %d\n", > > > > > pre_voltage); > > > > > + return pre_voltage; > > > > > + } > > > > > + > > > > > + /* scale up: set voltage first then freq. */ > > > > > + target_voltage = max(inter_voltage, voltage); > > > > > + if (pre_voltage <= target_voltage) { > > > > > + ret = mtk_ccifreq_set_voltage(drv, > > > > > target_voltage); > > > > > + if (ret) { > > > > > + dev_err(dev, "failed to scale up > > > > > voltage\n"); > > > > > + goto out_restore_voltage; > > > > > + } > > > > > + } > > > > > + > > > > > + /* switch the cci clock to intermediate clock source. > > > > > */ > > > > > + ret = clk_set_parent(drv->cci_clk, drv->inter_clk); > > > > > + if (ret) { > > > > > + dev_err(dev, "failed to re-parent cci > > > > > clock\n"); > > > > > + goto out_restore_voltage; > > > > > + } > > > > > + > > > > > + /* set the original clock to target rate. */ > > > > > + ret = clk_set_rate(cci_pll, *freq); > > > > > + if (ret) { > > > > > + dev_err(dev, "failed to set cci pll rate: > > > > > %d\n", ret); > > > > > + clk_set_parent(drv->cci_clk, cci_pll); > > > > > + goto out_restore_voltage; > > > > > + } > > > > > + > > > > > + /* switch the cci clock back to the original clock > > > > > source. */ > > > > > + ret = clk_set_parent(drv->cci_clk, cci_pll); > > > > > + if (ret) { > > > > > + dev_err(dev, "failed to re-parent cci > > > > > clock\n"); > > > > > + mtk_ccifreq_set_voltage(drv, inter_voltage); > > > > > + goto out_unlock; > > > > > + } > > > > > + > > > > > + /* > > > > > + * If the new voltage is lower than the intermediate > > > > > voltage or > > > > > the > > > > > + * original voltage, scale down to the new voltage. > > > > > + */ > > > > > + if (voltage < inter_voltage || voltage < pre_voltage) { > > > > > + ret = mtk_ccifreq_set_voltage(drv, voltage); > > > > > + if (ret) { > > > > > + dev_err(dev, "failed to scale down > > > > > voltage\n"); > > > > > + goto out_unlock; > > > > > + } > > > > > + } > > > > > + > > > > > + drv->pre_freq = *freq; > > > > > + mutex_unlock(&drv->reg_lock); > > > > > + > > > > > + return 0; > > > > > + > > > > > +out_restore_voltage: > > > > > + mtk_ccifreq_set_voltage(drv, pre_voltage); > > > > > + > > > > > +out_unlock: > > > > > + mutex_unlock(&drv->reg_lock); > > > > > + return ret; > > > > > +} > > > > > + > > > > > +static int mtk_ccifreq_opp_notifier(struct notifier_block > > > > > *nb, > > > > > + unsigned long event, void > > > > > *data) > > > > > +{ > > > > > + struct dev_pm_opp *opp = data; > > > > > + struct mtk_ccifreq_drv *drv; > > > > > + unsigned long freq, volt; > > > > > + > > > > > + drv = container_of(nb, struct mtk_ccifreq_drv, opp_nb); > > > > > + > > > > > + if (event == OPP_EVENT_ADJUST_VOLTAGE) { > > > > > + freq = dev_pm_opp_get_freq(opp); > > > > > + > > > > > + mutex_lock(&drv->reg_lock); > > > > > + /* current opp item is changed */ > > > > > + if (freq == drv->pre_freq) { > > > > > + volt = dev_pm_opp_get_voltage(opp); > > > > > + mtk_ccifreq_set_voltage(drv, volt); > > > > > + } > > > > > + mutex_unlock(&drv->reg_lock); > > > > > + } > > > > > + > > > > > + return 0; > > > > > +} > > > > > + > > > > > +static struct devfreq_dev_profile mtk_ccifreq_profile = { > > > > > + .target = mtk_ccifreq_target, > > > > > +}; > > > > > + > > > > > +static int mtk_ccifreq_probe(struct platform_device *pdev) > > > > > +{ > > > > > + struct device *dev = &pdev->dev; > > > > > + struct mtk_ccifreq_drv *drv; > > > > > + struct devfreq_passive_data *passive_data; > > > > > + struct dev_pm_opp *opp; > > > > > + unsigned long rate, opp_volt; > > > > > + int ret; > > > > > + > > > > > + drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL); > > > > > + if (!drv) > > > > > + return -ENOMEM; > > > > > + > > > > > + drv->dev = dev; > > > > > + drv->soc_data = (const struct mtk_ccifreq_platform_data > > > > > *) > > > > > + of_device_get_match_data(&pdev- > > > > > >dev); > > > > > + mutex_init(&drv->reg_lock); > > > > > + platform_set_drvdata(pdev, drv); > > > > > + > > > > > + drv->cci_clk = devm_clk_get(dev, "cci"); > > > > > + if (IS_ERR(drv->cci_clk)) { > > > > > + ret = PTR_ERR(drv->cci_clk); > > > > > + return dev_err_probe(dev, ret, > > > > > + "failed to get cci clk: > > > > > %d\n", > > > > > ret); > > > > > + } > > > > > + > > > > > + drv->inter_clk = devm_clk_get(dev, "intermediate"); > > > > > + if (IS_ERR(drv->inter_clk)) { > > > > > + ret = PTR_ERR(drv->inter_clk); > > > > > + dev_err_probe(dev, ret, > > > > > + "failed to get intermediate clk: > > > > > %d\n", > > > > > ret); > > > > > + goto out_free_resources; > > > > > + } > > > > > + > > > > > + drv->proc_reg = devm_regulator_get_optional(dev, > > > > > "proc"); > > > > > + if (IS_ERR(drv->proc_reg)) { > > > > > + ret = PTR_ERR(drv->proc_reg); > > > > > + dev_err_probe(dev, ret, > > > > > + "failed to get proc regulator: > > > > > %d\n", > > > > > ret); > > > > > + goto out_free_resources; > > > > > + } > > > > > + > > > > > + ret = regulator_enable(drv->proc_reg); > > > > > + if (ret) { > > > > > + dev_err(dev, "failed to enable proc > > > > > regulator\n"); > > > > > + goto out_free_resources; > > > > > + } > > > > > + > > > > > + drv->sram_reg = regulator_get_optional(dev, "sram"); > > > > > + if (IS_ERR(drv->sram_reg)) > > > > > + drv->sram_reg = NULL; > > > > > + else { > > > > > + ret = regulator_enable(drv->sram_reg); > > > > > + if (ret) { > > > > > + dev_err(dev, "failed to enable sram > > > > > regulator\n"); > > > > > + goto out_free_resources; > > > > > + } > > > > > + } > > > > > + > > > > > + /* > > > > > + * We assume min voltage is 0 and tracking target > > > > > voltage using > > > > > + * min_volt_shift for each iteration. > > > > > + * The retry_max is 3 times of expeted iteration count. > > > > > + */ > > > > > + drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv->soc_data- > > > > > > sram_max_volt, > > > > > > > > > > + drv->soc_data- > > > > > > proc_max_volt), > > > > > > > > > > + drv->soc_data- > > > > > > min_volt_shift); > > > > > > > > > > + > > > > > + ret = clk_prepare_enable(drv->cci_clk); > > > > > + if (ret) > > > > > + goto out_free_resources; > > > > > + > > > > > + ret = clk_prepare_enable(drv->inter_clk); > > > > > + if (ret) > > > > > + goto out_disable_cci_clk; > > > > > + > > > > > + ret = dev_pm_opp_of_add_table(dev); > > > > > + if (ret) { > > > > > + dev_err(dev, "failed to add opp table: %d\n", > > > > > ret); > > > > > + goto out_disable_inter_clk; > > > > > + } > > > > > + > > > > > + rate = clk_get_rate(drv->inter_clk); > > > > > + opp = dev_pm_opp_find_freq_ceil(dev, &rate); > > > > > + if (IS_ERR(opp)) { > > > > > + ret = PTR_ERR(opp); > > > > > + dev_err(dev, "failed to get intermediate opp: > > > > > %d\n", > > > > > ret); > > > > > + goto out_remove_opp_table; > > > > > + } > > > > > + drv->inter_voltage = dev_pm_opp_get_voltage(opp); > > > > > + dev_pm_opp_put(opp); > > > > > + > > > > > + rate = U32_MAX; > > > > > + opp = dev_pm_opp_find_freq_floor(drv->dev, &rate); > > > > > + if (IS_ERR(opp)) { > > > > > + dev_err(dev, "failed to get opp\n"); > > > > > + ret = PTR_ERR(opp); > > > > > + goto out_remove_opp_table; > > > > > + } > > > > > + > > > > > + opp_volt = dev_pm_opp_get_voltage(opp); > > > > > + dev_pm_opp_put(opp); > > > > > + ret = mtk_ccifreq_set_voltage(drv, opp_volt); > > > > > + if (ret) { > > > > > + dev_err(dev, "failed to scale to highest > > > > > voltage %lu in > > > > > proc_reg\n", > > > > > + opp_volt); > > > > > + goto out_remove_opp_table; > > > > > + } > > > > > + > > > > > + passive_data = devm_kzalloc(dev, sizeof(struct > > > > > devfreq_passive_data), > > > > > + GFP_KERNEL); > > > > > + if (!passive_data) { > > > > > + ret = -ENOMEM; > > > > > + goto out_remove_opp_table; > > > > > + } > > > > > + > > > > > + passive_data->parent_type = CPUFREQ_PARENT_DEV; > > > > > + drv->devfreq = devm_devfreq_add_device(dev, > > > > > &mtk_ccifreq_profile, > > > > > + DEVFREQ_GOV_PASS > > > > > IVE, > > > > > + passive_data); > > > > > + if (IS_ERR(drv->devfreq)) { > > > > > + ret = -EPROBE_DEFER; > > > > > + dev_err(dev, "failed to add devfreq device: > > > > > %d\n", > > > > > + PTR_ERR(drv->devfreq)); > > > > > + goto out_remove_opp_table; > > > > > + } > > > > > + > > > > > + drv->opp_nb.notifier_call = mtk_ccifreq_opp_notifier; > > > > > + ret = dev_pm_opp_register_notifier(dev, &drv->opp_nb); > > > > > + if (ret) { > > > > > + dev_err(dev, "failed to register opp notifier: > > > > > %d\n", > > > > > ret); > > > > > + goto out_remove_devfreq_device; > > > > > + } > > > > > + return 0; > > > > > + > > > > > +out_remove_devfreq_device: > > > > > + devm_devfreq_remove_device(dev, drv->devfreq); > > > > > + > > > > > +out_remove_opp_table: > > > > > + dev_pm_opp_of_remove_table(dev); > > > > > + > > > > > +out_disable_inter_clk: > > > > > + clk_disable_unprepare(drv->inter_clk); > > > > > + > > > > > +out_disable_cci_clk: > > > > > + clk_disable_unprepare(drv->cci_clk); > > > > > + > > > > > +out_free_resources: > > > > > + if (regulator_is_enabled(drv->proc_reg)) > > > > > + regulator_disable(drv->proc_reg); > > > > > + if (drv->sram_reg && regulator_is_enabled(drv- > > > > > >sram_reg)) > > > > > + regulator_disable(drv->sram_reg); > > > > > + > > > > > + if (!IS_ERR(drv->proc_reg)) > > > > > + regulator_put(drv->proc_reg); > > > > > + if (!IS_ERR(drv->sram_reg)) > > > > > + regulator_put(drv->sram_reg); > > > > > + if (!IS_ERR(drv->cci_clk)) > > > > > + clk_put(drv->cci_clk); > > > > > + if (!IS_ERR(drv->inter_clk)) > > > > > + clk_put(drv->inter_clk); > > > > > + > > > > > + return ret; > > > > > +} > > > > > + > > > > > +static int mtk_ccifreq_remove(struct platform_device *pdev) > > > > > +{ > > > > > + struct device *dev = &pdev->dev; > > > > > + struct mtk_ccifreq_drv *drv; > > > > > + > > > > > + drv = platform_get_drvdata(pdev); > > > > > + > > > > > + dev_pm_opp_unregister_notifier(dev, &drv->opp_nb); > > > > > + dev_pm_opp_of_remove_table(dev); > > > > > + clk_disable_unprepare(drv->inter_clk); > > > > > + clk_disable_unprepare(drv->cci_clk); > > > > > + regulator_disable(drv->proc_reg); > > > > > + if (drv->sram_reg) > > > > > + regulator_disable(drv->sram_reg); > > > > > + > > > > > + return 0; > > > > > +} > > > > > + > > > > > +static const struct mtk_ccifreq_platform_data > > > > > mt8183_platform_data = > > > > > { > > > > > + .min_volt_shift = 100000, > > > > > + .max_volt_shift = 200000, > > > > > + .proc_max_volt = 1150000, > > > > > + .sram_min_volt = 0, > > > > > + .sram_max_volt = 1150000, > > > > > +}; > > > > > + > > > > > +static const struct mtk_ccifreq_platform_data > > > > > mt8186_platform_data = > > > > > { > > > > > + .min_volt_shift = 100000, > > > > > + .max_volt_shift = 250000, > > > > > + .proc_max_volt = 1118750, > > > > > + .sram_min_volt = 850000, > > > > > + .sram_max_volt = 1118750, > > > > > +}; > > > > > + > > > > > +static const struct of_device_id mtk_ccifreq_machines[] = { > > > > > + { .compatible = "mediatek,mt8183-cci", .data = > > > > > &mt8183_platform_data }, > > > > > + { .compatible = "mediatek,mt8186-cci", .data = > > > > > &mt8186_platform_data }, > > > > > + { }, > > > > > +}; > > > > > +MODULE_DEVICE_TABLE(of, mtk_ccifreq_machines); > > > > > + > > > > > +static struct platform_driver mtk_ccifreq_platdrv = { > > > > > + .probe = mtk_ccifreq_probe, > > > > > + .remove = mtk_ccifreq_remove, > > > > > + .driver = { > > > > > + .name = "mtk-ccifreq", > > > > > + .of_match_table = mtk_ccifreq_machines, > > > > > + }, > > > > > +}; > > > > > +module_platform_driver(mtk_ccifreq_platdrv); > > > > > + > > > > > +MODULE_DESCRIPTION("MediaTek CCI devfreq driver"); > > > > > +MODULE_AUTHOR("Jia-Wei Chang <jia-wei.chang@mediatek.com>"); > > > > > +MODULE_LICENSE("GPL v2"); > > > > > > > > Hi Chanwoo, > > > > > > > > Just a kindly ping. > > > > Could you please give me some suggestion on this patch? > > > > Thanks! > > > > > > > > BRs, > > > > Johnson Wang > > > > > > > > > > Hi Johnson, > > > > > > Sorry for late reply.But I replied it yesterday as following: > > > Maybe, this reply[1] has not yet arrrived at your mail box. > > > [1] > > > > > > > https://lore.kernel.org/lkml/0846a062-2ea2-e6d4-03c8-992c2f2e24a0@gmail.com/T/#m3bc609d805d6e74ec7a105be0511926b4afbbaef > > > > > > As I described on reply[1], I updated the patches on devfreq- > > > testing > > > branch. Could you please test your patches based on devfreq- > > > testing > > > branch? > > > > > > > Hi Chanwoo, > > > > Thanks for your information. > > I've tested this patch based on the latest devfreq-testing branch. > > It encounters the same crash as Chen-Yu mentioned[1]. > > > > [1] > > https://lore.kernel.org/linux-pm/YniX1w+oI1eOCmCx@google.com/T/#u > > Hi Johnson, > > Thanks for the test. I'll drop the last patch caused of crash. > And I'll send v3 patchset right now. > > Hi Chanwoo, With your v3 patchset, this CCI devfreq driver works properly on the target. Thanks! BRs, Johnson Wang
On Wed, May 11, 2022 at 2:14 PM Johnson Wang <johnson.wang@mediatek.com> wrote: > > On Mon, 2022-05-09 at 20:51 +0900, Chanwoo Choi wrote: > > On 22. 5. 9. 14:57, Johnson Wang wrote: > > > On Sat, 2022-05-07 at 22:53 +0900, Chanwoo Choi wrote: > > > > On 22. 5. 6. 20:38, Johnson Wang wrote: > > > > > On Mon, 2022-04-25 at 20:55 +0800, Johnson Wang wrote: > > > > > > We introduce a devfreq driver for the MediaTek Cache Coherent > > > > > > Interconnect > > > > > > (CCI) used by some MediaTek SoCs. > > > > > > > > > > > > In this driver, we use the passive devfreq driver to get > > > > > > target > > > > > > frequencies > > > > > > and adjust voltages accordingly. In MT8183 and MT8186, the > > > > > > MediaTek > > > > > > CCI > > > > > > is supplied by the same regulators with the little core CPUs. > > > > > > > > > > > > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > > > > > > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > > > > > > --- > > > > > > This patch depends on "devfreq-testing"[1]. > > > > > > [1] > > > > > > > > > > > > > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing__;!!CTRNKA9wMg0ARbw!wnTkI_sh3TsliBPFP70DFwtSZGtKyPinFu9h2BwULWX-xrWF0C21rLV-n1HhstfmLw42$ > > > > > > > > > > > > --- > > > > > > drivers/devfreq/Kconfig | 10 + > > > > > > drivers/devfreq/Makefile | 1 + > > > > > > drivers/devfreq/mtk-cci-devfreq.c | 474 > > > > > > ++++++++++++++++++++++++++++++ > > > > > > 3 files changed, 485 insertions(+) > > > > > > create mode 100644 drivers/devfreq/mtk-cci-devfreq.c > > > > > > > > > > > > diff --git a/drivers/devfreq/Kconfig > > > > > > b/drivers/devfreq/Kconfig > > > > > > index 87eb2b837e68..9754d8b31621 100644 > > > > > > --- a/drivers/devfreq/Kconfig > > > > > > +++ b/drivers/devfreq/Kconfig > > > > > > @@ -120,6 +120,16 @@ config ARM_TEGRA_DEVFREQ > > > > > > It reads ACTMON counters of memory controllers and > > > > > > adjusts > > > > > > the > > > > > > operating frequencies and voltages with OPP support. > > > > > > > > > > > > +config ARM_MEDIATEK_CCI_DEVFREQ > > > > > > + tristate "MEDIATEK CCI DEVFREQ Driver" > > > > > > + depends on ARM_MEDIATEK_CPUFREQ || COMPILE_TEST > > > > > > + select DEVFREQ_GOV_PASSIVE > > > > > > + help > > > > > > + This adds a devfreq driver for MediaTek Cache > > > > > > Coherent > > > > > > Interconnect > > > > > > + which is shared the same regulators with the cpu > > > > > > cluster. It > > > > > > can track > > > > > > + buck voltages and update a proper CCI frequency. Use > > > > > > the > > > > > > notification > > > > > > + to get the regulator status. > > > > > > + > > > > > > config ARM_RK3399_DMC_DEVFREQ > > > > > > tristate "ARM RK3399 DMC DEVFREQ Driver" > > > > > > depends on (ARCH_ROCKCHIP && HAVE_ARM_SMCCC) || \ > > > > > > diff --git a/drivers/devfreq/Makefile > > > > > > b/drivers/devfreq/Makefile > > > > > > index 0b6be92a25d9..bf40d04928d0 100644 > > > > > > --- a/drivers/devfreq/Makefile > > > > > > +++ b/drivers/devfreq/Makefile > > > > > > @@ -11,6 +11,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += > > > > > > governor_passive.o > > > > > > obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o > > > > > > obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ) += imx-bus.o > > > > > > obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ) += imx8m-ddrc.o > > > > > > +obj-$(CONFIG_ARM_MEDIATEK_CCI_DEVFREQ) += mtk-cci- > > > > > > devfreq.o > > > > > > obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o > > > > > > obj-$(CONFIG_ARM_SUN8I_A33_MBUS_DEVFREQ) += sun8i-a33- > > > > > > mbus.o > > > > > > obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30- > > > > > > devfreq.o > > > > > > diff --git a/drivers/devfreq/mtk-cci-devfreq.c > > > > > > b/drivers/devfreq/mtk- > > > > > > cci-devfreq.c > > > > > > new file mode 100644 > > > > > > index 000000000000..b3e31c45a57c > > > > > > --- /dev/null > > > > > > +++ b/drivers/devfreq/mtk-cci-devfreq.c > > > > > > @@ -0,0 +1,474 @@ > > > > > > +// SPDX-License-Identifier: GPL-2.0-only > > > > > > +/* > > > > > > + * Copyright (C) 2022 MediaTek Inc. > > > > > > + */ > > > > > > + > > > > > > +#include <linux/clk.h> > > > > > > +#include <linux/devfreq.h> > > > > > > +#include <linux/minmax.h> > > > > > > +#include <linux/module.h> > > > > > > +#include <linux/of.h> > > > > > > +#include <linux/of_device.h> > > > > > > +#include <linux/platform_device.h> > > > > > > +#include <linux/pm_opp.h> > > > > > > +#include <linux/regulator/consumer.h> > > > > > > + > > > > > > +struct mtk_ccifreq_platform_data { > > > > > > + int min_volt_shift; > > > > > > + int max_volt_shift; > > > > > > + int proc_max_volt; > > > > > > + int sram_min_volt; > > > > > > + int sram_max_volt; > > > > > > +}; > > > > > > + > > > > > > +struct mtk_ccifreq_drv { > > > > > > + struct device *dev; > > > > > > + struct devfreq *devfreq; > > > > > > + struct regulator *proc_reg; > > > > > > + struct regulator *sram_reg; > > > > > > + struct clk *cci_clk; > > > > > > + struct clk *inter_clk; > > > > > > + int inter_voltage; > > > > > > + int pre_voltage; > > > > > > + unsigned long pre_freq; > > > > > > + /* Avoid race condition for regulators between notify > > > > > > and > > > > > > policy */ > > > > > > + struct mutex reg_lock; > > > > > > + struct notifier_block opp_nb; > > > > > > + const struct mtk_ccifreq_platform_data *soc_data; > > > > > > + int vtrack_max; > > > > > > +}; > > > > > > + > > > > > > +static int mtk_ccifreq_set_voltage(struct mtk_ccifreq_drv > > > > > > *drv, > > > > > > int > > > > > > new_voltage) > > > > > > +{ > > > > > > + const struct mtk_ccifreq_platform_data *soc_data = drv- > > > > > > > soc_data; > > > > > > > > > > > > + struct device *dev = drv->dev; > > > > > > + int pre_voltage, pre_vsram, new_vsram, vsram, voltage, > > > > > > ret; > > > > > > + int retry_max = drv->vtrack_max; > > > > > > + > > > > > > + if (!drv->sram_reg) { > > > > > > + ret = regulator_set_voltage(drv->proc_reg, > > > > > > new_voltage, > > > > > > + drv->soc_data- > > > > > > > proc_max_volt); > > > > > > > > > > > > + goto out_set_voltage; > > > > > > + } > > > > > > + > > > > > > + pre_voltage = regulator_get_voltage(drv->proc_reg); > > > > > > + if (pre_voltage < 0) { > > > > > > + dev_err(dev, "invalid vproc value: %d\n", > > > > > > pre_voltage); > > > > > > + return pre_voltage; > > > > > > + } > > > > > > + > > > > > > + pre_vsram = regulator_get_voltage(drv->sram_reg); > > > > > > + if (pre_vsram < 0) { > > > > > > + dev_err(dev, "invalid vsram value: %d\n", > > > > > > pre_vsram); > > > > > > + return pre_vsram; > > > > > > + } > > > > > > + > > > > > > + new_vsram = clamp(new_voltage + soc_data- > > > > > > >min_volt_shift, > > > > > > + soc_data->sram_min_volt, soc_data- > > > > > > > sram_max_volt); > > > > > > > > > > > > + > > > > > > + do { > > > > > > + if (pre_voltage <= new_voltage) { > > > > > > + vsram = clamp(pre_voltage + soc_data- > > > > > > > max_volt_shift, > > > > > > > > > > > > + soc_data->sram_min_volt, > > > > > > new_vsram); > > > > > > + ret = regulator_set_voltage(drv- > > > > > > >sram_reg, > > > > > > vsram, > > > > > > + soc_data- > > > > > > > sram_max_volt); > > > > > > > > > > > > + if (ret) > > > > > > + return ret; > > > > > > + > > > > > > + if (vsram == soc_data->sram_max_volt || > > > > > > + new_vsram == soc_data- > > > > > > >sram_min_volt) > > > > > > + voltage = new_voltage; > > > > > > + else > > > > > > + voltage = vsram - soc_data- > > > > > > > min_volt_shift; > > > > > > > > > > > > + > > > > > > + ret = regulator_set_voltage(drv- > > > > > > >proc_reg, > > > > > > voltage, > > > > > > + soc_data- > > > > > > > proc_max_volt); > > > > > > > > > > > > + if (ret) { > > > > > > + regulator_set_voltage(drv- > > > > > > >sram_reg, > > > > > > pre_vsram, > > > > > > + soc_data- > > > > > > > sram_max_volt); > > > > > > > > > > > > + return ret; > > > > > > + } > > > > > > + } else if (pre_voltage > new_voltage) { > > > > > > + voltage = max(new_voltage, > > > > > > + pre_vsram - soc_data- > > > > > > > max_volt_shift); > > > > > > > > > > > > + ret = regulator_set_voltage(drv- > > > > > > >proc_reg, > > > > > > voltage, > > > > > > + soc_data- > > > > > > > proc_max_volt); > > > > > > > > > > > > + if (ret) > > > > > > + return ret; > > > > > > + > > > > > > + if (voltage == new_voltage) > > > > > > + vsram = new_vsram; > > > > > > + else > > > > > > + vsram = max(new_vsram, > > > > > > + voltage + soc_data- > > > > > > > min_volt_shift); > > > > > > > > > > > > + > > > > > > + ret = regulator_set_voltage(drv- > > > > > > >sram_reg, > > > > > > vsram, > > > > > > + soc_data- > > > > > > > sram_max_volt); > > > > > > > > > > > > + if (ret) { > > > > > > + regulator_set_voltage(drv- > > > > > > >proc_reg, > > > > > > pre_voltage, > > > > > > + soc_data- > > > > > > > proc_max_volt); > > > > > > > > > > > > + return ret; > > > > > > + } > > > > > > + } > > > > > > + > > > > > > + pre_voltage = voltage; > > > > > > + pre_vsram = vsram; > > > > > > + > > > > > > + if (--retry_max < 0) { > > > > > > + dev_err(dev, > > > > > > + "over loop count, failed to set > > > > > > voltage\n"); > > > > > > + return -EINVAL; > > > > > > + } > > > > > > + } while (voltage != new_voltage || vsram != new_vsram); > > > > > > + > > > > > > +out_set_voltage: > > > > > > + if (!ret) > > > > > > + drv->pre_voltage = new_voltage; > > > > > > + > > > > > > + return ret; > > > > > > +} > > > > > > + > > > > > > +static int mtk_ccifreq_target(struct device *dev, unsigned > > > > > > long > > > > > > *freq, > > > > > > + u32 flags) > > > > > > +{ > > > > > > + struct mtk_ccifreq_drv *drv = dev_get_drvdata(dev); > > > > > > + struct clk *cci_pll = clk_get_parent(drv->cci_clk); > > > > > > + struct dev_pm_opp *opp; > > > > > > + unsigned long opp_rate; > > > > > > + int voltage, pre_voltage, inter_voltage, > > > > > > target_voltage, ret; > > > > > > + > > > > > > + if (!drv) > > > > > > + return -EINVAL; > > > > > > + > > > > > > + if (drv->pre_freq == *freq) > > > > > > + return 0; > > > > > > + > > > > > > + inter_voltage = drv->inter_voltage; > > > > > > + > > > > > > + opp_rate = *freq; > > > > > > + opp = devfreq_recommended_opp(dev, &opp_rate, 1); > > > > > > + if (IS_ERR(opp)) { > > > > > > + dev_err(dev, "failed to find opp for freq: > > > > > > %ld\n", > > > > > > opp_rate); > > > > > > + return PTR_ERR(opp); > > > > > > + } > > > > > > + > > > > > > + mutex_lock(&drv->reg_lock); > > > > > > + > > > > > > + voltage = dev_pm_opp_get_voltage(opp); > > > > > > + dev_pm_opp_put(opp); > > > > > > + > > > > > > + if (unlikely(drv->pre_voltage <= 0)) > > > > > > + pre_voltage = regulator_get_voltage(drv- > > > > > > >proc_reg); > > > > > > + else > > > > > > + pre_voltage = drv->pre_voltage; > > > > > > + > > > > > > + if (pre_voltage < 0) { > > > > > > + dev_err(dev, "invalid vproc value: %d\n", > > > > > > pre_voltage); > > > > > > + return pre_voltage; > > > > > > + } > > > > > > + > > > > > > + /* scale up: set voltage first then freq. */ > > > > > > + target_voltage = max(inter_voltage, voltage); > > > > > > + if (pre_voltage <= target_voltage) { > > > > > > + ret = mtk_ccifreq_set_voltage(drv, > > > > > > target_voltage); > > > > > > + if (ret) { > > > > > > + dev_err(dev, "failed to scale up > > > > > > voltage\n"); > > > > > > + goto out_restore_voltage; > > > > > > + } > > > > > > + } > > > > > > + > > > > > > + /* switch the cci clock to intermediate clock source. > > > > > > */ > > > > > > + ret = clk_set_parent(drv->cci_clk, drv->inter_clk); > > > > > > + if (ret) { > > > > > > + dev_err(dev, "failed to re-parent cci > > > > > > clock\n"); > > > > > > + goto out_restore_voltage; > > > > > > + } > > > > > > + > > > > > > + /* set the original clock to target rate. */ > > > > > > + ret = clk_set_rate(cci_pll, *freq); > > > > > > + if (ret) { > > > > > > + dev_err(dev, "failed to set cci pll rate: > > > > > > %d\n", ret); > > > > > > + clk_set_parent(drv->cci_clk, cci_pll); > > > > > > + goto out_restore_voltage; > > > > > > + } > > > > > > + > > > > > > + /* switch the cci clock back to the original clock > > > > > > source. */ > > > > > > + ret = clk_set_parent(drv->cci_clk, cci_pll); > > > > > > + if (ret) { > > > > > > + dev_err(dev, "failed to re-parent cci > > > > > > clock\n"); > > > > > > + mtk_ccifreq_set_voltage(drv, inter_voltage); > > > > > > + goto out_unlock; > > > > > > + } > > > > > > + > > > > > > + /* > > > > > > + * If the new voltage is lower than the intermediate > > > > > > voltage or > > > > > > the > > > > > > + * original voltage, scale down to the new voltage. > > > > > > + */ > > > > > > + if (voltage < inter_voltage || voltage < pre_voltage) { > > > > > > + ret = mtk_ccifreq_set_voltage(drv, voltage); > > > > > > + if (ret) { > > > > > > + dev_err(dev, "failed to scale down > > > > > > voltage\n"); > > > > > > + goto out_unlock; > > > > > > + } > > > > > > + } > > > > > > + > > > > > > + drv->pre_freq = *freq; > > > > > > + mutex_unlock(&drv->reg_lock); > > > > > > + > > > > > > + return 0; > > > > > > + > > > > > > +out_restore_voltage: > > > > > > + mtk_ccifreq_set_voltage(drv, pre_voltage); > > > > > > + > > > > > > +out_unlock: > > > > > > + mutex_unlock(&drv->reg_lock); > > > > > > + return ret; > > > > > > +} > > > > > > + > > > > > > +static int mtk_ccifreq_opp_notifier(struct notifier_block > > > > > > *nb, > > > > > > + unsigned long event, void > > > > > > *data) > > > > > > +{ > > > > > > + struct dev_pm_opp *opp = data; > > > > > > + struct mtk_ccifreq_drv *drv; > > > > > > + unsigned long freq, volt; > > > > > > + > > > > > > + drv = container_of(nb, struct mtk_ccifreq_drv, opp_nb); > > > > > > + > > > > > > + if (event == OPP_EVENT_ADJUST_VOLTAGE) { > > > > > > + freq = dev_pm_opp_get_freq(opp); > > > > > > + > > > > > > + mutex_lock(&drv->reg_lock); > > > > > > + /* current opp item is changed */ > > > > > > + if (freq == drv->pre_freq) { > > > > > > + volt = dev_pm_opp_get_voltage(opp); > > > > > > + mtk_ccifreq_set_voltage(drv, volt); > > > > > > + } > > > > > > + mutex_unlock(&drv->reg_lock); > > > > > > + } > > > > > > + > > > > > > + return 0; > > > > > > +} > > > > > > + > > > > > > +static struct devfreq_dev_profile mtk_ccifreq_profile = { > > > > > > + .target = mtk_ccifreq_target, > > > > > > +}; > > > > > > + > > > > > > +static int mtk_ccifreq_probe(struct platform_device *pdev) > > > > > > +{ > > > > > > + struct device *dev = &pdev->dev; > > > > > > + struct mtk_ccifreq_drv *drv; > > > > > > + struct devfreq_passive_data *passive_data; > > > > > > + struct dev_pm_opp *opp; > > > > > > + unsigned long rate, opp_volt; > > > > > > + int ret; > > > > > > + > > > > > > + drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL); > > > > > > + if (!drv) > > > > > > + return -ENOMEM; > > > > > > + > > > > > > + drv->dev = dev; > > > > > > + drv->soc_data = (const struct mtk_ccifreq_platform_data > > > > > > *) > > > > > > + of_device_get_match_data(&pdev- > > > > > > >dev); > > > > > > + mutex_init(&drv->reg_lock); > > > > > > + platform_set_drvdata(pdev, drv); > > > > > > + > > > > > > + drv->cci_clk = devm_clk_get(dev, "cci"); > > > > > > + if (IS_ERR(drv->cci_clk)) { > > > > > > + ret = PTR_ERR(drv->cci_clk); > > > > > > + return dev_err_probe(dev, ret, > > > > > > + "failed to get cci clk: > > > > > > %d\n", > > > > > > ret); > > > > > > + } > > > > > > + > > > > > > + drv->inter_clk = devm_clk_get(dev, "intermediate"); > > > > > > + if (IS_ERR(drv->inter_clk)) { > > > > > > + ret = PTR_ERR(drv->inter_clk); > > > > > > + dev_err_probe(dev, ret, > > > > > > + "failed to get intermediate clk: > > > > > > %d\n", > > > > > > ret); > > > > > > + goto out_free_resources; > > > > > > + } > > > > > > + > > > > > > + drv->proc_reg = devm_regulator_get_optional(dev, > > > > > > "proc"); > > > > > > + if (IS_ERR(drv->proc_reg)) { > > > > > > + ret = PTR_ERR(drv->proc_reg); > > > > > > + dev_err_probe(dev, ret, > > > > > > + "failed to get proc regulator: > > > > > > %d\n", > > > > > > ret); > > > > > > + goto out_free_resources; > > > > > > + } > > > > > > + > > > > > > + ret = regulator_enable(drv->proc_reg); > > > > > > + if (ret) { > > > > > > + dev_err(dev, "failed to enable proc > > > > > > regulator\n"); > > > > > > + goto out_free_resources; > > > > > > + } > > > > > > + > > > > > > + drv->sram_reg = regulator_get_optional(dev, "sram"); > > > > > > + if (IS_ERR(drv->sram_reg)) > > > > > > + drv->sram_reg = NULL; > > > > > > + else { > > > > > > + ret = regulator_enable(drv->sram_reg); > > > > > > + if (ret) { > > > > > > + dev_err(dev, "failed to enable sram > > > > > > regulator\n"); > > > > > > + goto out_free_resources; > > > > > > + } > > > > > > + } > > > > > > + > > > > > > + /* > > > > > > + * We assume min voltage is 0 and tracking target > > > > > > voltage using > > > > > > + * min_volt_shift for each iteration. > > > > > > + * The retry_max is 3 times of expeted iteration count. > > > > > > + */ > > > > > > + drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv->soc_data- > > > > > > > sram_max_volt, > > > > > > > > > > > > + drv->soc_data- > > > > > > > proc_max_volt), > > > > > > > > > > > > + drv->soc_data- > > > > > > > min_volt_shift); > > > > > > > > > > > > + > > > > > > + ret = clk_prepare_enable(drv->cci_clk); > > > > > > + if (ret) > > > > > > + goto out_free_resources; > > > > > > + > > > > > > + ret = clk_prepare_enable(drv->inter_clk); > > > > > > + if (ret) > > > > > > + goto out_disable_cci_clk; > > > > > > + > > > > > > + ret = dev_pm_opp_of_add_table(dev); > > > > > > + if (ret) { > > > > > > + dev_err(dev, "failed to add opp table: %d\n", > > > > > > ret); > > > > > > + goto out_disable_inter_clk; > > > > > > + } > > > > > > + > > > > > > + rate = clk_get_rate(drv->inter_clk); > > > > > > + opp = dev_pm_opp_find_freq_ceil(dev, &rate); > > > > > > + if (IS_ERR(opp)) { > > > > > > + ret = PTR_ERR(opp); > > > > > > + dev_err(dev, "failed to get intermediate opp: > > > > > > %d\n", > > > > > > ret); > > > > > > + goto out_remove_opp_table; > > > > > > + } > > > > > > + drv->inter_voltage = dev_pm_opp_get_voltage(opp); > > > > > > + dev_pm_opp_put(opp); > > > > > > + > > > > > > + rate = U32_MAX; > > > > > > + opp = dev_pm_opp_find_freq_floor(drv->dev, &rate); > > > > > > + if (IS_ERR(opp)) { > > > > > > + dev_err(dev, "failed to get opp\n"); > > > > > > + ret = PTR_ERR(opp); > > > > > > + goto out_remove_opp_table; > > > > > > + } > > > > > > + > > > > > > + opp_volt = dev_pm_opp_get_voltage(opp); > > > > > > + dev_pm_opp_put(opp); > > > > > > + ret = mtk_ccifreq_set_voltage(drv, opp_volt); > > > > > > + if (ret) { > > > > > > + dev_err(dev, "failed to scale to highest > > > > > > voltage %lu in > > > > > > proc_reg\n", > > > > > > + opp_volt); > > > > > > + goto out_remove_opp_table; > > > > > > + } > > > > > > + > > > > > > + passive_data = devm_kzalloc(dev, sizeof(struct > > > > > > devfreq_passive_data), > > > > > > + GFP_KERNEL); > > > > > > + if (!passive_data) { > > > > > > + ret = -ENOMEM; > > > > > > + goto out_remove_opp_table; > > > > > > + } > > > > > > + > > > > > > + passive_data->parent_type = CPUFREQ_PARENT_DEV; > > > > > > + drv->devfreq = devm_devfreq_add_device(dev, > > > > > > &mtk_ccifreq_profile, > > > > > > + DEVFREQ_GOV_PASS > > > > > > IVE, > > > > > > + passive_data); > > > > > > + if (IS_ERR(drv->devfreq)) { > > > > > > + ret = -EPROBE_DEFER; > > > > > > + dev_err(dev, "failed to add devfreq device: > > > > > > %d\n", > > > > > > + PTR_ERR(drv->devfreq)); > > > > > > + goto out_remove_opp_table; > > > > > > + } > > > > > > + > > > > > > + drv->opp_nb.notifier_call = mtk_ccifreq_opp_notifier; > > > > > > + ret = dev_pm_opp_register_notifier(dev, &drv->opp_nb); > > > > > > + if (ret) { > > > > > > + dev_err(dev, "failed to register opp notifier: > > > > > > %d\n", > > > > > > ret); > > > > > > + goto out_remove_devfreq_device; > > > > > > + } > > > > > > + return 0; > > > > > > + > > > > > > +out_remove_devfreq_device: > > > > > > + devm_devfreq_remove_device(dev, drv->devfreq); > > > > > > + > > > > > > +out_remove_opp_table: > > > > > > + dev_pm_opp_of_remove_table(dev); > > > > > > + > > > > > > +out_disable_inter_clk: > > > > > > + clk_disable_unprepare(drv->inter_clk); > > > > > > + > > > > > > +out_disable_cci_clk: > > > > > > + clk_disable_unprepare(drv->cci_clk); > > > > > > + > > > > > > +out_free_resources: > > > > > > + if (regulator_is_enabled(drv->proc_reg)) > > > > > > + regulator_disable(drv->proc_reg); > > > > > > + if (drv->sram_reg && regulator_is_enabled(drv- > > > > > > >sram_reg)) > > > > > > + regulator_disable(drv->sram_reg); > > > > > > + > > > > > > + if (!IS_ERR(drv->proc_reg)) > > > > > > + regulator_put(drv->proc_reg); > > > > > > + if (!IS_ERR(drv->sram_reg)) > > > > > > + regulator_put(drv->sram_reg); > > > > > > + if (!IS_ERR(drv->cci_clk)) > > > > > > + clk_put(drv->cci_clk); > > > > > > + if (!IS_ERR(drv->inter_clk)) > > > > > > + clk_put(drv->inter_clk); > > > > > > + > > > > > > + return ret; > > > > > > +} > > > > > > + > > > > > > +static int mtk_ccifreq_remove(struct platform_device *pdev) > > > > > > +{ > > > > > > + struct device *dev = &pdev->dev; > > > > > > + struct mtk_ccifreq_drv *drv; > > > > > > + > > > > > > + drv = platform_get_drvdata(pdev); > > > > > > + > > > > > > + dev_pm_opp_unregister_notifier(dev, &drv->opp_nb); > > > > > > + dev_pm_opp_of_remove_table(dev); > > > > > > + clk_disable_unprepare(drv->inter_clk); > > > > > > + clk_disable_unprepare(drv->cci_clk); > > > > > > + regulator_disable(drv->proc_reg); > > > > > > + if (drv->sram_reg) > > > > > > + regulator_disable(drv->sram_reg); > > > > > > + > > > > > > + return 0; > > > > > > +} > > > > > > + > > > > > > +static const struct mtk_ccifreq_platform_data > > > > > > mt8183_platform_data = > > > > > > { > > > > > > + .min_volt_shift = 100000, > > > > > > + .max_volt_shift = 200000, > > > > > > + .proc_max_volt = 1150000, > > > > > > + .sram_min_volt = 0, > > > > > > + .sram_max_volt = 1150000, > > > > > > +}; > > > > > > + > > > > > > +static const struct mtk_ccifreq_platform_data > > > > > > mt8186_platform_data = > > > > > > { > > > > > > + .min_volt_shift = 100000, > > > > > > + .max_volt_shift = 250000, > > > > > > + .proc_max_volt = 1118750, > > > > > > + .sram_min_volt = 850000, > > > > > > + .sram_max_volt = 1118750, > > > > > > +}; > > > > > > + > > > > > > +static const struct of_device_id mtk_ccifreq_machines[] = { > > > > > > + { .compatible = "mediatek,mt8183-cci", .data = > > > > > > &mt8183_platform_data }, > > > > > > + { .compatible = "mediatek,mt8186-cci", .data = > > > > > > &mt8186_platform_data }, > > > > > > + { }, > > > > > > +}; > > > > > > +MODULE_DEVICE_TABLE(of, mtk_ccifreq_machines); > > > > > > + > > > > > > +static struct platform_driver mtk_ccifreq_platdrv = { > > > > > > + .probe = mtk_ccifreq_probe, > > > > > > + .remove = mtk_ccifreq_remove, > > > > > > + .driver = { > > > > > > + .name = "mtk-ccifreq", > > > > > > + .of_match_table = mtk_ccifreq_machines, > > > > > > + }, > > > > > > +}; > > > > > > +module_platform_driver(mtk_ccifreq_platdrv); > > > > > > + > > > > > > +MODULE_DESCRIPTION("MediaTek CCI devfreq driver"); > > > > > > +MODULE_AUTHOR("Jia-Wei Chang <jia-wei.chang@mediatek.com>"); > > > > > > +MODULE_LICENSE("GPL v2"); > > > > > > > > > > Hi Chanwoo, > > > > > > > > > > Just a kindly ping. > > > > > Could you please give me some suggestion on this patch? > > > > > Thanks! > > > > > > > > > > BRs, > > > > > Johnson Wang > > > > > > > > > > > > > Hi Johnson, > > > > > > > > Sorry for late reply.But I replied it yesterday as following: > > > > Maybe, this reply[1] has not yet arrrived at your mail box. > > > > [1] > > > > > > > > > > > https://lore.kernel.org/lkml/0846a062-2ea2-e6d4-03c8-992c2f2e24a0@gmail.com/T/#m3bc609d805d6e74ec7a105be0511926b4afbbaef > > > > > > > > As I described on reply[1], I updated the patches on devfreq- > > > > testing > > > > branch. Could you please test your patches based on devfreq- > > > > testing > > > > branch? > > > > > > > > > > Hi Chanwoo, > > > > > > Thanks for your information. > > > I've tested this patch based on the latest devfreq-testing branch. > > > It encounters the same crash as Chen-Yu mentioned[1]. > > > > > > [1] > > > https://lore.kernel.org/linux-pm/YniX1w+oI1eOCmCx@google.com/T/#u > > > > Hi Johnson, > > > > Thanks for the test. I'll drop the last patch caused of crash. > > And I'll send v3 patchset right now. > > > > > > Hi Chanwoo, > > With your v3 patchset, this CCI devfreq driver works properly on the > target. > Thanks! Hi Johnson, Thanks for the test. I'll send v4 with a Tested-by tag and some typo fixup And then I'll merge them. As I know, you will send the next version of mediatek cci driver. After that, I'll merge your patches. Thanks.
diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig index 87eb2b837e68..9754d8b31621 100644 --- a/drivers/devfreq/Kconfig +++ b/drivers/devfreq/Kconfig @@ -120,6 +120,16 @@ config ARM_TEGRA_DEVFREQ It reads ACTMON counters of memory controllers and adjusts the operating frequencies and voltages with OPP support. +config ARM_MEDIATEK_CCI_DEVFREQ + tristate "MEDIATEK CCI DEVFREQ Driver" + depends on ARM_MEDIATEK_CPUFREQ || COMPILE_TEST + select DEVFREQ_GOV_PASSIVE + help + This adds a devfreq driver for MediaTek Cache Coherent Interconnect + which is shared the same regulators with the cpu cluster. It can track + buck voltages and update a proper CCI frequency. Use the notification + to get the regulator status. + config ARM_RK3399_DMC_DEVFREQ tristate "ARM RK3399 DMC DEVFREQ Driver" depends on (ARCH_ROCKCHIP && HAVE_ARM_SMCCC) || \ diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile index 0b6be92a25d9..bf40d04928d0 100644 --- a/drivers/devfreq/Makefile +++ b/drivers/devfreq/Makefile @@ -11,6 +11,7 @@ obj-$(CONFIG_DEVFREQ_GOV_PASSIVE) += governor_passive.o obj-$(CONFIG_ARM_EXYNOS_BUS_DEVFREQ) += exynos-bus.o obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ) += imx-bus.o obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ) += imx8m-ddrc.o +obj-$(CONFIG_ARM_MEDIATEK_CCI_DEVFREQ) += mtk-cci-devfreq.o obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ) += rk3399_dmc.o obj-$(CONFIG_ARM_SUN8I_A33_MBUS_DEVFREQ) += sun8i-a33-mbus.o obj-$(CONFIG_ARM_TEGRA_DEVFREQ) += tegra30-devfreq.o diff --git a/drivers/devfreq/mtk-cci-devfreq.c b/drivers/devfreq/mtk-cci-devfreq.c new file mode 100644 index 000000000000..b3e31c45a57c --- /dev/null +++ b/drivers/devfreq/mtk-cci-devfreq.c @@ -0,0 +1,474 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2022 MediaTek Inc. + */ + +#include <linux/clk.h> +#include <linux/devfreq.h> +#include <linux/minmax.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> +#include <linux/pm_opp.h> +#include <linux/regulator/consumer.h> + +struct mtk_ccifreq_platform_data { + int min_volt_shift; + int max_volt_shift; + int proc_max_volt; + int sram_min_volt; + int sram_max_volt; +}; + +struct mtk_ccifreq_drv { + struct device *dev; + struct devfreq *devfreq; + struct regulator *proc_reg; + struct regulator *sram_reg; + struct clk *cci_clk; + struct clk *inter_clk; + int inter_voltage; + int pre_voltage; + unsigned long pre_freq; + /* Avoid race condition for regulators between notify and policy */ + struct mutex reg_lock; + struct notifier_block opp_nb; + const struct mtk_ccifreq_platform_data *soc_data; + int vtrack_max; +}; + +static int mtk_ccifreq_set_voltage(struct mtk_ccifreq_drv *drv, int new_voltage) +{ + const struct mtk_ccifreq_platform_data *soc_data = drv->soc_data; + struct device *dev = drv->dev; + int pre_voltage, pre_vsram, new_vsram, vsram, voltage, ret; + int retry_max = drv->vtrack_max; + + if (!drv->sram_reg) { + ret = regulator_set_voltage(drv->proc_reg, new_voltage, + drv->soc_data->proc_max_volt); + goto out_set_voltage; + } + + pre_voltage = regulator_get_voltage(drv->proc_reg); + if (pre_voltage < 0) { + dev_err(dev, "invalid vproc value: %d\n", pre_voltage); + return pre_voltage; + } + + pre_vsram = regulator_get_voltage(drv->sram_reg); + if (pre_vsram < 0) { + dev_err(dev, "invalid vsram value: %d\n", pre_vsram); + return pre_vsram; + } + + new_vsram = clamp(new_voltage + soc_data->min_volt_shift, + soc_data->sram_min_volt, soc_data->sram_max_volt); + + do { + if (pre_voltage <= new_voltage) { + vsram = clamp(pre_voltage + soc_data->max_volt_shift, + soc_data->sram_min_volt, new_vsram); + ret = regulator_set_voltage(drv->sram_reg, vsram, + soc_data->sram_max_volt); + if (ret) + return ret; + + if (vsram == soc_data->sram_max_volt || + new_vsram == soc_data->sram_min_volt) + voltage = new_voltage; + else + voltage = vsram - soc_data->min_volt_shift; + + ret = regulator_set_voltage(drv->proc_reg, voltage, + soc_data->proc_max_volt); + if (ret) { + regulator_set_voltage(drv->sram_reg, pre_vsram, + soc_data->sram_max_volt); + return ret; + } + } else if (pre_voltage > new_voltage) { + voltage = max(new_voltage, + pre_vsram - soc_data->max_volt_shift); + ret = regulator_set_voltage(drv->proc_reg, voltage, + soc_data->proc_max_volt); + if (ret) + return ret; + + if (voltage == new_voltage) + vsram = new_vsram; + else + vsram = max(new_vsram, + voltage + soc_data->min_volt_shift); + + ret = regulator_set_voltage(drv->sram_reg, vsram, + soc_data->sram_max_volt); + if (ret) { + regulator_set_voltage(drv->proc_reg, pre_voltage, + soc_data->proc_max_volt); + return ret; + } + } + + pre_voltage = voltage; + pre_vsram = vsram; + + if (--retry_max < 0) { + dev_err(dev, + "over loop count, failed to set voltage\n"); + return -EINVAL; + } + } while (voltage != new_voltage || vsram != new_vsram); + +out_set_voltage: + if (!ret) + drv->pre_voltage = new_voltage; + + return ret; +} + +static int mtk_ccifreq_target(struct device *dev, unsigned long *freq, + u32 flags) +{ + struct mtk_ccifreq_drv *drv = dev_get_drvdata(dev); + struct clk *cci_pll = clk_get_parent(drv->cci_clk); + struct dev_pm_opp *opp; + unsigned long opp_rate; + int voltage, pre_voltage, inter_voltage, target_voltage, ret; + + if (!drv) + return -EINVAL; + + if (drv->pre_freq == *freq) + return 0; + + inter_voltage = drv->inter_voltage; + + opp_rate = *freq; + opp = devfreq_recommended_opp(dev, &opp_rate, 1); + if (IS_ERR(opp)) { + dev_err(dev, "failed to find opp for freq: %ld\n", opp_rate); + return PTR_ERR(opp); + } + + mutex_lock(&drv->reg_lock); + + voltage = dev_pm_opp_get_voltage(opp); + dev_pm_opp_put(opp); + + if (unlikely(drv->pre_voltage <= 0)) + pre_voltage = regulator_get_voltage(drv->proc_reg); + else + pre_voltage = drv->pre_voltage; + + if (pre_voltage < 0) { + dev_err(dev, "invalid vproc value: %d\n", pre_voltage); + return pre_voltage; + } + + /* scale up: set voltage first then freq. */ + target_voltage = max(inter_voltage, voltage); + if (pre_voltage <= target_voltage) { + ret = mtk_ccifreq_set_voltage(drv, target_voltage); + if (ret) { + dev_err(dev, "failed to scale up voltage\n"); + goto out_restore_voltage; + } + } + + /* switch the cci clock to intermediate clock source. */ + ret = clk_set_parent(drv->cci_clk, drv->inter_clk); + if (ret) { + dev_err(dev, "failed to re-parent cci clock\n"); + goto out_restore_voltage; + } + + /* set the original clock to target rate. */ + ret = clk_set_rate(cci_pll, *freq); + if (ret) { + dev_err(dev, "failed to set cci pll rate: %d\n", ret); + clk_set_parent(drv->cci_clk, cci_pll); + goto out_restore_voltage; + } + + /* switch the cci clock back to the original clock source. */ + ret = clk_set_parent(drv->cci_clk, cci_pll); + if (ret) { + dev_err(dev, "failed to re-parent cci clock\n"); + mtk_ccifreq_set_voltage(drv, inter_voltage); + goto out_unlock; + } + + /* + * If the new voltage is lower than the intermediate voltage or the + * original voltage, scale down to the new voltage. + */ + if (voltage < inter_voltage || voltage < pre_voltage) { + ret = mtk_ccifreq_set_voltage(drv, voltage); + if (ret) { + dev_err(dev, "failed to scale down voltage\n"); + goto out_unlock; + } + } + + drv->pre_freq = *freq; + mutex_unlock(&drv->reg_lock); + + return 0; + +out_restore_voltage: + mtk_ccifreq_set_voltage(drv, pre_voltage); + +out_unlock: + mutex_unlock(&drv->reg_lock); + return ret; +} + +static int mtk_ccifreq_opp_notifier(struct notifier_block *nb, + unsigned long event, void *data) +{ + struct dev_pm_opp *opp = data; + struct mtk_ccifreq_drv *drv; + unsigned long freq, volt; + + drv = container_of(nb, struct mtk_ccifreq_drv, opp_nb); + + if (event == OPP_EVENT_ADJUST_VOLTAGE) { + freq = dev_pm_opp_get_freq(opp); + + mutex_lock(&drv->reg_lock); + /* current opp item is changed */ + if (freq == drv->pre_freq) { + volt = dev_pm_opp_get_voltage(opp); + mtk_ccifreq_set_voltage(drv, volt); + } + mutex_unlock(&drv->reg_lock); + } + + return 0; +} + +static struct devfreq_dev_profile mtk_ccifreq_profile = { + .target = mtk_ccifreq_target, +}; + +static int mtk_ccifreq_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct mtk_ccifreq_drv *drv; + struct devfreq_passive_data *passive_data; + struct dev_pm_opp *opp; + unsigned long rate, opp_volt; + int ret; + + drv = devm_kzalloc(dev, sizeof(*drv), GFP_KERNEL); + if (!drv) + return -ENOMEM; + + drv->dev = dev; + drv->soc_data = (const struct mtk_ccifreq_platform_data *) + of_device_get_match_data(&pdev->dev); + mutex_init(&drv->reg_lock); + platform_set_drvdata(pdev, drv); + + drv->cci_clk = devm_clk_get(dev, "cci"); + if (IS_ERR(drv->cci_clk)) { + ret = PTR_ERR(drv->cci_clk); + return dev_err_probe(dev, ret, + "failed to get cci clk: %d\n", ret); + } + + drv->inter_clk = devm_clk_get(dev, "intermediate"); + if (IS_ERR(drv->inter_clk)) { + ret = PTR_ERR(drv->inter_clk); + dev_err_probe(dev, ret, + "failed to get intermediate clk: %d\n", ret); + goto out_free_resources; + } + + drv->proc_reg = devm_regulator_get_optional(dev, "proc"); + if (IS_ERR(drv->proc_reg)) { + ret = PTR_ERR(drv->proc_reg); + dev_err_probe(dev, ret, + "failed to get proc regulator: %d\n", ret); + goto out_free_resources; + } + + ret = regulator_enable(drv->proc_reg); + if (ret) { + dev_err(dev, "failed to enable proc regulator\n"); + goto out_free_resources; + } + + drv->sram_reg = regulator_get_optional(dev, "sram"); + if (IS_ERR(drv->sram_reg)) + drv->sram_reg = NULL; + else { + ret = regulator_enable(drv->sram_reg); + if (ret) { + dev_err(dev, "failed to enable sram regulator\n"); + goto out_free_resources; + } + } + + /* + * We assume min voltage is 0 and tracking target voltage using + * min_volt_shift for each iteration. + * The retry_max is 3 times of expeted iteration count. + */ + drv->vtrack_max = 3 * DIV_ROUND_UP(max(drv->soc_data->sram_max_volt, + drv->soc_data->proc_max_volt), + drv->soc_data->min_volt_shift); + + ret = clk_prepare_enable(drv->cci_clk); + if (ret) + goto out_free_resources; + + ret = clk_prepare_enable(drv->inter_clk); + if (ret) + goto out_disable_cci_clk; + + ret = dev_pm_opp_of_add_table(dev); + if (ret) { + dev_err(dev, "failed to add opp table: %d\n", ret); + goto out_disable_inter_clk; + } + + rate = clk_get_rate(drv->inter_clk); + opp = dev_pm_opp_find_freq_ceil(dev, &rate); + if (IS_ERR(opp)) { + ret = PTR_ERR(opp); + dev_err(dev, "failed to get intermediate opp: %d\n", ret); + goto out_remove_opp_table; + } + drv->inter_voltage = dev_pm_opp_get_voltage(opp); + dev_pm_opp_put(opp); + + rate = U32_MAX; + opp = dev_pm_opp_find_freq_floor(drv->dev, &rate); + if (IS_ERR(opp)) { + dev_err(dev, "failed to get opp\n"); + ret = PTR_ERR(opp); + goto out_remove_opp_table; + } + + opp_volt = dev_pm_opp_get_voltage(opp); + dev_pm_opp_put(opp); + ret = mtk_ccifreq_set_voltage(drv, opp_volt); + if (ret) { + dev_err(dev, "failed to scale to highest voltage %lu in proc_reg\n", + opp_volt); + goto out_remove_opp_table; + } + + passive_data = devm_kzalloc(dev, sizeof(struct devfreq_passive_data), + GFP_KERNEL); + if (!passive_data) { + ret = -ENOMEM; + goto out_remove_opp_table; + } + + passive_data->parent_type = CPUFREQ_PARENT_DEV; + drv->devfreq = devm_devfreq_add_device(dev, &mtk_ccifreq_profile, + DEVFREQ_GOV_PASSIVE, + passive_data); + if (IS_ERR(drv->devfreq)) { + ret = -EPROBE_DEFER; + dev_err(dev, "failed to add devfreq device: %d\n", + PTR_ERR(drv->devfreq)); + goto out_remove_opp_table; + } + + drv->opp_nb.notifier_call = mtk_ccifreq_opp_notifier; + ret = dev_pm_opp_register_notifier(dev, &drv->opp_nb); + if (ret) { + dev_err(dev, "failed to register opp notifier: %d\n", ret); + goto out_remove_devfreq_device; + } + return 0; + +out_remove_devfreq_device: + devm_devfreq_remove_device(dev, drv->devfreq); + +out_remove_opp_table: + dev_pm_opp_of_remove_table(dev); + +out_disable_inter_clk: + clk_disable_unprepare(drv->inter_clk); + +out_disable_cci_clk: + clk_disable_unprepare(drv->cci_clk); + +out_free_resources: + if (regulator_is_enabled(drv->proc_reg)) + regulator_disable(drv->proc_reg); + if (drv->sram_reg && regulator_is_enabled(drv->sram_reg)) + regulator_disable(drv->sram_reg); + + if (!IS_ERR(drv->proc_reg)) + regulator_put(drv->proc_reg); + if (!IS_ERR(drv->sram_reg)) + regulator_put(drv->sram_reg); + if (!IS_ERR(drv->cci_clk)) + clk_put(drv->cci_clk); + if (!IS_ERR(drv->inter_clk)) + clk_put(drv->inter_clk); + + return ret; +} + +static int mtk_ccifreq_remove(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct mtk_ccifreq_drv *drv; + + drv = platform_get_drvdata(pdev); + + dev_pm_opp_unregister_notifier(dev, &drv->opp_nb); + dev_pm_opp_of_remove_table(dev); + clk_disable_unprepare(drv->inter_clk); + clk_disable_unprepare(drv->cci_clk); + regulator_disable(drv->proc_reg); + if (drv->sram_reg) + regulator_disable(drv->sram_reg); + + return 0; +} + +static const struct mtk_ccifreq_platform_data mt8183_platform_data = { + .min_volt_shift = 100000, + .max_volt_shift = 200000, + .proc_max_volt = 1150000, + .sram_min_volt = 0, + .sram_max_volt = 1150000, +}; + +static const struct mtk_ccifreq_platform_data mt8186_platform_data = { + .min_volt_shift = 100000, + .max_volt_shift = 250000, + .proc_max_volt = 1118750, + .sram_min_volt = 850000, + .sram_max_volt = 1118750, +}; + +static const struct of_device_id mtk_ccifreq_machines[] = { + { .compatible = "mediatek,mt8183-cci", .data = &mt8183_platform_data }, + { .compatible = "mediatek,mt8186-cci", .data = &mt8186_platform_data }, + { }, +}; +MODULE_DEVICE_TABLE(of, mtk_ccifreq_machines); + +static struct platform_driver mtk_ccifreq_platdrv = { + .probe = mtk_ccifreq_probe, + .remove = mtk_ccifreq_remove, + .driver = { + .name = "mtk-ccifreq", + .of_match_table = mtk_ccifreq_machines, + }, +}; +module_platform_driver(mtk_ccifreq_platdrv); + +MODULE_DESCRIPTION("MediaTek CCI devfreq driver"); +MODULE_AUTHOR("Jia-Wei Chang <jia-wei.chang@mediatek.com>"); +MODULE_LICENSE("GPL v2");