Re: [PATCH v3 01/10] dt-bindings: iio: adc: Add AD4170

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 13 May 2025 09:33:40 -0300
Marcelo Schmitt <marcelo.schmitt@analog.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@analog.com>
Hi Marcelo,

A few additional comments from me.

J
> ---
> Change log v2 -> v3
> 
> [device tree changes]
> - Removed unneeded allOf.
> - Removed occurences of adi,sensor-type type re-declaration.
> - Created type for the AD4170 channels, allowing to avoid dt doc repetition.
> 
>  .../bindings/iio/adc/adi,ad4170.yaml          | 544 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 551 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..0a06258b6631
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4170.yaml
> @@ -0,0 +1,544 @@

> +  sensor-node:
...
> +
> +      adi,sensor-type:
> +        description: |
> +          Type of sensor connected to the device. Depending on the sensor type
> +          (weigh scale, RTD, or thermocouple) the values of sensor-node
> +          properties have slightly different constraints. This property
> +          specifies which particular external sensor is connected to the ADC so
> +          the sensor-node properties can be properly parsed and verified. The
> +          possible sensor types are:
> +          0: weigh scale;
> +          1: RTD;
> +          2: thermocouple.
> +        $ref: /schemas/types.yaml#/definitions/uint8

I think this should be an enum of strings.  That will give us a clean
way to extend it for other sensor types in future.

> +    required:
> +      - reg
> +      - diff-channels
> +      - bipolar
> +      - adi,sensor-type
> +      - adi,reference-select
> +
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ad4170
> +      - adi,ad4190
> +      - adi,ad4195
>
> +
> +  interrupts:
> +    maxItems: 1

What if they are both wired?  At that point shouldn't software
make up it's mind which to use?

> +
> +  interrupt-names:
> +    description:
> +      Specify which pin should be configured as Data Ready interrupt.
> +    enum:
> +      - sdo
> +      - dig_aux1
> +    default: sdo
> +