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

Jonathan Cameron
2024-06-23T16:39:18Z

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