All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Ahmad Byagowi" <ahmadexp@gmail.com>
Cc: conor+dt@kernel.org, lee@kernel.org, robh@kernel.org,
	devicetree@vger.kernel.org, linux-leds@vger.kernel.org
Subject: Re: [PATCH v6 1/2] dt-bindings: leds: Add IS32FL3207 controller
Date: Thu, 20 Aug 2026 17:48:52 +0000	[thread overview]
Message-ID: <20260820174852.BB1E61F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <22d13eb34c85e2c2be8c0ba1b7f3360e7cf9da71.1787247467.git.ahmadexp@gmail.com>

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 <ahmadexp@gmail.com>

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/cover.1787247467.git.ahmadexp@gmail.com?part=1

  reply	other threads:[~2026-08-20 17:48 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-20 17:40 [PATCH v6 0/2] leds: Add IS32FL3207 controller support Ahmad Byagowi
2026-08-20 17:40 ` [PATCH v6 1/2] dt-bindings: leds: Add IS32FL3207 controller Ahmad Byagowi
2026-08-20 17:48   ` sashiko-bot [this message]
2026-08-20 17:40 ` [PATCH v6 2/2] leds: is32fl3207: Add controller driver Ahmad Byagowi
2026-08-20 17:48   ` sashiko-bot

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=20260820174852.BB1E61F00A3A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=ahmadexp@gmail.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.