* [PATCH] leds: trigger: input-events: Restore previous brightness on input
@ 2026-09-24 6:09 Nikolay Plastinin via B4 Relay
2026-09-29 9:11 ` Lee Jones
0 siblings, 1 reply; 2+ messages in thread
From: Nikolay Plastinin via B4 Relay @ 2026-09-24 6:09 UTC (permalink / raw)
To: Lee Jones, Pavel Machek
Cc: Hans de Goede, linux-leds, linux-kernel, Nikolay Plastinin
From: Nikolay Plastinin <plaztininikolai@gmail.com>
The trigger turns the LEDs back on with LED_FULL, which the LED core
clamps to max_brightness. For keyboard backlights with several levels,
such as asus::kbd_backlight, this means the backlight always comes back
at maximum after being idle instead of at the level the user picked.
Remember the brightness in blink_brightness before turning the LEDs off
and use it when turning them back on, as the netdev trigger does.
Brightness changes made while the trigger is active are picked up too.
This needs per-LED handling, so convert the trigger from the simple
trigger API to a regular trigger with an activate callback.
Tested on an ASUS ROG Flow X13 GV302XV: with the keyboard backlight at
level 1, it now comes back at level 1 after going idle instead of at 3.
A level changed with the brightness hotkeys while the trigger is active
is restored after the next idle period as well.
Assisted-by: Claude Opus 5.5
Signed-off-by: Nikolay Plastinin <plaztininikolai@gmail.com>
---
This came up while looking at a keyboard backlight idle timeout for ASUS
laptops in asusctl:
https://github.com/OpenGamingCollective/asusctl/issues/98
The trigger walks trig->led_cdevs under rcu_read_lock() the same way
led_trigger_event() does. I can add a helper in led-triggers.c instead
if that is preferred.
---
drivers/leds/trigger/ledtrig-input-events.c | 56 +++++++++++++++++++++++++----
1 file changed, 50 insertions(+), 6 deletions(-)
diff --git a/drivers/leds/trigger/ledtrig-input-events.c b/drivers/leds/trigger/ledtrig-input-events.c
index d057b2a23..d1ef14ae8 100644
--- a/drivers/leds/trigger/ledtrig-input-events.c
+++ b/drivers/leds/trigger/ledtrig-input-events.c
@@ -10,6 +10,7 @@
#include <linux/leds.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
+#include <linux/rculist.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/workqueue.h>
@@ -28,7 +29,48 @@ static struct input_events_data {
unsigned long led_off_time;
} input_events_data;
-static struct led_trigger *input_events_led_trigger;
+static int input_events_activate(struct led_classdev *led_cdev)
+{
+ struct input_events_data *data = &input_events_data;
+ unsigned long flags;
+
+ spin_lock_irqsave(&data->lock, flags);
+
+ if (led_cdev->brightness)
+ led_cdev->blink_brightness = led_cdev->brightness;
+ if (!led_cdev->blink_brightness)
+ led_cdev->blink_brightness = led_cdev->max_brightness;
+
+ led_set_brightness(led_cdev, data->led_on ? led_cdev->blink_brightness : LED_OFF);
+
+ spin_unlock_irqrestore(&data->lock, flags);
+
+ return 0;
+}
+
+static struct led_trigger input_events_led_trigger = {
+ .name = "input-events",
+ .activate = input_events_activate,
+};
+
+/* Must be called with input_events_data.lock held */
+static void input_events_set_leds(bool on)
+{
+ struct led_classdev *led_cdev;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(led_cdev, &input_events_led_trigger.led_cdevs, trig_list) {
+ /*
+ * Remember the brightness the LED was last set to, so that it
+ * is restored instead of max brightness on the next input event.
+ */
+ if (led_cdev->brightness)
+ led_cdev->blink_brightness = led_cdev->brightness;
+
+ led_set_brightness(led_cdev, on ? led_cdev->blink_brightness : LED_OFF);
+ }
+ rcu_read_unlock();
+}
static void led_input_events_work(struct work_struct *work)
{
@@ -42,7 +84,7 @@ static void led_input_events_work(struct work_struct *work)
* running before a new event pushed led_off_time back.
*/
if (time_after_eq(jiffies, data->led_off_time)) {
- led_trigger_event(input_events_led_trigger, LED_OFF);
+ input_events_set_leds(false);
data->led_on = false;
}
@@ -59,7 +101,7 @@ static void input_events_event(struct input_handle *handle, unsigned int type,
spin_lock_irqsave(&data->lock, flags);
if (!data->led_on) {
- led_trigger_event(input_events_led_trigger, LED_FULL);
+ input_events_set_leds(true);
data->led_on = true;
}
data->led_off_time = jiffies + led_off_delay;
@@ -138,11 +180,13 @@ static int __init input_events_init(void)
INIT_DELAYED_WORK(&input_events_data.work, led_input_events_work);
spin_lock_init(&input_events_data.lock);
- led_trigger_register_simple("input-events", &input_events_led_trigger);
+ ret = led_trigger_register(&input_events_led_trigger);
+ if (ret)
+ return ret;
ret = input_register_handler(&input_events_handler);
if (ret) {
- led_trigger_unregister_simple(input_events_led_trigger);
+ led_trigger_unregister(&input_events_led_trigger);
return ret;
}
@@ -153,7 +197,7 @@ static void __exit input_events_exit(void)
{
input_unregister_handler(&input_events_handler);
cancel_delayed_work_sync(&input_events_data.work);
- led_trigger_unregister_simple(input_events_led_trigger);
+ led_trigger_unregister(&input_events_led_trigger);
}
module_init(input_events_init);
---
base-commit: f475845eaf3d749114a63270bf2efea459e14dd2
change-id: 20260924-leds-input-events-brightness-a76ae9a77b85
Best regards,
--
Nikolay Plastinin <plaztininikolai@gmail.com>
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] leds: trigger: input-events: Restore previous brightness on input
2026-09-24 6:09 [PATCH] leds: trigger: input-events: Restore previous brightness on input Nikolay Plastinin via B4 Relay
@ 2026-09-29 9:11 ` Lee Jones
0 siblings, 0 replies; 2+ messages in thread
From: Lee Jones @ 2026-09-29 9:11 UTC (permalink / raw)
To: plaztininikolai; +Cc: Pavel Machek, Hans de Goede, linux-leds, linux-kernel
On Thu, 24 Sep 2026, Nikolay Plastinin via B4 Relay wrote:
> From: Nikolay Plastinin <plaztininikolai@gmail.com>
>
> The trigger turns the LEDs back on with LED_FULL, which the LED core
> clamps to max_brightness. For keyboard backlights with several levels,
> such as asus::kbd_backlight, this means the backlight always comes back
> at maximum after being idle instead of at the level the user picked.
>
> Remember the brightness in blink_brightness before turning the LEDs off
> and use it when turning them back on, as the netdev trigger does.
> Brightness changes made while the trigger is active are picked up too.
> This needs per-LED handling, so convert the trigger from the simple
> trigger API to a regular trigger with an activate callback.
>
> Tested on an ASUS ROG Flow X13 GV302XV: with the keyboard backlight at
> level 1, it now comes back at level 1 after going idle instead of at 3.
> A level changed with the brightness hotkeys while the trigger is active
> is restored after the next idle period as well.
>
> Assisted-by: Claude Opus 5.5
> Signed-off-by: Nikolay Plastinin <plaztininikolai@gmail.com>
> ---
> This came up while looking at a keyboard backlight idle timeout for ASUS
> laptops in asusctl:
> https://github.com/OpenGamingCollective/asusctl/issues/98
>
> The trigger walks trig->led_cdevs under rcu_read_lock() the same way
> led_trigger_event() does. I can add a helper in led-triggers.c instead
> if that is preferred.
> ---
> drivers/leds/trigger/ledtrig-input-events.c | 56 +++++++++++++++++++++++++----
> 1 file changed, 50 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/leds/trigger/ledtrig-input-events.c b/drivers/leds/trigger/ledtrig-input-events.c
> index d057b2a23..d1ef14ae8 100644
> --- a/drivers/leds/trigger/ledtrig-input-events.c
> +++ b/drivers/leds/trigger/ledtrig-input-events.c
> @@ -10,6 +10,7 @@
> #include <linux/leds.h>
> #include <linux/module.h>
> #include <linux/moduleparam.h>
> +#include <linux/rculist.h>
> #include <linux/slab.h>
> #include <linux/spinlock.h>
> #include <linux/workqueue.h>
> @@ -28,7 +29,48 @@ static struct input_events_data {
> unsigned long led_off_time;
> } input_events_data;
>
> -static struct led_trigger *input_events_led_trigger;
> +static int input_events_activate(struct led_classdev *led_cdev)
> +{
> + struct input_events_data *data = &input_events_data;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&data->lock, flags);
> +
> + if (led_cdev->brightness)
> + led_cdev->blink_brightness = led_cdev->brightness;
Should we be overloading 'blink_brightness' like this? It would be cleaner to
allocate a small per-LED state struct in the 'activate()' callback, store it in
'led_cdev->trigger_data', and free it in a 'deactivate()' callback.
> + if (!led_cdev->blink_brightness)
> + led_cdev->blink_brightness = led_cdev->max_brightness;
> +
> + led_set_brightness(led_cdev, data->led_on ? led_cdev->blink_brightness : LED_OFF);
> +
> + spin_unlock_irqrestore(&data->lock, flags);
> +
> + return 0;
> +}
> +
> +static struct led_trigger input_events_led_trigger = {
> + .name = "input-events",
> + .activate = input_events_activate,
> +};
> +
> +/* Must be called with input_events_data.lock held */
> +static void input_events_set_leds(bool on)
> +{
> + struct led_classdev *led_cdev;
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(led_cdev, &input_events_led_trigger.led_cdevs, trig_list) {
Please avoid accessing 'led_cdevs' and 'trig_list' directly from outside the LED
core. As you suggested in the commit message, adding a helper in
'led-triggers.c' would be much preferred.
> + /*
> + * Remember the brightness the LED was last set to, so that it
> + * is restored instead of max brightness on the next input event.
> + */
> + if (led_cdev->brightness)
> + led_cdev->blink_brightness = led_cdev->brightness;
> +
> + led_set_brightness(led_cdev, on ? led_cdev->blink_brightness : LED_OFF);
> + }
> + rcu_read_unlock();
> +}
>
> static void led_input_events_work(struct work_struct *work)
> {
> @@ -42,7 +84,7 @@ static void led_input_events_work(struct work_struct *work)
> * running before a new event pushed led_off_time back.
> */
> if (time_after_eq(jiffies, data->led_off_time)) {
> - led_trigger_event(input_events_led_trigger, LED_OFF);
> + input_events_set_leds(false);
> data->led_on = false;
> }
>
> @@ -59,7 +101,7 @@ static void input_events_event(struct input_handle *handle, unsigned int type,
> spin_lock_irqsave(&data->lock, flags);
>
> if (!data->led_on) {
> - led_trigger_event(input_events_led_trigger, LED_FULL);
> + input_events_set_leds(true);
> data->led_on = true;
> }
> data->led_off_time = jiffies + led_off_delay;
> @@ -138,11 +180,13 @@ static int __init input_events_init(void)
> INIT_DELAYED_WORK(&input_events_data.work, led_input_events_work);
> spin_lock_init(&input_events_data.lock);
>
> - led_trigger_register_simple("input-events", &input_events_led_trigger);
> + ret = led_trigger_register(&input_events_led_trigger);
> + if (ret)
> + return ret;
>
> ret = input_register_handler(&input_events_handler);
> if (ret) {
> - led_trigger_unregister_simple(input_events_led_trigger);
> + led_trigger_unregister(&input_events_led_trigger);
> return ret;
> }
>
> @@ -153,7 +197,7 @@ static void __exit input_events_exit(void)
> {
> input_unregister_handler(&input_events_handler);
> cancel_delayed_work_sync(&input_events_data.work);
> - led_trigger_unregister_simple(input_events_led_trigger);
> + led_trigger_unregister(&input_events_led_trigger);
> }
>
> module_init(input_events_init);
>
> ---
> base-commit: f475845eaf3d749114a63270bf2efea459e14dd2
> change-id: 20260924-leds-input-events-brightness-a76ae9a77b85
>
> Best regards,
> --
> Nikolay Plastinin <plaztininikolai@gmail.com>
>
>
--
Lee Jones
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-09-29 9:11 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-24 6:09 [PATCH] leds: trigger: input-events: Restore previous brightness on input Nikolay Plastinin via B4 Relay
2026-09-29 9:11 ` Lee Jones
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®