This patch rearranges the core LED subsystem code, so that it now removes from drivers the responsibility of using work queues internally in case their brightness_set ops can sleep. Addition of two flags: LED_BRIGHTNESS_FAST and LED_BLINK_DISABLE as well as new_brightness_value property to the struct led_classdev allows for employing existing set_brightness_work to do the job. The modifications allow also to get rid of brightness_set_sync op, as flash LED devices can now be handled properly only basing on the SET_BRIGHTNESS_SYNC flag.
Signed-off-by: Jacek Anaszewski <[email protected]> Cc: Bryan Wu <[email protected]> Cc: Richard Purdie <[email protected]> Cc: Stas Sergeev <[email protected]> Cc: Pavel Machek <[email protected]> Cc: Sakari Ailus <[email protected]> Cc: Andreas Werner <[email protected]> Cc: Andrew Lunn <[email protected]> Cc: Antonio Ospite <[email protected]> Cc: Atsushi Nemoto <[email protected]> Cc: Ben Dooks <[email protected]> Cc: Chris Boot <[email protected]> Cc: Dan Murphy <[email protected]> Cc: Daniel Jeong <[email protected]> Cc: Daniel Mack <[email protected]> Cc: David S. Miller <[email protected]> Cc: Fabio Baltieri <[email protected]> Cc: Felipe Balbi <[email protected]> Cc: Florian Fainelli <[email protected]> Cc: G.Shark Jeong <[email protected]> Cc: Guennadi Liakhovetski <[email protected]> Cc: Ingi Kim <[email protected]> Cc: Jan-Simon Moeller <[email protected]> Cc: Johan Hovold <[email protected]> Cc: John Lenz <[email protected]> Cc: Jonas Gorski <[email protected]> Cc: Kim Kyuwon <[email protected]> Cc: Kristian Kielhofner <[email protected]> Cc: Kristoffer Ericson <[email protected]> Cc: Linus Walleij <[email protected]> Cc: Mark Brown <[email protected]> Cc: Michael Hennerich <[email protected]> Cc: Milo Kim <[email protected]> Cc: Márton Németh <[email protected]> Cc: Nate Case <[email protected]> Cc: NeilBrown <[email protected]> Cc: Nick Forbes <[email protected]> Cc: Paul Parsons <[email protected]> Cc: Peter Meerwald <[email protected]> Cc: Phil Sutter <[email protected]> Cc: Philippe Retornaz <[email protected]> Cc: Raphael Assenat <[email protected]> Cc: Richard Purdie <[email protected]> Cc: Rod Whitby <[email protected]> Cc: Dave Hansen <[email protected]> Cc: Rodolfo Giometti <[email protected]> Cc: Sebastian A. Siewior <[email protected]> Cc: Shuah Khan <[email protected]> Cc: Simon Guinot <[email protected]> Cc: Álvaro Fernández Rojas <[email protected]> --- Resending because previously this patch failed to reach linux-leds list. Also updated/removed defunct Cc addresses. Hi All, Since this patch will affect all the LED subsystem drivers I'd like it was tested by as many developers as possible to make sure that I haven't missed something. For the drivers which can sleep in their brightness_set ops (e.g. use mutex or gpio "cansleep" API) you only need to remove the work queues and move the code executed currently in the work queue task to the brightness_set op, as now LED core does the job. For drivers that are capable of setting brightness with use of MMIO you need to set the LED_BRIGHTNESS_FAST flag, so that LED core would know that it doesn't have to employ work queue. After the patch is positively verified I will create relevant patches for every LED class driver. This patch is based on linux-next_20150622. I am looking forward to your cooperation. Best Regards, Jacek Anaszewski drivers/leds/led-class.c | 18 +++++++---- drivers/leds/led-core.c | 50 +++++++++++++++++------------ drivers/leds/leds.h | 41 +++++++++++++---------- drivers/leds/trigger/ledtrig-backlight.c | 8 ++--- drivers/leds/trigger/ledtrig-default-on.c | 2 +- drivers/leds/trigger/ledtrig-gpio.c | 6 ++-- drivers/leds/trigger/ledtrig-heartbeat.c | 2 +- drivers/leds/trigger/ledtrig-oneshot.c | 4 +-- drivers/leds/trigger/ledtrig-transient.c | 8 ++--- include/linux/leds.h | 9 ++---- 10 files changed, 83 insertions(+), 65 deletions(-) diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c index beabfbc..07ccaeb 100644 --- a/drivers/leds/led-class.c +++ b/drivers/leds/led-class.c @@ -109,7 +109,7 @@ static void led_timer_function(unsigned long data) unsigned long delay; if (!led_cdev->blink_delay_on || !led_cdev->blink_delay_off) { - led_set_brightness_async(led_cdev, LED_OFF); + led_set_brightness_nosleep(led_cdev, LED_OFF); return; } @@ -121,10 +121,10 @@ static void led_timer_function(unsigned long data) brightness = led_get_brightness(led_cdev); if (!brightness) { /* Time to switch the LED on. */ - if (led_cdev->delayed_set_value) { + if (led_cdev->new_brightness_value) { led_cdev->blink_brightness = - led_cdev->delayed_set_value; - led_cdev->delayed_set_value = 0; + led_cdev->new_brightness_value; + led_cdev->new_brightness_value = 0; } brightness = led_cdev->blink_brightness; delay = led_cdev->blink_delay_on; @@ -137,7 +137,7 @@ static void led_timer_function(unsigned long data) delay = led_cdev->blink_delay_off; } - led_set_brightness_async(led_cdev, brightness); + led_set_brightness_nosleep(led_cdev, brightness); /* Return in next iteration if led is in one-shot mode and we are in * the final blink state so that the led is toggled each delay_on + @@ -161,9 +161,13 @@ static void set_brightness_delayed(struct work_struct *ws) struct led_classdev *led_cdev = container_of(ws, struct led_classdev, set_brightness_work); - led_stop_software_blink(led_cdev); + if (led_cdev->flags & LED_BLINK_DISABLE) { + led_stop_software_blink(led_cdev); + led_cdev->flags &= ~LED_BLINK_DISABLE; + return; + } - led_set_brightness_async(led_cdev, led_cdev->delayed_set_value); + __led_set_brightness(led_cdev, led_cdev->delayed_set_value); } /** diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c index 549de7e..0e13fb0 100644 --- a/drivers/leds/led-core.c +++ b/drivers/leds/led-core.c @@ -42,13 +42,14 @@ static void led_set_software_blink(struct led_classdev *led_cdev, /* never on - just set to off */ if (!delay_on) { - led_set_brightness_async(led_cdev, LED_OFF); + led_set_brightness_nosleep(led_cdev, LED_OFF); return; } /* never off - just set to brightness */ if (!delay_off) { - led_set_brightness_async(led_cdev, led_cdev->blink_brightness); + led_set_brightness_nosleep(led_cdev, + led_cdev->blink_brightness); return; } @@ -117,27 +118,36 @@ EXPORT_SYMBOL_GPL(led_stop_software_blink); void led_set_brightness(struct led_classdev *led_cdev, enum led_brightness brightness) { - int ret = 0; - - /* delay brightness if soft-blink is active */ + /* + * In case blinking is on delay brightness setting + * to the next timer tick. + */ if (led_cdev->blink_delay_on || led_cdev->blink_delay_off) { - led_cdev->delayed_set_value = brightness; - if (brightness == LED_OFF) - schedule_work(&led_cdev->set_brightness_work); - return; + led_cdev->new_brightness_value = brightness; + + /* New brightness will be set on next timer tick. */ + if (brightness != LED_OFF) + return; + /* + * If need to disable soft blinking delegate this to the + * work queue task to avoid problems in case we are + * called from hard irq context. + */ + led_cdev->flags |= LED_BLINK_DISABLE; + } else { + /* + * Don't use work queue if brightness has to be set as quickly + * as possible or if this is fast LED. + */ + if ((led_cdev->flags & SET_BRIGHTNESS_SYNC) || + (led_cdev->flags & LED_BRIGHTNESS_FAST)) { + __led_set_brightness(led_cdev, brightness); + return; + } } - if (led_cdev->flags & SET_BRIGHTNESS_ASYNC) { - led_set_brightness_async(led_cdev, brightness); - return; - } else if (led_cdev->flags & SET_BRIGHTNESS_SYNC) - ret = led_set_brightness_sync(led_cdev, brightness); - else - ret = -EINVAL; - - if (ret < 0) - dev_dbg(led_cdev->dev, "Setting LED brightness failed (%d)\n", - ret); + led_cdev->delayed_set_value = brightness; + schedule_work(&led_cdev->set_brightness_work); } EXPORT_SYMBOL(led_set_brightness); diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h index bc89d7a..dd9dd9b 100644 --- a/drivers/leds/leds.h +++ b/drivers/leds/leds.h @@ -2,8 +2,10 @@ * LED Core * * Copyright 2005 Openedhand Ltd. + * Copyright 2014, 2015 Samsung Electronics Co., Ltd. * * Author: Richard Purdie <[email protected]> + * Author: Jacek Anaszewski <[email protected]> * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as @@ -16,27 +18,15 @@ #include <linux/rwsem.h> #include <linux/leds.h> -static inline void led_set_brightness_async(struct led_classdev *led_cdev, +static inline void __led_set_brightness(struct led_classdev *led_cdev, enum led_brightness value) { - value = min(value, led_cdev->max_brightness); - led_cdev->brightness = value; - - if (!(led_cdev->flags & LED_SUSPENDED)) - led_cdev->brightness_set(led_cdev, value); -} - -static inline int led_set_brightness_sync(struct led_classdev *led_cdev, - enum led_brightness value) -{ - int ret = 0; - led_cdev->brightness = min(value, led_cdev->max_brightness); - if (!(led_cdev->flags & LED_SUSPENDED)) - ret = led_cdev->brightness_set_sync(led_cdev, - led_cdev->brightness); - return ret; + if (led_cdev->flags & LED_SUSPENDED) + return; + + led_cdev->brightness_set(led_cdev, led_cdev->brightness); } static inline int led_get_brightness(struct led_classdev *led_cdev) @@ -44,6 +34,23 @@ static inline int led_get_brightness(struct led_classdev *led_cdev) return led_cdev->brightness; } +static inline void led_set_brightness_nosleep(struct led_classdev *led_cdev, + enum led_brightness value) +{ + /* + * Delegate setting brightness to a work queue task only for slow LEDs + * as the FAST ones are guaranteed not to sleep while setting + * brightness. + */ + if (!(led_cdev->flags & LED_BRIGHTNESS_FAST)) { + led_cdev->delayed_set_value = value; + schedule_work(&led_cdev->set_brightness_work); + return; + } + + __led_set_brightness(led_cdev, value); +} + void led_stop_software_blink(struct led_classdev *led_cdev); extern struct rw_semaphore leds_list_lock; diff --git a/drivers/leds/trigger/ledtrig-backlight.c b/drivers/leds/trigger/ledtrig-backlight.c index 59eca17..1ca1f16 100644 --- a/drivers/leds/trigger/ledtrig-backlight.c +++ b/drivers/leds/trigger/ledtrig-backlight.c @@ -51,9 +51,9 @@ static int fb_notifier_callback(struct notifier_block *p, if ((n->old_status == UNBLANK) ^ n->invert) { n->brightness = led->brightness; - led_set_brightness_async(led, LED_OFF); + led_set_brightness_nosleep(led, LED_OFF); } else { - led_set_brightness_async(led, n->brightness); + led_set_brightness_nosleep(led, n->brightness); } n->old_status = new_status; @@ -89,9 +89,9 @@ static ssize_t bl_trig_invert_store(struct device *dev, /* After inverting, we need to update the LED. */ if ((n->old_status == BLANK) ^ n->invert) - led_set_brightness_async(led, LED_OFF); + led_set_brightness_nosleep(led, LED_OFF); else - led_set_brightness_async(led, n->brightness); + led_set_brightness_nosleep(led, n->brightness); return num; } diff --git a/drivers/leds/trigger/ledtrig-default-on.c b/drivers/leds/trigger/ledtrig-default-on.c index 6f38f88..ff455cb 100644 --- a/drivers/leds/trigger/ledtrig-default-on.c +++ b/drivers/leds/trigger/ledtrig-default-on.c @@ -19,7 +19,7 @@ static void defon_trig_activate(struct led_classdev *led_cdev) { - led_set_brightness_async(led_cdev, led_cdev->max_brightness); + led_set_brightness_nosleep(led_cdev, led_cdev->max_brightness); } static struct led_trigger defon_led_trigger = { diff --git a/drivers/leds/trigger/ledtrig-gpio.c b/drivers/leds/trigger/ledtrig-gpio.c index 4cc7040..51288a4 100644 --- a/drivers/leds/trigger/ledtrig-gpio.c +++ b/drivers/leds/trigger/ledtrig-gpio.c @@ -54,12 +54,12 @@ static void gpio_trig_work(struct work_struct *work) if (tmp) { if (gpio_data->desired_brightness) - led_set_brightness_async(gpio_data->led, + led_set_brightness_nosleep(gpio_data->led, gpio_data->desired_brightness); else - led_set_brightness_async(gpio_data->led, LED_FULL); + led_set_brightness_nosleep(gpio_data->led, LED_FULL); } else { - led_set_brightness_async(gpio_data->led, LED_OFF); + led_set_brightness_nosleep(gpio_data->led, LED_OFF); } } diff --git a/drivers/leds/trigger/ledtrig-heartbeat.c b/drivers/leds/trigger/ledtrig-heartbeat.c index fea6871..44c9f23 100644 --- a/drivers/leds/trigger/ledtrig-heartbeat.c +++ b/drivers/leds/trigger/ledtrig-heartbeat.c @@ -74,7 +74,7 @@ static void led_heartbeat_function(unsigned long data) break; } - led_set_brightness_async(led_cdev, brightness); + led_set_brightness_nosleep(led_cdev, brightness); mod_timer(&heartbeat_data->timer, jiffies + delay); } diff --git a/drivers/leds/trigger/ledtrig-oneshot.c b/drivers/leds/trigger/ledtrig-oneshot.c index fbd02cd..6729317 100644 --- a/drivers/leds/trigger/ledtrig-oneshot.c +++ b/drivers/leds/trigger/ledtrig-oneshot.c @@ -63,9 +63,9 @@ static ssize_t led_invert_store(struct device *dev, oneshot_data->invert = !!state; if (oneshot_data->invert) - led_set_brightness_async(led_cdev, LED_FULL); + led_set_brightness_nosleep(led_cdev, LED_FULL); else - led_set_brightness_async(led_cdev, LED_OFF); + led_set_brightness_nosleep(led_cdev, LED_OFF); return size; } diff --git a/drivers/leds/trigger/ledtrig-transient.c b/drivers/leds/trigger/ledtrig-transient.c index 3c34de4..1dddd8f 100644 --- a/drivers/leds/trigger/ledtrig-transient.c +++ b/drivers/leds/trigger/ledtrig-transient.c @@ -41,7 +41,7 @@ static void transient_timer_function(unsigned long data) struct transient_trig_data *transient_data = led_cdev->trigger_data; transient_data->activate = 0; - led_set_brightness_async(led_cdev, transient_data->restore_state); + led_set_brightness_nosleep(led_cdev, transient_data->restore_state); } static ssize_t transient_activate_show(struct device *dev, @@ -72,7 +72,7 @@ static ssize_t transient_activate_store(struct device *dev, if (state == 0 && transient_data->activate == 1) { del_timer(&transient_data->timer); transient_data->activate = state; - led_set_brightness_async(led_cdev, + led_set_brightness_nosleep(led_cdev, transient_data->restore_state); return size; } @@ -81,7 +81,7 @@ static ssize_t transient_activate_store(struct device *dev, if (state == 1 && transient_data->activate == 0 && transient_data->duration != 0) { transient_data->activate = state; - led_set_brightness_async(led_cdev, transient_data->state); + led_set_brightness_nosleep(led_cdev, transient_data->state); transient_data->restore_state = (transient_data->state == LED_FULL) ? LED_OFF : LED_FULL; mod_timer(&transient_data->timer, @@ -204,7 +204,7 @@ static void transient_trig_deactivate(struct led_classdev *led_cdev) if (led_cdev->activated) { del_timer_sync(&transient_data->timer); - led_set_brightness_async(led_cdev, + led_set_brightness_nosleep(led_cdev, transient_data->restore_state); device_remove_file(led_cdev->dev, &dev_attr_activate); device_remove_file(led_cdev->dev, &dev_attr_duration); diff --git a/include/linux/leds.h b/include/linux/leds.h index b122eea..ea99de9 100644 --- a/include/linux/leds.h +++ b/include/linux/leds.h @@ -48,17 +48,13 @@ struct led_classdev { #define SET_BRIGHTNESS_ASYNC (1 << 21) #define SET_BRIGHTNESS_SYNC (1 << 22) #define LED_DEV_CAP_FLASH (1 << 23) +#define LED_BRIGHTNESS_FAST (1 << 24) +#define LED_BLINK_DISABLE (1 << 25) /* Set LED brightness level */ /* Must not sleep, use a workqueue if needed */ void (*brightness_set)(struct led_classdev *led_cdev, enum led_brightness brightness); - /* - * Set LED brightness level immediately - it can block the caller for - * the time required for accessing a LED device register. - */ - int (*brightness_set_sync)(struct led_classdev *led_cdev, - enum led_brightness brightness); /* Get LED brightness level */ enum led_brightness (*brightness_get)(struct led_classdev *led_cdev); @@ -87,6 +83,7 @@ struct led_classdev { struct work_struct set_brightness_work; int delayed_set_value; + int new_brightness_value; #ifdef CONFIG_LEDS_TRIGGERS /* Protects the trigger data below */ -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
