linux-kernel - Re: [PATCH v2 2/4] dt-bindings: iio: adc: add AD4695 and similar ADCs

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Sun, 23 Jun 2024 17:39:11 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: David Lechner <dlechner@...libre.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Michael Hennerich
 <michael.hennerich@...log.com>, Nuno Sá <nuno.sa@...log.com>,
 Jonathan Corbet <corbet@....net>, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-doc@...r.kernel.org
Subject: Re: [PATCH v2 2/4] dt-bindings: iio: adc: add AD4695 and similar
 ADCs

On Tue, 18 Jun 2024 14:29:10 -0500
David Lechner <dlechner@...libre.com> wrote:

> On 6/17/24 2:53 PM, David Lechner wrote:
> > Add device tree bindings for AD4695 and similar ADCs.
> > 
> > Signed-off-by: David Lechner <dlechner@...libre.com>
> > ---
> >   
> ...
> 
> > +
> > +  interrupts:
> > +    minItems: 1
> > +    items:
> > +      - description:
> > +          Signal coming from the BSY_ALT_GP0 or GP3 pin that indicates a busy
> > +          condition.
> > +      - description:
> > +          Signal coming from the BSY_ALT_GP0 or GP2 pin that indicates an alert
> > +          condition.
> > +
> > +  interrupt-names:
> > +    minItems: 1
> > +    items:
> > +      - const: busy
> > +      - const: alert
> > +  
> 
> Since the interrupt can come from two different pins, it seems like we would
> need an extra property to specify this. Is there a standard way to do this?
> 
> Otherwise I will add something like:
> 
> adi,busy-on-gp3:
>   $ref: /schemas/types.yaml#/definitions/flag
>   description:
>     When present, the busy interrupt is coming from the GP3 pin, otherwise
>     the interrupt is coming from the BSY_ALT_GP0 pin.
>    
> adi,alert-on-gp2:
>   $ref: /schemas/types.yaml#/definitions/flag
>   description:
>     When present, the alert interrupt is coming from the GP2 pin, otherwise
>     the interrupt is coming from the BSY_ALT_GP0 pin.
Cut and paste?  Or it ends up on the same pin as the bsy? In which case that's
a single interrupt and it's up to software to decide how to use. I'll guess
it comes on GP1?
> 

More interrupt names.  We shouldn't restrict someone wiring all 4 if they want
to - we'll just use 2 that we choose in the driver.

interrupt-names
  minItems: 1
  items:
    - const: busy-gp0
    - const: busy-gp1
    - const: alert-gp2
    - cosnt: alert-gp1

T   
> 
> > +
> > +patternProperties:
> > +  "^channel@[0-9a-f]$":
> > +    type: object
> > +    $ref: adc.yaml
> > +    unevaluatedProperties: false
> > +    description:
> > +      Describes each individual channel. In addition the properties defined
> > +      below, bipolar from adc.yaml is also supported.
> > +
> > +    properties:
> > +      reg:
> > +        maximum: 15
> > +
> > +      diff-channels:
> > +        description:
> > +          Describes inputs used for differential channels. The first value must
> > +          be an even numbered input and the second value must be the next
> > +          consecutive odd numbered input.
> > +        items:
> > +          - minimum: 0
> > +            maximum: 14
> > +            multipleOf: 2
> > +          - minimum: 1
> > +            maximum: 15
> > +            not:
> > +              multipleOf: 2  
> 
> After some more testing, it turns out that I misunderstood the datasheet and
> this isn't actually fully differential, but rather pseudo-differential.
> 
> So when pairing with the next pin, it is similar to pairing with the COM pin
> where the negative input pin is connected to a constant voltage source.

Ok. I'm curious, how does it actually differ from a differential channel?
What was that test?  It doesn't cope with an actual differential pair and needs
a stable value on the negative?

> 
> > +
> > +      single-channel:
> > +        minimum: 0
> > +        maximum: 15
> > +
> > +      common-mode-channel:
> > +        description:
> > +          Describes the common mode channel for single channels. 0 is REFGND
> > +          and 1 is COM. Macros are available for these values in
> > +          dt-bindings/iio/adi,ad4695.h.
> > +        minimum: 0
> > +        maximum: 1
> > +        default: 0  
> 
> So I'm thinking the right thing to do here go back to using reg and the INx
> number and only have common-mode-channel (no diff-channels or single-channel).
> 
> common-mode-channel will need to be changed to allow INx numbers in addition
> to COM and REFGND.
> 
> This means that [PATCH v2 1/4] "dt-bindings: iio: adc: add common-mode-channel
> dependency" would be wrong since we would be using common-mode-channel without
> single-channel.
> 
> It also means we will need an optional in1-supply: true for all odd numbered
> inputs.
Ok. I'm not totally sure I see how this comes together but will wait for v3 rather
than trying to figure it out now.

Jonathan