Hi Jiri, On 5/13/24 7:09 AM, Jiri Slaby wrote: > On 11. 05. 24, 17:20, Hans de Goede wrote: >> A LED trigger's activate() callback gets called when the LED trigger >> gets activated for a specific LED, so that the trigger code can ensure >> the LED state matches the current state of the trigger condition. >> >> led_trigger_event() is intended for trigger condition state changes and >> iterates over _all_ LEDs which are controlled by this trigger changing >> the brightness of each of them. >> >> In the activate() case only the brightness of the LED which is being >> activated needs to change and that LED is passed as an argument to >> activate(), switch to led_set_brightness() to only change the brightness >> of the LED being activated. > > LGTM, but could you elaborate on what behavior this fixes? Should it be backported to stable? It does not really fix any user visible behavior. This is just something which I noticed while looking at all LED trigger activate callbacks because of some LED core patches I was writing. The code before this patch gets the job done, it syncs the VT capslock/numlock/etc status to the LEDs of a newly registered input device. But it also ends up calling led_set_brightness() on all already registered capslock/numlock/etc LEDs which is not necessary. So this is just a small optimization, not a bug fix. Regards, Hans > >> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> >> --- >> drivers/tty/vt/keyboard.c | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c >> index a2116e135a82..804355da46f5 100644 >> --- a/drivers/tty/vt/keyboard.c >> +++ b/drivers/tty/vt/keyboard.c >> @@ -1033,9 +1033,7 @@ static int kbd_led_trigger_activate(struct led_classdev *cdev) >> tasklet_disable(&keyboard_tasklet); >> if (ledstate != -1U) >> - led_trigger_event(&trigger->trigger, >> - ledstate & trigger->mask ? >> - LED_FULL : LED_OFF); >> + led_set_brightness(cdev, ledstate & trigger->mask ? LED_FULL : LED_OFF); >> tasklet_enable(&keyboard_tasklet); >> return 0; > > thanks,