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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y06fHg6JdSA2GbII@smile.fi.intel.com>
Date:   Tue, 18 Oct 2022 15:42:06 +0300
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Matti Vaittinen <mazziesaccount@...il.com>
Cc:     Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        "Vaittinen, Matti" <Matti.Vaittinen@...rohmeurope.com>,
        Jonathan Cameron <jic23@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Jagath Jog J <jagathjog1996@...il.com>,
        Nikita Yushchenko <nikita.yoush@...entembedded.com>,
        Cosmin Tanislav <demonsingur@...il.com>,
        "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH v2 4/5] iio: accel: Support Kionix/ROHM KX022A
 accelerometer

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@...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@...ux.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!

This is very good news, Matti, thanks for pushing this forward and finding
better solution!

-- 
With Best Regards,
Andy Shevchenko