Re: [PATCH v13 1/2] dt-bindings: leds: Add LTC3220 18 channel LED Driver

[email protected]
Newsgroups org.kernel.vger.linux-leds,org.kernel.vger.linux-devicetree
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Incorrect Property Constraints for `reg` causing ineffective bounds validation
- [Medium] Missing array bounds and value constraints for `led-sources` property
- [Medium] Encoding driver software limitations into Device Tree schema for aggregated LEDs
--

--- Patch [1]: [PATCH v13 1/2] dt-bindings: leds: Add LTC3220 18 channel LED Driver ---
commit 4d1504d2ccfbbb8ac2eccd2728a2c5c976fd4d95
Author: Edelweise Escala <[email protected]>

dt-bindings: leds: Add LTC3220 18 channel LED Driver

This commit adds device tree binding documentation for the Analog Devices
LTC3220 LED driver. The binding specifies properties for independent and
aggregated LED control across its 18 channels.

> diff --git a/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml b/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml
> new file mode 100644
> index 0000000000000..48215ad82d9a7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/adi,ltc3220.yaml

[ ... ]

> +patternProperties:
> +  '^led@([1-9a-f]|1[0-2])$':
> +    type: object
> +    $ref: /schemas/leds/common.yaml#
> +    unevaluatedProperties: false
> +    properties:

[Severity: Medium]
Should led-sources be explicitly constrained here?

Since it is inherited from common.yaml without local bounds, a device tree
could potentially define an array of more than 18 channels or use
out-of-bounds indices and still pass validation.

> +      reg:
> +        description:
> +          Output channel for the LED (1-18 maps to LED outputs D1-D18).
> +          Unit-address must be in hexadecimal (1-12 hex = 1-18 decimal).
> +          For aggregated LED control, define only one LED node with reg = <0x1>
> +          and use led-sources to list all controlled outputs. Only reg 1 should
> +          be present when using led-sources.

[Severity: Medium]
Does restricting reg to 1 when led-sources is present encode a software driver
limitation into the hardware description?

If the hardware allows grouping the 18 channels into multiple discrete
physical LEDs (e.g., LED A on channels 1+2, LED B on channels 3+4), this
schema restriction would prevent valid hardware topologies from being
described.

> +        items:
> +          - minimum: 1
> +            maximum: 18

[Severity: Medium]
Are these minimum and maximum constraints properly evaluated for reg arrays?

By nesting them inside an items array list, the schema applies the numeric
constraints to the array itself, which JSON Schema ignores, silently allowing
out-of-bounds values. Placing them directly under reg allows dt-schema to
correctly validate the integers.

> +
> +    required:
> +      - reg
> +
> +    if:
> +      required:
> +        - led-sources
> +    then:
> +      properties:
> +        reg:
> +          items:
> +            - const: 1

[Severity: Medium]
Does applying const: 1 inside an items list for the reg property fail
unconditionally on arrays?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.