Message-ID: <20250412170737.2789c5be@jic23-huawei>
Date: Sat, 12 Apr 2025 17:07:37 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Marcelo Schmitt <marcelo.schmitt@...log.com>
Cc: <linux-iio@...r.kernel.org>, <devicetree@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>, <lars@...afoo.de>,
 <Michael.Hennerich@...log.com>, <dlechner@...libre.com>,
 <nuno.sa@...log.com>, <andy@...nel.org>, <robh@...nel.org>,
 <krzk+dt@...nel.org>, <conor+dt@...nel.org>, <marcelo.schmitt1@...il.com>
Subject: Re: [PATCH v1 1/7] dt-bindings: iio: adc: Add AD4170

On Wed, 9 Apr 2025 09:24:18 -0300
Marcelo Schmitt <marcelo.schmitt@...log.com> wrote:

> Add device tree documentation for AD4170 and similar sigma-delta ADCs.
> The AD4170 is a 24-bit, multichannel, sigma-delta ADC.
> 
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@...log.com>
> ---
> The AD4170 design has features to aid interfacing with weigh scale and RTD
> sensors that are expected to be setup with external circuitry for proper
> sensor operation. A key characteristic of those sensors is that the circuit
> they are in must be excited with a pair of signals. The external circuit
> can be excited either by voltage supply or by AD4170 excitation signals.
> The sensor can then be read through a different pair of lines that are
> connected to AD4170 ADC.
> 
>  .../bindings/iio/adc/adi,ad4170.yaml          | 527 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 534 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml
> new file mode 100644
> index 000000000000..93fe3b4648a0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml
> @@ -0,0 +1,527 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/adi,ad4170.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices AD4170 and similar Analog to Digital Converters
> +
> +maintainers:
> +  - Marcelo Schmitt <marcelo.schmitt@...log.com>
> +
> +description: |
> +  Analog Devices AD4170 series of Sigma-delta Analog to Digital Converters.
> +  Specifications can be found at:
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4170-4.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4190-4.pdf
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4195-4.pdf
> +
> +$ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +$defs:
> +  sensor-node:
> +    type: object
> +    description: |
> +      Common properties of external sensor circuitry connected to the ADC.
> +
> +    properties:
> +      reg:
> +        description:
> +          Channel number. Connects the sensor to the channel with this number
> +          of the device.
> +        minimum: 1
> +        maximum: 16
> +
> +      diff-channels:

Maybe add some comments to the file. I was very confused on what this def
is for, but now I think it's just an adaption of an ADC channel to a specific
sensor type.

> +        $ref: /schemas/types.yaml#/definitions/uint32-array
> +        maxItems: 2
> +        minItems: 2
> +        description: |
> +          ADC analog input pins to which the sensor circuit is connected.
> +          The first value specifies the positive input pin, the second
> +          specifies the negative input pin. See adc.yaml for details.
> +
> +      bipolar:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: If provided, the channel is to be used in bipolar mode.
> +
> +      adi,sensor-type:
> +        description: Type of sensor connected to the device.
> +        $ref: /schemas/types.yaml#/definitions/uint8
> +
> +      adi,ac-excited:

All the existing excitation related bindings are
adi,excitation-*

Maybe adi,excitation-ac for this one?

> +        type: boolean
> +        description: |
> +          Whether the external circuit has to be AC or DC excited.

Hmm. The concept of a DC excitation source confused me a bit, but I guess
that's just a current source. 

> +
> +      adi,excitation-pins:
> +        $ref: /schemas/types.yaml#/definitions/uint32-array
> +        description: |
> +          ADC pins used for external circuit excitation. Some applications
> +          require optimum matching between excitation currents. Using excitation
> +          current pairs minimizes the excitation current mismatch and the
> +          excitation current drift matching on the ADC. Must describe either 1
> +          or 2 pairs of pins. E.g. <0 1>; <2 3>; <0 1>, <2 3>.
> +
> +      adi,excitation-current-microamp:
We have an existing rtd specific version of this but I guess it can be used for other
things with this chip so fair enough to generalize it.


> +        description: |
> +          Excitation current in microamperes to be output to each excitation pin
> +          specified by adi,excitation-pins property.
> +        enum: [0, 10, 50, 100, 250, 500, 1000, 1500]
> +        default: 0
> +
> +      adi,reference-select:
> +        description: |
> +          Select the reference source to use when converting on the specific
> +          channel. Valid values are:
> +          0: Differential reference voltage REFIN+ - REFIN−.
> +          1: Differential reference voltage REFIN2+ - REFIN2−.
> +          2: Internal 2.5V referece (REFOUT) relative to AVSS.
> +          3: Analog supply voltage (AVDD) relative AVSS.
> +          If this field is left empty, the first external reference is selected.
> +        $ref: /schemas/types.yaml#/definitions/uint8
> +        enum: [0, 1, 2, 3]
> +        default: 0
> +
> +    required:
> +      - reg
> +      - diff-channels
> +      - bipolar
> +      - adi,sensor-type
> +      - adi,excitation-pins
> +      - adi,reference-select

If it is required, why specify defaults above?

> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad4170
> +      - adi,ad4190
> +      - adi,ad4195
> +

> +patternProperties:
> +  "^channel@[0-9a-f]$":
> +    $ref: adc.yaml
> +    type: object
> +    unevaluatedProperties: false
> +    description: |
> +      Represents the external channels which are connected to the ADC.
> +
> +    properties:
> +      reg:
> +        description: |
> +          The channel number.
> +        items:
> +          minimum: 0
> +          maximum: 15
> +
> +      diff-channels:
> +        description: |
> +          This property is used for defining the inputs of a differential
> +          voltage channel. The first value is the positive input and the second
> +          value is the negative input of the channel.
> +
> +          Besides the analog input pins AIN0 to AIN8, there are special inputs
> +          that can be selected with the following values:
> +          17: Internal temperature sensor
> +          18: (AVDD-AVSS)/5
> +          19: (IOVDD-DGND)/5
> +          20: DAC output
> +          21: ALDO
> +          22: DLDO
> +          23: AVSS
> +          24: DGND
> +          25: REFIN+
> +          26: REFIN-
> +          27: REFIN2+
> +          28: REFIN2-
> +          29: REFOUT
> +          For the internal temperature sensor, use the input number for both
> +          inputs (i.e. diff-channels = <17 17>).
> +        items:
> +          enum: [0, 1, 2, 3, 4, 5, 6, 7, 8, 17, 18, 19, 20, 21, 22, 23, 24, 25,
> +                 26, 27, 28, 29]
> +
> +      single-channel: true
> +
> +      common-mode-channel: true
> +
> +      bipolar: true
> +
> +      adi,sensor-type:

This wants a fuller description somewhere in the binding.  What are the possible types?
Also why do we need them here if the sensor-nodes refer to channels?

> +        description: Sensor type for direct ADC sensors.
> +        $ref: /schemas/types.yaml#/definitions/uint8
> +        const: 0
> +
> +      adi,buffered-positive:
> +        description: |
> +          Enable precharge buffer, full buffer, or skip reference buffering of
> +          the positive voltage reference. Because the output impedance of the
> +          source driving the voltage reference inputs may be dynamic, RC
> +          combinations of those inputs can cause DC gain errors if the reference
> +          inputs go unbuffered into the ADC. Enable reference buffering if the
> +          provided reference source has dynamic high impedance output. Note the
> +          absolute voltage allowed on positive reference inputs (REFIN+,
> +          REFIN2+) is from AVSS − 50 mV to AVDD + 50 mV when the reference
> +          buffers are disabled but narrows to AVSS to AVDD when reference
> +          buffering is enabled or in precharge mode.
> +          0: Reference precharge buffer.
> +          1: Full Buffer.
> +          2: Bypass reference buffers (buffering disabled).
I'm not seeing a reason (from this description) to ever pick 0.
Maybe talk about what the precharge buffer alone gets you.

> +        $ref: /schemas/types.yaml#/definitions/uint8
> +        enum: [0, 1, 2]
> +        default: 0
> +
> +      adi,buffered-negative:
> +        description: |
> +          Enable precharge buffer, full buffer, or skip reference buffering of
> +          the negative voltage reference. Because the output impedance of the
> +          source driving the voltage reference inputs may be dynamic, RC
> +          combinations of those inputs can cause DC gain errors if the reference
> +          inputs go unbuffered into the ADC. Enable reference buffering if the
> +          provided reference source has dynamic high impedance output. Note the
> +          absolute voltage allowed on negative reference inputs (REFIN-,
> +          REFIN2-) is from AVSS − 50 mV to AVDD + 50 mV when the reference
> +          buffers are disabled but narrows to AVSS to AVDD when reference
> +          buffering is enabled or in precharge mode.
> +          0: Reference precharge buffer.
> +          1: Full Buffer.
> +          2: Bypass reference buffers (buffering disabled).
> +        $ref: /schemas/types.yaml#/definitions/uint8
> +        enum: [0, 1, 2]
> +        default: 0
> +
> +      adi,reference-select:
> +        description: |
> +          Select the reference source to use when converting on the specific
> +          channel. Valid values are:
> +          0: Differential reference voltage REFIN+ - REFIN−.
> +          1: Differential reference voltage REFIN2+ - REFIN2−.
> +          2: Internal 2.5V referece (REFOUT) relative to AVSS.
> +          3: Analog supply voltage (AVDD) relative AVSS.
> +          If this field is left empty, the internal reference is selected.
> +        $ref: /schemas/types.yaml#/definitions/uint8
> +        enum: [0, 1, 2, 3]
> +        default: 2
> +
> +    required:
> +      - reg
> +
> +    allOf:
> +      - oneOf:
> +          - required: [single-channel]
> +            properties:
> +              diff-channels: false
> +          - required: [diff-channels]
> +            properties:
> +              single-channel: false
> +              common-mode-channel: false
> +
> +  "^weighscale@":
> +    $ref: '#/$defs/sensor-node'
> +    unevaluatedProperties: false
> +
> +    properties:
> +      diff-channels: true
> +      bipolar: true
> +
> +      adi,sensor-type:
> +        description: Weigh scale sensor.
> +        $ref: /schemas/types.yaml#/definitions/uint8
> +        const: 1
> +
> +      adi,excitation-pins:
> +        description: |
> +          ADC pins to use for weigh scale bridge circuit excitation. Must
> +          describe either 1 or 2 pairs of pins. E.g. <0 1>; <2 3>; <0 1>, <2 3>.
> +        $ref: /schemas/types.yaml#/definitions/uint32-array
> +        minItems: 2
> +        maxItems: 4
> +        items:
> +          minimum: 0
> +          maximum: 20
> +
> +      adi,excitation-current-microamp:
> +        description: |
> +          Excitation current in microamperes to be output to each excitation pin
> +          specified by adi,excitation-pins property. If not provided and
> +          adi,ac-excited is true, use predefined ACX1, ACX1 negated, ACX2, and
> +          ACX2 negated signals to AC excite the weigh scale bridge. Those
> +          singals are output on GPIO2, GPIO0, GPIO3, and GPIO1, respectively.
> +        enum: [0, 10, 50, 100, 250, 500, 1000, 1500]
Why the repeat? This description is already in the sensor-node definition.

> +
> +      adi,power-down-switch-pin:
> +        description: |
> +          Number of the GPIO used as power-down switch for the bridge circuit.
> +        $ref: /schemas/types.yaml#/definitions/uint8
> +        enum: [0, 1]
> +

In general, can we reduce repetition of docs across the sensor types so we don't
repeat anything?

J

Powered by blists - more mailing lists