[PATCH togreg 0/2] iio: imu: inv_icm42607: fix PM error handling

[PATCH togreg 0/2] iio: imu: inv_icm42607: fix PM error handling

[Linmao Li]

The recently queued ICM-42607 PM support has two error paths that can leave
the PM core's state inconsistent with the device.

Patch 1 propagates sensor shutdown failures from runtime suspend, matching
the behavior of the sibling ICM-42600 driver. Patch 2 ensures that system
resume restores runtime PM management on both of its error paths, so that
a failed resume does not leave runtime PM disabled for good.

The Fixes commit is in iio.git togreg and has been included in
next-20260804. It has not reached mainline.

Based on iio.git togreg at 0efaefce4e95.

Both patches were compile-tested with W=1 and checked with smatch. No
ICM-42607 hardware or fault-injection setup was available.

Linmao Li (2):
  iio: imu: inv_icm42607: propagate runtime suspend errors
  iio: imu: inv_icm42607: restore runtime PM on system resume errors

 .../iio/imu/inv_icm42607/inv_icm42607_core.c  | 36 ++++++++++++-------
 1 file changed, 24 insertions(+), 12 deletions(-)


base-commit: 0efaefce4e95a3331550329c0078b2fb38b3ff1f
-- 
2.25.1

[PATCH togreg 1/2] iio: imu: inv_icm42607: propagate runtime suspend errors

[Linmao Li]

The runtime suspend callback always returns success even when updating
PWR_MGMT0 fails. The PM core can then mark the device suspended while one
or both sensors remain enabled.

The sibling ICM-42600 driver propagates the corresponding
inv_icm42600_set_pwr_mgmt0() failure from its runtime suspend callback.
Make ICM-42607 follow the same behavior by returning the sensor shutdown
error. Keep a void wrapper for the managed teardown action, where errors
can only be logged.

Fixes: 3007c1530f96 ("iio: imu: inv_icm42607: Add PM support for icm42607")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
Based on iio.git togreg at 0efaefce4e95.

 drivers/iio/imu/inv_icm42607/inv_icm42607_core.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
index 190e998f7b8ef..0da362967f63b 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
@@ -537,9 +537,8 @@ static int inv_icm42607_enable_vddio_reg(struct inv_icm42607_state *st)
 	return 0;
 }
 
-static void inv_icm42607_sensors_off(void *_data)
+static int inv_icm42607_sensors_off(struct inv_icm42607_state *st)
 {
-	struct inv_icm42607_state *st = _data;
 	const struct device *dev = regmap_get_device(st->map);
 	int ret;
 
@@ -552,6 +551,13 @@ static void inv_icm42607_sensors_off(void *_data)
 					 st->conf.accel.mode);
 	if (ret)
 		dev_err(dev, "Unable to turn off sensors\n");
+
+	return ret;
+}
+
+static void inv_icm42607_sensors_off_action(void *data)
+{
+	inv_icm42607_sensors_off(data);
 }
 
 static void inv_icm42607_disable_vddio_reg(void *_data)
@@ -619,7 +625,7 @@ int inv_icm42607_core_probe(struct regmap *regmap,
 	 * Ensure if sensors get turned on at some point, they're turned off
 	 * as part of teardown.
 	 */
-	ret = devm_add_action_or_reset(dev, inv_icm42607_sensors_off, st);
+	ret = devm_add_action_or_reset(dev, inv_icm42607_sensors_off_action, st);
 	if (ret)
 		return ret;
 
@@ -688,8 +694,7 @@ static int inv_icm42607_runtime_suspend(struct device *dev)
 	 * however the tradeoff is that an unused sensor won't be
 	 * turned off until the entire chip is no longer in use.
 	 */
-	inv_icm42607_sensors_off(st);
-	return 0;
+	return inv_icm42607_sensors_off(st);
 }
 
 EXPORT_NS_GPL_DEV_PM_OPS(inv_icm42607_pm_ops, IIO_ICM42607) = {
-- 
2.25.1

[PATCH togreg 2/2] iio: imu: inv_icm42607: restore runtime PM on system resume errors

[Linmao Li]

pm_runtime_force_suspend() leaves runtime PM disabled after it succeeds and
expects pm_runtime_force_resume() to restore runtime PM management during
system resume.

The resume callback returns early if enabling the vddio regulator or
synchronizing the register cache fails, skipping the matching
pm_runtime_force_resume() call. Runtime PM consequently remains disabled
after the system has resumed, so runtime autosuspend can no longer turn off
sensors enabled afterward.

Call pm_runtime_force_resume() on both error paths. Keep the first error as
the return value and report a runtime PM restore failure separately.

Fixes: 3007c1530f96 ("iio: imu: inv_icm42607: Add PM support for icm42607")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
Based on iio.git togreg at 0efaefce4e95.

 .../iio/imu/inv_icm42607/inv_icm42607_core.c  | 21 ++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
index 0da362967f63b..5384596dd8d79 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
@@ -667,19 +667,26 @@ static int inv_icm42607_suspend(struct device *dev)
 static int inv_icm42607_resume(struct device *dev)
 {
 	struct inv_icm42607_state *st = dev_get_drvdata(dev);
+	int resume_ret;
 	int ret;
 
 	ret = inv_icm42607_enable_vddio_reg(st);
-	if (ret)
-		return ret;
+	if (!ret) {
+		/* Sync the regcache again after regulator shutdown. */
+		regcache_mark_dirty(st->map);
+		ret = regcache_sync(st->map);
+	}
+
+	resume_ret = pm_runtime_force_resume(dev);
+	if (ret) {
+		if (resume_ret)
+			dev_warn(dev, "Failed to restore runtime PM state: %d\n",
+				 resume_ret);
 
-	/* Sync the regcache again after regulator shutdown. */
-	regcache_mark_dirty(st->map);
-	ret = regcache_sync(st->map);
-	if (ret)
 		return ret;
+	}
 
-	return pm_runtime_force_resume(dev);
+	return resume_ret;
 }
 
 static int inv_icm42607_runtime_suspend(struct device *dev)
-- 
2.25.1

Re: [PATCH togreg 2/2] iio: imu: inv_icm42607: restore runtime PM on system resume errors

[Andy Shevchenko]

On Wed, Aug 05, 2026 at 11:54:51AM +0800, Linmao Li wrote:
> pm_runtime_force_suspend() leaves runtime PM disabled after it succeeds and
> expects pm_runtime_force_resume() to restore runtime PM management during
> system resume.
> 
> The resume callback returns early if enabling the vddio regulator or
> synchronizing the register cache fails, skipping the matching
> pm_runtime_force_resume() call. Runtime PM consequently remains disabled
> after the system has resumed, so runtime autosuspend can no longer turn off
> sensors enabled afterward.
> 
> Call pm_runtime_force_resume() on both error paths. Keep the first error as
> the return value and report a runtime PM restore failure separately.

...

>  static int inv_icm42607_resume(struct device *dev)
>  {
>  	struct inv_icm42607_state *st = dev_get_drvdata(dev);
> +	int resume_ret;
>  	int ret;
>  
>  	ret = inv_icm42607_enable_vddio_reg(st);
> -	if (ret)
> -		return ret;
> +	if (!ret) {

This usually hints that the function needs to be refactored by splitting out
the wrapper that does PM and the core (current function).

> +		/* Sync the regcache again after regulator shutdown. */
> +		regcache_mark_dirty(st->map);
> +		ret = regcache_sync(st->map);
> +	}
> +
> +	resume_ret = pm_runtime_force_resume(dev);
> +	if (ret) {
> +		if (resume_ret)
> +			dev_warn(dev, "Failed to restore runtime PM state: %d\n",
> +				 resume_ret);
>  
> -	/* Sync the regcache again after regulator shutdown. */
> -	regcache_mark_dirty(st->map);
> -	ret = regcache_sync(st->map);
> -	if (ret)
>  		return ret;
> +	}
>  
> -	return pm_runtime_force_resume(dev);
> +	return resume_ret;
>  }

-- 
With Best Regards,
Andy Shevchenko

[PATCH togreg v2 0/2] iio: imu: inv_icm42607: fix PM error handling

[Linmao Li]

The recently queued ICM-42607 PM support has two error paths that can leave
the PM core's state inconsistent with the device.

Patch 1 propagates sensor shutdown failures from runtime suspend, matching
the behavior of the sibling ICM-42600 driver. Patch 2 ensures that system
resume restores runtime PM management on both of its error paths, so that
a failed resume does not leave runtime PM disabled for good.

Changes since v1:
- Patch 2: split the device side of inv_icm42607_resume() into a helper so
  the PM bookkeeping stays in the wrapper, per Andy's review.  No
  functional change.
- Rebased onto the current togreg head.
- Patch 1 is unchanged.

The Fixes commit is in iio.git togreg and has been included in
linux-next. It has not reached mainline.

Based on iio.git togreg at 350d1fb9204b.

Both patches were compile-tested with W=1 and checked with smatch. No
ICM-42607 hardware or fault-injection setup was available.

Linmao Li (2):
  iio: imu: inv_icm42607: propagate runtime suspend errors
  iio: imu: inv_icm42607: restore runtime PM on system resume errors

 .../iio/imu/inv_icm42607/inv_icm42607_core.c  | 41 ++++++++++++++-----
 1 file changed, 31 insertions(+), 10 deletions(-)


base-commit: 350d1fb9204b13c5f95e511e98b8bcb47574d425
-- 
2.25.1

[PATCH togreg v2 1/2] iio: imu: inv_icm42607: propagate runtime suspend errors

[Linmao Li]

The runtime suspend callback always returns success even when updating
PWR_MGMT0 fails. The PM core can then mark the device suspended while one
or both sensors remain enabled.

The sibling ICM-42600 driver propagates the corresponding
inv_icm42600_set_pwr_mgmt0() failure from its runtime suspend callback.
Make ICM-42607 follow the same behavior by returning the sensor shutdown
error. Keep a void wrapper for the managed teardown action, where errors
can only be logged.

Fixes: 3007c1530f96 ("iio: imu: inv_icm42607: Add PM support for icm42607")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
Unchanged since v1.

Based on iio.git togreg at 350d1fb9204b.

 drivers/iio/imu/inv_icm42607/inv_icm42607_core.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
index 190e998f7b8ef..0da362967f63b 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
@@ -537,9 +537,8 @@ static int inv_icm42607_enable_vddio_reg(struct inv_icm42607_state *st)
 	return 0;
 }
 
-static void inv_icm42607_sensors_off(void *_data)
+static int inv_icm42607_sensors_off(struct inv_icm42607_state *st)
 {
-	struct inv_icm42607_state *st = _data;
 	const struct device *dev = regmap_get_device(st->map);
 	int ret;
 
@@ -552,6 +551,13 @@ static void inv_icm42607_sensors_off(void *_data)
 					 st->conf.accel.mode);
 	if (ret)
 		dev_err(dev, "Unable to turn off sensors\n");
+
+	return ret;
+}
+
+static void inv_icm42607_sensors_off_action(void *data)
+{
+	inv_icm42607_sensors_off(data);
 }
 
 static void inv_icm42607_disable_vddio_reg(void *_data)
@@ -619,7 +625,7 @@ int inv_icm42607_core_probe(struct regmap *regmap,
 	 * Ensure if sensors get turned on at some point, they're turned off
 	 * as part of teardown.
 	 */
-	ret = devm_add_action_or_reset(dev, inv_icm42607_sensors_off, st);
+	ret = devm_add_action_or_reset(dev, inv_icm42607_sensors_off_action, st);
 	if (ret)
 		return ret;
 
@@ -688,8 +694,7 @@ static int inv_icm42607_runtime_suspend(struct device *dev)
 	 * however the tradeoff is that an unused sensor won't be
 	 * turned off until the entire chip is no longer in use.
 	 */
-	inv_icm42607_sensors_off(st);
-	return 0;
+	return inv_icm42607_sensors_off(st);
 }
 
 EXPORT_NS_GPL_DEV_PM_OPS(inv_icm42607_pm_ops, IIO_ICM42607) = {
-- 
2.25.1

[PATCH togreg v2 2/2] iio: imu: inv_icm42607: restore runtime PM on system resume errors

[Linmao Li]

pm_runtime_force_suspend() leaves runtime PM disabled after it succeeds and
expects pm_runtime_force_resume() to restore runtime PM management during
system resume.

The resume callback returns early if enabling the vddio regulator or
synchronizing the register cache fails, skipping the matching
pm_runtime_force_resume() call. Runtime PM consequently remains disabled
after the system has resumed, so runtime autosuspend can no longer turn off
sensors enabled afterward.

Call pm_runtime_force_resume() on both error paths. Keep the first error as
the return value and report a runtime PM restore failure separately.

Fixes: 3007c1530f96 ("iio: imu: inv_icm42607: Add PM support for icm42607")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
Changes since v1:
- Split the device side of inv_icm42607_resume() into a helper so the
  PM bookkeeping stays in the wrapper, per Andy's review.  No
  functional change.

Based on iio.git togreg at 350d1fb9204b.

 .../iio/imu/inv_icm42607/inv_icm42607_core.c  | 26 +++++++++++++++----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
index 0da362967f63b..6577f003d746e 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
@@ -664,9 +664,8 @@ static int inv_icm42607_suspend(struct device *dev)
 	return 0;
 }
 
-static int inv_icm42607_resume(struct device *dev)
+static int inv_icm42607_resume_core(struct inv_icm42607_state *st)
 {
-	struct inv_icm42607_state *st = dev_get_drvdata(dev);
 	int ret;
 
 	ret = inv_icm42607_enable_vddio_reg(st);
@@ -675,11 +674,28 @@ static int inv_icm42607_resume(struct device *dev)
 
 	/* Sync the regcache again after regulator shutdown. */
 	regcache_mark_dirty(st->map);
-	ret = regcache_sync(st->map);
-	if (ret)
+
+	return regcache_sync(st->map);
+}
+
+static int inv_icm42607_resume(struct device *dev)
+{
+	struct inv_icm42607_state *st = dev_get_drvdata(dev);
+	int resume_ret;
+	int ret;
+
+	ret = inv_icm42607_resume_core(st);
+
+	resume_ret = pm_runtime_force_resume(dev);
+	if (ret) {
+		if (resume_ret)
+			dev_warn(dev, "Failed to restore runtime PM state: %d\n",
+				 resume_ret);
+
 		return ret;
+	}
 
-	return pm_runtime_force_resume(dev);
+	return resume_ret;
 }
 
 static int inv_icm42607_runtime_suspend(struct device *dev)
-- 
2.25.1

Re: [PATCH togreg v2 2/2] iio: imu: inv_icm42607: restore runtime PM on system resume errors

[Andy Shevchenko]

On Tue, Aug 11, 2026 at 10:03:45AM +0800, Linmao Li wrote:
> pm_runtime_force_suspend() leaves runtime PM disabled after it succeeds and
> expects pm_runtime_force_resume() to restore runtime PM management during
> system resume.
> 
> The resume callback returns early if enabling the vddio regulator or
> synchronizing the register cache fails, skipping the matching
> pm_runtime_force_resume() call. Runtime PM consequently remains disabled
> after the system has resumed, so runtime autosuspend can no longer turn off
> sensors enabled afterward.
> 
> Call pm_runtime_force_resume() on both error paths. Keep the first error as
> the return value and report a runtime PM restore failure separately.

...

>  	/* Sync the regcache again after regulator shutdown. */
>  	regcache_mark_dirty(st->map);
> -	ret = regcache_sync(st->map);
> -	if (ret)

> +

I wouldn't add this blank line as these two are quite coupled. OTOH it's a
better style, so I leave it to Jonathan and others to decide.

> +	return regcache_sync(st->map);
> +}

...

> +static int inv_icm42607_resume(struct device *dev)
> +{
> +	struct inv_icm42607_state *st = dev_get_drvdata(dev);
> +	int resume_ret;
> +	int ret;
> +
> +	ret = inv_icm42607_resume_core(st);
> +
> +	resume_ret = pm_runtime_force_resume(dev);
> +	if (ret) {

I still don't get the logic here. Shouldn't we rather call the force_suspend()
last in the .suspend() and force_resume() first here?

> +		if (resume_ret)
> +			dev_warn(dev, "Failed to restore runtime PM state: %d\n",
> +				 resume_ret);
> +
>  		return ret;
> +	}
>  
> -	return pm_runtime_force_resume(dev);
> +	return resume_ret;
>  }

Okay, after reading other drivers I think the above can be written in a bit better form.

	struct inv_icm42607_state *st = dev_get_drvdata(dev);
	int ret;

	ret = inv_icm42607_resume_core(st);
	if (ret) {
		int rc;

		rc = pm_runtime_force_resume(dev);
		if (rc)
			dev_warn(dev, "Failed to restore runtime PM state: %d\n", rc);

// and yes, I would go with longer line here (that's why I renamed variable)

		return ret;
	}

	return pm_runtime_force_resume(dev);

It seems that if the above analysis is true (I haven't deeply checked that),
there are many drivers in the kernel suffer from the very same issue (when they
need to do some work before runtime PM resume may be called from inside the
system .resume() callback.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH togreg v2 2/2] iio: imu: inv_icm42607: restore runtime PM on system resume errors

[Linmao Li]

在 2026/8/11 16:54, Andy Shevchenko 写道:
> On Tue, Aug 11, 2026 at 10:03:45AM +0800, Linmao Li wrote:
>> pm_runtime_force_suspend() leaves runtime PM disabled after it succeeds and
>> expects pm_runtime_force_resume() to restore runtime PM management during
>> system resume.
>>
>> The resume callback returns early if enabling the vddio regulator or
>> synchronizing the register cache fails, skipping the matching
>> pm_runtime_force_resume() call. Runtime PM consequently remains disabled
>> after the system has resumed, so runtime autosuspend can no longer turn off
>> sensors enabled afterward.
>>
>> Call pm_runtime_force_resume() on both error paths. Keep the first error as
>> the return value and report a runtime PM restore failure separately.
> ...
>
>>   	/* Sync the regcache again after regulator shutdown. */
>>   	regcache_mark_dirty(st->map);
>> -	ret = regcache_sync(st->map);
>> -	if (ret)
>> +
> I wouldn't add this blank line as these two are quite coupled. OTOH it's a
> better style, so I leave it to Jonathan and others to decide.
>
>> +	return regcache_sync(st->map);
>> +}
> ...
>
>> +static int inv_icm42607_resume(struct device *dev)
>> +{
>> +	struct inv_icm42607_state *st = dev_get_drvdata(dev);
>> +	int resume_ret;
>> +	int ret;
>> +
>> +	ret = inv_icm42607_resume_core(st);
>> +
>> +	resume_ret = pm_runtime_force_resume(dev);
>> +	if (ret) {
> I still don't get the logic here. Shouldn't we rather call the force_suspend()
> last in the .suspend() and force_resume() first here?

pm_runtime_force_suspend() may invoke the .runtime_suspend callback,
which writes PWR_MGMT0 over the bus. It therefore has to run while
vddio is still enabled, before inv_icm42607_disable_vddio_reg().

On resume, vddio and the register cache need to be restored before
pm_runtime_force_resume() re-enables runtime PM. Otherwise runtime PM
could be enabled while the device-side resume has not completed.

This is the ordering already used by the driver; the series does not
change it.

>
>> +		if (resume_ret)
>> +			dev_warn(dev, "Failed to restore runtime PM state: %d\n",
>> +				 resume_ret);
>> +
>>   		return ret;
>> +	}
>>   
>> -	return pm_runtime_force_resume(dev);
>> +	return resume_ret;
>>   }
> Okay, after reading other drivers I think the above can be written in a bit better form.
Thanks, I have used this form in v3.
>
> 	struct inv_icm42607_state *st = dev_get_drvdata(dev);
> 	int ret;
>
> 	ret = inv_icm42607_resume_core(st);
> 	if (ret) {
> 		int rc;
>
> 		rc = pm_runtime_force_resume(dev);
> 		if (rc)
> 			dev_warn(dev, "Failed to restore runtime PM state: %d\n", rc);
>
> // and yes, I would go with longer line here (that's why I renamed variable)
>
> 		return ret;
> 	}
>
> 	return pm_runtime_force_resume(dev);
>
> It seems that if the above analysis is true (I haven't deeply checked that),
> there are many drivers in the kernel suffer from the very same issue (when they
> need to do some work before runtime PM resume may be called from inside the
> system .resume() callback.
>

Re: [PATCH togreg v2 0/2] iio: imu: inv_icm42607: fix PM error handling

[Andy Shevchenko]

On Tue, Aug 11, 2026 at 10:03:43AM +0800, Linmao Li wrote:
> The recently queued ICM-42607 PM support has two error paths that can leave
> the PM core's state inconsistent with the device.
> 
> Patch 1 propagates sensor shutdown failures from runtime suspend, matching
> the behavior of the sibling ICM-42600 driver. Patch 2 ensures that system
> resume restores runtime PM management on both of its error paths, so that
> a failed resume does not leave runtime PM disabled for good.
> 
> Changes since v1:
> - Patch 2: split the device side of inv_icm42607_resume() into a helper so
>   the PM bookkeeping stays in the wrapper, per Andy's review.  No
>   functional change.
> - Rebased onto the current togreg head.
> - Patch 1 is unchanged.
> 
> The Fixes commit is in iio.git togreg and has been included in
> linux-next. It has not reached mainline.

> Based on iio.git togreg at 350d1fb9204b.

Unneeded info since you are correctly used --base and we see that below.

> Both patches were compile-tested with W=1 and checked with smatch. No
> ICM-42607 hardware or fault-injection setup was available.
> 
> Linmao Li (2):
>   iio: imu: inv_icm42607: propagate runtime suspend errors
>   iio: imu: inv_icm42607: restore runtime PM on system resume errors
> 
>  .../iio/imu/inv_icm42607/inv_icm42607_core.c  | 41 ++++++++++++++-----
>  1 file changed, 31 insertions(+), 10 deletions(-)
> 
> 
> base-commit: 350d1fb9204b13c5f95e511e98b8bcb47574d425

-- 
With Best Regards,
Andy Shevchenko

[PATCH togreg v3 0/2] iio: imu: inv_icm42607: fix PM error handling

[Linmao Li]

The recently queued ICM-42607 PM support has two error paths that can leave
the PM core's state inconsistent with the device.

Patch 1 propagates sensor shutdown failures from runtime suspend, matching
the behavior of the sibling ICM-42600 driver. Patch 2 ensures that system
resume restores runtime PM management on both of its error paths, so that
a failed resume does not leave runtime PM disabled for good.

Changes since v2:
- Patch 2: restructure inv_icm42607_resume() along the lines Andy
  suggested.  No functional change.
- Dropped the redundant hand-written base note; --base already emits
  base-commit:.
- Patch 1 is unchanged.

Changes since v1:
- Patch 2: split the device side of inv_icm42607_resume() into a helper
  so the PM bookkeeping stays in the wrapper.  No functional change.
- Rebased onto the current togreg head.

On the ordering question from the v2 review: pm_runtime_force_suspend()
runs the .runtime_suspend callback, which writes PWR_MGMT0 over the bus,
so it has to happen while vddio is still enabled - that is, before
inv_icm42607_disable_vddio_reg() in .suspend().  .resume() then unwinds
in the opposite order, which is why pm_runtime_force_resume() comes last
there.  That ordering is what the driver already does; this series does
not change it.

The Fixes commit is in iio.git togreg and has been included in
linux-next. It has not reached mainline.

Both patches were compile-tested with W=1 and checked with smatch. No
ICM-42607 hardware or fault-injection setup was available.

Linmao Li (2):
  iio: imu: inv_icm42607: propagate runtime suspend errors
  iio: imu: inv_icm42607: restore runtime PM on system resume errors

 .../iio/imu/inv_icm42607/inv_icm42607_core.c  | 38 ++++++++++++++-----
 1 file changed, 29 insertions(+), 9 deletions(-)


base-commit: 350d1fb9204b13c5f95e511e98b8bcb47574d425
-- 
2.25.1

[PATCH togreg v3 1/2] iio: imu: inv_icm42607: propagate runtime suspend errors

[Linmao Li]

The runtime suspend callback always returns success even when updating
PWR_MGMT0 fails. The PM core can then mark the device suspended while one
or both sensors remain enabled.

The sibling ICM-42600 driver propagates the corresponding
inv_icm42600_set_pwr_mgmt0() failure from its runtime suspend callback.
Make ICM-42607 follow the same behavior by returning the sensor shutdown
error. Keep a void wrapper for the managed teardown action, where errors
can only be logged.

Fixes: 3007c1530f96 ("iio: imu: inv_icm42607: Add PM support for icm42607")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
Unchanged since v1.

 drivers/iio/imu/inv_icm42607/inv_icm42607_core.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
index 190e998f7b8ef..0da362967f63b 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
@@ -537,9 +537,8 @@ static int inv_icm42607_enable_vddio_reg(struct inv_icm42607_state *st)
 	return 0;
 }
 
-static void inv_icm42607_sensors_off(void *_data)
+static int inv_icm42607_sensors_off(struct inv_icm42607_state *st)
 {
-	struct inv_icm42607_state *st = _data;
 	const struct device *dev = regmap_get_device(st->map);
 	int ret;
 
@@ -552,6 +551,13 @@ static void inv_icm42607_sensors_off(void *_data)
 					 st->conf.accel.mode);
 	if (ret)
 		dev_err(dev, "Unable to turn off sensors\n");
+
+	return ret;
+}
+
+static void inv_icm42607_sensors_off_action(void *data)
+{
+	inv_icm42607_sensors_off(data);
 }
 
 static void inv_icm42607_disable_vddio_reg(void *_data)
@@ -619,7 +625,7 @@ int inv_icm42607_core_probe(struct regmap *regmap,
 	 * Ensure if sensors get turned on at some point, they're turned off
 	 * as part of teardown.
 	 */
-	ret = devm_add_action_or_reset(dev, inv_icm42607_sensors_off, st);
+	ret = devm_add_action_or_reset(dev, inv_icm42607_sensors_off_action, st);
 	if (ret)
 		return ret;
 
@@ -688,8 +694,7 @@ static int inv_icm42607_runtime_suspend(struct device *dev)
 	 * however the tradeoff is that an unused sensor won't be
 	 * turned off until the entire chip is no longer in use.
 	 */
-	inv_icm42607_sensors_off(st);
-	return 0;
+	return inv_icm42607_sensors_off(st);
 }
 
 EXPORT_NS_GPL_DEV_PM_OPS(inv_icm42607_pm_ops, IIO_ICM42607) = {
-- 
2.25.1

Re: [PATCH togreg v3 1/2] iio: imu: inv_icm42607: propagate runtime suspend errors

[Jonathan Cameron]

On Tue, 11 Aug 2026 18:33:00 +0800
Linmao Li <lilinmao@kylinos.cn> wrote:

> The runtime suspend callback always returns success even when updating
> PWR_MGMT0 fails. The PM core can then mark the device suspended while one
> or both sensors remain enabled.
> 
> The sibling ICM-42600 driver propagates the corresponding
> inv_icm42600_set_pwr_mgmt0() failure from its runtime suspend callback.
> Make ICM-42607 follow the same behavior by returning the sensor shutdown
> error. Keep a void wrapper for the managed teardown action, where errors
> can only be logged.

What is the practical affect of a sensor remaining enabled?  Bit of
power loss or something more significant?  This info matter when deciding
if we should rush this in during the rc phase, or wait for the next
merge window.

Jonathan


> 
> Fixes: 3007c1530f96 ("iio: imu: inv_icm42607: Add PM support for icm42607")
> Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
> ---
> Unchanged since v1.
> 
>  drivers/iio/imu/inv_icm42607/inv_icm42607_core.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> index 190e998f7b8ef..0da362967f63b 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> @@ -537,9 +537,8 @@ static int inv_icm42607_enable_vddio_reg(struct inv_icm42607_state *st)
>  	return 0;
>  }
>  
> -static void inv_icm42607_sensors_off(void *_data)
> +static int inv_icm42607_sensors_off(struct inv_icm42607_state *st)
>  {
> -	struct inv_icm42607_state *st = _data;
>  	const struct device *dev = regmap_get_device(st->map);
>  	int ret;
>  
> @@ -552,6 +551,13 @@ static void inv_icm42607_sensors_off(void *_data)
>  					 st->conf.accel.mode);
>  	if (ret)
>  		dev_err(dev, "Unable to turn off sensors\n");
> +
> +	return ret;
> +}
> +
> +static void inv_icm42607_sensors_off_action(void *data)
> +{
> +	inv_icm42607_sensors_off(data);
>  }
>  
>  static void inv_icm42607_disable_vddio_reg(void *_data)
> @@ -619,7 +625,7 @@ int inv_icm42607_core_probe(struct regmap *regmap,
>  	 * Ensure if sensors get turned on at some point, they're turned off
>  	 * as part of teardown.
>  	 */
> -	ret = devm_add_action_or_reset(dev, inv_icm42607_sensors_off, st);
> +	ret = devm_add_action_or_reset(dev, inv_icm42607_sensors_off_action, st);
>  	if (ret)
>  		return ret;
>  
> @@ -688,8 +694,7 @@ static int inv_icm42607_runtime_suspend(struct device *dev)
>  	 * however the tradeoff is that an unused sensor won't be
>  	 * turned off until the entire chip is no longer in use.
>  	 */
> -	inv_icm42607_sensors_off(st);
> -	return 0;
> +	return inv_icm42607_sensors_off(st);
>  }
>  
>  EXPORT_NS_GPL_DEV_PM_OPS(inv_icm42607_pm_ops, IIO_ICM42607) = {

Re: [PATCH togreg v3 1/2] iio: imu: inv_icm42607: propagate runtime suspend errors

[Linmao Li]

在 2026/8/16 4:52, Jonathan Cameron 写道:
> On Tue, 11 Aug 2026 18:33:00 +0800
> Linmao Li <lilinmao@kylinos.cn> wrote:
>
>> The runtime suspend callback always returns success even when updating
>> PWR_MGMT0 fails. The PM core can then mark the device suspended while one
>> or both sensors remain enabled.
>>
>> The sibling ICM-42600 driver propagates the corresponding
>> inv_icm42600_set_pwr_mgmt0() failure from its runtime suspend callback.
>> Make ICM-42607 follow the same behavior by returning the sensor shutdown
>> error. Keep a void wrapper for the managed teardown action, where errors
>> can only be logged.
> What is the practical affect of a sensor remaining enabled?  Bit of
> power loss or something more significant?  This info matter when deciding
> if we should rush this in during the rc phase, or wait for the next
> merge window.
As far as I can tell, the practical effect is additional power
consumption while the device is idle.  I found no corruption path, but I
have no ICM-42607 hardware to reproduce the failure or measure the
current.

It does not necessarily persist indefinitely.  If the PWR_MGMT0 write
fails, regmap may contain the requested OFF state while the hardware
remains ON.  A later sensor read requests an enabled mode, so
inv_icm42607_set_pwr_mgmt0() retries the write instead of taking its
"no change" return.  If the bus error was transient, the cache and
hardware are then resynchronized.

I also noticed a cost to this patch: returning an error such as -EIO from
.runtime_suspend() makes the PM core set power.runtime_error. Subsequent
reads then fail in PM_RUNTIME_ACQUIRE_AUTOSUSPEND() before any sensor
register is accessed, until the PM status is reset.  A successful system
suspend may clear that state.

The patch therefore trades a logged idle power leak that may recover on
a later access for a potentially sticky runtime-PM failure.  Since this
was found by inspection only, I would not rush it into the rc phase.  I
would rather revisit the error handling and target the next merge window.

Linmao

>
> Jonathan
>
>
>> Fixes: 3007c1530f96 ("iio: imu: inv_icm42607: Add PM support for icm42607")
>> Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
>> ---
>> Unchanged since v1.
>>
>>   drivers/iio/imu/inv_icm42607/inv_icm42607_core.c | 15 ++++++++++-----
>>   1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
>> index 190e998f7b8ef..0da362967f63b 100644
>> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
>> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
>> @@ -537,9 +537,8 @@ static int inv_icm42607_enable_vddio_reg(struct inv_icm42607_state *st)
>>   	return 0;
>>   }
>>   
>> -static void inv_icm42607_sensors_off(void *_data)
>> +static int inv_icm42607_sensors_off(struct inv_icm42607_state *st)
>>   {
>> -	struct inv_icm42607_state *st = _data;
>>   	const struct device *dev = regmap_get_device(st->map);
>>   	int ret;
>>   
>> @@ -552,6 +551,13 @@ static void inv_icm42607_sensors_off(void *_data)
>>   					 st->conf.accel.mode);
>>   	if (ret)
>>   		dev_err(dev, "Unable to turn off sensors\n");
>> +
>> +	return ret;
>> +}
>> +
>> +static void inv_icm42607_sensors_off_action(void *data)
>> +{
>> +	inv_icm42607_sensors_off(data);
>>   }
>>   
>>   static void inv_icm42607_disable_vddio_reg(void *_data)
>> @@ -619,7 +625,7 @@ int inv_icm42607_core_probe(struct regmap *regmap,
>>   	 * Ensure if sensors get turned on at some point, they're turned off
>>   	 * as part of teardown.
>>   	 */
>> -	ret = devm_add_action_or_reset(dev, inv_icm42607_sensors_off, st);
>> +	ret = devm_add_action_or_reset(dev, inv_icm42607_sensors_off_action, st);
>>   	if (ret)
>>   		return ret;
>>   
>> @@ -688,8 +694,7 @@ static int inv_icm42607_runtime_suspend(struct device *dev)
>>   	 * however the tradeoff is that an unused sensor won't be
>>   	 * turned off until the entire chip is no longer in use.
>>   	 */
>> -	inv_icm42607_sensors_off(st);
>> -	return 0;
>> +	return inv_icm42607_sensors_off(st);
>>   }
>>   
>>   EXPORT_NS_GPL_DEV_PM_OPS(inv_icm42607_pm_ops, IIO_ICM42607) = {

Re: [PATCH togreg v3 1/2] iio: imu: inv_icm42607: propagate runtime suspend errors

[Jonathan Cameron]

On Mon, 17 Aug 2026 17:32:43 +0800
Linmao Li <lilinmao@kylinos.cn> wrote:

> 在 2026/8/16 4:52, Jonathan Cameron 写道:
> > On Tue, 11 Aug 2026 18:33:00 +0800
> > Linmao Li <lilinmao@kylinos.cn> wrote:
> >  
> >> The runtime suspend callback always returns success even when updating
> >> PWR_MGMT0 fails. The PM core can then mark the device suspended while one
> >> or both sensors remain enabled.
> >>
> >> The sibling ICM-42600 driver propagates the corresponding
> >> inv_icm42600_set_pwr_mgmt0() failure from its runtime suspend callback.
> >> Make ICM-42607 follow the same behavior by returning the sensor shutdown
> >> error. Keep a void wrapper for the managed teardown action, where errors
> >> can only be logged.  
> > What is the practical affect of a sensor remaining enabled?  Bit of
> > power loss or something more significant?  This info matter when deciding
> > if we should rush this in during the rc phase, or wait for the next
> > merge window.  
> As far as I can tell, the practical effect is additional power
> consumption while the device is idle.  I found no corruption path, but I
> have no ICM-42607 hardware to reproduce the failure or measure the
> current.
> 
> It does not necessarily persist indefinitely.  If the PWR_MGMT0 write
> fails, regmap may contain the requested OFF state while the hardware
> remains ON.  A later sensor read requests an enabled mode, so
> inv_icm42607_set_pwr_mgmt0() retries the write instead of taking its
> "no change" return.  If the bus error was transient, the cache and
> hardware are then resynchronized.

Please capture some of that for the commit description for v2.

> 
> I also noticed a cost to this patch: returning an error such as -EIO from
> .runtime_suspend() makes the PM core set power.runtime_error. Subsequent
> reads then fail in PM_RUNTIME_ACQUIRE_AUTOSUSPEND() before any sensor
> register is accessed, until the PM status is reset.  A successful system
> suspend may clear that state.
> 
> The patch therefore trades a logged idle power leak that may recover on
> a later access for a potentially sticky runtime-PM failure.  Since this
> was found by inspection only, I would not rush it into the rc phase.  I
> would rather revisit the error handling and target the next merge window.
> 
That is fair enough.

J
> Linmao
> 
> >
> > Jonathan
> >
> >  
> >> Fixes: 3007c1530f96 ("iio: imu: inv_icm42607: Add PM support for icm42607")
> >> Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
> >> ---
> >> Unchanged since v1.
> >>
> >>   drivers/iio/imu/inv_icm42607/inv_icm42607_core.c | 15 ++++++++++-----
> >>   1 file changed, 10 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> >> index 190e998f7b8ef..0da362967f63b 100644
> >> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> >> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> >> @@ -537,9 +537,8 @@ static int inv_icm42607_enable_vddio_reg(struct inv_icm42607_state *st)
> >>   	return 0;
> >>   }
> >>   
> >> -static void inv_icm42607_sensors_off(void *_data)
> >> +static int inv_icm42607_sensors_off(struct inv_icm42607_state *st)
> >>   {
> >> -	struct inv_icm42607_state *st = _data;
> >>   	const struct device *dev = regmap_get_device(st->map);
> >>   	int ret;
> >>   
> >> @@ -552,6 +551,13 @@ static void inv_icm42607_sensors_off(void *_data)
> >>   					 st->conf.accel.mode);
> >>   	if (ret)
> >>   		dev_err(dev, "Unable to turn off sensors\n");
> >> +
> >> +	return ret;
> >> +}
> >> +
> >> +static void inv_icm42607_sensors_off_action(void *data)
> >> +{
> >> +	inv_icm42607_sensors_off(data);
> >>   }
> >>   
> >>   static void inv_icm42607_disable_vddio_reg(void *_data)
> >> @@ -619,7 +625,7 @@ int inv_icm42607_core_probe(struct regmap *regmap,
> >>   	 * Ensure if sensors get turned on at some point, they're turned off
> >>   	 * as part of teardown.
> >>   	 */
> >> -	ret = devm_add_action_or_reset(dev, inv_icm42607_sensors_off, st);
> >> +	ret = devm_add_action_or_reset(dev, inv_icm42607_sensors_off_action, st);
> >>   	if (ret)
> >>   		return ret;
> >>   
> >> @@ -688,8 +694,7 @@ static int inv_icm42607_runtime_suspend(struct device *dev)
> >>   	 * however the tradeoff is that an unused sensor won't be
> >>   	 * turned off until the entire chip is no longer in use.
> >>   	 */
> >> -	inv_icm42607_sensors_off(st);
> >> -	return 0;
> >> +	return inv_icm42607_sensors_off(st);
> >>   }
> >>   
> >>   EXPORT_NS_GPL_DEV_PM_OPS(inv_icm42607_pm_ops, IIO_ICM42607) = {  

Re: [PATCH togreg v3 1/2] iio: imu: inv_icm42607: propagate runtime suspend errors

[Linmao Li]

在 2026/8/24 8:20, Jonathan Cameron 写道:
> On Mon, 17 Aug 2026 17:32:43 +0800
> Linmao Li <lilinmao@kylinos.cn> wrote:
>
>> 在 2026/8/16 4:52, Jonathan Cameron 写道:
>>> On Tue, 11 Aug 2026 18:33:00 +0800
>>> Linmao Li <lilinmao@kylinos.cn> wrote:
>>>   
>>>> The runtime suspend callback always returns success even when updating
>>>> PWR_MGMT0 fails. The PM core can then mark the device suspended while one
>>>> or both sensors remain enabled.
>>>>
>>>> The sibling ICM-42600 driver propagates the corresponding
>>>> inv_icm42600_set_pwr_mgmt0() failure from its runtime suspend callback.
>>>> Make ICM-42607 follow the same behavior by returning the sensor shutdown
>>>> error. Keep a void wrapper for the managed teardown action, where errors
>>>> can only be logged.
>>> What is the practical affect of a sensor remaining enabled?  Bit of
>>> power loss or something more significant?  This info matter when deciding
>>> if we should rush this in during the rc phase, or wait for the next
>>> merge window.
>> As far as I can tell, the practical effect is additional power
>> consumption while the device is idle.  I found no corruption path, but I
>> have no ICM-42607 hardware to reproduce the failure or measure the
>> current.
>>
>> It does not necessarily persist indefinitely.  If the PWR_MGMT0 write
>> fails, regmap may contain the requested OFF state while the hardware
>> remains ON.  A later sensor read requests an enabled mode, so
>> inv_icm42607_set_pwr_mgmt0() retries the write instead of taking its
>> "no change" return.  If the bus error was transient, the cache and
>> hardware are then resynchronized.
> Please capture some of that for the commit description for v2.
I need to correct one detail there.  For the I2C and SPI regmap paths
this driver uses, defer_caching is enabled, and _regmap_raw_write_impl()
drops the affected cache entry when the bus write fails.  So the failure
does not normally leave a cached OFF value.

The conclusion is unchanged, but the mechanism is different: a later
access misses in the cache, so regmap_read() reads PWR_MGMT0 from the
hardware again.  If the bus error was transient, the driver then sees
the actual state and can restore the requested mode if necessary.

I will use the corrected explanation in the next revision.  Sorry for
the confusion.


Thanks,
Linmao
>
>> I also noticed a cost to this patch: returning an error such as -EIO from
>> .runtime_suspend() makes the PM core set power.runtime_error. Subsequent
>> reads then fail in PM_RUNTIME_ACQUIRE_AUTOSUSPEND() before any sensor
>> register is accessed, until the PM status is reset.  A successful system
>> suspend may clear that state.
>>
>> The patch therefore trades a logged idle power leak that may recover on
>> a later access for a potentially sticky runtime-PM failure.  Since this
>> was found by inspection only, I would not rush it into the rc phase.  I
>> would rather revisit the error handling and target the next merge window.
>>
> That is fair enough.
>
> J
>> Linmao
>>
>>> Jonathan
>>>
>>>   
>>>> Fixes: 3007c1530f96 ("iio: imu: inv_icm42607: Add PM support for icm42607")
>>>> Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
>>>> ---
>>>> Unchanged since v1.
>>>>
>>>>    drivers/iio/imu/inv_icm42607/inv_icm42607_core.c | 15 ++++++++++-----
>>>>    1 file changed, 10 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
>>>> index 190e998f7b8ef..0da362967f63b 100644
>>>> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
>>>> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
>>>> @@ -537,9 +537,8 @@ static int inv_icm42607_enable_vddio_reg(struct inv_icm42607_state *st)
>>>>    	return 0;
>>>>    }
>>>>    
>>>> -static void inv_icm42607_sensors_off(void *_data)
>>>> +static int inv_icm42607_sensors_off(struct inv_icm42607_state *st)
>>>>    {
>>>> -	struct inv_icm42607_state *st = _data;
>>>>    	const struct device *dev = regmap_get_device(st->map);
>>>>    	int ret;
>>>>    
>>>> @@ -552,6 +551,13 @@ static void inv_icm42607_sensors_off(void *_data)
>>>>    					 st->conf.accel.mode);
>>>>    	if (ret)
>>>>    		dev_err(dev, "Unable to turn off sensors\n");
>>>> +
>>>> +	return ret;
>>>> +}
>>>> +
>>>> +static void inv_icm42607_sensors_off_action(void *data)
>>>> +{
>>>> +	inv_icm42607_sensors_off(data);
>>>>    }
>>>>    
>>>>    static void inv_icm42607_disable_vddio_reg(void *_data)
>>>> @@ -619,7 +625,7 @@ int inv_icm42607_core_probe(struct regmap *regmap,
>>>>    	 * Ensure if sensors get turned on at some point, they're turned off
>>>>    	 * as part of teardown.
>>>>    	 */
>>>> -	ret = devm_add_action_or_reset(dev, inv_icm42607_sensors_off, st);
>>>> +	ret = devm_add_action_or_reset(dev, inv_icm42607_sensors_off_action, st);
>>>>    	if (ret)
>>>>    		return ret;
>>>>    
>>>> @@ -688,8 +694,7 @@ static int inv_icm42607_runtime_suspend(struct device *dev)
>>>>    	 * however the tradeoff is that an unused sensor won't be
>>>>    	 * turned off until the entire chip is no longer in use.
>>>>    	 */
>>>> -	inv_icm42607_sensors_off(st);
>>>> -	return 0;
>>>> +	return inv_icm42607_sensors_off(st);
>>>>    }
>>>>    
>>>>    EXPORT_NS_GPL_DEV_PM_OPS(inv_icm42607_pm_ops, IIO_ICM42607) = {

[PATCH togreg v3 2/2] iio: imu: inv_icm42607: restore runtime PM on system resume errors

[Linmao Li]

pm_runtime_force_suspend() leaves runtime PM disabled after it succeeds and
expects pm_runtime_force_resume() to restore runtime PM management during
system resume.

The resume callback returns early if enabling the vddio regulator or
synchronizing the register cache fails, skipping the matching
pm_runtime_force_resume() call. Runtime PM consequently remains disabled
after the system has resumed, so runtime autosuspend can no longer turn off
sensors enabled afterward.

Call pm_runtime_force_resume() on both error paths. Keep the first error as
the return value and report a runtime PM restore failure separately.

Fixes: 3007c1530f96 ("iio: imu: inv_icm42607: Add PM support for icm42607")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
Changes since v2:
- Restructure inv_icm42607_resume() along the lines Andy suggested:
  handle the error case in its own block and call
  pm_runtime_force_resume() directly on the success path.  No
  functional change.

Changes since v1:
- Split the device side of inv_icm42607_resume() into a helper so the
  PM bookkeeping stays in the wrapper.  No functional change.

 .../iio/imu/inv_icm42607/inv_icm42607_core.c  | 23 +++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
index 0da362967f63b..f4ef75da22c76 100644
--- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
+++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
@@ -664,9 +664,8 @@ static int inv_icm42607_suspend(struct device *dev)
 	return 0;
 }
 
-static int inv_icm42607_resume(struct device *dev)
+static int inv_icm42607_resume_core(struct inv_icm42607_state *st)
 {
-	struct inv_icm42607_state *st = dev_get_drvdata(dev);
 	int ret;
 
 	ret = inv_icm42607_enable_vddio_reg(st);
@@ -675,9 +674,25 @@ static int inv_icm42607_resume(struct device *dev)
 
 	/* Sync the regcache again after regulator shutdown. */
 	regcache_mark_dirty(st->map);
-	ret = regcache_sync(st->map);
-	if (ret)
+
+	return regcache_sync(st->map);
+}
+
+static int inv_icm42607_resume(struct device *dev)
+{
+	struct inv_icm42607_state *st = dev_get_drvdata(dev);
+	int ret;
+
+	ret = inv_icm42607_resume_core(st);
+	if (ret) {
+		int rc;
+
+		rc = pm_runtime_force_resume(dev);
+		if (rc)
+			dev_warn(dev, "Failed to restore runtime PM state: %d\n", rc);
+
 		return ret;
+	}
 
 	return pm_runtime_force_resume(dev);
 }
-- 
2.25.1

Re: [PATCH togreg v3 2/2] iio: imu: inv_icm42607: restore runtime PM on system resume errors

[Jonathan Cameron]

On Tue, 11 Aug 2026 18:33:01 +0800
Linmao Li <lilinmao@kylinos.cn> wrote:

> pm_runtime_force_suspend() leaves runtime PM disabled after it succeeds and
> expects pm_runtime_force_resume() to restore runtime PM management during
> system resume.
> 
> The resume callback returns early if enabling the vddio regulator or
> synchronizing the register cache fails, skipping the matching
> pm_runtime_force_resume() call. Runtime PM consequently remains disabled
> after the system has resumed, so runtime autosuspend can no longer turn off
> sensors enabled afterward.
> 
> Call pm_runtime_force_resume() on both error paths. Keep the first error as
> the return value and report a runtime PM restore failure separately.
> 
> Fixes: 3007c1530f96 ("iio: imu: inv_icm42607: Add PM support for icm42607")
> Signed-off-by: Linmao Li <lilinmao@kylinos.cn>

Sashiko has some comments on this:
https://sashiko.dev/#/patchset/20260811103301.1157404-1-lilinmao%40kylinos.cn

I would note that in some paths error handling is best effort.
There isn't always a sequence that leaves us in a remotely
useful state.  So maybe what you have here is the best we can do
even though it is a bit crazy to expect the driver to do anything
useful if it can't power the device.

> ---
> Changes since v2:
> - Restructure inv_icm42607_resume() along the lines Andy suggested:
>   handle the error case in its own block and call
>   pm_runtime_force_resume() directly on the success path.  No
>   functional change.
> 
> Changes since v1:
> - Split the device side of inv_icm42607_resume() into a helper so the
>   PM bookkeeping stays in the wrapper.  No functional change.
> 
>  .../iio/imu/inv_icm42607/inv_icm42607_core.c  | 23 +++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> index 0da362967f63b..f4ef75da22c76 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> @@ -664,9 +664,8 @@ static int inv_icm42607_suspend(struct device *dev)
>  	return 0;
>  }
>  
> -static int inv_icm42607_resume(struct device *dev)
> +static int inv_icm42607_resume_core(struct inv_icm42607_state *st)
>  {
> -	struct inv_icm42607_state *st = dev_get_drvdata(dev);
>  	int ret;
>  
>  	ret = inv_icm42607_enable_vddio_reg(st);
> @@ -675,9 +674,25 @@ static int inv_icm42607_resume(struct device *dev)
>  
>  	/* Sync the regcache again after regulator shutdown. */
>  	regcache_mark_dirty(st->map);
> -	ret = regcache_sync(st->map);
> -	if (ret)
> +
> +	return regcache_sync(st->map);
> +}
> +
> +static int inv_icm42607_resume(struct device *dev)
> +{
> +	struct inv_icm42607_state *st = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = inv_icm42607_resume_core(st);
> +	if (ret) {
> +		int rc;
> +
> +		rc = pm_runtime_force_resume(dev);
> +		if (rc)
> +			dev_warn(dev, "Failed to restore runtime PM state: %d\n", rc);
> +

There is a question from sashiko on whether this can be reached. 
Even though that may be the case I'd keep the the error print because
it hardens us against future changes.

>  		return ret;
> +	}
>  
>  	return pm_runtime_force_resume(dev);
>  }

Re: [PATCH togreg v3 2/2] iio: imu: inv_icm42607: restore runtime PM on system resume errors

[Linmao Li]

在 2026/8/16 4:58, Jonathan Cameron 写道:
> On Tue, 11 Aug 2026 18:33:01 +0800
> Linmao Li <lilinmao@kylinos.cn> wrote:
>
>> pm_runtime_force_suspend() leaves runtime PM disabled after it succeeds and
>> expects pm_runtime_force_resume() to restore runtime PM management during
>> system resume.
>>
>> The resume callback returns early if enabling the vddio regulator or
>> synchronizing the register cache fails, skipping the matching
>> pm_runtime_force_resume() call. Runtime PM consequently remains disabled
>> after the system has resumed, so runtime autosuspend can no longer turn off
>> sensors enabled afterward.
>>
>> Call pm_runtime_force_resume() on both error paths. Keep the first error as
>> the return value and report a runtime PM restore failure separately.
>>
>> Fixes: 3007c1530f96 ("iio: imu: inv_icm42607: Add PM support for icm42607")
>> Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
> Sashiko has some comments on this:
> https://sashiko.dev/#/patchset/20260811103301.1157404-1-lilinmao%40kylinos.cn
>
> I would note that in some paths error handling is best effort.
> There isn't always a sequence that leaves us in a remotely
> useful state.  So maybe what you have here is the best we can do
> even though it is a bit crazy to expect the driver to do anything
> useful if it can't power the device.
>
>> ---
>> Changes since v2:
>> - Restructure inv_icm42607_resume() along the lines Andy suggested:
>>    handle the error case in its own block and call
>>    pm_runtime_force_resume() directly on the success path.  No
>>    functional change.
>>
>> Changes since v1:
>> - Split the device side of inv_icm42607_resume() into a helper so the
>>    PM bookkeeping stays in the wrapper.  No functional change.
>>
>>   .../iio/imu/inv_icm42607/inv_icm42607_core.c  | 23 +++++++++++++++----
>>   1 file changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
>> index 0da362967f63b..f4ef75da22c76 100644
>> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
>> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
>> @@ -664,9 +664,8 @@ static int inv_icm42607_suspend(struct device *dev)
>>   	return 0;
>>   }
>>   
>> -static int inv_icm42607_resume(struct device *dev)
>> +static int inv_icm42607_resume_core(struct inv_icm42607_state *st)
>>   {
>> -	struct inv_icm42607_state *st = dev_get_drvdata(dev);
>>   	int ret;
>>   
>>   	ret = inv_icm42607_enable_vddio_reg(st);
>> @@ -675,9 +674,25 @@ static int inv_icm42607_resume(struct device *dev)
>>   
>>   	/* Sync the regcache again after regulator shutdown. */
>>   	regcache_mark_dirty(st->map);
>> -	ret = regcache_sync(st->map);
>> -	if (ret)
>> +
>> +	return regcache_sync(st->map);
>> +}
>> +
>> +static int inv_icm42607_resume(struct device *dev)
>> +{
>> +	struct inv_icm42607_state *st = dev_get_drvdata(dev);
>> +	int ret;
>> +
>> +	ret = inv_icm42607_resume_core(st);
>> +	if (ret) {
>> +		int rc;
>> +
>> +		rc = pm_runtime_force_resume(dev);
>> +		if (rc)
>> +			dev_warn(dev, "Failed to restore runtime PM state: %d\n", rc);
>> +
> There is a question from sashiko on whether this can be reached.
> Even though that may be the case I'd keep the the error print because
> it hardens us against future changes.
Agreed.  I would keep the warning, and I think it is reachable. When
pm_runtime_force_resume() needs to invoke a runtime-resume callback,
GET_CALLBACK() can select one from the PM domain, device type, class or
bus before falling back to the driver.  The NULL runtime_resume in this
driver's PM ops therefore does not mean that the entire callback chain is
empty.  For example, a generic PM domain runtime-resume callback can
fail.

On sashiko's other point, leaving runtime PM disabled would not by itself
block I/O with -EACCES here.  Provided there is no pre-existing
runtime_error, the read paths use PM_RUNTIME_ACQUIRE_AUTOSUSPEND(), which
calls pm_runtime_get_active() with RPM_TRANSPARENT.  The acquisition
therefore succeeds when runtime PM is disabled.

Calling pm_runtime_force_resume() on the error path consequently does not
newly expose accesses after a failed hardware resume; that possibility
already exists without the patch.  Depending on the transport and actual
hardware state, such accesses may either fail or return unusable data.
>
>>   		return ret;
>> +	}
>>   
>>   	return pm_runtime_force_resume(dev);
>>   }

Re: [PATCH togreg v2 0/2] iio: imu: inv_icm42607: fix PM error handling

[Jonathan Cameron]

On Tue, 11 Aug 2026 10:03:43 +0800
Linmao Li <lilinmao@kylinos.cn> wrote:

> The recently queued ICM-42607 PM support has two error paths that can leave
> the PM core's state inconsistent with the device.

For IIO at least (and I've never come across a subsystem that asks for the
style you have here) don't send new versions in reply to older ones.

It gets too complex if there is a lot of feedback and generally pushes
your patches many screens up from the most recent in potential reviewers
inboxes.  So you'll probably get fewer reviews.

Jonathan