| Message ID | 20220505115226.20130-1-rex-bc.chen@mediatek.com |
|---|---|
| Headers | show |
| Series | cpufreq: mediatek: Cleanup and support MT8183 and MT8186 | expand |
Il 05/05/22 13:52, Rex-BC Chen ha scritto: > We register the platform device when driver inits. However, we do not > unregister it when driver exits. > To resolve this, we declare the platform data to be a global static > variable and rename it to be "cpufreq_pdev". > With this global variable, we can do platform_device_unregister() when > driver exits. > > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com> Hello Rex, this commit needs a Fixes: tag. Cheers, Angelo
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 Thu, 2022-05-05 at 17:04 +0200, AngeloGioacchino Del Regno wrote: > Il 05/05/22 13:52, Rex-BC Chen ha scritto: > > We register the platform device when driver inits. However, we do > > not > > unregister it when driver exits. > > To resolve this, we declare the platform data to be a global static > > variable and rename it to be "cpufreq_pdev". > > With this global variable, we can do platform_device_unregister() > > when > > driver exits. > > > > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com> > > Hello Rex, > this commit needs a Fixes: tag. > > Cheers, > Angelo > Hello Angelo, Thanks for the reminder. I will add "Fixes: 501c574f4e3a ("cpufreq: mediatek: Add support of cpufreq to MT2701/MT7623 SoC")" BRs, Rex
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: > We register the platform device when driver inits. However, we do not > unregister it when driver exits. > To resolve this, we declare the platform data to be a global static > variable and rename it to be "cpufreq_pdev". > With this global variable, we can do platform_device_unregister() when > driver exits. > > Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com> > --- > drivers/cpufreq/mediatek-cpufreq.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) Applied. Thanks.
On 05-05-22, 19:52, Rex-BC Chen wrote: > Voltages and shifts are defined as macros originally. > There are different requirements of these values for each MediaTek SoCs. > Therefore, we add the platform data and move these values into it. > > 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 | 84 +++++++++++++++++++++--------- > 1 file changed, 58 insertions(+), 26 deletions(-) Applied. Thanks.
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 05-05-22, 19:52, Rex-BC Chen wrote: > Cpufreq is a DVFS driver used for power saving to scale the clock frequency > and supply the voltage for CPUs. This series do some cleanup for MediaTek > cpufreq drivers and add support for MediaTek SVS[2] and MediaTek CCI > devfreq[3] which are supported in MT8183 and MT8186. Applied 2-5, rest of them depend on the binding and dts patches to be Acked.
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
On Fri, 2022-05-06 at 09:50 +0530, Viresh Kumar wrote: > On 05-05-22, 19:52, Rex-BC Chen wrote: > > Cpufreq is a DVFS driver used for power saving to scale the clock > > frequency > > and supply the voltage for CPUs. This series do some cleanup for > > MediaTek > > cpufreq drivers and add support for MediaTek SVS[2] and MediaTek > > CCI > > devfreq[3] which are supported in MT8183 and MT8186. > > Applied 2-5, rest of them depend on the binding and dts patches to be > Acked. > Hello Viresh, Thanks for accepting our patches. As for rest patch: The cci series [1] is still under reviewing and it depends on chanwoo's series [2]. Therefore, I think it won't be so quick to be acked for these patches in my series. [1]: cci: https://lore.kernel.org/all/20220425125546.4129-1-johnson.wang@mediatek.com/ [2]: chanwoo's passive governor support for devfreq: https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing BRs, Rex
On Thu, 2022-05-12 at 10:57 +0530, Viresh Kumar wrote: > On 06-05-22, 14:32, Rex-BC Chen wrote: > > As for rest patch: > > The cci series [1] is still under reviewing and it depends on > > chanwoo's > > series [2]. > > Therefore, I think it won't be so quick to be acked for these > > patches > > in my series. > > I have applied 1, 6-7 now. I hope it is safe to apply them ? What > about other > three, can you get them acked now ? > Hello Viresh, Thanks for your help! Matthias is not the member of mediatek, so I think we still need his feedback for these three patches. BRs, Rex
On Thu, 2022-05-12 at 11:18 +0530, Viresh Kumar wrote: > On 12-05-22, 13:33, Rex-BC Chen wrote: > > Matthias is not the member of mediatek, so I think we still need > > his > > feedback for these three patches. > > Please ping him and ask for his feedback then. > ok, I will do this. Thanks! BRs, Rex
On 12/05/2022 08:05, Rex-BC Chen wrote: > On Thu, 2022-05-12 at 11:18 +0530, Viresh Kumar wrote: >> On 12-05-22, 13:33, Rex-BC Chen wrote: >>> Matthias is not the member of mediatek, so I think we still need >>> his >>> feedback for these three patches. >> >> Please ping him and ask for his feedback then. >> > > ok, I will do this. > Thanks! > Sorry for the late reply. I'll take the DTS patches through my tree. As I'm late to the game this will be for v5.20. Regards, Matthias
On Tue, 2022-05-17 at 11:57 +0200, Matthias Brugger wrote: > > On 12/05/2022 08:05, Rex-BC Chen wrote: > > On Thu, 2022-05-12 at 11:18 +0530, Viresh Kumar wrote: > > > On 12-05-22, 13:33, Rex-BC Chen wrote: > > > > Matthias is not the member of mediatek, so I think we still > > > > need > > > > his > > > > feedback for these three patches. > > > > > > Please ping him and ask for his feedback then. > > > > > > > ok, I will do this. > > Thanks! > > > > Sorry for the late reply. I'll take the DTS patches through my tree. > As I'm late > to the game this will be for v5.20. > > Regards, > Matthias Hello Matthias, Thanks for you reply, I send another version for this series. Please refer to this: https://patchwork.kernel.org/project/linux-mediatek/list/?series=641933 BRs, Rex
Cpufreq is a DVFS driver used for power saving to scale the clock frequency and supply the voltage for CPUs. This series do some cleanup for MediaTek cpufreq drivers and add support for MediaTek SVS[2] and MediaTek CCI devfreq[3] which are supported in MT8183 and MT8186. Changes for v6: 1. Reorder patches in this series. 2. Add a new patch to do unregister platform device. 3. Modify drivers from maintainer's advice. Changes for v5: 1. Modify the description for the reason we need to use mediatek,cci. 2. Drop [07/14] cpufreq: mediatek: Add .get function. Changes for V4: 1. Revise drivers from reviewers' suggestion. 2. Fix name of opp table issue. Changes for V3: 1. Rebased to linux-next-20220414. 2. Drop accepted patches. 3. Drop "cpufreq: mediatek: Use maximum voltage in init stage" because we make sure the voltage we set is safe for both mediatek cci and cpufreq. 4. Rename cci property to mediatek,cci. 5. Adjust order of cleanup patches. 6. Add new patches for cleanup, handle infinite loop and MT8183 dts. 7. Revise drivers from reviewers' suggestion. 8. Revise commit message of some patches to avoid confusion and misunderstand. 9. Revise "cpufreq: mediatek: Link CCI device to CPU". We do not return successful to pretend we set the target frequency done when cci is not ready. Instead, we find and set a safe voltage so that we can set the target cpufrequency. Changes for V2: 1. Drop the modification of transforming cpufreq-mediatek into yaml and only add the MediaTek CCI property for MediaTek cpufreq. 2. Split the original patches into several patches. Reference series: [1]: V1 of this series is present by Jia-Wei Chang. https://lore.kernel.org/all/20220307122151.11666-1-jia-wei.chang@mediatek.com/ [2]: The MediaTek CCI devfreq driver is introduced in another series. https://lore.kernel.org/all/20220425125546.4129-1-johnson.wang@mediatek.com/ [3]: The MediaTek SVS driver is introduced in another series. https://lore.kernel.org/all/20220420102044.10832-1-roger.lu@mediatek.com/ Jia-Wei Chang (2): cpufreq: mediatek: Refine mtk_cpufreq_voltage_tracking() cpufreq: mediatek: Add support for MT8186 Rex-BC Chen (8): dt-bindings: cpufreq: mediatek: Add MediaTek CCI property cpufreq: mediatek: Add platform_device_unregister when driver exit cpufreq: mediatek: Move voltage limits to platform data cpufreq: mediatek: Add opp notification support cpufreq: mediatek: Link CCI device to CPU arm64: dts: mediatek: Add opp table and clock property for MT8183 cpufreq arm64: dts: mediatek: Add MediaTek CCI node for MT8183 arm64: dts: mediatek: Add mediatek,cci property for MT8183 cpufreq .../bindings/cpufreq/cpufreq-mediatek.txt | 7 + arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 36 ++ .../arm64/boot/dts/mediatek/mt8183-kukui.dtsi | 4 + arch/arm64/boot/dts/mediatek/mt8183.dtsi | 285 +++++++++++++ drivers/cpufreq/mediatek-cpufreq.c | 400 ++++++++++++------ 5 files changed, 609 insertions(+), 123 deletions(-)