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

On Fri, 2026-08-14 at 09:30 +0000, [email protected] wrote:
> 
> New issues:
> - [High] st1202_blink_set alters the hardware state of sibling LED
> channels by zeroing out their PWM configurations, permanently turning
> off solid-ON LEDs and desynchronizing their physical state from the
> LED subsystem.

The zero-delay handling is not a bug. Converting a zero delay
to a default value is the established convention for blink_set()
implementations across the LED subsystem. The timer trigger does not
use zero delays to signal steady-on or steady-off states; those are
achieved through different triggers or by setting brightness directly.
The if-statement form used here was introduced in v6 at the maintainer's
request precisely because it matches how other blink_set callbacks in
the kernel handle this case.

> - [High] Hardware blink implementation incorrectly intercepts 0-delay
> values, breaking timer trigger off/on states.

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 hardcodes maximum brightness, completely
> ignoring the user's requested blink brightness.

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.


-- 
Manuel Fombuena