Re: [PATCH v3 1/2] dt-bindings: iio: dac: add docs for ad8460

[Krzysztof Kozlowski]

On Wed, Sep 04, 2024 at 10:30:39AM +0800, Mariel Tinaco wrote:
> This adds the bindings documentation for the 14-bit

Please do not use "This commit/patch/change", but imperative mood. See
longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> High Voltage, High Current, Waveform Generator
> Digital-to-Analog converter.
> 
> Signed-off-by: Mariel Tinaco <Mariel.Tinaco@xxxxxxxxxx>
> ---
>  .../bindings/iio/dac/adi,ad8460.yaml          | 154 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 161 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/dac/adi,ad8460.yaml

> +  adi,range-microvolt:
> +    description: Voltage output range specified as <minimum, maximum>
> +    oneOf:

This oneOf does not make sense. There is only one condition. Drop.

> +      - items:
> +          - enum: [0, -10000000, -20000000, -30000000, -40000000, -55000000]
> +          - enum: [10000000, 20000000, 30000000, 40000000, 55000000]

What's the default? It's not a required property.

> +
> +  adi,range-microamp:
> +    description: Current output range specified as <minimum, maximum>
> +    oneOf:
> +      - items:
> +          - enum: [-50000, -100000, -300000, -500000, -1000000]

I don't understand why 0 is not listed here.

> +          - enum: [50000, 100000, 300000, 500000, 1000000]
> +      - items:
> +          - const: 0
> +          - enum: [50000, 100000, 300000, 500000, 1000000]
> +

What's the default? It's not a required property.

> +  adi,max-millicelsius:
> +    description: Overtemperature threshold
> +    default: 50000
> +    minimum: 20000
> +    maximum: 150000
> +
> +  shutdown-reset-gpios:
> +    description: Corresponds to SDN_RESET pin. To exit shutdown
> +      or sleep mode, pulse SDN_RESET HIGH, then leave LOW.
> +    maxItems: 1
> +
> +  reset-gpios:
> +    description: Manual Power On Reset (POR). Pull this GPIO pin
> +      LOW and then HIGH to reset all digital registers to default
> +    maxItems: 1
> +
> +  shutdown-gpios:
> +    description: Corresponds to SDN_IO pin. Shutdown may be
> +      initiated by the user, by pulsing SDN_IO high. To exit shutdown,
> +      pulse SDN_IO low, then float.
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks

Some supplies are for sure required. Devices rarely can operate without
power provided.

> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +additionalProperties: false

unevaluatedProperties instead.

Best regards,
Krzysztof