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

On Tue, 11 Aug 2026 18:33:00 +0800
Linmao Li <[email protected]> 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 <[email protected]>
> ---
> 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) = {