1. Patchew
  2. linux
  3. [PATCH v2 0/3] imu: st_lsm6dsx: Add support for rotation sensor
  4. View patch

[PATCH v2 1/3] iio: imu: st_lsm6dsx: Set FIFO ODR for accelerometer and gyroscope only

Francesco Lavra posted 3 patches 3 weeks, 3 days ago
There is a newer version of this series
[PATCH v2 1/3] iio: imu: st_lsm6dsx: Set FIFO ODR for accelerometer and gyroscope only
Posted by Francesco Lavra 3 weeks, 3 days ago 
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 gyroscope; 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>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
index 55d877745575..1ee2fc5f5f1f 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -225,6 +225,10 @@ static int st_lsm6dsx_set_fifo_odr(struct st_lsm6dsx_sensor *sensor,
 	const struct st_lsm6dsx_reg *batch_reg;
 	u8 data;
 
+	/* Only internal sensors have a FIFO ODR configuration register. */
+	if (sensor->id >= ARRAY_SIZE(hw->settings->batch))
+		return 0;
+
 	batch_reg = &hw->settings->batch[sensor->id];
 	if (batch_reg->addr) {
 		int val;
-- 
2.39.5
Re: [PATCH v2 1/3] iio: imu: st_lsm6dsx: Set FIFO ODR for accelerometer and gyroscope only
Posted by Lorenzo Bianconi 3 weeks, 3 days ago 
> 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 gyroscope; 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>
> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> index 55d877745575..1ee2fc5f5f1f 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> @@ -225,6 +225,10 @@ static int st_lsm6dsx_set_fifo_odr(struct st_lsm6dsx_sensor *sensor,
>  	const struct st_lsm6dsx_reg *batch_reg;
>  	u8 data;
>  
> +	/* Only internal sensors have a FIFO ODR configuration register. */
> +	if (sensor->id >= ARRAY_SIZE(hw->settings->batch))
> +		return 0;

I guess it is more clear to check if the sensor is acc or gyro here.
What do you think? Something like:

	if (sensor->id != ST_LSM6DSX_ID_GYRO &&
	    sensor->id != ST_LSM6DSX_ID_ACC)
	    return 0;

Regards,
Lorenzo

> +
>  	batch_reg = &hw->settings->batch[sensor->id];
>  	if (batch_reg->addr) {
>  		int val;
> -- 
> 2.39.5
>
Re: [PATCH v2 1/3] iio: imu: st_lsm6dsx: Set FIFO ODR for accelerometer and gyroscope only
Posted by Jonathan Cameron 3 weeks, 2 days ago 
On Thu, 15 Jan 2026 14:13:01 +0100
Lorenzo Bianconi <lorenzo@kernel.org> 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 gyroscope; 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>
> > ---
> >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > index 55d877745575..1ee2fc5f5f1f 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > @@ -225,6 +225,10 @@ static int st_lsm6dsx_set_fifo_odr(struct st_lsm6dsx_sensor *sensor,
> >  	const struct st_lsm6dsx_reg *batch_reg;
> >  	u8 data;
> >  
> > +	/* Only internal sensors have a FIFO ODR configuration register. */
> > +	if (sensor->id >= ARRAY_SIZE(hw->settings->batch))
> > +		return 0;  
> 
> I guess it is more clear to check if the sensor is acc or gyro here.
> What do you think? Something like:
> 
> 	if (sensor->id != ST_LSM6DSX_ID_GYRO &&
> 	    sensor->id != ST_LSM6DSX_ID_ACC)
> 	    return 0;

Disadvantage is that to check for overflow we have to know those are 0 and 1.
I'm not sure which is better of the two here. One is more logically correct
the other is easier to review :)

> 
> Regards,
> Lorenzo
> 
> > +
> >  	batch_reg = &hw->settings->batch[sensor->id];
> >  	if (batch_reg->addr) {
> >  		int val;
> > -- 
> > 2.39.5
> >
Re: [PATCH v2 1/3] iio: imu: st_lsm6dsx: Set FIFO ODR for accelerometer and gyroscope only
Posted by Francesco Lavra 2 weeks, 6 days ago 
On Fri, 2026-01-16 at 19:47 +0000, Jonathan Cameron wrote:
> On Thu, 15 Jan 2026 14:13:01 +0100
> Lorenzo Bianconi <lorenzo@kernel.org> 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 gyroscope; 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>
> > > ---
> > >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > index 55d877745575..1ee2fc5f5f1f 100644
> > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > @@ -225,6 +225,10 @@ static int st_lsm6dsx_set_fifo_odr(struct
> > > st_lsm6dsx_sensor *sensor,
> > >         const struct st_lsm6dsx_reg *batch_reg;
> > >         u8 data;
> > >  
> > > +       /* Only internal sensors have a FIFO ODR configuration
> > > register. */
> > > +       if (sensor->id >= ARRAY_SIZE(hw->settings->batch))
> > > +               return 0;  
> > 
> > I guess it is more clear to check if the sensor is acc or gyro here.
> > What do you think? Something like:
> > 
> >         if (sensor->id != ST_LSM6DSX_ID_GYRO &&
> >             sensor->id != ST_LSM6DSX_ID_ACC)
> >             return 0;
> 
> Disadvantage is that to check for overflow we have to know those are 0
> and 1.
> I'm not sure which is better of the two here. One is more logically
> correct
> the other is easier to review :)

I'm keeping this as is, since there are pros and cons to changing it

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.