Re: [PATCH v2 01/17] iio: accel: fxls8962af: Discard pm_runtime_put_autosuspend() return values

[Brian Norris]

On Thu, Oct 22, 2026 at 09:53:12AM +0300, Andy Shevchenko wrote:
> On Wed, Oct 21, 2026 at 04:57:02PM -0700, Brian Norris wrote:
> > This driver propagates the return value of pm_runtime_put_autosuspend(),
> > which is not particularly useful.
> > 
> > Returning an error code from pm_runtime_put_autosuspend() merely means
> > that it has not queued up a timer or work item to check whether or not
> > the device can be suspended and there are many perfectly valid
> > situations in which that can happen, like after writing "on" to the
> > device's runtime PM "control" attribute in sysfs for one example.
> > 
> > For this reason, modify this driver to simply discard the
> > pm_runtime_put_autosuspend() return value, and inline the
> > fxls8962af_power_off/on() helpers to their callsites, as they're even
> > more trivial now.
> > 
> > This will facilitate a planned change of the
> > pm_runtime_put_autosuspend() return type to void in the future, similar
> > to commit 3afd8df02433 ("PM: runtime: Change pm_runtime_put() return
> > type to void").
> 
> ...
> 
> > static int fxls8962af_get_out(struct fxls8962af_data *data,
> 
> >  
> >  	is_active = fxls8962af_is_active(data);
> 
> >  	if (!is_active) {
> 
> Why do we have this check at all?

Frankly, I don't know. It seems logically wrong; you shouldn't even be
reading device registers without first grabbing a runtime PM reference.

But I'm also loathe to go tweak a bunch of driver-specific details I'm
completely unfamiliar with, when my main purpose is extremely simple --
to stop checking one return code.

All in all, I think this is beside the point, and I'd rather not try to
address it in this series. For one, even if I try to address it, there's
more chance of regressions that will distract from my purpose.

> > -		ret = fxls8962af_power_on(data);
> > -		if (ret)
> > +		ret = pm_runtime_resume_and_get(dev);
> > +		if (ret) {
> > +			dev_err(dev, "failed to power on: %d\n", ret);
> >  			return ret;
> > +		}
> >  	}
> >  
> >  	ret = regmap_bulk_read(data->regmap, chan->address,
> >  			       &raw_val, sizeof(data->lower_thres));
> >  
> >  	if (!is_active)
> > -		fxls8962af_power_off(data);
> > +		pm_runtime_put_autosuspend(dev);
> 
> If it's active my understanding that runtime PM keeps device already resumed,
> so these two will be just a reference counting calls, no?

Right. I don't really see the point.

Brian