Re: [PATCH V14 8/9] 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:
- [Low] Missing endianness initialization in INV_ICM42607_TEMP_CHAN.
- [Low] Leftover commented-out developmental code.
--

commit a30ed1de671168873a3fd8263c138bbcda62583a
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_temp.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
> new file mode 100644
> index 0000000000000..f7e286ecb5cd6
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
[ ... ]
> +static int inv_icm42607_temp_read(struct inv_icm42607_state *st, s16 *temp)
> +{
[ ... ]
> +	accel_mode = FIELD_GET(INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK, val);
> +	gyro_mode = FIELD_GET(INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK, val);
> +	if (!gyro_mode && !accel_mode) {
> +		/* enable accel sensor */
> +		conf.mode = INV_ICM42607_SENSOR_MODE_LOW_NOISE;
> +//		ret = inv_icm42607_set_accel_conf(st, &conf);

[Severity: Low]
This isn't a bug, but should this commented-out line of code be removed
from the final patch?

> +		ret = inv_icm42607_set_sensor_conf(st, &conf, IIO_ACCEL);
> +		if (ret)
> +			return ret;
> +	}
[ ... ]
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h
> new file mode 100644
> index 0000000000000..cb7b460ffb441
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h
[ ... ]
> +#define INV_ICM42607_TEMP_CHAN(_index)				\
> +{								\
> +	.type = IIO_TEMP,					\
> +	.info_mask_separate =					\
> +		BIT(IIO_CHAN_INFO_RAW) |			\
> +		BIT(IIO_CHAN_INFO_OFFSET) |			\
> +		BIT(IIO_CHAN_INFO_SCALE),			\
> +	.info_mask_shared_by_all =				\
> +		BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
> +	.info_mask_shared_by_all_available =			\
> +		BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
> +	.scan_index = _index,					\
> +	.scan_type = {						\
> +		.sign = 's',					\
> +		.realbits = 16,					\
> +		.storagebits = 16,				\
> +	},							\

[Severity: Low]
Does this scan_type struct need its endianness field initialized to IIO_BE?

Since get_unaligned_be16() is used when reading the sensor data in
inv_icm42607_temp_read(), it appears the hardware outputs big-endian data.
Omitting the endianness here defaults to IIO_CPU (0), which differs from the
gyro and accel channels that explicitly define it. Could this cause issues
if IIO buffered reads are implemented later?

> +}

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