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

[PATCH togreg v4 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.  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 v3:
- Patch 1: commit message expanded with the practical effect of the current
  behaviour, the cost of propagating the error, and a description of the
  recovery path that does not assume a particular regmap bus
  implementation.  No code change.
- Patch 2: unchanged.

Neither patch was reproduced on hardware; both were found by code
inspection.  They were compile-tested with W=1 and checked with smatch.

Whether patch 1 is worth making is still a fair question - it trades a
possible idle power leak that may be cleared by a later successful access
for a runtime PM error state that needs an explicit reset.  The commit
message spells that out; happy to drop it if you would rather not take
that trade.

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 v4 1/2] iio: imu: inv_icm42607: propagate runtime suspend errors

[Linmao Li]

The runtime suspend callback returns success when updating PWR_MGMT0
fails.  The PM core then marks the device suspended even though the
shutdown outcome is unknown.  If the write did not reach the sensor, the
sensors remain running and draw power while the device is idle.

The condition need not persist.  If the bus error clears, a later sensor
access either reads the current PWR_MGMT0 value from hardware or retries
programming the requested enabled mode, allowing normal operation to
resume.

Return the underlying errno to the PM core.  -EAGAIN and -EBUSY retain
their transient-error semantics; other errors put runtime PM into an
error state and cause later PM acquires to fail until the status is
explicitly reset.  In the normal idle case, a successful system suspend
can perform that reset.  This leaves the transient-versus-fatal
classification to the PM core and matches the sibling ICM-42600 driver.

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>
---
Changes since v3:
- Expand the commit message with the practical effect and the recovery
  behaviour, as requested.  No code change.

This was found by code inspection.  No ICM-42607 hardware or
fault-injection setup was available.

 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 v4 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>
---
Unchanged since v3.

This was found by code inspection.  No ICM-42607 hardware or
fault-injection setup was available.

 .../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 v4 0/2] iio: imu: inv_icm42607: fix PM error handling

[Jonathan Cameron]

On Mon, 24 Aug 2026 11:55:29 +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.
> 
> Patch 1 propagates sensor shutdown failures from runtime suspend.  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.

These look fine to me, but I want input from Chris (and ideally some sanity
check testing) before picking them up.  The dead chicken test that they
don't active break operation when we don't see errors is probably enough
given the analysis seems fine to me for what happens on error.

Thanks,

Jonathan

> 
> Changes since v3:
> - Patch 1: commit message expanded with the practical effect of the current
>   behaviour, the cost of propagating the error, and a description of the
>   recovery path that does not assume a particular regmap bus
>   implementation.  No code change.
> - Patch 2: unchanged.
> 
> Neither patch was reproduced on hardware; both were found by code
> inspection.  They were compile-tested with W=1 and checked with smatch.
> 
> Whether patch 1 is worth making is still a fair question - it trades a
> possible idle power leak that may be cleared by a later successful access
> for a runtime PM error state that needs an explicit reset.  The commit
> message spells that out; happy to drop it if you would rather not take
> that trade.
> 
> 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