[PATCH 0/2] fix read misalignment on LSM6DSM SensorHub

[PATCH 0/2] fix read misalignment on LSM6DSM SensorHub

[Lorenzo Bianconi]

fix read misalignment on untagged FIFO.
fix samples-in-pattern (sip) computation on LSM6DSM SensorHub

Lorenzo Bianconi (2):
  iio: imu: st_lsm6dsx: fix read misalignment on untagged FIFO
  iio: imu: st_lsm6dsx: specify slave odr in slv_odr

 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       |  3 ++
 .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 23 ++++++++++-----
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c  | 29 ++++++++++++++-----
 3 files changed, 41 insertions(+), 14 deletions(-)

-- 
2.24.1

[PATCH 1/2] iio: imu: st_lsm6dsx: fix read misalignment on untagged FIFO

[Lorenzo Bianconi]

st_lsm6dsx suffers of a read misalignment on untagged FIFO when
all 3 supported sensors (accel, gyro and ext device) are running
at different ODRs (the use-case is reported in the LSM6DSM Application
Note at pag 100).
Fix the issue taking into account decimation factor reading the FIFO
pattern.

Fixes: e485e2a2cfd6 ("iio: imu: st_lsm6dsx: enable sensor-hub support for lsm6dsm")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       |  2 ++
 .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 23 +++++++++++++------
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index f2113a63721a..e9e756b1e92f 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -337,6 +337,7 @@ enum st_lsm6dsx_fifo_mode {
  * @gain: Configured sensor sensitivity.
  * @odr: Output data rate of the sensor [Hz].
  * @watermark: Sensor watermark level.
+ * @decimator: Sensor decimation factor.
  * @sip: Number of samples in a given pattern.
  * @ts_ref: Sensor timestamp reference for hw one.
  * @ext_info: Sensor settings if it is connected to i2c controller
@@ -350,6 +351,7 @@ struct st_lsm6dsx_sensor {
 	u32 odr;
 
 	u16 watermark;
+	u8 decimator;
 	u8 sip;
 	s64 ts_ref;
 
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
index bb899345f2bb..afd00daeefb2 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -93,6 +93,7 @@ st_lsm6dsx_get_decimator_val(struct st_lsm6dsx_sensor *sensor, u32 max_odr)
 			break;
 	}
 
+	sensor->decimator = decimator;
 	return i == max_size ? 0 : st_lsm6dsx_decimator_table[i].val;
 }
 
@@ -337,7 +338,7 @@ static inline int st_lsm6dsx_read_block(struct st_lsm6dsx_hw *hw, u8 addr,
 int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
 {
 	struct st_lsm6dsx_sensor *acc_sensor, *gyro_sensor, *ext_sensor = NULL;
-	int err, acc_sip, gyro_sip, ts_sip, ext_sip, read_len, offset;
+	int err, sip, acc_sip, gyro_sip, ts_sip, ext_sip, read_len, offset;
 	u16 fifo_len, pattern_len = hw->sip * ST_LSM6DSX_SAMPLE_SIZE;
 	u16 fifo_diff_mask = hw->settings->fifo_ops.fifo_diff.mask;
 	u8 gyro_buff[ST_LSM6DSX_IIO_BUFF_SIZE];
@@ -399,19 +400,20 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
 		acc_sip = acc_sensor->sip;
 		ts_sip = hw->ts_sip;
 		offset = 0;
+		sip = 0;
 
 		while (acc_sip > 0 || gyro_sip > 0 || ext_sip > 0) {
-			if (gyro_sip > 0) {
+			if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) {
 				memcpy(gyro_buff, &hw->buff[offset],
 				       ST_LSM6DSX_SAMPLE_SIZE);
 				offset += ST_LSM6DSX_SAMPLE_SIZE;
 			}
-			if (acc_sip > 0) {
+			if (acc_sip > 0 && !(sip % acc_sensor->decimator)) {
 				memcpy(acc_buff, &hw->buff[offset],
 				       ST_LSM6DSX_SAMPLE_SIZE);
 				offset += ST_LSM6DSX_SAMPLE_SIZE;
 			}
-			if (ext_sip > 0) {
+			if (ext_sip > 0 && !(sip % ext_sensor->decimator)) {
 				memcpy(ext_buff, &hw->buff[offset],
 				       ST_LSM6DSX_SAMPLE_SIZE);
 				offset += ST_LSM6DSX_SAMPLE_SIZE;
@@ -441,18 +443,25 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
 				offset += ST_LSM6DSX_SAMPLE_SIZE;
 			}
 
-			if (gyro_sip-- > 0)
+			if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) {
 				iio_push_to_buffers_with_timestamp(
 					hw->iio_devs[ST_LSM6DSX_ID_GYRO],
 					gyro_buff, gyro_sensor->ts_ref + ts);
-			if (acc_sip-- > 0)
+				gyro_sip--;
+			}
+			if (acc_sip > 0 && !(sip % acc_sensor->decimator)) {
 				iio_push_to_buffers_with_timestamp(
 					hw->iio_devs[ST_LSM6DSX_ID_ACC],
 					acc_buff, acc_sensor->ts_ref + ts);
-			if (ext_sip-- > 0)
+				acc_sip--;
+			}
+			if (ext_sip > 0 && !(sip % ext_sensor->decimator)) {
 				iio_push_to_buffers_with_timestamp(
 					hw->iio_devs[ST_LSM6DSX_ID_EXT0],
 					ext_buff, ext_sensor->ts_ref + ts);
+				ext_sip--;
+			}
+			sip++;
 		}
 	}
 
-- 
2.24.1

Re: [PATCH 1/2] iio: imu: st_lsm6dsx: fix read misalignment on untagged FIFO

[Jonathan Cameron]

On Fri, 13 Mar 2020 18:54:41 +0100
Lorenzo Bianconi <lorenzo@kernel.org> wrote:

> st_lsm6dsx suffers of a read misalignment on untagged FIFO when
> all 3 supported sensors (accel, gyro and ext device) are running
> at different ODRs (the use-case is reported in the LSM6DSM Application
> Note at pag 100).
> Fix the issue taking into account decimation factor reading the FIFO
> pattern.
> 
> Fixes: e485e2a2cfd6 ("iio: imu: st_lsm6dsx: enable sensor-hub support for lsm6dsm")
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
Queued up.  Not totally sure which branch this will through, but may
well be in next merge window because of timing.

Thanks,

Jonathan

> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       |  2 ++
>  .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 23 +++++++++++++------
>  2 files changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index f2113a63721a..e9e756b1e92f 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -337,6 +337,7 @@ enum st_lsm6dsx_fifo_mode {
>   * @gain: Configured sensor sensitivity.
>   * @odr: Output data rate of the sensor [Hz].
>   * @watermark: Sensor watermark level.
> + * @decimator: Sensor decimation factor.
>   * @sip: Number of samples in a given pattern.
>   * @ts_ref: Sensor timestamp reference for hw one.
>   * @ext_info: Sensor settings if it is connected to i2c controller
> @@ -350,6 +351,7 @@ struct st_lsm6dsx_sensor {
>  	u32 odr;
>  
>  	u16 watermark;
> +	u8 decimator;
>  	u8 sip;
>  	s64 ts_ref;
>  
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> index bb899345f2bb..afd00daeefb2 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> @@ -93,6 +93,7 @@ st_lsm6dsx_get_decimator_val(struct st_lsm6dsx_sensor *sensor, u32 max_odr)
>  			break;
>  	}
>  
> +	sensor->decimator = decimator;
>  	return i == max_size ? 0 : st_lsm6dsx_decimator_table[i].val;
>  }
>  
> @@ -337,7 +338,7 @@ static inline int st_lsm6dsx_read_block(struct st_lsm6dsx_hw *hw, u8 addr,
>  int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
>  {
>  	struct st_lsm6dsx_sensor *acc_sensor, *gyro_sensor, *ext_sensor = NULL;
> -	int err, acc_sip, gyro_sip, ts_sip, ext_sip, read_len, offset;
> +	int err, sip, acc_sip, gyro_sip, ts_sip, ext_sip, read_len, offset;
>  	u16 fifo_len, pattern_len = hw->sip * ST_LSM6DSX_SAMPLE_SIZE;
>  	u16 fifo_diff_mask = hw->settings->fifo_ops.fifo_diff.mask;
>  	u8 gyro_buff[ST_LSM6DSX_IIO_BUFF_SIZE];
> @@ -399,19 +400,20 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
>  		acc_sip = acc_sensor->sip;
>  		ts_sip = hw->ts_sip;
>  		offset = 0;
> +		sip = 0;
>  
>  		while (acc_sip > 0 || gyro_sip > 0 || ext_sip > 0) {
> -			if (gyro_sip > 0) {
> +			if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) {
>  				memcpy(gyro_buff, &hw->buff[offset],
>  				       ST_LSM6DSX_SAMPLE_SIZE);
>  				offset += ST_LSM6DSX_SAMPLE_SIZE;
>  			}
> -			if (acc_sip > 0) {
> +			if (acc_sip > 0 && !(sip % acc_sensor->decimator)) {
>  				memcpy(acc_buff, &hw->buff[offset],
>  				       ST_LSM6DSX_SAMPLE_SIZE);
>  				offset += ST_LSM6DSX_SAMPLE_SIZE;
>  			}
> -			if (ext_sip > 0) {
> +			if (ext_sip > 0 && !(sip % ext_sensor->decimator)) {
>  				memcpy(ext_buff, &hw->buff[offset],
>  				       ST_LSM6DSX_SAMPLE_SIZE);
>  				offset += ST_LSM6DSX_SAMPLE_SIZE;
> @@ -441,18 +443,25 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
>  				offset += ST_LSM6DSX_SAMPLE_SIZE;
>  			}
>  
> -			if (gyro_sip-- > 0)
> +			if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) {
>  				iio_push_to_buffers_with_timestamp(
>  					hw->iio_devs[ST_LSM6DSX_ID_GYRO],
>  					gyro_buff, gyro_sensor->ts_ref + ts);
> -			if (acc_sip-- > 0)
> +				gyro_sip--;
> +			}
> +			if (acc_sip > 0 && !(sip % acc_sensor->decimator)) {
>  				iio_push_to_buffers_with_timestamp(
>  					hw->iio_devs[ST_LSM6DSX_ID_ACC],
>  					acc_buff, acc_sensor->ts_ref + ts);
> -			if (ext_sip-- > 0)
> +				acc_sip--;
> +			}
> +			if (ext_sip > 0 && !(sip % ext_sensor->decimator)) {
>  				iio_push_to_buffers_with_timestamp(
>  					hw->iio_devs[ST_LSM6DSX_ID_EXT0],
>  					ext_buff, ext_sensor->ts_ref + ts);
> +				ext_sip--;
> +			}
> +			sip++;
>  		}
>  	}
>  

Re: [PATCH 1/2] iio: imu: st_lsm6dsx: fix read misalignment on untagged FIFO

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 4915 bytes --]

On Mar 15, Jonathan Cameron wrote:
> On Fri, 13 Mar 2020 18:54:41 +0100
> Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> 
> > st_lsm6dsx suffers of a read misalignment on untagged FIFO when
> > all 3 supported sensors (accel, gyro and ext device) are running
> > at different ODRs (the use-case is reported in the LSM6DSM Application
> > Note at pag 100).
> > Fix the issue taking into account decimation factor reading the FIFO
> > pattern.
> > 
> > Fixes: e485e2a2cfd6 ("iio: imu: st_lsm6dsx: enable sensor-hub support for lsm6dsm")
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> Queued up.  Not totally sure which branch this will through, but may
> well be in next merge window because of timing.

I guess on the stable branch since e485e2a2cfd6 is already in Linus's tree

Regards,
Lorenzo

> 
> Thanks,
> 
> Jonathan
> 
> > ---
> >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       |  2 ++
> >  .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 23 +++++++++++++------
> >  2 files changed, 18 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > index f2113a63721a..e9e756b1e92f 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > @@ -337,6 +337,7 @@ enum st_lsm6dsx_fifo_mode {
> >   * @gain: Configured sensor sensitivity.
> >   * @odr: Output data rate of the sensor [Hz].
> >   * @watermark: Sensor watermark level.
> > + * @decimator: Sensor decimation factor.
> >   * @sip: Number of samples in a given pattern.
> >   * @ts_ref: Sensor timestamp reference for hw one.
> >   * @ext_info: Sensor settings if it is connected to i2c controller
> > @@ -350,6 +351,7 @@ struct st_lsm6dsx_sensor {
> >  	u32 odr;
> >  
> >  	u16 watermark;
> > +	u8 decimator;
> >  	u8 sip;
> >  	s64 ts_ref;
> >  
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > index bb899345f2bb..afd00daeefb2 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > @@ -93,6 +93,7 @@ st_lsm6dsx_get_decimator_val(struct st_lsm6dsx_sensor *sensor, u32 max_odr)
> >  			break;
> >  	}
> >  
> > +	sensor->decimator = decimator;
> >  	return i == max_size ? 0 : st_lsm6dsx_decimator_table[i].val;
> >  }
> >  
> > @@ -337,7 +338,7 @@ static inline int st_lsm6dsx_read_block(struct st_lsm6dsx_hw *hw, u8 addr,
> >  int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
> >  {
> >  	struct st_lsm6dsx_sensor *acc_sensor, *gyro_sensor, *ext_sensor = NULL;
> > -	int err, acc_sip, gyro_sip, ts_sip, ext_sip, read_len, offset;
> > +	int err, sip, acc_sip, gyro_sip, ts_sip, ext_sip, read_len, offset;
> >  	u16 fifo_len, pattern_len = hw->sip * ST_LSM6DSX_SAMPLE_SIZE;
> >  	u16 fifo_diff_mask = hw->settings->fifo_ops.fifo_diff.mask;
> >  	u8 gyro_buff[ST_LSM6DSX_IIO_BUFF_SIZE];
> > @@ -399,19 +400,20 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
> >  		acc_sip = acc_sensor->sip;
> >  		ts_sip = hw->ts_sip;
> >  		offset = 0;
> > +		sip = 0;
> >  
> >  		while (acc_sip > 0 || gyro_sip > 0 || ext_sip > 0) {
> > -			if (gyro_sip > 0) {
> > +			if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) {
> >  				memcpy(gyro_buff, &hw->buff[offset],
> >  				       ST_LSM6DSX_SAMPLE_SIZE);
> >  				offset += ST_LSM6DSX_SAMPLE_SIZE;
> >  			}
> > -			if (acc_sip > 0) {
> > +			if (acc_sip > 0 && !(sip % acc_sensor->decimator)) {
> >  				memcpy(acc_buff, &hw->buff[offset],
> >  				       ST_LSM6DSX_SAMPLE_SIZE);
> >  				offset += ST_LSM6DSX_SAMPLE_SIZE;
> >  			}
> > -			if (ext_sip > 0) {
> > +			if (ext_sip > 0 && !(sip % ext_sensor->decimator)) {
> >  				memcpy(ext_buff, &hw->buff[offset],
> >  				       ST_LSM6DSX_SAMPLE_SIZE);
> >  				offset += ST_LSM6DSX_SAMPLE_SIZE;
> > @@ -441,18 +443,25 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
> >  				offset += ST_LSM6DSX_SAMPLE_SIZE;
> >  			}
> >  
> > -			if (gyro_sip-- > 0)
> > +			if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) {
> >  				iio_push_to_buffers_with_timestamp(
> >  					hw->iio_devs[ST_LSM6DSX_ID_GYRO],
> >  					gyro_buff, gyro_sensor->ts_ref + ts);
> > -			if (acc_sip-- > 0)
> > +				gyro_sip--;
> > +			}
> > +			if (acc_sip > 0 && !(sip % acc_sensor->decimator)) {
> >  				iio_push_to_buffers_with_timestamp(
> >  					hw->iio_devs[ST_LSM6DSX_ID_ACC],
> >  					acc_buff, acc_sensor->ts_ref + ts);
> > -			if (ext_sip-- > 0)
> > +				acc_sip--;
> > +			}
> > +			if (ext_sip > 0 && !(sip % ext_sensor->decimator)) {
> >  				iio_push_to_buffers_with_timestamp(
> >  					hw->iio_devs[ST_LSM6DSX_ID_EXT0],
> >  					ext_buff, ext_sensor->ts_ref + ts);
> > +				ext_sip--;
> > +			}
> > +			sip++;
> >  		}
> >  	}
> >  
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] iio: imu: st_lsm6dsx: fix read misalignment on untagged FIFO

[Jonathan Cameron]

On Sun, 15 Mar 2020 13:05:13 +0100
Lorenzo Bianconi <lorenzo@kernel.org> wrote:

> On Mar 15, Jonathan Cameron wrote:
> > On Fri, 13 Mar 2020 18:54:41 +0100
> > Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> >   
> > > st_lsm6dsx suffers of a read misalignment on untagged FIFO when
> > > all 3 supported sensors (accel, gyro and ext device) are running
> > > at different ODRs (the use-case is reported in the LSM6DSM Application
> > > Note at pag 100).
> > > Fix the issue taking into account decimation factor reading the FIFO
> > > pattern.
> > > 
> > > Fixes: e485e2a2cfd6 ("iio: imu: st_lsm6dsx: enable sensor-hub support for lsm6dsm")
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>  
> > Queued up.  Not totally sure which branch this will through, but may
> > well be in next merge window because of timing.  
> 
> I guess on the stable branch since e485e2a2cfd6 is already in Linus's tree

Agreed, but fastest path to there might be via the merge window depending
on when I get a pull request out.

We are only rc5 I think. rc6 will be today most likely.  Only leaves a week
before we are too close to the possible merge window opening.

As I have an outstanding pull request with Greg, that may not be enough time.

If this goes in the merge window, it'll hit first or second stable release so before
any distros etc pick up the new kernel anyway.

Jonathan

> 
> Regards,
> Lorenzo
> 
> > 
> > Thanks,
> > 
> > Jonathan
> >   
> > > ---
> > >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       |  2 ++
> > >  .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 23 +++++++++++++------
> > >  2 files changed, 18 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > > index f2113a63721a..e9e756b1e92f 100644
> > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > > @@ -337,6 +337,7 @@ enum st_lsm6dsx_fifo_mode {
> > >   * @gain: Configured sensor sensitivity.
> > >   * @odr: Output data rate of the sensor [Hz].
> > >   * @watermark: Sensor watermark level.
> > > + * @decimator: Sensor decimation factor.
> > >   * @sip: Number of samples in a given pattern.
> > >   * @ts_ref: Sensor timestamp reference for hw one.
> > >   * @ext_info: Sensor settings if it is connected to i2c controller
> > > @@ -350,6 +351,7 @@ struct st_lsm6dsx_sensor {
> > >  	u32 odr;
> > >  
> > >  	u16 watermark;
> > > +	u8 decimator;
> > >  	u8 sip;
> > >  	s64 ts_ref;
> > >  
> > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > index bb899345f2bb..afd00daeefb2 100644
> > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > @@ -93,6 +93,7 @@ st_lsm6dsx_get_decimator_val(struct st_lsm6dsx_sensor *sensor, u32 max_odr)
> > >  			break;
> > >  	}
> > >  
> > > +	sensor->decimator = decimator;
> > >  	return i == max_size ? 0 : st_lsm6dsx_decimator_table[i].val;
> > >  }
> > >  
> > > @@ -337,7 +338,7 @@ static inline int st_lsm6dsx_read_block(struct st_lsm6dsx_hw *hw, u8 addr,
> > >  int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
> > >  {
> > >  	struct st_lsm6dsx_sensor *acc_sensor, *gyro_sensor, *ext_sensor = NULL;
> > > -	int err, acc_sip, gyro_sip, ts_sip, ext_sip, read_len, offset;
> > > +	int err, sip, acc_sip, gyro_sip, ts_sip, ext_sip, read_len, offset;
> > >  	u16 fifo_len, pattern_len = hw->sip * ST_LSM6DSX_SAMPLE_SIZE;
> > >  	u16 fifo_diff_mask = hw->settings->fifo_ops.fifo_diff.mask;
> > >  	u8 gyro_buff[ST_LSM6DSX_IIO_BUFF_SIZE];
> > > @@ -399,19 +400,20 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
> > >  		acc_sip = acc_sensor->sip;
> > >  		ts_sip = hw->ts_sip;
> > >  		offset = 0;
> > > +		sip = 0;
> > >  
> > >  		while (acc_sip > 0 || gyro_sip > 0 || ext_sip > 0) {
> > > -			if (gyro_sip > 0) {
> > > +			if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) {
> > >  				memcpy(gyro_buff, &hw->buff[offset],
> > >  				       ST_LSM6DSX_SAMPLE_SIZE);
> > >  				offset += ST_LSM6DSX_SAMPLE_SIZE;
> > >  			}
> > > -			if (acc_sip > 0) {
> > > +			if (acc_sip > 0 && !(sip % acc_sensor->decimator)) {
> > >  				memcpy(acc_buff, &hw->buff[offset],
> > >  				       ST_LSM6DSX_SAMPLE_SIZE);
> > >  				offset += ST_LSM6DSX_SAMPLE_SIZE;
> > >  			}
> > > -			if (ext_sip > 0) {
> > > +			if (ext_sip > 0 && !(sip % ext_sensor->decimator)) {
> > >  				memcpy(ext_buff, &hw->buff[offset],
> > >  				       ST_LSM6DSX_SAMPLE_SIZE);
> > >  				offset += ST_LSM6DSX_SAMPLE_SIZE;
> > > @@ -441,18 +443,25 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
> > >  				offset += ST_LSM6DSX_SAMPLE_SIZE;
> > >  			}
> > >  
> > > -			if (gyro_sip-- > 0)
> > > +			if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) {
> > >  				iio_push_to_buffers_with_timestamp(
> > >  					hw->iio_devs[ST_LSM6DSX_ID_GYRO],
> > >  					gyro_buff, gyro_sensor->ts_ref + ts);
> > > -			if (acc_sip-- > 0)
> > > +				gyro_sip--;
> > > +			}
> > > +			if (acc_sip > 0 && !(sip % acc_sensor->decimator)) {
> > >  				iio_push_to_buffers_with_timestamp(
> > >  					hw->iio_devs[ST_LSM6DSX_ID_ACC],
> > >  					acc_buff, acc_sensor->ts_ref + ts);
> > > -			if (ext_sip-- > 0)
> > > +				acc_sip--;
> > > +			}
> > > +			if (ext_sip > 0 && !(sip % ext_sensor->decimator)) {
> > >  				iio_push_to_buffers_with_timestamp(
> > >  					hw->iio_devs[ST_LSM6DSX_ID_EXT0],
> > >  					ext_buff, ext_sensor->ts_ref + ts);
> > > +				ext_sip--;
> > > +			}
> > > +			sip++;
> > >  		}
> > >  	}
> > >    
> >   

[PATCH 2/2] iio: imu: st_lsm6dsx: specify slave odr in slv_odr

[Lorenzo Bianconi]

Introduce slv_odr in ext_info data structure in order to distinguish
between sensor hub trigger (accel sensor) odr and i2c slave odr and
properly compute samples in FIFO pattern

Fixes: e485e2a2cfd6 ("iio: imu: st_lsm6dsx: enable sensor-hub support for lsm6dsm")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  1 +
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c | 29 +++++++++++++++-----
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index e9e756b1e92f..41cb20cb3809 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -357,6 +357,7 @@ struct st_lsm6dsx_sensor {
 
 	struct {
 		const struct st_lsm6dsx_ext_dev_settings *settings;
+		u32 slv_odr;
 		u8 addr;
 	} ext_info;
 };
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
index 95ddd19d1aa7..64ef07a30726 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
@@ -421,7 +421,8 @@ int st_lsm6dsx_shub_set_enable(struct st_lsm6dsx_sensor *sensor, bool enable)
 
 	settings = sensor->ext_info.settings;
 	if (enable) {
-		err = st_lsm6dsx_shub_set_odr(sensor, sensor->odr);
+		err = st_lsm6dsx_shub_set_odr(sensor,
+					      sensor->ext_info.slv_odr);
 		if (err < 0)
 			return err;
 	} else {
@@ -459,7 +460,7 @@ st_lsm6dsx_shub_read_oneshot(struct st_lsm6dsx_sensor *sensor,
 	if (err < 0)
 		return err;
 
-	delay = 1000000000 / sensor->odr;
+	delay = 1000000000 / sensor->ext_info.slv_odr;
 	usleep_range(delay, 2 * delay);
 
 	len = min_t(int, sizeof(data), ch->scan_type.realbits >> 3);
@@ -500,8 +501,8 @@ st_lsm6dsx_shub_read_raw(struct iio_dev *iio_dev,
 		iio_device_release_direct_mode(iio_dev);
 		break;
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		*val = sensor->odr / 1000;
-		*val2 = (sensor->odr % 1000) * 1000;
+		*val = sensor->ext_info.slv_odr / 1000;
+		*val2 = (sensor->ext_info.slv_odr % 1000) * 1000;
 		ret = IIO_VAL_INT_PLUS_MICRO;
 		break;
 	case IIO_CHAN_INFO_SCALE:
@@ -535,8 +536,20 @@ st_lsm6dsx_shub_write_raw(struct iio_dev *iio_dev,
 
 		val = val * 1000 + val2 / 1000;
 		err = st_lsm6dsx_shub_get_odr_val(sensor, val, &data);
-		if (!err)
-			sensor->odr = val;
+		if (!err) {
+			struct st_lsm6dsx_hw *hw = sensor->hw;
+			struct st_lsm6dsx_sensor *ref_sensor;
+			u8 odr_val;
+			int odr;
+
+			ref_sensor = iio_priv(hw->iio_devs[ST_LSM6DSX_ID_ACC]);
+			odr = st_lsm6dsx_check_odr(ref_sensor, val, &odr_val);
+			if (odr < 0)
+				return odr;
+
+			sensor->ext_info.slv_odr = val;
+			sensor->odr = odr;
+		}
 		break;
 	}
 	default:
@@ -613,6 +626,7 @@ st_lsm6dsx_shub_alloc_iiodev(struct st_lsm6dsx_hw *hw,
 			     const struct st_lsm6dsx_ext_dev_settings *info,
 			     u8 i2c_addr, const char *name)
 {
+	enum st_lsm6dsx_sensor_id ref_id = ST_LSM6DSX_ID_ACC;
 	struct iio_chan_spec *ext_channels;
 	struct st_lsm6dsx_sensor *sensor;
 	struct iio_dev *iio_dev;
@@ -628,7 +642,8 @@ st_lsm6dsx_shub_alloc_iiodev(struct st_lsm6dsx_hw *hw,
 	sensor = iio_priv(iio_dev);
 	sensor->id = id;
 	sensor->hw = hw;
-	sensor->odr = info->odr_table.odr_avl[0].milli_hz;
+	sensor->odr = hw->settings->odr_table[ref_id].odr_avl[0].milli_hz;
+	sensor->ext_info.slv_odr = info->odr_table.odr_avl[0].milli_hz;
 	sensor->gain = info->fs_table.fs_avl[0].gain;
 	sensor->ext_info.settings = info;
 	sensor->ext_info.addr = i2c_addr;
-- 
2.24.1

Re: [PATCH 2/2] iio: imu: st_lsm6dsx: specify slave odr in slv_odr

[Jonathan Cameron]

On Fri, 13 Mar 2020 18:54:42 +0100
Lorenzo Bianconi <lorenzo@kernel.org> wrote:

> Introduce slv_odr in ext_info data structure in order to distinguish
> between sensor hub trigger (accel sensor) odr and i2c slave odr and
> properly compute samples in FIFO pattern
> 
> Fixes: e485e2a2cfd6 ("iio: imu: st_lsm6dsx: enable sensor-hub support for lsm6dsm")
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

queued up.

> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  1 +
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c | 29 +++++++++++++++-----
>  2 files changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index e9e756b1e92f..41cb20cb3809 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -357,6 +357,7 @@ struct st_lsm6dsx_sensor {
>  
>  	struct {
>  		const struct st_lsm6dsx_ext_dev_settings *settings;
> +		u32 slv_odr;
>  		u8 addr;
>  	} ext_info;
>  };
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
> index 95ddd19d1aa7..64ef07a30726 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_shub.c
> @@ -421,7 +421,8 @@ int st_lsm6dsx_shub_set_enable(struct st_lsm6dsx_sensor *sensor, bool enable)
>  
>  	settings = sensor->ext_info.settings;
>  	if (enable) {
> -		err = st_lsm6dsx_shub_set_odr(sensor, sensor->odr);
> +		err = st_lsm6dsx_shub_set_odr(sensor,
> +					      sensor->ext_info.slv_odr);
>  		if (err < 0)
>  			return err;
>  	} else {
> @@ -459,7 +460,7 @@ st_lsm6dsx_shub_read_oneshot(struct st_lsm6dsx_sensor *sensor,
>  	if (err < 0)
>  		return err;
>  
> -	delay = 1000000000 / sensor->odr;
> +	delay = 1000000000 / sensor->ext_info.slv_odr;
>  	usleep_range(delay, 2 * delay);
>  
>  	len = min_t(int, sizeof(data), ch->scan_type.realbits >> 3);
> @@ -500,8 +501,8 @@ st_lsm6dsx_shub_read_raw(struct iio_dev *iio_dev,
>  		iio_device_release_direct_mode(iio_dev);
>  		break;
>  	case IIO_CHAN_INFO_SAMP_FREQ:
> -		*val = sensor->odr / 1000;
> -		*val2 = (sensor->odr % 1000) * 1000;
> +		*val = sensor->ext_info.slv_odr / 1000;
> +		*val2 = (sensor->ext_info.slv_odr % 1000) * 1000;
>  		ret = IIO_VAL_INT_PLUS_MICRO;
>  		break;
>  	case IIO_CHAN_INFO_SCALE:
> @@ -535,8 +536,20 @@ st_lsm6dsx_shub_write_raw(struct iio_dev *iio_dev,
>  
>  		val = val * 1000 + val2 / 1000;
>  		err = st_lsm6dsx_shub_get_odr_val(sensor, val, &data);
> -		if (!err)
> -			sensor->odr = val;
> +		if (!err) {
> +			struct st_lsm6dsx_hw *hw = sensor->hw;
> +			struct st_lsm6dsx_sensor *ref_sensor;
> +			u8 odr_val;
> +			int odr;
> +
> +			ref_sensor = iio_priv(hw->iio_devs[ST_LSM6DSX_ID_ACC]);
> +			odr = st_lsm6dsx_check_odr(ref_sensor, val, &odr_val);
> +			if (odr < 0)
> +				return odr;
> +
> +			sensor->ext_info.slv_odr = val;
> +			sensor->odr = odr;
> +		}
>  		break;
>  	}
>  	default:
> @@ -613,6 +626,7 @@ st_lsm6dsx_shub_alloc_iiodev(struct st_lsm6dsx_hw *hw,
>  			     const struct st_lsm6dsx_ext_dev_settings *info,
>  			     u8 i2c_addr, const char *name)
>  {
> +	enum st_lsm6dsx_sensor_id ref_id = ST_LSM6DSX_ID_ACC;
>  	struct iio_chan_spec *ext_channels;
>  	struct st_lsm6dsx_sensor *sensor;
>  	struct iio_dev *iio_dev;
> @@ -628,7 +642,8 @@ st_lsm6dsx_shub_alloc_iiodev(struct st_lsm6dsx_hw *hw,
>  	sensor = iio_priv(iio_dev);
>  	sensor->id = id;
>  	sensor->hw = hw;
> -	sensor->odr = info->odr_table.odr_avl[0].milli_hz;
> +	sensor->odr = hw->settings->odr_table[ref_id].odr_avl[0].milli_hz;
> +	sensor->ext_info.slv_odr = info->odr_table.odr_avl[0].milli_hz;
>  	sensor->gain = info->fs_table.fs_avl[0].gain;
>  	sensor->ext_info.settings = info;
>  	sensor->ext_info.addr = i2c_addr;