| Message ID | 20210507103220.2194009-1-sean@geanix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iio: imu: st_lsm6dsx: do not roundup set samplerate | expand |
> The correct way to set the sampling rate to 12.5 Hz before > commit f8710f0357bc ("iio: imu: st_lsm6dsx: express odr in mHZ") > was to write 13 Hz to sampling_frequency. > Before this patch writing 13 to samplerate results in sample rate set > to 26 Hz. > Now we return EINVAL if the sampling rate is not in table. > > Signed-off-by: Sean Nyekjaer <sean@geanix.com> I think this patch is breaking i2c-master support available in st_lsm6dsx, have you tested it? > --- > drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c > index a3ab16615729..a14a1df8537b 100644 > --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c > +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c > @@ -1427,7 +1427,7 @@ int st_lsm6dsx_check_odr(struct st_lsm6dsx_sensor *sensor, u32 odr, u8 *val) > * ext devices can run at different odr respect to > * accel sensor > */ > - if (odr_table->odr_avl[i].milli_hz >= odr) > + if (odr_table->odr_avl[i].milli_hz == odr) > break; > } > > -- > 2.31.0 >
On 07/05/2021 13.02, Lorenzo Bianconi wrote: >> The correct way to set the sampling rate to 12.5 Hz before >> commit f8710f0357bc ("iio: imu: st_lsm6dsx: express odr in mHZ") >> was to write 13 Hz to sampling_frequency. >> Before this patch writing 13 to samplerate results in sample rate set >> to 26 Hz. >> Now we return EINVAL if the sampling rate is not in table. >> >> Signed-off-by: Sean Nyekjaer <sean@geanix.com> > > I think this patch is breaking i2c-master support available in st_lsm6dsx, have you tested it? > I have not tested with i2c-master. But it's not nice to round up sample rate from user space without any notice. /Sean
> On 07/05/2021 13.02, Lorenzo Bianconi wrote: > >> The correct way to set the sampling rate to 12.5 Hz before > >> commit f8710f0357bc ("iio: imu: st_lsm6dsx: express odr in mHZ") > >> was to write 13 Hz to sampling_frequency. > >> Before this patch writing 13 to samplerate results in sample rate set > >> to 26 Hz. > >> Now we return EINVAL if the sampling rate is not in table. > >> > >> Signed-off-by: Sean Nyekjaer <sean@geanix.com> > > > > I think this patch is breaking i2c-master support available in st_lsm6dsx, have you tested it? > > > I have not tested with i2c-master. > But it's not nice to round up sample rate from user space without any notice. the point is accel odr is used as trigger for i2c-slave sampling. Connected i2c slave devices have different sample rates (e.g. LIS2MDL), so we need to select the lower accel odr greather than slave device one. Regards, Lorenzo > > /Sean >
On 10/05/2021 08.28, Lorenzo Bianconi wrote: >> On 07/05/2021 13.02, Lorenzo Bianconi wrote: >>>> The correct way to set the sampling rate to 12.5 Hz before >>>> commit f8710f0357bc ("iio: imu: st_lsm6dsx: express odr in mHZ") >>>> was to write 13 Hz to sampling_frequency. >>>> Before this patch writing 13 to samplerate results in sample rate set >>>> to 26 Hz. >>>> Now we return EINVAL if the sampling rate is not in table. >>>> >>>> Signed-off-by: Sean Nyekjaer <sean@geanix.com> >>> >>> I think this patch is breaking i2c-master support available in st_lsm6dsx, have you tested it? >>> >> I have not tested with i2c-master. >> But it's not nice to round up sample rate from user space without any notice. > > the point is accel odr is used as trigger for i2c-slave sampling. Connected > i2c slave devices have different sample rates (e.g. LIS2MDL), so we need to > select the lower accel odr greather than slave device one. > > Regards, > Lorenzo > We could open up for custom sample rates if i2c-slaves are enabled? /Sean
> On 10/05/2021 08.28, Lorenzo Bianconi wrote: > >> On 07/05/2021 13.02, Lorenzo Bianconi wrote: > >>>> The correct way to set the sampling rate to 12.5 Hz before > >>>> commit f8710f0357bc ("iio: imu: st_lsm6dsx: express odr in mHZ") > >>>> was to write 13 Hz to sampling_frequency. > >>>> Before this patch writing 13 to samplerate results in sample rate set > >>>> to 26 Hz. > >>>> Now we return EINVAL if the sampling rate is not in table. > >>>> > >>>> Signed-off-by: Sean Nyekjaer <sean@geanix.com> > >>> > >>> I think this patch is breaking i2c-master support available in st_lsm6dsx, have you tested it? > >>> > >> I have not tested with i2c-master. > >> But it's not nice to round up sample rate from user space without any notice. > > > > the point is accel odr is used as trigger for i2c-slave sampling. Connected > > i2c slave devices have different sample rates (e.g. LIS2MDL), so we need to > > select the lower accel odr greather than slave device one. > > > > Regards, > > Lorenzo > > > > We could open up for custom sample rates if i2c-slaves are enabled? can you please explain what you mean? > /Sean
On 10/05/2021 14.07, Lorenzo Bianconi wrote: >> On 10/05/2021 08.28, Lorenzo Bianconi wrote: >>>> On 07/05/2021 13.02, Lorenzo Bianconi wrote: >>>>>> The correct way to set the sampling rate to 12.5 Hz before >>>>>> commit f8710f0357bc ("iio: imu: st_lsm6dsx: express odr in mHZ") >>>>>> was to write 13 Hz to sampling_frequency. >>>>>> Before this patch writing 13 to samplerate results in sample rate set >>>>>> to 26 Hz. >>>>>> Now we return EINVAL if the sampling rate is not in table. >>>>>> >>>>>> Signed-off-by: Sean Nyekjaer <sean@geanix.com> >>>>> >>>>> I think this patch is breaking i2c-master support available in st_lsm6dsx, have you tested it? >>>>> >>>> I have not tested with i2c-master. >>>> But it's not nice to round up sample rate from user space without any notice. >>> >>> the point is accel odr is used as trigger for i2c-slave sampling. Connected >>> i2c slave devices have different sample rates (e.g. LIS2MDL), so we need to >>> select the lower accel odr greather than slave device one. >>> >>> Regards, >>> Lorenzo >>> >> >> We could open up for custom sample rates if i2c-slaves are enabled? > > can you please explain what you mean? > If i2c-slaves are enabled, it's allowed to set off table sample rates.(We roundup samplerate like now) If i2c-slaves are disabled, we return -EINVAL if we can't find the chosen samplerate in the samplerate table? /Sean
> On 10/05/2021 14.07, Lorenzo Bianconi wrote: > >> On 10/05/2021 08.28, Lorenzo Bianconi wrote: > >>>> On 07/05/2021 13.02, Lorenzo Bianconi wrote: > >>>>>> The correct way to set the sampling rate to 12.5 Hz before > >>>>>> commit f8710f0357bc ("iio: imu: st_lsm6dsx: express odr in mHZ") > >>>>>> was to write 13 Hz to sampling_frequency. > >>>>>> Before this patch writing 13 to samplerate results in sample rate set > >>>>>> to 26 Hz. > >>>>>> Now we return EINVAL if the sampling rate is not in table. > >>>>>> > >>>>>> Signed-off-by: Sean Nyekjaer <sean@geanix.com> > >>>>> > >>>>> I think this patch is breaking i2c-master support available in st_lsm6dsx, have you tested it? > >>>>> > >>>> I have not tested with i2c-master. > >>>> But it's not nice to round up sample rate from user space without any notice. > >>> > >>> the point is accel odr is used as trigger for i2c-slave sampling. Connected > >>> i2c slave devices have different sample rates (e.g. LIS2MDL), so we need to > >>> select the lower accel odr greather than slave device one. > >>> > >>> Regards, > >>> Lorenzo > >>> > >> > >> We could open up for custom sample rates if i2c-slaves are enabled? > > > > can you please explain what you mean? > > > If i2c-slaves are enabled, it's allowed to set off table sample rates.(We roundup samplerate like now) > If i2c-slaves are disabled, we return -EINVAL if we can't find the chosen samplerate in the samplerate table? IMO this is not a big deal because if you set the proper ODR (e.g. 26Hz) with the i2c-master disabled, the sample rate will not be rounded-up. If you set 20Hz, the accel ODR will be set to 26Hz. I think it does not worth to make the code uglier and more complex to maintain. Regards, Lorenzo > > /Sean
On 10/05/2021 16.47, Lorenzo Bianconi wrote: >> On 10/05/2021 14.07, Lorenzo Bianconi wrote: >>>> On 10/05/2021 08.28, Lorenzo Bianconi wrote: >>>>>> On 07/05/2021 13.02, Lorenzo Bianconi wrote: >>>>>>>> The correct way to set the sampling rate to 12.5 Hz before >>>>>>>> commit f8710f0357bc ("iio: imu: st_lsm6dsx: express odr in mHZ") >>>>>>>> was to write 13 Hz to sampling_frequency. >>>>>>>> Before this patch writing 13 to samplerate results in sample rate set >>>>>>>> to 26 Hz. >>>>>>>> Now we return EINVAL if the sampling rate is not in table. >>>>>>>> >>>>>>>> Signed-off-by: Sean Nyekjaer <sean@geanix.com> >>>>>>> >>>>>>> I think this patch is breaking i2c-master support available in st_lsm6dsx, have you tested it? >>>>>>> >>>>>> I have not tested with i2c-master. >>>>>> But it's not nice to round up sample rate from user space without any notice. >>>>> >>>>> the point is accel odr is used as trigger for i2c-slave sampling. Connected >>>>> i2c slave devices have different sample rates (e.g. LIS2MDL), so we need to >>>>> select the lower accel odr greather than slave device one. >>>>> >>>>> Regards, >>>>> Lorenzo >>>>> >>>> >>>> We could open up for custom sample rates if i2c-slaves are enabled? >>> >>> can you please explain what you mean? >>> >> If i2c-slaves are enabled, it's allowed to set off table sample rates.(We roundup samplerate like now) >> If i2c-slaves are disabled, we return -EINVAL if we can't find the chosen samplerate in the samplerate table? > > IMO this is not a big deal because if you set the proper ODR (e.g. 26Hz) with the > i2c-master disabled, the sample rate will not be rounded-up. If you set 20Hz, > the accel ODR will be set to 26Hz. I think it does not worth to make the code > uglier and more complex to maintain. > > Regards, > Lorenzo > Sure, but we had some existing code that was setting the samplerate to 13Hz as that was the way to set 12.5Hz samplerate in the past. Suddenly without any notice 13Hz(of cause the kernel was updated) resulted in a samplerate of 26Hz. :/ /Sean
diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c index a3ab16615729..a14a1df8537b 100644 --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c @@ -1427,7 +1427,7 @@ int st_lsm6dsx_check_odr(struct st_lsm6dsx_sensor *sensor, u32 odr, u8 *val) * ext devices can run at different odr respect to * accel sensor */ - if (odr_table->odr_avl[i].milli_hz >= odr) + if (odr_table->odr_avl[i].milli_hz == odr) break; }
The correct way to set the sampling rate to 12.5 Hz before commit f8710f0357bc ("iio: imu: st_lsm6dsx: express odr in mHZ") was to write 13 Hz to sampling_frequency. Before this patch writing 13 to samplerate results in sample rate set to 26 Hz. Now we return EINVAL if the sampling rate is not in table. Signed-off-by: Sean Nyekjaer <sean@geanix.com> --- drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)