diff options
author | Hans de Goede <hdegoede@redhat.com> | 2016-11-08 14:38:46 +0100 |
---|---|---|
committer | Jacek Anaszewski <j.anaszewski@samsung.com> | 2016-11-22 12:07:05 +0100 |
commit | a9c6ce57ec2f136d08141e8220a0ffaca216f7b0 (patch) | |
tree | f7a577053e7c3453ae8939d87b903dde4754b2e7 /drivers/leds | |
parent | leds: Add user LED driver for NIC78bx device (diff) | |
download | linux-a9c6ce57ec2f136d08141e8220a0ffaca216f7b0.tar.xz linux-a9c6ce57ec2f136d08141e8220a0ffaca216f7b0.zip |
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 <hdegoede@redhat.com>
Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Diffstat (limited to 'drivers/leds')
-rw-r--r-- | drivers/leds/led-class.c | 1 | ||||
-rw-r--r-- | drivers/leds/led-core.c | 52 |
2 files changed, 28 insertions, 25 deletions
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; |