mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v1 0/4] iio: Fix runtime PM leaks across multiple drivers
@ 2026-06-14  7:15 Biren Pandya
  2026-06-14  7:15 ` [PATCH v1 1/4] iio: accel: kxsd9: fix runtime PM imbalance on write_raw() error Biren Pandya
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Biren Pandya @ 2026-06-14  7:15 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Linus Walleij, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel, Biren Pandya

Hi Jonathan,

While auditing the IIO subsystem, I noticed a recurring pattern in 
several drivers where early returns or error paths fail to drop their 
pm_runtime_get_sync() references. This permanently leaves the device 
active and prevents autosuspension.

This series provides four independent fixes for these imbalances across 
the kxsd9, mpu3050, mpl115, and gp2ap002 drivers.

Because these drivers are independent, these patches do not depend on 
each other and can be merged individually.

Best regards,
Biren Pandya


 drivers/iio/accel/kxsd9.c       |  5 +++--
 drivers/iio/gyro/mpu3050-core.c | 16 +++++++++++-----
 drivers/iio/light/gp2ap002.c    |  2 +-
 drivers/iio/pressure/mpl115.c   |  4 ++--
 4 files changed, 17 insertions(+), 10 deletions(-)

-- 
2.50.1 (Apple Git-155)


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v1 1/4] iio: accel: kxsd9: fix runtime PM imbalance on write_raw() error
  2026-06-14  7:15 [PATCH v1 0/4] iio: Fix runtime PM leaks across multiple drivers Biren Pandya
@ 2026-06-14  7:15 ` Biren Pandya
  2026-06-14 13:14   ` Jonathan Cameron
  2026-06-14  7:15 ` [PATCH v1 2/4] iio: gyro: mpu3050: fix runtime PM leak when enabling trigger fails Biren Pandya
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Biren Pandya @ 2026-06-14  7:15 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Linus Walleij, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel, Biren Pandya

kxsd9_write_raw() takes a runtime PM reference with pm_runtime_get_sync()
but returns -EINVAL directly when a scale with a non-zero integer part is
requested, skipping the matching pm_runtime_put_autosuspend(). This leaks
a runtime PM usage-counter reference on every such write, after which the
device can no longer autosuspend.

Set the error code and fall through to the existing put instead of
returning early.

Signed-off-by: Biren Pandya <birenpandya@gmail.com>
Assisted-by: Claude:claude-opus-4-8 coccinelle
---
 drivers/iio/accel/kxsd9.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index 4717d80fc24a..7ac885d94d7f 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -147,8 +147,9 @@ static int kxsd9_write_raw(struct iio_dev *indio_dev,
 	if (mask == IIO_CHAN_INFO_SCALE) {
 		/* Check no integer component */
 		if (val)
-			return -EINVAL;
-		ret = kxsd9_write_scale(indio_dev, val2);
+			ret = -EINVAL;
+		else
+			ret = kxsd9_write_scale(indio_dev, val2);
 	}
 
 	pm_runtime_put_autosuspend(st->dev);
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v1 2/4] iio: gyro: mpu3050: fix runtime PM leak when enabling trigger fails
  2026-06-14  7:15 [PATCH v1 0/4] iio: Fix runtime PM leaks across multiple drivers Biren Pandya
  2026-06-14  7:15 ` [PATCH v1 1/4] iio: accel: kxsd9: fix runtime PM imbalance on write_raw() error Biren Pandya
@ 2026-06-14  7:15 ` Biren Pandya
  2026-06-14 13:22   ` Jonathan Cameron
                     ` (2 more replies)
  2026-06-14  7:15 ` [PATCH v1 3/4] iio: pressure: mpl115: fix runtime PM leak on read error Biren Pandya
                   ` (2 subsequent siblings)
  4 siblings, 3 replies; 13+ messages in thread
From: Biren Pandya @ 2026-06-14  7:15 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Linus Walleij, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel, Biren Pandya

mpu3050_drdy_trigger_set_state() calls pm_runtime_get_sync() when the
trigger is enabled, but several error paths in the enable branch return
directly without dropping the usage counter again. pm_runtime_get_sync()
increments the usage counter, so every failed enable leaks a runtime PM
reference and the device can no longer autosuspend. The driver state flag
hw_irq_trigger is also left set after a failed enable.

Jump to a common error label that drops the reference and clears
hw_irq_trigger, mirroring the disable path. The success path deliberately
keeps the reference, as the device must stay resumed while the trigger is
active.

Signed-off-by: Biren Pandya <birenpandya@gmail.com>
Assisted-by: Claude:claude-opus-4-8 coccinelle
---
 drivers/iio/gyro/mpu3050-core.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index d84e04e4b431..783be9a786df 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -994,14 +994,14 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
 		/* Disable all things in the FIFO */
 		ret = regmap_write(mpu3050->map, MPU3050_FIFO_EN, 0);
 		if (ret)
-			return ret;
+			goto err_power_down;
 
 		/* Reset and enable the FIFO */
 		ret = regmap_set_bits(mpu3050->map, MPU3050_USR_CTRL,
 				      MPU3050_USR_CTRL_FIFO_EN |
 				      MPU3050_USR_CTRL_FIFO_RST);
 		if (ret)
-			return ret;
+			goto err_power_down;
 
 		mpu3050->pending_fifo_footer = false;
 
@@ -1013,12 +1013,12 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
 				   MPU3050_FIFO_EN_GYRO_ZOUT |
 				   MPU3050_FIFO_EN_FOOTER);
 		if (ret)
-			return ret;
+			goto err_power_down;
 
 		/* Configure the sample engine */
 		ret = mpu3050_start_sampling(mpu3050);
 		if (ret)
-			return ret;
+			goto err_power_down;
 
 		/* Clear IRQ flag */
 		ret = regmap_read(mpu3050->map, MPU3050_INT_STATUS, &val);
@@ -1037,10 +1037,16 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
 
 		ret = regmap_write(mpu3050->map, MPU3050_INT_CFG, val);
 		if (ret)
-			return ret;
+			goto err_power_down;
 	}
 
 	return 0;
+
+err_power_down:
+	pm_runtime_put_autosuspend(mpu3050->dev);
+	mpu3050->hw_irq_trigger = false;
+
+	return ret;
 }
 
 static const struct iio_trigger_ops mpu3050_trigger_ops = {
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v1 3/4] iio: pressure: mpl115: fix runtime PM leak on read error
  2026-06-14  7:15 [PATCH v1 0/4] iio: Fix runtime PM leaks across multiple drivers Biren Pandya
  2026-06-14  7:15 ` [PATCH v1 1/4] iio: accel: kxsd9: fix runtime PM imbalance on write_raw() error Biren Pandya
  2026-06-14  7:15 ` [PATCH v1 2/4] iio: gyro: mpu3050: fix runtime PM leak when enabling trigger fails Biren Pandya
@ 2026-06-14  7:15 ` Biren Pandya
  2026-06-14 13:24   ` Jonathan Cameron
  2026-06-14  7:15 ` [PATCH v1 4/4] iio: light: gp2ap002: " Biren Pandya
  2026-06-14 13:31 ` [PATCH v1 0/4] iio: Fix runtime PM leaks across multiple drivers Jonathan Cameron
  4 siblings, 1 reply; 13+ messages in thread
From: Biren Pandya @ 2026-06-14  7:15 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Linus Walleij, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel, Biren Pandya

mpl115_read_raw() takes a runtime PM reference with pm_runtime_get_sync()
before reading the processed pressure or raw temperature, but on the read
error path it returns without calling pm_runtime_put_autosuspend(). Each
failed read therefore leaks a runtime PM reference and prevents the device
from autosuspending.

Drop the reference before checking the return value so both the success
and error paths are balanced.

Signed-off-by: Biren Pandya <birenpandya@gmail.com>
Assisted-by: Claude:claude-opus-4-8 coccinelle
---
 drivers/iio/pressure/mpl115.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/pressure/mpl115.c b/drivers/iio/pressure/mpl115.c
index 830a5065c008..16e112b796ba 100644
--- a/drivers/iio/pressure/mpl115.c
+++ b/drivers/iio/pressure/mpl115.c
@@ -106,18 +106,18 @@ static int mpl115_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_PROCESSED:
 		pm_runtime_get_sync(data->dev);
 		ret = mpl115_comp_pressure(data, val, val2);
+		pm_runtime_put_autosuspend(data->dev);
 		if (ret < 0)
 			return ret;
-		pm_runtime_put_autosuspend(data->dev);
 
 		return IIO_VAL_INT_PLUS_MICRO;
 	case IIO_CHAN_INFO_RAW:
 		pm_runtime_get_sync(data->dev);
 		/* temperature -5.35 C / LSB, 472 LSB is 25 C */
 		ret = mpl115_read_temp(data);
+		pm_runtime_put_autosuspend(data->dev);
 		if (ret < 0)
 			return ret;
-		pm_runtime_put_autosuspend(data->dev);
 		*val = ret >> 6;
 
 		return IIO_VAL_INT;
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v1 4/4] iio: light: gp2ap002: fix runtime PM leak on read error
  2026-06-14  7:15 [PATCH v1 0/4] iio: Fix runtime PM leaks across multiple drivers Biren Pandya
                   ` (2 preceding siblings ...)
  2026-06-14  7:15 ` [PATCH v1 3/4] iio: pressure: mpl115: fix runtime PM leak on read error Biren Pandya
@ 2026-06-14  7:15 ` Biren Pandya
  2026-06-14 13:25   ` Jonathan Cameron
  2026-06-14 13:31 ` [PATCH v1 0/4] iio: Fix runtime PM leaks across multiple drivers Jonathan Cameron
  4 siblings, 1 reply; 13+ messages in thread
From: Biren Pandya @ 2026-06-14  7:15 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Linus Walleij, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel, Biren Pandya

gp2ap002_read_raw() calls pm_runtime_get_sync() before reading the
lux value, but if gp2ap002_get_lux() fails, it returns directly. This
skips the pm_runtime_put_autosuspend() call at the "out" label,
permanently leaking a runtime PM reference and preventing the device
from autosuspending.

Replace the direct return with a "goto out" to ensure the reference
is properly dropped on the error path.

Signed-off-by: Biren Pandya <birenpandya@gmail.com>
---
 drivers/iio/light/gp2ap002.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/light/gp2ap002.c b/drivers/iio/light/gp2ap002.c
index a0d8a58f2704..886accf5e859 100644
--- a/drivers/iio/light/gp2ap002.c
+++ b/drivers/iio/light/gp2ap002.c
@@ -258,7 +258,7 @@ static int gp2ap002_read_raw(struct iio_dev *indio_dev,
 		case IIO_LIGHT:
 			ret = gp2ap002_get_lux(gp2ap002);
 			if (ret < 0)
-				return ret;
+				goto out;
 			*val = ret;
 			ret = IIO_VAL_INT;
 			goto out;
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v1 1/4] iio: accel: kxsd9: fix runtime PM imbalance on write_raw() error
  2026-06-14  7:15 ` [PATCH v1 1/4] iio: accel: kxsd9: fix runtime PM imbalance on write_raw() error Biren Pandya
@ 2026-06-14 13:14   ` Jonathan Cameron
  0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2026-06-14 13:14 UTC (permalink / raw)
  To: Biren Pandya
  Cc: Linus Walleij, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel

On Sun, 14 Jun 2026 12:45:46 +0530
Biren Pandya <birenpandya@gmail.com> wrote:

> kxsd9_write_raw() takes a runtime PM reference with pm_runtime_get_sync()
> but returns -EINVAL directly when a scale with a non-zero integer part is
> requested, skipping the matching pm_runtime_put_autosuspend(). This leaks
> a runtime PM usage-counter reference on every such write, after which the
> device can no longer autosuspend.
> 
> Set the error code and fall through to the existing put instead of
> returning early.
> 
> Signed-off-by: Biren Pandya <birenpandya@gmail.com>
> Assisted-by: Claude:claude-opus-4-8 coccinelle
Good find.  This one is nasty as it is easily triggered from userspace.
So I'm going to pick this up as it stands.  However, if you are interested
in a doing a follow up series that cleans things up further:

- The pm_runtime_get_sync() return value is not checked anywhere in this driver.
  So good to do that.
- The various places in this driver (including this one) where runtime_pm
  is involved would be nicer handled using the relatively new
  PM_RUNTIME_ACQUIRE_AUTOSUSPEND() / PM_RUNTIME_ACQUIRE_ERR()

Probably makes sense to do both of those in one go given they'll be touching
the same code.

Applied to the fixes-togreg branch of iio.git and marked for stable.
Note I'll be rebasing that branch on rc1 before doing a pull request.

Thanks,

Jonathan


> ---
>  drivers/iio/accel/kxsd9.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
> index 4717d80fc24a..7ac885d94d7f 100644
> --- a/drivers/iio/accel/kxsd9.c
> +++ b/drivers/iio/accel/kxsd9.c
> @@ -147,8 +147,9 @@ static int kxsd9_write_raw(struct iio_dev *indio_dev,
>  	if (mask == IIO_CHAN_INFO_SCALE) {
>  		/* Check no integer component */
>  		if (val)
> -			return -EINVAL;
> -		ret = kxsd9_write_scale(indio_dev, val2);
> +			ret = -EINVAL;
> +		else
> +			ret = kxsd9_write_scale(indio_dev, val2);
>  	}
>  
>  	pm_runtime_put_autosuspend(st->dev);


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v1 2/4] iio: gyro: mpu3050: fix runtime PM leak when enabling trigger fails
  2026-06-14  7:15 ` [PATCH v1 2/4] iio: gyro: mpu3050: fix runtime PM leak when enabling trigger fails Biren Pandya
@ 2026-06-14 13:22   ` Jonathan Cameron
  2026-07-17 17:50   ` [PATCH v3] " Biren Pandya
  2026-07-17 18:10   ` [PATCH v4] " Biren Pandya
  2 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2026-06-14 13:22 UTC (permalink / raw)
  To: Biren Pandya
  Cc: Linus Walleij, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel

On Sun, 14 Jun 2026 12:45:47 +0530
Biren Pandya <birenpandya@gmail.com> wrote:

> mpu3050_drdy_trigger_set_state() calls pm_runtime_get_sync() when the
> trigger is enabled, but several error paths in the enable branch return
> directly without dropping the usage counter again. pm_runtime_get_sync()
> increments the usage counter, so every failed enable leaks a runtime PM
> reference and the device can no longer autosuspend. The driver state flag
> hw_irq_trigger is also left set after a failed enable.
> 
> Jump to a common error label that drops the reference and clears
> hw_irq_trigger, mirroring the disable path. The success path deliberately
> keeps the reference, as the device must stay resumed while the trigger is
> active.
> 
> Signed-off-by: Biren Pandya <birenpandya@gmail.com>
> Assisted-by: Claude:claude-opus-4-8 coccinelle

This looks more or less fine to me, but I'd like to leave it on list
for a little while so hopefully someone who is more familiar with the device
can take a look.

A suggestion below for a less minimal but perhaps nicer way of doing this.
> ---
>  drivers/iio/gyro/mpu3050-core.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
> index d84e04e4b431..783be9a786df 100644
> --- a/drivers/iio/gyro/mpu3050-core.c
> +++ b/drivers/iio/gyro/mpu3050-core.c
> @@ -994,14 +994,14 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
>  		/* Disable all things in the FIFO */
>  		ret = regmap_write(mpu3050->map, MPU3050_FIFO_EN, 0);
>  		if (ret)
> -			return ret;
> +			goto err_power_down;
>  
>  		/* Reset and enable the FIFO */
>  		ret = regmap_set_bits(mpu3050->map, MPU3050_USR_CTRL,
>  				      MPU3050_USR_CTRL_FIFO_EN |
>  				      MPU3050_USR_CTRL_FIFO_RST);
>  		if (ret)
> -			return ret;
> +			goto err_power_down;
>  
>  		mpu3050->pending_fifo_footer = false;
>  
> @@ -1013,12 +1013,12 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
>  				   MPU3050_FIFO_EN_GYRO_ZOUT |
>  				   MPU3050_FIFO_EN_FOOTER);
>  		if (ret)
> -			return ret;
> +			goto err_power_down;
>  
>  		/* Configure the sample engine */
>  		ret = mpu3050_start_sampling(mpu3050);
>  		if (ret)
> -			return ret;
> +			goto err_power_down;
>  
>  		/* Clear IRQ flag */
>  		ret = regmap_read(mpu3050->map, MPU3050_INT_STATUS, &val);
> @@ -1037,10 +1037,16 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
>  
>  		ret = regmap_write(mpu3050->map, MPU3050_INT_CFG, val);
>  		if (ret)
> -			return ret;
> +			goto err_power_down;
>  	}
>  
>  	return 0;
> +
> +err_power_down:

I'm not particularly keen on a goto error block that only applies to one
leg of an if / else.  Given there is effectively not sharing of code between the enable
and disable paths, I'd rather see this whole thing broken into two helpers.
mpu3050_drdy_trigger_enable() and mpu3050_drdy_trigger_disable()

Then the error unwind would not have the issue of only applying to some code
paths as it would be in the helper.  Obviously it is a more invasive change
but I think it is worth doing rather that having a minimal fix then sweeping around
later to replace all that code.


> +	pm_runtime_put_autosuspend(mpu3050->dev);
> +	mpu3050->hw_irq_trigger = false;
> +
> +	return ret;
>  }
>  
>  static const struct iio_trigger_ops mpu3050_trigger_ops = {


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v1 3/4] iio: pressure: mpl115: fix runtime PM leak on read error
  2026-06-14  7:15 ` [PATCH v1 3/4] iio: pressure: mpl115: fix runtime PM leak on read error Biren Pandya
@ 2026-06-14 13:24   ` Jonathan Cameron
  0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2026-06-14 13:24 UTC (permalink / raw)
  To: Biren Pandya
  Cc: Linus Walleij, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel

On Sun, 14 Jun 2026 12:45:48 +0530
Biren Pandya <birenpandya@gmail.com> wrote:

> mpl115_read_raw() takes a runtime PM reference with pm_runtime_get_sync()
> before reading the processed pressure or raw temperature, but on the read
> error path it returns without calling pm_runtime_put_autosuspend(). Each
> failed read therefore leaks a runtime PM reference and prevents the device
> from autosuspending.
> 
> Drop the reference before checking the return value so both the success
> and error paths are balanced.
> 
> Signed-off-by: Biren Pandya <birenpandya@gmail.com>
> Assisted-by: Claude:claude-opus-4-8 coccinelle
Applied and marked for stable.
Thanks,

Jonathan

> ---
>  drivers/iio/pressure/mpl115.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/pressure/mpl115.c b/drivers/iio/pressure/mpl115.c
> index 830a5065c008..16e112b796ba 100644
> --- a/drivers/iio/pressure/mpl115.c
> +++ b/drivers/iio/pressure/mpl115.c
> @@ -106,18 +106,18 @@ static int mpl115_read_raw(struct iio_dev *indio_dev,
>  	case IIO_CHAN_INFO_PROCESSED:
>  		pm_runtime_get_sync(data->dev);
>  		ret = mpl115_comp_pressure(data, val, val2);
> +		pm_runtime_put_autosuspend(data->dev);
>  		if (ret < 0)
>  			return ret;
> -		pm_runtime_put_autosuspend(data->dev);
>  
>  		return IIO_VAL_INT_PLUS_MICRO;
>  	case IIO_CHAN_INFO_RAW:
>  		pm_runtime_get_sync(data->dev);
>  		/* temperature -5.35 C / LSB, 472 LSB is 25 C */
>  		ret = mpl115_read_temp(data);
> +		pm_runtime_put_autosuspend(data->dev);
>  		if (ret < 0)
>  			return ret;
> -		pm_runtime_put_autosuspend(data->dev);
>  		*val = ret >> 6;
>  
>  		return IIO_VAL_INT;


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v1 4/4] iio: light: gp2ap002: fix runtime PM leak on read error
  2026-06-14  7:15 ` [PATCH v1 4/4] iio: light: gp2ap002: " Biren Pandya
@ 2026-06-14 13:25   ` Jonathan Cameron
  0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2026-06-14 13:25 UTC (permalink / raw)
  To: Biren Pandya
  Cc: Linus Walleij, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel

On Sun, 14 Jun 2026 12:45:49 +0530
Biren Pandya <birenpandya@gmail.com> wrote:

> gp2ap002_read_raw() calls pm_runtime_get_sync() before reading the
> lux value, but if gp2ap002_get_lux() fails, it returns directly. This
> skips the pm_runtime_put_autosuspend() call at the "out" label,
> permanently leaking a runtime PM reference and preventing the device
> from autosuspending.
> 
> Replace the direct return with a "goto out" to ensure the reference
> is properly dropped on the error path.
> 
> Signed-off-by: Biren Pandya <birenpandya@gmail.com>

Similar to patch one this would benefit from a follow up cleanup to
make use of the ACQUIRE macros.

This is good though, so applied to the fixes-togreg branch of iio.git
and marked for stable.

Thanks

Jonathan


> ---
>  drivers/iio/light/gp2ap002.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/light/gp2ap002.c b/drivers/iio/light/gp2ap002.c
> index a0d8a58f2704..886accf5e859 100644
> --- a/drivers/iio/light/gp2ap002.c
> +++ b/drivers/iio/light/gp2ap002.c
> @@ -258,7 +258,7 @@ static int gp2ap002_read_raw(struct iio_dev *indio_dev,
>  		case IIO_LIGHT:
>  			ret = gp2ap002_get_lux(gp2ap002);
>  			if (ret < 0)
> -				return ret;
> +				goto out;
>  			*val = ret;
>  			ret = IIO_VAL_INT;
>  			goto out;


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v1 0/4] iio: Fix runtime PM leaks across multiple drivers
  2026-06-14  7:15 [PATCH v1 0/4] iio: Fix runtime PM leaks across multiple drivers Biren Pandya
                   ` (3 preceding siblings ...)
  2026-06-14  7:15 ` [PATCH v1 4/4] iio: light: gp2ap002: " Biren Pandya
@ 2026-06-14 13:31 ` Jonathan Cameron
  4 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2026-06-14 13:31 UTC (permalink / raw)
  To: Biren Pandya
  Cc: Linus Walleij, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel

On Sun, 14 Jun 2026 12:45:45 +0530
Biren Pandya <birenpandya@gmail.com> wrote:

> Hi Jonathan,
> 
> While auditing the IIO subsystem, I noticed a recurring pattern in 
> several drivers where early returns or error paths fail to drop their 
> pm_runtime_get_sync() references. This permanently leaves the device 
> active and prevents autosuspension.
> 
> This series provides four independent fixes for these imbalances across 
> the kxsd9, mpu3050, mpl115, and gp2ap002 drivers.
> 
> Because these drivers are independent, these patches do not depend on 
> each other and can be merged individually.

Oops. Too me until just after I'd replied to patch 4 to notice
that these don't have the necessary Fixes tags.

I've added them, but keep that in mind for next time you send a fix.

Thanks,

Jonathan



> 
> Best regards,
> Biren Pandya
> 
> 
>  drivers/iio/accel/kxsd9.c       |  5 +++--
>  drivers/iio/gyro/mpu3050-core.c | 16 +++++++++++-----
>  drivers/iio/light/gp2ap002.c    |  2 +-
>  drivers/iio/pressure/mpl115.c   |  4 ++--
>  4 files changed, 17 insertions(+), 10 deletions(-)
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3] iio: gyro: mpu3050: fix runtime PM leak when enabling trigger fails
  2026-06-14  7:15 ` [PATCH v1 2/4] iio: gyro: mpu3050: fix runtime PM leak when enabling trigger fails Biren Pandya
  2026-06-14 13:22   ` Jonathan Cameron
@ 2026-07-17 17:50   ` Biren Pandya
  2026-07-17 18:01     ` Andy Shevchenko
  2026-07-17 18:10   ` [PATCH v4] " Biren Pandya
  2 siblings, 1 reply; 13+ messages in thread
From: Biren Pandya @ 2026-07-17 17:50 UTC (permalink / raw)
  To: Linus Walleij, Jonathan Cameron
  Cc: David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel, Biren Pandya

mpu3050_drdy_trigger_set_state() calls pm_runtime_get_sync() when the
trigger is enabled, but several error paths in the enable branch return
directly without dropping the usage counter again. pm_runtime_get_sync()
increments the usage counter, so every failed enable leaks a runtime PM
reference and the device can no longer autosuspend. The driver state flag
hw_irq_trigger is also left set after a failed enable.

Jump to a common error label that drops the reference and clears
hw_irq_trigger, mirroring the disable path. The success path deliberately
keeps the reference, as the device must stay resumed while the trigger is
active.

Signed-off-by: Biren Pandya <birenpandya@gmail.com>
Assisted-by: Claude:claude-opus-4-8 coccinelle
---
 drivers/iio/gyro/mpu3050-core.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index d84e04e4b4314..1f23e700a0dae 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -761,7 +761,7 @@ static const struct iio_info mpu3050_info = {
 	.attrs = &mpu3050_attribute_group,
 };
 
-/**
+/*
  * mpu3050_read_mem() - read MPU-3050 internal memory
  * @mpu3050: device to read from
  * @bank: target bank
@@ -994,14 +994,14 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
 		/* Disable all things in the FIFO */
 		ret = regmap_write(mpu3050->map, MPU3050_FIFO_EN, 0);
 		if (ret)
-			return ret;
+			goto err_power_down;
 
 		/* Reset and enable the FIFO */
 		ret = regmap_set_bits(mpu3050->map, MPU3050_USR_CTRL,
 				      MPU3050_USR_CTRL_FIFO_EN |
 				      MPU3050_USR_CTRL_FIFO_RST);
 		if (ret)
-			return ret;
+			goto err_power_down;
 
 		mpu3050->pending_fifo_footer = false;
 
@@ -1013,12 +1013,12 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
 				   MPU3050_FIFO_EN_GYRO_ZOUT |
 				   MPU3050_FIFO_EN_FOOTER);
 		if (ret)
-			return ret;
+			goto err_power_down;
 
 		/* Configure the sample engine */
 		ret = mpu3050_start_sampling(mpu3050);
 		if (ret)
-			return ret;
+			goto err_power_down;
 
 		/* Clear IRQ flag */
 		ret = regmap_read(mpu3050->map, MPU3050_INT_STATUS, &val);
@@ -1037,10 +1037,16 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
 
 		ret = regmap_write(mpu3050->map, MPU3050_INT_CFG, val);
 		if (ret)
-			return ret;
+			goto err_power_down;
 	}
 
 	return 0;
+
+err_power_down:
+	pm_runtime_put_autosuspend(mpu3050->dev);
+	mpu3050->hw_irq_trigger = false;
+
+	return ret;
 }
 
 static const struct iio_trigger_ops mpu3050_trigger_ops = {
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3] iio: gyro: mpu3050: fix runtime PM leak when enabling trigger fails
  2026-07-17 17:50   ` [PATCH v3] " Biren Pandya
@ 2026-07-17 18:01     ` Andy Shevchenko
  0 siblings, 0 replies; 13+ messages in thread
From: Andy Shevchenko @ 2026-07-17 18:01 UTC (permalink / raw)
  To: Biren Pandya
  Cc: Linus Walleij, Jonathan Cameron, David Lechner, Nuno Sá,
	Andy Shevchenko, linux-iio, linux-kernel

On Fri, Jul 17, 2026 at 11:20:22PM +0530, Biren Pandya wrote:
> mpu3050_drdy_trigger_set_state() calls pm_runtime_get_sync() when the
> trigger is enabled, but several error paths in the enable branch return
> directly without dropping the usage counter again. pm_runtime_get_sync()
> increments the usage counter, so every failed enable leaks a runtime PM
> reference and the device can no longer autosuspend. The driver state flag
> hw_irq_trigger is also left set after a failed enable.
> 
> Jump to a common error label that drops the reference and clears
> hw_irq_trigger, mirroring the disable path. The success path deliberately
> keeps the reference, as the device must stay resumed while the trigger is
> active.

...

> -/**
> +/*
>   * mpu3050_read_mem() - read MPU-3050 internal memory
>   * @mpu3050: device to read from
>   * @bank: target bank

Stray and actually wrong change. Please, use more brains and less AI.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v4] iio: gyro: mpu3050: fix runtime PM leak when enabling trigger fails
  2026-06-14  7:15 ` [PATCH v1 2/4] iio: gyro: mpu3050: fix runtime PM leak when enabling trigger fails Biren Pandya
  2026-06-14 13:22   ` Jonathan Cameron
  2026-07-17 17:50   ` [PATCH v3] " Biren Pandya
@ 2026-07-17 18:10   ` Biren Pandya
  2 siblings, 0 replies; 13+ messages in thread
From: Biren Pandya @ 2026-07-17 18:10 UTC (permalink / raw)
  To: Linus Walleij, Jonathan Cameron
  Cc: David Lechner, Nuno Sá,
	Andy Shevchenko, Andy Shevchenko, linux-iio, linux-kernel,
	Biren Pandya

mpu3050_drdy_trigger_set_state() calls pm_runtime_get_sync() when the
trigger is enabled, but several error paths in the enable branch return
directly without dropping the usage counter again. pm_runtime_get_sync()
increments the usage counter, so every failed enable leaks a runtime PM
reference and the device can no longer autosuspend. The driver state flag
hw_irq_trigger is also left set after a failed enable.

Jump to a common error label that drops the reference and clears
hw_irq_trigger, mirroring the disable path. The success path deliberately
keeps the reference, as the device must stay resumed while the trigger is
active.

Signed-off-by: Biren Pandya <birenpandya@gmail.com>
---
Changes in v4:
- Reverted stray change converting `mpu3050_read_mem()` kernel-doc to a regular comment.
- Dropped AI assistance tag from the commit message.
Changes in v3:
- Changed to a unified error label instead of individual drops.
Changes in v2:
- Fixed a missed PM release in the first error branch.

 drivers/iio/gyro/mpu3050-core.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/gyro/mpu3050-core.c b/drivers/iio/gyro/mpu3050-core.c
index d84e04e4b4314..783be9a786df0 100644
--- a/drivers/iio/gyro/mpu3050-core.c
+++ b/drivers/iio/gyro/mpu3050-core.c
@@ -994,14 +994,14 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
 		/* Disable all things in the FIFO */
 		ret = regmap_write(mpu3050->map, MPU3050_FIFO_EN, 0);
 		if (ret)
-			return ret;
+			goto err_power_down;
 
 		/* Reset and enable the FIFO */
 		ret = regmap_set_bits(mpu3050->map, MPU3050_USR_CTRL,
 				      MPU3050_USR_CTRL_FIFO_EN |
 				      MPU3050_USR_CTRL_FIFO_RST);
 		if (ret)
-			return ret;
+			goto err_power_down;
 
 		mpu3050->pending_fifo_footer = false;
 
@@ -1013,12 +1013,12 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
 				   MPU3050_FIFO_EN_GYRO_ZOUT |
 				   MPU3050_FIFO_EN_FOOTER);
 		if (ret)
-			return ret;
+			goto err_power_down;
 
 		/* Configure the sample engine */
 		ret = mpu3050_start_sampling(mpu3050);
 		if (ret)
-			return ret;
+			goto err_power_down;
 
 		/* Clear IRQ flag */
 		ret = regmap_read(mpu3050->map, MPU3050_INT_STATUS, &val);
@@ -1037,10 +1037,16 @@ static int mpu3050_drdy_trigger_set_state(struct iio_trigger *trig,
 
 		ret = regmap_write(mpu3050->map, MPU3050_INT_CFG, val);
 		if (ret)
-			return ret;
+			goto err_power_down;
 	}
 
 	return 0;
+
+err_power_down:
+	pm_runtime_put_autosuspend(mpu3050->dev);
+	mpu3050->hw_irq_trigger = false;
+
+	return ret;
 }
 
 static const struct iio_trigger_ops mpu3050_trigger_ops = {
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-07-17 18:10 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-14  7:15 [PATCH v1 0/4] iio: Fix runtime PM leaks across multiple drivers Biren Pandya
2026-06-14  7:15 ` [PATCH v1 1/4] iio: accel: kxsd9: fix runtime PM imbalance on write_raw() error Biren Pandya
2026-06-14 13:14   ` Jonathan Cameron
2026-06-14  7:15 ` [PATCH v1 2/4] iio: gyro: mpu3050: fix runtime PM leak when enabling trigger fails Biren Pandya
2026-06-14 13:22   ` Jonathan Cameron
2026-07-17 17:50   ` [PATCH v3] " Biren Pandya
2026-07-17 18:01     ` Andy Shevchenko
2026-07-17 18:10   ` [PATCH v4] " Biren Pandya
2026-06-14  7:15 ` [PATCH v1 3/4] iio: pressure: mpl115: fix runtime PM leak on read error Biren Pandya
2026-06-14 13:24   ` Jonathan Cameron
2026-06-14  7:15 ` [PATCH v1 4/4] iio: light: gp2ap002: " Biren Pandya
2026-06-14 13:25   ` Jonathan Cameron
2026-06-14 13:31 ` [PATCH v1 0/4] iio: Fix runtime PM leaks across multiple drivers Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox