From a9c6ce57ec2f136d08141e8220a0ffaca216f7b0 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 8 Nov 2016 14:38:46 +0100 Subject: led: core: Use atomic bit-field for the blink-flags All the LED_BLINK* flags are accessed read-modify-write from e.g. led_set_brightness and led_blink_set_oneshot while both set_brightness_work and the blink_timer may be running. If these race then the modify step done by one of them may be lost, switch the LED_BLINK* flags to a new atomic work_flags bit-field to avoid this race. Signed-off-by: Hans de Goede Signed-off-by: Jacek Anaszewski --- drivers/leds/led-class.c | 1 + drivers/leds/led-core.c | 52 +++++++++++++++++++++++++----------------------- 2 files changed, 28 insertions(+), 25 deletions(-) (limited to 'drivers/leds') diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c index d946eb166781..326ee6e925a2 100644 --- a/drivers/leds/led-class.c +++ b/drivers/leds/led-class.c @@ -204,6 +204,7 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev) dev_warn(parent, "Led %s renamed to %s due to name collision", led_cdev->name, dev_name(led_cdev->dev)); + led_cdev->work_flags = 0; #ifdef CONFIG_LEDS_TRIGGERS init_rwsem(&led_cdev->trigger_lock); #endif diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c index 3bce44893021..bd6bb4d44f05 100644 --- a/drivers/leds/led-core.c +++ b/drivers/leds/led-core.c @@ -53,12 +53,13 @@ static void led_timer_function(unsigned long data) if (!led_cdev->blink_delay_on || !led_cdev->blink_delay_off) { led_set_brightness_nosleep(led_cdev, LED_OFF); - led_cdev->flags &= ~LED_BLINK_SW; + clear_bit(LED_BLINK_SW, &led_cdev->work_flags); return; } - if (led_cdev->flags & LED_BLINK_ONESHOT_STOP) { - led_cdev->flags &= ~(LED_BLINK_ONESHOT_STOP | LED_BLINK_SW); + if (test_and_clear_bit(LED_BLINK_ONESHOT_STOP, + &led_cdev->work_flags)) { + clear_bit(LED_BLINK_SW, &led_cdev->work_flags); return; } @@ -73,10 +74,9 @@ static void led_timer_function(unsigned long data) * Do it only if there is no pending blink brightness * change, to avoid overwriting the new value. */ - if (!(led_cdev->flags & LED_BLINK_BRIGHTNESS_CHANGE)) + if (!test_and_clear_bit(LED_BLINK_BRIGHTNESS_CHANGE, + &led_cdev->work_flags)) led_cdev->blink_brightness = brightness; - else - led_cdev->flags &= ~LED_BLINK_BRIGHTNESS_CHANGE; brightness = LED_OFF; delay = led_cdev->blink_delay_off; } @@ -87,13 +87,15 @@ static void led_timer_function(unsigned long data) * the final blink state so that the led is toggled each delay_on + * delay_off milliseconds in worst case. */ - if (led_cdev->flags & LED_BLINK_ONESHOT) { - if (led_cdev->flags & LED_BLINK_INVERT) { + if (test_bit(LED_BLINK_ONESHOT, &led_cdev->work_flags)) { + if (test_bit(LED_BLINK_INVERT, &led_cdev->work_flags)) { if (brightness) - led_cdev->flags |= LED_BLINK_ONESHOT_STOP; + set_bit(LED_BLINK_ONESHOT_STOP, + &led_cdev->work_flags); } else { if (!brightness) - led_cdev->flags |= LED_BLINK_ONESHOT_STOP; + set_bit(LED_BLINK_ONESHOT_STOP, + &led_cdev->work_flags); } } @@ -106,10 +108,9 @@ static void set_brightness_delayed(struct work_struct *ws) container_of(ws, struct led_classdev, set_brightness_work); int ret = 0; - if (led_cdev->flags & LED_BLINK_DISABLE) { + if (test_and_clear_bit(LED_BLINK_DISABLE, &led_cdev->work_flags)) { led_cdev->delayed_set_value = LED_OFF; led_stop_software_blink(led_cdev); - led_cdev->flags &= ~LED_BLINK_DISABLE; } ret = __led_set_brightness(led_cdev, led_cdev->delayed_set_value); @@ -152,7 +153,7 @@ static void led_set_software_blink(struct led_classdev *led_cdev, return; } - led_cdev->flags |= LED_BLINK_SW; + set_bit(LED_BLINK_SW, &led_cdev->work_flags); mod_timer(&led_cdev->blink_timer, jiffies + 1); } @@ -161,7 +162,7 @@ static void led_blink_setup(struct led_classdev *led_cdev, unsigned long *delay_on, unsigned long *delay_off) { - if (!(led_cdev->flags & LED_BLINK_ONESHOT) && + if (!test_bit(LED_BLINK_ONESHOT, &led_cdev->work_flags) && led_cdev->blink_set && !led_cdev->blink_set(led_cdev, delay_on, delay_off)) return; @@ -188,8 +189,8 @@ void led_blink_set(struct led_classdev *led_cdev, { del_timer_sync(&led_cdev->blink_timer); - led_cdev->flags &= ~LED_BLINK_ONESHOT; - led_cdev->flags &= ~LED_BLINK_ONESHOT_STOP; + clear_bit(LED_BLINK_ONESHOT, &led_cdev->work_flags); + clear_bit(LED_BLINK_ONESHOT_STOP, &led_cdev->work_flags); led_blink_setup(led_cdev, delay_on, delay_off); } @@ -200,17 +201,17 @@ void led_blink_set_oneshot(struct led_classdev *led_cdev, unsigned long *delay_off, int invert) { - if ((led_cdev->flags & LED_BLINK_ONESHOT) && + if (test_bit(LED_BLINK_ONESHOT, &led_cdev->work_flags) && timer_pending(&led_cdev->blink_timer)) return; - led_cdev->flags |= LED_BLINK_ONESHOT; - led_cdev->flags &= ~LED_BLINK_ONESHOT_STOP; + set_bit(LED_BLINK_ONESHOT, &led_cdev->work_flags); + clear_bit(LED_BLINK_ONESHOT_STOP, &led_cdev->work_flags); if (invert) - led_cdev->flags |= LED_BLINK_INVERT; + set_bit(LED_BLINK_INVERT, &led_cdev->work_flags); else - led_cdev->flags &= ~LED_BLINK_INVERT; + clear_bit(LED_BLINK_INVERT, &led_cdev->work_flags); led_blink_setup(led_cdev, delay_on, delay_off); } @@ -221,7 +222,7 @@ void led_stop_software_blink(struct led_classdev *led_cdev) del_timer_sync(&led_cdev->blink_timer); led_cdev->blink_delay_on = 0; led_cdev->blink_delay_off = 0; - led_cdev->flags &= ~LED_BLINK_SW; + clear_bit(LED_BLINK_SW, &led_cdev->work_flags); } EXPORT_SYMBOL_GPL(led_stop_software_blink); @@ -232,17 +233,18 @@ void led_set_brightness(struct led_classdev *led_cdev, * If software blink is active, delay brightness setting * until the next timer tick. */ - if (led_cdev->flags & LED_BLINK_SW) { + 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. */ if (brightness == LED_OFF) { - led_cdev->flags |= LED_BLINK_DISABLE; + set_bit(LED_BLINK_DISABLE, &led_cdev->work_flags); schedule_work(&led_cdev->set_brightness_work); } else { - led_cdev->flags |= LED_BLINK_BRIGHTNESS_CHANGE; + set_bit(LED_BLINK_BRIGHTNESS_CHANGE, + &led_cdev->work_flags); led_cdev->blink_brightness = brightness; } return; -- cgit v1.2.3