Re: [PATCH 1/2] dt-bindings: iio: adc: Add TI ADS1220

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3350 bytes --]

On Wed, Jun 10, 2026 at 03:29:02PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] The `diff-channels` property constraint uses invalid YAML flow sequence syntax (`items: [const: 0, const: 1]`) that will cause a fatal parser error or evaluate to a single-element list with a collapsed duplicate dictionary key.
> - [Medium] The child node regex and `reg` maximum artificially limit the device tree to defining at most 7 channels, despite the hardware supporting 12 unique multiplexer configurations.
> --
> 
> commit 2b3665a0096105a4bb2db6266e47dec0a0802cff
> Author: Nguyen Minh Tien <zizuzacker@gmail.com>
> 
> dt-bindings: iio: adc: Add TI ADS1220
> 
> This commit adds a device tree binding for the TI ADS1220 24-bit ADC.
> It describes the SPI device, its per-input channel child nodes,
> external reference selection, and the DRDY interrupt.
> 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1220.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1220.yaml
> > new file mode 100644
> > index 0000000000000..1fedffc2a1475
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1220.yaml
> [ ... ]
> > +patternProperties:
> > +  "^channel@[0-6]$":
> > +    $ref: adc.yaml
> > +    type: object
> > +    description: Represents one ADC input configuration (channel).
> > +
> > +    properties:
> > +      reg:
> > +        minimum: 0
> > +        maximum: 6
> 
> [Severity: Medium]
> The ADS1220 hardware supports 12 unique multiplexer combinations
> (8 differential pairs and 4 single-ended inputs). Does this regex and
> reg maximum artificially limit the device tree to defining at most 7
> channels?
> 
> System integrators might not be able to represent all available
> hardware configurations if this limit is enforced.
> 
> [ ... ]
> > +      diff-channels:
> > +        description:
> > +          Differential input pair routable by the ADS1220 multiplexer.
> > +        oneOf:
> > +          - items: [const: 0, const: 1]
> > +          - items: [const: 0, const: 2]
> > +          - items: [const: 0, const: 3]
> > +          - items: [const: 1, const: 2]
> > +          - items: [const: 1, const: 3]
> > +          - items: [const: 2, const: 3]
> > +          - items: [const: 1, const: 0]
> > +          - items: [const: 3, const: 2]
> 
> [Severity: High]
> Will this syntax pass the dt_binding_check validation?
> 
> In YAML, a flow sequence like [const: 0, const: 1] evaluates to a list
> containing a single implicit mapping with duplicate keys. The dt-schema
> toolchain utilizes a strict parser that typically fails on duplicate
> dictionary keys.
> 
> If it did parse, it would likely collapse the duplicate keys into a
> single element, forcing the diff-channels array to have exactly one
> element instead of the expected two.
> 
> Would it be better to use a valid flow mapping sequence, such as
> - items: [{const: 0}, {const: 1}]?

dt_binding_check passes actually. Although this syntax has no other
users in the tree. The only other diff-channel user I saw that applied
as tight restrictions as this one spelt it out as
  items:
    - const: 0
    - const: 1
etc but I have no objection to this if it works.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]