Message-ID: <20250921-unadvised-uninjured-cdd7a6e6f326@spud>
Date: Sun, 21 Sep 2025 23:20:01 +0100
From: Conor Dooley <conor@...nel.org>
To: Marcelo Schmitt <marcelo.schmitt1@...il.com>
Cc: 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,
	jic23@...nel.org, michael.hennerich@...log.com, nuno.sa@...log.com,
	eblanc@...libre.com, dlechner@...libre.com, andy@...nel.org,
	robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
	corbet@....net
Subject: Re: [PATCH v2 7/8] dt-bindings: iio: adc: adi,ad4030: Add ADAQ4216
 and ADAQ4224

On Fri, Sep 19, 2025 at 06:12:05PM -0300, Marcelo Schmitt wrote:
> On 09/19, Conor Dooley wrote:
> > On Thu, Sep 18, 2025 at 02:39:29PM -0300, Marcelo Schmitt 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 v1 -> v2
> > > - Use pattern to specify devices that require gain related properties.
> > > - Disallow gain related properties for devices that don't come with embedded PGA.
> > > - Documented VDDH and VDD_FDA supplies for ADAQ4216 and ADAQ4224.
> > > - Updated PGA gain constants.
> > > 
> > >  .../bindings/iio/adc/adi,ad4030.yaml          | 65 +++++++++++++++++--
> > >  1 file changed, 60 insertions(+), 5 deletions(-)
> > > 
> ...
> > >  
> > > +  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-value:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > 
> > How come this is "value" rather than "gain"?
> 
> Because, for this one, I drew inspiration from ad7191 bindings [1] in the hopes
> of avoiding creating new properties or using discontinued/deprecated
> nomenclature [2].
> 
> The thing is, we now have ADC chips coming with PGA circuitry in front of ADC
> inputs. Those PGAs are usually set/configured through hardware connections
> (e.g. dedicated GPIOs or pin-strapped) and have been described in dt-bindings.
> Though, since these added PGAs don't follow a pattern with respect to the
> provided gain, different properties began to appear. ad7380 and ad4000 use
> adi,gain-milli to describe PGA gain [3, 4], ad7191 uses adi,pga-value and,
> more recently, adaq7768-1 has been proposed with adi,aaf-gain-bp [5].
> adaq7768-1 is arguably a slightly different case since the signal gain stems
> from an anti-aliasing filter, but it nevertheless results in signal attenuation
> much like some PGAs.
> 
> I personally like the -milli (or even -permille) nomenclature because 4 digits
> have been more than enough to describe the gains (at least so far). Though, I
> acknowledge the base points suffix (-bp) which is documented in
> property-units.yaml [6]. The only thing I don't like much about -bp for
> describing PGA gain is that PGA gains are often described in terms of unitless
> scale factors, while bp implies the value to be described as a percent.
> 
> Anyways, whatever property name is chosen, it will probably be better settle to
> something rather than arguing about property names each time a new ADC comes
> with an integrated PGA.

If PGA gains are common, then ye it would make sense to have a standard
property. I guess one of the problems with doing so is that there isn't
a standard/common binding for adcs themselves, so without making one
it'd involve reviewers pushing people to the standard one. I suppose the
current adc.yaml could be made into adc-channel.yaml and adc.yaml
repurposed. I bet there are more properties than just PGA gain that
could go there.

My personal objection to "pga-value" is that it doesn't communicate by
itself what aspect of the pga it actually controls. I don't really care
what "unit" qualifier is used that much or if one is used at all. That's
more of a thing for yourself and other IIO developers to handle.

Part of me is bothered though that all these gains are not in dB! But
I'd imagine there are not really any ADCs where the registers don't
deal in unitless gain and using dB would be nothing more than an
additional headache for software developers.

> [1] Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml
> [2] https://lore.kernel.org/linux-iio/510f6efb-ada3-4848-ac8e-16fa5d1b5284@kernel.org/
> [3] Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml
> [4] Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> [5] https://lore.kernel.org/linux-iio/46842d4cf2c1149bd64188f94c60ce5e4f3b2beb.1757001160.git.Jonathan.Santos@analog.com/
> [6] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml
> 
> > 
> > > +    description: |
> > > +      Should be present if PGA control inputs are pin-strapped. The values
> > > +      specify the gain per mille. For example, 333 means the input signal is
> > > +      scaled by a 0.333 factor (i.e. attenuated to one third of it's original
> > > +      magnitude). Possible values:
> > > +      Gain 333 (A1=0, A0=0)
> > > +      Gain 555 (A1=0, A0=1)
> > > +      Gain 2222 (A1=1, A0=0)
> > > +      Gain 6666 (A1=1, A0=1)
> > > +      If defined, pga-gpios must be absent.
> > > +    enum: [333, 555, 2222, 6666]
> > > +
> 
> Thanks,
> Marcelo

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)

Powered by blists - more mailing lists