| Message ID | 20230809153455.29056-1-sumitg@nvidia.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cpufreq: tegra194: remove opp table in exit hook | expand |
>>>> Add exit hook and remove OPP table when all the CPU's in a policy >>>> are offlined. It will fix the below error messages when onlining >>>> first CPU from a policy whose all CPU's were previously offlined. >>>> >>>> debugfs: File 'cpu5' in directory 'opp' already present! >>>> debugfs: File 'cpu6' in directory 'opp' already present! >>>> debugfs: File 'cpu7' in directory 'opp' already present! >>>> >>>> Fixes: f41e1442ac5b ("cpufreq: tegra194: add OPP support and set bandwidth") >>>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com> >>>> --- >>>> drivers/cpufreq/tegra194-cpufreq.c | 13 +++++++++++++ >>>> 1 file changed, 13 insertions(+) >>>> >>>> diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c >>>> index c90b30469165..66a9c23544db 100644 >>>> --- a/drivers/cpufreq/tegra194-cpufreq.c >>>> +++ b/drivers/cpufreq/tegra194-cpufreq.c >>>> @@ -454,6 +454,8 @@ static int tegra_cpufreq_init_cpufreq_table(struct cpufreq_policy *policy, >>>> if (ret < 0) >>>> return ret; >>>> >>>> + dev_pm_opp_put(opp); >>>> + >>>> freq_table[j].driver_data = pos->driver_data; >>>> freq_table[j].frequency = pos->frequency; >>>> j++; >>>> @@ -508,6 +510,16 @@ static int tegra194_cpufreq_init(struct cpufreq_policy *policy) >>>> return 0; >>>> } >>>> >>>> +static int tegra194_cpufreq_exit(struct cpufreq_policy *policy) >>>> +{ >>>> + struct device *cpu_dev = get_cpu_device(policy->cpu); >>>> + >>>> + dev_pm_opp_remove_all_dynamic(cpu_dev); >>>> + dev_pm_opp_of_cpumask_remove_table(policy->related_cpus); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> static int tegra194_cpufreq_set_target(struct cpufreq_policy *policy, >>>> unsigned int index) >>>> { >>>> @@ -535,6 +547,7 @@ static struct cpufreq_driver tegra194_cpufreq_driver = { >>>> .target_index = tegra194_cpufreq_set_target, >>>> .get = tegra194_get_speed, >>>> .init = tegra194_cpufreq_init, >>>> + .exit = tegra194_cpufreq_exit, >>>> .attr = cpufreq_generic_attr, >>>> }; >>> >>> If it is only about hotplugging of the CPUs, then you can also do this I guess. >>> >>> commit 263abfe74b5f ("cpufreq: dt: Implement online/offline() callbacks") > > You should do this as well, this makes hotplugging paths much faster. i.e. on > top of this patch. > Sent a separate patch to add online/offline callbacks. https://lore.kernel.org/lkml/20230825111920.8257-1-sumitg@nvidia.com/ Also, sent v2 of this patch with updated commit description. https://lore.kernel.org/lkml/20230825111617.8069-1-sumitg@nvidia.com/ Thank you, Sumit Gupta >>> But since your driver is capable of being built as a module, I suggest you try >>> to build it as one and insert remove it multiple times. It must cause you some >>> trouble as you don't implement an .exit() before this patch. >>> >>> Eventually, I think you need to do both, what this patch and 263abfe74b5f do. >>> Just that the reasons need to be correct for both the changes. >>> >>> -- >>> viresh >> >> Hi Viresh, >> I got the same message on inserting and removing the module multiple times >> as you suggested. After applying this change, the message is not coming. So, >> the current change is resolving both scenarios as __cpufreq_offline() calls >> either exit() or offline(). >> I can update the commit message to mention both scenarios and keep change as >> it is? >> >> cpufreq_remove_dev >> |-__cpufreq_offline >> |--tegra194_cpufreq_exit >> >> cpuhp_cpufreq_offline >> |-__cpufreq_offline >> |--tegra194_cpufreq_exit > > -- > viresh
diff --git a/drivers/cpufreq/tegra194-cpufreq.c b/drivers/cpufreq/tegra194-cpufreq.c index c90b30469165..66a9c23544db 100644 --- a/drivers/cpufreq/tegra194-cpufreq.c +++ b/drivers/cpufreq/tegra194-cpufreq.c @@ -454,6 +454,8 @@ static int tegra_cpufreq_init_cpufreq_table(struct cpufreq_policy *policy, if (ret < 0) return ret; + dev_pm_opp_put(opp); + freq_table[j].driver_data = pos->driver_data; freq_table[j].frequency = pos->frequency; j++; @@ -508,6 +510,16 @@ static int tegra194_cpufreq_init(struct cpufreq_policy *policy) return 0; } +static int tegra194_cpufreq_exit(struct cpufreq_policy *policy) +{ + struct device *cpu_dev = get_cpu_device(policy->cpu); + + dev_pm_opp_remove_all_dynamic(cpu_dev); + dev_pm_opp_of_cpumask_remove_table(policy->related_cpus); + + return 0; +} + static int tegra194_cpufreq_set_target(struct cpufreq_policy *policy, unsigned int index) { @@ -535,6 +547,7 @@ static struct cpufreq_driver tegra194_cpufreq_driver = { .target_index = tegra194_cpufreq_set_target, .get = tegra194_get_speed, .init = tegra194_cpufreq_init, + .exit = tegra194_cpufreq_exit, .attr = cpufreq_generic_attr, };
Add exit hook and remove OPP table when all the CPU's in a policy are offlined. It will fix the below error messages when onlining first CPU from a policy whose all CPU's were previously offlined. debugfs: File 'cpu5' in directory 'opp' already present! debugfs: File 'cpu6' in directory 'opp' already present! debugfs: File 'cpu7' in directory 'opp' already present! Fixes: f41e1442ac5b ("cpufreq: tegra194: add OPP support and set bandwidth") Signed-off-by: Sumit Gupta <sumitg@nvidia.com> --- drivers/cpufreq/tegra194-cpufreq.c | 13 +++++++++++++ 1 file changed, 13 insertions(+)