Re: [PATCH 2/3] iio: imu: st_lsm6dsx: set FIFO ODR for accelerometer and magnetometer only

[Jonathan Cameron <jic23@kernel.org>]

On Sun, 11 Jan 2026 16:23:51 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri,  9 Jan 2026 19:15:27 +0100
> Francesco Lavra <flavra@baylibre.com> wrote:
> 
> > The st_lsm6dsx_set_fifo_odr() function, which is called when enabling and
> > disabling the hardware FIFO, checks the contents of the hw->settings->batch
> > array at index sensor->id, and then sets the current ODR value in sensor
> > registers that depend on whether the register address is set in the above
> > array element. This logic is valid for internal sensors only, i.e. the
> > accelerometer and magnetometer; however, since commit c91c1c844ebd ("iio:
> > imu: st_lsm6dsx: add i2c embedded controller support"), this function is
> > called also when configuring the hardware FIFO for external sensors (i.e.
> > sensors accessed through the sensor hub functionality), which can result in
> > unrelated device registers being written.
> > 
> > Add a check to the beginning of st_lsm6dsx_set_fifo_odr() so that it does
> > not touch any registers unless it is called for internal sensors.
> > 
> > Fixes: c91c1c844ebd ("iio: imu: st_lsm6dsx: add i2c embedded controller support")
> > Signed-off-by: Francesco Lavra <flavra@baylibre.com>
> This seems fine to me. Ideally it would have been first patch in the series
> as this is one we want to backport.  I'll leave it on list little while
> though to see if Lorenzo or anyone else has time to take a look.
> 
One thing...

> Thanks,
> 
> Jonathan
> 
> 
> > ---
> >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > index 5ac45e6230b5..9db48e835d4f 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > @@ -225,6 +225,9 @@ static int st_lsm6dsx_set_fifo_odr(struct st_lsm6dsx_sensor *sensor,
> >  	const struct st_lsm6dsx_reg *batch_reg;
> >  	u8 data;
> >  
> > +	/* Only accel and gyro have batch registers. */

It's true we can't use the batch register, but not clear from this comment
that the else path below is inappropriate.  Is it the absence of patch register
or just that the set_fifo_odr is meaningless for other sensors that matters?
I think this comment needs to provide more detail.

> > +	if (sensor->id >= ARRAY_SIZE(hw->settings->batch))
> > +		return 0;
> >  	batch_reg = &hw->settings->batch[sensor->id];
> >  	if (batch_reg->addr) {
> >  		int val;
>