Re: [PATCH v9 2/3] dt-bindings: iio: dac: Add AD5529R

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] The `spi-device-addr` property specifies a default value of integer `0`, but it is typed as a `uint32-array` in standard SPI bindings, causing schema validation to fail.
- [Low] The `default` values for `output-range-microvolt` are incorrectly placed inside the tuple `items` definition, rendering them ineffective in Device Tree schemas.
- [Low] The child node schema uses `additionalProperties: false` alongside a `$ref: dac.yaml` reference, which improperly rejects inherited standard properties like `label`.
--

--- Patch [2]: [PATCH v9 2/3] dt-bindings: iio: dac: Add AD5529R ---
commit 8e1fecbc9f92b4d6893726d1c141e8c978f52489
Author: Janani Sunil <janani.sunil@analog.com>

dt-bindings: iio: dac: Add AD5529R

Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
buffered voltage output digital-to-analog converter (DAC) with an
integrated precision reference.

> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml
> new file mode 100644
> index 0000000000000..9c390d460e5ff
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5529r.yaml

[ ... ]

> +  spi-device-addr:
> +    minItems: 1
> +    maxItems: 4
> +    items:
> +      enum: [0, 1, 2, 3]
> +    default: 0

[Severity: Low]
Does this default value cause schema validation to fail?

Since this property can contain multiple items, standard SPI bindings
often type these as a uint32-array. Should the default be formatted
as an array like default: [0] instead of a scalar integer 0?

> +    description:
> +      Hardware address of each device, selected by the ID0 and ID1 pins.
> +      Up to four AD5529R devices can share a single SPI chip select.

[ ... ]

> +  "^channel@([0-9a-f]{1,2})$":
> +    $ref: dac.yaml
> +    type: object
> +    description: Child nodes for individual channel configuration
> +
> +    properties:
> +      reg:
> +        description:
> +          Flattened channel number across all AD5529R devices sharing the chip
> +          select. Within each 16-channel block, the physical channel number is reg % 16.
> +        minimum: 0
> +        maximum: 63
> +
> +      output-range-microvolt:
> +        description:
> +          Output voltage range for this channel as [min, max] in
> +          microvolts.
> +        oneOf:
> +          - items:
> +              - const: 0
> +                default: 0
> +              - enum: [5000000, 10000000, 20000000, 40000000]
> +                default: 5000000

[Severity: Low]
Are these default values correctly placed?

Placing the default keyword inside the tuple items definition generally
renders them ineffective in Device Tree schemas when the property is
entirely omitted.

Should the default be applied to the property as a whole instead?

> +          - items:
> +              - const: -5000000
> +              - const: 5000000

[ ... ]

> +    required:
> +      - reg
> +
> +    additionalProperties: false

[Severity: Low]
Will this additionalProperties: false constraint improperly reject standard
properties?

Since this child node includes a $ref: dac.yaml reference earlier in the
block, this constraint typically prevents the use of inherited properties
like label and causes schema validation errors.

Should this be unevaluatedProperties: false instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820-ad5529r-driver-v9-0-ba62e0b2a816@analog.com?part=2