Re: [PATCH v3 1/1] leds: st1202: Add hardware-accelerated blink support

Manuel Fombuena <[email protected]> Fri, 24 Jul 2026 16:38:13 +0100
Newsgroups org.kernel.vger.linux-leds,dev.linux.lists.sashiko-reviews
Message-ID <GV1PR08MB8497EC69C453ABC4F63C3149C5CF2@GV1PR08MB8497.eurprd08.prod.outlook.com>
On Fri, 2026-07-24 at 12:12 +0000, [email protected] wrote:

> New issues:
> - [High] st1202_blink_set() forcibly zeroes out the active PWM pattern
> slots for all other LEDs, unintentionally destroying the state of
> independently configured steady-ON LEDs.

This is an inherent hardware constraint. The LED1202 uses a single
global pattern sequencer with shared timing registers, making it
impossible for two channels to run independent blink configurations
simultaneously. When blink_set() is called, the shared timing is
reconfigured for the requested delays and other channels' PWM slots are
set to LED_OFF so they remain dark rather than outputting unintended
values. This behaviour is described in the commit message.

> - [Medium] st1202_blink_set() ignores the LED's configured brightness
> and forces the hardware current limit to maximum (U8_MAX).

led_cdev->blink_brightness is set inside led_set_software_blink(), which
is the fallback path taken when blink_set() is absent or returns non-
zero. Since st1202_blink_set() returns 0 on success,
led_set_software_blink() is never reached and blink_brightness is not
updated by the core before our callback is invoked. Using it would risk
reading 0 or a stale value from a previous software blink, causing the
LED to blink invisibly. U8_MAX is intentional.

> Pre-existing issues:
> - [High] The driver assigns a sleeping function
> (st1202_brightness_set) to the non-blocking led_cdev.brightness_set
> callback, leading to kernel panics when called from atomic context.

This is a pre-existing issue outside the scope of this patch and will be
addressed in a follow-up submission.

--
Manuel Fombuena