Re: [PATCH 1/2] dt-bindings: iio: adc: Add binding for AD7380 ADCs

[David Lechner]

On Sun, Dec 10, 2023 at 2:49 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On Fri,  8 Dec 2023 09:51:40 -0600
> David Lechner <dlechner@xxxxxxxxxxxx> wrote:
>
> > This adds a binding specification for the Analog Devices Inc. AD7380
> > family of ADCs.
> >
> > Signed-off-by: David Lechner <dlechner@xxxxxxxxxxxx>
> Hi David,
>
> Comments inline.  A question for Mark Brown on the 2-wire bit..
> Do we have existing DT bindings for devices with parallel spi data
> outputs?
>
> > ---
> >  .../devicetree/bindings/iio/adc/adi,ad7380.yaml    | 102 +++++++++++++++++++++
> >  MAINTAINERS                                        |   9 ++
> >  2 files changed, 111 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml
> > new file mode 100644
> > index 000000000000..e9a0b72cd9d3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7380.yaml
> ...
>
>
> > +  * https://www.analog.com/en/products/ad7380.html
> > +  * https://www.analog.com/en/products/ad7381.html
> > +  * https://www.analog.com/en/products/ad7383.html
> > +  * https://www.analog.com/en/products/ad7384.html
>
> > +  adi,sdo-mode:
> > +    $ref: /schemas/types.yaml#/definitions/string
> > +    enum: [ 1-wire, 2-wire ]
> > +    description:
> > +      In 1-wire mode, the SDOA pin acts as the sole data line and the SDOB/ALERT
> > +      pin acts as the ALERT interrupt signal. In 2-wire mode, data for input A
> > +      is read from SDOA and data for input B is read from SDOB/ALERT (and the
> > +      ALERT interrupt signal is not available).
>
> This is fun...  If I understand correctly 2-wire requires two SPI buses (or a complex
> spi controller that does parallel serial channels).

No, it wouldn't work with two separate SPI busses. Only a special
controller with parallel serial channels.

> What would description for that
> look like in DT and can we not establish what is wanted here from that bus description
> rather than an adi specific property?
>
> Seems a bit like parallel-memories.

I don't think this is the same as parallel-memories. Looking at the
the patch [1] it looks like parallel memories requires multiple CS
lines to connect to multiple chips to make the multiple chips behave
as one chip but otherwise works like SPI_RX_DUAL.

This ADC, on the other hand, works as if two chips that share
everything except the MISO line. Each MISO line (SDOA and SDOB) shifts
out one bit of two different words in parallel on each line instead of
two bits of the same word like SPI_RX_DUAL busses.

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git/commit/?h=for-6.8&id=88a50c1663ffa9f6b31705c6bf7a887a2c8d9434

>
> Mark, any insights into what we should do to describe this?
>
> > +
> > +  vcc-supply:
> > +    description: A 3V to 3.6V supply that powers the chip.
> > +
> > +  vlogic-supply:
> > +    description:
> > +      A 1.65V to 3.6V supply for the logic pins.
> > +
> > +  refio-supply:
> > +    description:
> > +      A 2.5V to 3.3V supply for the external reference voltage. When omitted,
> > +      the internal 2.5V reference is used.
> > +
> > +  interrupts:
> > +    description:
> > +      When the device is using 1-wire mode, this property is used to optionally
> > +      specify the ALERT interrupt.
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - adi,sdo-mode
>
> Could define a default of 1-wire?  Simplifies things a little in the bindings.
>
> > +  - vcc-supply
> > +  - vlogic-supply
>
>