Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Edelweise Escala" <edelweise.escala@analog.com>
Cc: linux-leds@vger.kernel.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org, lee@kernel.org, robh@kernel.org
Subject: Re: [PATCH v13 1/2] dt-bindings: leds: Add LTC3220 18 channel LED Driver
Date: Thu, 09 Jul 2026 00:53:43 +0000	[thread overview]
Message-ID: <20260709005343.DAE521F00A3E@smtp.kernel.org> (raw)
In-Reply-To: <20260709-ltc3220-driver-v13-1-9b53237642eb@analog.com>

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 <edelweise.escala@analog.com>

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/20260709-ltc3220-driver-v13-0-9b53237642eb@analog.com?part=1

  reply	other threads:[~2026-07-09  0:53 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-09  0:41 [PATCH v13 0/2] Add Support for LTC3220 18 Channel LED Driver Edelweise Escala
2026-07-09  0:41 ` [PATCH v13 1/2] dt-bindings: leds: Add LTC3220 18 channel " Edelweise Escala
2026-07-09  0:53   ` sashiko-bot [this message]
2026-07-09  1:12     ` Escala, Edelweise
2026-07-09  0:41 ` [PATCH v13 2/2] leds: ltc3220: Add Support for " Edelweise Escala
2026-07-09  0:55   ` sashiko-bot
2026-07-09  1:07     ` Escala, Edelweise

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260709005343.DAE521F00A3E@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=edelweise.escala@analog.com \
    --cc=lee@kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox