Re: [PATCH v2 1/2] dt-bindings: iio: adc: Add Nuvoton MA35D1 EADC

[Jonathan Cameron]

On Mon, 13 Jul 2026 16:11:26 +0800
Chi-Wen Weng <cwweng.linux@xxxxxxxxx> wrote:

> From: Chi-Wen Weng <cwweng@xxxxxxxxxxx>
> 
> Add devicetree binding for the Enhanced ADC controller found on
> Nuvoton MA35D1 SoCs.
> 
> The controller has one register region, up to four end-of-conversion
> interrupts, one functional clock, and an optional reset line. The
> binding also describes the optional external reference supply and the
> optional DMA request supported by the hardware.
> 
> ADC input pins are described using standard channel child nodes. The
> external ADC input channels are 0 to 7. Differential inputs are described
> with diff-channels and are limited to the fixed hardware pairs.
> 
> Signed-off-by: Chi-Wen Weng <cwweng@xxxxxxxxxxx>

Hi, A few comments below,

> ---
>  .../bindings/iio/adc/nuvoton,ma35d1-eadc.yaml | 163 ++++++++++++++++++
>  1 file changed, 163 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml
> new file mode 100644
> index 000000000000..1be5470643d1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml
...

> +  interrupts:
> +    minItems: 1
> +    maxItems: 4
> +    items:
> +      - description: ADINT0 end-of-conversion interrupt
> +      - description: ADINT1 end-of-conversion interrupt
> +      - description: ADINT2 end-of-conversion interrupt
> +      - description: ADINT3 end-of-conversion interrupt
Not sure this doc helps. What are these interrupts for?
The driver only uses one of them so why are there 4?

May well need interrupt-names to allow gaps in the list to
work but hard to tell without more information.


> +
> +patternProperties:
> +  '^channel@[0-7]$':
> +    type: object
> +    $ref: adc.yaml
> +    unevaluatedProperties: false
> +
> +    properties:
> +      reg:
> +        maximum: 7
> +
> +      diff-channels: true
Do you need that explicitly. I think adc.yaml already brings that in.

> +
> +    allOf:
> +      - if:
> +          properties:
> +            reg:
> +              const: 0
> +        then:
> +          properties:
> +            diff-channels:
> +              items:
> +                - const: 0
> +                - const: 4
> +      - if:
> +          properties:
> +            reg:
> +              const: 1
> +        then:
> +          properties:
> +            diff-channels:
> +              items:
> +                - const: 1
> +                - const: 5
> +      - if:
> +          properties:
> +            reg:
> +              const: 2
> +        then:
> +          properties:
> +            diff-channels:
> +              items:
> +                - const: 2
> +                - const: 6
> +      - if:
> +          properties:
> +            reg:
> +              const: 3
> +        then:
> +          properties:
> +            diff-channels:

For these take a look at the description of diff-channels.
It states that where the pairs are hard wired you can just use reg
and not specify diff-channels.  Seems to apply here and will greatly
simplify this binding.

> +              items:
> +                - const: 3
> +                - const: 7
> +      - if:
> +          properties:
> +            reg:
> +              enum: [4, 5, 6, 7]

What are these channels?  There is a reference to one
internally wired one so what are the other 3?


> +        then:
> +          not:
> +            required:
> +              - diff-channels
> +
> +    required:
> +      - reg