Re: [PATCH v2] counter: ti-eqep: balance pm_runtime on devm_clk_get_enabled() failure

From: Andy Shevchenko

Date: Wed May 27 2026 - 04:30:18 EST


On Wed, May 27, 2026 at 10:18 AM William Breathitt Gray <wbg@xxxxxxxxxx> wrote:
> On Wed, May 27, 2026 at 10:10:20AM +0200, Andy Shevchenko wrote:
> > On Mon, May 25, 2026 at 5:21 PM Stepan Ionichev <sozdayvek@xxxxxxxxx> wrote:

...

> > > pm_runtime_get_sync(dev);
> > >
> > > clk = devm_clk_get_enabled(dev, NULL);
> > > - if (IS_ERR(clk))
> > > - return dev_err_probe(dev, PTR_ERR(clk), "failed to enable clock\n");
> > > + if (IS_ERR(clk)) {
> > > + err = dev_err_probe(dev, PTR_ERR(clk), "failed to enable clock\n");
> > > + goto err_pm;
> >
> > This is not right. The devm_*() should not be followed by goto.
> >
> > > + }

...
> >
> > > +err_pm:
> > > + pm_runtime_put_sync(dev);
> > > + pm_runtime_disable(dev);
> > > + return err;
> >
> > You have to use the respective devm_pm_*() at the same time.

...

> > William, this patch should not be applied (or should be reverted) as
> > in current form it replaces one issue by another.
>
> I'll revert for now. Would the proper fix be to replace all
> pm_runtime_*() with their respective devm_pm_*() calls?

Not all, but the ones that are opposite to
pm_runtime_put*()/pm_runtime_disable(). It should be done carefully as
well as PM runtime may be required before final registering of the
device to access the registers (I haven't checked the driver, so I
don't know if simple use devm_pm_*() will be enough).

--
With Best Regards,
Andy Shevchenko