Re: [PATCH v13 1/2] dt-bindings: leds: Add LTC3220 18 channel LED Driver
| 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