Re: [PATCH v6 2/2] iio: accel: Add support for the Bosch-Sensortec BMI088

From: Mike Looijmans
Date: Mon Jan 25 2021 - 03:06:56 EST


See below


Met vriendelijke groet / kind regards,

Mike Looijmans
System Expert


TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@xxxxxxxxxxxxxxxxx
W: www.topicproducts.com

Please consider the environment before printing this e-mail
On 24-01-2021 14:23, Jonathan Cameron wrote:
On Sun, 24 Jan 2021 00:21:13 +0100
Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:

On Sat, Jan 23, 2021 at 4:35 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
[Me]
Next, I think it is better to let suspend/resume, i.e. system PM
reuse runtime PM since you're implementing that. This is why
we invented PM runtime force resume and force suspend.
Here the driver is turning more off for full suspend than in the
runtime path. If that results in significant extra delay then
it's not appropriate to have that in the runtime suspend path.
I see the point.

The resume path calls bmi088_accel_enable() which incurs
a 5ms delay.

The runtime resume path incurs a 1 ms delay.

The runtime autosuspend kicks in after 2 ms.

It's set to 2 seconds as I understand it. This to support reading a single value every second or so.


Maybe the simplification of not doing the deeper power saving
mode is worth the extra power cost or extra delay, but
I'm not yet convinced.
I would personally set the autosuspend to ~20ms and just use
one path and take a hit of 5 ms whenever we go down between
measures if it is a system that is for human interaction, but for
control systems this more complex set-up may be better for
response latencies.

The current approach may be better tuned to perfection and
we are all perfectionists :D

I'm just worrying a little about bugs and maintainability.
Fully understood. Though for things like this I like to leave
it at the discretion of the driver author as fairly safe they
are a user of the device.

May well make sense to go with the longer times as you
suggest though! Over to you Mike :)

I've been digging in the datasheet and it's really unclear how you're supposed to control the two power registers.

I think it's best to just put both control values into on/off state at the same time. I also prefer the simplicity of Linus' suggestion. I'll do some testing to see if the device behaves properly.

--
Mike Looijmans