Re: [PATCH v4 5/5] iio: imu: inv_icm42607: Add accelerometer calibbias support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] iio: imu: inv_icm42607: calibration bias lost across suspend
--

commit 265a6fbb240f73261702e44167f3a068fc676f5f
Author: Kanak Shilledar <kanak.shilledar@axis.com>

iio: imu: inv_icm42607: Add accelerometer calibbias support

Expose IIO_CHAN_INFO_CALIBBIAS on the accelerometer channels. The
registers are stored in MREG1. The calibration bias is written to
OFFSET_USER4 to OFFSET_USER8 registers in MREG1. Reject the out of
limit calibbias values instead of clamping it.

Datasheet: https://www.invensense.tdk.com/en-us/products/3-axis/icm-42370-p
Datasheet: https://www.lcsc.com/product-detail/C5129967.html

> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
> index 8f61bc9014526..39c5a3f80b2eb 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c

[ ... ]

> @@ -176,6 +189,171 @@ static int inv_icm42607_accel_write_odr(struct iio_dev *indio_dev,
>  	return inv_icm42607_set_sensor_conf(st, &conf, IIO_ACCEL);
>  }
>  

[ ... ]

> +static int inv_icm42607_accel_write_offset(struct iio_dev *indio_dev,
> +					   struct iio_chan_spec const *chan,
> +					   int val, int val2)
> +{

[ ... ]

> +	switch (chan->channel2) {
> +	case IIO_MOD_X:
> +		/* OFFSET_USER4 upper nibble is shared. */
> +		ret = regmap_update_bits(st->map, INV_ICM42607_REG_OFFSET_USER4,
> +					 GENMASK(7, 4), (offset & 0xF00) >> 4);

[Severity: Medium]
Are user-configured accelerometer calibration biases lost across system
suspend and resume cycles?

Since inv_icm42607_accel_write_offset() writes directly to the MREG1 bank
registers in drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c, and the
driver explicitly marks MREG registers as volatile in
inv_icm42607_is_volatile_reg():

    if ((reg & INV_ICM42607_REG_BANK_MASK) != INV_ICM42607_BANK0)
        return true;

won't regcache_sync() skip these registers upon resume?

If the vddio_supply is turned off during suspend, the sensor's internal
state is wiped. Because regmap doesn't cache volatile registers, the
calibration biases would be reset to zero upon resume instead of being
restored.

Could this be updated to cache the user's bias setting in the driver's
software state so it can be restored after resume?

> +		if (ret)
> +			return ret;
> +
> +		return regmap_write(st->map, INV_ICM42607_REG_OFFSET_USER5,
> +				    offset & 0xFF);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917-b4-inv_icm42370p-v4-0-fd9373f123c9@axis.com?part=5