Re: [PATCH V18 5/9] iio: imu: inv_icm42607: Add PM support for icm42607

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Calling `pm_runtime_force_suspend()` in the `.remove()` callbacks causes an unbalanced runtime PM `disable_depth` due to conflict with `devm_pm_runtime_set_active_enabled()`, breaking power management on driver rebind.
--

--- Patch [5]: [PATCH V18 5/9] iio: imu: inv_icm42607: Add PM support for icm42607 ---
commit ab55ef85474a1fe3f2d09669c982ebc220a804ca
Author: Chris Morgan <macromorgan@hotmail.com>

iio: imu: inv_icm42607: Add PM support for icm42607

Add power management support for the ICM42607 device driver.

> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> index 6ec1d730017e2..c127ea0a94c4c 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c

[ ... ]

> @@ -275,10 +378,75 @@ int inv_icm42607_core_probe(struct regmap *regmap,
>  	if (ret)
>  		return ret;
>  
> +	ret = devm_pm_runtime_set_active_enabled(dev);
> +	if (ret)
> +		return ret;
> +
> +	pm_runtime_set_autosuspend_delay(dev, INV_ICM42607_SUSPEND_DELAY_MS);
> +	pm_runtime_use_autosuspend(dev);
> +
>  	return 0;
>  }

[ ... ]

> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
> index f2b9067815b0e..04ad372917832 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c

[ ... ]

> @@ -56,6 +57,11 @@ static int inv_icm42607_probe(struct i2c_client *client)
>  	return inv_icm42607_core_probe(regmap, hw, inv_icm42607_i2c_bus_setup);
>  }
>  
> +static void inv_icm42607_i2c_remove(struct i2c_client *client)
> +{
> +	pm_runtime_force_suspend(&client->dev);
> +}

[Severity: Medium]
Does calling pm_runtime_force_suspend() here cause an unbalanced
disable_depth counter?

Because devm_pm_runtime_set_active_enabled() is called during probe in
inv_icm42607_core_probe(), devres will automatically call
pm_runtime_disable() on device removal.

Since pm_runtime_force_suspend() also disables runtime PM internally,
it seems this results in the disable_depth counter being incremented twice.
If the driver is unbound and rebound later, will it fail to ever re-enable
runtime PM because the disable depth remains strictly greater than zero?

[ ... ]

> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c
> index eb04036a6712f..995393dce9429 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c

[ ... ]

> @@ -65,6 +66,11 @@ static int inv_icm42607_probe(struct spi_device *spi)
>  	return inv_icm42607_core_probe(regmap, hw, inv_icm42607_spi_bus_setup);
>  }
>  
> +static void inv_icm42607_spi_remove(struct spi_device *spi)
> +{
> +	pm_runtime_force_suspend(&spi->dev);
> +}

[Severity: Medium]
Could this run into the same double-disable issue as the I2C remove function
due to the devres pm_runtime_disable() action?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720202343.70138-1-macroalpha82@gmail.com?part=5