* [PATCH] leds: core: Fix race condition for software blink
@ 2026-04-23 11:36 Craig McQueen
2026-05-07 11:59 ` Lee Jones
0 siblings, 1 reply; 3+ messages in thread
From: Craig McQueen @ 2026-04-23 11:36 UTC (permalink / raw)
To: linux-leds; +Cc: Craig McQueen
led_set_brightness() function: Change handling of software blink to
avoid race conditions when stopping blink and setting brightness.
Triggers may call led_set_brightness(LED_OFF),
led_set_brightness(LED_FULL) in quick succession to disable blinking and
turn the LED on. If the delayed work task has not yet disabled blinking
by the time the second call occurs, then the brightness also needs to be
changed in the delayed work task.
Signed-off-by: Craig McQueen <craig@mcqueen.au>
---
drivers/leds/led-core.c | 33 ++++++++++++++++++++-------------
1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 385e78af1dac..073c547068cc 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -303,24 +303,31 @@ EXPORT_SYMBOL_GPL(led_stop_software_blink);
void led_set_brightness(struct led_classdev *led_cdev, unsigned int brightness)
{
- /*
- * If software blink is active, delay brightness setting
- * until the next timer tick.
- */
- if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) {
+ if (brightness) {
/*
- * If we need to disable soft blinking delegate this to the
- * work queue task to avoid problems in case we are called
- * from hard irq context.
+ * If software blink disable is pending, also queue brightness setting.
+ * If software blink is active, delay brightness setting
+ * until the next timer tick.
*/
- if (!brightness) {
- set_bit(LED_BLINK_DISABLE, &led_cdev->work_flags);
+ if (test_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags) ||
+ test_bit(LED_BLINK_DISABLE, &led_cdev->work_flags)) {
+ led_cdev->delayed_set_value = brightness;
+ set_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags);
queue_work(led_cdev->wq, &led_cdev->set_brightness_work);
- } else {
- set_bit(LED_BLINK_BRIGHTNESS_CHANGE,
- &led_cdev->work_flags);
+ return;
+ } else if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) {
led_cdev->new_blink_brightness = brightness;
+ set_bit(LED_BLINK_BRIGHTNESS_CHANGE, &led_cdev->work_flags);
+ return;
}
+ } else if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) {
+ /*
+ * If we need to disable soft blinking delegate this to the
+ * work queue task to avoid problems in case we are called
+ * from hard irq context.
+ */
+ set_bit(LED_BLINK_DISABLE, &led_cdev->work_flags);
+ queue_work(led_cdev->wq, &led_cdev->set_brightness_work);
return;
}
--
2.52.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] leds: core: Fix race condition for software blink
2026-04-23 11:36 [PATCH] leds: core: Fix race condition for software blink Craig McQueen
@ 2026-05-07 11:59 ` Lee Jones
2026-05-08 0:40 ` Craig McQueen
0 siblings, 1 reply; 3+ messages in thread
From: Lee Jones @ 2026-05-07 11:59 UTC (permalink / raw)
To: Craig McQueen; +Cc: linux-leds
On Thu, 23 Apr 2026, Craig McQueen wrote:
> led_set_brightness() function: Change handling of software blink to
> avoid race conditions when stopping blink and setting brightness.
>
> Triggers may call led_set_brightness(LED_OFF),
> led_set_brightness(LED_FULL) in quick succession to disable blinking and
> turn the LED on. If the delayed work task has not yet disabled blinking
> by the time the second call occurs, then the brightness also needs to be
> changed in the delayed work task.
>
> Signed-off-by: Craig McQueen <craig@mcqueen.au>
> ---
> drivers/leds/led-core.c | 33 ++++++++++++++++++++-------------
> 1 file changed, 20 insertions(+), 13 deletions(-)
I think this needs more eyes on it and to be thoroughly tested by others.
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index 385e78af1dac..073c547068cc 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -303,24 +303,31 @@ EXPORT_SYMBOL_GPL(led_stop_software_blink);
>
> void led_set_brightness(struct led_classdev *led_cdev, unsigned int brightness)
> {
> - /*
> - * If software blink is active, delay brightness setting
> - * until the next timer tick.
> - */
> - if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) {
> + if (brightness) {
> /*
> - * If we need to disable soft blinking delegate this to the
> - * work queue task to avoid problems in case we are called
> - * from hard irq context.
> + * If software blink disable is pending, also queue brightness setting.
> + * If software blink is active, delay brightness setting
> + * until the next timer tick.
> */
> - if (!brightness) {
> - set_bit(LED_BLINK_DISABLE, &led_cdev->work_flags);
> + if (test_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags) ||
> + test_bit(LED_BLINK_DISABLE, &led_cdev->work_flags)) {
> + led_cdev->delayed_set_value = brightness;
> + set_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags);
> queue_work(led_cdev->wq, &led_cdev->set_brightness_work);
> - } else {
> - set_bit(LED_BLINK_BRIGHTNESS_CHANGE,
> - &led_cdev->work_flags);
> + return;
> + } else if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) {
> led_cdev->new_blink_brightness = brightness;
> + set_bit(LED_BLINK_BRIGHTNESS_CHANGE, &led_cdev->work_flags);
> + return;
> }
> + } else if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) {
> + /*
> + * If we need to disable soft blinking delegate this to the
> + * work queue task to avoid problems in case we are called
> + * from hard irq context.
> + */
> + set_bit(LED_BLINK_DISABLE, &led_cdev->work_flags);
> + queue_work(led_cdev->wq, &led_cdev->set_brightness_work);
> return;
> }
>
> --
> 2.52.0
>
>
--
Lee Jones
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] leds: core: Fix race condition for software blink
2026-05-07 11:59 ` Lee Jones
@ 2026-05-08 0:40 ` Craig McQueen
0 siblings, 0 replies; 3+ messages in thread
From: Craig McQueen @ 2026-05-08 0:40 UTC (permalink / raw)
To: Lee Jones; +Cc: linux-leds
Lee Jones wrote
> On Thu, 23 Apr 2026, Craig McQueen wrote:
>
> > led_set_brightness() function: Change handling of software blink to
> > avoid race conditions when stopping blink and setting brightness.
> >
> > Triggers may call led_set_brightness(LED_OFF),
> > led_set_brightness(LED_FULL) in quick succession to disable blinking and
> > turn the LED on. If the delayed work task has not yet disabled blinking
> > by the time the second call occurs, then the brightness also needs to be
> > changed in the delayed work task.
> >
> > Signed-off-by: Craig McQueen <craig@mcqueen.au>
> > ---
> > drivers/leds/led-core.c | 33 ++++++++++++++++++++-------------
> > 1 file changed, 20 insertions(+), 13 deletions(-)
>
> I think this needs more eyes on it and to be thoroughly tested by others.
The problem I have seen is when a Linux driver controls a trigger to change it from flashing to steady-on. I've seen it specifically with LED core's software blinking.
/* First stop the blinking */
led_trigger_event(&udev->led_trigger, LED_OFF);
/* Next turn the LED steady-on */
led_trigger_event(&udev->led_trigger, LED_FULL);
Without this patch, the LED usually ends up in the off state, rather than on.
I have tested this patch on a Rockchip RK3328 based system (ARM 64).
> > diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> > index 385e78af1dac..073c547068cc 100644
> > --- a/drivers/leds/led-core.c
> > +++ b/drivers/leds/led-core.c
> > @@ -303,24 +303,31 @@ EXPORT_SYMBOL_GPL(led_stop_software_blink);
> >
> > void led_set_brightness(struct led_classdev *led_cdev, unsigned int brightness)
> > {
> > - /*
> > - * If software blink is active, delay brightness setting
> > - * until the next timer tick.
> > - */
> > - if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) {
> > + if (brightness) {
> > /*
> > - * If we need to disable soft blinking delegate this to the
> > - * work queue task to avoid problems in case we are called
> > - * from hard irq context.
> > + * If software blink disable is pending, also queue brightness setting.
> > + * If software blink is active, delay brightness setting
> > + * until the next timer tick.
> > */
> > - if (!brightness) {
> > - set_bit(LED_BLINK_DISABLE, &led_cdev->work_flags);
> > + if (test_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags) ||
> > + test_bit(LED_BLINK_DISABLE, &led_cdev->work_flags)) {
> > + led_cdev->delayed_set_value = brightness;
> > + set_bit(LED_SET_BRIGHTNESS, &led_cdev->work_flags);
> > queue_work(led_cdev->wq, &led_cdev->set_brightness_work);
> > - } else {
> > - set_bit(LED_BLINK_BRIGHTNESS_CHANGE,
> > - &led_cdev->work_flags);
> > + return;
> > + } else if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) {
> > led_cdev->new_blink_brightness = brightness;
> > + set_bit(LED_BLINK_BRIGHTNESS_CHANGE, &led_cdev->work_flags);
> > + return;
> > }
> > + } else if (test_bit(LED_BLINK_SW, &led_cdev->work_flags)) {
> > + /*
> > + * If we need to disable soft blinking delegate this to the
> > + * work queue task to avoid problems in case we are called
> > + * from hard irq context.
> > + */
> > + set_bit(LED_BLINK_DISABLE, &led_cdev->work_flags);
> > + queue_work(led_cdev->wq, &led_cdev->set_brightness_work);
> > return;
> > }
> >
> > --
> > 2.52.0
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-08 0:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-23 11:36 [PATCH] leds: core: Fix race condition for software blink Craig McQueen
2026-05-07 11:59 ` Lee Jones
2026-05-08 0:40 ` Craig McQueen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox