Re: [PATCH 1/6] dt-bindings: iio: adc: Add AD7768

[David Lechner]

On 7/9/26 3:50 AM, Janani Sunil wrote:
> Devicetree Bindings for AD7768-4 (4 channel) and AD7768 (8 channel)
> simultaneous sampling ADC
> 
> Signed-off-by: Janani Sunil <janani.sunil@xxxxxxxxxx>
> ---
>  .../devicetree/bindings/iio/adc/adi,ad7768.yaml    | 285 +++++++++++++++++++++
>  MAINTAINERS                                        |   7 +
>  2 files changed, 292 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7768.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7768.yaml
> new file mode 100644
> index 000000000000..b74fe6aef01c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7768.yaml
> @@ -0,0 +1,285 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad7768.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD7768 and AD7768-4 ADC
> +
> +maintainers:
> +  - Janani Sunil <janani.sunil@xxxxxxxxxx>
> +
> +description: |
> +  The AD7768 is a 8-channel, 24-bit simultaneous sampling ADC with configurable
> +  power and performance modes. The AD7768-4 is a 4-channel version.

Probably worth mentioning that this binding is for SPI mode, not for pin control
mode.

And that the io-backend is for the data output interface, so consumes the DOUTx,
DCLK, DRDY. Maybe also START? And SYNC_{IN,OUT}?

> +
> +  Datasheet at:
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad7768.pdf

A link to the example HDL project for the io-backend would be helpful too.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad7768
> +      - adi,ad7768-4
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +    description: Master clock (MCLK)

Could also be XTAL{1,2}.

> +
> +  avdd-supply:
> +    description: Analog power supply AVDD1 (4.5V to 5.5V)
> +
> +  avss-supply:
> +    description: Analog ground/negative supply AVSS (0V to -2.75V)
> +

What about AV{DD,SS}{1,2}{A,B} supplies?

> +  dvdd-supply:
> +    description: Analog power supply AVDD2 (2.0V to 5.5V)
> +
> +  iovdd-supply:
> +    description: Digital I/O power supply (1.8V or 2.25V to 3.6V)
> +
> +  vref-supply:
> +    description: ADC reference voltage supply

There is more that one reference voltage input. so ref1-supply, ref2-supply.

vref is the name of an internal signal, so doesn't make sense here.

> +
> +  reset-gpios:
> +    maxItems: 1
> +    description: GPIO connected to the active-low RESET pin
> +
> +  gpio-controller: true
> +
> +  '#gpio-cells':
> +    const: 2
> +
> +  adi,data-lines-number:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [1, 2, 4, 8]
> +    description:
> +      Number of data output lines used for serial interface.

This could be made a bit more clear that this is a secondary data
output interface, not the SPI lines.

> +      AD7768 supports 1, 2, or 8 lines. AD7768-4 supports 1 or 4 lines.
> +
> +  adi,common-mode-output:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum:
> +      - avdd-avss-half
> +      - 1.65V
> +      - 2.5V
> +      - 2.14V
> +    description:
> +      Common mode voltage output selection.

Why not using standard regulator provider bindings for this?

> +
> +  adi,vcm-power-down:
> +    type: boolean
> +    description: Power down the common mode output buffer

Is the buffer separate from the output? In that case I would expect
buffer to be in the property name, otherwise this should just be
part of the enum options above (and the default one at that).

> +
> +  adi,power-mode:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum:
> +      - low
> +      - median
> +      - fast
> +    description:
> +      Power mode selection.

Unless there are pins that control this, it seems like it should be
left up to the driver to decide how to set this.

In this case, it looks like the power mode also influences sample rate
which is normally something controlled at runtime.

> +
> +  io-backends:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +patternProperties:
> +  "^channel@[0-7]$":
> +    type: object
> +    description: |
> +      Represents the external channels which are connected to the device.
> +      AD7768 supports channels 0-7, AD7768-4 supports channels 0-3.
> +
> +    properties:
> +      reg:
> +        minimum: 0
> +        maximum: 7
> +        description: The channel number
> +
> +      adi,ch-mode:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        minimum: 0
> +        maximum: 1
> +        description: |
> +          Channel mode selection. The AD7768 supports two independent
> +          configuration profiles (Mode A and Mode B) for filter and
> +          decimation settings. Each channel can be assigned to either mode:
> +            0 - Channel uses Mode A filter and decimation settings
> +            1 - Channel uses Mode B filter and decimation settings

This could probably be handled in the driver. E.g. allow per-channel settings
that just get stored in a data structure. Then when starting a buffered read
check that all enabled channels have at most 2 different groups of settings.
Then do all of the required register config at that time.

> +
> +      adi,prebuf-pos-en:
> +        type: boolean
> +        description: Enable positive input precharge buffer
> +
> +      adi,prebuf-neg-en:
> +        type: boolean
> +        description: Enable negative input precharge buffer
> +
> +      adi,refbuf-pos-en:
> +        type: boolean
> +        description: Enable positive reference buffer
> +
> +      adi,refbuf-neg-en:
> +        type: boolean
> +        description: Enable negative reference buffer
> +
> +    required:
> +      - reg
> +      - adi,ch-mode
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - avdd-supply
> +  - avss-supply
> +  - dvdd-supply
> +  - iovdd-supply
> +  - vref-supply

> +  - adi,data-lines-number

Suggest making the highest number of lines the default instead
of making this property required.


> +  - adi,common-mode-output

Should probably default to disabled, unless it is always used?
Changing it to a regulator provider makes this not relevant.

> +  - io-backends
> +