linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iio: accel: kxsd9: Fix runtime PM error handling and scoping
@ 2026-06-15 20:03 Biren Pandya
  2026-06-16  8:44 ` Andy Shevchenko
  2026-06-21 17:41 ` Jonathan Cameron
  0 siblings, 2 replies; 3+ messages in thread
From: Biren Pandya @ 2026-06-15 20:03 UTC (permalink / raw)
  To: Jonathan Cameron, linux-iio
  Cc: David Lechner, Nuno Sá, Andy Shevchenko, Sakari Ailus,
	linux-kernel, Biren Pandya

The kxsd9 driver uses pm_runtime_get_sync() in multiple places without
checking its return value, which can lead to silent failures. It also
relies on manual pm_runtime_put_autosuspend() calls in various return
paths, which is prone to memory leaks if a return path is missed.

Fix the runtime PM handling by using the new scoped guards and proper
error checking:
- In kxsd9_write_raw() and kxsd9_read_raw(), use the scoped
  PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND() macro to safely automate
  cleanup on function return, while maintaining the behavior of
  waking up a suspended device via pm_runtime_resume_and_get().
  This allows us to cleanly remove all manual puts and error gotos.
- In kxsd9_buffer_preenable(), replace pm_runtime_get_sync() with
  pm_runtime_resume_and_get() to safely check for errors and handle
  usage-counter unwinding on failure.
- In kxsd9_common_remove(), replace pm_runtime_get_sync() with
  pm_runtime_resume_and_get() and only invoke kxsd9_power_down()
  if the resume succeeds. This prevents unbalanced regulator disable
  warnings and bus timeouts if the device is already suspended.

Signed-off-by: Biren Pandya <birenpandya@gmail.com>
---
 drivers/iio/accel/kxsd9.c | 50 +++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 26 deletions(-)

diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
index 4717d80fc24a..f23d1660e6da 100644
--- a/drivers/iio/accel/kxsd9.c
+++ b/drivers/iio/accel/kxsd9.c
@@ -139,68 +139,63 @@ static int kxsd9_write_raw(struct iio_dev *indio_dev,
 			   int val2,
 			   long mask)
 {
-	int ret = -EINVAL;
 	struct kxsd9_state *st = iio_priv(indio_dev);
 
-	pm_runtime_get_sync(st->dev);
+	PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(st->dev, pm);
+	if (PM_RUNTIME_ACQUIRE_ERR(&pm))
+		return PM_RUNTIME_ACQUIRE_ERR(&pm);
 
 	if (mask == IIO_CHAN_INFO_SCALE) {
 		/* Check no integer component */
 		if (val)
 			return -EINVAL;
-		ret = kxsd9_write_scale(indio_dev, val2);
+		return kxsd9_write_scale(indio_dev, val2);
 	}
 
-	pm_runtime_put_autosuspend(st->dev);
-
-	return ret;
+	return -EINVAL;
 }
 
 static int kxsd9_read_raw(struct iio_dev *indio_dev,
 			  struct iio_chan_spec const *chan,
 			  int *val, int *val2, long mask)
 {
-	int ret = -EINVAL;
 	struct kxsd9_state *st = iio_priv(indio_dev);
 	unsigned int regval;
 	__be16 raw_val;
 	u16 nval;
+	int ret;
 
-	pm_runtime_get_sync(st->dev);
+	PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(st->dev, pm);
+	if (PM_RUNTIME_ACQUIRE_ERR(&pm))
+		return PM_RUNTIME_ACQUIRE_ERR(&pm);
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		ret = regmap_bulk_read(st->map, chan->address, &raw_val,
 				       sizeof(raw_val));
 		if (ret)
-			goto error_ret;
+			return ret;
 		nval = be16_to_cpu(raw_val);
 		/* Only 12 bits are valid */
 		nval >>= 4;
 		*val = nval;
-		ret = IIO_VAL_INT;
-		break;
+		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_OFFSET:
 		/* This has a bias of -2048 */
 		*val = KXSD9_ZERO_G_OFFSET;
-		ret = IIO_VAL_INT;
-		break;
+		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
 		ret = regmap_read(st->map,
 				  KXSD9_REG_CTRL_C,
 				  &regval);
 		if (ret < 0)
-			goto error_ret;
+			return ret;
 		*val = 0;
 		*val2 = kxsd9_micro_scales[regval & KXSD9_CTRL_C_FS_MASK];
-		ret = IIO_VAL_INT_PLUS_MICRO;
-		break;
+		return IIO_VAL_INT_PLUS_MICRO;
 	}
 
-error_ret:
-	pm_runtime_put_autosuspend(st->dev);
-
-	return ret;
+	return -EINVAL;
 };
 
 static irqreturn_t kxsd9_trigger_handler(int irq, void *p)
@@ -239,9 +234,7 @@ static int kxsd9_buffer_preenable(struct iio_dev *indio_dev)
 {
 	struct kxsd9_state *st = iio_priv(indio_dev);
 
-	pm_runtime_get_sync(st->dev);
-
-	return 0;
+	return pm_runtime_resume_and_get(st->dev);
 }
 
 static int kxsd9_buffer_postdisable(struct iio_dev *indio_dev)
@@ -476,13 +469,18 @@ void kxsd9_common_remove(struct device *dev)
 {
 	struct iio_dev *indio_dev = dev_get_drvdata(dev);
 	struct kxsd9_state *st = iio_priv(indio_dev);
+	int ret;
 
 	iio_triggered_buffer_cleanup(indio_dev);
 	iio_device_unregister(indio_dev);
-	pm_runtime_get_sync(dev);
-	pm_runtime_put_noidle(dev);
+
+	ret = pm_runtime_resume_and_get(dev);
 	pm_runtime_disable(dev);
-	kxsd9_power_down(st);
+
+	if (ret >= 0) {
+		kxsd9_power_down(st);
+		pm_runtime_put_noidle(dev);
+	}
 }
 EXPORT_SYMBOL_NS(kxsd9_common_remove, "IIO_KXSD9");
 
-- 
2.50.1 (Apple Git-155)


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

* Re: [PATCH] iio: accel: kxsd9: Fix runtime PM error handling and scoping
  2026-06-15 20:03 [PATCH] iio: accel: kxsd9: Fix runtime PM error handling and scoping Biren Pandya
@ 2026-06-16  8:44 ` Andy Shevchenko
  2026-06-21 17:41 ` Jonathan Cameron
  1 sibling, 0 replies; 3+ messages in thread
From: Andy Shevchenko @ 2026-06-16  8:44 UTC (permalink / raw)
  To: Biren Pandya
  Cc: Jonathan Cameron, linux-iio, David Lechner, Nuno Sá,
	Andy Shevchenko, Sakari Ailus, linux-kernel

On Tue, Jun 16, 2026 at 01:33:30AM +0530, Biren Pandya wrote:
> The kxsd9 driver uses pm_runtime_get_sync() in multiple places without
> checking its return value, which can lead to silent failures. It also
> relies on manual pm_runtime_put_autosuspend() calls in various return
> paths, which is prone to memory leaks if a return path is missed.
> 
> Fix the runtime PM handling by using the new scoped guards and proper
> error checking:
> - In kxsd9_write_raw() and kxsd9_read_raw(), use the scoped
>   PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND() macro to safely automate
>   cleanup on function return, while maintaining the behavior of
>   waking up a suspended device via pm_runtime_resume_and_get().
>   This allows us to cleanly remove all manual puts and error gotos.
> - In kxsd9_buffer_preenable(), replace pm_runtime_get_sync() with
>   pm_runtime_resume_and_get() to safely check for errors and handle
>   usage-counter unwinding on failure.
> - In kxsd9_common_remove(), replace pm_runtime_get_sync() with
>   pm_runtime_resume_and_get() and only invoke kxsd9_power_down()
>   if the resume succeeds. This prevents unbalanced regulator disable
>   warnings and bus timeouts if the device is already suspended.

...

> void kxsd9_common_remove(struct device *dev)
>  {
>  	struct iio_dev *indio_dev = dev_get_drvdata(dev);
>  	struct kxsd9_state *st = iio_priv(indio_dev);
> +	int ret;
>  
>  	iio_triggered_buffer_cleanup(indio_dev);
>  	iio_device_unregister(indio_dev);
> -	pm_runtime_get_sync(dev);
> -	pm_runtime_put_noidle(dev);
> +
> +	ret = pm_runtime_resume_and_get(dev);
>  	pm_runtime_disable(dev);
> -	kxsd9_power_down(st);

> +	if (ret >= 0) {

Use standard pattern.

> +		kxsd9_power_down(st);
> +		pm_runtime_put_noidle(dev);
> +	}
>  }

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH] iio: accel: kxsd9: Fix runtime PM error handling and scoping
  2026-06-15 20:03 [PATCH] iio: accel: kxsd9: Fix runtime PM error handling and scoping Biren Pandya
  2026-06-16  8:44 ` Andy Shevchenko
@ 2026-06-21 17:41 ` Jonathan Cameron
  1 sibling, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2026-06-21 17:41 UTC (permalink / raw)
  To: Biren Pandya
  Cc: linux-iio, David Lechner, Nuno Sá, Andy Shevchenko,
	Sakari Ailus, linux-kernel

On Tue, 16 Jun 2026 01:33:30 +0530
Biren Pandya <birenpandya@gmail.com> wrote:

> The kxsd9 driver uses pm_runtime_get_sync() in multiple places without
> checking its return value, which can lead to silent failures. It also
> relies on manual pm_runtime_put_autosuspend() calls in various return
> paths, which is prone to memory leaks if a return path is missed.
> 
> Fix the runtime PM handling by using the new scoped guards and proper
> error checking:
> - In kxsd9_write_raw() and kxsd9_read_raw(), use the scoped
>   PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND() macro to safely automate
>   cleanup on function return, while maintaining the behavior of
>   waking up a suspended device via pm_runtime_resume_and_get().
>   This allows us to cleanly remove all manual puts and error gotos.

Somewhere here mention that one of the puts was missing.

> - In kxsd9_buffer_preenable(), replace pm_runtime_get_sync() with
>   pm_runtime_resume_and_get() to safely check for errors and handle
>   usage-counter unwinding on failure.
> - In kxsd9_common_remove(), replace pm_runtime_get_sync() with
>   pm_runtime_resume_and_get() and only invoke kxsd9_power_down()
>   if the resume succeeds. This prevents unbalanced regulator disable
>   warnings and bus timeouts if the device is already suspended.
> 
> Signed-off-by: Biren Pandya <birenpandya@gmail.com>

Given this fixes a path, needs a Fixes tag.

Otherwise, what Andy said.

> ---
>  drivers/iio/accel/kxsd9.c | 50 +++++++++++++++++++--------------------
>  1 file changed, 24 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxsd9.c b/drivers/iio/accel/kxsd9.c
> index 4717d80fc24a..f23d1660e6da 100644
> --- a/drivers/iio/accel/kxsd9.c
> +++ b/drivers/iio/accel/kxsd9.c
> @@ -139,68 +139,63 @@ static int kxsd9_write_raw(struct iio_dev *indio_dev,
>  			   int val2,
>  			   long mask)
>  {
> -	int ret = -EINVAL;
>  	struct kxsd9_state *st = iio_priv(indio_dev);
>  
> -	pm_runtime_get_sync(st->dev);
> +	PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(st->dev, pm);
> +	if (PM_RUNTIME_ACQUIRE_ERR(&pm))
> +		return PM_RUNTIME_ACQUIRE_ERR(&pm);
>  
>  	if (mask == IIO_CHAN_INFO_SCALE) {
>  		/* Check no integer component */
>  		if (val)
>  			return -EINVAL;

This one is a real fix that needed addressing as it exits
with an error code leaving the power on.

> -		ret = kxsd9_write_scale(indio_dev, val2);
> +		return kxsd9_write_scale(indio_dev, val2);
>  	}
>  
> -	pm_runtime_put_autosuspend(st->dev);
> -
> -	return ret;
> +	return -EINVAL;
>  }



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

end of thread, other threads:[~2026-06-21 17:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-15 20:03 [PATCH] iio: accel: kxsd9: Fix runtime PM error handling and scoping Biren Pandya
2026-06-16  8:44 ` Andy Shevchenko
2026-06-21 17:41 ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).