Re: [PATCH] omap-cpufreq: Fix regulator resource leak in probe()

[Andreas Kemnade <andreas@kemnade.info>]

On Tue, 6 Jan 2026 10:20:55 +0530
Viresh Kumar <viresh.kumar@linaro.org> wrote:

> On 05-01-26, 10:14, Andreas Kemnade wrote:
> > On Mon, 15 Dec 2025 11:03:27 +0800
> > Haotian Zhang <vulab@iscas.ac.cn> wrote:
> >   
> > > The current omap_cpufreq_probe() uses regulator_get() to obtain the MPU
> > > regulator but does not release it in omap_cpufreq_remove() or when
> > > cpufreq_register_driver() fails, leading to a potential resource leak.
> > > 
> > > Use devm_regulator_get() instead of regulator_get() so that the regulator
> > > resource is automatically released.
> > > 
> > > Fixes: 53dfe8a884e6 ("cpufreq: OMAP: scale voltage along with frequency")
> > > Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
> > > ---
> > >  drivers/cpufreq/omap-cpufreq.c | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
> > > index bbb01d93b54b..f83f85996b36 100644
> > > --- a/drivers/cpufreq/omap-cpufreq.c
> > > +++ b/drivers/cpufreq/omap-cpufreq.c
> > > @@ -157,7 +157,7 @@ static int omap_cpufreq_probe(struct platform_device *pdev)
> > >  		return -EINVAL;
> > >  	}
> > >  
> > > -	mpu_reg = regulator_get(mpu_dev, "vcc");
> > > +	mpu_reg = devm_regulator_get(mpu_dev, "vcc");
> > >  	if (IS_ERR(mpu_reg)) {
> > >  		pr_warn("%s: unable to get MPU regulator\n", __func__);
> > >  		mpu_reg = NULL;
> > > @@ -169,7 +169,6 @@ static int omap_cpufreq_probe(struct platform_device *pdev)
> > >  		if (regulator_get_voltage(mpu_reg) < 0) {
> > >  			pr_warn("%s: physical regulator not present for MPU\n",
> > >  				__func__);
> > > -			regulator_put(mpu_reg);  
> > 
> > so it it not useable and could be released which is not done anymare 
> > with your patch. It is not an error path here.  
> 
> Right. Perhaps devm_regulator_put() here would be good enough.
> 
ok, didn't expect such a function, so that should be the cleanest approach.

> > >  			mpu_reg = NULL;  
> > 
> > And this should happen after removal, too. I feel some discomfort with
> > variables pointing to freed ressources. So I think rather add
> > the regulator_put and the = NULL to the remove function.  
> 
> I don't see a reason why this extra step should be performed after the driver is
> removed. `mpu_reg` can't be used after that.
> 
hmm, it is performed when the device is removed/unbound, which does not necessarily
mean the driver is removed. But that does not prevent trouble if something
is still trying to access stuff here after driver removal. So it is not really
helpful.

Hmm, how does a device gets bound to this driver?

Lets gets back to this very basic question. I am usually using CPUFREQ_DT.
Are there any signs of usage of this driver?

omap2plus_defconfig creates in .config
#
# CPU frequency scaling drivers
#
CONFIG_CPUFREQ_DT=m
# CONFIG_CPUFREQ_VIRT is not set
CONFIG_CPUFREQ_DT_PLATDEV=y
# CONFIG_ARM_OMAP2PLUS_CPUFREQ is not set
CONFIG_ARM_TI_CPUFREQ=y
# end of CPU Frequency scaling

So this thing is not used. Everything with omap2plus uses devicetree,
so probably no user at all for it. So I think we can deorbit the whole
thing.

But the fix is good for stable. So I would propose to add this
fix (to let it propagate to stable) and deorbit this driver.

Regards,
Andreas