Re: [PATCH 1/2] dt-bindings: iio: adc: add ad7944 ADCs

[Jonathan Cameron <jic23@kernel.org>]

On Tue,  6 Feb 2024 11:25:59 -0600
David Lechner <dlechner@baylibre.com> wrote:

> This adds a new binding for the Analog Devices, Inc. AD7944, AD7985, and
> AD7986 ADCs.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>

Hi David,

Some tricky corners...
3-wire here for example doesn't mean what I at least expected it to.

> ---
>  .../devicetree/bindings/iio/adc/adi,ad7944.yaml    | 231 +++++++++++++++++++++
>  MAINTAINERS                                        |   8 +
>  2 files changed, 239 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7944.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7944.yaml
> new file mode 100644
> index 000000000000..a023adbeba42
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7944.yaml
> @@ -0,0 +1,231 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad7944.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices PulSAR LFCSP Analog to Digital Converters
> +
> +maintainers:
> +  - Michael Hennerich <Michael.Hennerich@analog.com>
> +  - Nuno Sá <nuno.sa@analog.com

I hope Nuno + Michael will ack this. Bit mean to drop them in it otherwise
(funny though :)

> +
> +description: |
> +  A family of pin-compatible single channel differential analog to digital
> +  converters with SPI support in a LFCSP package.
> +
> +  * https://www.analog.com/en/products/ad7944.html
> +  * https://www.analog.com/en/products/ad7985.html
> +  * https://www.analog.com/en/products/ad7986.html
> +
> +$ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad7944
> +      - adi,ad7985
> +      - adi,ad7986
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    maximum: 111111111

So 9ns for 3-write and 4-wire, but I think it's 11ns for chained.
Maybe it's not worth constraining that.

> +
> +  spi-cpha: true
> +
> +  adi,spi-mode:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum: [ 3-wire, 4-wire, chain ]
> +    default: 4-wire
> +    description:
> +      This chip can operate in a 3-wire mode where SDI is tied to VIO, a 4-wire
> +      mode where SDI acts as the CS line, or a chain mode where SDI of one chip
> +      is tied to the SDO of the next chip in the chain and the SDI of the last
> +      chip in the chain is tied to GND.

there is a standard property in spi-controller.yaml for 3-wire. Does that cover
the selection between 3-wire and 4-wire here?  Seems like this might behave
differently from that (and so perhaps we shouldn't use 3-wire as the description
to avoid confusion, normally 3-wire is a half duplex link I think).

Chain mode is more fun.  We've had that before and I'm trying to remember what
the bindings look like. Devices like ad7280a do a different form of chaining.

Anyhow, main thing here is we need to be careful that the terms don't overlap
with other possible interpretations.

I think what this really means is:

3-wire - no chip select, exclusive use of the SPI bus (yuk)
4-write - conventional SPI with CS
chained - the 3 wire mode really but with some timing effects?

Can we figure out if chained is going on at runtime?







> +
> +  avdd-supply:
> +    description: A 2.5V supply that powers the analog circuitry.
> +
> +  dvdd-supply:
> +    description: A 2.5V supply that powers the digital circuitry.
> +
> +  vio-supply:
> +    description:
> +      A 1.8V to 2.7V supply for the digital inputs and outputs.
> +
> +  bvdd-supply:
> +    description:
> +      A voltage supply for the buffered power. When using an external reference
> +      without an internal buffer (PDREF high, REFIN low), this should be
> +      connected to the same supply as ref-supply. Otherwise, when using an
> +      internal reference or an external reference with an internal buffer, this
> +      is connected to a 5V supply.
> +
> +  ref-supply:
> +    description:
> +      Voltage regulator for the reference voltage (REF). This property is
> +      omitted when using an internal reference.
> +
> +  refin-supply:
> +    description:
> +      Voltage regulator for the reference buffer input (REFIN). When using an
> +      external buffer with internal reference, this should be connected to a
> +      1.2V external reference voltage supply.
> +
> +  adi,reference:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum: [ internal, internal-buffer, external ]

I'm a bit lost on this one - but think we can get rid of it in favour of using
the fact someone wired up the supplies to indicate their intent?

> +    default: internal
> +    description: |
> +      This property is used to specify the reference voltage source.
> +
> +      * internal: PDREF is wired low. The internal 4.096V reference voltage is
> +        used. The REF pin outputs 4.096V and REFIN outputs 1.2V.

So if neither refin-supply or ref-supply is present then this is the one to use.

> +      * internal-buffer: PDREF is wired high. REFIN is supplied with 1.2V. The
> +        buffered internal 4.096V reference voltage is used. The REF pin outputs
> +        4.096V.

So if refin-supply is supplied this is the expected choice?

> +      * external: PDREF is wired high and REFIN is wired low. The supply
> +        connnected the REF pin is used as the reference voltage.

So if a ref-supply is provided this is expected choice?

If we are going to rule you supplying refin and ref supplies. 

> +
> +  cnv-gpios:
> +    description:
> +      The Convert Input (CNV). This input has multiple functions. It initiates
> +      the conversions and selects the SPI mode of the device (chain or CS). In
> +      3-wire mode, this property is omitted if the CNV pin is connected to the
> +      CS line of the SPI controller.
> +    maxItems: 1

ah, that's exciting - so in 3-wire mode, we basically put the CS on a different pin...

Mark, perhaps you can suggest how to handle this complex family of spi variants?

Jonathan