[RESEND,0/2] Solve data access delay of ST sensors

Message

TOSCANELLI Massimo Feb. 7, 2022, 9:04 a.m. UTC

Reading raw data from ST sensors implies enabling the device and
waiting for its activation.
This leads to significant delays every time the data is fetched,
especially if the operation has to be repeated.

The introduction of the 'always_on' flag as sysfs attribute can
solve this issue, by allowing the user to keep the device enabled
during the entire read session and therefore, to perform a much
faster data access.

This implementation has been already tested on a ST magnetometer.

I forgot to add maintainers as recepients, so I resend the patches.

Massimo Toscanelli (2):
  iio: st_sensors: add always_on flag
  iio: st_magn_core.c: activate always_on attribute

 .../iio/common/st_sensors/st_sensors_core.c   | 85 +++++++++++++++++--
 drivers/iio/magnetometer/st_magn_core.c       |  2 +
 include/linux/iio/common/st_sensors.h         | 14 +++
 3 files changed, 96 insertions(+), 5 deletions(-)


base-commit: dcb85f85fa6f142aae1fe86f399d4503d49f2b60

Comments

Jonathan Cameron Feb. 7, 2022, 1:58 p.m. UTC | #1

On Mon,  7 Feb 2022 09:04:41 +0000
Massimo Toscanelli <[e-mail]> wrote:

> Reading raw data from ST sensors implies enabling the device and
> waiting for its activation.
> This leads to significant delays every time the data is fetched,
> especially if the operation has to be repeated.
> 
> The introduction of the 'always_on' flag as sysfs attribute can
> solve this issue, by allowing the user to keep the device enabled
> during the entire read session and therefore, to perform a much
> faster data access.
> 
> This implementation has been already tested on a ST magnetometer.
> 
> I forgot to add maintainers as recepients, so I resend the patches.
> 
> Massimo Toscanelli (2):
>   iio: st_sensors: add always_on flag
>   iio: st_magn_core.c: activate always_on attribute
> 
>  .../iio/common/st_sensors/st_sensors_core.c   | 85 +++++++++++++++++--
>  drivers/iio/magnetometer/st_magn_core.c       |  2 +
>  include/linux/iio/common/st_sensors.h         | 14 +++
>  3 files changed, 96 insertions(+), 5 deletions(-)
> 
> 
> base-commit: dcb85f85fa6f142aae1fe86f399d4503d49f2b60

Hi Massimo,

The standard approach to avoiding rapid power up and power down cycles
is to use runtime_pm with autosuspend and a period set at a period
of perhaps 1 second.  Would that work for you?  You'll pay the costs
of power up only on the first read after a period of not reading.

Exposing the control to userspace for this sort of thing is normally
a bad idea because userspace generally has no idea if it should use it
as there is no clean way of telling userspace the costs of not using
it (as those will be device specific).

If you have a usecase that requires regular reading you should also
think about whether using the buffered interface is more appropriate.
IIRC that will keep these devices powered up continuously whilst
the buffer is enabled and will provide a lower overhead way of
reading data than sysfs reads.

Jonathan

Linus Walleij Feb. 11, 2022, 1:12 a.m. UTC | #2

On Mon, Feb 7, 2022 at 2:59 PM Jonathan Cameron
<[e-mail]> wrote:
> On Mon,  7 Feb 2022 09:04:41 +0000
> Massimo Toscanelli <[e-mail]> wrote:

> The standard approach to avoiding rapid power up and power down cycles
> is to use runtime_pm with autosuspend and a period set at a period
> of perhaps 1 second.  Would that work for you?  You'll pay the costs
> of power up only on the first read after a period of not reading.
>
> Exposing the control to userspace for this sort of thing is normally
> a bad idea because userspace generally has no idea if it should use it
> as there is no clean way of telling userspace the costs of not using
> it (as those will be device specific).
>
> If you have a usecase that requires regular reading you should also
> think about whether using the buffered interface is more appropriate.
> IIRC that will keep these devices powered up continuously whilst
> the buffer is enabled and will provide a lower overhead way of
> reading data than sysfs reads.

I see that I have repeted similar comments.

Sorry for the hammering.

I agree with Jonathan's stance here.

Yours,
Linus Walleij

TOSCANELLI Massimo Feb. 11, 2022, 3:14 p.m. UTC | #3

Hello Linus and Jonathan

> -----Original Message-----
> From: Linus Walleij <[e-mail]>
> Sent: 11 February 2022 02:12
> To: Jonathan Cameron <[e-mail]>
> Cc: TOSCANELLI Massimo <[e-mail]>;
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail];
> [e-mail]; [e-mail]; LI Qingwu <qing-
> [e-mail]>; [e-mail]; linux-
> [e-mail]; GEO-CHHER-bsp-development <bsp-
> [e-mail]>
> Subject: Re: [PATCH RESEND 0/2] Solve data access delay of ST sensors
> 
> 
> On Mon, Feb 7, 2022 at 2:59 PM Jonathan Cameron
> <[e-mail]> wrote:
> > On Mon,  7 Feb 2022 09:04:41 +0000
> > Massimo Toscanelli <[e-mail]> wrote:
> 
> > The standard approach to avoiding rapid power up and power down cycles
> > is to use runtime_pm with autosuspend and a period set at a period of
> > perhaps 1 second.  Would that work for you?  You'll pay the costs of
> > power up only on the first read after a period of not reading.
> >
> > Exposing the control to userspace for this sort of thing is normally a
> > bad idea because userspace generally has no idea if it should use it
> > as there is no clean way of telling userspace the costs of not using
> > it (as those will be device specific).
> >
> > If you have a usecase that requires regular reading you should also
> > think about whether using the buffered interface is more appropriate.
> > IIRC that will keep these devices powered up continuously whilst the
> > buffer is enabled and will provide a lower overhead way of reading
> > data than sysfs reads.
> 
> I see that I have repeted similar comments.
> 
Thanks a lot for your suggestions, I decided to go for the second solution proposed by Jonathan.
Using the buffered interface should be enough, you can discard the patches.

Massimo