| Message ID | 20220505115226.20130-6-rex-bc.chen@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | viresh kumar |
| Headers | show |
| Series | cpufreq: mediatek: Cleanup and support MT8183 and MT8186 | expand |
On Thu, 2022-05-05 at 19:52 +0800, Rex-BC Chen wrote: > From this opp notifier, cpufreq should listen to opp notification and > do Hello Viresh, There is still ">" in this patch... I think the root cause could be the "From" word in the beginning of this message. I will not use "From" in next version.. BRs, Rex > proper actions when receiving events of disable and voltage > adjustment. > > One of the user for this opp notifier is MediaTek SVS. > The MediaTek Smart Voltage Scaling (SVS) is a hardware which > calculates > suitable SVS bank voltages to OPP voltage table. > > 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> > Reviewed-by: AngeloGioacchino Del Regno < > angelogioacchino.delregno@collabora.com> > --- > drivers/cpufreq/mediatek-cpufreq.c | 91 +++++++++++++++++++++++++++- > -- > 1 file changed, 83 insertions(+), 8 deletions(-) > > diff --git a/drivers/cpufreq/mediatek-cpufreq.c > b/drivers/cpufreq/mediatek-cpufreq.c > index fe205eca657d..06d80ee06bbf 100644 > --- a/drivers/cpufreq/mediatek-cpufreq.c > +++ b/drivers/cpufreq/mediatek-cpufreq.c > @@ -46,6 +46,11 @@ struct mtk_cpu_dvfs_info { > int intermediate_voltage; > bool need_voltage_tracking; > int pre_vproc; > + /* Avoid race condition for regulators between notify and > policy */ > + struct mutex reg_lock; > + struct notifier_block opp_nb; > + unsigned int opp_cpu; > + unsigned long opp_freq; > const struct mtk_cpufreq_platform_data *soc_data; > int vtrack_max; > }; > @@ -182,6 +187,8 @@ static int mtk_cpufreq_set_target(struct > cpufreq_policy *policy, > > pre_freq_hz = clk_get_rate(cpu_clk); > > + mutex_lock(&info->reg_lock); > + > if (unlikely(info->pre_vproc <= 0)) > pre_vproc = regulator_get_voltage(info->proc_reg); > else > @@ -214,7 +221,7 @@ static int mtk_cpufreq_set_target(struct > cpufreq_policy *policy, > dev_err(cpu_dev, > "cpu%d: failed to scale up voltage!\n", > policy->cpu); > mtk_cpufreq_set_voltage(info, pre_vproc); > - return ret; > + goto out; > } > } > > @@ -224,8 +231,7 @@ static int mtk_cpufreq_set_target(struct > cpufreq_policy *policy, > dev_err(cpu_dev, > "cpu%d: failed to re-parent cpu clock!\n", > policy->cpu); > mtk_cpufreq_set_voltage(info, pre_vproc); > - WARN_ON(1); > - return ret; > + goto out; > } > > /* Set the original PLL to target rate. */ > @@ -235,7 +241,7 @@ static int mtk_cpufreq_set_target(struct > cpufreq_policy *policy, > "cpu%d: failed to scale cpu clock rate!\n", > policy->cpu); > clk_set_parent(cpu_clk, armpll); > mtk_cpufreq_set_voltage(info, pre_vproc); > - return ret; > + goto out; > } > > /* Set parent of CPU clock back to the original PLL. */ > @@ -244,8 +250,7 @@ static int mtk_cpufreq_set_target(struct > cpufreq_policy *policy, > dev_err(cpu_dev, > "cpu%d: failed to re-parent cpu clock!\n", > policy->cpu); > mtk_cpufreq_set_voltage(info, inter_vproc); > - WARN_ON(1); > - return ret; > + goto out; > } > > /* > @@ -260,15 +265,72 @@ static int mtk_cpufreq_set_target(struct > cpufreq_policy *policy, > clk_set_parent(cpu_clk, info->inter_clk); > clk_set_rate(armpll, pre_freq_hz); > clk_set_parent(cpu_clk, armpll); > - return ret; > + goto out; > } > } > > - return 0; > + info->opp_freq = freq_hz; > + > +out: > + mutex_unlock(&info->reg_lock); > + > + return ret; > } > > #define DYNAMIC_POWER "dynamic-power-coefficient" > > +static int mtk_cpufreq_opp_notifier(struct notifier_block *nb, > + unsigned long event, void *data) > +{ > + struct dev_pm_opp *opp = data; > + struct dev_pm_opp *new_opp; > + struct mtk_cpu_dvfs_info *info; > + unsigned long freq, volt; > + struct cpufreq_policy *policy; > + int ret = 0; > + > + info = container_of(nb, struct mtk_cpu_dvfs_info, opp_nb); > + > + if (event == OPP_EVENT_ADJUST_VOLTAGE) { > + freq = dev_pm_opp_get_freq(opp); > + > + mutex_lock(&info->reg_lock); > + if (info->opp_freq == freq) { > + volt = dev_pm_opp_get_voltage(opp); > + ret = mtk_cpufreq_set_voltage(info, volt); > + if (ret) > + dev_err(info->cpu_dev, > + "failed to scale voltage: > %d\n", ret); > + } > + mutex_unlock(&info->reg_lock); > + } else if (event == OPP_EVENT_DISABLE) { > + freq = dev_pm_opp_get_freq(opp); > + > + /* case of current opp item is disabled */ > + if (info->opp_freq == freq) { > + freq = 1; > + new_opp = dev_pm_opp_find_freq_ceil(info- > >cpu_dev, > + &freq); > + if (IS_ERR(new_opp)) { > + dev_err(info->cpu_dev, > + "all opp items are > disabled\n"); > + ret = PTR_ERR(new_opp); > + return notifier_from_errno(ret); > + } > + > + dev_pm_opp_put(new_opp); > + policy = cpufreq_cpu_get(info->opp_cpu); > + if (policy) { > + cpufreq_driver_target(policy, freq / > 1000, > + CPUFREQ_RELATION_ > L); > + cpufreq_cpu_put(policy); > + } > + } > + } > + > + return notifier_from_errno(ret); > +} > + > static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, > int cpu) > { > struct device *cpu_dev; > @@ -357,6 +419,18 @@ static int mtk_cpu_dvfs_info_init(struct > mtk_cpu_dvfs_info *info, int cpu) > info->intermediate_voltage = dev_pm_opp_get_voltage(opp); > dev_pm_opp_put(opp); > > + mutex_init(&info->reg_lock); > + > + info->opp_cpu = cpu; > + info->opp_nb.notifier_call = mtk_cpufreq_opp_notifier; > + ret = dev_pm_opp_register_notifier(cpu_dev, &info->opp_nb); > + if (ret) { > + dev_err(cpu_dev, "cpu%d: failed to register opp > notifier\n", cpu); > + goto out_disable_inter_clock; > + } > + > + info->opp_freq = clk_get_rate(info->cpu_clk); > + > /* > * If SRAM regulator is present, software "voltage tracking" is > needed > * for this CPU power domain. > @@ -421,6 +495,7 @@ static void mtk_cpu_dvfs_info_release(struct > mtk_cpu_dvfs_info *info) > } > > dev_pm_opp_of_cpumask_remove_table(&info->cpus); > + dev_pm_opp_unregister_notifier(info->cpu_dev, &info->opp_nb); > } > > static int mtk_cpufreq_init(struct cpufreq_policy *policy)
On Fri, May 6, 2022 at 9:56 AM Rex-BC Chen <rex-bc.chen@mediatek.com> wrote: > > On Thu, 2022-05-05 at 19:52 +0800, Rex-BC Chen wrote: > > From this opp notifier, cpufreq should listen to opp notification and > > do > > Hello Viresh, > > There is still ">" in this patch... > I think the root cause could be the "From" word in the beginning of > this message. > I will not use "From" in next version.. Could this be a bug in lore? I'm not seeing this extra ">" in either the email in my inbox, viewed raw, nor the patch downloaded from patchwork [1]. ChenYu [1] https://patchwork.kernel.org/project/linux-mediatek/patch/20220505115226.20130-6-rex-bc.chen@mediatek.com/mbox/ > > BRs, > Rex > > > proper actions when receiving events of disable and voltage > > adjustment. > > > > One of the user for this opp notifier is MediaTek SVS. > > The MediaTek Smart Voltage Scaling (SVS) is a hardware which > > calculates > > suitable SVS bank voltages to OPP voltage table. > > > > 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> > > Reviewed-by: AngeloGioacchino Del Regno < > > angelogioacchino.delregno@collabora.com> > > --- > > drivers/cpufreq/mediatek-cpufreq.c | 91 +++++++++++++++++++++++++++- > > -- > > 1 file changed, 83 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/cpufreq/mediatek-cpufreq.c > > b/drivers/cpufreq/mediatek-cpufreq.c > > index fe205eca657d..06d80ee06bbf 100644 > > --- a/drivers/cpufreq/mediatek-cpufreq.c > > +++ b/drivers/cpufreq/mediatek-cpufreq.c > > @@ -46,6 +46,11 @@ struct mtk_cpu_dvfs_info { > > int intermediate_voltage; > > bool need_voltage_tracking; > > int pre_vproc; > > + /* Avoid race condition for regulators between notify and > > policy */ > > + struct mutex reg_lock; > > + struct notifier_block opp_nb; > > + unsigned int opp_cpu; > > + unsigned long opp_freq; > > const struct mtk_cpufreq_platform_data *soc_data; > > int vtrack_max; > > }; > > @@ -182,6 +187,8 @@ static int mtk_cpufreq_set_target(struct > > cpufreq_policy *policy, > > > > pre_freq_hz = clk_get_rate(cpu_clk); > > > > + mutex_lock(&info->reg_lock); > > + > > if (unlikely(info->pre_vproc <= 0)) > > pre_vproc = regulator_get_voltage(info->proc_reg); > > else > > @@ -214,7 +221,7 @@ static int mtk_cpufreq_set_target(struct > > cpufreq_policy *policy, > > dev_err(cpu_dev, > > "cpu%d: failed to scale up voltage!\n", > > policy->cpu); > > mtk_cpufreq_set_voltage(info, pre_vproc); > > - return ret; > > + goto out; > > } > > } > > > > @@ -224,8 +231,7 @@ static int mtk_cpufreq_set_target(struct > > cpufreq_policy *policy, > > dev_err(cpu_dev, > > "cpu%d: failed to re-parent cpu clock!\n", > > policy->cpu); > > mtk_cpufreq_set_voltage(info, pre_vproc); > > - WARN_ON(1); > > - return ret; > > + goto out; > > } > > > > /* Set the original PLL to target rate. */ > > @@ -235,7 +241,7 @@ static int mtk_cpufreq_set_target(struct > > cpufreq_policy *policy, > > "cpu%d: failed to scale cpu clock rate!\n", > > policy->cpu); > > clk_set_parent(cpu_clk, armpll); > > mtk_cpufreq_set_voltage(info, pre_vproc); > > - return ret; > > + goto out; > > } > > > > /* Set parent of CPU clock back to the original PLL. */ > > @@ -244,8 +250,7 @@ static int mtk_cpufreq_set_target(struct > > cpufreq_policy *policy, > > dev_err(cpu_dev, > > "cpu%d: failed to re-parent cpu clock!\n", > > policy->cpu); > > mtk_cpufreq_set_voltage(info, inter_vproc); > > - WARN_ON(1); > > - return ret; > > + goto out; > > } > > > > /* > > @@ -260,15 +265,72 @@ static int mtk_cpufreq_set_target(struct > > cpufreq_policy *policy, > > clk_set_parent(cpu_clk, info->inter_clk); > > clk_set_rate(armpll, pre_freq_hz); > > clk_set_parent(cpu_clk, armpll); > > - return ret; > > + goto out; > > } > > } > > > > - return 0; > > + info->opp_freq = freq_hz; > > + > > +out: > > + mutex_unlock(&info->reg_lock); > > + > > + return ret; > > } > > > > #define DYNAMIC_POWER "dynamic-power-coefficient" > > > > +static int mtk_cpufreq_opp_notifier(struct notifier_block *nb, > > + unsigned long event, void *data) > > +{ > > + struct dev_pm_opp *opp = data; > > + struct dev_pm_opp *new_opp; > > + struct mtk_cpu_dvfs_info *info; > > + unsigned long freq, volt; > > + struct cpufreq_policy *policy; > > + int ret = 0; > > + > > + info = container_of(nb, struct mtk_cpu_dvfs_info, opp_nb); > > + > > + if (event == OPP_EVENT_ADJUST_VOLTAGE) { > > + freq = dev_pm_opp_get_freq(opp); > > + > > + mutex_lock(&info->reg_lock); > > + if (info->opp_freq == freq) { > > + volt = dev_pm_opp_get_voltage(opp); > > + ret = mtk_cpufreq_set_voltage(info, volt); > > + if (ret) > > + dev_err(info->cpu_dev, > > + "failed to scale voltage: > > %d\n", ret); > > + } > > + mutex_unlock(&info->reg_lock); > > + } else if (event == OPP_EVENT_DISABLE) { > > + freq = dev_pm_opp_get_freq(opp); > > + > > + /* case of current opp item is disabled */ > > + if (info->opp_freq == freq) { > > + freq = 1; > > + new_opp = dev_pm_opp_find_freq_ceil(info- > > >cpu_dev, > > + &freq); > > + if (IS_ERR(new_opp)) { > > + dev_err(info->cpu_dev, > > + "all opp items are > > disabled\n"); > > + ret = PTR_ERR(new_opp); > > + return notifier_from_errno(ret); > > + } > > + > > + dev_pm_opp_put(new_opp); > > + policy = cpufreq_cpu_get(info->opp_cpu); > > + if (policy) { > > + cpufreq_driver_target(policy, freq / > > 1000, > > + CPUFREQ_RELATION_ > > L); > > + cpufreq_cpu_put(policy); > > + } > > + } > > + } > > + > > + return notifier_from_errno(ret); > > +} > > + > > static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, > > int cpu) > > { > > struct device *cpu_dev; > > @@ -357,6 +419,18 @@ static int mtk_cpu_dvfs_info_init(struct > > mtk_cpu_dvfs_info *info, int cpu) > > info->intermediate_voltage = dev_pm_opp_get_voltage(opp); > > dev_pm_opp_put(opp); > > > > + mutex_init(&info->reg_lock); > > + > > + info->opp_cpu = cpu; > > + info->opp_nb.notifier_call = mtk_cpufreq_opp_notifier; > > + ret = dev_pm_opp_register_notifier(cpu_dev, &info->opp_nb); > > + if (ret) { > > + dev_err(cpu_dev, "cpu%d: failed to register opp > > notifier\n", cpu); > > + goto out_disable_inter_clock; > > + } > > + > > + info->opp_freq = clk_get_rate(info->cpu_clk); > > + > > /* > > * If SRAM regulator is present, software "voltage tracking" is > > needed > > * for this CPU power domain. > > @@ -421,6 +495,7 @@ static void mtk_cpu_dvfs_info_release(struct > > mtk_cpu_dvfs_info *info) > > } > > > > dev_pm_opp_of_cpumask_remove_table(&info->cpus); > > + dev_pm_opp_unregister_notifier(info->cpu_dev, &info->opp_nb); > > } > > > > static int mtk_cpufreq_init(struct cpufreq_policy *policy) > > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
+Konstantin On 06-05-22, 11:22, Chen-Yu Tsai wrote: > On Fri, May 6, 2022 at 9:56 AM Rex-BC Chen <rex-bc.chen@mediatek.com> wrote: > > > > On Thu, 2022-05-05 at 19:52 +0800, Rex-BC Chen wrote: > > > From this opp notifier, cpufreq should listen to opp notification and > > > do > > > > Hello Viresh, > > > > There is still ">" in this patch... > > I think the root cause could be the "From" word in the beginning of > > this message. > > I will not use "From" in next version.. > > Could this be a bug in lore? > > I'm not seeing this extra ">" in either the email in my inbox, viewed > raw, nor the patch downloaded from patchwork [1]. > > > ChenYu > > [1] https://patchwork.kernel.org/project/linux-mediatek/patch/20220505115226.20130-6-rex-bc.chen@mediatek.com/mbox/ Interesting. Konstantin, we are witnessing an additional ">" symbol in the first line of the commit log for this particular patch for some reason. https://lore.kernel.org/lkml/20220505115226.20130-6-rex-bc.chen@mediatek.com/raw Any idea ?
On 05-05-22, 19:52, Rex-BC Chen wrote: > >From this opp notifier, cpufreq should listen to opp notification and do > proper actions when receiving events of disable and voltage adjustment. > > One of the user for this opp notifier is MediaTek SVS. > The MediaTek Smart Voltage Scaling (SVS) is a hardware which calculates > suitable SVS bank voltages to OPP voltage table. > > 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> > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > --- > drivers/cpufreq/mediatek-cpufreq.c | 91 +++++++++++++++++++++++++++--- > 1 file changed, 83 insertions(+), 8 deletions(-) > > diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c > index fe205eca657d..06d80ee06bbf 100644 > --- a/drivers/cpufreq/mediatek-cpufreq.c > +++ b/drivers/cpufreq/mediatek-cpufreq.c > @@ -46,6 +46,11 @@ struct mtk_cpu_dvfs_info { > int intermediate_voltage; > bool need_voltage_tracking; > int pre_vproc; > + /* Avoid race condition for regulators between notify and policy */ > + struct mutex reg_lock; > + struct notifier_block opp_nb; > + unsigned int opp_cpu; > + unsigned long opp_freq; The name opp_freq doesn't fit well, I renamed it to current_freq. > const struct mtk_cpufreq_platform_data *soc_data; > int vtrack_max; > }; > @@ -182,6 +187,8 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, > > pre_freq_hz = clk_get_rate(cpu_clk); > > + mutex_lock(&info->reg_lock); > + > if (unlikely(info->pre_vproc <= 0)) > pre_vproc = regulator_get_voltage(info->proc_reg); > else > @@ -214,7 +221,7 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, > dev_err(cpu_dev, > "cpu%d: failed to scale up voltage!\n", policy->cpu); > mtk_cpufreq_set_voltage(info, pre_vproc); > - return ret; > + goto out; > } > } > > @@ -224,8 +231,7 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, > dev_err(cpu_dev, > "cpu%d: failed to re-parent cpu clock!\n", policy->cpu); > mtk_cpufreq_set_voltage(info, pre_vproc); > - WARN_ON(1); > - return ret; > + goto out; > } > > /* Set the original PLL to target rate. */ > @@ -235,7 +241,7 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, > "cpu%d: failed to scale cpu clock rate!\n", policy->cpu); > clk_set_parent(cpu_clk, armpll); > mtk_cpufreq_set_voltage(info, pre_vproc); > - return ret; > + goto out; > } > > /* Set parent of CPU clock back to the original PLL. */ > @@ -244,8 +250,7 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, > dev_err(cpu_dev, > "cpu%d: failed to re-parent cpu clock!\n", policy->cpu); > mtk_cpufreq_set_voltage(info, inter_vproc); > - WARN_ON(1); > - return ret; > + goto out; > } > > /* > @@ -260,15 +265,72 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, > clk_set_parent(cpu_clk, info->inter_clk); > clk_set_rate(armpll, pre_freq_hz); > clk_set_parent(cpu_clk, armpll); > - return ret; > + goto out; > } > } > > - return 0; > + info->opp_freq = freq_hz; > + > +out: > + mutex_unlock(&info->reg_lock); > + > + return ret; > } > > #define DYNAMIC_POWER "dynamic-power-coefficient" > > +static int mtk_cpufreq_opp_notifier(struct notifier_block *nb, > + unsigned long event, void *data) > +{ > + struct dev_pm_opp *opp = data; > + struct dev_pm_opp *new_opp; > + struct mtk_cpu_dvfs_info *info; > + unsigned long freq, volt; > + struct cpufreq_policy *policy; > + int ret = 0; > + > + info = container_of(nb, struct mtk_cpu_dvfs_info, opp_nb); > + > + if (event == OPP_EVENT_ADJUST_VOLTAGE) { > + freq = dev_pm_opp_get_freq(opp); > + > + mutex_lock(&info->reg_lock); > + if (info->opp_freq == freq) { > + volt = dev_pm_opp_get_voltage(opp); > + ret = mtk_cpufreq_set_voltage(info, volt); > + if (ret) > + dev_err(info->cpu_dev, > + "failed to scale voltage: %d\n", ret); > + } > + mutex_unlock(&info->reg_lock); > + } else if (event == OPP_EVENT_DISABLE) { > + freq = dev_pm_opp_get_freq(opp); > + > + /* case of current opp item is disabled */ > + if (info->opp_freq == freq) { > + freq = 1; > + new_opp = dev_pm_opp_find_freq_ceil(info->cpu_dev, > + &freq); > + if (IS_ERR(new_opp)) { > + dev_err(info->cpu_dev, > + "all opp items are disabled\n"); > + ret = PTR_ERR(new_opp); > + return notifier_from_errno(ret); > + } > + > + dev_pm_opp_put(new_opp); > + policy = cpufreq_cpu_get(info->opp_cpu); > + if (policy) { > + cpufreq_driver_target(policy, freq / 1000, > + CPUFREQ_RELATION_L); > + cpufreq_cpu_put(policy); > + } > + } > + } > + > + return notifier_from_errno(ret); > +} > + > static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu) > { > struct device *cpu_dev; > @@ -357,6 +419,18 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu) > info->intermediate_voltage = dev_pm_opp_get_voltage(opp); > dev_pm_opp_put(opp); > > + mutex_init(&info->reg_lock); > + > + info->opp_cpu = cpu; > + info->opp_nb.notifier_call = mtk_cpufreq_opp_notifier; > + ret = dev_pm_opp_register_notifier(cpu_dev, &info->opp_nb); > + if (ret) { > + dev_err(cpu_dev, "cpu%d: failed to register opp notifier\n", cpu); > + goto out_disable_inter_clock; > + } > + > + info->opp_freq = clk_get_rate(info->cpu_clk); This is a resource as well, which should have been initialized before registering notifier. Applied with above changes. Thanks.
On Fri, 2022-05-06 at 09:45 +0530, Viresh Kumar wrote: > On 05-05-22, 19:52, Rex-BC Chen wrote: > > > From this opp notifier, cpufreq should listen to opp notification > > > and do > > > > proper actions when receiving events of disable and voltage > > adjustment. > > > > One of the user for this opp notifier is MediaTek SVS. > > The MediaTek Smart Voltage Scaling (SVS) is a hardware which > > calculates > > suitable SVS bank voltages to OPP voltage table. > > > > 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> > > Reviewed-by: AngeloGioacchino Del Regno < > > angelogioacchino.delregno@collabora.com> > > --- > > drivers/cpufreq/mediatek-cpufreq.c | 91 > > +++++++++++++++++++++++++++--- > > 1 file changed, 83 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/cpufreq/mediatek-cpufreq.c > > b/drivers/cpufreq/mediatek-cpufreq.c > > index fe205eca657d..06d80ee06bbf 100644 > > --- a/drivers/cpufreq/mediatek-cpufreq.c > > +++ b/drivers/cpufreq/mediatek-cpufreq.c > > @@ -46,6 +46,11 @@ struct mtk_cpu_dvfs_info { > > int intermediate_voltage; > > bool need_voltage_tracking; > > int pre_vproc; > > + /* Avoid race condition for regulators between notify and > > policy */ > > + struct mutex reg_lock; > > + struct notifier_block opp_nb; > > + unsigned int opp_cpu; > > + unsigned long opp_freq; > > The name opp_freq doesn't fit well, I renamed it to current_freq. > > > const struct mtk_cpufreq_platform_data *soc_data; > > int vtrack_max; > > }; > > @@ -182,6 +187,8 @@ static int mtk_cpufreq_set_target(struct > > cpufreq_policy *policy, > > > > pre_freq_hz = clk_get_rate(cpu_clk); > > > > + mutex_lock(&info->reg_lock); > > + > > if (unlikely(info->pre_vproc <= 0)) > > pre_vproc = regulator_get_voltage(info->proc_reg); > > else > > @@ -214,7 +221,7 @@ static int mtk_cpufreq_set_target(struct > > cpufreq_policy *policy, > > dev_err(cpu_dev, > > "cpu%d: failed to scale up voltage!\n", > > policy->cpu); > > mtk_cpufreq_set_voltage(info, pre_vproc); > > - return ret; > > + goto out; > > } > > } > > > > @@ -224,8 +231,7 @@ static int mtk_cpufreq_set_target(struct > > cpufreq_policy *policy, > > dev_err(cpu_dev, > > "cpu%d: failed to re-parent cpu clock!\n", > > policy->cpu); > > mtk_cpufreq_set_voltage(info, pre_vproc); > > - WARN_ON(1); > > - return ret; > > + goto out; > > } > > > > /* Set the original PLL to target rate. */ > > @@ -235,7 +241,7 @@ static int mtk_cpufreq_set_target(struct > > cpufreq_policy *policy, > > "cpu%d: failed to scale cpu clock rate!\n", > > policy->cpu); > > clk_set_parent(cpu_clk, armpll); > > mtk_cpufreq_set_voltage(info, pre_vproc); > > - return ret; > > + goto out; > > } > > > > /* Set parent of CPU clock back to the original PLL. */ > > @@ -244,8 +250,7 @@ static int mtk_cpufreq_set_target(struct > > cpufreq_policy *policy, > > dev_err(cpu_dev, > > "cpu%d: failed to re-parent cpu clock!\n", > > policy->cpu); > > mtk_cpufreq_set_voltage(info, inter_vproc); > > - WARN_ON(1); > > - return ret; > > + goto out; > > } > > > > /* > > @@ -260,15 +265,72 @@ static int mtk_cpufreq_set_target(struct > > cpufreq_policy *policy, > > clk_set_parent(cpu_clk, info->inter_clk); > > clk_set_rate(armpll, pre_freq_hz); > > clk_set_parent(cpu_clk, armpll); > > - return ret; > > + goto out; > > } > > } > > > > - return 0; > > + info->opp_freq = freq_hz; > > + > > +out: > > + mutex_unlock(&info->reg_lock); > > + > > + return ret; > > } > > > > #define DYNAMIC_POWER "dynamic-power-coefficient" > > > > +static int mtk_cpufreq_opp_notifier(struct notifier_block *nb, > > + unsigned long event, void *data) > > +{ > > + struct dev_pm_opp *opp = data; > > + struct dev_pm_opp *new_opp; > > + struct mtk_cpu_dvfs_info *info; > > + unsigned long freq, volt; > > + struct cpufreq_policy *policy; > > + int ret = 0; > > + > > + info = container_of(nb, struct mtk_cpu_dvfs_info, opp_nb); > > + > > + if (event == OPP_EVENT_ADJUST_VOLTAGE) { > > + freq = dev_pm_opp_get_freq(opp); > > + > > + mutex_lock(&info->reg_lock); > > + if (info->opp_freq == freq) { > > + volt = dev_pm_opp_get_voltage(opp); > > + ret = mtk_cpufreq_set_voltage(info, volt); > > + if (ret) > > + dev_err(info->cpu_dev, > > + "failed to scale voltage: > > %d\n", ret); > > + } > > + mutex_unlock(&info->reg_lock); > > + } else if (event == OPP_EVENT_DISABLE) { > > + freq = dev_pm_opp_get_freq(opp); > > + > > + /* case of current opp item is disabled */ > > + if (info->opp_freq == freq) { > > + freq = 1; > > + new_opp = dev_pm_opp_find_freq_ceil(info- > > >cpu_dev, > > + &freq); > > + if (IS_ERR(new_opp)) { > > + dev_err(info->cpu_dev, > > + "all opp items are > > disabled\n"); > > + ret = PTR_ERR(new_opp); > > + return notifier_from_errno(ret); > > + } > > + > > + dev_pm_opp_put(new_opp); > > + policy = cpufreq_cpu_get(info->opp_cpu); > > + if (policy) { > > + cpufreq_driver_target(policy, freq / > > 1000, > > + CPUFREQ_RELATION_ > > L); > > + cpufreq_cpu_put(policy); > > + } > > + } > > + } > > + > > + return notifier_from_errno(ret); > > +} > > + > > static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, > > int cpu) > > { > > struct device *cpu_dev; > > @@ -357,6 +419,18 @@ static int mtk_cpu_dvfs_info_init(struct > > mtk_cpu_dvfs_info *info, int cpu) > > info->intermediate_voltage = dev_pm_opp_get_voltage(opp); > > dev_pm_opp_put(opp); > > > > + mutex_init(&info->reg_lock); > > + > > + info->opp_cpu = cpu; > > + info->opp_nb.notifier_call = mtk_cpufreq_opp_notifier; > > + ret = dev_pm_opp_register_notifier(cpu_dev, &info->opp_nb); > > + if (ret) { > > + dev_err(cpu_dev, "cpu%d: failed to register opp > > notifier\n", cpu); > > + goto out_disable_inter_clock; > > + } > > + > > + info->opp_freq = clk_get_rate(info->cpu_clk); > > This is a resource as well, which should have been initialized before > registering notifier. > > Applied with above changes. Thanks. > Hello Viresh, Thanks for your help! BRs, Rex
diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c index fe205eca657d..06d80ee06bbf 100644 --- a/drivers/cpufreq/mediatek-cpufreq.c +++ b/drivers/cpufreq/mediatek-cpufreq.c @@ -46,6 +46,11 @@ struct mtk_cpu_dvfs_info { int intermediate_voltage; bool need_voltage_tracking; int pre_vproc; + /* Avoid race condition for regulators between notify and policy */ + struct mutex reg_lock; + struct notifier_block opp_nb; + unsigned int opp_cpu; + unsigned long opp_freq; const struct mtk_cpufreq_platform_data *soc_data; int vtrack_max; }; @@ -182,6 +187,8 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, pre_freq_hz = clk_get_rate(cpu_clk); + mutex_lock(&info->reg_lock); + if (unlikely(info->pre_vproc <= 0)) pre_vproc = regulator_get_voltage(info->proc_reg); else @@ -214,7 +221,7 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, dev_err(cpu_dev, "cpu%d: failed to scale up voltage!\n", policy->cpu); mtk_cpufreq_set_voltage(info, pre_vproc); - return ret; + goto out; } } @@ -224,8 +231,7 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, dev_err(cpu_dev, "cpu%d: failed to re-parent cpu clock!\n", policy->cpu); mtk_cpufreq_set_voltage(info, pre_vproc); - WARN_ON(1); - return ret; + goto out; } /* Set the original PLL to target rate. */ @@ -235,7 +241,7 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, "cpu%d: failed to scale cpu clock rate!\n", policy->cpu); clk_set_parent(cpu_clk, armpll); mtk_cpufreq_set_voltage(info, pre_vproc); - return ret; + goto out; } /* Set parent of CPU clock back to the original PLL. */ @@ -244,8 +250,7 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, dev_err(cpu_dev, "cpu%d: failed to re-parent cpu clock!\n", policy->cpu); mtk_cpufreq_set_voltage(info, inter_vproc); - WARN_ON(1); - return ret; + goto out; } /* @@ -260,15 +265,72 @@ static int mtk_cpufreq_set_target(struct cpufreq_policy *policy, clk_set_parent(cpu_clk, info->inter_clk); clk_set_rate(armpll, pre_freq_hz); clk_set_parent(cpu_clk, armpll); - return ret; + goto out; } } - return 0; + info->opp_freq = freq_hz; + +out: + mutex_unlock(&info->reg_lock); + + return ret; } #define DYNAMIC_POWER "dynamic-power-coefficient" +static int mtk_cpufreq_opp_notifier(struct notifier_block *nb, + unsigned long event, void *data) +{ + struct dev_pm_opp *opp = data; + struct dev_pm_opp *new_opp; + struct mtk_cpu_dvfs_info *info; + unsigned long freq, volt; + struct cpufreq_policy *policy; + int ret = 0; + + info = container_of(nb, struct mtk_cpu_dvfs_info, opp_nb); + + if (event == OPP_EVENT_ADJUST_VOLTAGE) { + freq = dev_pm_opp_get_freq(opp); + + mutex_lock(&info->reg_lock); + if (info->opp_freq == freq) { + volt = dev_pm_opp_get_voltage(opp); + ret = mtk_cpufreq_set_voltage(info, volt); + if (ret) + dev_err(info->cpu_dev, + "failed to scale voltage: %d\n", ret); + } + mutex_unlock(&info->reg_lock); + } else if (event == OPP_EVENT_DISABLE) { + freq = dev_pm_opp_get_freq(opp); + + /* case of current opp item is disabled */ + if (info->opp_freq == freq) { + freq = 1; + new_opp = dev_pm_opp_find_freq_ceil(info->cpu_dev, + &freq); + if (IS_ERR(new_opp)) { + dev_err(info->cpu_dev, + "all opp items are disabled\n"); + ret = PTR_ERR(new_opp); + return notifier_from_errno(ret); + } + + dev_pm_opp_put(new_opp); + policy = cpufreq_cpu_get(info->opp_cpu); + if (policy) { + cpufreq_driver_target(policy, freq / 1000, + CPUFREQ_RELATION_L); + cpufreq_cpu_put(policy); + } + } + } + + return notifier_from_errno(ret); +} + static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu) { struct device *cpu_dev; @@ -357,6 +419,18 @@ static int mtk_cpu_dvfs_info_init(struct mtk_cpu_dvfs_info *info, int cpu) info->intermediate_voltage = dev_pm_opp_get_voltage(opp); dev_pm_opp_put(opp); + mutex_init(&info->reg_lock); + + info->opp_cpu = cpu; + info->opp_nb.notifier_call = mtk_cpufreq_opp_notifier; + ret = dev_pm_opp_register_notifier(cpu_dev, &info->opp_nb); + if (ret) { + dev_err(cpu_dev, "cpu%d: failed to register opp notifier\n", cpu); + goto out_disable_inter_clock; + } + + info->opp_freq = clk_get_rate(info->cpu_clk); + /* * If SRAM regulator is present, software "voltage tracking" is needed * for this CPU power domain. @@ -421,6 +495,7 @@ static void mtk_cpu_dvfs_info_release(struct mtk_cpu_dvfs_info *info) } dev_pm_opp_of_cpumask_remove_table(&info->cpus); + dev_pm_opp_unregister_notifier(info->cpu_dev, &info->opp_nb); } static int mtk_cpufreq_init(struct cpufreq_policy *policy)