Messages in this thread | ![/](/images/icornerl.gif) | | Date | Mon, 31 Aug 2020 11:21:07 +0530 | From | Viresh Kumar <> | Subject | Re: [PATCH] opp: Drop unnecessary check frmo dev_pm_opp_attach_genpd() |
| |
On 27-08-20, 14:14, Stephan Gerhold wrote: > Only partially related to this patch, but actually I noticed that > dev_pm_opp_attach_genpd() does not work correctly if it is called > multiple times. > > For example, qcom-cpufreq-nvmem calls this for every CPU because it is > not aware that the OPP table is shared between the CPUs.
It could have called it from cpufreq_driver->init, but yeah I see the problem here.
> dev_pm_opp_attach_genpd() does not check if opp_table->genpd_virt_devs > is already set, so when it is called again for other CPUs we will: > > - Cause a memory leak (opp_table->genpd_virt_devs is just replaced > with new memory) > - Attach the power domains multiple times > - Never detach the power domains from earlier calls > - Crash when dev_pm_opp_detach_genpd() is called the second time > > Oh well. :) > > I think the function should just return and do nothing if the power > domains were already attached, just like dev_pm_opp_set_supported_hw() > etc. But this is a bit complicated to implement with the "virt_devs" > parameter, since callers will probably assume that to be valid if we > return success.
Or maybe at least make it work by returning the OPP table and not setting the virt_devs.
> Another advantage of my proposal to remove the virt_devs parameter [1] :)
Yes, I do see the advantage there, lets see where that discussion goes.
-- viresh
| ![\](/images/icornerr.gif) |