Re: [PATCH V19 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:
- [High] Manual sensor shutdown in `remove` races with devm-managed IIO device unregistration, allowing hardware constraints to be violated.
--

--- Patch [5]: [PATCH V19 5/9] iio: imu: inv_icm42607: Add PM support for icm42607 ---
commit b056ec561cd9320015e132187a3e7a3626b06b64
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_i2c.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
> index f2b9067815b0e..4a8e4f7168038 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
[ ... ]
> @@ -56,6 +57,13 @@ 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)
> +{
> +	struct inv_icm42607_state *st = dev_get_drvdata(&client->dev);
> +
> +	inv_icm42607_sensors_off(st);

[Severity: High]
Can this manual shutdown race with devm-managed IIO device unregistration?

Since the IIO devices are registered with devm-managed functions during probe,
the devres release phase will execute after this remove callback returns. This
means the IIO devices remain fully accessible to userspace during this period.

If a concurrent userspace read occurs just after inv_icm42607_sensors_off
finishes but before the devres release phase begins, could it re-acquire the
runtime PM reference via PM_RUNTIME_ACQUIRE_AUTOSUSPEND and turn the sensors
back on?

When the read completes and drops the PM reference, an autosuspend would be
queued. Then, devres runs, unregistering the IIO device and executing
pm_runtime_disable(), which would cancel the pending autosuspend and leave
the hardware permanently on.

Finally, the devm action for inv_icm42607_disable_vddio_reg() would cut power
to the chip while the sensors are still active, bypassing the mandatory 45ms
gyro power-down delay.

> +}
> +
>  static const struct i2c_device_id inv_icm42607_id[] = {
[ ... ]
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c
> index eb04036a6712f..99e112c958a00 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c
[ ... ]
> @@ -65,6 +66,13 @@ 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)
> +{
> +	struct inv_icm42607_state *st = dev_get_drvdata(&spi->dev);
> +
> +	inv_icm42607_sensors_off(st);

[Severity: High]
Does this have the same race condition with devm-managed IIO device
unregistration as the I2C remove function?

> +}
> +
>  static const struct spi_device_id inv_icm42607_spi_id_table[] = {

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