[PATCH v2 0/3] iio: light: gp2ap002: runtime PM and error path fixes

[PATCH v2 0/3] iio: light: gp2ap002: runtime PM and error path fixes

[Nikhil Gautam]

Hi,

This series fixes three issues in the gp2ap002 driver, all present
since the driver was introduced.

Patch 1 fixes an unbalanced runtime PM reference count reachable from
userspace by writing the event enable attribute twice with the same
value, as the IIO core does not deduplicate such writes.

Patch 2 fixes the runtime suspend error path leaving the IRQ disable
depth unbalanced, permanently disabling proximity events after a
failed suspend.

Patch 3 fixes regulator enable leaks in the runtime resume error
paths, which matters as the supplies may be shared with other
devices.

Regards,
Nikhil Gautam

Changes in v2:
- Removed new lines between Tags as per Andy's comment
- Removed dup call pm_runtime_mark_last_busy() in
  gp2ap002_write_event_config() as already handled in
  pm_runtime_put_autosuspend() as per Andy's comment

Nikhil Gautam (3):
  iio: light: gp2ap002: Fix unbalanced runtime PM on repeated event
    writes
  iio: light: gp2ap002: re-enable irq if runtime suspend fails
  iio: light: gp2ap002: Fix regulator leaks in runtime resume error path

 drivers/iio/light/gp2ap002.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

-- 
2.39.5

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

[Nikhil Gautam]

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 <nikhilgtr@gmail.com>
---
 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;
 }
 
-- 
2.39.5

[PATCH v2 2/3] iio: light: gp2ap002: re-enable irq if runtime suspend fails

[Nikhil Gautam]

gp2ap002_runtime_suspend() disables the irq before writing OPMOD. If
the write fails, the callback returns an error with the irq still
disabled while the PM core marks the device active again.

re-enable the irq before returning the error so the irq state matches
the active state the PM core restores.

Fixes: 97d642e23037c ("iio: light: Add a driver for Sharp GP2AP002x00F")
Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
---
 drivers/iio/light/gp2ap002.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/light/gp2ap002.c b/drivers/iio/light/gp2ap002.c
index 966459994995..e8ba9c00dabb 100644
--- a/drivers/iio/light/gp2ap002.c
+++ b/drivers/iio/light/gp2ap002.c
@@ -649,6 +649,7 @@ static int gp2ap002_runtime_suspend(struct device *dev)
 	/* Disable chip and IRQ, everything off */
 	ret = regmap_write(gp2ap002->map, GP2AP002_OPMOD, 0x00);
 	if (ret) {
+		enable_irq(gp2ap002->irq);
 		dev_err(gp2ap002->dev, "error setting up operation mode\n");
 		return ret;
 	}
-- 
2.39.5

[PATCH v2 3/3] iio: light: gp2ap002: Fix regulator leaks in runtime resume error path

[Nikhil Gautam]

If enabling vio fails, vdd is left enabled; if re-initializing the
sensor fails, both regulators are left enabled.

unwind previously enabled regulators on failure, mirroring the error
handling already used in probe.

Fixes: 97d642e23037c ("iio: light: Add a driver for Sharp GP2AP002x00F")
Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
---
 drivers/iio/light/gp2ap002.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/light/gp2ap002.c b/drivers/iio/light/gp2ap002.c
index e8ba9c00dabb..430336b51f83 100644
--- a/drivers/iio/light/gp2ap002.c
+++ b/drivers/iio/light/gp2ap002.c
@@ -677,7 +677,7 @@ static int gp2ap002_runtime_resume(struct device *dev)
 	ret = regulator_enable(gp2ap002->vio);
 	if (ret) {
 		dev_err(dev, "failed to enable VIO regulator in resume path\n");
-		return ret;
+		goto out_disable_vdd;
 	}
 
 	msleep(20);
@@ -685,13 +685,19 @@ static int gp2ap002_runtime_resume(struct device *dev)
 	ret = gp2ap002_init(gp2ap002);
 	if (ret) {
 		dev_err(dev, "re-initialization failed\n");
-		return ret;
+		goto out_disable_vio;
 	}
 
 	/* Re-activate the IRQ */
 	enable_irq(gp2ap002->irq);
 
 	return 0;
+
+out_disable_vio:
+	regulator_disable(gp2ap002->vio);
+out_disable_vdd:
+	regulator_disable(gp2ap002->vdd);
+	return ret;
 }
 
 static DEFINE_RUNTIME_DEV_PM_OPS(gp2ap002_dev_pm_ops, gp2ap002_runtime_suspend,
-- 
2.39.5

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

[Jonathan Cameron]

On Wed, 22 Jul 2026 21:52:45 +0530
Nikhil Gautam <nikhilgtr@gmail.com> 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 <nikhilgtr@gmail.com>

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

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

[Jonathan Cameron]

On Sat, 25 Jul 2026 01:52:45 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Wed, 22 Jul 2026 21:52:45 +0530
> Nikhil Gautam <nikhilgtr@gmail.com> 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 <nikhilgtr@gmail.com>  
> 
> 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.
Huh. no idea what that comment was.  Never mind.
> 
> > ---
> >  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;
> >  }
> >    
> 
> 

Re: [PATCH v2 3/3] iio: light: gp2ap002: Fix regulator leaks in runtime resume error path

[Jonathan Cameron]

On Wed, 22 Jul 2026 21:52:47 +0530
Nikhil Gautam <nikhilgtr@gmail.com> wrote:

> If enabling vio fails, vdd is left enabled; if re-initializing the
> sensor fails, both regulators are left enabled.
> 
> unwind previously enabled regulators on failure, mirroring the error
> handling already used in probe.
> 
> Fixes: 97d642e23037c ("iio: light: Add a driver for Sharp GP2AP002x00F")
> Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
I'm already carrying this fix via a patch from Laxman,
so dropping this one.


Applied 1 and 2 but there was some noise due to other changes near this
code so please take a look.

Applied to the fixes-togreg branch of iio.git

Thanks

Jonathan

> ---
>  drivers/iio/light/gp2ap002.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/light/gp2ap002.c b/drivers/iio/light/gp2ap002.c
> index e8ba9c00dabb..430336b51f83 100644
> --- a/drivers/iio/light/gp2ap002.c
> +++ b/drivers/iio/light/gp2ap002.c
> @@ -677,7 +677,7 @@ static int gp2ap002_runtime_resume(struct device *dev)
>  	ret = regulator_enable(gp2ap002->vio);
>  	if (ret) {
>  		dev_err(dev, "failed to enable VIO regulator in resume path\n");
> -		return ret;
> +		goto out_disable_vdd;
>  	}
>  
>  	msleep(20);
> @@ -685,13 +685,19 @@ static int gp2ap002_runtime_resume(struct device *dev)
>  	ret = gp2ap002_init(gp2ap002);
>  	if (ret) {
>  		dev_err(dev, "re-initialization failed\n");
> -		return ret;
> +		goto out_disable_vio;
>  	}
>  
>  	/* Re-activate the IRQ */
>  	enable_irq(gp2ap002->irq);
>  
>  	return 0;
> +
> +out_disable_vio:
> +	regulator_disable(gp2ap002->vio);
> +out_disable_vdd:
> +	regulator_disable(gp2ap002->vdd);
> +	return ret;
>  }
>  
>  static DEFINE_RUNTIME_DEV_PM_OPS(gp2ap002_dev_pm_ops, gp2ap002_runtime_suspend,