Re: [PATCH net-next v4 1/5] dt-bindings: leds: Add IS32FL3207 controller
| Newsgroups | org.kernel.vger.linux-leds,org.kernel.vger.linux-devicetree |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The `multi-led` node constraints arbitrarily limit the maximum base channel to 15, preventing valid configurations like a bicolor LED group on the final two channels (OUT17 and OUT18). -- --- Patch [1]: [PATCH net-next v4 1/5] dt-bindings: leds: Add IS32FL3207 controller --- commit e5368ebd3d680b7d1dca52de92294047f625cbb9 Author: Ahmad Byagowi <[email protected]> dt-bindings: leds: Add IS32FL3207 controller The IS32FL3207 is an 18-channel constant-current LED controller with per-output PWM and current scaling. Describe I2C addressing, power and shutdown controls, RISET, and individual or multicolor output groupings. > diff --git a/Documentation/devicetree/bindings/leds/issi,is32fl3207.yaml b/Documentation/devicetree/bindings/leds/issi,is32fl3207.yaml > new file mode 100644 > --- /dev/null > +++ b/Documentation/devicetree/bindings/leds/issi,is32fl3207.yaml [ ... ] > + "^multi-led@[0-9a-f]$": > + type: object [Severity: Medium] Does this pattern inadvertently reject valid unit addresses above 15? Since the hardware supports 18 independent channels, a bicolor LED placed on channels 16 and 17 (OUT17 and OUT18) would logically need the node name multi-led@10, which this single hex digit pattern would reject. > + $ref: leds-class-multicolor.yaml# > + unevaluatedProperties: false > + > + properties: > + reg: > + minimum: 0 > + maximum: 15 [Severity: Medium] Could this maximum limit be too restrictive? If a bicolor LED group is placed on the final two channels (OUT17 and OUT18), the base channel would need to be 16 (reg = <16>), which is rejected by this limit. > + description: | > + Zero-based lowest-numbered LED output used by the group. Values 0 > + through 15 correspond to OUT1 through OUT16. A group based at > + channel 15 (OUT16) may include channels 16 and 17 (OUT17 and OUT18). [ ... ] > + anyOf: > + - required: [led@0] > + - required: [led@1] > + - required: [led@2] > + - required: [led@3] > + - required: [led@4] > + - required: [led@5] > + - required: [led@6] > + - required: [led@7] > + - required: [led@8] > + - required: [led@9] > + - required: [led@a] > + - required: [led@b] > + - required: [led@c] > + - required: [led@d] > + - required: [led@e] > + - required: [led@f] [Severity: Medium] Are there cases where a multicolor group could be formed exclusively by led@10 and/or led@11? If a bicolor LED group uses the last two outputs, the schema validation would fail because this list stops checking at led@f. -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1