Received-SPF: pass (google.com: domain of linux-kernel+bounces-112472-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <cover.1711131830.git.marcelo.schmitt@analog.com>
 <81665b5f0d37d593e6d299528de8d68da8574077.1711131830.git.marcelo.schmitt@analog.com>
 <20240323184454.201edbc3@jic23-huawei>
In-Reply-To: <20240323184454.201edbc3@jic23-huawei>
From: David Lechner <dlechner@baylibre.com>
Date: Sat, 23 Mar 2024 15:18:48 -0500
Message-ID: <CAMknhBFRa-AwM3o-AdDDmPnwLAer8x=9TJNasSbY2bu5h9mMdQ@mail.gmail.com>
Subject: Re: [PATCH 1/2] dt-bindings: iio: adc: Add AD4000
To: Jonathan Cameron <jic23@kernel.org>
Cc: Marcelo Schmitt <marcelo.schmitt@analog.com>, lars@metafoo.de, 
	Michael.Hennerich@analog.com, robh+dt@kernel.org, 
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, 
	marcelo.schmitt1@gmail.com, linux-iio@vger.kernel.org, 
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sat, Mar 23, 2024 at 1:45=E2=80=AFPM Jonathan Cameron <jic23@kernel.org>=
 wrote:
>
> On Fri, 22 Mar 2024 19:05:08 -0300
> Marcelo Schmitt <marcelo.schmitt@analog.com> wrote:
>
> > Add device tree documentation for AD4000 series of ADC devices.
> >
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data=
-sheets/ad4000-4004-4008.pdf
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data=
-sheets/ad4001-4005.pdf
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data=
-sheets/ad4002-4006-4010.pdf
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data=
-sheets/ad4003-4007-4011.pdf
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data=
-sheets/ad4020-4021-4022.pdf
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data=
-sheets/adaq4001.pdf
> > Datasheet: https://www.analog.com/media/en/technical-documentation/data=
-sheets/adaq4003.pdf
> >
> > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > ---
> > Pasting relevant comment from cover letter here to aid reviewers.
> >
> > These devices have the same SPI (Strange Peripheral Interface) as AD794=
4
> > devices, which has been documented in ad7944.rst [1].
> > The device tree description for SPI connections and mode can be the sam=
e as of
> > ad7944 adi,spi-mode [2].
> > Because ad4000 driver does not currently support daisy-chain mode, I si=
mplified
> > things a little bit. If having a more complete doc is preferred, I'm fi=
ne
> > changing to that.

Yes, having a complete binding is always preferred [1]. Bindings
should never omit anything just because it isn't implemented in the
driver.

[1]: https://www.kernel.org/doc/html/latest/devicetree/bindings/writing-bin=
dings.html

..

> > +
> > +  adi,spi-cs-mode:
>
> We've just merged a driver for the ad7944 and bindings which has a
> similar 3-wire-mode.  Please share the approach used in that binding.
> Whilst it seems we don't have the other mode here, I think we still want
> to use a similar enum.

The ad40xx chips actually do have the same daisy chain mode. So the
exact same property and all enum values apply.

> +CC David to take a look at this one given he went through long
> discussions on how to deal with it for the driver he was working on
> so probably remembers the reasoning etc better than I do :)
>

In addition to the SPI wiring modes, the proposed bindings are also
missing power supplies and the busy interrupt.

Also, since the ADAQ chips are quite different from the AD chips, it
would be very helpful for reviewers (and git history) to split out
adding those chips to the DT bindings and driver into separate
patches. This way we can clearly see which features only apply to the
ADAQ chips.

Here is what I would consider a reasonably complete binding for the
AD40XX chips (excluding ADAQ for now as I suggested).


---
# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
%YAML 1.2
---
$id: http://devicetree.org/schemas/iio/adc/adi,ad4000.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#

title: Analog Devices AD4000 and similar Analog to Digital Converters

maintainers:
  - Marcelo Schmitt <marcelo.schmitt@analog.com>

description: |
  Analog Devices AD4000 family of Analog to Digital Converters with SPI sup=
port.
  Specifications can be found at:
    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4=
000-4004-4008.pdf
    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4=
001-4005.pdf
    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4=
002-4006-4010.pdf
    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4=
003-4007-4011.pdf
    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4=
020-4021-4022.pdf

$ref: /schemas/spi/spi-peripheral-props.yaml#

properties:
  compatible:
    enum:
      - adi,ad4000
      - adi,ad4001
      - adi,ad4002
      - adi,ad4003
      - adi,ad4004
      - adi,ad4005
      - adi,ad4006
      - adi,ad4007
      - adi,ad4008
      - adi,ad4010
      - adi,ad4011
      - adi,ad4020
      - adi,ad4021
      - adi,ad4022

  reg:
    maxItems: 1

  spi-max-frequency:
    maximum: 102040816 # for VIO > 2.7 V, 81300813 for VIO > 1.7 V

  spi-cpha: true

  adi,spi-mode:
    $ref: /schemas/types.yaml#/definitions/string
    enum: [ single, chain ]
    description: |
      This property indicates the SPI wiring configuration.

      When this property is omitted, it is assumed that the device is using=
 what
      the datasheet calls "4-wire mode". This is the conventional SPI mode =
used
      when there are multiple devices on the same bus. In this mode, the CN=
V
      line is used to initiate the conversion and the SDI line is connected=
 to
      CS on the SPI controller.

      When this property is present, it indicates that the device is using =
one
      of the following alternative wiring configurations:

      * single: The datasheet calls this "3-wire mode". (NOTE: The datashee=
t's
        definition of 3-wire mode is NOT at all related to the standard
        spi-3wire property!) This mode is often used when the ADC is the on=
ly
        device on the bus. In this mode, SDI is tied to VIO, and the CNV li=
ne
        can be connected to the CS line of the SPI controller or to a GPIO,=
 in
        which case the CS line of the controller is unused.
      * chain: The datasheet calls this "chain mode". This mode is used to =
save
        on wiring when multiple ADCs are used. In this mode, the SDI line o=
f
        one chip is tied to the SDO of the next chip in the chain and the S=
DI of
        the last chip in the chain is tied to GND. Only the first chip in t=
he
        chain is connected to the SPI bus. The CNV line of all chips are ti=
ed
        together. The CS line of the SPI controller can be used as the CNV =
line
        only if it is active high.

  '#daisy-chained-devices': true

  vdd-supply:
    description: A 1.8V supply that powers the chip (VDD).

  vio-supply:
    description:
      A 1.8V to 5V supply for the digital inputs and outputs (VIO).

  ref-supply:
    description:
      A 2.5 to 5V supply for the external reference voltage (REF).

  cnv-gpios:
    description:
      The Convert Input (CNV). This input has multiple functions. It initia=
tes
      the conversions and selects the SPI mode of the device (chain or CS).=
 In
      'single' mode, this property is omitted if the CNV pin is connected t=
o the
      CS line of the SPI controller.
    maxItems: 1

  interrupts:
    description:
      The SDO pin can also function as a busy indicator. This node should b=
e
      connected to an interrupt that is triggered when the SDO line goes lo=
w
      while the SDI line is high and the CNV line is low ('single' mode) or=
 the
      SDI line is low and the CNV line is high ('multi' mode); or when the =
SDO
      line goes high while the SDI and CNV lines are high (chain mode),
    maxItems: 1

required:
  - compatible
  - reg
  - vdd-supply
  - vio-supply
  - ref-supply

allOf:
  # in '4-wire' mode, cnv-gpios is required, for other modes it is optional
  - if:
      not:
        required:
          - adi,spi-mode
    then:
      required:
        - cnv-gpios
  # chain mode has lower SCLK max rate
  - if:
      required:
        - adi,spi-mode
      properties:
        adi,spi-mode:
          const: chain
    then:
      properties:
        spi-max-frequency:
          maximum: 50000000 # for VIO > 2.7 V, 40000000 for VIO > 1.7 V
      required:
        - '#daisy-chained-devices'
    else:
      properties:
        '#daisy-chained-devices': false

unevaluatedProperties: false

examples:
  - |
    #include <dt-bindings/gpio/gpio.h>
    spi {
        #address-cells =3D <1>;
        #size-cells =3D <0>;
        adc@0 {
            compatible =3D "adi,ad4020";
            reg =3D <0>;
            spi-max-frequency =3D <71000000>;
            vdd-supply =3D <&supply_1_8V>;
            vio-supply =3D <&supply_1_8V>;
            ref-supply =3D <&supply_5V>;
            cnv-gpios =3D <&gpio0 88 GPIO_ACTIVE_HIGH>;
        };
    };