Re: [PATCH net-next v4 1/5] dt-bindings: leds: Add IS32FL3207 controller

[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 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
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.