From: Ruslan Zalata <rz@fabmicro.ru>
To: Maxime Ripard <maxime@cerno.tech>
Cc: Guenter Roeck <linux@roeck-us.net>,
Icenowy Zheng <icenowy@aosc.io>, Jean Delvare <jdelvare@suse.com>,
Chen-Yu Tsai <wens@csie.org>,
Jernej Skrabec <jernej.skrabec@gmail.com>,
Samuel Holland <samuel@sholland.org>,
linux-kernel@vger.kernel.org, linux-hwmon@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-sunxi@lists.linux.dev
Subject: Re: [PATCH v2] hwmon: (sun4i-lradc) Add driver for LRADC found on Allwinner A13/A20 SoC
Date: Wed, 04 May 2022 02:37:32 +0500 [thread overview]
Message-ID: <18ded45dcd670edcc9eb9811e7c7c034@fabmicro.ru> (raw)
In-Reply-To: <20220503161446.tl3qoqqnkzq2f3hn@houat>
Hi Maxime,
> At the hardware level, I'd assume you would either use the LRADC as an
> actual ADC, or use it to drive buttons, right?
Yes, exactly.
> So I don't think a new device tree binding is such a deal breaker since
> you have to describe it differently anyway.
...
> Since that would be a completely different use-case, the IIO driver
> doesn't have to support input right away, it can be done later if
> needed.
>
> And you could have the two drivers compiled at the same time.
As I got you right, you propose do add new bindings, say
"allwinner,sun4i-a10-lradc-hwmon" and "allwinner,sun8i-a83t-lradc-hwmon"
for new driver, which will allow two drivers (hwmon and keyboard) be
compiled and loaded at same time, only that one listed in DT will be
instantiated. If two are listed at same time, one of the calls to
devm_request_irq() will return with an error preventing second driver to
be probed (some error message would be necessary to let user know what's
going on). If this is ok, I will implement it.
I think moving this driver to IIO framework is overkill. We use LRADC to
monitor battery temp and state (voltage) and that's what HWMON was made
for. It's simple, easy and elegant. IIO, on the other hand, is for data
acquisition and is much more complex beast. Can we stay with HWMON,
please ? :)
I looked through the code for a number of iio/adc drivers and I could
see that all of them initiate ADC conversion inside read(), then wait
for completion and return single sample. For me this very flawed
approach because a) much more overhead/load on the system, b) initiating
conversion may (and will) take more time than a single consequent
conversion, c) samples will be read in irregular periods of time, hence
acquired data will not be consistent for any further processing (like
FFT). So, this whole IIO framework is no way better than HWMON, yet more
complex. At least for ADCs. :-)
A better approach for an IIO/ADC driver would be to implement some
serialization mechanism to let reads go in sync with updates (IRQs),
with buffering, guaranteeing no same sample is read twice and no sample
is lost. The read() would return next available sample from buffer with
nearly zero overhead or sleep till data is available.
And the best way is to extend IIO framework to support ring buffers
mechanism like the one proposed by Analog Devices, but that's a way
different story. Link:
https://events.static.linuxfound.org/sites/events/files/slides/iio_high_speed.pdf
Regards,
Ruslan.
Fabmicro, LLC.
next prev parent reply other threads:[~2022-05-03 21:38 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20220428210906.29527-1-rz@fabmicro.ru>
2022-04-28 21:30 ` [PATCH v2] hwmon: (sun4i-lradc) Add driver for LRADC found on Allwinner A13/A20 SoC Guenter Roeck
2022-04-29 0:28 ` Ruslan Zalata
2022-04-29 5:32 ` Guenter Roeck
2022-04-29 6:03 ` Guenter Roeck
2022-05-02 23:47 ` Ruslan Zalata
2022-04-29 13:24 ` Jonathan Cameron
2022-05-02 23:20 ` Ruslan Zalata
2022-05-02 11:00 ` Maxime Ripard
2022-05-02 11:15 ` Icenowy Zheng
2022-05-02 11:21 ` Maxime Ripard
2022-05-03 2:02 ` Guenter Roeck
2022-05-03 15:26 ` Ruslan Zalata
2022-05-03 16:14 ` Maxime Ripard
2022-05-03 21:37 ` Ruslan Zalata [this message]
2022-05-04 14:12 ` Maxime Ripard
2022-05-02 13:31 ` Guenter Roeck
2022-05-02 13:39 ` Maxime Ripard
2022-05-02 16:21 ` Guenter Roeck
2022-05-03 8:50 ` Ruslan Zalata
2022-05-03 16:07 ` Maxime Ripard
2022-05-04 19:33 Jamie Cuthbert
2022-05-04 19:33 Jamie Cuthbert
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=18ded45dcd670edcc9eb9811e7c7c034@fabmicro.ru \
--to=rz@fabmicro.ru \
--cc=icenowy@aosc.io \
--cc=jdelvare@suse.com \
--cc=jernej.skrabec@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sunxi@lists.linux.dev \
--cc=linux@roeck-us.net \
--cc=maxime@cerno.tech \
--cc=samuel@sholland.org \
--cc=wens@csie.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).