Messages in this thread | | | Date | Fri, 15 Apr 2022 14:24:19 +0200 | Subject | Re: [PATCH V3 04/15] cpufreq: mediatek: Record previous target vproc value | From | AngeloGioacchino Del Regno <> |
| |
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
| |