[PATCH v2 00/17] iio: Stop looking at pm_runtime_put_autosuspend() return codes

[PATCH v2 00/17] iio: Stop looking at pm_runtime_put_autosuspend() return codes

[Brian Norris]

Similar to this effort:

  Subject: [PATCH v1 00/23] PM: runtime: Convert pm_runtime_put() to a void function
  https://lore.kernel.org/all/6245770.lOV4Wx5bFT@rafael.j.wysocki/

i.e., commit 3afd8df02433 ("PM: runtime: Change pm_runtime_put() return
type to void") and friends.

As with pm_runtime_put(), there is very little value in return-code
checking for pm_runtime_put_autosuspend().

Checking the return code is more likely to produce a race condition
(fighting with user space writing "on" to the "control" attribute) than
to tell a consumer useful info.

Because the IIO subsystem has the bulk of drivers that inspect
pm_runtime_put_autosuspend(), and because those drivers have a lot of
other issues touching the same code (which Jonathan suggested [1] I
fix -- I fix some, but not all), I'm sending an IIO-specific series
under a v2 label (since many of the changes are still borrowed from the
earlier series).

For reference, the larger v1 series can be found at [2].

- Brian

[1] https://lore.kernel.org/all/20261018211807.66bfea0d@jic23-hlaptop/
[2] https://lore.kernel.org/all/20261006231900.3230373-1-briannorris@chromium.org/

Changes in v2:
 * Split iio into its own series
 * Split into per-driver patches
 * Fix various usage bugs in these iio drivers in the adjacent code
 * Remove now-unnecessary helpers
 * Maintain Reviewed-by from v1 for drivers where there was no
   additional change (aside from splitting up patches)

Brian Norris (17):
  iio: accel: fxls8962af: Discard pm_runtime_put_autosuspend() return
    values
  iio: accel: kxcjk-1013: Discard pm_runtime_put_autosuspend() return
    values
  iio: accel: mma8452: Discard pm_runtime_put_autosuspend() return
    values
  iio: accel: mma955x: Discard pm_runtime_put_autosuspend() return
    values
  iio: accel: bmc150: Discard pm_runtime_put_autosuspend() return values
  iio: adc: rcar-gyroadc: Discard pm_runtime_put_autosuspend() return
    values
  iio: adc: ti-ads1015: Discard pm_runtime_put_autosuspend() return
    values
  iio: chemical: atlas-sensor: Discard pm_runtime_put_autosuspend()
    return values
  iio: hid-sensors: Discard pm_runtime_put_autosuspend() return values
  iio: gyro: bmg160: Discard pm_runtime_put_autosuspend() return values
  iio: gyro: fxas21002c: Discard pm_runtime_put_autosuspend() return
    values
  iio: imu: kmx61: Discard pm_runtime_put_autosuspend() return values
  iio: light: apds9960: Discard pm_runtime_put_autosuspend() return
    values
  iio: light: pa12203001: Discard pm_runtime_put_autosuspend() return
    values
  iio: light: rpr0521: Discard pm_runtime_put_autosuspend() return
    values
  iio: light: tsl2583: Discard pm_runtime_put_autosuspend() return
    values
  iio: light: us5182d: Discard pm_runtime_put_autosuspend() return
    values

 drivers/iio/accel/bmc150-accel-core.c         |  75 +++-----
 drivers/iio/accel/fxls8962af-core.c           |  56 +++---
 drivers/iio/accel/kxcjk-1013.c                | 118 ++++++------
 drivers/iio/accel/mma8452.c                   | 100 ++++++----
 drivers/iio/accel/mma9551.c                   |  21 ++-
 drivers/iio/accel/mma9551_core.c              |  39 +---
 drivers/iio/accel/mma9551_core.h              |   1 -
 drivers/iio/accel/mma9553.c                   |  36 ++--
 drivers/iio/adc/rcar-gyroadc.c                |   4 +-
 drivers/iio/adc/ti-ads1015.c                  |  50 ++---
 drivers/iio/chemical/atlas-sensor.c           |   4 +-
 .../common/hid-sensors/hid-sensor-trigger.c   |  14 +-
 drivers/iio/gyro/bmg160_core.c                | 178 ++++++++----------
 drivers/iio/gyro/fxas21002c_core.c            |  33 +---
 drivers/iio/imu/kmx61.c                       |  25 +--
 drivers/iio/light/apds9960.c                  |  48 ++---
 drivers/iio/light/pa12203001.c                |  26 +--
 drivers/iio/light/rpr0521.c                   |  57 +++---
 drivers/iio/light/tsl2583.c                   |  27 +--
 drivers/iio/light/us5182d.c                   |  32 +---
 20 files changed, 401 insertions(+), 543 deletions(-)

-- 
2.56.0.218.g1ec904b173-goog

[PATCH v2 01/17] iio: accel: fxls8962af: Discard pm_runtime_put_autosuspend() return values

[Brian Norris]

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.

For this reason, modify this driver to simply discard the
pm_runtime_put_autosuspend() return value, and inline the
fxls8962af_power_off/on() helpers to their callsites, as they're even
more trivial now.

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

Changes in v2:
 * Rebase to linux-next properly
 * Drop a few drivers' changes, as they stopped checking
   pm_runtime_put_autosuspend() return code already
 * Split iio patch into per-driver patches
 * Inlining

 drivers/iio/accel/fxls8962af-core.c | 56 +++++++++++++----------------
 1 file changed, 24 insertions(+), 32 deletions(-)

diff --git a/drivers/iio/accel/fxls8962af-core.c b/drivers/iio/accel/fxls8962af-core.c
index 55183c69465d..496e9bf2c4ea 100644
--- a/drivers/iio/accel/fxls8962af-core.c
+++ b/drivers/iio/accel/fxls8962af-core.c
@@ -205,30 +205,6 @@ enum fxls8962af_int_pin {
 	FXLS8962AF_PIN_INT2,
 };
 
-static int fxls8962af_power_on(struct fxls8962af_data *data)
-{
-	struct device *dev = regmap_get_device(data->regmap);
-	int ret;
-
-	ret = pm_runtime_resume_and_get(dev);
-	if (ret)
-		dev_err(dev, "failed to power on\n");
-
-	return ret;
-}
-
-static int fxls8962af_power_off(struct fxls8962af_data *data)
-{
-	struct device *dev = regmap_get_device(data->regmap);
-	int ret;
-
-	ret = pm_runtime_put_autosuspend(dev);
-	if (ret)
-		dev_err(dev, "failed to power off\n");
-
-	return ret;
-}
-
 static int fxls8962af_standby(struct fxls8962af_data *data)
 {
 	return regmap_clear_bits(data->regmap, FXLS8962AF_SENS_CONFIG1,
@@ -263,16 +239,18 @@ static int fxls8962af_get_out(struct fxls8962af_data *data,
 
 	is_active = fxls8962af_is_active(data);
 	if (!is_active) {
-		ret = fxls8962af_power_on(data);
-		if (ret)
+		ret = pm_runtime_resume_and_get(dev);
+		if (ret) {
+			dev_err(dev, "failed to power on: %d\n", ret);
 			return ret;
+		}
 	}
 
 	ret = regmap_bulk_read(data->regmap, chan->address,
 			       &raw_val, sizeof(data->lower_thres));
 
 	if (!is_active)
-		fxls8962af_power_off(data);
+		pm_runtime_put_autosuspend(dev);
 
 	if (ret) {
 		dev_err(dev, "failed to get out reg 0x%lx\n", chan->address);
@@ -628,6 +606,7 @@ fxls8962af_write_event_config(struct iio_dev *indio_dev,
 			      enum iio_event_direction dir, bool state)
 {
 	struct fxls8962af_data *data = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(data->regmap);
 	u8 enable_event, enable_bits;
 	int ret, value;
 
@@ -687,18 +666,22 @@ fxls8962af_write_event_config(struct iio_dev *indio_dev,
 
 	if (data->enable_event) {
 		fxls8962af_active(data);
-		ret = fxls8962af_power_on(data);
+		ret = pm_runtime_resume_and_get(dev);
+		if (ret) {
+			dev_err(dev, "failed to power on: %d\n", ret);
+			return ret;
+		}
 	} else {
 		if (!iio_device_claim_direct(indio_dev))
 			return -EBUSY;
 
 		/* Not in buffered mode so disable power */
-		ret = fxls8962af_power_off(data);
+		pm_runtime_put_autosuspend(dev);
 
 		iio_device_release_direct(indio_dev);
 	}
 
-	return ret;
+	return 0;
 }
 
 static const struct iio_event_spec fxls8962af_event[] = {
@@ -840,7 +823,15 @@ static int __fxls8962af_fifo_set_mode(struct fxls8962af_data *data, bool onoff)
 
 static int fxls8962af_buffer_preenable(struct iio_dev *indio_dev)
 {
-	return fxls8962af_power_on(iio_priv(indio_dev));
+	struct fxls8962af_data *data = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(data->regmap);
+	int ret;
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret)
+		dev_err(dev, "failed to power on: %d\n", ret);
+
+	return ret;
 }
 
 static int fxls8962af_buffer_postenable(struct iio_dev *indio_dev)
@@ -889,9 +880,10 @@ static int fxls8962af_buffer_predisable(struct iio_dev *indio_dev)
 static int fxls8962af_buffer_postdisable(struct iio_dev *indio_dev)
 {
 	struct fxls8962af_data *data = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(data->regmap);
 
 	if (!data->enable_event)
-		fxls8962af_power_off(data);
+		pm_runtime_put_autosuspend(dev);
 
 	return 0;
 }
-- 
2.56.0.218.g1ec904b173-goog

Re: [PATCH v2 01/17] iio: accel: fxls8962af: Discard pm_runtime_put_autosuspend() return values

[Andy Shevchenko]

On Wed, Oct 21, 2026 at 04:57:02PM -0700, Brian Norris 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.
> 
> For this reason, modify this driver to simply discard the
> pm_runtime_put_autosuspend() return value, and inline the
> fxls8962af_power_off/on() helpers to their callsites, as they're even
> more trivial now.
> 
> 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").

...

> static int fxls8962af_get_out(struct fxls8962af_data *data,

>  
>  	is_active = fxls8962af_is_active(data);

>  	if (!is_active) {

Why do we have this check at all?

> -		ret = fxls8962af_power_on(data);
> -		if (ret)
> +		ret = pm_runtime_resume_and_get(dev);
> +		if (ret) {
> +			dev_err(dev, "failed to power on: %d\n", ret);
>  			return ret;
> +		}
>  	}
>  
>  	ret = regmap_bulk_read(data->regmap, chan->address,
>  			       &raw_val, sizeof(data->lower_thres));
>  
>  	if (!is_active)
> -		fxls8962af_power_off(data);
> +		pm_runtime_put_autosuspend(dev);

If it's active my understanding that runtime PM keeps device already resumed,
so these two will be just a reference counting calls, no?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 01/17] iio: accel: fxls8962af: Discard pm_runtime_put_autosuspend() return values

[Brian Norris]

On Thu, Oct 22, 2026 at 09:53:12AM +0300, Andy Shevchenko wrote:
> On Wed, Oct 21, 2026 at 04:57:02PM -0700, Brian Norris 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.
> > 
> > For this reason, modify this driver to simply discard the
> > pm_runtime_put_autosuspend() return value, and inline the
> > fxls8962af_power_off/on() helpers to their callsites, as they're even
> > more trivial now.
> > 
> > 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").
> 
> ...
> 
> > static int fxls8962af_get_out(struct fxls8962af_data *data,
> 
> >  
> >  	is_active = fxls8962af_is_active(data);
> 
> >  	if (!is_active) {
> 
> Why do we have this check at all?

Frankly, I don't know. It seems logically wrong; you shouldn't even be
reading device registers without first grabbing a runtime PM reference.

But I'm also loathe to go tweak a bunch of driver-specific details I'm
completely unfamiliar with, when my main purpose is extremely simple --
to stop checking one return code.

All in all, I think this is beside the point, and I'd rather not try to
address it in this series. For one, even if I try to address it, there's
more chance of regressions that will distract from my purpose.

> > -		ret = fxls8962af_power_on(data);
> > -		if (ret)
> > +		ret = pm_runtime_resume_and_get(dev);
> > +		if (ret) {
> > +			dev_err(dev, "failed to power on: %d\n", ret);
> >  			return ret;
> > +		}
> >  	}
> >  
> >  	ret = regmap_bulk_read(data->regmap, chan->address,
> >  			       &raw_val, sizeof(data->lower_thres));
> >  
> >  	if (!is_active)
> > -		fxls8962af_power_off(data);
> > +		pm_runtime_put_autosuspend(dev);
> 
> If it's active my understanding that runtime PM keeps device already resumed,
> so these two will be just a reference counting calls, no?

Right. I don't really see the point.

Brian

[PATCH v2 02/17] iio: accel: kxcjk-1013: Discard pm_runtime_put_autosuspend() return values

[Brian Norris]

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.

For this reason, modify this driver to discard the
pm_runtime_put_autosuspend() return value, and inline these pm_runtime
calls for simplicity.

While doing this, it becomes obvious that kxcjk1013_write_event_config()
and kxcjk1013_data_rdy_trigger_set_state() are intentionally mismanaging
runtime PM reference counts and instead are relying on (racy)
autosuspend delay for correct ordering. Undo this choice, and instead
properly drop the reference count only after we're done touching the
hardware.

Use guard() for managing the mutex in kxcjk1013_write_event_config(),
because it helps simplify the above refactoring, making it less
error-prone.

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

Changes in v2:
 * Split iio patch into per-driver patches
 * Inlining
 * Refcount ordering fixes

 drivers/iio/accel/kxcjk-1013.c | 118 ++++++++++++++++-----------------
 1 file changed, 57 insertions(+), 61 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index b3d753bd9476..3b1a8e67b266 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -13,6 +13,7 @@
 #include <linux/string.h>
 #include <linux/types.h>
 #include <linux/acpi.h>
+#include <linux/cleanup.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
@@ -628,25 +629,6 @@ static int kxcjk1013_get_startup_times(struct kxcjk1013_data *data)
 	return KXCJK1013_MAX_STARTUP_TIME_US;
 }
 
-static int kxcjk1013_set_power_state(struct kxcjk1013_data *data, bool on)
-{
-#ifdef CONFIG_PM
-	int ret;
-
-	if (on)
-		ret = pm_runtime_resume_and_get(&data->client->dev);
-	else
-		ret = pm_runtime_put_autosuspend(&data->client->dev);
-	if (ret < 0) {
-		dev_err(&data->client->dev,
-			"Failed: %s for %d\n", __func__, on);
-		return ret;
-	}
-#endif
-
-	return 0;
-}
-
 static int kxcjk1013_chip_update_thresholds(struct kxcjk1013_data *data)
 {
 	const struct kx_chipset_regs *regs = data->info->regs;
@@ -895,20 +877,22 @@ static int kxcjk1013_read_raw(struct iio_dev *indio_dev,
 		if (iio_buffer_enabled(indio_dev))
 			ret = -EBUSY;
 		else {
-			ret = kxcjk1013_set_power_state(data, true);
+			ret = pm_runtime_resume_and_get(&data->client->dev);
 			if (ret < 0) {
+				dev_err(&data->client->dev,
+					"failed to resume: %d\n", ret);
 				mutex_unlock(&data->mutex);
 				return ret;
 			}
 			ret = kxcjk1013_get_acc_reg(data, chan->scan_index);
 			if (ret < 0) {
-				kxcjk1013_set_power_state(data, false);
+				pm_runtime_put_autosuspend(&data->client->dev);
 				mutex_unlock(&data->mutex);
 				return ret;
 			}
 			*val = sign_extend32(ret >> chan->scan_type.shift,
 					     chan->scan_type.realbits - 1);
-			ret = kxcjk1013_set_power_state(data, false);
+			pm_runtime_put_autosuspend(&data->client->dev);
 		}
 		mutex_unlock(&data->mutex);
 
@@ -1033,39 +1017,35 @@ static int kxcjk1013_write_event_config(struct iio_dev *indio_dev,
 	if (state == data->ev_enable_state)
 		return 0;
 
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);
 
 	if (!state && data->motion_trigger_on) {
 		data->ev_enable_state = 0;
-		mutex_unlock(&data->mutex);
 		return 0;
 	}
 
-	/*
-	 * We will expect the enable and disable to do operation in
-	 * reverse order. This will happen here anyway as our
-	 * resume operation uses sync mode runtime pm calls, the
-	 * suspend operation will be delayed by autosuspend delay
-	 * So the disable operation will still happen in reverse of
-	 * enable operation. When runtime pm is disabled the mode
-	 * is always on so sequence doesn't matter
-	 */
-	ret = kxcjk1013_set_power_state(data, state);
-	if (ret < 0) {
-		mutex_unlock(&data->mutex);
-		return ret;
-	}
+	if (state) {
+		ret = pm_runtime_resume_and_get(&data->client->dev);
+		if (ret < 0) {
+			dev_err(&data->client->dev, "failed to resume: %d\n",
+				ret);
+			return ret;
+		}
 
-	ret =  kxcjk1013_setup_interrupt(data, state, false);
-	if (ret < 0) {
-		kxcjk1013_set_power_state(data, false);
-		data->ev_enable_state = 0;
-		mutex_unlock(&data->mutex);
-		return ret;
+		ret = kxcjk1013_setup_interrupt(data, true, false);
+		if (ret < 0) {
+			pm_runtime_put_autosuspend(&data->client->dev);
+			return ret;
+		}
+	} else {
+		ret = kxcjk1013_setup_interrupt(data, false, false);
+		if (ret < 0)
+			return ret;
+
+		pm_runtime_put_autosuspend(&data->client->dev);
 	}
 
 	data->ev_enable_state = state;
-	mutex_unlock(&data->mutex);
 
 	return 0;
 }
@@ -1073,15 +1053,22 @@ static int kxcjk1013_write_event_config(struct iio_dev *indio_dev,
 static int kxcjk1013_buffer_preenable(struct iio_dev *indio_dev)
 {
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
+	int ret;
 
-	return kxcjk1013_set_power_state(data, true);
+	ret = pm_runtime_resume_and_get(&data->client->dev);
+	if (ret < 0)
+		dev_err(&data->client->dev, "failed to resume: %d\n", ret);
+
+	return ret;
 }
 
 static int kxcjk1013_buffer_postdisable(struct iio_dev *indio_dev)
 {
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
 
-	return kxcjk1013_set_power_state(data, false);
+	pm_runtime_put_autosuspend(&data->client->dev);
+
+	return 0;
 }
 
 static ssize_t kxcjk1013_get_samp_freq_avail(struct device *dev,
@@ -1224,32 +1211,41 @@ static int kxcjk1013_data_rdy_trigger_set_state(struct iio_trigger *trig,
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
 	int ret;
 
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);
 
 	if (!state && data->ev_enable_state && data->motion_trigger_on) {
 		data->motion_trigger_on = false;
-		mutex_unlock(&data->mutex);
 		return 0;
 	}
 
-	ret = kxcjk1013_set_power_state(data, state);
-	if (ret < 0) {
-		mutex_unlock(&data->mutex);
-		return ret;
-	}
-	ret = kxcjk1013_setup_interrupt(data, state, data->motion_trig != trig);
-	if (ret < 0) {
-		kxcjk1013_set_power_state(data, false);
-		mutex_unlock(&data->mutex);
-		return ret;
+	if (state) {
+		ret = pm_runtime_resume_and_get(&data->client->dev);
+		if (ret < 0) {
+			dev_err(&data->client->dev, "failed to resume: %d\n",
+				ret);
+			return ret;
+		}
+
+		ret = kxcjk1013_setup_interrupt(data, true,
+						data->motion_trig != trig);
+		if (ret < 0) {
+			pm_runtime_put_autosuspend(&data->client->dev);
+			return ret;
+		}
+	} else {
+		ret = kxcjk1013_setup_interrupt(data, false,
+						data->motion_trig != trig);
+		if (ret < 0)
+			return ret;
+
+		pm_runtime_put_autosuspend(&data->client->dev);
 	}
+
 	if (data->motion_trig == trig)
 		data->motion_trigger_on = state;
 	else
 		data->dready_trigger_on = state;
 
-	mutex_unlock(&data->mutex);
-
 	return 0;
 }
 
-- 
2.56.0.218.g1ec904b173-goog

Re: [PATCH v2 02/17] iio: accel: kxcjk-1013: Discard pm_runtime_put_autosuspend() return values

[Andy Shevchenko]

On Wed, Oct 21, 2026 at 04:57:03PM -0700, Brian Norris 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.
> 
> For this reason, modify this driver to discard the
> pm_runtime_put_autosuspend() return value, and inline these pm_runtime
> calls for simplicity.
> 
> While doing this, it becomes obvious that kxcjk1013_write_event_config()
> and kxcjk1013_data_rdy_trigger_set_state() are intentionally mismanaging
> runtime PM reference counts and instead are relying on (racy)
> autosuspend delay for correct ordering. Undo this choice, and instead
> properly drop the reference count only after we're done touching the
> hardware.
> 
> Use guard() for managing the mutex in kxcjk1013_write_event_config(),
> because it helps simplify the above refactoring, making it less
> error-prone.
> 
> 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").

...

> static int kxcjk1013_read_raw(struct iio_dev *indio_dev,

>  		else {
> -			ret = kxcjk1013_set_power_state(data, true);
> +			ret = pm_runtime_resume_and_get(&data->client->dev);
>  			if (ret < 0) {
> +				dev_err(&data->client->dev,
> +					"failed to resume: %d\n", ret);
>  				mutex_unlock(&data->mutex);
>  				return ret;
>  			}
>  			ret = kxcjk1013_get_acc_reg(data, chan->scan_index);
>  			if (ret < 0) {
> -				kxcjk1013_set_power_state(data, false);
> +				pm_runtime_put_autosuspend(&data->client->dev);
>  				mutex_unlock(&data->mutex);
>  				return ret;
>  			}
>  			*val = sign_extend32(ret >> chan->scan_type.shift,
>  					     chan->scan_type.realbits - 1);
> -			ret = kxcjk1013_set_power_state(data, false);
> +			pm_runtime_put_autosuspend(&data->client->dev);

So, now you can use on of PM_RUNTIME_ACQUIRE*() and drop unneeded calls?
And why not using guard()() or scoped_guard() here for mutex lock?

>  		}

...

> +	if (state) {
> +		ret = pm_runtime_resume_and_get(&data->client->dev);
> +		if (ret < 0) {
> +			dev_err(&data->client->dev, "failed to resume: %d\n",
> +				ret);
> +			return ret;
> +		}
> +
> +		ret = kxcjk1013_setup_interrupt(data, true,
> +						data->motion_trig != trig);
> +		if (ret < 0) {
> +			pm_runtime_put_autosuspend(&data->client->dev);
> +			return ret;
> +		}
> +	} else {
> +		ret = kxcjk1013_setup_interrupt(data, false,
> +						data->motion_trig != trig);
> +		if (ret < 0)
> +			return ret;
> +
> +		pm_runtime_put_autosuspend(&data->client->dev);
>  	}

Sounds like a dup code, can't we have PM runtime calls along with
kxcjk1013_setup_interrupt() be provided as two helpers and be called
as necessary?

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 03/17] iio: accel: mma8452: Discard pm_runtime_put_autosuspend() return values

[Brian Norris]

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.

For this reason, modify this driver to simply discard the
pm_runtime_put_autosuspend() return value, and inline the pm_runtime
helper, since the behavior is the same with and without CONFIG_PM.

While I'm at it: I see that mma8452_data_rdy_trigger_set_state() and
mma8452_write_event_config() mismanage the runtime PM reference count,
and instead rely on (racy) autosuspend delay for correct ordering.
Undo this choice, and instead properly drop the reference count only
after we're done touching the hardware.

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

Changes in v2:
 * Rebase to linux-next properly
 * Drop a few drivers' changes, as they stopped checking
   pm_runtime_put_autosuspend() return code already
 * Split iio patch into per-driver patches
 * Inlining
 * Autosuspend race fixes

 drivers/iio/accel/mma8452.c | 100 ++++++++++++++++++++++--------------
 1 file changed, 61 insertions(+), 39 deletions(-)

diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c
index fe62a903f0e2..7466c40f6239 100644
--- a/drivers/iio/accel/mma8452.c
+++ b/drivers/iio/accel/mma8452.c
@@ -220,26 +220,6 @@ static int mma8452_drdy(struct mma8452_data *data)
 	return -EIO;
 }
 
-static int mma8452_set_runtime_pm_state(struct i2c_client *client, bool on)
-{
-#ifdef CONFIG_PM
-	struct device *dev = &client->dev;
-	int ret;
-
-	if (on)
-		ret = pm_runtime_resume_and_get(dev);
-	else
-		ret = pm_runtime_put_autosuspend(dev);
-	if (ret < 0) {
-		dev_err(dev, "failed to change power state to %d\n", on);
-
-		return ret;
-	}
-#endif
-
-	return 0;
-}
-
 static int mma8452_read(struct mma8452_data *data, __be16 buf[3])
 {
 	int ret = mma8452_drdy(data);
@@ -247,18 +227,20 @@ static int mma8452_read(struct mma8452_data *data, __be16 buf[3])
 	if (ret < 0)
 		return ret;
 
-	ret = mma8452_set_runtime_pm_state(data->client, true);
-	if (ret)
+	ret = pm_runtime_resume_and_get(&data->client->dev);
+	if (ret < 0) {
+		dev_err(&data->client->dev, "failed to resume: %d\n", ret);
 		return ret;
+	}
 
 	ret = i2c_smbus_read_i2c_block_data(data->client, MMA8452_OUT_X,
 					    3 * sizeof(__be16), (u8 *)buf);
 	if (ret < 0)
 		return ret;
 
-	ret = mma8452_set_runtime_pm_state(data->client, false);
+	pm_runtime_put_autosuspend(&data->client->dev);
 
-	return ret;
+	return 0;
 }
 
 static ssize_t mma8452_show_int_plus_micros(char *buf, const int (*vals)[2],
@@ -974,17 +956,25 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
 	if (ret)
 		return ret;
 
-	ret = mma8452_set_runtime_pm_state(data->client, state);
-	if (ret)
-		return ret;
+	if (state) {
+		ret = pm_runtime_resume_and_get(&data->client->dev);
+		if (ret < 0) {
+			dev_err(&data->client->dev, "failed to resume: %d\n",
+				ret);
+			return ret;
+		}
+	}
 
 	switch (dir) {
 	case IIO_EV_DIR_FALLING:
-		return mma8452_set_freefall_mode(data, state);
+		ret = mma8452_set_freefall_mode(data, state);
+		break;
 	case IIO_EV_DIR_RISING:
 		val = i2c_smbus_read_byte_data(data->client, ev_regs->ev_cfg);
-		if (val < 0)
-			return val;
+		if (val < 0) {
+			ret = val;
+			break;
+		}
 
 		if (state) {
 			if (mma8452_freefall_mode_enabled(data)) {
@@ -997,7 +987,7 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
 					ev_regs->ev_cfg_chan_shift);
 		} else {
 			if (mma8452_freefall_mode_enabled(data))
-				return 0;
+				break;
 
 			val &= ~BIT(chan->scan_index +
 					ev_regs->ev_cfg_chan_shift);
@@ -1005,10 +995,23 @@ static int mma8452_write_event_config(struct iio_dev *indio_dev,
 
 		val |= ev_regs->ev_cfg_ele;
 
-		return mma8452_change_config(data, ev_regs->ev_cfg, val);
+		ret = mma8452_change_config(data, ev_regs->ev_cfg, val);
+		break;
 	default:
-		return -EINVAL;
+		ret = -EINVAL;
+		break;
+	}
+
+	if (ret) {
+		if (state)
+			pm_runtime_put_autosuspend(&data->client->dev);
+		return ret;
 	}
+
+	if (!state)
+		pm_runtime_put_autosuspend(&data->client->dev);
+
+	return 0;
 }
 
 static void mma8452_transient_interrupt(struct iio_dev *indio_dev)
@@ -1447,20 +1450,39 @@ static int mma8452_data_rdy_trigger_set_state(struct iio_trigger *trig,
 	struct mma8452_data *data = iio_priv(indio_dev);
 	int reg, ret;
 
-	ret = mma8452_set_runtime_pm_state(data->client, state);
-	if (ret)
-		return ret;
+	if (state) {
+		ret = pm_runtime_resume_and_get(&data->client->dev);
+		if (ret < 0) {
+			dev_err(&data->client->dev, "failed to resume: %d\n",
+				ret);
+			return ret;
+		}
+	}
 
 	reg = i2c_smbus_read_byte_data(data->client, MMA8452_CTRL_REG4);
-	if (reg < 0)
-		return reg;
+	if (reg < 0) {
+		ret = reg;
+		goto err_put_pm;
+	}
 
 	if (state)
 		reg |= MMA8452_INT_DRDY;
 	else
 		reg &= ~MMA8452_INT_DRDY;
 
-	return mma8452_change_config(data, MMA8452_CTRL_REG4, reg);
+	ret = mma8452_change_config(data, MMA8452_CTRL_REG4, reg);
+	if (ret)
+		goto err_put_pm;
+
+	if (!state)
+		pm_runtime_put_autosuspend(&data->client->dev);
+
+	return 0;
+
+err_put_pm:
+	if (state)
+		pm_runtime_put_autosuspend(&data->client->dev);
+	return ret;
 }
 
 static const struct iio_trigger_ops mma8452_trigger_ops = {
-- 
2.56.0.218.g1ec904b173-goog

Re: [PATCH v2 03/17] iio: accel: mma8452: Discard pm_runtime_put_autosuspend() return values

[Andy Shevchenko]

On Wed, Oct 21, 2026 at 04:57:04PM -0700, Brian Norris 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.
> 
> For this reason, modify this driver to simply discard the
> pm_runtime_put_autosuspend() return value, and inline the pm_runtime
> helper, since the behavior is the same with and without CONFIG_PM.
> 
> While I'm at it: I see that mma8452_data_rdy_trigger_set_state() and
> mma8452_write_event_config() mismanage the runtime PM reference count,
> and instead rely on (racy) autosuspend delay for correct ordering.
> Undo this choice, and instead properly drop the reference count only
> after we're done touching the hardware.
> 
> 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").

...

> static int mma8452_read(struct mma8452_data *data, __be16 buf[3])

> -	ret = mma8452_set_runtime_pm_state(data->client, true);
> -	if (ret)
> +	ret = pm_runtime_resume_and_get(&data->client->dev);
> +	if (ret < 0) {
> +		dev_err(&data->client->dev, "failed to resume: %d\n", ret);
>  		return ret;
> +	}
>  
>  	ret = i2c_smbus_read_i2c_block_data(data->client, MMA8452_OUT_X,
>  					    3 * sizeof(__be16), (u8 *)buf);
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = mma8452_set_runtime_pm_state(data->client, false);
> +	pm_runtime_put_autosuspend(&data->client->dev);
>  
> -	return ret;
> +	return 0;

Now it can use PM_RUNTIME_ACQUIRE*(), no?

>  }

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 04/17] iio: accel: mma955x: Discard pm_runtime_put_autosuspend() return values

[Brian Norris]

These drivers propagate 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.

For this reason, modify these drivers to simply discard the
pm_runtime_put_autosuspend() return value, and inline the pm_runtime
helpers now that they behave the same with and without CONFIG_PM.

While at it, I see that mma9553_write_event_config() mismanages the
runtime PM reference count. It should only put() a reference after it's
done touching the hardware, not before. Resolve this ordering issue.

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

Changes in v2:
 * Split iio patch into per-driver patches
 * Inlining
 * Refcount ordering

 drivers/iio/accel/mma9551.c      | 21 +++++++++--------
 drivers/iio/accel/mma9551_core.c | 39 ++++----------------------------
 drivers/iio/accel/mma9551_core.h |  1 -
 drivers/iio/accel/mma9553.c      | 36 ++++++++++++++++++++---------
 4 files changed, 42 insertions(+), 55 deletions(-)

diff --git a/drivers/iio/accel/mma9551.c b/drivers/iio/accel/mma9551.c
index 182f488edab1..4c549fb106bc 100644
--- a/drivers/iio/accel/mma9551.c
+++ b/drivers/iio/accel/mma9551.c
@@ -71,9 +71,11 @@ static int mma9551_read_incli_chan(struct i2c_client *client,
 		return -EINVAL;
 	}
 
-	ret = mma9551_set_power_state(client, true);
-	if (ret < 0)
+	ret = pm_runtime_resume_and_get(&client->dev);
+	if (ret < 0) {
+		dev_err(&client->dev, "failed to resume: %d\n", ret);
 		return ret;
+	}
 
 	ret = mma9551_read_status_byte(client, MMA9551_APPID_TILT,
 				       reg_addr, &angle);
@@ -96,7 +98,7 @@ static int mma9551_read_incli_chan(struct i2c_client *client,
 	ret = IIO_VAL_INT;
 
 out_poweroff:
-	mma9551_set_power_state(client, false);
+	pm_runtime_put_autosuspend(&client->dev);
 	return ret;
 }
 
@@ -178,9 +180,7 @@ static int mma9551_config_incli_event(struct iio_dev *indio_dev,
 		if (ret < 0)
 			return ret;
 
-		ret = mma9551_set_power_state(data->client, false);
-		if (ret < 0)
-			return ret;
+		pm_runtime_put_autosuspend(&data->client->dev);
 	} else {
 		int bitnum;
 
@@ -200,15 +200,18 @@ static int mma9551_config_incli_event(struct iio_dev *indio_dev,
 		}
 
 
-		ret = mma9551_set_power_state(data->client, true);
-		if (ret < 0)
+		ret = pm_runtime_resume_and_get(&data->client->dev);
+		if (ret < 0) {
+			dev_err(&data->client->dev, "failed to resume: %d\n",
+				ret);
 			return ret;
+		}
 
 		ret = mma9551_gpio_config(data->client,
 					  (enum mma9551_gpio_pin)mma_axis,
 					  MMA9551_APPID_TILT, bitnum, 0);
 		if (ret < 0) {
-			mma9551_set_power_state(data->client, false);
+			pm_runtime_put_autosuspend(&data->client->dev);
 			return ret;
 		}
 	}
diff --git a/drivers/iio/accel/mma9551_core.c b/drivers/iio/accel/mma9551_core.c
index 2ccb1fb19b96..68df195d5b24 100644
--- a/drivers/iio/accel/mma9551_core.c
+++ b/drivers/iio/accel/mma9551_core.c
@@ -654,37 +654,6 @@ int mma9551_set_device_state(struct i2c_client *client, bool enable)
 }
 EXPORT_SYMBOL_NS(mma9551_set_device_state, "IIO_MMA9551");
 
-/**
- * mma9551_set_power_state() - sets runtime PM state
- * @client:	I2C client
- * @on:		Use true to power on device, false to power off
- *
- * Resume or suspend the device using Runtime PM.
- * The device will suspend after the autosuspend delay.
- *
- * Returns: 0 on success, negative value on failure.
- */
-int mma9551_set_power_state(struct i2c_client *client, bool on)
-{
-#ifdef CONFIG_PM
-	int ret;
-
-	if (on)
-		ret = pm_runtime_resume_and_get(&client->dev);
-	else
-		ret = pm_runtime_put_autosuspend(&client->dev);
-	if (ret < 0) {
-		dev_err(&client->dev,
-			"failed to change power state to %d\n", on);
-
-		return ret;
-	}
-#endif
-
-	return 0;
-}
-EXPORT_SYMBOL_NS(mma9551_set_power_state, "IIO_MMA9551");
-
 /**
  * mma9551_sleep() - sleep
  * @freq:	Application frequency
@@ -741,9 +710,11 @@ int mma9551_read_accel_chan(struct i2c_client *client,
 		return -EINVAL;
 	}
 
-	ret = mma9551_set_power_state(client, true);
-	if (ret < 0)
+	ret = pm_runtime_resume_and_get(&client->dev);
+	if (ret < 0) {
+		dev_err(&client->dev, "failed to resume: %d\n", ret);
 		return ret;
+	}
 
 	ret = mma9551_read_status_word(client, MMA9551_APPID_AFE,
 				       reg_addr, &raw_accel);
@@ -755,7 +726,7 @@ int mma9551_read_accel_chan(struct i2c_client *client,
 	ret = IIO_VAL_INT;
 
 out_poweroff:
-	mma9551_set_power_state(client, false);
+	pm_runtime_put_autosuspend(&client->dev);
 	return ret;
 }
 EXPORT_SYMBOL_NS(mma9551_read_accel_chan, "IIO_MMA9551");
diff --git a/drivers/iio/accel/mma9551_core.h b/drivers/iio/accel/mma9551_core.h
index 543454a31dc0..1d55fdfd0a92 100644
--- a/drivers/iio/accel/mma9551_core.h
+++ b/drivers/iio/accel/mma9551_core.h
@@ -62,7 +62,6 @@ int mma9551_gpio_config(struct i2c_client *client, enum mma9551_gpio_pin pin,
 			u8 app_id, u8 bitnum, int polarity);
 int mma9551_read_version(struct i2c_client *client);
 int mma9551_set_device_state(struct i2c_client *client, bool enable);
-int mma9551_set_power_state(struct i2c_client *client, bool on);
 void mma9551_sleep(int freq);
 int mma9551_read_accel_chan(struct i2c_client *client,
 			    const struct iio_chan_spec *chan,
diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
index 5a36d415a837..2ccb68dfbcd4 100644
--- a/drivers/iio/accel/mma9553.c
+++ b/drivers/iio/accel/mma9553.c
@@ -610,10 +610,16 @@ static int mma9553_write_raw(struct iio_dev *indio_dev,
 		if (data->stepcnt_enabled == !!val)
 			return 0;
 		mutex_lock(&data->mutex);
-		ret = mma9551_set_power_state(data->client, val);
-		if (ret < 0) {
-			mutex_unlock(&data->mutex);
-			return ret;
+		if (val) {
+			ret = pm_runtime_resume_and_get(&data->client->dev);
+			if (ret < 0) {
+				dev_err(&data->client->dev,
+					"failed to resume: %d\n", ret);
+				mutex_unlock(&data->mutex);
+				return ret;
+			}
+		} else {
+			pm_runtime_put_autosuspend(&data->client->dev);
 		}
 		data->stepcnt_enabled = val;
 		mutex_unlock(&data->mutex);
@@ -737,24 +743,32 @@ static int mma9553_write_event_config(struct iio_dev *indio_dev,
 
 	mutex_lock(&data->mutex);
 
-	ret = mma9551_set_power_state(data->client, state);
-	if (ret < 0)
-		goto err_out;
+	if (state) {
+		ret = pm_runtime_resume_and_get(&data->client->dev);
+		if (ret < 0) {
+			dev_err(&data->client->dev, "failed to resume: %d\n",
+				ret);
+			goto err_out;
+		}
+	}
+
 	event->enabled = state;
 
 	ret = mma9553_conf_gpio(data);
 	if (ret < 0)
 		goto err_conf_gpio;
 
+	if (!state)
+		pm_runtime_put_autosuspend(&data->client->dev);
+
 	mutex_unlock(&data->mutex);
 
 	return 0;
 
 err_conf_gpio:
-	if (state) {
-		event->enabled = false;
-		mma9551_set_power_state(data->client, false);
-	}
+	if (state)
+		pm_runtime_put_autosuspend(&data->client->dev);
+	event->enabled = !state;
 err_out:
 	mutex_unlock(&data->mutex);
 	return ret;
-- 
2.56.0.218.g1ec904b173-goog

[PATCH v2 05/17] iio: accel: bmc150: Discard pm_runtime_put_autosuspend() return values

[Brian Norris]

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.

For this reason, modify this driver to simply discard the
pm_runtime_put_autosuspend() return value, and inline the pm_runtime
helper, now that it behaves the same with and without CONFIG_PM.

While at it, I notice that bmc150_accel_set_interrupt() mismanages the
reference counts. It should only drop a reference after it's done
touching the hardware in question. Adjust the local &intr->users
reference count alongside the error handling too, to ensure proper
balance.

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

Changes in v2:
 * Split iio patch into per-driver patches
 * Inlining
 * Refcount ordering

 drivers/iio/accel/bmc150-accel-core.c | 75 +++++++++++----------------
 1 file changed, 31 insertions(+), 44 deletions(-)

diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c
index 46bb36d9366e..9ad6429fb584 100644
--- a/drivers/iio/accel/bmc150-accel-core.c
+++ b/drivers/iio/accel/bmc150-accel-core.c
@@ -326,29 +326,6 @@ static int bmc150_accel_get_startup_times(struct bmc150_accel_data *data)
 
 	return BMC150_ACCEL_MAX_STARTUP_TIME_MS;
 }
-
-static int bmc150_accel_set_power_state(struct bmc150_accel_data *data, bool on)
-{
-	struct device *dev = regmap_get_device(data->regmap);
-	int ret;
-
-	if (on)
-		ret = pm_runtime_resume_and_get(dev);
-	else
-		ret = pm_runtime_put_autosuspend(dev);
-	if (ret < 0) {
-		dev_err(dev,
-			"Failed: %s for %d\n", __func__, on);
-		return ret;
-	}
-
-	return 0;
-}
-#else
-static int bmc150_accel_set_power_state(struct bmc150_accel_data *data, bool on)
-{
-	return 0;
-}
 #endif
 
 #ifdef CONFIG_ACPI
@@ -530,24 +507,18 @@ static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data, int i,
 	if (state) {
 		if (atomic_inc_return(&intr->users) > 1)
 			return 0;
+
+		ret = pm_runtime_resume_and_get(dev);
+		if (ret < 0) {
+			dev_err(dev, "failed to resume: %d\n", ret);
+			atomic_dec(&intr->users);
+			return ret;
+		}
 	} else {
 		if (atomic_dec_return(&intr->users) > 0)
 			return 0;
 	}
 
-	/*
-	 * We will expect the enable and disable to do operation in reverse
-	 * order. This will happen here anyway, as our resume operation uses
-	 * sync mode runtime pm calls. The suspend operation will be delayed
-	 * by autosuspend delay.
-	 * So the disable operation will still happen in reverse order of
-	 * enable operation. When runtime pm is disabled the mode is always on,
-	 * so sequence doesn't matter.
-	 */
-	ret = bmc150_accel_set_power_state(data, state);
-	if (ret < 0)
-		return ret;
-
 	/* map the interrupt to the appropriate pins */
 	ret = regmap_update_bits(data->regmap, info->map_reg, info->map_bitmask,
 				 (state ? info->map_bitmask : 0));
@@ -564,10 +535,18 @@ static int bmc150_accel_set_interrupt(struct bmc150_accel_data *data, int i,
 		goto out_fix_power_state;
 	}
 
+	if (!state)
+		pm_runtime_put_autosuspend(dev);
+
 	return 0;
 
 out_fix_power_state:
-	bmc150_accel_set_power_state(data, false);
+	if (state) {
+		pm_runtime_put_autosuspend(dev);
+		atomic_dec(&intr->users);
+	} else {
+		atomic_inc(&intr->users);
+	}
 	return ret;
 }
 
@@ -625,8 +604,9 @@ static int bmc150_accel_get_axis(struct bmc150_accel_data *data,
 	__le16 raw_val;
 
 	mutex_lock(&data->mutex);
-	ret = bmc150_accel_set_power_state(data, true);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0) {
+		dev_err(dev, "failed to resume: %d\n", ret);
 		mutex_unlock(&data->mutex);
 		return ret;
 	}
@@ -635,16 +615,14 @@ static int bmc150_accel_get_axis(struct bmc150_accel_data *data,
 			       &raw_val, sizeof(raw_val));
 	if (ret < 0) {
 		dev_err(dev, "Error reading axis %d\n", axis);
-		bmc150_accel_set_power_state(data, false);
+		pm_runtime_put_autosuspend(dev);
 		mutex_unlock(&data->mutex);
 		return ret;
 	}
 	*val = sign_extend32(le16_to_cpu(raw_val) >> chan->scan_type.shift,
 			     chan->scan_type.realbits - 1);
-	ret = bmc150_accel_set_power_state(data, false);
+	pm_runtime_put_autosuspend(dev);
 	mutex_unlock(&data->mutex);
-	if (ret < 0)
-		return ret;
 
 	return IIO_VAL_INT;
 }
@@ -1477,8 +1455,14 @@ static int bmc150_accel_fifo_set_mode(struct bmc150_accel_data *data)
 static int bmc150_accel_buffer_preenable(struct iio_dev *indio_dev)
 {
 	struct bmc150_accel_data *data = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(data->regmap);
+	int ret;
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret < 0)
+		dev_err(dev, "failed to resume: %d\n", ret);
 
-	return bmc150_accel_set_power_state(data, true);
+	return ret;
 }
 
 static int bmc150_accel_buffer_postenable(struct iio_dev *indio_dev)
@@ -1540,8 +1524,11 @@ static int bmc150_accel_buffer_predisable(struct iio_dev *indio_dev)
 static int bmc150_accel_buffer_postdisable(struct iio_dev *indio_dev)
 {
 	struct bmc150_accel_data *data = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(data->regmap);
 
-	return bmc150_accel_set_power_state(data, false);
+	pm_runtime_put_autosuspend(dev);
+
+	return 0;
 }
 
 static const struct iio_buffer_setup_ops bmc150_accel_buffer_ops = {
-- 
2.56.0.218.g1ec904b173-goog

[PATCH v2 06/17] iio: adc: rcar-gyroadc: Discard pm_runtime_put_autosuspend() return values

[Brian Norris]

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.

For this reason, modify this driver to simply discard the
pm_runtime_put_autosuspend() 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>
---

Changes in v2:
 * Split iio patch into per-driver patches

 drivers/iio/adc/rcar-gyroadc.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/iio/adc/rcar-gyroadc.c b/drivers/iio/adc/rcar-gyroadc.c
index d38cf3caeae0..0d3b061d6ec2 100644
--- a/drivers/iio/adc/rcar-gyroadc.c
+++ b/drivers/iio/adc/rcar-gyroadc.c
@@ -199,10 +199,8 @@ static int rcar_gyroadc_read_raw(struct iio_dev *indio_dev,
 		*val = readl(priv->regs + datareg);
 		*val &= BIT(priv->sample_width) - 1;
 
-		ret = pm_runtime_put_autosuspend(priv->dev);
+		pm_runtime_put_autosuspend(priv->dev);
 		iio_device_release_direct(indio_dev);
-		if (ret < 0)
-			return ret;
 
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
-- 
2.56.0.218.g1ec904b173-goog

Re: [PATCH v2 06/17] iio: adc: rcar-gyroadc: Discard pm_runtime_put_autosuspend() return values

[Geert Uytterhoeven]

On Thu, 22 Oct 2026 at 02:01, Brian Norris <briannorris@chromium.org> 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.
>
> For this reason, modify this driver to simply discard the
> pm_runtime_put_autosuspend() 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>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v2 07/17] iio: adc: ti-ads1015: Discard pm_runtime_put_autosuspend() return values

[Brian Norris]

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.

For this reason, modify this driver to simply discard the
pm_runtime_put_autosuspend() return value. Inline
ads1015_set_power_state() while we're at it, since its behavior is now
the same with and without CONFIG_PM, and it is otherwise trivial.

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

Changes in v2:
 * Split iio patch into per-driver patches
 * Inlining

 drivers/iio/adc/ti-ads1015.c | 50 +++++++++++++-----------------------
 1 file changed, 18 insertions(+), 32 deletions(-)

diff --git a/drivers/iio/adc/ti-ads1015.c b/drivers/iio/adc/ti-ads1015.c
index 0f9244107c6d..a677ec7abda2 100644
--- a/drivers/iio/adc/ti-ads1015.c
+++ b/drivers/iio/adc/ti-ads1015.c
@@ -367,30 +367,6 @@ static const struct iio_chan_spec tla2024_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(ADS1015_TIMESTAMP),
 };
 
-
-#ifdef CONFIG_PM
-static int ads1015_set_power_state(struct ads1015_data *data, bool on)
-{
-	int ret;
-	struct device *dev = regmap_get_device(data->regmap);
-
-	if (on)
-		ret = pm_runtime_resume_and_get(dev);
-	else
-		ret = pm_runtime_put_autosuspend(dev);
-
-	return ret < 0 ? ret : 0;
-}
-
-#else /* !CONFIG_PM */
-
-static int ads1015_set_power_state(struct ads1015_data *data, bool on)
-{
-	return 0;
-}
-
-#endif /* !CONFIG_PM */
-
 static
 int ads1015_get_adc_result(struct ads1015_data *data, int chan, int *val)
 {
@@ -533,26 +509,29 @@ static int ads1015_read_avail(struct iio_dev *indio_dev,
 static int __ads1015_read_info_raw(struct ads1015_data *data,
 				   struct iio_chan_spec const *chan, int *val)
 {
+	struct device *dev = regmap_get_device(data->regmap);
 	int ret;
 
 	if (ads1015_event_channel_enabled(data) &&
 	    data->event_channel != chan->address)
 		return -EBUSY;
 
-	ret = ads1015_set_power_state(data, true);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0)
 		return ret;
 
 	ret = ads1015_get_adc_result(data, chan->address, val);
 	if (ret < 0) {
-		ads1015_set_power_state(data, false);
+		pm_runtime_put_autosuspend(dev);
 		return ret;
 	}
 
 	*val = sign_extend32(*val >> chan->scan_type.shift,
 			     chan->scan_type.realbits - 1);
 
-	return ads1015_set_power_state(data, false);
+	pm_runtime_put_autosuspend(dev);
+
+	return 0;
 }
 
 static int ads1015_read_raw(struct iio_dev *indio_dev,
@@ -708,6 +687,7 @@ static int ads1015_read_event_config(struct iio_dev *indio_dev,
 static int ads1015_enable_event_config(struct ads1015_data *data,
 	const struct iio_chan_spec *chan, int comp_mode)
 {
+	struct device *dev = regmap_get_device(data->regmap);
 	int low_thresh = data->thresh_data[chan->address].low_thresh;
 	int high_thresh = data->thresh_data[chan->address].high_thresh;
 	int ret;
@@ -736,7 +716,7 @@ static int ads1015_enable_event_config(struct ads1015_data *data,
 	if (ret)
 		return ret;
 
-	ret = ads1015_set_power_state(data, true);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0)
 		return ret;
 
@@ -745,7 +725,7 @@ static int ads1015_enable_event_config(struct ads1015_data *data,
 	ret = ads1015_get_adc_result(data, chan->address, &val);
 	if (ret) {
 		ads1015_event_channel_disable(data, chan->address);
-		ads1015_set_power_state(data, false);
+		pm_runtime_put_autosuspend(dev);
 	}
 
 	return ret;
@@ -775,7 +755,9 @@ static int ads1015_disable_event_config(struct ads1015_data *data,
 
 	ads1015_event_channel_disable(data, chan->address);
 
-	return ads1015_set_power_state(data, false);
+	pm_runtime_put_autosuspend(regmap_get_device(data->regmap));
+
+	return 0;
 }
 
 static int ads1015_write_event_config(struct iio_dev *indio_dev,
@@ -836,12 +818,16 @@ static int ads1015_buffer_preenable(struct iio_dev *indio_dev)
 	if (ads1015_event_channel_enabled(data))
 		return -EBUSY;
 
-	return ads1015_set_power_state(iio_priv(indio_dev), true);
+	return pm_runtime_resume_and_get(regmap_get_device(data->regmap));
 }
 
 static int ads1015_buffer_postdisable(struct iio_dev *indio_dev)
 {
-	return ads1015_set_power_state(iio_priv(indio_dev), false);
+	struct ads1015_data *data = iio_priv(indio_dev);
+
+	pm_runtime_put_autosuspend(regmap_get_device(data->regmap));
+
+	return 0;
 }
 
 static const struct iio_buffer_setup_ops ads1015_buffer_setup_ops = {
-- 
2.56.0.218.g1ec904b173-goog

[PATCH v2 08/17] iio: chemical: atlas-sensor: Discard pm_runtime_put_autosuspend() return values

[Brian Norris]

The atlas-sensor 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.

For this reason, modify this driver to simply discard the
pm_runtime_put_autosuspend() 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>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---

Changes in v2:
 * Split iio patch into per-driver patches

 drivers/iio/chemical/atlas-sensor.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/iio/chemical/atlas-sensor.c b/drivers/iio/chemical/atlas-sensor.c
index 1d4a9919df1a..48598475512f 100644
--- a/drivers/iio/chemical/atlas-sensor.c
+++ b/drivers/iio/chemical/atlas-sensor.c
@@ -426,9 +426,7 @@ static int atlas_buffer_predisable(struct iio_dev *indio_dev)
 	if (ret)
 		return ret;
 
-	ret = pm_runtime_put_autosuspend(&data->client->dev);
-	if (ret)
-		return ret;
+	pm_runtime_put_autosuspend(&data->client->dev);
 
 	return 0;
 }
-- 
2.56.0.218.g1ec904b173-goog

[PATCH v2 09/17] iio: hid-sensors: Discard pm_runtime_put_autosuspend() return values

[Brian Norris]

The hid-sensors 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.

For this reason, modify this driver to simply discard the
pm_runtime_put_autosuspend() 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>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
---

Changes in v2:
 * Split iio patch into per-driver patches

 .../iio/common/hid-sensors/hid-sensor-trigger.c    | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
index 60808e2430ca..d76a6d9f04f6 100644
--- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
+++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
@@ -154,21 +154,17 @@ int hid_sensor_power_state(struct hid_sensor_common *st, bool state)
 {
 
 #ifdef CONFIG_PM
-	int ret;
-
 	if (atomic_add_unless(&st->runtime_pm_enable, 1, 1))
 		pm_runtime_enable(&st->pdev->dev);
 
 	if (state) {
 		atomic_inc(&st->user_requested_state);
-		ret = pm_runtime_resume_and_get(&st->pdev->dev);
-	} else {
-		atomic_dec(&st->user_requested_state);
-		pm_runtime_use_autosuspend(&st->pdev->dev);
-		ret = pm_runtime_put_autosuspend(&st->pdev->dev);
+		return pm_runtime_resume_and_get(&st->pdev->dev);
 	}
-	if (ret < 0)
-		return ret;
+
+	atomic_dec(&st->user_requested_state);
+	pm_runtime_use_autosuspend(&st->pdev->dev);
+	pm_runtime_put_autosuspend(&st->pdev->dev);
 
 	return 0;
 #else
-- 
2.56.0.218.g1ec904b173-goog

[PATCH v2 10/17] iio: gyro: bmg160: Discard pm_runtime_put_autosuspend() return values

[Brian Norris]

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.

For this reason, modify this driver to simply discard the
pm_runtime_put_autosuspend() return value, and inline
bmg160_set_power_state() now that it behaves the same with and without
CONFIG_PM and is otherwise trivial.

While at it, I notice that bmg160_write_event_config() and
bmg160_data_rdy_trigger_set_state() mismanage the runtime PM reference
count intentionally, relying on racy autosuspend behavior for
correctness. I refactor this to manage refcounts properly instead.
I use guard() for the mutex, to make this less error prone.

Additionally, bmg160_write_raw() had one error path where a
pm_runtime_resume_and_get() failure would also drop a runtime PM
refcount (pm_runtime_put_autosuspend()), which potentially leads to
underflow. I fix that.

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

Changes in v2:
 * Split iio patch into per-driver patches
 * Inlining
 * guard()

 drivers/iio/gyro/bmg160_core.c | 178 ++++++++++++++-------------------
 1 file changed, 77 insertions(+), 101 deletions(-)

diff --git a/drivers/iio/gyro/bmg160_core.c b/drivers/iio/gyro/bmg160_core.c
index 0958b7d31a92..0281a3a52105 100644
--- a/drivers/iio/gyro/bmg160_core.c
+++ b/drivers/iio/gyro/bmg160_core.c
@@ -8,6 +8,7 @@
 #include <linux/interrupt.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
+#include <linux/cleanup.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/iio/iio.h>
@@ -313,30 +314,6 @@ static int bmg160_chip_init(struct bmg160_data *data)
 	return 0;
 }
 
-static int bmg160_set_power_state(struct bmg160_data *data, bool on)
-{
-#ifdef CONFIG_PM
-	struct device *dev = regmap_get_device(data->regmap);
-	int ret;
-
-	if (on)
-		ret = pm_runtime_get_sync(dev);
-	else
-		ret = pm_runtime_put_autosuspend(dev);
-
-	if (ret < 0) {
-		dev_err(dev, "Failed: bmg160_set_power_state for %d\n", on);
-
-		if (on)
-			pm_runtime_put_noidle(dev);
-
-		return ret;
-	}
-#endif
-
-	return 0;
-}
-
 static int bmg160_setup_any_motion_interrupt(struct bmg160_data *data,
 					     bool status)
 {
@@ -503,8 +480,9 @@ static int bmg160_get_temp(struct bmg160_data *data, int *val)
 	unsigned int raw_val;
 
 	mutex_lock(&data->mutex);
-	ret = bmg160_set_power_state(data, true);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0) {
+		dev_err(dev, "failed to resume: %d\n", ret);
 		mutex_unlock(&data->mutex);
 		return ret;
 	}
@@ -512,16 +490,14 @@ static int bmg160_get_temp(struct bmg160_data *data, int *val)
 	ret = regmap_read(data->regmap, BMG160_REG_TEMP, &raw_val);
 	if (ret < 0) {
 		dev_err(dev, "Error reading reg_temp\n");
-		bmg160_set_power_state(data, false);
+		pm_runtime_put_autosuspend(dev);
 		mutex_unlock(&data->mutex);
 		return ret;
 	}
 
 	*val = sign_extend32(raw_val, 7);
-	ret = bmg160_set_power_state(data, false);
+	pm_runtime_put_autosuspend(dev);
 	mutex_unlock(&data->mutex);
-	if (ret < 0)
-		return ret;
 
 	return IIO_VAL_INT;
 }
@@ -533,8 +509,9 @@ static int bmg160_get_axis(struct bmg160_data *data, int axis, int *val)
 	__le16 raw_val;
 
 	mutex_lock(&data->mutex);
-	ret = bmg160_set_power_state(data, true);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0) {
+		dev_err(dev, "failed to resume: %d\n", ret);
 		mutex_unlock(&data->mutex);
 		return ret;
 	}
@@ -543,16 +520,14 @@ static int bmg160_get_axis(struct bmg160_data *data, int axis, int *val)
 			       sizeof(raw_val));
 	if (ret < 0) {
 		dev_err(dev, "Error reading axis %d\n", axis);
-		bmg160_set_power_state(data, false);
+		pm_runtime_put_autosuspend(dev);
 		mutex_unlock(&data->mutex);
 		return ret;
 	}
 
 	*val = sign_extend32(le16_to_cpu(raw_val), 15);
-	ret = bmg160_set_power_state(data, false);
+	pm_runtime_put_autosuspend(dev);
 	mutex_unlock(&data->mutex);
-	if (ret < 0)
-		return ret;
 
 	return IIO_VAL_INT;
 }
@@ -624,6 +599,7 @@ static int bmg160_write_raw(struct iio_dev *indio_dev,
 			    int val, int val2, long mask)
 {
 	struct bmg160_data *data = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(data->regmap);
 	int ret;
 
 	switch (mask) {
@@ -636,18 +612,14 @@ static int bmg160_write_raw(struct iio_dev *indio_dev,
 		 * register. Since we will be in suspend mode by default, change
 		 * mode to power on for other writes.
 		 */
-		ret = bmg160_set_power_state(data, true);
+		ret = pm_runtime_resume_and_get(dev);
 		if (ret < 0) {
+			dev_err(dev, "failed to resume: %d\n", ret);
 			mutex_unlock(&data->mutex);
 			return ret;
 		}
 		ret = bmg160_set_bw(data, val);
-		if (ret < 0) {
-			bmg160_set_power_state(data, false);
-			mutex_unlock(&data->mutex);
-			return ret;
-		}
-		ret = bmg160_set_power_state(data, false);
+		pm_runtime_put_autosuspend(dev);
 		mutex_unlock(&data->mutex);
 		return ret;
 	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
@@ -655,19 +627,14 @@ static int bmg160_write_raw(struct iio_dev *indio_dev,
 			return -EINVAL;
 
 		mutex_lock(&data->mutex);
-		ret = bmg160_set_power_state(data, true);
+		ret = pm_runtime_resume_and_get(dev);
 		if (ret < 0) {
-			bmg160_set_power_state(data, false);
+			dev_err(dev, "failed to resume: %d\n", ret);
 			mutex_unlock(&data->mutex);
 			return ret;
 		}
 		ret = bmg160_set_filter(data, val);
-		if (ret < 0) {
-			bmg160_set_power_state(data, false);
-			mutex_unlock(&data->mutex);
-			return ret;
-		}
-		ret = bmg160_set_power_state(data, false);
+		pm_runtime_put_autosuspend(dev);
 		mutex_unlock(&data->mutex);
 		return ret;
 	case IIO_CHAN_INFO_SCALE:
@@ -676,18 +643,14 @@ static int bmg160_write_raw(struct iio_dev *indio_dev,
 
 		mutex_lock(&data->mutex);
 		/* Refer to comments above for the suspend mode ops */
-		ret = bmg160_set_power_state(data, true);
+		ret = pm_runtime_resume_and_get(dev);
 		if (ret < 0) {
+			dev_err(dev, "failed to resume: %d\n", ret);
 			mutex_unlock(&data->mutex);
 			return ret;
 		}
 		ret = bmg160_set_scale(data, val2);
-		if (ret < 0) {
-			bmg160_set_power_state(data, false);
-			mutex_unlock(&data->mutex);
-			return ret;
-		}
-		ret = bmg160_set_power_state(data, false);
+		pm_runtime_put_autosuspend(dev);
 		mutex_unlock(&data->mutex);
 		return ret;
 	default:
@@ -759,42 +722,40 @@ static int bmg160_write_event_config(struct iio_dev *indio_dev,
 				     bool state)
 {
 	struct bmg160_data *data = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(data->regmap);
 	int ret;
 
 	if (state == data->ev_enable_state)
 		return 0;
 
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);
 
 	if (!state && data->motion_trigger_on) {
 		data->ev_enable_state = 0;
-		mutex_unlock(&data->mutex);
 		return 0;
 	}
-	/*
-	 * We will expect the enable and disable to do operation
-	 * in reverse order. This will happen here anyway as our
-	 * resume operation uses sync mode runtime pm calls, the
-	 * suspend operation will be delayed by autosuspend delay
-	 * So the disable operation will still happen in reverse of
-	 * enable operation. When runtime pm is disabled the mode
-	 * is always on so sequence doesn't matter
-	 */
-	ret = bmg160_set_power_state(data, state);
-	if (ret < 0) {
-		mutex_unlock(&data->mutex);
-		return ret;
-	}
 
-	ret =  bmg160_setup_any_motion_interrupt(data, state);
-	if (ret < 0) {
-		bmg160_set_power_state(data, false);
-		mutex_unlock(&data->mutex);
-		return ret;
+	if (state) {
+		ret = pm_runtime_resume_and_get(dev);
+		if (ret < 0) {
+			dev_err(dev, "failed to resume: %d\n", ret);
+			return ret;
+		}
+
+		ret = bmg160_setup_any_motion_interrupt(data, true);
+		if (ret < 0) {
+			pm_runtime_put_autosuspend(dev);
+			return ret;
+		}
+	} else {
+		ret = bmg160_setup_any_motion_interrupt(data, false);
+		if (ret < 0)
+			return ret;
+
+		pm_runtime_put_autosuspend(dev);
 	}
 
 	data->ev_enable_state = state;
-	mutex_unlock(&data->mutex);
 
 	return 0;
 }
@@ -929,41 +890,47 @@ static int bmg160_data_rdy_trigger_set_state(struct iio_trigger *trig,
 {
 	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
 	struct bmg160_data *data = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(data->regmap);
 	int ret;
 
-	mutex_lock(&data->mutex);
+	guard(mutex)(&data->mutex);
 
 	if (!state && data->ev_enable_state && data->motion_trigger_on) {
 		data->motion_trigger_on = false;
-		mutex_unlock(&data->mutex);
 		return 0;
 	}
 
-	/*
-	 * Refer to comment in bmg160_write_event_config for
-	 * enable/disable operation order
-	 */
-	ret = bmg160_set_power_state(data, state);
-	if (ret < 0) {
-		mutex_unlock(&data->mutex);
-		return ret;
-	}
-	if (data->motion_trig == trig)
-		ret =  bmg160_setup_any_motion_interrupt(data, state);
-	else
-		ret = bmg160_setup_new_data_interrupt(data, state);
-	if (ret < 0) {
-		bmg160_set_power_state(data, false);
-		mutex_unlock(&data->mutex);
-		return ret;
+	if (state) {
+		ret = pm_runtime_resume_and_get(dev);
+		if (ret < 0) {
+			dev_err(dev, "failed to resume: %d\n", ret);
+			return ret;
+		}
+
+		if (data->motion_trig == trig)
+			ret = bmg160_setup_any_motion_interrupt(data, true);
+		else
+			ret = bmg160_setup_new_data_interrupt(data, true);
+		if (ret < 0) {
+			pm_runtime_put_autosuspend(dev);
+			return ret;
+		}
+	} else {
+		if (data->motion_trig == trig)
+			ret = bmg160_setup_any_motion_interrupt(data, false);
+		else
+			ret = bmg160_setup_new_data_interrupt(data, false);
+		if (ret < 0)
+			return ret;
+
+		pm_runtime_put_autosuspend(dev);
 	}
+
 	if (data->motion_trig == trig)
 		data->motion_trigger_on = state;
 	else
 		data->dready_trigger_on = state;
 
-	mutex_unlock(&data->mutex);
-
 	return 0;
 }
 
@@ -1046,15 +1013,24 @@ static irqreturn_t bmg160_data_rdy_trig_poll(int irq, void *private)
 static int bmg160_buffer_preenable(struct iio_dev *indio_dev)
 {
 	struct bmg160_data *data = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(data->regmap);
+	int ret;
 
-	return bmg160_set_power_state(data, true);
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret < 0)
+		dev_err(dev, "failed to resume: %d\n", ret);
+
+	return ret;
 }
 
 static int bmg160_buffer_postdisable(struct iio_dev *indio_dev)
 {
 	struct bmg160_data *data = iio_priv(indio_dev);
+	struct device *dev = regmap_get_device(data->regmap);
 
-	return bmg160_set_power_state(data, false);
+	pm_runtime_put_autosuspend(dev);
+
+	return 0;
 }
 
 static const struct iio_buffer_setup_ops bmg160_buffer_setup_ops = {
-- 
2.56.0.218.g1ec904b173-goog

[PATCH v2 11/17] iio: gyro: fxas21002c: Discard pm_runtime_put_autosuspend() return values

[Brian Norris]

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.

For this reason, modify this driver to simply discard the
pm_runtime_put_autosuspend() return value, and inline the helpers now
that they're even more trivial.

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

Changes in v2:
 * Split iio patch into per-driver patches
 * Inlining

 drivers/iio/gyro/fxas21002c_core.c | 33 ++++++++----------------------
 1 file changed, 9 insertions(+), 24 deletions(-)

diff --git a/drivers/iio/gyro/fxas21002c_core.c b/drivers/iio/gyro/fxas21002c_core.c
index a88670207cec..1d891387b121 100644
--- a/drivers/iio/gyro/fxas21002c_core.c
+++ b/drivers/iio/gyro/fxas21002c_core.c
@@ -364,18 +364,6 @@ 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);
@@ -383,22 +371,20 @@ static int fxas21002c_temp_get(struct fxas21002c_data *data, int *val)
 	int ret;
 
 	mutex_lock(&data->lock);
-	ret = fxas21002c_pm_get(data);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0)
 		goto data_unlock;
 
 	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);
+		pm_runtime_put_autosuspend(dev);
 		goto data_unlock;
 	}
 
 	*val = sign_extend32(temp, 7);
 
-	ret = fxas21002c_pm_put(data);
-	if (ret < 0)
-		goto data_unlock;
+	pm_runtime_put_autosuspend(dev);
 
 	ret = IIO_VAL_INT;
 
@@ -416,7 +402,7 @@ static int fxas21002c_axis_get(struct fxas21002c_data *data,
 	int ret;
 
 	mutex_lock(&data->lock);
-	ret = fxas21002c_pm_get(data);
+	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0)
 		goto data_unlock;
 
@@ -424,15 +410,13 @@ static int fxas21002c_axis_get(struct fxas21002c_data *data,
 			       &axis_be, sizeof(axis_be));
 	if (ret < 0) {
 		dev_err(dev, "failed to read axis: %d: %d\n", index, ret);
-		fxas21002c_pm_put(data);
+		pm_runtime_put_autosuspend(dev);
 		goto data_unlock;
 	}
 
 	*val = sign_extend32(be16_to_cpu(axis_be), 15);
 
-	ret = fxas21002c_pm_put(data);
-	if (ret < 0)
-		goto data_unlock;
+	pm_runtime_put_autosuspend(dev);
 
 	ret = IIO_VAL_INT;
 
@@ -725,10 +709,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 +726,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.218.g1ec904b173-goog

[PATCH v2 12/17] iio: imu: kmx61: Discard pm_runtime_put_autosuspend() return values

[Brian Norris]

The kmx61 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.

For this reason, modify this driver to simply discard the
pm_runtime_put_autosuspend() return value.

Note: kmx61_set_power_state() is a possible inlining candidate, but it
performs some non-trivial logic on the {acc,mag}_ps fields, so I leave
it alone.

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

Changes in v2:
 * Split iio patch into per-driver patches

 drivers/iio/imu/kmx61.c | 25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/imu/kmx61.c b/drivers/iio/imu/kmx61.c
index a4962b88ef58..9d626882adb0 100644
--- a/drivers/iio/imu/kmx61.c
+++ b/drivers/iio/imu/kmx61.c
@@ -747,16 +747,15 @@ static int kmx61_set_power_state(struct kmx61_data *data, bool on, u8 device)
 		data->mag_ps = on;
 	}
 
-	if (on)
+	if (on) {
 		ret = pm_runtime_resume_and_get(&data->client->dev);
-	else
-		ret = pm_runtime_put_autosuspend(&data->client->dev);
-	if (ret < 0) {
-		dev_err(&data->client->dev,
-			"Failed: kmx61_set_power_state for %d, ret %d\n",
-			on, ret);
-
-		return ret;
+		if (ret < 0) {
+			dev_err(&data->client->dev,
+				"%s: failed to resume: %d\n", __func__, ret);
+			return ret;
+		}
+	} else {
+		pm_runtime_put_autosuspend(&data->client->dev);
 	}
 #endif
 	return 0;
@@ -801,15 +800,11 @@ static int kmx61_read_raw(struct iio_dev *indio_dev,
 			return ret;
 
 		ret = kmx61_read_measurement(data, base_reg, chan->scan_index);
-		if (ret < 0) {
-			kmx61_set_power_state(data, false, chan->address);
+		kmx61_set_power_state(data, false, chan->address);
+		if (ret < 0)
 			return ret;
-		}
 		*val = sign_extend32(ret >> chan->scan_type.shift,
 				     chan->scan_type.realbits - 1);
-		ret = kmx61_set_power_state(data, false, chan->address);
-		if (ret)
-			return ret;
 		return IIO_VAL_INT;
 	}
 	case IIO_CHAN_INFO_SCALE:
-- 
2.56.0.218.g1ec904b173-goog

[PATCH v2 13/17] iio: light: apds9960: Discard pm_runtime_put_autosuspend() return values

[Brian Norris]

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.

For this reason, modify this driver to simply discard the
pm_runtime_put_autosuspend() return value. Inline the "off" helper, as
it's trivial now, and rename to apds9960_power_on().

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>
---
I'm not sure if apds9960_write_event_config()'s existing logic is
correct, but I don't want to mess with it.

Changes in v2:
 * Split iio patch into per-driver patches
 * Inline the put() helper

 drivers/iio/light/apds9960.c | 48 ++++++++++++++----------------------
 1 file changed, 18 insertions(+), 30 deletions(-)

diff --git a/drivers/iio/light/apds9960.c b/drivers/iio/light/apds9960.c
index afe520de60e2..0e084ffba1a6 100644
--- a/drivers/iio/light/apds9960.c
+++ b/drivers/iio/light/apds9960.c
@@ -476,38 +476,23 @@ static int apds9960_set_als_gain(struct apds9960_data *data, int val)
 	return ret;
 }
 
-#ifdef CONFIG_PM
-static int apds9960_set_power_state(struct apds9960_data *data, bool on)
+static void apds9960_power_on(struct apds9960_data *data)
 {
 	struct device *dev = &data->client->dev;
-	int ret = 0;
+	int suspended;
 
 	mutex_lock(&data->lock);
 
-	if (on) {
-		int suspended;
-
-		suspended = pm_runtime_suspended(dev);
-		ret = pm_runtime_get_sync(dev);
+	suspended = pm_runtime_suspended(dev);
+	pm_runtime_get_sync(dev);
 
-		/* Allow one integration cycle before allowing a reading */
-		if (suspended)
-			usleep_range(data->als_adc_int_us,
-				     APDS9960_MAX_INT_TIME_IN_US);
-	} else {
-		ret = pm_runtime_put_autosuspend(dev);
-	}
+	/* Allow one integration cycle before allowing a reading */
+	if (suspended)
+		usleep_range(data->als_adc_int_us,
+			     APDS9960_MAX_INT_TIME_IN_US);
 
 	mutex_unlock(&data->lock);
-
-	return ret;
 }
-#else
-static int apds9960_set_power_state(struct apds9960_data *data, bool on)
-{
-	return 0;
-}
-#endif
 
 static int apds9960_read_raw(struct iio_dev *indio_dev,
 			     struct iio_chan_spec const *chan,
@@ -522,7 +507,7 @@ static int apds9960_read_raw(struct iio_dev *indio_dev,
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		apds9960_set_power_state(data, true);
+		apds9960_power_on(data);
 		switch (chan->type) {
 		case IIO_PROXIMITY:
 			ret = regmap_read(data->regmap, chan->address, val);
@@ -540,7 +525,7 @@ static int apds9960_read_raw(struct iio_dev *indio_dev,
 		default:
 			ret = -EINVAL;
 		}
-		apds9960_set_power_state(data, false);
+		pm_runtime_put_autosuspend(&data->client->dev);
 		break;
 	case IIO_CHAN_INFO_INT_TIME:
 		/* RGB + ALS sensors only have integration time */
@@ -767,7 +752,6 @@ static int apds9960_write_event_config(struct iio_dev *indio_dev,
 		if (ret)
 			return ret;
 		data->pxs_int = state;
-		apds9960_set_power_state(data, state);
 		break;
 	case IIO_INTENSITY:
 		if (data->als_int == state)
@@ -777,12 +761,16 @@ static int apds9960_write_event_config(struct iio_dev *indio_dev,
 		if (ret)
 			return ret;
 		data->als_int = state;
-		apds9960_set_power_state(data, state);
 		break;
 	default:
 		return -EINVAL;
 	}
 
+	if (state)
+		apds9960_power_on(data);
+	else
+		pm_runtime_put_autosuspend(&data->client->dev);
+
 	return 0;
 }
 
@@ -1078,7 +1066,7 @@ static int apds9960_probe(struct i2c_client *client)
 	pm_runtime_set_autosuspend_delay(&client->dev, 5000);
 	pm_runtime_use_autosuspend(&client->dev);
 
-	apds9960_set_power_state(data, true);
+	apds9960_power_on(data);
 
 	ret = apds9960_regfield_init(data);
 	if (ret)
@@ -1105,12 +1093,12 @@ static int apds9960_probe(struct i2c_client *client)
 	if (ret)
 		goto error_power_down;
 
-	apds9960_set_power_state(data, false);
+	pm_runtime_put_autosuspend(&client->dev);
 
 	return 0;
 
 error_power_down:
-	apds9960_set_power_state(data, false);
+	pm_runtime_put_autosuspend(&client->dev);
 
 	return ret;
 }
-- 
2.56.0.218.g1ec904b173-goog

[PATCH v2 14/17] iio: light: pa12203001: Discard pm_runtime_put_autosuspend() return values

[Brian Norris]

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.

For this reason, modify this driver to simply discard the
pm_runtime_put_autosuspend() return value, and inline some of the
runtime PM helper function, now that it behaves the same with and
without CONFIG_PM.

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

Changes in v2:
 * Split iio patch into per-driver patches
 * Inlining

 drivers/iio/light/pa12203001.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/light/pa12203001.c b/drivers/iio/light/pa12203001.c
index c0f4db8d95f7..0a0502a60a06 100644
--- a/drivers/iio/light/pa12203001.c
+++ b/drivers/iio/light/pa12203001.c
@@ -154,13 +154,12 @@ static inline int pa12203001_px_enable(struct pa12203001_data *data, u8 enable)
 	return 0;
 }
 
-static int pa12203001_set_power_state(struct pa12203001_data *data, bool on,
-				      u8 mask)
+static int pa12203001_runtime_resume_and_get(struct pa12203001_data *data, u8 mask)
 {
 #ifdef CONFIG_PM
 	int ret;
 
-	if (on && (mask & PA12203001_ALS_EN_MASK)) {
+	if (mask & PA12203001_ALS_EN_MASK) {
 		mutex_lock(&data->lock);
 		if (data->px_enabled) {
 			ret = pa12203001_als_enable(data,
@@ -173,7 +172,7 @@ static int pa12203001_set_power_state(struct pa12203001_data *data, bool on,
 		mutex_unlock(&data->lock);
 	}
 
-	if (on && (mask & PA12203001_PX_EN_MASK)) {
+	if (mask & PA12203001_PX_EN_MASK) {
 		mutex_lock(&data->lock);
 		if (data->als_enabled) {
 			ret = pa12203001_px_enable(data, PA12203001_PX_EN_MASK);
@@ -185,10 +184,7 @@ static int pa12203001_set_power_state(struct pa12203001_data *data, bool on,
 		mutex_unlock(&data->lock);
 	}
 
-	if (on)
-		return pm_runtime_resume_and_get(&data->client->dev);
-
-	return pm_runtime_put_autosuspend(&data->client->dev);
+	return pm_runtime_resume_and_get(&data->client->dev);
 
 err:
 	mutex_unlock(&data->lock);
@@ -213,7 +209,7 @@ static int pa12203001_read_raw(struct iio_dev *indio_dev,
 		switch (chan->type) {
 		case IIO_LIGHT:
 			dev_mask = PA12203001_ALS_EN_MASK;
-			ret = pa12203001_set_power_state(data, true, dev_mask);
+			ret = pa12203001_runtime_resume_and_get(data, dev_mask);
 			if (ret < 0)
 				return ret;
 			/*
@@ -226,13 +222,11 @@ static int pa12203001_read_raw(struct iio_dev *indio_dev,
 				goto reg_err;
 
 			*val = le16_to_cpu(reg_word);
-			ret = pa12203001_set_power_state(data, false, dev_mask);
-			if (ret < 0)
-				return ret;
+			pm_runtime_put_autosuspend(&data->client->dev);
 			break;
 		case IIO_PROXIMITY:
 			dev_mask = PA12203001_PX_EN_MASK;
-			ret = pa12203001_set_power_state(data, true, dev_mask);
+			ret = pa12203001_runtime_resume_and_get(data, dev_mask);
 			if (ret < 0)
 				return ret;
 			ret = regmap_read(data->map, PA12203001_REG_PDH,
@@ -241,9 +235,7 @@ static int pa12203001_read_raw(struct iio_dev *indio_dev,
 				goto reg_err;
 
 			*val = reg_byte;
-			ret = pa12203001_set_power_state(data, false, dev_mask);
-			if (ret < 0)
-				return ret;
+			pm_runtime_put_autosuspend(&data->client->dev);
 			break;
 		default:
 			return -EINVAL;
@@ -262,7 +254,7 @@ static int pa12203001_read_raw(struct iio_dev *indio_dev,
 	}
 
 reg_err:
-	pa12203001_set_power_state(data, false, dev_mask);
+	pm_runtime_put_autosuspend(&data->client->dev);
 	return ret;
 }
 
-- 
2.56.0.218.g1ec904b173-goog

[PATCH v2 15/17] iio: light: rpr0521: Discard pm_runtime_put_autosuspend() return values

[Brian Norris]

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.

For this reason, modify this driver to simply discard the
pm_runtime_put_autosuspend() 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>
---

Changes in v2:
 * Split iio patch into per-driver patches
 * Simplify some conditional logic

 drivers/iio/light/rpr0521.c | 57 ++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 33 deletions(-)

diff --git a/drivers/iio/light/rpr0521.c b/drivers/iio/light/rpr0521.c
index 7d82efc31f3a..6305d7a9c282 100644
--- a/drivers/iio/light/rpr0521.c
+++ b/drivers/iio/light/rpr0521.c
@@ -357,32 +357,31 @@ static int rpr0521_set_power_state(struct rpr0521_data *data, bool on,
 	 * Note: If either measurement is re-enabled before _suspend(),
 	 * both stay enabled until _suspend().
 	 */
-	if (on)
-		ret = pm_runtime_resume_and_get(&data->client->dev);
-	else
-		ret = pm_runtime_put_autosuspend(&data->client->dev);
+	if (!on) {
+		pm_runtime_put_autosuspend(&data->client->dev);
+
+		return 0;
+	}
+
+	ret = pm_runtime_resume_and_get(&data->client->dev);
 	if (ret < 0) {
-		dev_err(&data->client->dev,
-			"Failed: rpr0521_set_power_state for %d, ret %d\n",
-			on, ret);
+		dev_err(&data->client->dev, "Failed to resume: %d\n", ret);
 		return ret;
 	}
 
-	if (on) {
-		/* If _resume() was not called, enable measurement now. */
-		if (data->als_ps_need_en) {
-			ret = rpr0521_als_enable(data, RPR0521_MODE_ALS_ENABLE);
-			if (ret)
-				return ret;
-			data->als_ps_need_en = false;
-		}
+	/* If _resume() was not called, enable measurement now. */
+	if (data->als_ps_need_en) {
+		ret = rpr0521_als_enable(data, RPR0521_MODE_ALS_ENABLE);
+		if (ret)
+			return ret;
+		data->als_ps_need_en = false;
+	}
 
-		if (data->pxs_ps_need_en) {
-			ret = rpr0521_pxs_enable(data, RPR0521_MODE_PXS_ENABLE);
-			if (ret)
-				return ret;
-			data->pxs_ps_need_en = false;
-		}
+	if (data->pxs_ps_need_en) {
+		ret = rpr0521_pxs_enable(data, RPR0521_MODE_PXS_ENABLE);
+		if (ret)
+			return ret;
+		data->pxs_ps_need_en = false;
 	}
 #endif
 	return 0;
@@ -545,17 +544,14 @@ static int rpr0521_buffer_preenable(struct iio_dev *indio_dev)
 
 static int rpr0521_buffer_postdisable(struct iio_dev *indio_dev)
 {
-	int err;
 	struct rpr0521_data *data = iio_priv(indio_dev);
 
 	mutex_lock(&data->lock);
-	err = rpr0521_set_power_state(data, false,
-		(RPR0521_MODE_PXS_MASK | RPR0521_MODE_ALS_MASK));
+	rpr0521_set_power_state(data, false,
+				RPR0521_MODE_PXS_MASK | RPR0521_MODE_ALS_MASK);
 	mutex_unlock(&data->lock);
-	if (err)
-		dev_err(&data->client->dev, "_buffer_postdisable fail\n");
 
-	return err;
+	return 0;
 }
 
 static const struct iio_buffer_setup_ops rpr0521_buffer_setup_ops = {
@@ -718,12 +714,7 @@ static int rpr0521_read_info_raw(struct rpr0521_data *data,
 	ret = regmap_bulk_read(data->regmap,
 			       rpr0521_data_reg[chan->address].address,
 			       &raw_data, sizeof(raw_data));
-	if (ret < 0) {
-		rpr0521_set_power_state(data, false, device_mask);
-		return ret;
-	}
-
-	ret = rpr0521_set_power_state(data, false, device_mask);
+	rpr0521_set_power_state(data, false, device_mask);
 	if (ret < 0)
 		return ret;
 
-- 
2.56.0.218.g1ec904b173-goog

[PATCH v2 16/17] iio: light: tsl2583: Discard pm_runtime_put_autosuspend() return values

[Brian Norris]

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.

For this reason, modify this driver to simply discard the
pm_runtime_put_autosuspend() 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>
---

Changes in v2:
 * Split iio patch into per-driver patches

 drivers/iio/light/tsl2583.c | 27 ++++-----------------------
 1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/drivers/iio/light/tsl2583.c b/drivers/iio/light/tsl2583.c
index 2d041b7530e6..a5b2b7fb6dd9 100644
--- a/drivers/iio/light/tsl2583.c
+++ b/drivers/iio/light/tsl2583.c
@@ -647,7 +647,7 @@ static int tsl2583_read_raw(struct iio_dev *indio_dev,
 			    int *val, int *val2, long mask)
 {
 	struct tsl2583_chip *chip = iio_priv(indio_dev);
-	int ret, pm_ret;
+	int ret;
 
 	ret = pm_runtime_resume_and_get(&chip->client->dev);
 	if (ret < 0)
@@ -716,19 +716,7 @@ static int tsl2583_read_raw(struct iio_dev *indio_dev,
 read_done:
 	mutex_unlock(&chip->als_mutex);
 
-	if (ret < 0) {
-		pm_runtime_put_autosuspend(&chip->client->dev);
-		return ret;
-	}
-
-	/*
-	 * Preserve the ret variable if the call to
-	 * pm_runtime_put_autosuspend() is successful so the reading
-	 * (if applicable) is returned to user space.
-	 */
-	pm_ret = pm_runtime_put_autosuspend(&chip->client->dev);
-	if (pm_ret < 0)
-		return pm_ret;
+	pm_runtime_put_autosuspend(&chip->client->dev);
 
 	return ret;
 }
@@ -780,16 +768,9 @@ static int tsl2583_write_raw(struct iio_dev *indio_dev,
 
 	mutex_unlock(&chip->als_mutex);
 
-	if (ret < 0) {
-		pm_runtime_put_autosuspend(&chip->client->dev);
-		return ret;
-	}
+	pm_runtime_put_autosuspend(&chip->client->dev);
 
-	ret = pm_runtime_put_autosuspend(&chip->client->dev);
-	if (ret < 0)
-		return ret;
-
-	return 0;
+	return ret;
 }
 
 static const struct iio_info tsl2583_info = {
-- 
2.56.0.218.g1ec904b173-goog

[PATCH v2 17/17] iio: light: us5182d: Discard pm_runtime_put_autosuspend() return values

[Brian Norris]

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.

For this reason, modify this driver to simply discard the
pm_runtime_put_autosuspend() 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>
---

Changes in v2:
 * Split iio patch into per-driver patches

 drivers/iio/light/us5182d.c | 32 ++++++++++----------------------
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/iio/light/us5182d.c b/drivers/iio/light/us5182d.c
index ab518311fd79..98279bd1ab25 100644
--- a/drivers/iio/light/us5182d.c
+++ b/drivers/iio/light/us5182d.c
@@ -366,45 +366,36 @@ static int us5182d_set_power_state(struct us5182d_data *data, bool on)
 	if (on)
 		return pm_runtime_resume_and_get(&data->client->dev);
 
-	return pm_runtime_put_autosuspend(&data->client->dev);
+	pm_runtime_put_autosuspend(&data->client->dev);
+
+	return 0;
 }
 
 static int us5182d_read_value(struct us5182d_data *data,
 			      struct iio_chan_spec const *chan)
 {
-	int ret, value;
+	int ret;
 
 	mutex_lock(&data->lock);
 
 	if (data->power_mode == US5182D_ONESHOT) {
 		ret = us5182d_oneshot_en(data);
 		if (ret < 0)
-			goto out_err;
+			goto out;
 	}
 
 	ret = us5182d_set_power_state(data, true);
 	if (ret < 0)
-		goto out_err;
+		goto out;
 
 	if (chan->type == IIO_LIGHT)
 		ret = us5182d_get_als(data);
 	else
 		ret = us5182d_get_px(data);
-	if (ret < 0)
-		goto out_poweroff;
-
-	value = ret;
 
-	ret = us5182d_set_power_state(data, false);
-	if (ret < 0)
-		goto out_err;
-
-	mutex_unlock(&data->lock);
-	return value;
-
-out_poweroff:
 	us5182d_set_power_state(data, false);
-out_err:
+
+out:
 	mutex_unlock(&data->lock);
 	return ret;
 }
@@ -676,11 +667,8 @@ static int us5182d_write_event_config(struct iio_dev *indio_dev,
 		goto err;
 	}
 
-	if (!state) {
-		ret = us5182d_set_power_state(data, false);
-		if (ret < 0)
-			goto err;
-	}
+	if (!state)
+		us5182d_set_power_state(data, false);
 
 	if (!data->falling_en && !data->rising_en && !data->default_continuous)
 		data->power_mode = US5182D_ONESHOT;
-- 
2.56.0.218.g1ec904b173-goog

Re: [PATCH v2 00/17] iio: Stop looking at pm_runtime_put_autosuspend() return codes

[Andy Shevchenko]

On Wed, Oct 21, 2026 at 04:57:01PM -0700, Brian Norris wrote:
> Similar to this effort:
> 
>   Subject: [PATCH v1 00/23] PM: runtime: Convert pm_runtime_put() to a void function
>   https://lore.kernel.org/all/6245770.lOV4Wx5bFT@rafael.j.wysocki/
> 
> i.e., commit 3afd8df02433 ("PM: runtime: Change pm_runtime_put() return
> type to void") and friends.
> 
> As with pm_runtime_put(), there is very little value in return-code
> checking for pm_runtime_put_autosuspend().
> 
> Checking the return code is more likely to produce a race condition
> (fighting with user space writing "on" to the "control" attribute) than
> to tell a consumer useful info.
> 
> Because the IIO subsystem has the bulk of drivers that inspect
> pm_runtime_put_autosuspend(), and because those drivers have a lot of
> other issues touching the same code (which Jonathan suggested [1] I
> fix -- I fix some, but not all), I'm sending an IIO-specific series
> under a v2 label (since many of the changes are still borrowed from the
> earlier series).

After three patches I stopped because it seems more works needs in each of
the individual drivers. Yes, the series can be applied as is in exchange for
a bigger additional churn in the future. My preference is to see everything
settled down in a single step.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 00/17] iio: Stop looking at pm_runtime_put_autosuspend() return codes

[Brian Norris]

On Thu, Oct 22, 2026 at 10:01:14AM +0300, Andy Shevchenko wrote:
> After three patches I stopped because it seems more works needs in each of
> the individual drivers. Yes, the series can be applied as is in exchange for
> a bigger additional churn in the future. My preference is to see everything
> settled down in a single step.

There are limits in how much work I'm willing to do on a bunch of
drivers I don't use, can't test, and frankly have no actual interest in.
I'm sure if I spent a few more minutes on each driver, I could find
another dozen things to try to improve.

I can go back to the v1 approach (fix only what I came to do) if you'd
prefer.

Or, I'd prefer to stick with the sentiment Jonathan expressed in
optionally requesting additional refactoring:
https://lore.kernel.org/all/20261018211807.66bfea0d@jic23-hlaptop/
that most of these changes are optional, and that fixing 1 thing is not
a subscription to fixing everything.

Brian