[PATCH 1/4] dt-bindings: iio: adc: adi,ad4000: Add PulSAR

Marcelo Schmitt posted 4 patches 1 week, 1 day ago
There is a newer version of this series
[PATCH 1/4] dt-bindings: iio: adc: adi,ad4000: Add PulSAR
Posted by Marcelo Schmitt 1 week, 1 day ago
Extend the AD4000 series device tree documentation to also describe
PulSAR devices.

Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.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
 
 $ref: /schemas/spi/spi-peripheral-props.yaml#
 
@@ -63,6 +78,38 @@ properties:
 
       - const: adi,adaq4003
 
+      - const: adi,ad7946
+      - items:
+          - enum:
+              - adi,ad7942
+          - const: adi,ad7946
+
+      - const: adi,ad7983
+      - items:
+          - enum:
+              - adi,ad7980
+              - adi,ad7988-5
+              - adi,ad7686
+              - adi,ad7685
+              - adi,ad7694
+              - adi,ad7988-1
+          - const: adi,ad7983
+
+      - const: adi,ad7688
+      - items:
+          - enum:
+              - adi,ad7693
+              - adi,ad7687
+          - const: adi,ad7688
+
+      - const: adi,ad7984
+      - items:
+          - enum:
+              - adi,ad7982
+              - adi,ad7690
+              - adi,ad7691
+          - const: adi,ad7984
+
   reg:
     maxItems: 1
 
@@ -129,10 +176,76 @@ required:
   - compatible
   - reg
   - vdd-supply
-  - vio-supply
   - ref-supply
 
 allOf:
+  # AD7694 doesn't have a VIO pin
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,ad4000
+              - adi,ad4001
+              - adi,ad4002
+              - adi,ad4003
+              - adi,ad4004
+              - adi,ad4005
+              - adi,ad4006
+              - adi,ad4007
+              - adi,ad4008
+              - adi,ad4010
+              - adi,ad4011
+              - adi,ad4020
+              - adi,ad4021
+              - adi,ad4022
+              - adi,adaq4001
+              - adi,adaq4003
+              - adi,ad7685
+              - adi,ad7686
+              - adi,ad7687
+              - adi,ad7688
+              - adi,ad7690
+              - adi,ad7691
+              - adi,ad7693
+              - adi,ad7942
+              - adi,ad7946
+              - adi,ad7980
+              - adi,ad7982
+              - adi,ad7983
+              - adi,ad7984
+              - adi,ad7988-1
+              - adi,ad7988-5
+    then:
+      required:
+        - vio-supply
+  # Single-channel PulSAR devices have SDI either tied to VIO, GND, or host CS.
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,ad7685
+              - adi,ad7686
+              - adi,ad7687
+              - adi,ad7688
+              - adi,ad7690
+              - adi,ad7691
+              - adi,ad7693
+              - adi,ad7694
+              - adi,ad7942
+              - adi,ad7946
+              - adi,ad7980
+              - adi,ad7982
+              - adi,ad7983
+              - adi,ad7984
+              - adi,ad7988-1
+              - adi,ad7988-5
+    then:
+      properties:
+        adi,sdi-pin:
+          enum: [ high, low, cs ]
+          default: high
   # The configuration register can only be accessed if SDI is connected to MOSI
   - if:
       required:
-- 
2.45.2
Re: [PATCH 1/4] dt-bindings: iio: adc: adi,ad4000: Add PulSAR
Posted by David Lechner 1 week ago
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@analog.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.

>  
>  $ref: /schemas/spi/spi-peripheral-props.yaml#
>  
> @@ -63,6 +78,38 @@ properties:
>  
>        - const: adi,adaq4003
>  
> +      - const: adi,ad7946
> +      - items:
> +          - enum:
> +              - adi,ad7942
> +          - const: adi,ad7946
> +
> +      - const: adi,ad7983
> +      - items:
> +          - enum:
> +              - adi,ad7980
> +              - adi,ad7988-5
> +              - adi,ad7686
> +              - adi,ad7685
> +              - adi,ad7694
> +              - adi,ad7988-1
> +          - const: adi,ad7983
> +
> +      - const: adi,ad7688
> +      - items:
> +          - enum:
> +              - adi,ad7693
> +              - adi,ad7687
> +          - const: adi,ad7688
> +
> +      - const: adi,ad7984
> +      - items:
> +          - enum:
> +              - adi,ad7982
> +              - adi,ad7690
> +              - adi,ad7691
> +          - const: adi,ad7984
> +

IMHO, having fallbacks just makes the bindings harder to use and doesn't
actually provide any useful benefit.

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.

>    reg:
>      maxItems: 1
>  
> @@ -129,10 +176,76 @@ required:
>    - compatible
>    - reg
>    - vdd-supply
> -  - vio-supply
>    - ref-supply
>  
>  allOf:
> +  # AD7694 doesn't have a VIO pin

It sounds like using not: could make this if: a lot shorter.

Also, it looks like ad7983 doesn't have the pin either.

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - adi,ad4000
> +              - adi,ad4001
> +              - adi,ad4002
> +              - adi,ad4003
> +              - adi,ad4004
> +              - adi,ad4005
> +              - adi,ad4006
> +              - adi,ad4007
> +              - adi,ad4008
> +              - adi,ad4010
> +              - adi,ad4011
> +              - adi,ad4020
> +              - adi,ad4021
> +              - adi,ad4022
> +              - adi,adaq4001
> +              - adi,adaq4003
> +              - adi,ad7685
> +              - adi,ad7686
> +              - adi,ad7687
> +              - adi,ad7688
> +              - adi,ad7690
> +              - adi,ad7691
> +              - adi,ad7693
> +              - adi,ad7942
> +              - adi,ad7946
> +              - adi,ad7980
> +              - adi,ad7982
> +              - adi,ad7983
> +              - adi,ad7984
> +              - adi,ad7988-1
> +              - adi,ad7988-5
> +    then:
> +      required:
> +        - vio-supply
> +  # Single-channel PulSAR devices have SDI either tied to VIO, GND, or host CS.

To me, the more interesting thing to say here is that the sdi
option is omitted because these chips don't have a programmable
register.

> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - adi,ad7685
> +              - adi,ad7686
> +              - adi,ad7687
> +              - adi,ad7688
> +              - adi,ad7690
> +              - adi,ad7691
> +              - adi,ad7693
> +              - adi,ad7694
> +              - adi,ad7942
> +              - adi,ad7946
> +              - adi,ad7980
> +              - adi,ad7982
> +              - adi,ad7983
> +              - adi,ad7984
> +              - adi,ad7988-1
> +              - adi,ad7988-5
> +    then:
> +      properties:
> +        adi,sdi-pin:
> +          enum: [ high, low, cs ]
> +          default: high

For the similar ad7944, Rob suggested that the default should be the equivalent
of "cs" since that is most like "regular" SPI. So I think it makes sense do the
same here. (The adi,spi-mode property in the ad7944 binding is named a bit
different, single = high, chain = low and _property omitted_ (default) = cs)

>    # The configuration register can only be accessed if SDI is connected to MOSI
>    - if:
>        required:
Re: [PATCH 1/4] dt-bindings: iio: adc: adi,ad4000: Add PulSAR
Posted by Marcelo Schmitt 4 days, 14 hours ago
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@analog.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

> 
> >  
> >  $ref: /schemas/spi/spi-peripheral-props.yaml#
> >  
> > @@ -63,6 +78,38 @@ properties:
> >  
> >        - const: adi,adaq4003
> >  
> > +      - const: adi,ad7946
> > +      - items:
> > +          - enum:
> > +              - adi,ad7942
> > +          - const: adi,ad7946
> > +
> > +      - const: adi,ad7983
> > +      - items:
> > +          - enum:
> > +              - adi,ad7980
> > +              - adi,ad7988-5
> > +              - adi,ad7686
> > +              - adi,ad7685
> > +              - adi,ad7694
> > +              - adi,ad7988-1
> > +          - const: adi,ad7983
> > +
> > +      - const: adi,ad7688
> > +      - items:
> > +          - enum:
> > +              - adi,ad7693
> > +              - adi,ad7687
> > +          - const: adi,ad7688
> > +
> > +      - const: adi,ad7984
> > +      - items:
> > +          - enum:
> > +              - adi,ad7982
> > +              - adi,ad7690
> > +              - adi,ad7691
> > +          - const: adi,ad7984
> > +
> 
> IMHO, having fallbacks just makes the bindings harder to use and doesn't
> actually provide any useful benefit.
> 
Having fallbacks was a suggestion from a dt maintainer to the ad4000 series.
I assumed they would ask it for PulSAR too. Will wait a comment from a dt
maintainer to change it.

> 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.

> >    reg:
> >      maxItems: 1
> >  
> > @@ -129,10 +176,76 @@ required:
> >    - compatible
> >    - reg
> >    - vdd-supply
> > -  - vio-supply
> >    - ref-supply
> >  
> >  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

> 
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - adi,ad4000
> > +              - adi,ad4001
> > +              - adi,ad4002
> > +              - adi,ad4003
> > +              - adi,ad4004
> > +              - adi,ad4005
> > +              - adi,ad4006
> > +              - adi,ad4007
> > +              - adi,ad4008
> > +              - adi,ad4010
> > +              - adi,ad4011
> > +              - adi,ad4020
> > +              - adi,ad4021
> > +              - adi,ad4022
> > +              - adi,adaq4001
> > +              - adi,adaq4003
> > +              - adi,ad7685
> > +              - adi,ad7686
> > +              - adi,ad7687
> > +              - adi,ad7688
> > +              - adi,ad7690
> > +              - adi,ad7691
> > +              - adi,ad7693
> > +              - adi,ad7942
> > +              - adi,ad7946
> > +              - adi,ad7980
> > +              - adi,ad7982
> > +              - adi,ad7983
> > +              - adi,ad7984
> > +              - adi,ad7988-1
> > +              - adi,ad7988-5
> > +    then:
> > +      required:
> > +        - vio-supply
> > +  # Single-channel PulSAR devices have SDI either tied to VIO, GND, or host CS.
> 
> To me, the more interesting thing to say here is that the sdi
> option is omitted because these chips don't have a programmable
> register.

Yes, that's correct. But the adi,sdi-pin property is about what is connected
to the SDI/MOSI pin so I kept the comment about hw connections only.
We could in theory connect SDI to host MOSI and set MOSI idle high (if the
controller supports that), but that is harder to describe.

> 
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - adi,ad7685
> > +              - adi,ad7686
> > +              - adi,ad7687
> > +              - adi,ad7688
> > +              - adi,ad7690
> > +              - adi,ad7691
> > +              - adi,ad7693
> > +              - adi,ad7694
> > +              - adi,ad7942
> > +              - adi,ad7946
> > +              - adi,ad7980
> > +              - adi,ad7982
> > +              - adi,ad7983
> > +              - adi,ad7984
> > +              - adi,ad7988-1
> > +              - adi,ad7988-5
> > +    then:
> > +      properties:
> > +        adi,sdi-pin:
> > +          enum: [ high, low, cs ]
> > +          default: high
> 
> For the similar ad7944, Rob suggested that the default should be the equivalent
> of "cs" since that is most like "regular" SPI. So I think it makes sense do the
> same here. (The adi,spi-mode property in the ad7944 binding is named a bit
> different, single = high, chain = low and _property omitted_ (default) = cs)
Ack

> 
> >    # The configuration register can only be accessed if SDI is connected to MOSI
> >    - if:
> >        required:
>
Re: [PATCH 1/4] dt-bindings: iio: adc: adi,ad4000: Add PulSAR
Posted by Marcelo Schmitt 4 days, 7 hours ago
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@analog.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.
Re: [PATCH 1/4] dt-bindings: iio: adc: adi,ad4000: Add PulSAR
Posted by David Lechner 4 days, 7 hours ago
On 11/18/24 12:25 PM, Marcelo Schmitt 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@analog.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
> 
I think all of the 6s, 8s and 9s were playing tricks on my brain when I wrote that.
Looking again now, it looks fine to me. Sorry for the noise.
Re: [PATCH 1/4] dt-bindings: iio: adc: adi,ad4000: Add PulSAR
Posted by David Lechner 4 days, 10 hours ago
On 11/18/24 5:24 AM, 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@analog.com>
>>> ---

...

>>>  
>>>  $ref: /schemas/spi/spi-peripheral-props.yaml#
>>>  
>>> @@ -63,6 +78,38 @@ properties:
>>>  
>>>        - const: adi,adaq4003
>>>  
>>> +      - const: adi,ad7946
>>> +      - items:
>>> +          - enum:
>>> +              - adi,ad7942
>>> +          - const: adi,ad7946
>>> +
>>> +      - const: adi,ad7983
>>> +      - items:
>>> +          - enum:
>>> +              - adi,ad7980
>>> +              - adi,ad7988-5
>>> +              - adi,ad7686
>>> +              - adi,ad7685
>>> +              - adi,ad7694
>>> +              - adi,ad7988-1
>>> +          - const: adi,ad7983
>>> +
>>> +      - const: adi,ad7688
>>> +      - items:
>>> +          - enum:
>>> +              - adi,ad7693
>>> +              - adi,ad7687
>>> +          - const: adi,ad7688
>>> +
>>> +      - const: adi,ad7984
>>> +      - items:
>>> +          - enum:
>>> +              - adi,ad7982
>>> +              - adi,ad7690
>>> +              - adi,ad7691
>>> +          - const: adi,ad7984
>>> +
>>
>> IMHO, having fallbacks just makes the bindings harder to use and doesn't
>> actually provide any useful benefit.
>>
> Having fallbacks was a suggestion from a dt maintainer to the ad4000 series.
> I assumed they would ask it for PulSAR too. Will wait a comment from a dt
> maintainer to change it.
> 
>> 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.
> 
OK. Another difference for consideration that I noticed is that on some chips,
the SDO line can generate a BUSY interrupt while others can't. Not sure if
that matters from the point of view of fallbacks or not.