Re: [PATCH v12 1/2] dt-bindings: adc: add AD7173

From: David Lechner
Date: Thu Jan 18 2024 - 17:43:23 EST


On Thu, Jan 18, 2024 at 6:50 AM Dumitru Ceclan <mitrutzceclan@gmailcom> wrote:
>
> The AD7173 family offer a complete integrated Sigma-Delta ADC solution
> which can be used in high precision, low noise single channel applications
> or higher speed multiplexed applications. The Sigma-Delta ADC is intended
> primarily for measurement of signals close to DC but also delivers
> outstanding performance with input bandwidths out to ~10kHz.
>
> Signed-off-by: Dumitru Ceclan <mitrutzceclan@xxxxxxxxx>
> ---
>
> V11->V12
> - Drop "binding", describe hardware in binding description
> - Rename refin and refin2 to vref and vref2
> - Add better description to external references to better show that the voltage
> value that needs to be specified is the difference between the positive and
> negative reference pins
> - Add optional clocks properties
> - Add optional adi,clock-select property
> - Add option for second interrupt, error
> - Add description to interrupts
> V10->V11
> - Fix example warning: '#gpio-cells' is a dependency of 'gpio-controller'
> - Add description to #gpio-cells property
> V9->V10
> - Fix dt_binding_check type warning from adi,reference-select
> V8->v9
> - Add gpio-controller and "#gpio-cells" properties
> - Add missing avdd2 and iovdd supplies
> - Add string type to reference-select
> V7->V8
> - include missing fix from V6
> V6->V7 <no changes>
> V5->V6
> - Moved global required property to proper placement
> V4 -> V5
> - Use string enum instead of integers for "adi,reference-select"
> - Fix conditional checking in regards to compatible
> V3 -> V4
> - include supply attributes
> - add channel attribute for selecting conversion reference
> V2 -> V3
> - remove redundant descriptions
> - use referenced 'bipolar' property
> - remove newlines from example
> V1 -> V2 <no changes>
>
> .../bindings/iio/adc/adi,ad7173.yaml | 242 ++++++++++++++++++
> 1 file changed, 242 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> new file mode 100644
> index 000000000000..4d0870cc014c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7173.yaml
> @@ -0,0 +1,242 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2023 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad7173.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD7173 ADC
> +
> +maintainers:
> + - Ceclan Dumitru <dumitru.ceclan@xxxxxxxxxx>
> +
> +description: |
> + Analog Devices AD717x ADC's:
> + The AD717x family offer a complete integrated Sigma-Delta ADC solution which
> + can be used in high precision, low noise single channel applications
> + (Life Science measurements) or higher speed multiplexed applications
> + (Factory Automation PLC Input modules). The Sigma-Delta ADC is intended
> + primarily for measurement of signals close to DC but also delivers
> + outstanding performance with input bandwidths out to ~10kHz.
> +
> + Datasheets for supported chips:
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7172-2.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7173-8.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7175-2.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7176-2.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - adi,ad7172-2
> + - adi,ad7173-8
> + - adi,ad7175-2
> + - adi,ad7176-2
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + minItems: 1
> + description: |
> + Ready interrupt: multiplexed with SPI data out. While SPI CS is low,
> + can be used to indicate the completion of a conversion.
> +
> + Error: The three error bits in the status register (ADC_ERROR, CRC_ERROR,
> + and REG_ERROR) are OR'ed, inverted, and mapped to the ERROR pin. Therefore,
> + the ERROR pin indicates that an error has occurred.
> +
> + interrupt-names:
> + minItems: 1
> + items:
> + - const: rdy
> + - const: err
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + spi-max-frequency:
> + maximum: 20000000
> +
> + gpio-controller:
> + description: Marks the device node as a GPIO controller.
> +
> + "#gpio-cells":
> + const: 2
> + description:
> + The first cell is the GPIO number and the second cell specifies
> + GPIO flags, as defined in <dt-bindings/gpio/gpio.h>.
> +
> + vref-supply:
> + description: |
> + Differential external reference supply used for conversion. The reference
> + voltage (Vref) specified here must be the voltage difference between the
> + REF+ and REF- pins: Vref = (REF+) - (REF-).
> +
> + vref2-supply:
> + description: |
> + Differential external reference supply used for conversion. The reference
> + voltage (Vref2) specified here must be the voltage difference between the
> + REF2+ and REF2- pins: Vref2 = (REF2+) - (REF2-).
> +
> + avdd-supply:
> + description: avdd supply, can be used as reference for conversion.

I think it would be helpful to have a description similar to
vref-supply here. This is the voltage between AVDD and AVSS. So in
both cases AVDD=5V, AVSS=0V and AVDD=+2.5V, AVSS=-2.5V, this supply
should report 5V.

> +
> + avdd2-supply:
> + description: avdd2 supply, used as the input to the internal voltage regulator.

This supply is also referenced to AVSS.

> +
> + iovdd-supply:
> + description: iovdd supply, used for the chip digital interface.
> +
> + clocks:
> + maxItems: 1
> + description: |
> + Optional external clock source. Can include one clock source: external
> + clock or external crystal.
> +
> + clock-names:
> + enum:
> + - ext-clk
> + - xtal
> +
> + adi,clock-select:
> + description: |
> + Select the ADC clock source. Valid values are:
> + int : Internal oscillator
> + int-out : Internal oscillator with output on XTAL2 pin
> + ext-clk : External clock input on XTAL2 pin
> + xtal : External crystal on XTAL1 and XTAL2 pins
> +
> + $ref: /schemas/types.yaml#/definitions/string
> + enum:
> + - int
> + - int-out
> + - ext-clk
> + - xtal
> + default: int
> +
> +patternProperties:
> + "^channel@[0-9a-f]$":
> + type: object
> + $ref: adc.yaml
> + unevaluatedProperties: false
> +
> + properties:
> + reg:
> + minimum: 0
> + maximum: 15
> +
> + diff-channels:
> + items:
> + minimum: 0
> + maximum: 31
> +
> + adi,reference-select:
> + description: |
> + Select the reference source to use when converting on
> + the specific channel. Valid values are:
> + vref : REF+ /REF−
> + vref2 : REF2+ /REF2−
> + refout-avss: REFOUT/AVSS (Internal reference)
> + avdd : AVDD

Could write this as AVDD/AVSS to be consistent with the other 3 options.

(Or if this is really AVDD to 0V, we may need to reconsider some of
our other decisions.)

> +
> + External reference ref2 only available on ad7173-8.
> + If not specified, internal reference used.
> + $ref: /schemas/types.yaml#/definitions/string
> + enum:
> + - vref
> + - vref2
> + - refout-avss
> + - avdd
> + default: refout-avss
> +
> + required:
> + - reg
> + - diff-channels
> +
> +required:
> + - compatible
> + - reg
> +
> +allOf:
> + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> + - if:
> + properties:
> + compatible:
> + not:
> + contains:
> + const: adi,ad7173-8
> + then:
> + properties:
> + vref2-supply: false
> + patternProperties:
> + "^channel@[0-9a-f]$":
> + properties:
> + adi,reference-select:
> + enum:
> + - vref
> + - refout-avss
> + - avdd
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@0 {
> + compatible = "adi,ad7173-8";
> + reg = <0>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
> + interrupt-names = "rdy";
> + interrupt-parent = <&gpio>;
> + spi-max-frequency = <5000000>;
> + gpio-controller;
> + #gpio-cells = <2>;
> +
> + vref-supply = <&dummy_regulator>;
> +
> + channel@0 {
> + reg = <0>;
> + bipolar;
> + diff-channels = <0 1>;
> + adi,reference-select = "vref";
> + };
> +
> + channel@1 {
> + reg = <1>;
> + diff-channels = <2 3>;
> + };
> +
> + channel@2 {
> + reg = <2>;
> + bipolar;
> + diff-channels = <4 5>;
> + };
> +
> + channel@3 {
> + reg = <3>;
> + bipolar;
> + diff-channels = <6 7>;
> + };
> +
> + channel@4 {
> + reg = <4>;
> + diff-channels = <8 9>;
> + adi,reference-select = "avdd";
> + };
> + };
> + };
> --
> 2.42.0
>