[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20221014142232.000038df@huawei.com>
Date: Fri, 14 Oct 2022 14:22:32 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: "Vaittinen, Matti" <Matti.Vaittinen@...rohmeurope.com>
CC: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Jonathan Cameron <jic23@...nel.org>,
Matti Vaittinen <mazziesaccount@...il.com>,
"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, 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");
> >>>
> >>> Why?! We usually do not allow module parameters in the new code.
> >>
> >> Badly broken hardware - was my suggestion. Alternatives if there
> >> are usecases that need to use the fifo, but it can wedge hard in a
> >> fashion that is impossible to prevent from the driver? My gut
> >> feeling is still drop the support entirely with a strong comment in
> >> the code that the hardware is broken in a fashion we don't know how
> >> to work around.
>
> I did some quick study regarding couple of other Kionix sensors. (like
> KX122 and old KX022 - without the 'A'). It seems to me that the register
> interfaces between many of the sensors are largely identical. Extending
> the driver to support those seems pretty straightforward (scales and
> resolution may need tweaking, as does the FIFO size) but register
> contents and even offsets are largely identical.
Last kionix part I had was a kxsd9 and I don't recall that having a fifo
so obviously didn't hit this issue.
>
> As said, it seems the Kionix sensors may have different size of internal
> FIFOs, or even no FIFO at all. So, maybe we could provide a
> "kionix,fifo-enable" flag (or even "kionix,fifo-size") from device-tree?
For device where we don't have reports of this issue, that should be derived
from the compatible (or even better a whoami register if there is one).
The driver should know the fifo-size if it isn't discoverable.
> 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.
>
> >
> > I also would drop this from upstream and if anybody curious, provide
> > some kind of GitHub gist for that.
>
> Well, I think we all agree that downstream code hosted in some
> unofficial github repositories are rarely that valuable. They're less
> reliable, less tested, less reviewed, less secure and pretty much
> impossible to maintain in a way that interested user could get a version
> matching his preferred kernel.
>
> There are reasons why I (people) keep sending the drivers to upstream -
> and why some companies even spend $$ for that. Having this feature in
> downstream repo is not nearly on same page for user's point of view as
> is having the support upstream.
It's not really support if it comes with big warnings and potentially we
even taint the kernel of someone turns it on...
>
> > 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...
:)
Jonathan
Powered by blists - more mailing lists