diff mbox series

[RESEND] leds: Ensure hardware blinking turns off when requested

Message ID 20230405165028.73579-1-eajames@linux.ibm.com
State New
Headers
Series [RESEND] leds: Ensure hardware blinking turns off when requested |

Commit Message

Eddie James April 5, 2023, 4:50 p.m. UTC 
  If an LED is blinking through the hardware and the trigger is deactivated,
the driver will set the brightness to 0 to disable blinking. However, if
the LED driver doesn't support non-blocking operations, the request for
setting brightness to 0 will get queued. Then if a user requests a new
non-zero brightness before the request for 0 brightness is executed, the
LED blinking will not get disabled since the brightness will simply be
updated.
Fix this by flushing the workqueue before updating the requested
brightness.

Fixes: 4d71a4a12b13 ("leds: Add support for setting brightness in a synchronous way")
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/leds/led-class.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Lee Jones April 20, 2023, 8:22 a.m. UTC | #1 
On Wed, 05 Apr 2023, Eddie James wrote:

> If an LED is blinking through the hardware and the trigger is deactivated,
> the driver will set the brightness to 0 to disable blinking. However, if
> the LED driver doesn't support non-blocking operations, the request for
> setting brightness to 0 will get queued. Then if a user requests a new
> non-zero brightness before the request for 0 brightness is executed, the
> LED blinking will not get disabled since the brightness will simply be
> updated.
> Fix this by flushing the workqueue before updating the requested
> brightness.

You make a convincing argument for this.
 
> Fixes: 4d71a4a12b13 ("leds: Add support for setting brightness in a synchronous way")
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  drivers/leds/led-class.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 9255bc11f99d..78a270300d30 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -57,6 +57,8 @@ static ssize_t brightness_store(struct device *dev,
>  
>  	if (state == LED_OFF)
>  		led_trigger_remove(led_cdev);
> +	/* flush out any request to disable blinking */
> +	flush_work(&led_cdev->set_brightness_work);
>  	led_set_brightness(led_cdev, state);
>  	flush_work(&led_cdev->set_brightness_work);

Just to be clear, you want to flush the work before AND after setting
the brightness?

In the commit message, could you make it clear why simply moving this
flush won't work.
diff mbox series

Patch

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 9255bc11f99d..78a270300d30 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -57,6 +57,8 @@  static ssize_t brightness_store(struct device *dev,
 
 	if (state == LED_OFF)
 		led_trigger_remove(led_cdev);
+	/* flush out any request to disable blinking */
+	flush_work(&led_cdev->set_brightness_work);
 	led_set_brightness(led_cdev, state);
 	flush_work(&led_cdev->set_brightness_work);