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