Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752638AbbF3IBz (ORCPT ); Tue, 30 Jun 2015 04:01:55 -0400 Received: from mailout4.samsung.com ([203.254.224.34]:34521 "EHLO mailout4.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750800AbbF3IBn (ORCPT ); Tue, 30 Jun 2015 04:01:43 -0400 MIME-version: 1.0 Content-type: text/plain; charset=UTF-8 X-AuditID: cbfee61a-f79516d000006302-39-55924cdf1b16 Content-transfer-encoding: 8BIT From: Jacek Anaszewski To: linux-leds@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Jacek Anaszewski , Bryan Wu , Richard Purdie , Stas Sergeev , Pavel Machek , Sakari Ailus , Andreas Werner , Andrew Lunn , Antonio Ospite , Atsushi Nemoto , Ben Dooks , Chris Boot , Dan Murphy , Daniel Jeong , Daniel Mack , "David S. Miller" , Fabio Baltieri , Felipe Balbi , Florian Fainelli , "G.Shark Jeong" , Guennadi Liakhovetski , Ingi Kim , Jan-Simon Moeller , Johan Hovold , John Lenz , Jonas Gorski , Kim Kyuwon , Kristian Kielhofner , Kristoffer Ericson , Linus Walleij , Mark Brown , Michael Hennerich , Milo Kim , =?UTF-8?q?M=C3=A1rton=20N=C3=A9meth?= , Nate Case , NeilBrown , Nick Forbes , Paul Parsons , Peter Meerwald , Phil Sutter , Philippe Retornaz , Raphael Assenat , Richard Purdie , Rod Whitby , Dave Hansen , Rodolfo Giometti , "Sebastian A. Siewior" , Shuah Khan , Simon Guinot , =?UTF-8?q?=C3=81lvaro=20Fern=C3=A1ndez=20Rojas?= Subject: [PATCH/RFC RESEND] leds: Use set_brightness_work for brightness_set ops that can sleep Date: Tue, 30 Jun 2015 10:01:08 +0200 Message-id: <1435651268-9657-1-git-send-email-j.anaszewski@samsung.com> X-Mailer: git-send-email 1.7.9.5 X-Brightmail-Tracker: H4sIAAAAAAAAA02Se0yTVxjGc75zvguN1Q8UPWKipslmsmVqo8vemLpopvFkEuMfotk0mwU+ i5HbWiGCyehkLgqI2BaMDauXCVhEnNwsN5VqHaUQ5ggEFRCd3QXv3EZBUQsx878nz/N78z5/ PBIOu8VHSLsT9yrGRH28RlAR3+SfoZ/0R1qilruHZ0B2Yx6B9l43hiLrFmi6lwElviECBbcs GMo86yH//kMBPLXHOGio8WM48GqAwA/OqxwUtv9IwDZRyEO2r4qHzLFdMPqslcCzSw4OLt+Z xGCtz+LBdXtYgCOdfyO4W94iwIHuHB4ClYdEcHgpeK7X8WB7WcJBR12hANWPyxGM5dYIkHe9 UoSS8XICT59OEPAfbRKh8aSZwH9trzE8dvl4KOtx8zCRXU8g1z0fels8AnS0eQhcemDnwN96 B8FvzV4OAo39GOrr/DwMVo1ycPGhCkYCa+BwbgFZs5K9rjyD2EDnWnaur5djQ85iwjrHSzGr ct7mWPHB85idPTQmspHBAZHV2ntF9vzFN8xRdA2zitLDArN25fDM2dHPsZ6uBmHzh1+rdLFK /O5Uxbjs852qOHd+Jp/cfRDtuzJwljeja0lZKESi8kqa4y0Xp/Vc+nvfRSELqaQw+Tiiw5Mv UTBQy6F0zNpHspAkYXkRvfHHnqCN5SXU4jiDp/kAom2ZJ0gwEGQtDfzziAvyc+SFtLY1Lchg uXkWHek5OsXMlvXUafuZC2oif0BtBafF6V8baK35ydQtlRfTQosuD820v9fC/n8L+3stTiFc isKV5JhkU7QhQbvUpE8wpSQalsYkJVSgqaH9FeFCTjO4kSwhzQw11Vmiwnh9qiktwY2ohDVz 1N/Pf2upY/Vp6Yox6VtjSrxicqMFEtHMU3uTIqPCZIN+r7JHUZIV47uUk0IizMgl/PJxxDp4 9Fx7+lfH/i1P+PPhuzL4Yq5h1b70Qd+pyK7RzwJbbwaO6cSWe19uRARtF2+mzB0t0vbd1xHW nP/TVwUNFybiwqtj22NmpVsz99s2jqtX1Hl9/vHosozq7SHbhi3aLyoNqw3f1aR+umHy3M72 TUOhrqbVFSPRF/7t3qEhpji99iNsNOnfAFch2nNkAwAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 16886 Lines: 476 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 Cc: Bryan Wu Cc: Richard Purdie Cc: Stas Sergeev Cc: Pavel Machek Cc: Sakari Ailus Cc: Andreas Werner Cc: Andrew Lunn Cc: Antonio Ospite Cc: Atsushi Nemoto Cc: Ben Dooks Cc: Chris Boot Cc: Dan Murphy Cc: Daniel Jeong Cc: Daniel Mack Cc: David S. Miller Cc: Fabio Baltieri Cc: Felipe Balbi Cc: Florian Fainelli Cc: G.Shark Jeong Cc: Guennadi Liakhovetski Cc: Ingi Kim Cc: Jan-Simon Moeller Cc: Johan Hovold Cc: John Lenz Cc: Jonas Gorski Cc: Kim Kyuwon Cc: Kristian Kielhofner Cc: Kristoffer Ericson Cc: Linus Walleij Cc: Mark Brown Cc: Michael Hennerich Cc: Milo Kim Cc: Márton Németh Cc: Nate Case Cc: NeilBrown Cc: Nick Forbes Cc: Paul Parsons Cc: Peter Meerwald Cc: Phil Sutter Cc: Philippe Retornaz Cc: Raphael Assenat Cc: Richard Purdie Cc: Rod Whitby Cc: Dave Hansen Cc: Rodolfo Giometti Cc: Sebastian A. Siewior Cc: Shuah Khan Cc: Simon Guinot Cc: Álvaro Fernández Rojas --- 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 + * Author: Jacek Anaszewski * * 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 #include -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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/