Re: [PATCH v2 1/3] iio: light: gp2ap002: Fix unbalanced runtime PM on repeated event writes

On Wed, 22 Jul 2026 21:52:45 +0530
Nikhil Gautam <[email protected]> wrote:

> The IIO core does not filter duplicate writes to the event enable
> attribute, so writing the same value twice invokes
> write_event_config() twice. Enabling twice leaks a runtime PM
> reference, preventing the device from ever suspending again;
> disabling twice underflows the usage count and triggers a
> "Runtime PM usage count underflow" warning.
> 
> Bail out early when the requested state matches the current state.
> While at it, switch to pm_runtime_resume_and_get() so a failed
> resume is propagated to userspace instead of silently marking the
> event enabled.
> 
> Fixes: 97d642e23037c ("iio: light: Add a driver for Sharp GP2AP002x00F")
> Signed-off-by: Nikhil Gautam <[email protected]>

https://sashiko.dev/#/patchset/20260722162247.8229-1-nikhilgtr%40gmail.com

Sashiko raises a point about concurrent accessors. My previous understanding
was that we were fine if there was only a single file involved.  Having
dug around a bit I'm fairly sure that is correct.
The comment here implying to me that we are fine.
https://elixir.bootlin.com/linux/v7.1.4/source/fs/kernfs/file.c#L340

Now if we had more than one file then we couldn't rely on this.
Hence to me it's a bit fragile and we should probably add a local lock
anyway.

One other comment inline.

> ---
>  drivers/iio/light/gp2ap002.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/light/gp2ap002.c b/drivers/iio/light/gp2ap002.c
> index 42859e5b1089..966459994995 100644
> --- a/drivers/iio/light/gp2ap002.c
> +++ b/drivers/iio/light/gp2ap002.c
> @@ -343,6 +343,10 @@ static int gp2ap002_write_event_config(struct iio_dev *indio_dev,
>  				       bool state)
>  {
>  	struct gp2ap002 *gp2ap002 = iio_priv(indio_dev);
> +	int ret;
> +
> +	if (state == gp2ap002->enabled)
> +		return 0;
>  
>  	if (state) {
>  		/*
> @@ -350,14 +354,15 @@ static int gp2ap002_write_event_config(struct iio_dev *indio_dev,
>  		 * already) and reintialize the sensor by using runtime_pm
>  		 * callbacks.
>  		 */
> -		pm_runtime_get_sync(gp2ap002->dev);
> -		gp2ap002->enabled = true;

> +
> +		ret = pm_runtime_resume_and_get(gp2ap002->dev);
> +		if (ret)
> +			return ret;
>  	} else {
> -		pm_runtime_mark_last_busy(gp2ap002->dev);
>  		pm_runtime_put_autosuspend(gp2ap002->dev);
> -		gp2ap002->enabled = false;
>  	}
>  
> +	gp2ap002->enabled = state;
>  	return 0;
>  }
>