All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iio: imu: st_lsm6dsx: do not roundup set samplerate
@ 2021-05-07 10:32 Sean Nyekjaer
  2021-05-07 11:02 ` Lorenzo Bianconi
  0 siblings, 1 reply; 9+ messages in thread
From: Sean Nyekjaer @ 2021-05-07 10:32 UTC (permalink / raw)
  To: lorenzo.bianconi83, jic23; +Cc: Sean Nyekjaer, linux-iio

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(-)

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


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

* Re: [PATCH] iio: imu: st_lsm6dsx: do not roundup set samplerate
  2021-05-07 10:32 [PATCH] iio: imu: st_lsm6dsx: do not roundup set samplerate Sean Nyekjaer
@ 2021-05-07 11:02 ` Lorenzo Bianconi
  2021-05-10  5:25   ` Sean Nyekjaer
  0 siblings, 1 reply; 9+ messages in thread
From: Lorenzo Bianconi @ 2021-05-07 11:02 UTC (permalink / raw)
  To: Sean Nyekjaer; +Cc: lorenzo.bianconi83, jic23, linux-iio

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

> 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
> 

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

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

* Re: [PATCH] iio: imu: st_lsm6dsx: do not roundup set samplerate
  2021-05-07 11:02 ` Lorenzo Bianconi
@ 2021-05-10  5:25   ` Sean Nyekjaer
  2021-05-10  6:28     ` Lorenzo Bianconi
  0 siblings, 1 reply; 9+ messages in thread
From: Sean Nyekjaer @ 2021-05-10  5:25 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: lorenzo.bianconi83, jic23, linux-iio

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


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

* Re: [PATCH] iio: imu: st_lsm6dsx: do not roundup set samplerate
  2021-05-10  5:25   ` Sean Nyekjaer
@ 2021-05-10  6:28     ` Lorenzo Bianconi
  2021-05-10  6:41       ` Sean Nyekjaer
  0 siblings, 1 reply; 9+ messages in thread
From: Lorenzo Bianconi @ 2021-05-10  6:28 UTC (permalink / raw)
  To: Sean Nyekjaer; +Cc: lorenzo.bianconi83, jic23, linux-iio

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

> 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
> 

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

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

* Re: [PATCH] iio: imu: st_lsm6dsx: do not roundup set samplerate
  2021-05-10  6:28     ` Lorenzo Bianconi
@ 2021-05-10  6:41       ` Sean Nyekjaer
  2021-05-10 12:07         ` Lorenzo Bianconi
  0 siblings, 1 reply; 9+ messages in thread
From: Sean Nyekjaer @ 2021-05-10  6:41 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: lorenzo.bianconi83, jic23, linux-iio

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

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

* Re: [PATCH] iio: imu: st_lsm6dsx: do not roundup set samplerate
  2021-05-10  6:41       ` Sean Nyekjaer
@ 2021-05-10 12:07         ` Lorenzo Bianconi
  2021-05-10 12:14           ` Sean Nyekjaer
  0 siblings, 1 reply; 9+ messages in thread
From: Lorenzo Bianconi @ 2021-05-10 12:07 UTC (permalink / raw)
  To: Sean Nyekjaer; +Cc: lorenzo.bianconi83, jic23, linux-iio

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

> 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

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

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

* Re: [PATCH] iio: imu: st_lsm6dsx: do not roundup set samplerate
  2021-05-10 12:07         ` Lorenzo Bianconi
@ 2021-05-10 12:14           ` Sean Nyekjaer
  2021-05-10 14:47             ` Lorenzo Bianconi
  0 siblings, 1 reply; 9+ messages in thread
From: Sean Nyekjaer @ 2021-05-10 12:14 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: lorenzo.bianconi83, jic23, linux-iio

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

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

* Re: [PATCH] iio: imu: st_lsm6dsx: do not roundup set samplerate
  2021-05-10 12:14           ` Sean Nyekjaer
@ 2021-05-10 14:47             ` Lorenzo Bianconi
  2021-05-10 16:52               ` Sean Nyekjaer
  0 siblings, 1 reply; 9+ messages in thread
From: Lorenzo Bianconi @ 2021-05-10 14:47 UTC (permalink / raw)
  To: Sean Nyekjaer; +Cc: lorenzo.bianconi83, jic23, linux-iio

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

> 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

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

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

* Re: [PATCH] iio: imu: st_lsm6dsx: do not roundup set samplerate
  2021-05-10 14:47             ` Lorenzo Bianconi
@ 2021-05-10 16:52               ` Sean Nyekjaer
  0 siblings, 0 replies; 9+ messages in thread
From: Sean Nyekjaer @ 2021-05-10 16:52 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: lorenzo.bianconi83, jic23, linux-iio

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

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

end of thread, other threads:[~2021-05-10 16:52 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-07 10:32 [PATCH] iio: imu: st_lsm6dsx: do not roundup set samplerate Sean Nyekjaer
2021-05-07 11:02 ` Lorenzo Bianconi
2021-05-10  5:25   ` Sean Nyekjaer
2021-05-10  6:28     ` Lorenzo Bianconi
2021-05-10  6:41       ` Sean Nyekjaer
2021-05-10 12:07         ` Lorenzo Bianconi
2021-05-10 12:14           ` Sean Nyekjaer
2021-05-10 14:47             ` Lorenzo Bianconi
2021-05-10 16:52               ` Sean Nyekjaer

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.