Re: [PATCH v4 6/8] iio: adc: ti-ads1015: Convert to managed resources

[Archit Anant]

On Mon, Aug 31, 2026 at 4:28 AM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On 2026-08-27 22:28:45+03:00, Andy Shevchenko wrote:
> > On Thu, Aug 27, 2026 at 10:50:58PM +0530, Archit Anant wrote:
> >
> > > Use managed resource helpers for the mutex and runtime PM setup.
> > >
> > > Replace mutex_init() with devm_mutex_init() and replace the separate
> > > pm_runtime_set_active() and devm_pm_runtime_enable() calls with
> > > devm_pm_runtime_set_active_enabled().
> > >
> > > This simplifies resource management and ensures resources are
> > > automatically cleaned up on probe failure and device removal.
> >
> > ...
> >
> > >     data = iio_priv(indio_dev);
> > >     i2c_set_clientdata(client, indio_dev);
> > >
> > > -   mutex_init(&data->lock);
> > > +   ret = devm_mutex_init(dev, &data->lock);
> > > +   if (ret)
> > > +           return ret;
> >
> > This part is good and maybe introduced in a separate commit (depends on the
> > resolution for the below).
> >
> > ...
> >
> > > -   ret = pm_runtime_set_active(dev);
> > > -   if (ret)
> > > -           return ret;
> > >     pm_runtime_set_autosuspend_delay(dev, ADS1015_SLEEP_DELAY_MS);
> > >     pm_runtime_use_autosuspend(dev);
> > > -   ret = devm_pm_runtime_enable(dev);
> > > +   ret = devm_pm_runtime_set_active_enabled(dev);
> > >     if (ret)
> > >             return ret;
> >
> > This should be done with care. There were some changes lately that actually
> > moved from devm_*() back to regular calls due to some races in runtime PM.

I reviewed the series again. Considering Andy's concerns, I looked
through other patches that included this change and the concerns
raised there.
However, in this driver the runtime suspend/resume callbacks only
update the conversion-mode bits through regmap. The existing
ads1015_power_down() devm action also only switches the device to
single-shot mode; it does not manipulate runtime-PM usage counts or
shared resources such as regulators or clocks (as in other cases).
Thus, I don't see a runtime-PM reference-count or resource-balance
issue introduced by the conversion.
Is there a type of condition I have missed? If there is a relevant
thread, I would be happy to check that out.

>
> In particular here, the patch description needs to say what this
> changes.  It is not a noop change.
>
> I'm not saying it is wrong, but I would do it in a seperate patch
> where you can clearly talk about what changes as a result of this
> and what affect that can be expected to have.

I understand now. Although the cleanup callback itself is simple, the
change to managed runtime PM also changes the teardown semantics, so
it is not a no-op change. I'll move this to a separate patch and
explicitly describe the behavioral change.

-- 
Sincerely,
Archit Anant