Re: [PATCH V20 6/9] iio: imu: inv_icm42607: Add Accelerometer for icm42607

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Raw reads via `inv_icm42607_read_sensor` do not wait for the first sample to be generated, causing spurious `-EINVAL` errors on wakeup.
- [Low] Passing `unsigned int *` to parameters expecting `enum inv_icm42607_sensor_mode *` causes a compiler warning and potential strict-aliasing violation.
--

--- Patch [6]: [PATCH V20 6/9] iio: imu: inv_icm42607: Add Accelerometer for icm42607 ---
commit 34b41c497c47a25cee214dea5331f64210422963
Author: Chris Morgan <macromorgan@hotmail.com>

iio: imu: inv_icm42607: Add Accelerometer for icm42607
    
Add icm42607 accelerometer sensor for icm42607.

> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> index e6d2fc7c0a344..22cd37e0616e3 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c

[ ... ]

> @@ -191,6 +202,174 @@ static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,

[ ... ]

> +int inv_icm42607_set_sensor_conf(struct inv_icm42607_state *st,
> +				 struct inv_icm42607_sensor_conf *conf,
> +				 enum iio_chan_type chan_type)
> +{
> +	struct inv_icm42607_sensor_conf *oldconf;
> +	unsigned int accel_mode, gyro_mode;
> +	bool config0, config1;
> +	unsigned int val;
> +	int ret;

[ ... ]

> +	/*
> +	 * Since we are only making one-shot calls today, we can avoid needless
> +	 * on/off cycles if we rely on the runtime power management to handle
> +	 * shutting off the sensors when not in use. The downside is if we
> +	 * enable both sensors then only read from one, we have to wait for
> +	 * runtime PM to turn it off. So just keep the mode of the sensor we
> +	 * are not actively using to whatever it's already set as.
> +	 */
> +	ret = inv_icm42607_get_pwr_mgmt0(st, &gyro_mode, &accel_mode);

[Severity: Low]
Does this code cause an incompatible pointer type warning?

inv_icm42607_get_pwr_mgmt0() expects enum inv_icm42607_sensor_mode *
parameters, but &gyro_mode and &accel_mode are unsigned int *. Recent kernel
versions (since 6.8) treat this as a build error by default.

[ ... ]

> +int inv_icm42607_read_sensor(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     s16 *val)
> +{
> +	struct inv_icm42607_sensor_conf conf = INV_ICM42607_SENSOR_CONF_INIT;
> +	struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);

[ ... ]

> +	/* enable sensor */
> +	conf.mode = sensor_st->power_mode;
> +	conf.filter = sensor_st->filter;
> +	ret = inv_icm42607_set_sensor_conf(st, &conf, chan->type);
> +	if (ret)
> +		return ret;
> +
> +	/* read sensor register data */
> +	ret = regmap_bulk_read(st->map, reg, &data, sizeof(data));

[Severity: Medium]
Does this sequence need to wait for the first sample to be generated after
powering on the sensor?

It appears that if the sensor is turned on by a userspace read, the driver
waits for the physical hardware startup time in inv_icm42607_set_pwr_mgmt0(),
but doesn't wait for the first sample to be generated based on the configured
ODR (which can take an additional 1/ODR time).

Could reading immediately after power on yield the invalid data marker
(-32768) and return a spurious -EINVAL?

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