Re: [PATCH] leds: core: Fix race condition for software blink

[Lee Jones <lee@kernel.org>]

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