Re: [PATCH v2 09/17] iio: hid-sensors: Discard pm_runtime_put_autosuspend() return values

[srinivas pandruvada]

On Mon, 2026-11-02 at 01:52 +0000, Jonathan Cameron wrote:
> On Wed, 21 Oct 2026 16:57:10 -0700
> Brian Norris <briannorris@xxxxxxxxxxxx> wrote:
> 
> > The hid-sensors 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.
> > 
> > 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").
> > 
> > Signed-off-by: Brian Norris <briannorris@xxxxxxxxxxxx>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
> +CC the various hid-sensors maintainers.
> 

    Acked-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>


> Anyone remember why this does
> pm_runtime_use_autosuspend(&st->pdev->dev);
> in this code?
> 
> That is unusual...  My immediate instinct is to
> get rid of it, but maybe there is something subtle
> going on.

When reading sensor via raw interface, you need to power-up, read and
power-down (This is done via runtime PM). To avoid repeating this
sequence of power up and power down for consecutive fast reads, auto
suspend is used. If there are reads with in 3 seconds, the sensor will
keep power up.

Thanks,
Srinivas

> 
> > ---
> > 
> > Changes in v2:
> >  * Split iio patch into per-driver patches
> > 
> >  .../iio/common/hid-sensors/hid-sensor-trigger.c    | 14 +++++-----
> > ----
> >  1 file changed, 5 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > index 60808e2430ca..d76a6d9f04f6 100644
> > --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> > @@ -154,21 +154,17 @@ int hid_sensor_power_state(struct
> > hid_sensor_common *st, bool state)
> >  {
> >  
> >  #ifdef CONFIG_PM
> > -	int ret;
> > -
> >  	if (atomic_add_unless(&st->runtime_pm_enable, 1, 1))
> >  		pm_runtime_enable(&st->pdev->dev);
> >  
> >  	if (state) {
> >  		atomic_inc(&st->user_requested_state);
> > -		ret = pm_runtime_resume_and_get(&st->pdev->dev);
> > -	} else {
> > -		atomic_dec(&st->user_requested_state);
> > -		pm_runtime_use_autosuspend(&st->pdev->dev);
> > -		ret = pm_runtime_put_autosuspend(&st->pdev->dev);
> > +		return pm_runtime_resume_and_get(&st->pdev->dev);
> >  	}
> > -	if (ret < 0)
> > -		return ret;
> > +
> > +	atomic_dec(&st->user_requested_state);
> > +	pm_runtime_use_autosuspend(&st->pdev->dev);
> This is odd enough to make me nervous even though the patch is
> obviously a no-op change.
> 
> 
> > +	pm_runtime_put_autosuspend(&st->pdev->dev);
> >  
> >  	return 0;
> >  #else
>