Message-ID: <20250930182649.GA3340740-robh@kernel.org>
Date: Tue, 30 Sep 2025 13:26:49 -0500
From: Rob Herring <robh@...nel.org>
To: David Lechner <dlechner@...libre.com>
Cc: Jonathan Cameron <jic23@...nel.org>,
	Marcelo Schmitt <marcelo.schmitt@...log.com>,
	linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
	linux-doc@...r.kernel.org, linux-spi@...r.kernel.org,
	linux-kernel@...r.kernel.org, michael.hennerich@...log.com,
	nuno.sa@...log.com, eblanc@...libre.com, andy@...nel.org,
	krzk+dt@...nel.org, conor+dt@...nel.org, corbet@....net,
	marcelo.schmitt1@...il.com,
	Linus Walleij <linus.walleij@...aro.org>,
	Bartosz Golaszewski <brgl@...ev.pl>, linux-gpio@...r.kernel.org
Subject: Re: [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216
 and ADAQ4224

On Mon, Sep 29, 2025 at 06:16:10PM +0200, David Lechner wrote:
> On Mon, Sep 29, 2025 at 4:31 PM Rob Herring <robh@...nel.org> wrote:
> >
> > On Sun, Sep 28, 2025 at 11:19:55AM +0100, Jonathan Cameron wrote:
> > > On Fri, 26 Sep 2025 17:40:47 -0300
> > > Marcelo Schmitt <marcelo.schmitt@...log.com> wrote:
> > >
> > > > ADAQ4216 and ADAQ4224 are similar to AD4030 except that ADAQ devices have a
> > > > PGA (programmable gain amplifier) that scales the input signal prior to it
> > > > reaching the ADC inputs. The PGA is controlled through a couple of pins (A0
> > > > and A1) that set one of four possible signal gain configurations.
> > > >
> > > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@...log.com>
> > > > ---
> > > > Change log v2 -> v3
> > > > - PGA gain now described in decibels.
> > > >
> > > > The PGA gain is not going to fit well as a channel property because it may
> > > > affect more than one channel as in AD7191.
> > > > https://www.analog.com/media/en/technical-documentation/data-sheets/AD7191.pdf
> > > >
> > > > I consulted a very trustworthy source [1, 2] and learned that describing signal
> > > > gains in decibels is a common practice. I now think it would be ideal to describe
> > > > these PGA and PGA-like gains with properties in decibel units and this patch
> > > > is an attempt of doing so. The only problem with this approach is that we end up
> > > > with negative values when the gain is lower than 1 (the signal is attenuated)
> > > > and device tree specification doesn't support signed integer types. As the
> > > > docs being proposed fail dt_binding_check, I guess I have to nack the patch myself.
> > > > Any chance of dt specification eventually support signed integers?
> > > > Any suggestions appreciated.
> > > >
> > > > [1] https://en.wikipedia.org/wiki/Decibel
> > > > [2] https://en.wikipedia.org/wiki/Gain_(electronics)
> > >
> > > I still wonder if the better way to describe this is to ignore that it
> > > has anything to do with PGA as such and instead describe the pin strapping.
> > >
> > > DT folk, is there an existing way to do that? My grep skills are failing to
> > > spot one.
> > >
> > > We've papered over this for a long time in various IIO drivers by controlling
> > > directly what the pin strap controls with weird and wonderful device specific
> > > bindings. I wonder if we can't have a gpio driver + binding that rejects all
> > > config and just lets us check the current state of an output pin.  Kind of a
> > > fixed mode regulator equivalent for gpios.
> >
> > If these are connected to GPIOs, isn't it possible that someone will
> > want to change their value?
> >
> > Other than some generic 'pinstrap-gpios' property, I don't see what we'd
> > do here? I don't feel like pin strapping GPIOs is something that we see
> > all that often.
> >
> > Rob
> 
> I think the idea is that it is not actually a GPIO, just a hard-wired
> connection. We would want to have a "fixed-gpios" to describe these
> hard-wired connections as GPIOs so that we don't have to write complex
> binding for chip config GPIOs. I've seen configuration pins like on at
> least half a dozed of the ADCs I've been working on/reviewing over the
> last two years (since I got involved in IIO again).

Until I read the example, I totally missed what you want here...

Can you point me to some existing bindings?

IIRC, Linus has expressed not caring for cases of using GPIO API on 
things that are not GPIOs. That was more like registers which can 
read the state of signals. Better let him weigh in before we go too far 
down this path.

> 
> For example, there might be 4 mode pins, so we would like to just have
> a mode-gpios property. So this could be all 4 connected to GPIOs, all
> 4 hard-wired, or a mix.
> 
> (The actual bindings would need more thought, but this should give the
> general idea)
> 
> fixed_gpio: hard-wires {
>     compatible = "fixed-gpios";
>     gpio-controller;
>     #gpio-cells = <1>;
> };
> 
> gpio0: gpio-controller@...0000 {
>     compatible = "vendor,soc-gpios";
>     gpio-controller;
>     #gpio-cells = <2>;
> };
> 
> spi {
>     adc@0 {
>         compatible = "vendor,adc";
>         /* All gpios */
>         mode-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>,
>                      <&gpio0 1 GPIO_ACTIVE_HIGH>,
>                      <&gpio0 2 GPIO_ACTIVE_HIGH>,
>                      <&gpio0 3 GPIO_ACTIVE_HIGH>;
>          /* or all hard-wired */
>         mode-gpios = <&fixed_gpio 0 GPIO_FIXED_HIGH>,
>                      <&fixed_gpio GPIO_FIXED_HIGH>,
>                      <&fixed_gpio GPIO_FIXED_LOW>,
>                      <&fixed_gpio GPIO_FIXED_LOW>;
>          /* or mixed */
>         mode-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>,
>                      <&gpio0 1 GPIO_ACTIVE_HIGH>,
>                      <&fixed_gpio GPIO_FIXED_LOW>,
>                      <&fixed_gpio GPIO_FIXED_LOW>;

The above seems reasonable to me.

Just to throw out an alternative, phandle values of 0 and -1 are 
generally reserved. Historically that means just skip the entry. 
However, you could use that and do something like this:

mode-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>,
             <&gpio0 1 GPIO_ACTIVE_HIGH>,
             <0>,
             <0xffffffff>;

So 0 means low and ~0 means high. The only advantage I see with it is 
you don't need a "fixed-gpios" driver. Also, I'm not sure how that would 
work with requesting GPIOs given you've essentially defined only 2 GPIO 
lines (high and low). Though Bartosz is doing some work on non-exclusive 
GPIOs.

Rob

Powered by blists - more mailing lists