| Message ID | 20220415055916.28350-5-rex-bc.chen@mediatek.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cpufreq: mediatek: Cleanup and support MT8183 and MT8186 | expand |
On Fri, 2022-04-15 at 14:24 +0200, AngeloGioacchino Del Regno wrote: > Il 15/04/22 07:59, Rex-BC Chen ha scritto: > > From: Jia-Wei Chang <jia-wei.chang@mediatek.com> > > > > We found the buck voltage may not be exactly the same with what we > > set > > because CPU may share the same buck with other module. > > Therefore, we need to record the previous desired value instead of > > reading > > it from regulators. > > > > Signed-off-by: Andrew-sh.Cheng <andrew-sh.cheng@mediatek.com> > > Signed-off-by: Jia-Wei Chang <jia-wei.chang@mediatek.com> > > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com> > > --- > > drivers/cpufreq/mediatek-cpufreq.c | 17 +++++++++++++---- > > 1 file changed, 13 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/cpufreq/mediatek-cpufreq.c > > b/drivers/cpufreq/mediatek-cpufreq.c > > index ff27f77e8ee6..fa8b193bf27b 100644 > > --- a/drivers/cpufreq/mediatek-cpufreq.c > > +++ b/drivers/cpufreq/mediatek-cpufreq.c > > @@ -40,6 +40,7 @@ struct mtk_cpu_dvfs_info { > > struct list_head list_head; > > int intermediate_voltage; > > bool need_voltage_tracking; > > + int pre_vproc; > > }; > > > > static LIST_HEAD(dvfs_info_list); > > @@ -191,11 +192,17 @@ static int > > mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info, > > > > static int mtk_cpufreq_set_voltage(struct mtk_cpu_dvfs_info > > *info, int vproc) > > { > > + int ret; > > + > > if (info->need_voltage_tracking) > > - return mtk_cpufreq_voltage_tracking(info, vproc); > > + ret = mtk_cpufreq_voltage_tracking(info, vproc); > > else > > - return regulator_set_voltage(info->proc_reg, vproc, > > - vproc + VOLT_TOL); > > + ret = regulator_set_voltage(info->proc_reg, vproc, > > + MAX_VOLT_LIMIT); > > + if (!ret) > > + info->pre_vproc = vproc; > > + > > + return ret; > > } > > > > static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, > > @@ -213,7 +220,9 @@ static int mtk_cpufreq_set_target(struct > > cpufreq_policy *policy, > > inter_vproc = info->intermediate_voltage; > > > > pre_freq_hz = clk_get_rate(cpu_clk); > > - pre_vproc = regulator_get_voltage(info->proc_reg); > > + pre_vproc = info->pre_vproc; > > + if (pre_vproc <= 0) > > + pre_vproc = regulator_get_voltage(info->proc_reg); > > I would do it like that, instead: > > if (unlikely(info->pre_vproc <= 0)) > pre_vproc = regulator_get_voltage(info->proc_reg); > else > pre_vproc = info->pre_vproc; > > ....as even though it is indeed possible that info->pre_vproc is <= > 0, it is > very unlikely to happen ;-) > This also solves a 'pre_vproc' double assignment issue, by the way. > > Cheers, > Angelo > > > Hello Angelo, OK, I will add this in next version. Thanks for your suggestion. BRs, Rex
diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c index ff27f77e8ee6..fa8b193bf27b 100644 --- a/drivers/cpufreq/mediatek-cpufreq.c +++ b/drivers/cpufreq/mediatek-cpufreq.c @@ -40,6 +40,7 @@ struct mtk_cpu_dvfs_info { struct list_head list_head; int intermediate_voltage; bool need_voltage_tracking; + int pre_vproc; }; static LIST_HEAD(dvfs_info_list); @@ -191,11 +192,17 @@ static int mtk_cpufreq_voltage_tracking(struct mtk_cpu_dvfs_info *info, static int mtk_cpufreq_set_voltage(struct mtk_cpu_dvfs_info *info, int vproc) { + int ret; + if (info->need_voltage_tracking) - return mtk_cpufreq_voltage_tracking(info, vproc); + ret = mtk_cpufreq_voltage_tracking(info, vproc); else - return regulator_set_voltage(info->proc_reg, vproc, - vproc + VOLT_TOL); + ret = regulator_set_voltage(info->proc_reg, vproc, + MAX_VOLT_LIMIT); + if (!ret) + info->pre_vproc = vproc; + + return ret; } static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, @@ -213,7 +220,9 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, inter_vproc = info->intermediate_voltage; pre_freq_hz = clk_get_rate(cpu_clk); - pre_vproc = regulator_get_voltage(info->proc_reg); + pre_vproc = info->pre_vproc; + if (pre_vproc <= 0) + pre_vproc = regulator_get_voltage(info->proc_reg); if (pre_vproc < 0) { dev_err(cpu_dev, "invalid Vproc value: %d\n", pre_vproc); return pre_vproc;