Re: [PATCH] cpufreq: qcom-hw: add missing devm_release_mem_region() call

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Tue 12 Jan 22:31 CST 2021, Viresh Kumar wrote:

> On 12-01-21, 08:44, Bjorn Andersson wrote:
> > Intuitively I feel that resources allocated in cpufreq_driver->init()
> > should be explicitly freed in cpufreq_driver->exit() and should thereby
> > not use devm to track the allocations.
> 
> I agree.
> 
> > But afaict when qcom_cpufreq_hw_driver_remove() calls
> > cpufreq_unregister_driver() to end up in cpufreq_remove_dev() it will
> > only call cpufreq_driver->exit() iff cpufreq_driver->offline() is
> > implemented - which it isn't in our case.
> 
> cpufreq_offline() calls exit() in your case. So no memory leak here.
> 

Okay, so cpufreq_offline() calls exit iff there's no offline implemented
and therefor cpufreq_remove_dev() need to call exit if the other code
path was taken in cpufreq_offline()...that's...clever...

Thanks for confirming that exit() will be invoked, then we should stop
using the devm versions of the allocators.


But that said, why are the ioremap done at init and not at probe time?

Regards,
Bjorn

> > So without using devm to track
> > this we would leak the memory - which also implies that we're leaking
> > the "freq_table" when this happens.
> > 
> > But isn't that simply a typo in cpufreq_remove_dev()? And can't we just
> > use ioremap()/iounmap() here instead?
> 
> -- 
> viresh