Re: [PATCH v3 08/13] dt-bindings: iio: adc: adi,ad4134: Document SPI connection mode

[Marcelo Schmitt]

On 09/29, sashiko-bot@xxxxxxxxxx wrote:
...
> > @@ -141,13 +161,30 @@ required:
> [ ... ]
> > +  - oneOf:
> > +      - required: [adi,spi-mode, '#daisy-chained-devices']
> > +        properties:
> > +          adi,spi-mode:
> > +            const: one-channel-chain
> > +          spi-rx-bus-width:
> > +            const: 1
> > +      - required: [adi,spi-mode, '#daisy-chained-devices']
> 
> [Severity: Medium]
> Could this regression allow incomplete device trees to pass validation?
> 
> In JSON Schema, property value constraints only apply if the property is
> actually present in the node instance. Without including spi-rx-bus-width
> in the required array for the two-channel-chain branch, a node could omit
> the property entirely and bypass the multi-lane constraint, falling back
> to an incorrect 1-lane default at runtime.
> 
> Should spi-rx-bus-width be added to the required list here?

I'll do so and further test dt_binding_check catches any invalid combination
of properties and values.

> 
> > +        properties:
> > +          adi,spi-mode:
> > +            const: two-channel-chain
> > +          spi-rx-bus-width:
> > +            const: 1, 1
> 
> [Severity: Medium]
> Does this YAML constraint evaluation cause a regression where valid inputs
> are rejected?
> 
> In YAML, const: 1, 1 parses as the literal string "1, 1". Since
> spi-rx-bus-width is defined as a uint32-array, the JSON representation is
> parsed as an array. Comparing an array against a string using const always
> fails.
> 
> This would prevent valid hardware descriptions from passing dt_binding_check
> validation. Should this be formatted as an explicit array to match the type?

Will fix that following the suggestion from Rob's review.

> 
> > +      - properties:
> > +          '#daisy-chained-devices': false
> 
> [Severity: Medium]
> Does this regression in the fallback branch bypass the intended daisy chain
> property enforcement?
> 
> Using oneOf with a fallback that evaluates '#daisy-chained-devices': false
> allows any configuration missing this property to evaluate to true for
> this branch.
> 
> For instance, if adi,spi-mode is set to one-channel-chain but the daisy
> chain property is missing, the schema matches exactly this fallback
> subschema. This causes the oneOf requirement to pass entirely, skipping the
> intended validation.

Will double check that and, if needed, change how the constraints are written.