Re: [PATCH V13 8/9] iio: imu: inv_icm42607: Add Gyroscope to icm42607

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The gyro sensor state is not reverted after a raw read, causing it to remain permanently enabled during all future active PM states.
--

commit 608315b9e755ed3de8730d6f1fc3e2dc0c68b7a5
Author: Chris Morgan <macromorgan@hotmail.com>

iio: imu: inv_icm42607: Add Gyroscope to icm42607

This commit adds gyroscope functions and IIO device registration to the
icm42607 driver.

> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
> new file mode 100644
> index 0000000000000..ef73560b39d73
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c

[ ... ]

> +static int inv_icm42607_gyro_read_sensor(struct iio_dev *indio_dev,
> +					 struct iio_chan_spec const *chan,
> +					 s16 *val)
> +{
> +	struct inv_icm42607_sensor_conf conf = INV_ICM42607_SENSOR_CONF_INIT;
> +	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
> +	struct inv_icm42607_sensor_state *gyro_st = iio_priv(indio_dev);
> +	struct device *dev = regmap_get_device(st->map);
> +	unsigned int reg;
> +	u8 data[2];
> +	int ret;

[ ... ]

> +	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
> +	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
> +	if (ret)
> +		return ret;
> +
> +	guard(mutex)(&st->lock);
> +
> +	/* enable gyro sensor */
> +	conf.mode = gyro_st->power_mode;
> +	conf.filter = gyro_st->filter;
> +	ret = inv_icm42607_set_gyro_conf(st, &conf, NULL);
> +	if (ret)
> +		return ret;
> +
> +	/* read gyro register data */
> +	ret = regmap_bulk_read(st->map, reg, data, sizeof(data));
> +	if (ret)
> +		return ret;
> +
> +	*val = get_unaligned_be16(data);
> +	if (*val == INV_ICM42607_DATA_INVALID)
> +		return -EINVAL;
> +
> +	return 0;
> +}

[Severity: Medium]
Does this leave the gyroscope permanently enabled after a raw read?

The function enables the gyro by setting conf.mode = gyro_st->power_mode
and calling inv_icm42607_set_gyro_conf(), but does not appear to revert it
to an off state before returning. 

When the device later autosuspends, inv_icm42607_runtime_suspend() saves this
active state into st->suspended.gyro. On every subsequent PM resume,
inv_icm42607_runtime_resume() will restore the gyro to ON.

Could this cause an unnecessary 45ms startup delay and waste power during all
future active PM states?

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