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

[Jonathan Cameron]

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.

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.

> ---
> 
> 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