From: Lee Jones <lee@kernel.org>
To: sashiko-reviews@lists.linux.dev
Cc: Rillian Grant <rillian.grant@gmail.com>, linux-leds@vger.kernel.org
Subject: Re: [RFC PATCH 2/2] leds: flash: Add support for the TI LM3643 dual LED flash driver
Date: Thu, 27 Aug 2026 14:56:21 +0100 [thread overview]
Message-ID: <20260827135621.GQ770273@google.com> (raw)
In-Reply-To: <20260821084622.B77771F000E9@smtp.kernel.org>
On Fri, 21 Aug 2026, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Turning off an LED fails to clear the chip mode bits, causing power drain and breaking flash strobe teardown.
Please review, explain or fix the Sashiko review(s).
If this is a set, this request includes the other patches too.
> --
>
> --- Patch [2]: [RFC PATCH 2/2] leds: flash: Add support for the TI LM3643 dual LED flash driver ---
> commit bdf1945ab184ca005c2f401ecd5408571ece1de3
> Author: Rillian Grant <rillian.grant@gmail.com>
>
> leds: flash: Add support for the TI LM3643 dual LED flash driver
>
> The LM3643 is an I2C dual LED flash driver. Its two high-side current
> sources share a synchronous boost converter and each supports torch and
> flash modes with programmable current and flash timeout alongside fault
> reporting.
>
> > diff --git a/drivers/leds/flash/leds-lm3643.c b/drivers/leds/flash/leds-lm3643.c
> > new file mode 100644
> > --- /dev/null
> > +++ b/drivers/leds/flash/leds-lm3643.c
>
> [ ... ]
>
> > +static int lm3643_torch_set_brightness(struct led_classdev *led_cdev,
> > + enum led_brightness brightness)
> > +{
>
> [ ... ]
>
> > + if (brightness == 0)
> > + return regmap_update_bits(led->regmap, LM3643_REG_ENABLE,
> > + chan->enable_bit, 0);
>
> [Severity: High]
> When brightness is 0, this only clears the channel's enable bit but leaves
> the LM3643_MODE_MASK bits intact. If no other LEDs are active, does this
> leave the chip in an active power state (Torch or Flash mode) instead of
> transitioning to LM3643_MODE_STANDBY, potentially causing continuous battery
> drain?
>
> [ ... ]
>
> > +static int lm3643_flash_strobe_set(struct led_classdev_flash *fled_cdev, bool state)
> > +{
>
> [ ... ]
>
> > + if (!state) {
> > + if (mode != LM3643_MODE_FLASH || !(enable_reg & chan->enable_bit))
> > + return 0;
> > +
> > + return regmap_update_bits(led->regmap, LM3643_REG_ENABLE,
> > + LM3643_MODE_MASK, LM3643_MODE_STANDBY);
> > + }
>
> [Severity: High]
> If the torch brightness was set to 0 during a flash, the channel's enable
> bit will have been cleared. During the subsequent teardown call to
> lm3643_flash_strobe_set() with state = false, does this early return trigger
> because the enable bit is no longer set?
>
> This would fail to restore the mode to LM3643_MODE_STANDBY, permanently
> stranding the driver's state machine in Flash mode.
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260821083620.68324-1-rillian.grant@gmail.com?part=2
--
Lee Jones
prev parent reply other threads:[~2026-08-27 13:56 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-21 8:36 [RFC PATCH 0/2] leds: flash: LM3643 dual LED flash driver Rillian Grant
2026-08-21 8:36 ` [RFC PATCH 1/2] dt-bindings: leds: Document TI " Rillian Grant
2026-08-21 8:44 ` sashiko-bot
2026-08-21 8:36 ` [RFC PATCH 2/2] leds: flash: Add support for the " Rillian Grant
2026-08-21 8:46 ` sashiko-bot
2026-08-27 13:56 ` Lee Jones [this message]
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=20260827135621.GQ770273@google.com \
--to=lee@kernel.org \
--cc=linux-leds@vger.kernel.org \
--cc=rillian.grant@gmail.com \
--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.