[PATCH v3 0/2] iio: fxas210002c: Discard pm_runtime_put_autosuspend() return and related cleanup.

[PATCH v3 0/2] iio: fxas210002c: Discard pm_runtime_put_autosuspend() return and related cleanup.

[Jonathan Cameron]

From: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

This is broken out from
https://lore.kernel.org/linux-iio/20261021235818.3059519-1-briannorris@chromium.org/
[PATCH v2 00/17] iio: Stop looking at pm_runtime_put_autosuspend() return codes

Understandably Brian has expressed that his series was focused on
just one thing - dropping the return values. Given this highlighted
some other related code improvements, I am going to have little series
expanding on individual patches from Brian's series - tweaking Brian's
patches where I think it makes sense and adding additional patches
for related cleanup.

Note, if I run out of time and we are near the end of the cycle I
may just pick up Brian's more minimal set!

Brian Norris (1):
  iio: gyro: fxas21002c: Discard pm_runtime_put_autosuspend() return
    values

Jonathan Cameron (1):
  iio: gyro: fxs210002c: Use guard(mutex)() for remaining simple cases.

 drivers/iio/gyro/fxas21002c_core.c | 137 +++++++++--------------------
 1 file changed, 44 insertions(+), 93 deletions(-)

-- 
2.56.0

[PATCH v3 1/2] iio: gyro: fxas21002c: Discard pm_runtime_put_autosuspend() return values

[Jonathan Cameron]

From: Brian Norris <briannorris@chromium.org>

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

With the driver modified to discard the pm_runtime_put_autosuspend()
return value, it becomes simple to use PM_RUNTIME_ACQUIRE() and
guard(mutex)() to simpify two locations. At the remaining location
simply dicusard the 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@chromium.org>
Signed-off-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

---

Brian, given this tweaked your patch a fair bit, please let me know
if you are fine with this. Hopefully I didn't mess anything up!
---
 drivers/iio/gyro/fxas21002c_core.c | 66 +++++++++---------------------
 1 file changed, 20 insertions(+), 46 deletions(-)

diff --git a/drivers/iio/gyro/fxas21002c_core.c b/drivers/iio/gyro/fxas21002c_core.c
index a88670207cec..fe1ac3e841dc 100644
--- a/drivers/iio/gyro/fxas21002c_core.c
+++ b/drivers/iio/gyro/fxas21002c_core.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2019 Linaro Ltd.
  */
 
+#include <linux/cleanup.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
 #include <linux/pm.h>
@@ -364,48 +365,28 @@ static int fxas21002c_write(struct fxas21002c_data *data,
 	return ret;
 }
 
-static int  fxas21002c_pm_get(struct fxas21002c_data *data)
-{
-	return pm_runtime_resume_and_get(regmap_get_device(data->regmap));
-}
-
-static int  fxas21002c_pm_put(struct fxas21002c_data *data)
-{
-	struct device *dev = regmap_get_device(data->regmap);
-
-	return pm_runtime_put_autosuspend(dev);
-}
-
 static int fxas21002c_temp_get(struct fxas21002c_data *data, int *val)
 {
 	struct device *dev = regmap_get_device(data->regmap);
 	unsigned int temp;
 	int ret;
 
-	mutex_lock(&data->lock);
-	ret = fxas21002c_pm_get(data);
-	if (ret < 0)
-		goto data_unlock;
+	guard(mutex)(&data->lock);
+
+	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)
+		return ret;
 
 	ret = regmap_field_read(data->regmap_fields[F_TEMP], &temp);
 	if (ret < 0) {
 		dev_err(dev, "failed to read temp: %d\n", ret);
-		fxas21002c_pm_put(data);
-		goto data_unlock;
+		return ret;
 	}
 
 	*val = sign_extend32(temp, 7);
 
-	ret = fxas21002c_pm_put(data);
-	if (ret < 0)
-		goto data_unlock;
-
-	ret = IIO_VAL_INT;
-
-data_unlock:
-	mutex_unlock(&data->lock);
-
-	return ret;
+	return IIO_VAL_INT;
 }
 
 static int fxas21002c_axis_get(struct fxas21002c_data *data,
@@ -415,31 +396,23 @@ static int fxas21002c_axis_get(struct fxas21002c_data *data,
 	__be16 axis_be;
 	int ret;
 
-	mutex_lock(&data->lock);
-	ret = fxas21002c_pm_get(data);
-	if (ret < 0)
-		goto data_unlock;
+	guard(mutex)(&data->lock);
+
+	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
+	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
+	if (ret)
+		return ret;
 
 	ret = regmap_bulk_read(data->regmap, FXAS21002C_AXIS_TO_REG(index),
 			       &axis_be, sizeof(axis_be));
 	if (ret < 0) {
 		dev_err(dev, "failed to read axis: %d: %d\n", index, ret);
-		fxas21002c_pm_put(data);
-		goto data_unlock;
+		return ret;
 	}
 
 	*val = sign_extend32(be16_to_cpu(axis_be), 15);
 
-	ret = fxas21002c_pm_put(data);
-	if (ret < 0)
-		goto data_unlock;
-
-	ret = IIO_VAL_INT;
-
-data_unlock:
-	mutex_unlock(&data->lock);
-
-	return ret;
+	return IIO_VAL_INT;
 }
 
 static int fxas21002c_odr_get(struct fxas21002c_data *data, int *odr)
@@ -725,10 +698,11 @@ static irqreturn_t fxas21002c_trigger_handler(int irq, void *p)
 	struct iio_poll_func *pf = p;
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct fxas21002c_data *data = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(data->regmap);
 	int ret;
 
 	mutex_lock(&data->lock);
-	ret = fxas21002c_pm_get(data);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0)
 		goto out_unlock;
 
@@ -741,7 +715,7 @@ static irqreturn_t fxas21002c_trigger_handler(int irq, void *p)
 					   data->timestamp);
 
 out_pm_put:
-	fxas21002c_pm_put(data);
+	pm_runtime_put_autosuspend(dev);
 
 out_unlock:
 	mutex_unlock(&data->lock);
-- 
2.56.0

Re: [PATCH v3 1/2] iio: gyro: fxas21002c: Discard pm_runtime_put_autosuspend() return values

[Andy Shevchenko]

On Mon, Oct 26, 2026 at 03:20:50AM +0000, Jonathan Cameron wrote:
> 
> This 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.
> 
> With the driver modified to discard the pm_runtime_put_autosuspend()
> return value, it becomes simple to use PM_RUNTIME_ACQUIRE() and
> guard(mutex)() to simpify two locations. At the remaining location
> simply dicusard the 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").

Yep, that's what I meant by doing one step forward. This version looks
exactly how I would imagine it!

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 1/2] iio: gyro: fxas21002c: Discard pm_runtime_put_autosuspend() return values

[Brian Norris]

On Mon, Oct 26, 2026 at 03:20:50AM +0000, Jonathan Cameron wrote:
> From: Brian Norris <briannorris@chromium.org>
> 
> This 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.
> 
> With the driver modified to discard the pm_runtime_put_autosuspend()
> return value, it becomes simple to use PM_RUNTIME_ACQUIRE() and
> guard(mutex)() to simpify two locations. At the remaining location
> simply dicusard the 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@chromium.org>
> Signed-off-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
> 
> ---
> 
> Brian, given this tweaked your patch a fair bit, please let me know
> if you are fine with this. Hopefully I didn't mess anything up!

Fine with me. Not sure if it's appropriate for "my own" patch, but:

Reviewed-by: Brian Norris <briannorris@chromium.org>

[PATCH v3 2/2] iio: gyro: fxs210002c: Use guard(mutex)() for remaining simple cases.

[Jonathan Cameron]

From: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

With the exception of the more complex flow in the trigger handler,
all uses of the data->lock are switched to using guard()()
simplifying various error flows.

Signed-off-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
---
 drivers/iio/gyro/fxas21002c_core.c | 71 ++++++++++--------------------
 1 file changed, 24 insertions(+), 47 deletions(-)

diff --git a/drivers/iio/gyro/fxas21002c_core.c b/drivers/iio/gyro/fxas21002c_core.c
index fe1ac3e841dc..b6e6db4ad94c 100644
--- a/drivers/iio/gyro/fxas21002c_core.c
+++ b/drivers/iio/gyro/fxas21002c_core.c
@@ -341,28 +341,21 @@ static int fxas21002c_write(struct fxas21002c_data *data,
 	int actual_mode;
 	int ret;
 
-	mutex_lock(&data->lock);
+	guard(mutex)(&data->lock);
 
 	actual_mode = fxas21002c_mode_get(data);
-	if (actual_mode < 0) {
-		ret = actual_mode;
-		goto out_unlock;
-	}
+	if (actual_mode < 0)
+		return actual_mode;
 
 	ret = fxas21002c_mode_set(data, FXAS21002C_MODE_READY);
 	if (ret < 0)
-		goto out_unlock;
+		return ret;
 
 	ret = regmap_field_write(data->regmap_fields[field], bits);
 	if (ret < 0)
-		goto out_unlock;
-
-	ret = fxas21002c_mode_set(data, data->prev_mode);
-
-out_unlock:
-	mutex_unlock(&data->lock);
+		return ret;
 
-	return ret;
+	return fxas21002c_mode_set(data, data->prev_mode);
 }
 
 static int fxas21002c_temp_get(struct fxas21002c_data *data, int *val)
@@ -420,19 +413,15 @@ static int fxas21002c_odr_get(struct fxas21002c_data *data, int *odr)
 	unsigned int odr_bits;
 	int ret;
 
-	mutex_lock(&data->lock);
+	guard(mutex)(&data->lock);
+
 	ret = regmap_field_read(data->regmap_fields[F_DR], &odr_bits);
 	if (ret < 0)
-		goto data_unlock;
+		return ret;
 
 	*odr = fxas21002c_odr_hz_from_value(data, odr_bits);
 
-	ret = IIO_VAL_INT;
-
-data_unlock:
-	mutex_unlock(&data->lock);
-
-	return ret;
+	return IIO_VAL_INT;
 }
 
 static int fxas21002c_odr_set(struct fxas21002c_data *data, int odr)
@@ -451,19 +440,15 @@ static int fxas21002c_lpf_get(struct fxas21002c_data *data, int *val2)
 	unsigned int bw_bits;
 	int ret;
 
-	mutex_lock(&data->lock);
+	guard(mutex)(&data->lock);
+
 	ret = regmap_field_read(data->regmap_fields[F_BW], &bw_bits);
 	if (ret < 0)
-		goto data_unlock;
+		return ret;
 
 	*val2 = fxas21002c_lpf_bw_from_value(data, bw_bits) * 10000;
 
-	ret = IIO_VAL_INT_PLUS_MICRO;
-
-data_unlock:
-	mutex_unlock(&data->lock);
-
-	return ret;
+	return IIO_VAL_INT_PLUS_MICRO;
 }
 
 static int fxas21002c_lpf_set(struct fxas21002c_data *data, int bw)
@@ -495,19 +480,15 @@ static int fxas21002c_hpf_get(struct fxas21002c_data *data, int *val2)
 	unsigned int sel_bits;
 	int ret;
 
-	mutex_lock(&data->lock);
+	guard(mutex)(&data->lock);
+
 	ret = regmap_field_read(data->regmap_fields[F_SEL], &sel_bits);
 	if (ret < 0)
-		goto data_unlock;
+		return ret;
 
 	*val2 = fxas21002c_hpf_sel_from_value(data, sel_bits);
 
-	ret = IIO_VAL_INT_PLUS_MICRO;
-
-data_unlock:
-	mutex_unlock(&data->lock);
-
-	return ret;
+	return IIO_VAL_INT_PLUS_MICRO;
 }
 
 static int fxas21002c_hpf_set(struct fxas21002c_data *data, int sel)
@@ -527,23 +508,19 @@ static int fxas21002c_scale_get(struct fxas21002c_data *data, int *val)
 	int scale;
 	int ret;
 
-	mutex_lock(&data->lock);
+	guard(mutex)(&data->lock);
+
 	ret = regmap_field_read(data->regmap_fields[F_FS], &fs_bits);
 	if (ret < 0)
-		goto data_unlock;
+		return ret;
 
 	scale = fxas21002c_range_fs_from_value(data, fs_bits);
-	if (scale < 0) {
-		ret = scale;
-		goto data_unlock;
-	}
+	if (scale < 0)
+		return scale;
 
 	*val = scale;
 
-data_unlock:
-	mutex_unlock(&data->lock);
-
-	return ret;
+	return 0;
 }
 
 static int fxas21002c_scale_set(struct fxas21002c_data *data, int range)
-- 
2.56.0

Re: [PATCH v3 2/2] iio: gyro: fxs210002c: Use guard(mutex)() for remaining simple cases.

[Andy Shevchenko]

On Mon, Oct 26, 2026 at 03:20:51AM +0000, Jonathan Cameron wrote:

> With the exception of the more complex flow in the trigger handler,
> all uses of the data->lock are switched to using guard()()
> simplifying various error flows.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/2] iio: gyro: fxs210002c: Use guard(mutex)() for remaining simple cases.

[Brian Norris]

On Mon, Oct 26, 2026 at 03:20:51AM +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
> 
> With the exception of the more complex flow in the trigger handler,
> all uses of the data->lock are switched to using guard()()
> simplifying various error flows.
> 
> Signed-off-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
> ---
>  drivers/iio/gyro/fxas21002c_core.c | 71 ++++++++++--------------------
>  1 file changed, 24 insertions(+), 47 deletions(-)

super-nit: it seems like the name of this driver is "fxas21002c", but
your title is "fxs210002c". Understandable that one might miss 1 out of
10 gibberish part numbers ;)

Otherwise:

Reviewed-by: Brian Norris <briannorris@chromium.org>