Re: [PATCH v1 1/6] dt-bindings: leds: Document TI LM3533 LED controller

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] The top-level `ti,lm3533.yaml` mandates the presence of all sub-nodes through the `required` list, which is an anti-pattern for optional hardware blocks.
- [High] The `ti,lm3533-backlight.yaml` schema uses standard LED properties (`led-sources` and `led-max-microamp`) without defining their types or pulling them in from the core LED schema.
- [High] The `light-sensor` node will fail `dt_binding_check` because `additionalProperties: false` rejects standard properties like `interrupt-parent`.
- [Medium] The `led-sources` item constraint in `ti,lm3533-leds.yaml` allows an out-of-bounds index due to an off-by-one error.
- [Medium] The parent node `ti,lm3533.yaml` enforces child nodes with unit addresses but fails to require `#address-cells` and `#size-cells`.
--

commit 79de640d527303f53ad8d32d99e560b245a95df7
Author: Svyatoslav Ryhel <clamor95@gmail.com>

dt-bindings: leds: Document TI LM3533 LED controller

Document the LM3533 - a complete power source for backlight, keypad and
indicator LEDs in smartphone handsets.

> diff --git a/Documentation/devicetree/bindings/leds/backlight/ti,lm3533-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/ti,lm3533-backlight.yaml
> new file mode 100644
> index 0000000000000..866b0fb8ed047
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/backlight/ti,lm3533-backlight.yaml
> @@ -0,0 +1,68 @@
[ ... ]
> +allOf:
> +  - $ref: /schemas/leds/backlight/common.yaml#
> +
> +properties:
> +  compatible:
> +    const: ti,lm3533-backlight
> +
> +  reg:
> +    description: Control bank selection (0 = bank A, 1 = bank B).
> +    maximum: 1
> +
> +  led-max-microamp:
> +    description: maximum current in uA with a 800 uA step.
> +    minimum: 5000
> +    maximum: 29800
> +    default: 5000
> +
> +  led-sources:

The common backlight schema does not define types for led-sources or
led-max-microamp. Since this schema neither includes leds/common.yaml nor
provides explicit type references, will this lack strict type validation and
potentially cause dt_binding_check to fail? Should this include a reference
to /schemas/leds/common.yaml#?

[ ... ]
> diff --git a/Documentation/devicetree/bindings/leds/ti,lm3533-leds.yaml b/Documentation/devicetree/bindings/leds/ti,lm3533-leds.yaml
> new file mode 100644
> index 0000000000000..a582d7d2f955b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/ti,lm3533-leds.yaml
> @@ -0,0 +1,66 @@
[ ... ]
> +  led-sources:
> +    description: |
> +      LVLED associated with this control bank. May be more than 1 source per bank.
> +        0 - LVLED1
> +        1 - LVLED2
> +        2 - LVLED3
> +        3 - LVLED4
> +        4 - LVLED5
> +    minItems: 1
> +    maxItems: 5
> +    items:
> +      maximum: 5

The description explicitly defines the valid LVLED source indices as 0
through 4. Does the maximum constraint of 5 allow an invalid index of 5 to
pass schema validation? Should it be restricted to maximum: 4?

[ ... ]
> diff --git a/Documentation/devicetree/bindings/leds/ti,lm3533.yaml b/Documentation/devicetree/bindings/leds/ti,lm3533.yaml
> new file mode 100644
> index 0000000000000..2e200f1724004
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/ti,lm3533.yaml
> @@ -0,0 +1,190 @@
[ ... ]
> +  light-sensor:
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      compatible:
> +        const: ti,lm3533-als
> +
> +      interrupts:
> +        maxItems: 1

This object uses additionalProperties: false but does not list standard
properties like interrupt-parent in its properties block. Will this cause
schema validation to fail when processing device trees that use
interrupt-parent, as shown in the example later in this binding?

[ ... ]
> +required:
> +  - compatible
> +  - reg
> +  - light-sensor
> +  - backlight@0
> +  - backlight@1
> +  - led@2
> +  - led@3
> +  - led@4
> +  - led@5

Are all of these sub-nodes physically required for every device tree?

Including backlight@0, backlight@1, led@2 through led@5, and light-sensor in
the required list forces every device tree to instantiate them even if they
are unconnected or unused on the board.

Since this parent node hosts multiple subnodes that utilize the reg property,
shouldn't #address-cells and #size-cells also be included in the required
array to ensure they are always present when child nodes are defined?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260517074306.30937-1-clamor95@gmail.com?part=1