Re: [RFC PATCH v2 4/5] iio: accel: Support Kionix/ROHM KX022A accelerometer

From: Matti Vaittinen
Date: Tue Oct 18 2022 - 07:27:29 EST

On 10/14/22 16:22, Jonathan Cameron wrote:
On Tue, 11 Oct 2022 09:10:21 +0000
"Vaittinen, Matti" <Matti.Vaittinen@xxxxxxxxxxxxxxxxx> wrote:

On 10/10/22 09:15, Andy Shevchenko wrote:
On Sun, Oct 09, 2022 at 01:33:51PM +0100, Jonathan Cameron wrote:
On Thu, 6 Oct 2022 21:32:11 +0300 Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
On Thu, Oct 06, 2022 at 05:38:14PM +0300, Matti Vaittinen wrote:

...
+module_param(g_kx022a_use_buffer, bool, 0);
+MODULE_PARM_DESC(g_kx022a_use_buffer, + "Buffer samples. Use
at own risk. Fifo must not overflow");
// snip

This would be a way to have the FIFO disabled by default and warn users
via dt-binding docs if they decide to explicitly enable the FIFO.
(Besides, I believe the FIFO is usable on at least some of the Kionix
sensors - because I've heard it is used on a few platforms).

This could give us safe defaults while not shutting the doors from those
who wish to use the FIFO. Sure we need a buy-in from Krzysztof / Rob,
but that may be less of an obstacle compared to the module param if Greg
is so strongly oppsoing those. (And the dt-property could also provide
some technical merites as these sensors seem to really have differencies
in FIFOs).

I'm dubious about having this for known broken parts - but I guess you
can propose it and see what the dt-maintainers say.

I don't want to see fifo size in the dt binding though.

// snip

Also it needs some communication
with a vendor to clarify the things.

I do communication with the vendor on a daily basis :] Nowadays Kionix
is part of ROHM, and Finland SWDC has been collaboration with Kionix
even before they "merged" (but I have not been part of the "sensor team"
back then).

Unfortunately, reaching the correct people inside the company is hard
and occasionally impossible - long story...

:)

Just a note. I have done some further investigation (further testing combined with tracing the SPI lines) and also got some answer(s) from the ASIC designers. First thing is that the underlying FIFO is 258 bytes and can hold up-to 43 HiRes samples. This is also fixed in the recent data-sheet. The register informing how many bytes of data is stored in FIFO is still only 8bits. When FIFO is full, the magic value 255 is used to indicate the 258 bytes of data. This explains the strange 42,5 samples (255 bytes) of data being reported.

Furthermore, I've noticed that the FIFO appearing to be "stuck", eg. amount of data stored in FIFO not decreasing when data is read is 100% reproducible. The condition is that the PC1 bit (accelerometer state control) is toggled before the FIFO is read. The issue does not seem to be reproducible if the PC1 is not touched. I have also asked if this is a known limitation.

Anyways, it seems we have a solution to the problem. We simply handle the case when 255 bytes is reported correctly (by reading 258 bytes of valid data) - and we prevent changing the sensor configuration when FIFO is enabled.

I will implement these changes to the v3 and drop both the module-param and the dt-property. Sorry for the hassle and thanks for the patience/support!

Yours
-- Matti

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

Attachment: OpenPGP_0x40497F0C4693EF47.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature