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

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 18 Jun 2024 14:29:10 -0500
David Lechner <dlechner@baylibre.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@baylibre.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