lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z0CkOTGhGhfV18OG@debian-BULLSEYE-live-builder-AMD64>
Date: Fri, 22 Nov 2024 12:33:13 -0300
From: Marcelo Schmitt <marcelo.schmitt1@...il.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Marcelo Schmitt <marcelo.schmitt@...log.com>, lars@...afoo.de,
	Michael.Hennerich@...log.com, jic23@...nel.org, 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 v3 1/4] dt-bindings: iio: adc: adi,ad4000: Add PulSAR

On 11/20, Krzysztof Kozlowski wrote:
> On Tue, Nov 19, 2024 at 09:53:40AM -0300, Marcelo Schmitt wrote:
> > Extend the AD4000 series device tree documentation to also describe
> > PulSAR devices.
> > 
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@...log.com>
> > ---
> > No changes from v2 -> v3.
> > 
> >  .../bindings/iio/adc/adi,ad4000.yaml          | 71 +++++++++++++++++++
> >  1 file changed, 71 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > index e413a9d8d2a2..4dbb3d2876f9 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> > @@ -19,6 +19,20 @@ 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/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 +77,37 @@ properties:
> >  
> >        - const: adi,adaq4003
> >  
> > +      - const: adi,ad7946
> 
> All such cases are just one enum. That's the preferred syntax.
> 
Ack

> 
> > +      - items:
> > +          - enum:
> > +              - adi,ad7942
> > +          - const: adi,ad7946
> > +
> > +      - const: adi,ad7983
> > +      - items:
> > +          - enum:
> > +              - adi,ad7980
> > +              - adi,ad7988-5
> > +              - adi,ad7686
> > +              - adi,ad7685
> 
> Keep alphabetical order.

Do the fallbacks declared here have any impact on the match try order or on how
the compatible list should be ordered?
The only significant difference between each group of devices is the sample rate.
A faster device can read at slower sample rates so if somebody knows to have
a 16-bit pseudo-differential PulSAR but doesn't know about the exact model they
could have a compatible like
      compatible = "adi,ad7980", "adi,ad7988-5", "adi,ad7686", "adi,ad7685",
                   "adi,ad7988-1", "adi,ad7983";

to try from fastest to slowest device.
The dt doc would indicate that order in the fallback list?
      - items:
          - enum:
              - adi,ad7980    # Fastest 16-bit pseudo-differential ADC
              - adi,ad7988-5  # 2nd fastest 16-bit pseudo-differential ADC
              - adi,ad7686    # 3rd fastest 16-bit pseudo-differential ADC
              - adi,ad7685    # 4th fastest 16-bit pseudo-differential ADC
              - adi,ad7988-1  # 5th fastest 16-bit pseudo-differential ADC
          - const: adi,ad7983 # Slowest 16-bit pseudo-differential ADC

https://www.analog.com/media/en/technical-documentation/data-sheets/ad7691.pdf
has a nice table with the different devices and sample rates.

writing-bindings.rst says "DO use fallback compatibles when devices are the same
as or a subset of prior implementations."
But, how can we use fallbacks properly?
>From Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml I'm
inferring only one fallback should be provided per group of devices.

> 
> > +              - 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
> >  
> > @@ -133,6 +178,32 @@ required:
> >    - ref-supply
> >  
> >  allOf:
> > +  # Single-channel PulSAR devices have SDI either tied to VIO, GND, or host CS.
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - adi,ad7685
> 
> Why do you need this? It's fallback is already here.

So dtbs_check can provide an error message if for example compatible = "adi,ad7687";
and adi,sdi-pin = "sdi";

zynq-coraz7s-ad7687.dtb: adc@0: adi,sdi-pin:0: 'sdi' is not one of ['high', 'low', 'cs']
> 
> > +              - 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
> 
> Best regards,
> Krzysztof
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ