Re: [PATCH V17 9/9] arm64: dts: rockchip: Add icm42607p IMU for RG-DS

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing register cache synchronization and device re-initialization on system resume causes the device to operate with incorrect hardware configurations after a power loss.
- [Medium] Temperature channel scale and offset read requests are improperly intercepted and rejected, returning -EINVAL.
--

--- Patch [9]: [PATCH V17 9/9] arm64: dts: rockchip: Add icm42607p IMU for RG-DS ---
commit 71c9a85a61febe0138b79e4a249c5aff0b95a564
Author: Chris Morgan <macromorgan@hotmail.com>

arm64: dts: rockchip: Add icm42607p IMU for RG-DS

This commit adds the Invensense ICM42607P IMU for the Anbernic RG-DS.
It sets the compatible string, reg address, and mount-matrix.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3568-anbernic-rg-ds.dts b/arch/arm64/boot/dts/rockchip/rk3568-anbernic-rg-ds.dts
> index 8d906ab02c5fb..b770bfd5268d8 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3568-anbernic-rg-ds.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3568-anbernic-rg-ds.dts
> @@ -871,7 +871,13 @@ aw87391_pa_r: audio-codec@5b {
>  		sound-name-prefix = "Right Amp";
>  	};
>  
> -	/* invensense,icm42607p at 0x68 */
> +	icm42607p: imu@68 {
> +		compatible = "invensense,icm42607p";
> +		reg = <0x68>;
> +		mount-matrix = "-1", "0", "0",
> +			       "0", "1", "0",
> +			       "0", "0", "-1";
> +	};
>  };

[Severity: High]
Does this system resume handler in the icm42607p driver need to explicitly
mark the regmap cache dirty and sync it?

drivers/iio/imu/inv_icm42607/inv_icm42607_core.c:inv_icm42607_resume() {
    static int inv_icm42607_resume(struct device *dev)
    {
        struct inv_icm42607_state *st = dev_get_drvdata(dev);
        int ret;

        ret = inv_icm42607_enable_vddio_reg(st);
        if (ret)
            return ret;

        return pm_runtime_force_resume(dev);
    }
}

If the IMU loses power during system suspend (e.g., VDD is cut by the PM
domain), its internal registers reset to hardware defaults.

Because the regmap cache is not marked dirty on resume, subsequent writes
might skip configuration registers since the cached values match the desired
values.

This could leave the device operating with default, unconfigured reset values
(e.g., incorrect endianness, incorrect scale/ODR), corrupting sensor data.

[Severity: Medium]
Do these read functions improperly intercept and reject temperature channel
scale and offset read requests?

drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c:inv_icm42607_accel_read_raw() {
        switch (mask) {
        case IIO_CHAN_INFO_RAW:
            if (chan->type == IIO_TEMP)
                return inv_icm42607_temp_read_raw(indio_dev, chan,
                                                  val, val2, mask);
            ret = inv_icm42607_read_sensor(indio_dev, chan, &data);
            if (ret)
                return ret;
            *val = data;
            return IIO_VAL_INT;
        case IIO_CHAN_INFO_SCALE:
            if (chan->type != IIO_ACCEL)
                return -EINVAL;
            return inv_icm42607_accel_read_scale(indio_dev, val, val2);
}

drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c:inv_icm42607_gyro_read_raw() {
        case IIO_CHAN_INFO_SCALE:
            if (chan->type != IIO_ANGL_VEL)
                return -EINVAL;
            return inv_icm42607_gyro_read_scale(indio_dev, val, val2);
}

This scopes the IIO_TEMP delegation to only IIO_CHAN_INFO_RAW, causing
IIO_CHAN_INFO_SCALE and IIO_CHAN_INFO_OFFSET queries for the temperature
channel to fall through and fail.

Userspace applications attempting to read the temperature scale or offset from
sysfs will receive an -EINVAL error. This could prevent userspace from
correctly interpreting the raw temperature readings.

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