On Tue, Oct 18, 2022 at 02:27:20PM +0300, Matti Vaittinen wrote: > 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! This is very good news, Matti, thanks for pushing this forward and finding better solution! -- With Best Regards, Andy Shevchenko
