* Re: [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224 [not found] ` <5dc08b622dac1db561f26034c93910ccff75e965.1758916484.git.marcelo.schmitt@analog.com> @ 2025-09-28 10:19 ` Jonathan Cameron 2025-09-29 14:31 ` Rob Herring 0 siblings, 1 reply; 7+ messages in thread From: Jonathan Cameron @ 2025-09-28 10:19 UTC (permalink / raw) To: Marcelo Schmitt Cc: linux-iio, devicetree, linux-doc, linux-spi, linux-kernel, michael.hennerich, nuno.sa, eblanc, dlechner, andy, robh, krzk+dt, conor+dt, corbet, marcelo.schmitt1, Linus Walleij, Bartosz Golaszewski, linux-gpio On Fri, 26 Sep 2025 17:40:47 -0300 Marcelo Schmitt <marcelo.schmitt@analog.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@analog.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. +CC Linus, Bartosz and gpio list. > > Thanks, > Marcelo > > .../bindings/iio/adc/adi,ad4030.yaml | 84 +++++++++++++++++-- > 1 file changed, 79 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml > index 564b6f67a96e..20462fa6c39d 100644 > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4030.yaml > @@ -19,6 +19,8 @@ description: | > * https://www.analog.com/media/en/technical-documentation/data-sheets/ad4030-24-4032-24.pdf > * https://www.analog.com/media/en/technical-documentation/data-sheets/ad4630-24_ad4632-24.pdf > * https://www.analog.com/media/en/technical-documentation/data-sheets/ad4630-16-4632-16.pdf > + * https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4216.pdf > + * https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4224.pdf > > $ref: /schemas/spi/spi-peripheral-props.yaml# > > @@ -31,6 +33,8 @@ properties: > - adi,ad4630-24 > - adi,ad4632-16 > - adi,ad4632-24 > + - adi,adaq4216 > + - adi,adaq4224 > > reg: > maxItems: 1 > @@ -54,6 +58,14 @@ properties: > description: > Internal buffered Reference. Used when ref-supply is not connected. > > + vddh-supply: > + description: > + PGIA Positive Power Supply. > + > + vdd-fda-supply: > + description: > + FDA Positive Power Supply. > + > cnv-gpios: > description: > The Convert Input (CNV). It initiates the sampling conversions. > @@ -64,6 +76,26 @@ properties: > The Reset Input (/RST). Used for asynchronous device reset. > maxItems: 1 > > + pga-gpios: > + description: > + A0 and A1 pins for gain selection. For devices that have PGA configuration > + input pins, pga-gpios should be defined if adi,gain-milli is absent. > + minItems: 2 > + maxItems: 2 > + > + adi,pga-gain-db: > + description: | > + Should be present if PGA control inputs are pin-strapped. The values > + specify the rounded decibel gain calculated from the voltage gain. > + Possible values: > + -10 (A1=0, A0=0), (1/3 V/V gain) > + -5 (A1=0, A0=1), (5/9 V/V gain) > + 7 (A1=1, A0=0), (20/9 V/V gain) > + 16 (A1=1, A0=1), (20/3 V/V gain) > + If defined, pga-gpios must be absent. > + enum: [-10, -5, 7, 16] > + default: -10 > + > pwms: > description: PWM signal connected to the CNV pin. > maxItems: 1 > @@ -86,11 +118,33 @@ required: > - vio-supply > - cnv-gpios > > -oneOf: > - - required: > - - ref-supply > - - required: > - - refin-supply > +allOf: > + - oneOf: > + - required: > + - ref-supply > + - required: > + - refin-supply > + # ADAQ devices require a gain property to indicate how hardware PGA is set > + - if: > + properties: > + compatible: > + contains: > + pattern: ^adi,adaq > + then: > + allOf: > + - required: [vddh-supply, vdd-fda-supply] > + properties: > + ref-supply: false > + - oneOf: > + - required: > + - adi,pga-value > + - required: > + - pga-gpios > + else: > + properties: > + adi,pga-value: false > + pga-gpios: false > + > > unevaluatedProperties: false > > @@ -114,3 +168,23 @@ examples: > reset-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>; > }; > }; > + - | > + #include <dt-bindings/gpio/gpio.h> > + spi { > + #address-cells = <1>; > + #size-cells = <0>; > + > + adc@0 { > + compatible = "adi,adaq4216"; > + reg = <0>; > + spi-max-frequency = <80000000>; > + vdd-5v-supply = <&supply_5V>; > + vdd-1v8-supply = <&supply_1_8V>; > + vio-supply = <&supply_1_8V>; > + ref-supply = <&supply_5V>; > + cnv-gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>; > + reset-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>; > + adi,pga-gain-db = <-5>; > + }; > + }; > +... ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224 2025-09-28 10:19 ` [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224 Jonathan Cameron @ 2025-09-29 14:31 ` Rob Herring 2025-09-29 16:16 ` David Lechner 0 siblings, 1 reply; 7+ messages in thread From: Rob Herring @ 2025-09-29 14:31 UTC (permalink / raw) To: Jonathan Cameron Cc: Marcelo Schmitt, linux-iio, devicetree, linux-doc, linux-spi, linux-kernel, michael.hennerich, nuno.sa, eblanc, dlechner, andy, krzk+dt, conor+dt, corbet, marcelo.schmitt1, Linus Walleij, Bartosz Golaszewski, linux-gpio 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@analog.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@analog.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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224 2025-09-29 14:31 ` Rob Herring @ 2025-09-29 16:16 ` David Lechner 2025-09-30 14:47 ` Marcelo Schmitt 2025-09-30 18:26 ` Rob Herring 0 siblings, 2 replies; 7+ messages in thread From: David Lechner @ 2025-09-29 16:16 UTC (permalink / raw) To: Rob Herring Cc: Jonathan Cameron, Marcelo Schmitt, linux-iio, devicetree, linux-doc, linux-spi, linux-kernel, michael.hennerich, nuno.sa, eblanc, andy, krzk+dt, conor+dt, corbet, marcelo.schmitt1, Linus Walleij, Bartosz Golaszewski, linux-gpio On Mon, Sep 29, 2025 at 4:31 PM Rob Herring <robh@kernel.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@analog.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@analog.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). 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@4000000 { 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>; }; }; ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224 2025-09-29 16:16 ` David Lechner @ 2025-09-30 14:47 ` Marcelo Schmitt 2025-09-30 17:02 ` Marcelo Schmitt 2025-09-30 18:26 ` Rob Herring 1 sibling, 1 reply; 7+ messages in thread From: Marcelo Schmitt @ 2025-09-30 14:47 UTC (permalink / raw) To: David Lechner Cc: Rob Herring, Jonathan Cameron, Marcelo Schmitt, linux-iio, devicetree, linux-doc, linux-spi, linux-kernel, michael.hennerich, nuno.sa, eblanc, andy, krzk+dt, conor+dt, corbet, Linus Walleij, Bartosz Golaszewski, linux-gpio On 09/29, David Lechner wrote: > On Mon, Sep 29, 2025 at 4:31 PM Rob Herring <robh@kernel.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@analog.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@analog.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). Yes, the alternative to having GPIOs would be to have pins hard-wired set to a specific logic level. And the connection don't need to be to GPIOs. The gain pins on the ADC chip can be connected to anything that keeps a constant logic level while we capture data from the ADC. > > 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. Having some pins hard-wired and some connected to GPIOs is possible, but that would make things even more complex as each pin on the ADC chip sets a different portion of the gain. IMHO, mixed GPIO/hard-wired configuration starts looking like over engineering and I haven't been requested for so much configuration flexibility. Having either all hard-wired or all connected to GPIOs should be ok. I'm not familiar with pinctrl dt-bindings, but I was wondering if we could get to something similar with pinctrl. Based on some pinctrl bindings, I think fixed-level GPIOs could look like the following (for the 4 pin-mode example). pinctrl0: pincontroller@0 { compatible = "vendor,model-pinctrl"; all-low-state: some-gpio-pins { pins = "gpio0", "gpio1", "gpio2", "gpio3"; function = "gpio"; output-low; }; all-high-state: some-gpio-pins { pins = "gpio0", "gpio1", "gpio2", "gpio3"; function = "gpio"; output-high; }; most-high-state: some-gpio-pins { pins1 { pins = "gpio0", "gpio1", "gpio2"; function = "gpio"; output-high; }; pins2 { pins = "gpio3"; function = "gpio"; output-low; }; }; }; spi { adc@0 { compatible = "vendor,adc"; /* All gpios */ pga-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 */ pinctrl-names = "minimum-gain", "moderate-gain", "maximum-gain"; pinctrl-0 = <&all-low-state>, <&most-high-state>, <&all-high-state>; }; }; Though, the above is still relying on GPIOs which is not a requirement from ADC peripheral perspective. Also, if GPIOs are available, one can just provide them through pga-gpios and have full control over the signal gain with the IIO driver. It boils down to just telling software what are the logical levels at two pins on the ADC chip when GPIOs are not provided. Thanks, Marcelo > > (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@4000000 { > 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>; > }; > }; ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224 2025-09-30 14:47 ` Marcelo Schmitt @ 2025-09-30 17:02 ` Marcelo Schmitt 0 siblings, 0 replies; 7+ messages in thread From: Marcelo Schmitt @ 2025-09-30 17:02 UTC (permalink / raw) To: David Lechner Cc: Rob Herring, Jonathan Cameron, Marcelo Schmitt, linux-iio, devicetree, linux-doc, linux-spi, linux-kernel, michael.hennerich, nuno.sa, eblanc, andy, krzk+dt, conor+dt, corbet, Linus Walleij, Bartosz Golaszewski, linux-gpio ... > > > > > 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@analog.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) > > > > ... > > Though, the above is still relying on GPIOs which is not a requirement from > ADC peripheral perspective. Also, if GPIOs are available, one can just provide > them through pga-gpios and have full control over the signal gain with the IIO > driver. It boils down to just telling software what are the logical levels at > two pins on the ADC chip when GPIOs are not provided. > Though, as mentioned, the state of A0 and A1 pins defines a certain gain applied to ADC input signal. Because signal gains seem to be usually described in decibels, the proposed dt-binding allows to provide the gain value in decibels and then software figures out what A0 and A1 logical levels are from the provided decibels. The actual levels of A0 and A1 then have to be set according to the provided decibel gain. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224 2025-09-29 16:16 ` David Lechner 2025-09-30 14:47 ` Marcelo Schmitt @ 2025-09-30 18:26 ` Rob Herring 2025-10-01 11:55 ` David Lechner 1 sibling, 1 reply; 7+ messages in thread From: Rob Herring @ 2025-09-30 18:26 UTC (permalink / raw) To: David Lechner Cc: Jonathan Cameron, Marcelo Schmitt, linux-iio, devicetree, linux-doc, linux-spi, linux-kernel, michael.hennerich, nuno.sa, eblanc, andy, krzk+dt, conor+dt, corbet, marcelo.schmitt1, Linus Walleij, Bartosz Golaszewski, linux-gpio 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@kernel.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@analog.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@analog.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@4000000 { > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224 2025-09-30 18:26 ` Rob Herring @ 2025-10-01 11:55 ` David Lechner 0 siblings, 0 replies; 7+ messages in thread From: David Lechner @ 2025-10-01 11:55 UTC (permalink / raw) To: Rob Herring Cc: Jonathan Cameron, Marcelo Schmitt, linux-iio, devicetree, linux-doc, linux-spi, linux-kernel, michael.hennerich, nuno.sa, eblanc, andy, krzk+dt, conor+dt, corbet, marcelo.schmitt1, Linus Walleij, Bartosz Golaszewski, linux-gpio On Tue, Sep 30, 2025 at 8:26 PM Rob Herring <robh@kernel.org> wrote: > > 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@kernel.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@analog.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@analog.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? Perhaps the best example is adi,ad7194.yaml [1]. It has odr-gpios for a 2 pin input to select 4 possible ODR values in the case where they are connected to gpios and can be configured at runtime. Then it has a separate adi,odr-value property to give the hardwired value in cases where they are not connected to gpios. The binding currently doesn't allow having one pin connected to a gpio and one hardwired. The same binding also has pga-gpios and adi,pga-value which work the same and just control a different configuration parameter. adi,ad7606.yaml [2] is a bit less complete. It has adi,oversampling-ratio-gpios but it only has adi,sw-mode to indicate that all 3 oversampling pins are hard-wired high. It doesn't have a way to specify other hard-wired states. IIRC, the AD7616 chip in this family also has some more similar config selection pins that aren't documented yet. In adi,ad7625 [3], we ended up making 4 enX-gpios for single properties plus a adi,en0-always-on boolean flag property for each ENX pin instead of a en-gpios array of 4 gpios. This was a case where it was highly likely that there would be a mix of hard-wired pins and gpio-connected pins, so it seemed to be the simplest way to describe it at the time. It would have been much more ergonomic though if we could have used the single array. [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml#n52 [2]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml#n127 [3]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/iio/adc/adi,ad7625.yaml#n70 > > 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@4000000 { > > 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 That works as long as we don't need other pin states like high-impedance. I haven't seen anything like that yet though. > 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. Could work, or just dynamically allocate one when needed. > > Rob > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-10-01 11:55 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1758916484.git.marcelo.schmitt@analog.com>
[not found] ` <5dc08b622dac1db561f26034c93910ccff75e965.1758916484.git.marcelo.schmitt@analog.com>
2025-09-28 10:19 ` [PATCH v3 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216 and ADAQ4224 Jonathan Cameron
2025-09-29 14:31 ` Rob Herring
2025-09-29 16:16 ` David Lechner
2025-09-30 14:47 ` Marcelo Schmitt
2025-09-30 17:02 ` Marcelo Schmitt
2025-09-30 18:26 ` Rob Herring
2025-10-01 11:55 ` David Lechner
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).