diff mbox series

[v3,2/2] PM / devfreq: mediatek: Introduce MediaTek CCI devfreq driver

Message ID 20220425125546.4129-3-johnson.wang@mediatek.com (mailing list archive)
State Changes Requested, archived
Delegated to: Chanwoo Choi
Headers show
Series Introduce MediaTek CCI devfreq driver | expand

Commit Message

Johnson Wang April 25, 2022, 12:55 p.m. UTC 
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

Comments

kernel test robot April 26, 2022, 12:45 p.m. UTC | #1 
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-20220422]
[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/20220426/202204262037.g8ZK9iOF-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 drivers/devfreq/ sound/core/

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: 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 +378 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
kernel test robot April 27, 2022, 7:09 a.m. UTC | #2 
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
kernel test robot April 27, 2022, 9:25 a.m. UTC | #3 
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
Johnson Wang April 27, 2022, 10:11 a.m. UTC | #4 
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
Chen, Rong A April 28, 2022, 11:39 a.m. UTC | #5 
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
>
Johnson Wang May 6, 2022, 11:38 a.m. UTC | #6 
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
Chen-Yu Tsai May 6, 2022, 12:03 p.m. UTC | #7 
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
Chanwoo Choi May 6, 2022, 6:57 p.m. UTC | #8 
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?
Chanwoo Choi May 7, 2022, 1:53 p.m. UTC | #9 
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?
Chanwoo Choi May 9, 2022, 2:53 a.m. UTC | #10 
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
Johnson Wang May 9, 2022, 5:57 a.m. UTC | #11 
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
Chanwoo Choi May 9, 2022, 11:51 a.m. UTC | #12 
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.
Johnson Wang May 11, 2022, 5:14 a.m. UTC | #13 
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
Chanwoo Choi May 11, 2022, 7:08 a.m. UTC | #14 
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 mbox series

Patch

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");