Re: [PATCH V8 07/10] iio: imu: inv_icm42607: Add Temp Support in icm42607

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The temperature sensor is left enabled indefinitely after a single read, causing a power leak and preventing low-power modes.
- [Medium] The true Runtime PM error code is incorrectly masked and replaced with a hardcoded -ENXIO.
--

commit 454b81f6136c888691f5c12d16c333a82da009f2
Author: Chris Morgan <macromorgan@hotmail.com>

iio: imu: inv_icm42607: Add Temp Support in icm42607

This commit adds functions for reading temperature sensor data in the
inv_icm42607 driver.

[ ... ]

> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
> new file mode 100644
> index 0000000000000..7e9cb072a0110
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
> @@ -0,0 +1,80 @@
[ ... ]
> +static int inv_icm42607_temp_read(struct inv_icm42607_state *st, s16 *temp)
> +{
> +	struct device *dev = regmap_get_device(st->map);
> +	__be16 *raw;
> +	int ret;
> +
> +	PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
> +	if (PM_RUNTIME_ACQUIRE_ERR(&pm))
> +		return -ENXIO;

Since PM_RUNTIME_ACQUIRE_ERR() evaluates to the actual integer error code
returned by the underlying runtime PM call, does returning a hardcoded
-ENXIO here mask the true cause of the PM failure from userspace?

Could this propagate the actual error code instead?

> +
> +	guard(mutex)(&st->lock);
> +
> +	ret = inv_icm42607_set_temp_conf(st, true, NULL);
> +	if (ret)
> +		return ret;
> +
> +	raw = &st->buffer[0];
> +	ret = regmap_bulk_read(st->map, INV_ICM42607_REG_TEMP_DATA1, raw, sizeof(*raw));
> +	if (ret)
> +		return ret;
> +
> +	*temp = be16_to_cpup(raw);
> +	if (*temp == INV_ICM42607_DATA_INVALID)
> +		return -EINVAL;
> +
> +	return 0;
> +}

Is the temperature sensor left enabled indefinitely after a single read?

The sensor is explicitly powered on by calling inv_icm42607_set_temp_conf()
with enable set to true, but it doesn't appear to be turned off before
returning.

If another channel like the accelerometer is actively streaming, the device
will not power down due to runtime PM autosuspend. Because st->conf.temp_en
remains true, it appears this would actively prevent the accelerometer from
entering its low-power RC oscillator clock mode, which requires the
temperature sensor to be disabled.

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