Re: [PATCH V18 8/9] iio: imu: inv_icm42607: Add Temp Support in icm42607

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The temperature channel advertises IIO_CHAN_INFO_SAMP_FREQ but fails to handle it in its read_raw implementation, causing -EINVAL.
--

--- Patch [8]: [PATCH V18 8/9] iio: imu: inv_icm42607: Add Temp Support in icm42607 ---
commit d42e76e3efb32de0c89c91668f6d156c61cbf937
Author: Chris Morgan <macromorgan@hotmail.com>

iio: imu: inv_icm42607: Add Temp Support in icm42607
    
Add functions for reading temperature sensor data.

> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
> index ad1135d48902d..718a0201b6eda 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
[ ... ]
> @@ -187,6 +190,8 @@ static int inv_icm42607_accel_read_raw(struct iio_dev *indio_dev,
>  	switch (chan->type) {
>  	case IIO_ACCEL:
>  		break;
> +	case IIO_TEMP:
> +		return inv_icm42607_temp_read_raw(indio_dev, chan, val, val2, mask);

[Severity: Medium]
Will this early return bypass the IIO_CHAN_INFO_SAMP_FREQ handler located
further down in inv_icm42607_accel_read_raw()?

Since the temperature channel advertises IIO_CHAN_INFO_SAMP_FREQ in
INV_ICM42607_TEMP_CHAN, this causes requests for the sampling frequency to
return -EINVAL instead of the actual sampling frequency.

>  	default:
>  		return -EINVAL;
>  	}

> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
> index 6af7b143e6a15..e7d5e552bb5cb 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
[ ... ]
> @@ -184,6 +187,8 @@ static int inv_icm42607_gyro_read_raw(struct iio_dev *indio_dev,
>  	switch (chan->type) {
>  	case IIO_ANGL_VEL:
>  		break;
> +	case IIO_TEMP:
> +		return inv_icm42607_temp_read_raw(indio_dev, chan, val, val2, mask);

[Severity: Medium]
Similar to the accel implementation, does this early return intercept IIO_TEMP
and incorrectly skip the IIO_CHAN_INFO_SAMP_FREQ fallback?

>  	default:
>  		return -EINVAL;
>  	}

> 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..8681ef2b49e21
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
[ ... ]
> @@ -0,0 +1,98 @@
[ ... ]
> +int inv_icm42607_temp_read_raw(struct iio_dev *indio_dev,
> +				struct iio_chan_spec const *chan,
> +				int *val, int *val2, long mask)
> +{
> +	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
> +	s16 temp;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = inv_icm42607_temp_read(st, &temp);
> +		if (ret)
> +			return ret;
> +		*val = temp;
> +		return IIO_VAL_INT;
> +	/*
> +	 * T°C = (temp / 128) + 25
> +	 * Tm°C = 1000 * ((temp * 100 / 12800) + 25)
> +	 * scale: 100000 / 12800 ~= 7.8125
> +	 * offset: 3200
> +	 */
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 7;
> +		*val2 = 812500000;
> +		return IIO_VAL_INT_PLUS_NANO;
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = 3200;
> +		return IIO_VAL_INT;
> +	default:

[Severity: Medium]
Since this function handles the mask for IIO_TEMP, should it also handle
IIO_CHAN_INFO_SAMP_FREQ? 

It currently falls through to the default -EINVAL case, causing in-kernel
reads for the sampling frequency to fail, despite the temperature channel
advertising it.

> +		return -EINVAL;
> +	}
> +}

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