lkml.org 
[lkml]   [2022]   [Oct]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH v2 4/5] iio: accel: Support Kionix/ROHM KX022A accelerometer
On 10/14/22 16:22, Jonathan Cameron wrote:
> On Tue, 11 Oct 2022 09:10:21 +0000
> "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com> 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@linux.intel.com> 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! ~~

[unhandled content-type:application/pgp-keys][unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2022-10-18 13:28    [W:0.239 / U:0.632 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site