| Message ID | 20220415055916.28350-5-rex-bc.chen@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cpufreq: mediatek: Cleanup and support MT8183 and MT8186 | expand |
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
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;