[PATCH] iio: imu: st_lsm6dsx: discard samples during filters settling time

[PATCH] iio: imu: st_lsm6dsx: discard samples during filters settling time

[Lorenzo Bianconi]

During digital filters settling time the driver is expected to drop
samples since they can be corrupted. Introduce the capability to drop
a given number of samples according to the configured ODR.
Add the sample_to_discard data for LSM6DSM sensor.

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       | 11 ++++
 .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 58 +++++++++++++++----
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  | 18 ++++++
 3 files changed, 77 insertions(+), 10 deletions(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index 499fcf8875b4..8e119d78730b 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
@@ -137,6 +137,13 @@ struct st_lsm6dsx_odr_table_entry {
 	int odr_len;
 };
 
+struct st_lsm6dsx_samples_to_discard {
+	struct {
+		u32 milli_hz;
+		u16 samples;
+	} val[ST_LSM6DSX_ODR_LIST_SIZE];
+};
+
 struct st_lsm6dsx_fs {
 	u32 gain;
 	u8 val;
@@ -291,6 +298,7 @@ struct st_lsm6dsx_ext_dev_settings {
  * @irq_config: interrupts related registers.
  * @drdy_mask: register info for data-ready mask (addr + mask).
  * @odr_table: Hw sensors odr table (Hz + val).
+ * @samples_to_discard: Number of samples to discard for filters settling time.
  * @fs_table: Hw sensors gain table (gain + val).
  * @decimator: List of decimator register info (addr + mask).
  * @batch: List of FIFO batching register info (addr + mask).
@@ -323,6 +331,7 @@ struct st_lsm6dsx_settings {
 	} irq_config;
 	struct st_lsm6dsx_reg drdy_mask;
 	struct st_lsm6dsx_odr_table_entry odr_table[2];
+	struct st_lsm6dsx_samples_to_discard samples_to_discard[2];
 	struct st_lsm6dsx_fs_table_entry fs_table[2];
 	struct st_lsm6dsx_reg decimator[ST_LSM6DSX_MAX_ID];
 	struct st_lsm6dsx_reg batch[ST_LSM6DSX_MAX_ID];
@@ -353,6 +362,7 @@ enum st_lsm6dsx_fifo_mode {
  * @hw: Pointer to instance of struct st_lsm6dsx_hw.
  * @gain: Configured sensor sensitivity.
  * @odr: Output data rate of the sensor [Hz].
+ * @samples_to_discard: Number of samples to discard for filters settling time.
  * @watermark: Sensor watermark level.
  * @decimator: Sensor decimation factor.
  * @sip: Number of samples in a given pattern.
@@ -367,6 +377,7 @@ struct st_lsm6dsx_sensor {
 	u32 gain;
 	u32 odr;
 
+	u16 samples_to_discard;
 	u16 watermark;
 	u8 decimator;
 	u8 sip;
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
index 7dd5205aea5b..c1059a79f5ff 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -457,17 +457,29 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
 			}
 
 			if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) {
-				iio_push_to_buffers_with_timestamp(
-					hw->iio_devs[ST_LSM6DSX_ID_GYRO],
-					&hw->scan[ST_LSM6DSX_ID_GYRO],
-					gyro_sensor->ts_ref + ts);
+				/* We need to discards gyro samples during
+				 * filters settling time
+				 */
+				if (gyro_sensor->samples_to_discard > 0)
+					gyro_sensor->samples_to_discard--;
+				else
+					iio_push_to_buffers_with_timestamp(
+						hw->iio_devs[ST_LSM6DSX_ID_GYRO],
+						&hw->scan[ST_LSM6DSX_ID_GYRO],
+						gyro_sensor->ts_ref + ts);
 				gyro_sip--;
 			}
 			if (acc_sip > 0 && !(sip % acc_sensor->decimator)) {
-				iio_push_to_buffers_with_timestamp(
-					hw->iio_devs[ST_LSM6DSX_ID_ACC],
-					&hw->scan[ST_LSM6DSX_ID_ACC],
-					acc_sensor->ts_ref + ts);
+				/* We need to discards accel samples during
+				 * filters settling time
+				 */
+				if (acc_sensor->samples_to_discard > 0)
+					acc_sensor->samples_to_discard--;
+				else
+					iio_push_to_buffers_with_timestamp(
+						hw->iio_devs[ST_LSM6DSX_ID_ACC],
+						&hw->scan[ST_LSM6DSX_ID_ACC],
+						acc_sensor->ts_ref + ts);
 				acc_sip--;
 			}
 			if (ext_sip > 0 && !(sip % ext_sensor->decimator)) {
@@ -541,8 +553,12 @@ st_lsm6dsx_push_tagged_data(struct st_lsm6dsx_hw *hw, u8 tag,
 	}
 
 	sensor = iio_priv(iio_dev);
-	iio_push_to_buffers_with_timestamp(iio_dev, data,
-					   ts + sensor->ts_ref);
+	/* We need to discards gyro samples during filters settling time */
+	if (sensor->samples_to_discard > 0)
+		sensor->samples_to_discard--;
+	else
+		iio_push_to_buffers_with_timestamp(iio_dev, data,
+						   ts + sensor->ts_ref);
 
 	return 0;
 }
@@ -654,6 +670,25 @@ int st_lsm6dsx_flush_fifo(struct st_lsm6dsx_hw *hw)
 	return err;
 }
 
+static void
+st_lsm6dsx_update_samples_to_discard(struct st_lsm6dsx_sensor *sensor)
+{
+	const struct st_lsm6dsx_samples_to_discard *data;
+	int i;
+
+	if (sensor->id != ST_LSM6DSX_ID_GYRO &&
+	    sensor->id != ST_LSM6DSX_ID_ACC)
+		return;
+
+	data = &sensor->hw->settings->samples_to_discard[sensor->id];
+	for (i = 0; i < ST_LSM6DSX_ODR_LIST_SIZE; i++) {
+		if (data->val[i].milli_hz == sensor->odr) {
+			sensor->samples_to_discard = data->val[i].samples;
+			return;
+		}
+	}
+}
+
 int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable)
 {
 	struct st_lsm6dsx_hw *hw = sensor->hw;
@@ -673,6 +708,9 @@ int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable)
 			goto out;
 	}
 
+	if (enable)
+		st_lsm6dsx_update_samples_to_discard(sensor);
+
 	err = st_lsm6dsx_device_set_enable(sensor, enable);
 	if (err < 0)
 		goto out;
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
index 3f6060c64f32..966df6ffe874 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -634,6 +634,24 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.fs_len = 4,
 			},
 		},
+		.samples_to_discard = {
+			[ST_LSM6DSX_ID_ACC] = {
+				.val[0] = {  12500, 1 },
+				.val[1] = {  26000, 1 },
+				.val[2] = {  52000, 1 },
+				.val[3] = { 104000, 2 },
+				.val[4] = { 208000, 2 },
+				.val[5] = { 416000, 2 },
+			},
+			[ST_LSM6DSX_ID_GYRO] = {
+				.val[0] = {  12500,  2 },
+				.val[1] = {  26000,  5 },
+				.val[2] = {  52000,  7 },
+				.val[3] = { 104000, 12 },
+				.val[4] = { 208000, 20 },
+				.val[5] = { 416000, 36 },
+			},
+		},
 		.irq_config = {
 			.irq1 = {
 				.addr = 0x0d,
-- 
2.39.1

Re: [PATCH] iio: imu: st_lsm6dsx: discard samples during filters settling time

[Philippe De Muyter]

Hello Lorenzo,

thank you for your patch.

On Wed, Feb 08, 2023 at 03:42:31PM +0100, Lorenzo Bianconi wrote:
> 
> During digital filters settling time the driver is expected to drop
> samples since they can be corrupted. Introduce the capability to drop
> a given number of samples according to the configured ODR.
> Add the sample_to_discard data for LSM6DSM sensor.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       | 11 ++++
>  .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 58 +++++++++++++++----
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  | 18 ++++++
>  3 files changed, 77 insertions(+), 10 deletions(-)
> 

I had successfully applied the previous one, but not yet had time
to test it, but this one I cannot apply.

On which branch/tag does it apply ?

Best regards

Philippe

Re: [PATCH] iio: imu: st_lsm6dsx: discard samples during filters settling time

[Lorenzo Bianconi]

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

> Hello Lorenzo,
> 
> thank you for your patch.
> 
> On Wed, Feb 08, 2023 at 03:42:31PM +0100, Lorenzo Bianconi wrote:
> > 
> > During digital filters settling time the driver is expected to drop
> > samples since they can be corrupted. Introduce the capability to drop
> > a given number of samples according to the configured ODR.
> > Add the sample_to_discard data for LSM6DSM sensor.
> > 
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       | 11 ++++
> >  .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 58 +++++++++++++++----
> >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  | 18 ++++++
> >  3 files changed, 77 insertions(+), 10 deletions(-)
> > 
> 
> I had successfully applied the previous one, but not yet had time
> to test it, but this one I cannot apply.
> 
> On which branch/tag does it apply ?

I am using testing branch from linux-iio tree:

git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git

Regards,
Lorenzo

> 
> Best regards
> 
> Philippe

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

Re: [PATCH] iio: imu: st_lsm6dsx: discard samples during filters settling time

[Philippe De Muyter]

Hello again Lorenzo,

On Wed, Feb 08, 2023 at 05:34:23PM +0100, Lorenzo Bianconi wrote:

> Date: Wed, 8 Feb 2023 17:34:23 +0100
> From: Lorenzo Bianconi <lorenzo@kernel.org>
> To: Philippe De Muyter <phdm@macq.eu>
> Cc: jic23@kernel.org, linux-iio@vger.kernel.org,
> 	lorenzo.bianconi@redhat.com
> Subject: Re: [PATCH] iio: imu: st_lsm6dsx: discard samples during filters
> 	settling time
> 
> > Hello Lorenzo,
> > 
> > thank you for your patch.
> > 
> > I had successfully applied the previous one, but not yet had time
> > to test it, but this one I cannot apply.
> > 
> > On which branch/tag does it apply ?
> 
> I am using testing branch from linux-iio tree:
> 
> git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
> 
> Regards,
> Lorenzo

I have fetched it with :

  git fetch https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git +testing

git am complains with :
 $ git am ~/st_lsm6dsx-real.patch
 Applying: iio: imu: st_lsm6dsx: discard samples during filters settling time
 error: patch failed: drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h:137
 error: drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h: patch does not apply
 error: patch failed: drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c:457
 error: drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c: patch does not apply
 error: patch failed: drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c:634
 error: drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c: patch does not apply
 Patch failed at 0001 iio: imu: st_lsm6dsx: discard samples during filters settling time
 hint: Use 'git am --show-current-patch=diff' to see the failed patch
 When you have resolved this problem, run "git am --continue".
 If you prefer to skip this patch, run "git am --skip" instead.
 To restore the original branch and stop patching, run "git am --abort".

and patch -p1 with :
 $ patch -p1 < ~/st_lsm6dsx-real.patch
 patching file drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
 Hunk #1 succeeded at 144 with fuzz 2 (offset 7 lines).
 Hunk #2 FAILED at 298.
 Hunk #3 FAILED at 330.
 Hunk #4 FAILED at 360.
 Hunk #5 FAILED at 374.
 4 out of 5 hunks FAILED -- saving rejects to file drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h.rej
 patching file drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
 Hunk #1 FAILED at 457.
 Hunk #2 FAILED at 541.
 Hunk #3 succeeded at 673 with fuzz 1 (offset 19 lines).
 Hunk #4 FAILED at 692.
 3 out of 4 hunks FAILED -- saving rejects to file drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c.rej
 patching file drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
 Hunk #1 FAILED at 634.
 1 out of 1 hunk FAILED -- saving rejects to file drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c.rej


Could it be something caused by your or my mail-transfer-agent ?

Best regards

Philippe

Re: [PATCH] iio: imu: st_lsm6dsx: discard samples during filters settling time

[Lorenzo Bianconi]

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

> Hello again Lorenzo,
> 
> On Wed, Feb 08, 2023 at 05:34:23PM +0100, Lorenzo Bianconi wrote:
> 
> > Date: Wed, 8 Feb 2023 17:34:23 +0100
> > From: Lorenzo Bianconi <lorenzo@kernel.org>
> > To: Philippe De Muyter <phdm@macq.eu>
> > Cc: jic23@kernel.org, linux-iio@vger.kernel.org,
> > 	lorenzo.bianconi@redhat.com
> > Subject: Re: [PATCH] iio: imu: st_lsm6dsx: discard samples during filters
> > 	settling time
> > 
> > > Hello Lorenzo,
> > > 
> > > thank you for your patch.
> > > 
> > > I had successfully applied the previous one, but not yet had time
> > > to test it, but this one I cannot apply.
> > > 
> > > On which branch/tag does it apply ?
> > 
> > I am using testing branch from linux-iio tree:
> > 
> > git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git
> > 
> > Regards,
> > Lorenzo
> 
> I have fetched it with :
> 
>   git fetch https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git +testing
> 
> git am complains with :
>  $ git am ~/st_lsm6dsx-real.patch
>  Applying: iio: imu: st_lsm6dsx: discard samples during filters settling time
>  error: patch failed: drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h:137
>  error: drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h: patch does not apply
>  error: patch failed: drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c:457
>  error: drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c: patch does not apply
>  error: patch failed: drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c:634
>  error: drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c: patch does not apply
>  Patch failed at 0001 iio: imu: st_lsm6dsx: discard samples during filters settling time
>  hint: Use 'git am --show-current-patch=diff' to see the failed patch
>  When you have resolved this problem, run "git am --continue".
>  If you prefer to skip this patch, run "git am --skip" instead.
>  To restore the original branch and stop patching, run "git am --abort".
> 
> and patch -p1 with :
>  $ patch -p1 < ~/st_lsm6dsx-real.patch
>  patching file drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
>  Hunk #1 succeeded at 144 with fuzz 2 (offset 7 lines).
>  Hunk #2 FAILED at 298.
>  Hunk #3 FAILED at 330.
>  Hunk #4 FAILED at 360.
>  Hunk #5 FAILED at 374.
>  4 out of 5 hunks FAILED -- saving rejects to file drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h.rej
>  patching file drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
>  Hunk #1 FAILED at 457.
>  Hunk #2 FAILED at 541.
>  Hunk #3 succeeded at 673 with fuzz 1 (offset 19 lines).
>  Hunk #4 FAILED at 692.
>  3 out of 4 hunks FAILED -- saving rejects to file drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c.rej
>  patching file drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>  Hunk #1 FAILED at 634.
>  1 out of 1 hunk FAILED -- saving rejects to file drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c.rej
> 
> 
> Could it be something caused by your or my mail-transfer-agent ?
> 
> Best regards
> 
> Philippe

Can you please try the following branch?

https://github.com/LorenzoBianconi/linux-iio/tree/st_lsm6dsx_discard_sample

Regards,
Lorenzo


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

Re: [PATCH] iio: imu: st_lsm6dsx: discard samples during filters settling time

[Lorenzo Bianconi]

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

> During digital filters settling time the driver is expected to drop
> samples since they can be corrupted. Introduce the capability to drop
> a given number of samples according to the configured ODR.
> Add the sample_to_discard data for LSM6DSM sensor.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       | 11 ++++
>  .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 58 +++++++++++++++----
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  | 18 ++++++
>  3 files changed, 77 insertions(+), 10 deletions(-)

I forgot to say I tested this patch on my LSM6DSM and it works fine for me.

Regards,
Lorenzo

> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index 499fcf8875b4..8e119d78730b 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -137,6 +137,13 @@ struct st_lsm6dsx_odr_table_entry {
>  	int odr_len;
>  };
>  
> +struct st_lsm6dsx_samples_to_discard {
> +	struct {
> +		u32 milli_hz;
> +		u16 samples;
> +	} val[ST_LSM6DSX_ODR_LIST_SIZE];
> +};
> +
>  struct st_lsm6dsx_fs {
>  	u32 gain;
>  	u8 val;
> @@ -291,6 +298,7 @@ struct st_lsm6dsx_ext_dev_settings {
>   * @irq_config: interrupts related registers.
>   * @drdy_mask: register info for data-ready mask (addr + mask).
>   * @odr_table: Hw sensors odr table (Hz + val).
> + * @samples_to_discard: Number of samples to discard for filters settling time.
>   * @fs_table: Hw sensors gain table (gain + val).
>   * @decimator: List of decimator register info (addr + mask).
>   * @batch: List of FIFO batching register info (addr + mask).
> @@ -323,6 +331,7 @@ struct st_lsm6dsx_settings {
>  	} irq_config;
>  	struct st_lsm6dsx_reg drdy_mask;
>  	struct st_lsm6dsx_odr_table_entry odr_table[2];
> +	struct st_lsm6dsx_samples_to_discard samples_to_discard[2];
>  	struct st_lsm6dsx_fs_table_entry fs_table[2];
>  	struct st_lsm6dsx_reg decimator[ST_LSM6DSX_MAX_ID];
>  	struct st_lsm6dsx_reg batch[ST_LSM6DSX_MAX_ID];
> @@ -353,6 +362,7 @@ enum st_lsm6dsx_fifo_mode {
>   * @hw: Pointer to instance of struct st_lsm6dsx_hw.
>   * @gain: Configured sensor sensitivity.
>   * @odr: Output data rate of the sensor [Hz].
> + * @samples_to_discard: Number of samples to discard for filters settling time.
>   * @watermark: Sensor watermark level.
>   * @decimator: Sensor decimation factor.
>   * @sip: Number of samples in a given pattern.
> @@ -367,6 +377,7 @@ struct st_lsm6dsx_sensor {
>  	u32 gain;
>  	u32 odr;
>  
> +	u16 samples_to_discard;
>  	u16 watermark;
>  	u8 decimator;
>  	u8 sip;
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> index 7dd5205aea5b..c1059a79f5ff 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> @@ -457,17 +457,29 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
>  			}
>  
>  			if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) {
> -				iio_push_to_buffers_with_timestamp(
> -					hw->iio_devs[ST_LSM6DSX_ID_GYRO],
> -					&hw->scan[ST_LSM6DSX_ID_GYRO],
> -					gyro_sensor->ts_ref + ts);
> +				/* We need to discards gyro samples during
> +				 * filters settling time
> +				 */
> +				if (gyro_sensor->samples_to_discard > 0)
> +					gyro_sensor->samples_to_discard--;
> +				else
> +					iio_push_to_buffers_with_timestamp(
> +						hw->iio_devs[ST_LSM6DSX_ID_GYRO],
> +						&hw->scan[ST_LSM6DSX_ID_GYRO],
> +						gyro_sensor->ts_ref + ts);
>  				gyro_sip--;
>  			}
>  			if (acc_sip > 0 && !(sip % acc_sensor->decimator)) {
> -				iio_push_to_buffers_with_timestamp(
> -					hw->iio_devs[ST_LSM6DSX_ID_ACC],
> -					&hw->scan[ST_LSM6DSX_ID_ACC],
> -					acc_sensor->ts_ref + ts);
> +				/* We need to discards accel samples during
> +				 * filters settling time
> +				 */
> +				if (acc_sensor->samples_to_discard > 0)
> +					acc_sensor->samples_to_discard--;
> +				else
> +					iio_push_to_buffers_with_timestamp(
> +						hw->iio_devs[ST_LSM6DSX_ID_ACC],
> +						&hw->scan[ST_LSM6DSX_ID_ACC],
> +						acc_sensor->ts_ref + ts);
>  				acc_sip--;
>  			}
>  			if (ext_sip > 0 && !(sip % ext_sensor->decimator)) {
> @@ -541,8 +553,12 @@ st_lsm6dsx_push_tagged_data(struct st_lsm6dsx_hw *hw, u8 tag,
>  	}
>  
>  	sensor = iio_priv(iio_dev);
> -	iio_push_to_buffers_with_timestamp(iio_dev, data,
> -					   ts + sensor->ts_ref);
> +	/* We need to discards gyro samples during filters settling time */
> +	if (sensor->samples_to_discard > 0)
> +		sensor->samples_to_discard--;
> +	else
> +		iio_push_to_buffers_with_timestamp(iio_dev, data,
> +						   ts + sensor->ts_ref);
>  
>  	return 0;
>  }
> @@ -654,6 +670,25 @@ int st_lsm6dsx_flush_fifo(struct st_lsm6dsx_hw *hw)
>  	return err;
>  }
>  
> +static void
> +st_lsm6dsx_update_samples_to_discard(struct st_lsm6dsx_sensor *sensor)
> +{
> +	const struct st_lsm6dsx_samples_to_discard *data;
> +	int i;
> +
> +	if (sensor->id != ST_LSM6DSX_ID_GYRO &&
> +	    sensor->id != ST_LSM6DSX_ID_ACC)
> +		return;
> +
> +	data = &sensor->hw->settings->samples_to_discard[sensor->id];
> +	for (i = 0; i < ST_LSM6DSX_ODR_LIST_SIZE; i++) {
> +		if (data->val[i].milli_hz == sensor->odr) {
> +			sensor->samples_to_discard = data->val[i].samples;
> +			return;
> +		}
> +	}
> +}
> +
>  int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable)
>  {
>  	struct st_lsm6dsx_hw *hw = sensor->hw;
> @@ -673,6 +708,9 @@ int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable)
>  			goto out;
>  	}
>  
> +	if (enable)
> +		st_lsm6dsx_update_samples_to_discard(sensor);
> +
>  	err = st_lsm6dsx_device_set_enable(sensor, enable);
>  	if (err < 0)
>  		goto out;
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index 3f6060c64f32..966df6ffe874 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -634,6 +634,24 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  				.fs_len = 4,
>  			},
>  		},
> +		.samples_to_discard = {
> +			[ST_LSM6DSX_ID_ACC] = {
> +				.val[0] = {  12500, 1 },
> +				.val[1] = {  26000, 1 },
> +				.val[2] = {  52000, 1 },
> +				.val[3] = { 104000, 2 },
> +				.val[4] = { 208000, 2 },
> +				.val[5] = { 416000, 2 },
> +			},
> +			[ST_LSM6DSX_ID_GYRO] = {
> +				.val[0] = {  12500,  2 },
> +				.val[1] = {  26000,  5 },
> +				.val[2] = {  52000,  7 },
> +				.val[3] = { 104000, 12 },
> +				.val[4] = { 208000, 20 },
> +				.val[5] = { 416000, 36 },
> +			},
> +		},
>  		.irq_config = {
>  			.irq1 = {
>  				.addr = 0x0d,
> -- 
> 2.39.1
> 

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

Re: [PATCH] iio: imu: st_lsm6dsx: discard samples during filters settling time

[Philippe De Muyter]

Hello Lorenzo,

On Sun, Feb 12, 2023 at 11:21:32AM +0100, Lorenzo Bianconi wrote:
> Date: Sun, 12 Feb 2023 11:21:32 +0100
> From: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> To: Lorenzo Bianconi <lorenzo@kernel.org>
> Cc: jic23@kernel.org, phdm@macq.eu, linux-iio@vger.kernel.org
> Subject: Re: [PATCH] iio: imu: st_lsm6dsx: discard samples during filters
> 	settling time
> 
> > During digital filters settling time the driver is expected to drop
> > samples since they can be corrupted. Introduce the capability to drop
> > a given number of samples according to the configured ODR.
> > Add the sample_to_discard data for LSM6DSM sensor.
> > 
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       | 11 ++++
> >  .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 58 +++++++++++++++----
> >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  | 18 ++++++
> >  3 files changed, 77 insertions(+), 10 deletions(-)
> 
> I forgot to say I tested this patch on my LSM6DSM and it works fine for me.
> 
> Regards,
> Lorenzo
> 

It works fine for me too, with a ism330dlc.

	Reported-by: Philippe De Muyter <phdm@macqel.be>
	Tested-by: Philippe De Muyter <phdm@macqel.be>

However I have another bug, with our without the patch : frequently
my test, using a loop around iio-generic-buffer, blocks on the poll syscall.
No value comes anymore.  This happens both with the gyro as with the accel
component.

More info follows.

Best regards

Philippe

Re: [PATCH] iio: imu: st_lsm6dsx: no answer after some iio_generic_buffer test cycles

[Philippe De Muyter]

Hello Lorenzo,

On Mon, Feb 13, 2023 at 10:19:57AM +0100, Philippe De Muyter wrote:
> I have a bug, with our without the patch : frequently
> my test, using a loop around iio-generic-buffer, blocks on the poll syscall.
> No value comes anymore.  This happens both with the gyro as with the accel
> component.
> 
> More info follows.

Here is the way I test :

 # iio/lsiio 
 Device 001: ism330dlc_gyro
 Device 002: ism330dlc_accel
 Device 000: ina3221x
 # for axis in x y z; do echo 1 >/sys/bus/iio/devices/iio\:device1/scan_elements/in_anglvel_${axis}_en; done
 # echo 416 > /sys/bus/iio/devices/iio\:device1/sampling_frequency
 # while true; do  sudo iio/iio_generic_buffer -n ism330dlc_gyro -g -c 6 -a; sleep 2; done
 iio device number being used is 1
 trigger-less mode selected
 Auto-channels selected but some channels are already activated in sysfs
 Proceeding without activating any channels
 0.002291 -0.073762 0.033139 
 0.003971 -0.075289 0.035583 
 0.003971 -0.076969 0.036194 
 0.004123 -0.074220 0.037110 
 0.003207 -0.074678 0.034667 
 0.002596 -0.073456 0.035277 
 iio device number being used is 1
 trigger-less mode selected
 Auto-channels selected but some channels are already activated in sysfs
 Proceeding without activating any channels
 0.002138 -0.074067 0.034819 
 0.003512 -0.075442 0.034819 
 0.001985 -0.074373 0.035430 
 0.002902 -0.074373 0.035888 
 0.002138 -0.074220 0.036346 
 0.003360 -0.076053 0.035277

After a quick time, iio_generic_buffer gets blocked in the poll syscall.
Hitting 'CTRL-C' kills the iio_generic_buffer process, and the next one
then receives values.

This happens with both gyro and accel.

Do you encounter the same problem ?

My kernel is actually a 4.9 one with a backport of iio core infrastructure
and iio/imu/st_lsm6dsx of 6.2, but I could have missed something important.

Best regards

Philippe

Re: [PATCH] iio: imu: st_lsm6dsx: no answer after some iio_generic_buffer test cycles

[Lorenzo Bianconi]

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

> Hello Lorenzo,

Hi Philippe,

> 
> On Mon, Feb 13, 2023 at 10:19:57AM +0100, Philippe De Muyter wrote:
> > I have a bug, with our without the patch : frequently
> > my test, using a loop around iio-generic-buffer, blocks on the poll syscall.
> > No value comes anymore.  This happens both with the gyro as with the accel
> > component.
> > 
> > More info follows.
> 
> Here is the way I test :
> 
>  # iio/lsiio 
>  Device 001: ism330dlc_gyro
>  Device 002: ism330dlc_accel
>  Device 000: ina3221x
>  # for axis in x y z; do echo 1 >/sys/bus/iio/devices/iio\:device1/scan_elements/in_anglvel_${axis}_en; done
>  # echo 416 > /sys/bus/iio/devices/iio\:device1/sampling_frequency
>  # while true; do  sudo iio/iio_generic_buffer -n ism330dlc_gyro -g -c 6 -a; sleep 2; done
>  iio device number being used is 1
>  trigger-less mode selected
>  Auto-channels selected but some channels are already activated in sysfs
>  Proceeding without activating any channels
>  0.002291 -0.073762 0.033139 
>  0.003971 -0.075289 0.035583 
>  0.003971 -0.076969 0.036194 
>  0.004123 -0.074220 0.037110 
>  0.003207 -0.074678 0.034667 
>  0.002596 -0.073456 0.035277 
>  iio device number being used is 1
>  trigger-less mode selected
>  Auto-channels selected but some channels are already activated in sysfs
>  Proceeding without activating any channels
>  0.002138 -0.074067 0.034819 
>  0.003512 -0.075442 0.034819 
>  0.001985 -0.074373 0.035430 
>  0.002902 -0.074373 0.035888 
>  0.002138 -0.074220 0.036346 
>  0.003360 -0.076053 0.035277
> 
> After a quick time, iio_generic_buffer gets blocked in the poll syscall.

are you running level or edge interrupts? If you are using edge ones can you
please check your kernel has the following fix?

commit 3f9bce7a22a3f8ac9d885c9d75bc45569f24ac8b
Author: Lorenzo Bianconi <lorenzo@kernel.org>
Date:   Sat Nov 14 19:39:05 2020 +0100

    iio: imu: st_lsm6dsx: fix edge-trigger interrupts

> Hitting 'CTRL-C' kills the iio_generic_buffer process, and the next one
> then receives values.
> 
> This happens with both gyro and accel.
> 
> Do you encounter the same problem ?
> 
> My kernel is actually a 4.9 one with a backport of iio core infrastructure
> and iio/imu/st_lsm6dsx of 6.2, but I could have missed something important.

Are you able to test with an upstream kernel?

Regards,
Lorenzo

> 
> Best regards
> 
> Philippe

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

Re: [PATCH] iio: imu: st_lsm6dsx: no answer after some iio_generic_buffer test cycles

[Philippe De Muyter]

Hi Lorenzo,

On Mon, Feb 13, 2023 at 11:53:24AM +0100, Lorenzo Bianconi wrote:
> 
> are you running level or edge interrupts? If you are using edge ones can you
> please check your kernel has the following fix?
> 
> commit 3f9bce7a22a3f8ac9d885c9d75bc45569f24ac8b
> Author: Lorenzo Bianconi <lorenzo@kernel.org>
> Date:   Sat Nov 14 19:39:05 2020 +0100
> 
>     iio: imu: st_lsm6dsx: fix edge-trigger interrupts
> 
I had missed this one.  Adding it fixes the bug.

Thank you !

Philippe

Re: [PATCH] iio: imu: st_lsm6dsx: discard samples during filters settling time

[Jonathan Cameron]

On Wed,  8 Feb 2023 15:42:31 +0100
Lorenzo Bianconi <lorenzo@kernel.org> wrote:

> During digital filters settling time the driver is expected to drop
> samples since they can be corrupted. Introduce the capability to drop
> a given number of samples according to the configured ODR.
> Add the sample_to_discard data for LSM6DSM sensor.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

Is this only necessary for the particular sensor you have provided
values for?  Or is it more general?

I think the code will currently just set the number of samples to discard
to 0 for other cases (as no value set for those sensor types).
That's fine if 0 is definitely the right value for those other sensors.

Thanks,

Jonathan


> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       | 11 ++++
>  .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 58 +++++++++++++++----
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  | 18 ++++++
>  3 files changed, 77 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index 499fcf8875b4..8e119d78730b 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -137,6 +137,13 @@ struct st_lsm6dsx_odr_table_entry {
>  	int odr_len;
>  };
>  
> +struct st_lsm6dsx_samples_to_discard {
> +	struct {
> +		u32 milli_hz;
> +		u16 samples;
> +	} val[ST_LSM6DSX_ODR_LIST_SIZE];
> +};
> +
>  struct st_lsm6dsx_fs {
>  	u32 gain;
>  	u8 val;
> @@ -291,6 +298,7 @@ struct st_lsm6dsx_ext_dev_settings {
>   * @irq_config: interrupts related registers.
>   * @drdy_mask: register info for data-ready mask (addr + mask).
>   * @odr_table: Hw sensors odr table (Hz + val).
> + * @samples_to_discard: Number of samples to discard for filters settling time.
>   * @fs_table: Hw sensors gain table (gain + val).
>   * @decimator: List of decimator register info (addr + mask).
>   * @batch: List of FIFO batching register info (addr + mask).
> @@ -323,6 +331,7 @@ struct st_lsm6dsx_settings {
>  	} irq_config;
>  	struct st_lsm6dsx_reg drdy_mask;
>  	struct st_lsm6dsx_odr_table_entry odr_table[2];
> +	struct st_lsm6dsx_samples_to_discard samples_to_discard[2];
>  	struct st_lsm6dsx_fs_table_entry fs_table[2];
>  	struct st_lsm6dsx_reg decimator[ST_LSM6DSX_MAX_ID];
>  	struct st_lsm6dsx_reg batch[ST_LSM6DSX_MAX_ID];
> @@ -353,6 +362,7 @@ enum st_lsm6dsx_fifo_mode {
>   * @hw: Pointer to instance of struct st_lsm6dsx_hw.
>   * @gain: Configured sensor sensitivity.
>   * @odr: Output data rate of the sensor [Hz].
> + * @samples_to_discard: Number of samples to discard for filters settling time.
>   * @watermark: Sensor watermark level.
>   * @decimator: Sensor decimation factor.
>   * @sip: Number of samples in a given pattern.
> @@ -367,6 +377,7 @@ struct st_lsm6dsx_sensor {
>  	u32 gain;
>  	u32 odr;
>  
> +	u16 samples_to_discard;
>  	u16 watermark;
>  	u8 decimator;
>  	u8 sip;
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> index 7dd5205aea5b..c1059a79f5ff 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> @@ -457,17 +457,29 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
>  			}
>  
>  			if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) {
> -				iio_push_to_buffers_with_timestamp(
> -					hw->iio_devs[ST_LSM6DSX_ID_GYRO],
> -					&hw->scan[ST_LSM6DSX_ID_GYRO],
> -					gyro_sensor->ts_ref + ts);
> +				/* We need to discards gyro samples during

Trivial but wrong comment syntax. If that's all that comes up I'll fix it here
and in other instances below when applying.

> +				 * filters settling time
> +				 */
> +				if (gyro_sensor->samples_to_discard > 0)
> +					gyro_sensor->samples_to_discard--;
> +				else
> +					iio_push_to_buffers_with_timestamp(
> +						hw->iio_devs[ST_LSM6DSX_ID_GYRO],
> +						&hw->scan[ST_LSM6DSX_ID_GYRO],
> +						gyro_sensor->ts_ref + ts);
>  				gyro_sip--;
>  			}
>  			if (acc_sip > 0 && !(sip % acc_sensor->decimator)) {
> -				iio_push_to_buffers_with_timestamp(
> -					hw->iio_devs[ST_LSM6DSX_ID_ACC],
> -					&hw->scan[ST_LSM6DSX_ID_ACC],
> -					acc_sensor->ts_ref + ts);
> +				/* We need to discards accel samples during
> +				 * filters settling time
> +				 */
> +				if (acc_sensor->samples_to_discard > 0)
> +					acc_sensor->samples_to_discard--;
> +				else
> +					iio_push_to_buffers_with_timestamp(
> +						hw->iio_devs[ST_LSM6DSX_ID_ACC],
> +						&hw->scan[ST_LSM6DSX_ID_ACC],
> +						acc_sensor->ts_ref + ts);
>  				acc_sip--;
>  			}
>  			if (ext_sip > 0 && !(sip % ext_sensor->decimator)) {
> @@ -541,8 +553,12 @@ st_lsm6dsx_push_tagged_data(struct st_lsm6dsx_hw *hw, u8 tag,
>  	}
>  
>  	sensor = iio_priv(iio_dev);
> -	iio_push_to_buffers_with_timestamp(iio_dev, data,
> -					   ts + sensor->ts_ref);
> +	/* We need to discards gyro samples during filters settling time */
> +	if (sensor->samples_to_discard > 0)
> +		sensor->samples_to_discard--;
> +	else
> +		iio_push_to_buffers_with_timestamp(iio_dev, data,
> +						   ts + sensor->ts_ref);
>  
>  	return 0;
>  }
> @@ -654,6 +670,25 @@ int st_lsm6dsx_flush_fifo(struct st_lsm6dsx_hw *hw)
>  	return err;
>  }
>  
> +static void
> +st_lsm6dsx_update_samples_to_discard(struct st_lsm6dsx_sensor *sensor)
> +{
> +	const struct st_lsm6dsx_samples_to_discard *data;
> +	int i;
> +
> +	if (sensor->id != ST_LSM6DSX_ID_GYRO &&
> +	    sensor->id != ST_LSM6DSX_ID_ACC)
> +		return;
> +
> +	data = &sensor->hw->settings->samples_to_discard[sensor->id];
> +	for (i = 0; i < ST_LSM6DSX_ODR_LIST_SIZE; i++) {
> +		if (data->val[i].milli_hz == sensor->odr) {
> +			sensor->samples_to_discard = data->val[i].samples;
> +			return;
> +		}
> +	}
> +}
> +
>  int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable)
>  {
>  	struct st_lsm6dsx_hw *hw = sensor->hw;
> @@ -673,6 +708,9 @@ int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable)
>  			goto out;
>  	}
>  
> +	if (enable)
> +		st_lsm6dsx_update_samples_to_discard(sensor);
> +
>  	err = st_lsm6dsx_device_set_enable(sensor, enable);
>  	if (err < 0)
>  		goto out;
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index 3f6060c64f32..966df6ffe874 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -634,6 +634,24 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  				.fs_len = 4,
>  			},
>  		},
> +		.samples_to_discard = {
> +			[ST_LSM6DSX_ID_ACC] = {
> +				.val[0] = {  12500, 1 },
> +				.val[1] = {  26000, 1 },
> +				.val[2] = {  52000, 1 },
> +				.val[3] = { 104000, 2 },
> +				.val[4] = { 208000, 2 },
> +				.val[5] = { 416000, 2 },
> +			},
> +			[ST_LSM6DSX_ID_GYRO] = {
> +				.val[0] = {  12500,  2 },
> +				.val[1] = {  26000,  5 },
> +				.val[2] = {  52000,  7 },
> +				.val[3] = { 104000, 12 },
> +				.val[4] = { 208000, 20 },
> +				.val[5] = { 416000, 36 },
> +			},
> +		},
>  		.irq_config = {
>  			.irq1 = {
>  				.addr = 0x0d,

Re: [PATCH] iio: imu: st_lsm6dsx: discard samples during filters settling time

[Lorenzo Bianconi]

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

> On Wed,  8 Feb 2023 15:42:31 +0100
> Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> 
> > During digital filters settling time the driver is expected to drop
> > samples since they can be corrupted. Introduce the capability to drop
> > a given number of samples according to the configured ODR.
> > Add the sample_to_discard data for LSM6DSM sensor.
> > 
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> Is this only necessary for the particular sensor you have provided
> values for?  Or is it more general?
> 
> I think the code will currently just set the number of samples to discard
> to 0 for other cases (as no value set for those sensor types).
> That's fine if 0 is definitely the right value for those other sensors.

 I think all the sensors have this information in the datasheet/application
 note. However, even if we add sample_to_discard just for LSM6DSM for the moment,
 we do not introduce any regression for the other sensors with respect to the
 previous codebase since sample_to_discard will be just set to 0 (so we do not
 discard any sample). I can add sample_to_discard for LSM6DSO but at the
 moment I do not have other devices for testing.
 For LSM6DSO, do you prefer to add it in v2 or is it fine a follow-up patch?

 Regards,
 Lorenzo

> 
> Thanks,
> 
> Jonathan
> 
> 
> > ---
> >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       | 11 ++++
> >  .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 58 +++++++++++++++----
> >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  | 18 ++++++
> >  3 files changed, 77 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > index 499fcf8875b4..8e119d78730b 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > @@ -137,6 +137,13 @@ struct st_lsm6dsx_odr_table_entry {
> >  	int odr_len;
> >  };
> >  
> > +struct st_lsm6dsx_samples_to_discard {
> > +	struct {
> > +		u32 milli_hz;
> > +		u16 samples;
> > +	} val[ST_LSM6DSX_ODR_LIST_SIZE];
> > +};
> > +
> >  struct st_lsm6dsx_fs {
> >  	u32 gain;
> >  	u8 val;
> > @@ -291,6 +298,7 @@ struct st_lsm6dsx_ext_dev_settings {
> >   * @irq_config: interrupts related registers.
> >   * @drdy_mask: register info for data-ready mask (addr + mask).
> >   * @odr_table: Hw sensors odr table (Hz + val).
> > + * @samples_to_discard: Number of samples to discard for filters settling time.
> >   * @fs_table: Hw sensors gain table (gain + val).
> >   * @decimator: List of decimator register info (addr + mask).
> >   * @batch: List of FIFO batching register info (addr + mask).
> > @@ -323,6 +331,7 @@ struct st_lsm6dsx_settings {
> >  	} irq_config;
> >  	struct st_lsm6dsx_reg drdy_mask;
> >  	struct st_lsm6dsx_odr_table_entry odr_table[2];
> > +	struct st_lsm6dsx_samples_to_discard samples_to_discard[2];
> >  	struct st_lsm6dsx_fs_table_entry fs_table[2];
> >  	struct st_lsm6dsx_reg decimator[ST_LSM6DSX_MAX_ID];
> >  	struct st_lsm6dsx_reg batch[ST_LSM6DSX_MAX_ID];
> > @@ -353,6 +362,7 @@ enum st_lsm6dsx_fifo_mode {
> >   * @hw: Pointer to instance of struct st_lsm6dsx_hw.
> >   * @gain: Configured sensor sensitivity.
> >   * @odr: Output data rate of the sensor [Hz].
> > + * @samples_to_discard: Number of samples to discard for filters settling time.
> >   * @watermark: Sensor watermark level.
> >   * @decimator: Sensor decimation factor.
> >   * @sip: Number of samples in a given pattern.
> > @@ -367,6 +377,7 @@ struct st_lsm6dsx_sensor {
> >  	u32 gain;
> >  	u32 odr;
> >  
> > +	u16 samples_to_discard;
> >  	u16 watermark;
> >  	u8 decimator;
> >  	u8 sip;
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > index 7dd5205aea5b..c1059a79f5ff 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > @@ -457,17 +457,29 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
> >  			}
> >  
> >  			if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) {
> > -				iio_push_to_buffers_with_timestamp(
> > -					hw->iio_devs[ST_LSM6DSX_ID_GYRO],
> > -					&hw->scan[ST_LSM6DSX_ID_GYRO],
> > -					gyro_sensor->ts_ref + ts);
> > +				/* We need to discards gyro samples during
> 
> Trivial but wrong comment syntax. If that's all that comes up I'll fix it here
> and in other instances below when applying.
> 
> > +				 * filters settling time
> > +				 */
> > +				if (gyro_sensor->samples_to_discard > 0)
> > +					gyro_sensor->samples_to_discard--;
> > +				else
> > +					iio_push_to_buffers_with_timestamp(
> > +						hw->iio_devs[ST_LSM6DSX_ID_GYRO],
> > +						&hw->scan[ST_LSM6DSX_ID_GYRO],
> > +						gyro_sensor->ts_ref + ts);
> >  				gyro_sip--;
> >  			}
> >  			if (acc_sip > 0 && !(sip % acc_sensor->decimator)) {
> > -				iio_push_to_buffers_with_timestamp(
> > -					hw->iio_devs[ST_LSM6DSX_ID_ACC],
> > -					&hw->scan[ST_LSM6DSX_ID_ACC],
> > -					acc_sensor->ts_ref + ts);
> > +				/* We need to discards accel samples during
> > +				 * filters settling time
> > +				 */
> > +				if (acc_sensor->samples_to_discard > 0)
> > +					acc_sensor->samples_to_discard--;
> > +				else
> > +					iio_push_to_buffers_with_timestamp(
> > +						hw->iio_devs[ST_LSM6DSX_ID_ACC],
> > +						&hw->scan[ST_LSM6DSX_ID_ACC],
> > +						acc_sensor->ts_ref + ts);
> >  				acc_sip--;
> >  			}
> >  			if (ext_sip > 0 && !(sip % ext_sensor->decimator)) {
> > @@ -541,8 +553,12 @@ st_lsm6dsx_push_tagged_data(struct st_lsm6dsx_hw *hw, u8 tag,
> >  	}
> >  
> >  	sensor = iio_priv(iio_dev);
> > -	iio_push_to_buffers_with_timestamp(iio_dev, data,
> > -					   ts + sensor->ts_ref);
> > +	/* We need to discards gyro samples during filters settling time */
> > +	if (sensor->samples_to_discard > 0)
> > +		sensor->samples_to_discard--;
> > +	else
> > +		iio_push_to_buffers_with_timestamp(iio_dev, data,
> > +						   ts + sensor->ts_ref);
> >  
> >  	return 0;
> >  }
> > @@ -654,6 +670,25 @@ int st_lsm6dsx_flush_fifo(struct st_lsm6dsx_hw *hw)
> >  	return err;
> >  }
> >  
> > +static void
> > +st_lsm6dsx_update_samples_to_discard(struct st_lsm6dsx_sensor *sensor)
> > +{
> > +	const struct st_lsm6dsx_samples_to_discard *data;
> > +	int i;
> > +
> > +	if (sensor->id != ST_LSM6DSX_ID_GYRO &&
> > +	    sensor->id != ST_LSM6DSX_ID_ACC)
> > +		return;
> > +
> > +	data = &sensor->hw->settings->samples_to_discard[sensor->id];
> > +	for (i = 0; i < ST_LSM6DSX_ODR_LIST_SIZE; i++) {
> > +		if (data->val[i].milli_hz == sensor->odr) {
> > +			sensor->samples_to_discard = data->val[i].samples;
> > +			return;
> > +		}
> > +	}
> > +}
> > +
> >  int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable)
> >  {
> >  	struct st_lsm6dsx_hw *hw = sensor->hw;
> > @@ -673,6 +708,9 @@ int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable)
> >  			goto out;
> >  	}
> >  
> > +	if (enable)
> > +		st_lsm6dsx_update_samples_to_discard(sensor);
> > +
> >  	err = st_lsm6dsx_device_set_enable(sensor, enable);
> >  	if (err < 0)
> >  		goto out;
> > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > index 3f6060c64f32..966df6ffe874 100644
> > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > @@ -634,6 +634,24 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
> >  				.fs_len = 4,
> >  			},
> >  		},
> > +		.samples_to_discard = {
> > +			[ST_LSM6DSX_ID_ACC] = {
> > +				.val[0] = {  12500, 1 },
> > +				.val[1] = {  26000, 1 },
> > +				.val[2] = {  52000, 1 },
> > +				.val[3] = { 104000, 2 },
> > +				.val[4] = { 208000, 2 },
> > +				.val[5] = { 416000, 2 },
> > +			},
> > +			[ST_LSM6DSX_ID_GYRO] = {
> > +				.val[0] = {  12500,  2 },
> > +				.val[1] = {  26000,  5 },
> > +				.val[2] = {  52000,  7 },
> > +				.val[3] = { 104000, 12 },
> > +				.val[4] = { 208000, 20 },
> > +				.val[5] = { 416000, 36 },
> > +			},
> > +		},
> >  		.irq_config = {
> >  			.irq1 = {
> >  				.addr = 0x0d,
> 

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

Re: [PATCH] iio: imu: st_lsm6dsx: discard samples during filters settling time

[Philippe De Muyter]

Hello Lorenzo and Jonathan,

On Mon, Feb 20, 2023 at 10:12:29AM +0100, Lorenzo Bianconi wrote:
> On Sat, Feb 18, 2023 at 01:56:22PM +0000, Jonathan Cameron wrote:
> 
> > On Wed,  8 Feb 2023 15:42:31 +0100
> > Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> > 
> > > During digital filters settling time the driver is expected to drop
> > > samples since they can be corrupted. Introduce the capability to drop
> > > a given number of samples according to the configured ODR.
> > > Add the sample_to_discard data for LSM6DSM sensor.
> > > 
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > 
> > Is this only necessary for the particular sensor you have provided
> > values for?  Or is it more general?
> > 
> > I think the code will currently just set the number of samples to discard
> > to 0 for other cases (as no value set for those sensor types).
> > That's fine if 0 is definitely the right value for those other sensors.
> 
>  I think all the sensors have this information in the datasheet/application
>  note. However, even if we add sample_to_discard just for LSM6DSM for the moment,
>  we do not introduce any regression for the other sensors with respect to the
>  previous codebase since sample_to_discard will be just set to 0 (so we do not
>  discard any sample). I can add sample_to_discard for LSM6DSO but at the
>  moment I do not have other devices for testing.
>  For LSM6DSO, do you prefer to add it in v2 or is it fine a follow-up patch?

How comes your patch really drops samples on my st,ism330dlc IMU ?

Best regards

Philippe

Re: [PATCH] iio: imu: st_lsm6dsx: discard samples during filters settling time

[Lorenzo Bianconi]

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

> Hello Lorenzo and Jonathan,
> 
> On Mon, Feb 20, 2023 at 10:12:29AM +0100, Lorenzo Bianconi wrote:
> > On Sat, Feb 18, 2023 at 01:56:22PM +0000, Jonathan Cameron wrote:
> > 
> > > On Wed,  8 Feb 2023 15:42:31 +0100
> > > Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> > > 
> > > > During digital filters settling time the driver is expected to drop
> > > > samples since they can be corrupted. Introduce the capability to drop
> > > > a given number of samples according to the configured ODR.
> > > > Add the sample_to_discard data for LSM6DSM sensor.
> > > > 
> > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > 
> > > Is this only necessary for the particular sensor you have provided
> > > values for?  Or is it more general?
> > > 
> > > I think the code will currently just set the number of samples to discard
> > > to 0 for other cases (as no value set for those sensor types).
> > > That's fine if 0 is definitely the right value for those other sensors.
> > 
> >  I think all the sensors have this information in the datasheet/application
> >  note. However, even if we add sample_to_discard just for LSM6DSM for the moment,
> >  we do not introduce any regression for the other sensors with respect to the
> >  previous codebase since sample_to_discard will be just set to 0 (so we do not
> >  discard any sample). I can add sample_to_discard for LSM6DSO but at the
> >  moment I do not have other devices for testing.
> >  For LSM6DSO, do you prefer to add it in v2 or is it fine a follow-up patch?
> 
> How comes your patch really drops samples on my st,ism330dlc IMU ?

LSM6DSM and ISM330DLC share the register map.

Regards,
Lorenzo

> 
> Best regards
> 
> Philippe

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

Re: [PATCH] iio: imu: st_lsm6dsx: discard samples during filters settling time

[Jonathan Cameron]

On Mon, 20 Feb 2023 10:12:29 +0100
Lorenzo Bianconi <lorenzo@kernel.org> wrote:

> > On Wed,  8 Feb 2023 15:42:31 +0100
> > Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> >   
> > > During digital filters settling time the driver is expected to drop
> > > samples since they can be corrupted. Introduce the capability to drop
> > > a given number of samples according to the configured ODR.
> > > Add the sample_to_discard data for LSM6DSM sensor.
> > > 
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>  
> > 
> > Is this only necessary for the particular sensor you have provided
> > values for?  Or is it more general?
> > 
> > I think the code will currently just set the number of samples to discard
> > to 0 for other cases (as no value set for those sensor types).
> > That's fine if 0 is definitely the right value for those other sensors.  
> 
>  I think all the sensors have this information in the datasheet/application
>  note. However, even if we add sample_to_discard just for LSM6DSM for the moment,
>  we do not introduce any regression for the other sensors with respect to the
>  previous codebase since sample_to_discard will be just set to 0 (so we do not
>  discard any sample). I can add sample_to_discard for LSM6DSO but at the
>  moment I do not have other devices for testing.
>  For LSM6DSO, do you prefer to add it in v2 or is it fine a follow-up patch?

It's fine to do this as and when we can test particular devices
(or frankly just assume datasheet correct). 

We should call it out in the patch description though so hopefully people
notice they need to add it for sensors they care about.

> 
>  Regards,
>  Lorenzo
> 
> > 
> > Thanks,
> > 
> > Jonathan
> > 
> >   
> > > ---
> > >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       | 11 ++++
> > >  .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 58 +++++++++++++++----
> > >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  | 18 ++++++
> > >  3 files changed, 77 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > > index 499fcf8875b4..8e119d78730b 100644
> > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > > @@ -137,6 +137,13 @@ struct st_lsm6dsx_odr_table_entry {
> > >  	int odr_len;
> > >  };
> > >  
> > > +struct st_lsm6dsx_samples_to_discard {
> > > +	struct {
> > > +		u32 milli_hz;
> > > +		u16 samples;
> > > +	} val[ST_LSM6DSX_ODR_LIST_SIZE];
> > > +};
> > > +
> > >  struct st_lsm6dsx_fs {
> > >  	u32 gain;
> > >  	u8 val;
> > > @@ -291,6 +298,7 @@ struct st_lsm6dsx_ext_dev_settings {
> > >   * @irq_config: interrupts related registers.
> > >   * @drdy_mask: register info for data-ready mask (addr + mask).
> > >   * @odr_table: Hw sensors odr table (Hz + val).
> > > + * @samples_to_discard: Number of samples to discard for filters settling time.
> > >   * @fs_table: Hw sensors gain table (gain + val).
> > >   * @decimator: List of decimator register info (addr + mask).
> > >   * @batch: List of FIFO batching register info (addr + mask).
> > > @@ -323,6 +331,7 @@ struct st_lsm6dsx_settings {
> > >  	} irq_config;
> > >  	struct st_lsm6dsx_reg drdy_mask;
> > >  	struct st_lsm6dsx_odr_table_entry odr_table[2];
> > > +	struct st_lsm6dsx_samples_to_discard samples_to_discard[2];
> > >  	struct st_lsm6dsx_fs_table_entry fs_table[2];
> > >  	struct st_lsm6dsx_reg decimator[ST_LSM6DSX_MAX_ID];
> > >  	struct st_lsm6dsx_reg batch[ST_LSM6DSX_MAX_ID];
> > > @@ -353,6 +362,7 @@ enum st_lsm6dsx_fifo_mode {
> > >   * @hw: Pointer to instance of struct st_lsm6dsx_hw.
> > >   * @gain: Configured sensor sensitivity.
> > >   * @odr: Output data rate of the sensor [Hz].
> > > + * @samples_to_discard: Number of samples to discard for filters settling time.
> > >   * @watermark: Sensor watermark level.
> > >   * @decimator: Sensor decimation factor.
> > >   * @sip: Number of samples in a given pattern.
> > > @@ -367,6 +377,7 @@ struct st_lsm6dsx_sensor {
> > >  	u32 gain;
> > >  	u32 odr;
> > >  
> > > +	u16 samples_to_discard;
> > >  	u16 watermark;
> > >  	u8 decimator;
> > >  	u8 sip;
> > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > index 7dd5205aea5b..c1059a79f5ff 100644
> > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > @@ -457,17 +457,29 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
> > >  			}
> > >  
> > >  			if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) {
> > > -				iio_push_to_buffers_with_timestamp(
> > > -					hw->iio_devs[ST_LSM6DSX_ID_GYRO],
> > > -					&hw->scan[ST_LSM6DSX_ID_GYRO],
> > > -					gyro_sensor->ts_ref + ts);
> > > +				/* We need to discards gyro samples during  
> > 
> > Trivial but wrong comment syntax. If that's all that comes up I'll fix it here
> > and in other instances below when applying.
> >   
> > > +				 * filters settling time
> > > +				 */
> > > +				if (gyro_sensor->samples_to_discard > 0)
> > > +					gyro_sensor->samples_to_discard--;
> > > +				else
> > > +					iio_push_to_buffers_with_timestamp(
> > > +						hw->iio_devs[ST_LSM6DSX_ID_GYRO],
> > > +						&hw->scan[ST_LSM6DSX_ID_GYRO],
> > > +						gyro_sensor->ts_ref + ts);
> > >  				gyro_sip--;
> > >  			}
> > >  			if (acc_sip > 0 && !(sip % acc_sensor->decimator)) {
> > > -				iio_push_to_buffers_with_timestamp(
> > > -					hw->iio_devs[ST_LSM6DSX_ID_ACC],
> > > -					&hw->scan[ST_LSM6DSX_ID_ACC],
> > > -					acc_sensor->ts_ref + ts);
> > > +				/* We need to discards accel samples during
> > > +				 * filters settling time
> > > +				 */
> > > +				if (acc_sensor->samples_to_discard > 0)
> > > +					acc_sensor->samples_to_discard--;
> > > +				else
> > > +					iio_push_to_buffers_with_timestamp(
> > > +						hw->iio_devs[ST_LSM6DSX_ID_ACC],
> > > +						&hw->scan[ST_LSM6DSX_ID_ACC],
> > > +						acc_sensor->ts_ref + ts);
> > >  				acc_sip--;
> > >  			}
> > >  			if (ext_sip > 0 && !(sip % ext_sensor->decimator)) {
> > > @@ -541,8 +553,12 @@ st_lsm6dsx_push_tagged_data(struct st_lsm6dsx_hw *hw, u8 tag,
> > >  	}
> > >  
> > >  	sensor = iio_priv(iio_dev);
> > > -	iio_push_to_buffers_with_timestamp(iio_dev, data,
> > > -					   ts + sensor->ts_ref);
> > > +	/* We need to discards gyro samples during filters settling time */
> > > +	if (sensor->samples_to_discard > 0)
> > > +		sensor->samples_to_discard--;
> > > +	else
> > > +		iio_push_to_buffers_with_timestamp(iio_dev, data,
> > > +						   ts + sensor->ts_ref);
> > >  
> > >  	return 0;
> > >  }
> > > @@ -654,6 +670,25 @@ int st_lsm6dsx_flush_fifo(struct st_lsm6dsx_hw *hw)
> > >  	return err;
> > >  }
> > >  
> > > +static void
> > > +st_lsm6dsx_update_samples_to_discard(struct st_lsm6dsx_sensor *sensor)
> > > +{
> > > +	const struct st_lsm6dsx_samples_to_discard *data;
> > > +	int i;
> > > +
> > > +	if (sensor->id != ST_LSM6DSX_ID_GYRO &&
> > > +	    sensor->id != ST_LSM6DSX_ID_ACC)
> > > +		return;
> > > +
> > > +	data = &sensor->hw->settings->samples_to_discard[sensor->id];
> > > +	for (i = 0; i < ST_LSM6DSX_ODR_LIST_SIZE; i++) {
> > > +		if (data->val[i].milli_hz == sensor->odr) {
> > > +			sensor->samples_to_discard = data->val[i].samples;
> > > +			return;
> > > +		}
> > > +	}
> > > +}
> > > +
> > >  int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable)
> > >  {
> > >  	struct st_lsm6dsx_hw *hw = sensor->hw;
> > > @@ -673,6 +708,9 @@ int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable)
> > >  			goto out;
> > >  	}
> > >  
> > > +	if (enable)
> > > +		st_lsm6dsx_update_samples_to_discard(sensor);
> > > +
> > >  	err = st_lsm6dsx_device_set_enable(sensor, enable);
> > >  	if (err < 0)
> > >  		goto out;
> > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > > index 3f6060c64f32..966df6ffe874 100644
> > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > > @@ -634,6 +634,24 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
> > >  				.fs_len = 4,
> > >  			},
> > >  		},
> > > +		.samples_to_discard = {
> > > +			[ST_LSM6DSX_ID_ACC] = {
> > > +				.val[0] = {  12500, 1 },
> > > +				.val[1] = {  26000, 1 },
> > > +				.val[2] = {  52000, 1 },
> > > +				.val[3] = { 104000, 2 },
> > > +				.val[4] = { 208000, 2 },
> > > +				.val[5] = { 416000, 2 },
> > > +			},
> > > +			[ST_LSM6DSX_ID_GYRO] = {
> > > +				.val[0] = {  12500,  2 },
> > > +				.val[1] = {  26000,  5 },
> > > +				.val[2] = {  52000,  7 },
> > > +				.val[3] = { 104000, 12 },
> > > +				.val[4] = { 208000, 20 },
> > > +				.val[5] = { 416000, 36 },
> > > +			},
> > > +		},
> > >  		.irq_config = {
> > >  			.irq1 = {
> > >  				.addr = 0x0d,  
> >   
> 

Re: [PATCH] iio: imu: st_lsm6dsx: discard samples during filters settling time

[Lorenzo Bianconi]

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

> On Mon, 20 Feb 2023 10:12:29 +0100
> Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> 
> > > On Wed,  8 Feb 2023 15:42:31 +0100
> > > Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> > >   
> > > > During digital filters settling time the driver is expected to drop
> > > > samples since they can be corrupted. Introduce the capability to drop
> > > > a given number of samples according to the configured ODR.
> > > > Add the sample_to_discard data for LSM6DSM sensor.
> > > > 
> > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>  
> > > 
> > > Is this only necessary for the particular sensor you have provided
> > > values for?  Or is it more general?
> > > 
> > > I think the code will currently just set the number of samples to discard
> > > to 0 for other cases (as no value set for those sensor types).
> > > That's fine if 0 is definitely the right value for those other sensors.  
> > 
> >  I think all the sensors have this information in the datasheet/application
> >  note. However, even if we add sample_to_discard just for LSM6DSM for the moment,
> >  we do not introduce any regression for the other sensors with respect to the
> >  previous codebase since sample_to_discard will be just set to 0 (so we do not
> >  discard any sample). I can add sample_to_discard for LSM6DSO but at the
> >  moment I do not have other devices for testing.
> >  For LSM6DSO, do you prefer to add it in v2 or is it fine a follow-up patch?
> 
> It's fine to do this as and when we can test particular devices
> (or frankly just assume datasheet correct). 
> 
> We should call it out in the patch description though so hopefully people
> notice they need to add it for sensors they care about.

ack, I will post v2 adding LSM6DSO in this case and improving commit log.

Regards,
Lorenzo

> 
> > 
> >  Regards,
> >  Lorenzo
> > 
> > > 
> > > Thanks,
> > > 
> > > Jonathan
> > > 
> > >   
> > > > ---
> > > >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h       | 11 ++++
> > > >  .../iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c    | 58 +++++++++++++++----
> > > >  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c  | 18 ++++++
> > > >  3 files changed, 77 insertions(+), 10 deletions(-)
> > > > 
> > > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > > > index 499fcf8875b4..8e119d78730b 100644
> > > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> > > > @@ -137,6 +137,13 @@ struct st_lsm6dsx_odr_table_entry {
> > > >  	int odr_len;
> > > >  };
> > > >  
> > > > +struct st_lsm6dsx_samples_to_discard {
> > > > +	struct {
> > > > +		u32 milli_hz;
> > > > +		u16 samples;
> > > > +	} val[ST_LSM6DSX_ODR_LIST_SIZE];
> > > > +};
> > > > +
> > > >  struct st_lsm6dsx_fs {
> > > >  	u32 gain;
> > > >  	u8 val;
> > > > @@ -291,6 +298,7 @@ struct st_lsm6dsx_ext_dev_settings {
> > > >   * @irq_config: interrupts related registers.
> > > >   * @drdy_mask: register info for data-ready mask (addr + mask).
> > > >   * @odr_table: Hw sensors odr table (Hz + val).
> > > > + * @samples_to_discard: Number of samples to discard for filters settling time.
> > > >   * @fs_table: Hw sensors gain table (gain + val).
> > > >   * @decimator: List of decimator register info (addr + mask).
> > > >   * @batch: List of FIFO batching register info (addr + mask).
> > > > @@ -323,6 +331,7 @@ struct st_lsm6dsx_settings {
> > > >  	} irq_config;
> > > >  	struct st_lsm6dsx_reg drdy_mask;
> > > >  	struct st_lsm6dsx_odr_table_entry odr_table[2];
> > > > +	struct st_lsm6dsx_samples_to_discard samples_to_discard[2];
> > > >  	struct st_lsm6dsx_fs_table_entry fs_table[2];
> > > >  	struct st_lsm6dsx_reg decimator[ST_LSM6DSX_MAX_ID];
> > > >  	struct st_lsm6dsx_reg batch[ST_LSM6DSX_MAX_ID];
> > > > @@ -353,6 +362,7 @@ enum st_lsm6dsx_fifo_mode {
> > > >   * @hw: Pointer to instance of struct st_lsm6dsx_hw.
> > > >   * @gain: Configured sensor sensitivity.
> > > >   * @odr: Output data rate of the sensor [Hz].
> > > > + * @samples_to_discard: Number of samples to discard for filters settling time.
> > > >   * @watermark: Sensor watermark level.
> > > >   * @decimator: Sensor decimation factor.
> > > >   * @sip: Number of samples in a given pattern.
> > > > @@ -367,6 +377,7 @@ struct st_lsm6dsx_sensor {
> > > >  	u32 gain;
> > > >  	u32 odr;
> > > >  
> > > > +	u16 samples_to_discard;
> > > >  	u16 watermark;
> > > >  	u8 decimator;
> > > >  	u8 sip;
> > > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > > index 7dd5205aea5b..c1059a79f5ff 100644
> > > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> > > > @@ -457,17 +457,29 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
> > > >  			}
> > > >  
> > > >  			if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) {
> > > > -				iio_push_to_buffers_with_timestamp(
> > > > -					hw->iio_devs[ST_LSM6DSX_ID_GYRO],
> > > > -					&hw->scan[ST_LSM6DSX_ID_GYRO],
> > > > -					gyro_sensor->ts_ref + ts);
> > > > +				/* We need to discards gyro samples during  
> > > 
> > > Trivial but wrong comment syntax. If that's all that comes up I'll fix it here
> > > and in other instances below when applying.
> > >   
> > > > +				 * filters settling time
> > > > +				 */
> > > > +				if (gyro_sensor->samples_to_discard > 0)
> > > > +					gyro_sensor->samples_to_discard--;
> > > > +				else
> > > > +					iio_push_to_buffers_with_timestamp(
> > > > +						hw->iio_devs[ST_LSM6DSX_ID_GYRO],
> > > > +						&hw->scan[ST_LSM6DSX_ID_GYRO],
> > > > +						gyro_sensor->ts_ref + ts);
> > > >  				gyro_sip--;
> > > >  			}
> > > >  			if (acc_sip > 0 && !(sip % acc_sensor->decimator)) {
> > > > -				iio_push_to_buffers_with_timestamp(
> > > > -					hw->iio_devs[ST_LSM6DSX_ID_ACC],
> > > > -					&hw->scan[ST_LSM6DSX_ID_ACC],
> > > > -					acc_sensor->ts_ref + ts);
> > > > +				/* We need to discards accel samples during
> > > > +				 * filters settling time
> > > > +				 */
> > > > +				if (acc_sensor->samples_to_discard > 0)
> > > > +					acc_sensor->samples_to_discard--;
> > > > +				else
> > > > +					iio_push_to_buffers_with_timestamp(
> > > > +						hw->iio_devs[ST_LSM6DSX_ID_ACC],
> > > > +						&hw->scan[ST_LSM6DSX_ID_ACC],
> > > > +						acc_sensor->ts_ref + ts);
> > > >  				acc_sip--;
> > > >  			}
> > > >  			if (ext_sip > 0 && !(sip % ext_sensor->decimator)) {
> > > > @@ -541,8 +553,12 @@ st_lsm6dsx_push_tagged_data(struct st_lsm6dsx_hw *hw, u8 tag,
> > > >  	}
> > > >  
> > > >  	sensor = iio_priv(iio_dev);
> > > > -	iio_push_to_buffers_with_timestamp(iio_dev, data,
> > > > -					   ts + sensor->ts_ref);
> > > > +	/* We need to discards gyro samples during filters settling time */
> > > > +	if (sensor->samples_to_discard > 0)
> > > > +		sensor->samples_to_discard--;
> > > > +	else
> > > > +		iio_push_to_buffers_with_timestamp(iio_dev, data,
> > > > +						   ts + sensor->ts_ref);
> > > >  
> > > >  	return 0;
> > > >  }
> > > > @@ -654,6 +670,25 @@ int st_lsm6dsx_flush_fifo(struct st_lsm6dsx_hw *hw)
> > > >  	return err;
> > > >  }
> > > >  
> > > > +static void
> > > > +st_lsm6dsx_update_samples_to_discard(struct st_lsm6dsx_sensor *sensor)
> > > > +{
> > > > +	const struct st_lsm6dsx_samples_to_discard *data;
> > > > +	int i;
> > > > +
> > > > +	if (sensor->id != ST_LSM6DSX_ID_GYRO &&
> > > > +	    sensor->id != ST_LSM6DSX_ID_ACC)
> > > > +		return;
> > > > +
> > > > +	data = &sensor->hw->settings->samples_to_discard[sensor->id];
> > > > +	for (i = 0; i < ST_LSM6DSX_ODR_LIST_SIZE; i++) {
> > > > +		if (data->val[i].milli_hz == sensor->odr) {
> > > > +			sensor->samples_to_discard = data->val[i].samples;
> > > > +			return;
> > > > +		}
> > > > +	}
> > > > +}
> > > > +
> > > >  int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable)
> > > >  {
> > > >  	struct st_lsm6dsx_hw *hw = sensor->hw;
> > > > @@ -673,6 +708,9 @@ int st_lsm6dsx_update_fifo(struct st_lsm6dsx_sensor *sensor, bool enable)
> > > >  			goto out;
> > > >  	}
> > > >  
> > > > +	if (enable)
> > > > +		st_lsm6dsx_update_samples_to_discard(sensor);
> > > > +
> > > >  	err = st_lsm6dsx_device_set_enable(sensor, enable);
> > > >  	if (err < 0)
> > > >  		goto out;
> > > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > > > index 3f6060c64f32..966df6ffe874 100644
> > > > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > > > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> > > > @@ -634,6 +634,24 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
> > > >  				.fs_len = 4,
> > > >  			},
> > > >  		},
> > > > +		.samples_to_discard = {
> > > > +			[ST_LSM6DSX_ID_ACC] = {
> > > > +				.val[0] = {  12500, 1 },
> > > > +				.val[1] = {  26000, 1 },
> > > > +				.val[2] = {  52000, 1 },
> > > > +				.val[3] = { 104000, 2 },
> > > > +				.val[4] = { 208000, 2 },
> > > > +				.val[5] = { 416000, 2 },
> > > > +			},
> > > > +			[ST_LSM6DSX_ID_GYRO] = {
> > > > +				.val[0] = {  12500,  2 },
> > > > +				.val[1] = {  26000,  5 },
> > > > +				.val[2] = {  52000,  7 },
> > > > +				.val[3] = { 104000, 12 },
> > > > +				.val[4] = { 208000, 20 },
> > > > +				.val[5] = { 416000, 36 },
> > > > +			},
> > > > +		},
> > > >  		.irq_config = {
> > > >  			.irq1 = {
> > > >  				.addr = 0x0d,  
> > >   
> > 
> 

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