All of lore.kernel.org
 help / color / mirror / Atom feed
* st_lsm6dsx : first two values of ism330dlc_gyro are wrong
@ 2023-02-05 10:12 Philippe De Muyter
  2023-02-05 14:13 ` Jonathan Cameron
  0 siblings, 1 reply; 7+ messages in thread
From: Philippe De Muyter @ 2023-02-05 10:12 UTC (permalink / raw)
  To: lorenzo, linux-iio

Hello Lorenzo and list,

I use the imu/st_lsm6dsx (i2c) driver to drive a ism330dlc imu.

Every time I start a new acquisition of gyro values, the first two
values read are wrong, as can be see here :

 $ sudo ./iio_generic_buffer -n ism330dlc_gyro -g -c 10 -a
 iio device number being used is 1
 trigger-less mode selected
 No channels are enabled, enabling all channels
 Enabling: in_anglvel_z_en
 Enabling: in_timestamp_en
 Enabling: in_anglvel_y_en
 Enabling: in_anglvel_x_en
 -0.138924 -0.915246 0.470628 1675591514696125669
 -0.012699 -0.362151 0.143208 1675591514772675669
 0.001989 -0.076500 0.035190 1675591514849250669
 0.002295 -0.076194 0.035343 1675591514925825669
 0.002142 -0.076041 0.035343 1675591515002400669
 0.001989 -0.076041 0.035343 1675591515078975669
 0.001836 -0.076347 0.035649 1675591515155525669
 0.001836 -0.076500 0.035649 1675591515232075669
 0.001989 -0.076500 0.035649 1675591515308625669
 0.001989 -0.076347 0.035649 1675591515385200669
 Disabling: in_anglvel_z_en
 Disabling: in_timestamp_en
 Disabling: in_anglvel_y_en
 Disabling: in_anglvel_x_en
 $

Is that a normal behaviour for a gyro in that family or is it be caused
by a software or hardware bug ?

Best regards

Philippe

-- 
Philippe De Muyter +32 2 6101532 Macq SA rue de l'Aeronef 2 B-1140 Bruxelles

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: st_lsm6dsx : first two values of ism330dlc_gyro are wrong
  2023-02-05 10:12 st_lsm6dsx : first two values of ism330dlc_gyro are wrong Philippe De Muyter
@ 2023-02-05 14:13 ` Jonathan Cameron
  2023-02-05 15:22   ` Philippe De Muyter
  2023-02-06  9:58   ` Lorenzo Bianconi
  0 siblings, 2 replies; 7+ messages in thread
From: Jonathan Cameron @ 2023-02-05 14:13 UTC (permalink / raw)
  To: Philippe De Muyter; +Cc: lorenzo, linux-iio

On Sun, 5 Feb 2023 11:12:23 +0100
Philippe De Muyter <phdm@macq.eu> wrote:

> Hello Lorenzo and list,
> 
> I use the imu/st_lsm6dsx (i2c) driver to drive a ism330dlc imu.
> 
> Every time I start a new acquisition of gyro values, the first two
> values read are wrong, as can be see here :
> 
>  $ sudo ./iio_generic_buffer -n ism330dlc_gyro -g -c 10 -a
>  iio device number being used is 1
>  trigger-less mode selected
>  No channels are enabled, enabling all channels
>  Enabling: in_anglvel_z_en
>  Enabling: in_timestamp_en
>  Enabling: in_anglvel_y_en
>  Enabling: in_anglvel_x_en
>  -0.138924 -0.915246 0.470628 1675591514696125669
>  -0.012699 -0.362151 0.143208 1675591514772675669
>  0.001989 -0.076500 0.035190 1675591514849250669
>  0.002295 -0.076194 0.035343 1675591514925825669
>  0.002142 -0.076041 0.035343 1675591515002400669
>  0.001989 -0.076041 0.035343 1675591515078975669
>  0.001836 -0.076347 0.035649 1675591515155525669
>  0.001836 -0.076500 0.035649 1675591515232075669
>  0.001989 -0.076500 0.035649 1675591515308625669
>  0.001989 -0.076347 0.035649 1675591515385200669
>  Disabling: in_anglvel_z_en
>  Disabling: in_timestamp_en
>  Disabling: in_anglvel_y_en
>  Disabling: in_anglvel_x_en
>  $
> 
> Is that a normal behaviour for a gyro in that family or is it be caused
> by a software or hardware bug ?

So, some random thoughts on what might be going on...
1) Stale data in the fifo.  Could you run this experiment twice whilst being
   careful not to move the device between the runs.  If we still see the wrong
   values at the start then it's not that...

2) Device takes a little whilst to stabilize. Possibly this is down to the
   low pass filters requiring a few samples before they deliver stable output.
   From a quick glance I don't think we provide any userspace control of those
   filters and I think LPR1 is left in default state of disabled.

You could try messing with the sampling frequency as that may affect the number
of bad samples you see and give us more of a clue (it affects lpf2 directly).

Jonathan

> 
> Best regards
> 
> Philippe
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: st_lsm6dsx : first two values of ism330dlc_gyro are wrong
  2023-02-05 14:13 ` Jonathan Cameron
@ 2023-02-05 15:22   ` Philippe De Muyter
  2023-02-06  9:58   ` Lorenzo Bianconi
  1 sibling, 0 replies; 7+ messages in thread
From: Philippe De Muyter @ 2023-02-05 15:22 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: lorenzo, linux-iio

On Sun, Feb 05, 2023 at 02:13:42PM +0000, Jonathan Cameron wrote:
> 
> On Sun, 5 Feb 2023 11:12:23 +0100
> Philippe De Muyter <phdm@macq.eu> wrote:
> 
> > I use the imu/st_lsm6dsx (i2c) driver to drive a ism330dlc imu.
> >
> > Every time I start a new acquisition of gyro values, the first two
> > values read are wrong, as can be see here :
> >
> 
> So, some random thoughts on what might be going on...
> 1) Stale data in the fifo.  Could you run this experiment twice whilst being
>    careful not to move the device between the runs.  If we still see the wrong
>    values at the start then it's not that...

Here are the results (I have enabled by hand the three in_anglvel channels)
 # 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.133416 -0.916011 0.466650
 -0.014382 -0.360774 0.140454
 -0.000612 -0.076653 0.035190
 -0.000459 -0.076347 0.035496
 -0.000306 -0.076347 0.035037
 -0.000612 -0.076347 0.035037
 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.134640 -0.911880 0.467415
 -0.013923 -0.361233 0.142443
 -0.000765 -0.076041 0.035496
 -0.000459 -0.076041 0.035190
 -0.000306 -0.076347 0.035343
 -0.000612 -0.076347 0.035343
 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.137853 -0.912645 0.468792
 -0.013770 -0.361233 0.142443
 -0.000153 -0.076500 0.035190
 -0.000306 -0.076347 0.035190
 -0.000306 -0.076194 0.035190
 -0.000153 -0.076194 0.035343
 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.136017 -0.914787 0.467874
 -0.014229 -0.361080 0.142290
 -0.000153 -0.076500 0.035649
 0.000918 -0.076347 0.039933
 0.000306 -0.076500 0.038556
 0.000459 -0.076194 0.036414
 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.133722 -0.914787 0.469710
 -0.013617 -0.361386 0.142290
 -0.000459 -0.076347 0.035037
 -0.000459 -0.076194 0.035190
 -0.000459 -0.075888 0.035037
 -0.000612 -0.076347 0.035649
 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.135711 -0.915399 0.468792
 -0.013923 -0.360927 0.142137
 -0.000459 -0.076347 0.035343
 -0.000459 -0.076194 0.035190
 -0.000306 -0.076041 0.035496
 -0.000459 -0.075888 0.035190
 ^C

> 
> 2) Device takes a little whilst to stabilize. Possibly this is down to the
>    low pass filters requiring a few samples before they deliver stable output.
>    From a quick glance I don't think we provide any userspace control of those
>    filters and I think LPR1 is left in default state of disabled.
> 
> You could try messing with the sampling frequency as that may affect the number
> of bad samples you see and give us more of a clue (it affects lpf2 directly).

Above is with default sampling frequency : 12.5

with 26, I get also two bad values, but different from the previous ones :

 Proceeding without activating any channels
 -0.242811 -1.215279 0.709155
 -0.055539 -1.216197 0.466650
 0.002142 -0.075582 0.034578
 0.002142 -0.075123 0.035955
 0.002295 -0.075582 0.036261
 0.002142 -0.075429 0.035955
 
with 52, I now get more bad values :
 
 Proceeding without activating any channels
 -0.267750 -0.745569 0.558909
 -0.486999 -3.376863 1.719567
 0.130815 -0.705942 0.054315
 0.001989 -0.075582 0.036414
 0.001224 -0.075123 0.035955
 0.001836 -0.075888 0.036108
 0.002448 -0.075582 0.036261
 0.002295 -0.075888 0.035190
 0.002295 -0.075888 0.036261
 0.002907 -0.075888 0.036261
 
with 106, I get evenmore bad values
 
 Proceeding without activating any channels
 -0.024174 -0.082773 -0.154377
 -0.412335 -0.387702 0.665856
 -0.450432 -2.840751 1.692486
 -1.784439 -4.931190 4.186539
 -1.090125 -4.231215 2.580345
 0.674730 -3.370896 0.127908
 0.562275 -2.838456 0.115209
 0.085680 -0.486846 0.045900
 0.003825 -0.081702 0.040392
 0.004437 -0.078030 0.037179
 0.004284 -0.076806 0.036261
 0.003213 -0.075735 0.035343
 0.003672 -0.075735 0.035343
 0.002295 -0.074970 0.036567
 0.002448 -0.076041 0.037179
 0.002448 -0.076347 0.037179
 0.002142 -0.076347 0.037332
 0.001989 -0.076194 0.036108
 0.001989 -0.076194 0.035955
 0.002142 -0.075276 0.036106

Philippe

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: st_lsm6dsx : first two values of ism330dlc_gyro are wrong
  2023-02-05 14:13 ` Jonathan Cameron
  2023-02-05 15:22   ` Philippe De Muyter
@ 2023-02-06  9:58   ` Lorenzo Bianconi
  2023-02-06 14:33     ` Jonathan Cameron
  1 sibling, 1 reply; 7+ messages in thread
From: Lorenzo Bianconi @ 2023-02-06  9:58 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Philippe De Muyter, linux-iio

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

> On Sun, 5 Feb 2023 11:12:23 +0100
> Philippe De Muyter <phdm@macq.eu> wrote:
> 
> > Hello Lorenzo and list,
> > 
> > I use the imu/st_lsm6dsx (i2c) driver to drive a ism330dlc imu.
> > 
> > Every time I start a new acquisition of gyro values, the first two
> > values read are wrong, as can be see here :
> > 
> >  $ sudo ./iio_generic_buffer -n ism330dlc_gyro -g -c 10 -a
> >  iio device number being used is 1
> >  trigger-less mode selected
> >  No channels are enabled, enabling all channels
> >  Enabling: in_anglvel_z_en
> >  Enabling: in_timestamp_en
> >  Enabling: in_anglvel_y_en
> >  Enabling: in_anglvel_x_en
> >  -0.138924 -0.915246 0.470628 1675591514696125669
> >  -0.012699 -0.362151 0.143208 1675591514772675669
> >  0.001989 -0.076500 0.035190 1675591514849250669
> >  0.002295 -0.076194 0.035343 1675591514925825669
> >  0.002142 -0.076041 0.035343 1675591515002400669
> >  0.001989 -0.076041 0.035343 1675591515078975669
> >  0.001836 -0.076347 0.035649 1675591515155525669
> >  0.001836 -0.076500 0.035649 1675591515232075669
> >  0.001989 -0.076500 0.035649 1675591515308625669
> >  0.001989 -0.076347 0.035649 1675591515385200669
> >  Disabling: in_anglvel_z_en
> >  Disabling: in_timestamp_en
> >  Disabling: in_anglvel_y_en
> >  Disabling: in_anglvel_x_en
> >  $
> > 
> > Is that a normal behaviour for a gyro in that family or is it be caused
> > by a software or hardware bug ?
> 
> So, some random thoughts on what might be going on...
> 1) Stale data in the fifo.  Could you run this experiment twice whilst being
>    careful not to move the device between the runs.  If we still see the wrong
>    values at the start then it's not that...

When the device is powered-down we set the FIFO in bypass mode and in-flight
samples are discarded.

> 
> 2) Device takes a little whilst to stabilize. Possibly this is down to the
>    low pass filters requiring a few samples before they deliver stable output.
>    From a quick glance I don't think we provide any userspace control of those
>    filters and I think LPR1 is left in default state of disabled.

I would say this issue is related to the "Accelerometer and gyroscope
turn-on/off time" (section 3.9 in the sensor application note).

https://www.st.com/resource/en/application_note/an5125-ism330dlc-3d-accelerometer-and-3d-gyroscope-with-digital-output-for-industrial-applications-stmicroelectronics.pdf

@Jonathan: do you think we should discard these sample in the driver or in the
user-space app? I would say this can be a general issue. What do you think?

Regards,
Lorenzo

> 
> You could try messing with the sampling frequency as that may affect the number
> of bad samples you see and give us more of a clue (it affects lpf2 directly).
> 
> Jonathan
> 
> > 
> > Best regards
> > 
> > Philippe
> > 
> 

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: st_lsm6dsx : first two values of ism330dlc_gyro are wrong
  2023-02-06  9:58   ` Lorenzo Bianconi
@ 2023-02-06 14:33     ` Jonathan Cameron
  2023-02-06 14:37       ` Lorenzo Bianconi
  2023-02-07 23:33       ` Lorenzo Bianconi
  0 siblings, 2 replies; 7+ messages in thread
From: Jonathan Cameron @ 2023-02-06 14:33 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: Jonathan Cameron, Philippe De Muyter, linux-iio

On Mon, 6 Feb 2023 10:58:08 +0100
Lorenzo Bianconi <lorenzo@kernel.org> wrote:

> > On Sun, 5 Feb 2023 11:12:23 +0100
> > Philippe De Muyter <phdm@macq.eu> wrote:
> >   
> > > Hello Lorenzo and list,
> > > 
> > > I use the imu/st_lsm6dsx (i2c) driver to drive a ism330dlc imu.
> > > 
> > > Every time I start a new acquisition of gyro values, the first two
> > > values read are wrong, as can be see here :
> > > 
> > >  $ sudo ./iio_generic_buffer -n ism330dlc_gyro -g -c 10 -a
> > >  iio device number being used is 1
> > >  trigger-less mode selected
> > >  No channels are enabled, enabling all channels
> > >  Enabling: in_anglvel_z_en
> > >  Enabling: in_timestamp_en
> > >  Enabling: in_anglvel_y_en
> > >  Enabling: in_anglvel_x_en
> > >  -0.138924 -0.915246 0.470628 1675591514696125669
> > >  -0.012699 -0.362151 0.143208 1675591514772675669
> > >  0.001989 -0.076500 0.035190 1675591514849250669
> > >  0.002295 -0.076194 0.035343 1675591514925825669
> > >  0.002142 -0.076041 0.035343 1675591515002400669
> > >  0.001989 -0.076041 0.035343 1675591515078975669
> > >  0.001836 -0.076347 0.035649 1675591515155525669
> > >  0.001836 -0.076500 0.035649 1675591515232075669
> > >  0.001989 -0.076500 0.035649 1675591515308625669
> > >  0.001989 -0.076347 0.035649 1675591515385200669
> > >  Disabling: in_anglvel_z_en
> > >  Disabling: in_timestamp_en
> > >  Disabling: in_anglvel_y_en
> > >  Disabling: in_anglvel_x_en
> > >  $
> > > 
> > > Is that a normal behaviour for a gyro in that family or is it be caused
> > > by a software or hardware bug ?  
> > 
> > So, some random thoughts on what might be going on...
> > 1) Stale data in the fifo.  Could you run this experiment twice whilst being
> >    careful not to move the device between the runs.  If we still see the wrong
> >    values at the start then it's not that...  
> 
> When the device is powered-down we set the FIFO in bypass mode and in-flight
> samples are discarded.
> 
> > 
> > 2) Device takes a little whilst to stabilize. Possibly this is down to the
> >    low pass filters requiring a few samples before they deliver stable output.
> >    From a quick glance I don't think we provide any userspace control of those
> >    filters and I think LPR1 is left in default state of disabled.  
> 
> I would say this issue is related to the "Accelerometer and gyroscope
> turn-on/off time" (section 3.9 in the sensor application note).
> 
> https://www.st.com/resource/en/application_note/an5125-ism330dlc-3d-accelerometer-and-3d-gyroscope-with-digital-output-for-industrial-applications-stmicroelectronics.pdf
> 
> @Jonathan: do you think we should discard these sample in the driver or in the
> user-space app? I would say this can be a general issue. What do you think?

In driver.  This isn't an uncommon problem for sensors and userspace would in 
general have no idea how many samples to drop.  Also dependent on the sampling
rates etc so if we support control of those, we'll want to have the driver
drop the right number of samples.  Though yikes at the top end. You can have
to drop 540 samples... Ah well. That is at 6kHz sampling so still not very long.

The only alternative would be to expose the current number to drop to userspace
but then existing userspace code would not drop them.  Hence I think it needs
to be in driver.

Jonathan


> 
> Regards,
> Lorenzo
> 
> > 
> > You could try messing with the sampling frequency as that may affect the number
> > of bad samples you see and give us more of a clue (it affects lpf2 directly).
> > 
> > Jonathan
> >   
> > > 
> > > Best regards
> > > 
> > > Philippe
> > >   
> >   
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: st_lsm6dsx : first two values of ism330dlc_gyro are wrong
  2023-02-06 14:33     ` Jonathan Cameron
@ 2023-02-06 14:37       ` Lorenzo Bianconi
  2023-02-07 23:33       ` Lorenzo Bianconi
  1 sibling, 0 replies; 7+ messages in thread
From: Lorenzo Bianconi @ 2023-02-06 14:37 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Jonathan Cameron, Philippe De Muyter, linux-iio

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

> On Mon, 6 Feb 2023 10:58:08 +0100
> Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> 
> > > On Sun, 5 Feb 2023 11:12:23 +0100
> > > Philippe De Muyter <phdm@macq.eu> wrote:
> > >   
> > > > Hello Lorenzo and list,
> > > > 
> > > > I use the imu/st_lsm6dsx (i2c) driver to drive a ism330dlc imu.
> > > > 
> > > > Every time I start a new acquisition of gyro values, the first two
> > > > values read are wrong, as can be see here :
> > > > 
> > > >  $ sudo ./iio_generic_buffer -n ism330dlc_gyro -g -c 10 -a
> > > >  iio device number being used is 1
> > > >  trigger-less mode selected
> > > >  No channels are enabled, enabling all channels
> > > >  Enabling: in_anglvel_z_en
> > > >  Enabling: in_timestamp_en
> > > >  Enabling: in_anglvel_y_en
> > > >  Enabling: in_anglvel_x_en
> > > >  -0.138924 -0.915246 0.470628 1675591514696125669
> > > >  -0.012699 -0.362151 0.143208 1675591514772675669
> > > >  0.001989 -0.076500 0.035190 1675591514849250669
> > > >  0.002295 -0.076194 0.035343 1675591514925825669
> > > >  0.002142 -0.076041 0.035343 1675591515002400669
> > > >  0.001989 -0.076041 0.035343 1675591515078975669
> > > >  0.001836 -0.076347 0.035649 1675591515155525669
> > > >  0.001836 -0.076500 0.035649 1675591515232075669
> > > >  0.001989 -0.076500 0.035649 1675591515308625669
> > > >  0.001989 -0.076347 0.035649 1675591515385200669
> > > >  Disabling: in_anglvel_z_en
> > > >  Disabling: in_timestamp_en
> > > >  Disabling: in_anglvel_y_en
> > > >  Disabling: in_anglvel_x_en
> > > >  $
> > > > 
> > > > Is that a normal behaviour for a gyro in that family or is it be caused
> > > > by a software or hardware bug ?  
> > > 
> > > So, some random thoughts on what might be going on...
> > > 1) Stale data in the fifo.  Could you run this experiment twice whilst being
> > >    careful not to move the device between the runs.  If we still see the wrong
> > >    values at the start then it's not that...  
> > 
> > When the device is powered-down we set the FIFO in bypass mode and in-flight
> > samples are discarded.
> > 
> > > 
> > > 2) Device takes a little whilst to stabilize. Possibly this is down to the
> > >    low pass filters requiring a few samples before they deliver stable output.
> > >    From a quick glance I don't think we provide any userspace control of those
> > >    filters and I think LPR1 is left in default state of disabled.  
> > 
> > I would say this issue is related to the "Accelerometer and gyroscope
> > turn-on/off time" (section 3.9 in the sensor application note).
> > 
> > https://www.st.com/resource/en/application_note/an5125-ism330dlc-3d-accelerometer-and-3d-gyroscope-with-digital-output-for-industrial-applications-stmicroelectronics.pdf
> > 
> > @Jonathan: do you think we should discard these sample in the driver or in the
> > user-space app? I would say this can be a general issue. What do you think?
> 
> In driver.  This isn't an uncommon problem for sensors and userspace would in 
> general have no idea how many samples to drop.  Also dependent on the sampling
> rates etc so if we support control of those, we'll want to have the driver
> drop the right number of samples.  Though yikes at the top end. You can have
> to drop 540 samples... Ah well. That is at 6kHz sampling so still not very long.
> 
> The only alternative would be to expose the current number to drop to userspace
> but then existing userspace code would not drop them.  Hence I think it needs
> to be in driver.

ack, I will work on it.

Regards,
Lorenzo

> 
> Jonathan
> 
> 
> > 
> > Regards,
> > Lorenzo
> > 
> > > 
> > > You could try messing with the sampling frequency as that may affect the number
> > > of bad samples you see and give us more of a clue (it affects lpf2 directly).
> > > 
> > > Jonathan
> > >   
> > > > 
> > > > Best regards
> > > > 
> > > > Philippe
> > > >   
> > >   
> > 
> 

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: st_lsm6dsx : first two values of ism330dlc_gyro are wrong
  2023-02-06 14:33     ` Jonathan Cameron
  2023-02-06 14:37       ` Lorenzo Bianconi
@ 2023-02-07 23:33       ` Lorenzo Bianconi
  1 sibling, 0 replies; 7+ messages in thread
From: Lorenzo Bianconi @ 2023-02-07 23:33 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Jonathan Cameron, Philippe De Muyter, linux-iio, lorenzo.bianconi

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

> On Mon, 6 Feb 2023 10:58:08 +0100
> Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> 
> > > On Sun, 5 Feb 2023 11:12:23 +0100
> > > Philippe De Muyter <phdm@macq.eu> wrote:
> > >   
> > > > Hello Lorenzo and list,
> > > > 
> > > > I use the imu/st_lsm6dsx (i2c) driver to drive a ism330dlc imu.
> > > > 
> > > > Every time I start a new acquisition of gyro values, the first two
> > > > values read are wrong, as can be see here :
> > > > 
> > > >  $ sudo ./iio_generic_buffer -n ism330dlc_gyro -g -c 10 -a
> > > >  iio device number being used is 1
> > > >  trigger-less mode selected
> > > >  No channels are enabled, enabling all channels
> > > >  Enabling: in_anglvel_z_en
> > > >  Enabling: in_timestamp_en
> > > >  Enabling: in_anglvel_y_en
> > > >  Enabling: in_anglvel_x_en
> > > >  -0.138924 -0.915246 0.470628 1675591514696125669
> > > >  -0.012699 -0.362151 0.143208 1675591514772675669
> > > >  0.001989 -0.076500 0.035190 1675591514849250669
> > > >  0.002295 -0.076194 0.035343 1675591514925825669
> > > >  0.002142 -0.076041 0.035343 1675591515002400669
> > > >  0.001989 -0.076041 0.035343 1675591515078975669
> > > >  0.001836 -0.076347 0.035649 1675591515155525669
> > > >  0.001836 -0.076500 0.035649 1675591515232075669
> > > >  0.001989 -0.076500 0.035649 1675591515308625669
> > > >  0.001989 -0.076347 0.035649 1675591515385200669
> > > >  Disabling: in_anglvel_z_en
> > > >  Disabling: in_timestamp_en
> > > >  Disabling: in_anglvel_y_en
> > > >  Disabling: in_anglvel_x_en
> > > >  $
> > > > 
> > > > Is that a normal behaviour for a gyro in that family or is it be caused
> > > > by a software or hardware bug ?  
> > > 
> > > So, some random thoughts on what might be going on...
> > > 1) Stale data in the fifo.  Could you run this experiment twice whilst being
> > >    careful not to move the device between the runs.  If we still see the wrong
> > >    values at the start then it's not that...  
> > 
> > When the device is powered-down we set the FIFO in bypass mode and in-flight
> > samples are discarded.
> > 
> > > 
> > > 2) Device takes a little whilst to stabilize. Possibly this is down to the
> > >    low pass filters requiring a few samples before they deliver stable output.
> > >    From a quick glance I don't think we provide any userspace control of those
> > >    filters and I think LPR1 is left in default state of disabled.  
> > 
> > I would say this issue is related to the "Accelerometer and gyroscope
> > turn-on/off time" (section 3.9 in the sensor application note).
> > 
> > https://www.st.com/resource/en/application_note/an5125-ism330dlc-3d-accelerometer-and-3d-gyroscope-with-digital-output-for-industrial-applications-stmicroelectronics.pdf
> > 
> > @Jonathan: do you think we should discard these sample in the driver or in the
> > user-space app? I would say this can be a general issue. What do you think?
> 
> In driver.  This isn't an uncommon problem for sensors and userspace would in 
> general have no idea how many samples to drop.  Also dependent on the sampling
> rates etc so if we support control of those, we'll want to have the driver
> drop the right number of samples.  Though yikes at the top end. You can have
> to drop 540 samples... Ah well. That is at 6kHz sampling so still not very long.
> 
> The only alternative would be to expose the current number to drop to userspace
> but then existing userspace code would not drop them.  Hence I think it needs
> to be in driver.
> 
> Jonathan
> 
> 
> > 
> > Regards,
> > Lorenzo
> > 
> > > 
> > > You could try messing with the sampling frequency as that may affect the number
> > > of bad samples you see and give us more of a clue (it affects lpf2 directly).
> > > 
> > > Jonathan
> > >   
> > > > 
> > > > Best regards
> > > > 
> > > > Philippe

Hi Philippe,

can you please test the patch below? Please note it is just compiled tested.

Regards,
Lorenzo

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
index 499fcf8875b4..2fb4d75ad096 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;
+	} map[ST_LSM6DSX_ODR_LIST_SIZE];
+};
+
 struct st_lsm6dsx_fs {
 	u32 gain;
 	u8 val;
@@ -323,6 +330,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 +361,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 transitory.
  * @watermark: Sensor watermark level.
  * @decimator: Sensor decimation factor.
  * @sip: Number of samples in a given pattern.
@@ -367,6 +376,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..1b78238db388 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -418,12 +418,22 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
 				       &hw->buff[offset],
 				       sizeof(hw->scan[ST_LSM6DSX_ID_GYRO].channels));
 				offset += sizeof(hw->scan[ST_LSM6DSX_ID_GYRO].channels);
+				/* We need to discards gyro samples during
+				 * filters settiling time
+				 */
+				if (gyro_sensor->samples_to_discard)
+					gyro_sensor->samples_to_discard--;
 			}
 			if (acc_sip > 0 && !(sip % acc_sensor->decimator)) {
 				memcpy(hw->scan[ST_LSM6DSX_ID_ACC].channels,
 				       &hw->buff[offset],
 				       sizeof(hw->scan[ST_LSM6DSX_ID_ACC].channels));
 				offset += sizeof(hw->scan[ST_LSM6DSX_ID_ACC].channels);
+				/* We need to discards accel samples during
+				 * filters settiling time
+				 */
+				if (acc_sensor->samples_to_discard)
+					acc_sensor->samples_to_discard--;
 			}
 			if (ext_sip > 0 && !(sip % ext_sensor->decimator)) {
 				memcpy(hw->scan[ST_LSM6DSX_ID_EXT0].channels,
@@ -456,14 +466,16 @@ int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
 				offset += ST_LSM6DSX_SAMPLE_SIZE;
 			}
 
-			if (gyro_sip > 0 && !(sip % gyro_sensor->decimator)) {
+			if (gyro_sip > 0 && !(sip % gyro_sensor->decimator) &&
+			    !gyro_sensor->samples_to_discard) {
 				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)) {
+			if (acc_sip > 0 && !(sip % acc_sensor->decimator) &&
+			    !acc_sensor->samples_to_discard) {
 				iio_push_to_buffers_with_timestamp(
 					hw->iio_devs[ST_LSM6DSX_ID_ACC],
 					&hw->scan[ST_LSM6DSX_ID_ACC],
@@ -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 settiling time */
+	if (!sensor->samples_to_discard)
+		iio_push_to_buffers_with_timestamp(iio_dev, data,
+						   ts + sensor->ts_ref);
+	else
+		sensor->samples_to_discard--;
 
 	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->map[i].milli_hz == sensor->odr) {
+			sensor->samples_to_discard = data->map[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..a74a702205e8 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
@@ -610,6 +610,24 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
 				.odr_len = 6,
 			},
 		},
+		.samples_to_discard = {
+			[ST_LSM6DSX_ID_ACC] = {
+				.map[0] = {  12500, 1 },
+				.map[1] = {  26000, 1 },
+				.map[2] = {  52000, 1 },
+				.map[3] = { 104000, 2 },
+				.map[4] = { 208000, 2 },
+				.map[5] = { 416000, 2 },
+			},
+			[ST_LSM6DSX_ID_GYRO] = {
+				.map[0] = {  12500, 2 },
+				.map[1] = {  26000, 3 },
+				.map[2] = {  52000, 3 },
+				.map[3] = { 104000, 4 },
+				.map[4] = { 208000, 5 },
+				.map[5] = { 416000, 6 },
+			},
+		},
 		.fs_table = {
 			[ST_LSM6DSX_ID_ACC] = {
 				.reg = {

> > > >   
> > >   
> > 
> 

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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-02-07 23:33 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-05 10:12 st_lsm6dsx : first two values of ism330dlc_gyro are wrong Philippe De Muyter
2023-02-05 14:13 ` Jonathan Cameron
2023-02-05 15:22   ` Philippe De Muyter
2023-02-06  9:58   ` Lorenzo Bianconi
2023-02-06 14:33     ` Jonathan Cameron
2023-02-06 14:37       ` Lorenzo Bianconi
2023-02-07 23:33       ` Lorenzo Bianconi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.