Re: [PATCH v6 1/2] 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 `minimum: 149` constraint on `led-max-microamp` incorrectly rejects valid, lower-current configurations.
--

--- Patch [1]: [PATCH v6 1/2] dt-bindings: leds: Add IS32FL3207 controller ---
commit 4fc20b49cd66c5af5de8db1465fd9c2b64f3c2ad
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.

> diff --git a/Documentation/devicetree/bindings/leds/issi,is32fl3207.yaml b/Documentation/devicetree/bindings/leds/issi,is32fl3207.yaml
> new file mode 100644
> index 0000000000000..32eaf14ca7cf3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/issi,is32fl3207.yaml
> @@ -0,0 +1,285 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/issi,is32fl3207.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Lumissil IS32FL3207 18-channel LED controller

[ ... ]

> +patternProperties:
> +  "^led@[0-9a-f]+$":
> +    type: object
> +    $ref: common.yaml#
> +    unevaluatedProperties: false
> +
> +    properties:
> +      reg:
> +        minimum: 0
> +        maximum: 17
> +        description: |
> +          Zero-based LED output number. Values 0 through 17 correspond to
> +          OUT1 through OUT18.
> +
> +      led-max-microamp:
> +        minimum: 149
> +        maximum: 38250

[Severity: Medium]
Does this minimum constraint inadvertently reject valid lower-current limits
for single LEDs?

When a board designer uses a larger issi,riset-ohms (e.g., 100 kOhm) to
achieve a lower full-scale current, they may need to limit a specific LED to
a peak current well below 149 uA (e.g., 50 uA).

Since the hardware can represent much smaller currents with a large RISET,
could this prevent developers from enforcing valid safety limits below 149 uA,
forcing them to over-specify the limit?

> +        description: |
> +          Board-safe peak current for this output. The value must not exceed
> +          the full-scale current derived from issi,riset-ohms. The minimum is
> +          the smallest nonzero current representable at the maximum permitted
> +          full-scale current.

[ ... ]

> +  "^multi-led(@[0-9a-f]|-[0-9]+)?$":
> +    type: object
> +    $ref: leds-class-multicolor.yaml#
> +    unevaluatedProperties: false

[ ... ]

> +    patternProperties:
> +      "^led@[0-9a-f]+$":
> +        type: object
> +        $ref: common.yaml#
> +        unevaluatedProperties: false
> +
> +        properties:
> +          reg:
> +            minimum: 0
> +            maximum: 17
> +            description: |
> +              Zero-based LED output number. Values 0 through 17 correspond to
> +              OUT1 through OUT18.
> +
> +          led-max-microamp:
> +            minimum: 149
> +            maximum: 38250

[Severity: Medium]
Does this also incorrectly restrict nested LEDs inside multi-led groups from
having valid maximum current limits below 149 uA?

> +            description: |
> +              Board-safe peak current for this output. The value must not
> +              exceed the full-scale current derived from issi,riset-ohms. The
> +              minimum is the smallest nonzero current representable at the
> +              maximum permitted full-scale current.

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