Message-ID: <20241124131147.0d616f82@jic23-huawei>
Date: Sun, 24 Nov 2024 13:11:47 +0000
From: Jonathan Cameron <jic23@...nel.org>
To: Marcelo Schmitt <marcelo.schmitt1@...il.com>
Cc: David Lechner <dlechner@...libre.com>, Marcelo Schmitt
 <marcelo.schmitt@...log.com>, lars@...afoo.de,
 Michael.Hennerich@...log.com, robh@...nel.org, krzk+dt@...nel.org,
 conor+dt@...nel.org, linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] dt-bindings: iio: adc: adi,ad4000: Add PulSAR

On Mon, 18 Nov 2024 15:25:58 -0300
Marcelo Schmitt <marcelo.schmitt1@...il.com> wrote:

> On 11/18, Marcelo Schmitt wrote:
> > On 11/15, David Lechner wrote:  
> > > On 11/14/24 5:50 PM, Marcelo Schmitt wrote:  
> > > > Extend the AD4000 series device tree documentation to also describe
> > > > PulSAR devices.
> > > > 
> > > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@...log.com>
> > > > ---
> > > >  .../bindings/iio/adc/adi,ad4000.yaml          | 115 +++++++++++++++++-
> > > >  1 file changed, 114 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > > > index e413a9d8d2a2..35049071a9de 100644
> > > > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > > > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > > > @@ -19,6 +19,21 @@ description: |
> > > >      https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
> > > >      https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
> > > >      https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7685.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7686.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7687.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7688.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7690.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7691.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7693.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7694.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7942.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7946.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7980.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7982.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7983.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7984.pdf
> > > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7988-1_7988-5.pdf  
> > > 
> > > It would be nice to sort these lowest number first.  
> > 
> > Ack
> >   
> Actually, I didn't get how I'm expected to sort those.
> Isn't ad4000 < ad7685?
> Or did you mean to put adaq at the end?
> 
> ad4000-4004-4008.pdf
> ...
> ad4020-4021-4022.pdf
> ad7685.pdf
> ...
> ad7988-1_7988-5.pdf
> adaq4001.pdf
> adaq4003.pdf
> 
> 
> [...]
> >   
> > > And with this many chips, it can be easy to overlook a small difference
> > > in one chips, like ad7694 not having VIO pin, so is it really fallback
> > > compatible? Easier to just avoid the question and not have fallbacks.
> > >   
> > The absence of a VIO pin does not change how the driver handles the devices.
> > They are compatible from software perspective.
> >   
> [...]
> > > >  
> > > >  allOf:
> > > > +  # AD7694 doesn't have a VIO pin  
> > > 
> > > It sounds like using not: could make this if: a lot shorter.  
> > 
> > Ack
> >   
> > > 
> > > Also, it looks like ad7983 doesn't have the pin either.  
> > 
> > Ack  
> 
> I forgot the ad4000 driver fails if VIO is not provided so I was wrong when I
> said AD7694 was software compatible with the other ADCs.
> I see now AD7694 also doesn't have SDI pin.
> Aside from the VIO and SDI pins, AD7694 is similar to AD7685 both being 16-bit
> precision 250kSPS pseudo-differential ADCs.
> The AD7683 part you mentioned is similar to AD7988-1, both 16-bit
> pseudo-differential 100kSPS.
> To avoid complicating things, I'm dropping support for AD7694.
> AD7685 and AD7988-1 are the parts with features similar to AD7694 and AD7683,
> respectively.

General rule is that fallbacks only work if a part is a strict subset
functionality wise or identical.   If we get it wrong, then that's fine
as the more specific compatible allows us to fix things up even for
dt files that are in products, but if we know there are incompatibilities
or things we want to check for in the binding then don't do fallbacks in
the first palce.   E.g. if there is a pin that is only on a fancy device
then even if it will work fine with a driver binding to the simpler one, we
may want to not use a fallback because we want the dt-binding to verify
the pin is only provided for the more sophisticated device.

Gets fuzzy however in some cases!

Usually what DT maintainers are looking for is a clear statement of how
the devices are different.

Jonathan

Powered by blists - more mailing lists