[PATCH v5 3/4] iio: imu: st_lsm6dsx: Fix check for invalid samples from FIFO

Francesco Lavra posted 4 patches 2 weeks, 2 days ago
[PATCH v5 3/4] iio: imu: st_lsm6dsx: Fix check for invalid samples from FIFO
Posted by Francesco Lavra 2 weeks, 2 days ago
The DRDY_MASK feature implemented in sensor chips marks gyroscope and
accelerometer invalid samples (i.e. samples that have been acquired during
the settling time of sensor filters) with the special values 0x7FFFh,
0x7FFE, and 0x7FFD.
The driver checks FIFO samples against these special values in order to
discard invalid samples; however, it does the check regardless of the type
of samples being processed, whereas this feature is specific to gyroscope
and accelerometer data. This could cause valid samples to be discarded.

Fix the above check so that it takes into account the type of samples being
processed. In st_lsm6dsx_push_tagged_data(), change the type of the data
parameter to __le16 *, to reflect the fact that this function is called
with an aligned data argument and avoid casting to __le16 * when checking
sample values.

Fixes: 960506ed2c69 ("iio: imu: st_lsm6dsx: enable drdy-mask if available")
Signed-off-by: Francesco Lavra <flavra@baylibre.com>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
index 5b28a3ffcc3d..ded9a96076e6 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -539,14 +539,14 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
 #define ST_LSM6DSX_INVALID_SAMPLE	0x7ffd
 static int
 st_lsm6dsx_push_tagged_data(struct st_lsm6dsx_hw *hw, u8 tag,
-			    u8 *data, s64 ts)
+			    __le16 *data, s64 ts)
 {
-	s16 val = le16_to_cpu(*(__le16 *)data);
 	struct st_lsm6dsx_sensor *sensor;
 	struct iio_dev *iio_dev;
 
 	/* invalid sample during bootstrap phase */
-	if (val >= ST_LSM6DSX_INVALID_SAMPLE)
+	if ((tag == ST_LSM6DSX_GYRO_TAG || tag == ST_LSM6DSX_ACC_TAG) &&
+	    (s16)le16_to_cpup(data) >= ST_LSM6DSX_INVALID_SAMPLE)
 		return -EINVAL;
 
 	/*
@@ -670,7 +670,8 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
 					reset_ts = true;
 				ts *= hw->ts_gain;
 			} else {
-				st_lsm6dsx_push_tagged_data(hw, tag, iio_buff,
+				st_lsm6dsx_push_tagged_data(hw, tag,
+							    (__le16 *)iio_buff,
 							    ts);
 			}
 		}
-- 
2.39.5
Re: [PATCH v5 3/4] iio: imu: st_lsm6dsx: Fix check for invalid samples from FIFO
Posted by Jonathan Cameron 2 weeks, 2 days ago
On Thu, 22 Jan 2026 17:23:34 +0100
Francesco Lavra <flavra@baylibre.com> wrote:

> The DRDY_MASK feature implemented in sensor chips marks gyroscope and
> accelerometer invalid samples (i.e. samples that have been acquired during
> the settling time of sensor filters) with the special values 0x7FFFh,
> 0x7FFE, and 0x7FFD.
> The driver checks FIFO samples against these special values in order to
> discard invalid samples; however, it does the check regardless of the type
> of samples being processed, whereas this feature is specific to gyroscope
> and accelerometer data. This could cause valid samples to be discarded.
> 
> Fix the above check so that it takes into account the type of samples being
> processed. In st_lsm6dsx_push_tagged_data(), change the type of the data
> parameter to __le16 *, to reflect the fact that this function is called
> with an aligned data argument and avoid casting to __le16 * when checking
> sample values.
Hi Francesco,

I'm going to guess Andy meant all the way up rather than pushing the cast
upwards.  I think this can be done in a fashion that cleans up the type
representation in general.
> 
> Fixes: 960506ed2c69 ("iio: imu: st_lsm6dsx: enable drdy-mask if available")
> Signed-off-by: Francesco Lavra <flavra@baylibre.com>


> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> index 5b28a3ffcc3d..ded9a96076e6 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> @@ -539,14 +539,14 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
>  #define ST_LSM6DSX_INVALID_SAMPLE	0x7ffd
>  static int
>  st_lsm6dsx_push_tagged_data(struct st_lsm6dsx_hw *hw, u8 tag,
> -			    u8 *data, s64 ts)
> +			    __le16 *data, s64 ts)
>  {
> -	s16 val = le16_to_cpu(*(__le16 *)data);
>  	struct st_lsm6dsx_sensor *sensor;
>  	struct iio_dev *iio_dev;
>  
>  	/* invalid sample during bootstrap phase */
> -	if (val >= ST_LSM6DSX_INVALID_SAMPLE)
> +	if ((tag == ST_LSM6DSX_GYRO_TAG || tag == ST_LSM6DSX_ACC_TAG) &&
> +	    (s16)le16_to_cpup(data) >= ST_LSM6DSX_INVALID_SAMPLE)
>  		return -EINVAL;
>  
>  	/*
> @@ -670,7 +670,8 @@ int st_lsm6dsx_read_tagged_fifo(struct st_lsm6dsx_hw *hw)
>  					reset_ts = true;
>  				ts *= hw->ts_gain;
>  			} else {
> -				st_lsm6dsx_push_tagged_data(hw, tag, iio_buff,
> +				st_lsm6dsx_push_tagged_data(hw, tag,
> +							    (__le16 *)iio_buff,

This is ugly but at least it's near the declaration so improvement on previous.

However, I smell a cleaner solution.  If I read the buffer definition correctly
it could be replaced with

struct {
	__le16 data[3];
	aligned_s64 timestamp;
} iio_buff = { };

//note the zeroing because the code never writes the hole which is
bad...  Also that the { } is guaranteed to fill the hole with the build options
the kernel uses (there is a selftest for this).


Which will let you pass iio_buf->data to this call.


>  							    ts);
>  			}
>  		}
Re: [PATCH v5 3/4] iio: imu: st_lsm6dsx: Fix check for invalid samples from FIFO
Posted by Andy Shevchenko 2 weeks, 2 days ago
On Thu, Jan 22, 2026 at 08:07:57PM +0000, Jonathan Cameron wrote:
> On Thu, 22 Jan 2026 17:23:34 +0100
> Francesco Lavra <flavra@baylibre.com> wrote:
> 
> > The DRDY_MASK feature implemented in sensor chips marks gyroscope and
> > accelerometer invalid samples (i.e. samples that have been acquired during
> > the settling time of sensor filters) with the special values 0x7FFFh,
> > 0x7FFE, and 0x7FFD.
> > The driver checks FIFO samples against these special values in order to
> > discard invalid samples; however, it does the check regardless of the type
> > of samples being processed, whereas this feature is specific to gyroscope
> > and accelerometer data. This could cause valid samples to be discarded.
> > 
> > Fix the above check so that it takes into account the type of samples being
> > processed. In st_lsm6dsx_push_tagged_data(), change the type of the data
> > parameter to __le16 *, to reflect the fact that this function is called
> > with an aligned data argument and avoid casting to __le16 * when checking
> > sample values.

> I'm going to guess Andy meant all the way up rather than pushing the cast
> upwards.  I think this can be done in a fashion that cleans up the type
> representation in general.

Indeed. This version is not so better than the previous.

...

> > -				st_lsm6dsx_push_tagged_data(hw, tag, iio_buff,
> > +				st_lsm6dsx_push_tagged_data(hw, tag,
> > +							    (__le16 *)iio_buff,
> 
> This is ugly but at least it's near the declaration so improvement on previous.
> 
> However, I smell a cleaner solution.  If I read the buffer definition correctly
> it could be replaced with
> 
> struct {
> 	__le16 data[3];
> 	aligned_s64 timestamp;
> } iio_buff = { };
> 
> //note the zeroing because the code never writes the hole which is
> bad...  Also that the { } is guaranteed to fill the hole with the build options
> the kernel uses (there is a selftest for this).
> 
> Which will let you pass iio_buf->data to this call.

Yes, this looks way better and allows to get rid of all assumptions and
castings.

-- 
With Best Regards,
Andy Shevchenko