Hi David!
Thanks for the input!
On 16/02/2025 19:50, David Lechner wrote:
On 2/11/25 1:07 PM, Jonathan Cameron wrote:
On Tue, 11 Feb 2025 10:52:51 +0200
Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:
Hi Jonathan,
Thanks for the review and all the comments!
Just a note that I am currently spending some quality time with
rebuilding the floor of my house. Leaking floor drain can cause a bit of
a work... :rolleyes: So, my time with upstream work is a bit limited -
although writing an occasional bug or two can help one to keep the
balance ;)
Anyways, my replies and new versions may be slower than usual..
On 08/02/2025 18:41, Jonathan Cameron wrote:
On Wed, 5 Feb 2025 15:34:51 +0200
Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:
There are ADC ICs which may have some of the AIN pins usable for other
functions. These ICs may have some of the AIN pins wired so that they
should not be used for ADC.
(Preferred?) way for marking pins which can be used as ADC inputs is to
add corresponding channels@N nodes in the device tree as described in
the ADC binding yaml.
Add couple of helper functions which can be used to retrieve the channel
information from the device node.
Signed-off-by: Matti Vaittinen <mazziesaccount@xxxxxxxxx>
---
Revision history:
RFC v1 => v2:
- New patch
I think it might be nice to have helpers for fetching also the other
generic (non vendor specific) ADC properties (as listed in the
Documentation/devicetree/bindings/iio/adc/adc.yaml) - but as I don't
have use for those in BD79124 driver (at least not for now), I don't
imnplement them yet. Anyways, this commit creates a place for such
helpers.
There is often a mix of vendor specific and not in channel nodes.
Hence I'm not sure how widely this will be and it is driver
specific which of the standard things make sense.
I definitely agree. Still, in my experience, no written standard
standardizes use as well as written helpers ;) More we support parsing
the generic helpers by the (add subsystem here)-core, more the driver
writes will use the generic properties (instead of brewing vendor
specific ones).
So before I'd consider a helper like this I'd want to see it alongside
a bunch of users including some of the complex ones so that we know
it generalizes well enough. It doesn't make sense to introduce
it otherwise - just keep the code in the specific drivers instead.
It's an interesting idea, but not a trivial one :)
I agree it's not trivial. But I believe adding helpers one-by-one to
cover 'normal' use-cases guide the use of the properties. Those who need
something more exotic can always implement their custom handlers - and
then a reviewer of such handler can ask "why" ;)
I'd be fine with a series taking on the task of converting handling of
all the documented properties in adc.yaml
If we do less than that it may never get wide adoption and we end
up with a bit of generic looking infrastructure that isn't generic.
Having reviewed quite a few patches recently that make use of these
generic channel properties (and writing one driver myself), I don't
really see how we could make generic functions for these that would
be any simpler than calling the fwnode_property functions directly
other than maybe saving a few arguments.
I started this for the BD79124 - which is very simple ADC. What it
requires from the devicetree is the channel ID. What needs to be done in
oreder to get it?
1. Locate the ADC node (device-node)
2. Loop through the sub-nodes and identify subnodes that denote
channels.
3. Get the channel number (in this case from the reg-property).
I assume this is very usual need for the simple ADC drivers, and none of
this needs to be hardware specific (when we have simple input channels
(no need for the differential channel support).
Providing a simple helper doing just this should remove code duplication
from drivers. Furthermore, this could help standardize the mechanism for
identifying the channel nodes - in my patch this would require channel
node to be named channel@<N>. (I see some drivers which don't require
this but just assume any sub-node is for channel information. This works
badly with chips which may provide multiple functions and thus I think
it'd be nice to guide towards naming the nodes to channel@N when possible).
The "normal" operation for
these properties usually involves poking some registers on the chip
(could be immediately or deferred) to configure it. So the only thing
we could generalize is reading the property value, but not doing
anything with that information.
Absolutely. I had no intention to do anything hardware specific with
these helpers. What I'd love to see is IIO-helpers which allowed passing
device pointer and a pointer to iio_info - and getting the (known by the
helper) device-tree properties parsed and filled in iio_info.
This sure does not help more complex drivers which require some 'not
easily parsed' data from the device tree. But for simple drivers like
the BD79124 this would allow drivers to omit open-coding the loop
locating the channel nodes and getting the channel data.
Maybe a small improvement but one can't deny it still is an improvement,
right? :)
Yours,
-- Matti
