linux-leds.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 00/10] LED core improvements
@ 2015-10-07  9:10 Jacek Anaszewski
  2015-10-07  9:10 ` [PATCH v3 01/10] leds: core: Add two new LED_BLINK_ flags Jacek Anaszewski
                   ` (9 more replies)
  0 siblings, 10 replies; 22+ messages in thread
From: Jacek Anaszewski @ 2015-10-07  9:10 UTC (permalink / raw)
  To: linux-leds; +Cc: linux-kernel, pavel, sakari.ailus, andrew, Jacek Anaszewski

This is a third version of the patch set preparing the
ground for removing work queues from LED class drivers.
It is based on RFC [1].
Andrew, Pavel, Sakari - thanks for a review.

================
Changes from v2:
================
- added led_set_brightness_nopm() to be used from pm ops,
  and modified led_set_brightness_nosleep() to call the former
- removed patch that renames EXPORT_SYMBOL to EXPORT_SYMBOL_GPL
  in the led-core.c - it is postponed until clear consensus is reached on it
- added acks from Pavel Machek
- removed from the set a patch that has been applied:
  [PATCH 1/5] leds: core: Move LED core callbacks out of led-class.c

================
Changes from v1:
================
- removed useless use of else in led_set_brightness()
- switched to removing error code instead of emitting
  warning from led_set_brightness_sync() in case software
  blink fallback is enabled
- moved __led_set_brightness() contents to set_brightness_delayed()
- removed description of internal led_set_brightness_nosleep() from
  led-class.txt documentation, and modified description of
  led_set_brightness() and led_set_brightness_sync()
- renamed LED_BLINK_CHANGE flag to more meaningful
  LED_BLINK_BRIGHTNESS_CHANGE
- Fixed stylistic issues in a few comments
- Adopted old brightness_set_sync() op description to
  a new brightness_set_blocking() op
- Fixed led_set_brightness_nosleep to avoid scheduling
  spurious set_brightness_work
- made LED core using EXPORT_SYMBOL_GPL consistently
- switched v4l2-flash-led-class wrapper to using
  led_set_brightness_sync API for setting torch brightness
- merged with patch set
  "LED flash: Set brightness in a sync way on demand"


======================
Original cover letter:
======================

This patch set prepares the ground for removing work queues
from LED class drivers, and is a follow up of the patch set [1].
LED core modifications have been reorganized to make them
more clear and easier to review. The patch set is reduced
in comparison to it its predecessor, to expose the modifications
indispensable for the LED core to gain the capability of handling
brightness_set_blocking ops, that is without work queues.

Thanks,
Jacek Anaszewski

[1] https://lkml.org/lkml/2015/8/20/426

Jacek Anaszewski (10):
  leds: core: Add two new LED_BLINK_ flags
  leds: Rename brightness_set_sync op to brightness_set_blocking
  leds: core: Add led_set_brightness_nosleep{nopm} functions
  leds: core: Use set_brightness_work for the blocking op
  leds: core: Drivers shouldn't enforce SYNC/ASYNC brightness setting
  Documentation: leds: Add description of brightness setting API
  leds: max77693: Remove work queue
  leds: aat1290: Remove work queue
  leds: ktd2692: Remove work queue
  media: flash: use led_set_brightness_sync for torch brightness

 Documentation/leds/leds-class.txt              |   13 +++
 drivers/leds/led-class-flash.c                 |    6 +-
 drivers/leds/led-class.c                       |    6 +-
 drivers/leds/led-core.c                        |  116 ++++++++++++++++++------
 drivers/leds/leds-aat1290.c                    |   50 +++-------
 drivers/leds/leds-ktd2692.c                    |   41 ++-------
 drivers/leds/leds-max77693.c                   |   57 ++----------
 drivers/leds/leds.h                            |   27 +-----
 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       |    4 +-
 drivers/leds/trigger/ledtrig-oneshot.c         |    4 +-
 drivers/leds/trigger/ledtrig-transient.c       |    8 +-
 drivers/media/v4l2-core/v4l2-flash-led-class.c |    8 +-
 include/linux/leds.h                           |   27 ++++--
 16 files changed, 177 insertions(+), 206 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v3 01/10] leds: core: Add two new LED_BLINK_ flags
  2015-10-07  9:10 [PATCH v3 00/10] LED core improvements Jacek Anaszewski
@ 2015-10-07  9:10 ` Jacek Anaszewski
  2015-10-16 15:32   ` Sakari Ailus
  2015-10-07  9:10 ` [PATCH v3 02/10] leds: Rename brightness_set_sync op to brightness_set_blocking Jacek Anaszewski
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Jacek Anaszewski @ 2015-10-07  9:10 UTC (permalink / raw)
  To: linux-leds; +Cc: linux-kernel, pavel, sakari.ailus, andrew, Jacek Anaszewski

This patch adds LED_BLINK_BRIGHTNESS_CHANGE flag to indicate that blink
brightness has changed, and LED_BLINK_DISABLE flag to indicate that
blinking deactivation has been requested. In order to use the flags
led_timer_function and set_brightness_delayed callbacks as well as
led_set_brightness() function are being modified. The main goal of these
modifications is to prepare set_brightness_work for extension of the
scope of its responsibilities.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/leds/led-core.c |   36 ++++++++++++++++++++++++++----------
 include/linux/leds.h    |   10 ++++++----
 2 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index c1c3af0..1ba7fac 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -44,18 +44,18 @@ 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) {
-			led_cdev->blink_brightness =
-					led_cdev->delayed_set_value;
-			led_cdev->delayed_set_value = 0;
-		}
 		brightness = led_cdev->blink_brightness;
 		delay = led_cdev->blink_delay_on;
 	} else {
 		/* Store the current brightness value to be able
 		 * to restore it when the delay_off period is over.
+		 * Do it only if there is no pending blink brightness
+		 * change, to avoid overwriting the new value.
 		 */
-		led_cdev->blink_brightness = brightness;
+		if (!(led_cdev->flags & LED_BLINK_BRIGHTNESS_CHANGE))
+			led_cdev->blink_brightness = brightness;
+		else
+			led_cdev->flags &= ~LED_BLINK_BRIGHTNESS_CHANGE;
 		brightness = LED_OFF;
 		delay = led_cdev->blink_delay_off;
 	}
@@ -84,7 +84,11 @@ 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_cdev->delayed_set_value = LED_OFF;
+		led_stop_software_blink(led_cdev);
+		led_cdev->flags &= ~LED_BLINK_DISABLE;
+	}
 
 	led_set_brightness_async(led_cdev, led_cdev->delayed_set_value);
 }
@@ -192,11 +196,23 @@ void led_set_brightness(struct led_classdev *led_cdev,
 {
 	int ret = 0;
 
-	/* delay brightness if soft-blink is active */
+	/*
+	 * In case blinking is on delay brightness setting
+	 * until 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)
+		/*
+		 * 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;
 			schedule_work(&led_cdev->set_brightness_work);
+		} else {
+			led_cdev->flags |= LED_BLINK_BRIGHTNESS_CHANGE;
+			led_cdev->blink_brightness = brightness;
+		}
 		return;
 	}
 
diff --git a/include/linux/leds.h b/include/linux/leds.h
index b122eea..84521e5 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -44,10 +44,12 @@ struct led_classdev {
 #define LED_BLINK_ONESHOT	(1 << 17)
 #define LED_BLINK_ONESHOT_STOP	(1 << 18)
 #define LED_BLINK_INVERT	(1 << 19)
-#define LED_SYSFS_DISABLE	(1 << 20)
-#define SET_BRIGHTNESS_ASYNC	(1 << 21)
-#define SET_BRIGHTNESS_SYNC	(1 << 22)
-#define LED_DEV_CAP_FLASH	(1 << 23)
+#define LED_BLINK_BRIGHTNESS_CHANGE (1 << 20)
+#define LED_BLINK_DISABLE	(1 << 21)
+#define LED_SYSFS_DISABLE	(1 << 22)
+#define SET_BRIGHTNESS_ASYNC	(1 << 23)
+#define SET_BRIGHTNESS_SYNC	(1 << 24)
+#define LED_DEV_CAP_FLASH	(1 << 25)
 
 	/* Set LED brightness level */
 	/* Must not sleep, use a workqueue if needed */
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 02/10] leds: Rename brightness_set_sync op to brightness_set_blocking
  2015-10-07  9:10 [PATCH v3 00/10] LED core improvements Jacek Anaszewski
  2015-10-07  9:10 ` [PATCH v3 01/10] leds: core: Add two new LED_BLINK_ flags Jacek Anaszewski
@ 2015-10-07  9:10 ` Jacek Anaszewski
  2015-10-16 15:34   ` Sakari Ailus
  2015-10-07  9:10 ` [PATCH v3 03/10] leds: core: Add led_set_brightness_nosleep{nopm} functions Jacek Anaszewski
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Jacek Anaszewski @ 2015-10-07  9:10 UTC (permalink / raw)
  To: linux-leds; +Cc: linux-kernel, pavel, sakari.ailus, andrew, Jacek Anaszewski

The initial purpose of brightness_set_sync op, introduced along with
the LED flash class extension, was to add a means for setting torch LED
brightness as soon as possible, which couldn't have been guaranteed by
brightness_set op. This patch renames the op to brightness_set_blocking,
which describes its purpose in a more generic way. It is beneficial
in view of the prospective changes in the LED core, aiming at removing
the need for using work queues in LED class drivers that can sleep
or use delays while setting brightness.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Pavel Machek <pavel@ucw.cz>
---
 drivers/leds/led-class-flash.c |    2 +-
 drivers/leds/leds-aat1290.c    |    2 +-
 drivers/leds/leds-ktd2692.c    |    2 +-
 drivers/leds/leds-max77693.c   |    2 +-
 drivers/leds/leds.h            |    2 +-
 include/linux/leds.h           |    4 ++--
 6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
index 3b25734..300a2c9 100644
--- a/drivers/leds/led-class-flash.c
+++ b/drivers/leds/led-class-flash.c
@@ -298,7 +298,7 @@ int led_classdev_flash_register(struct device *parent,
 	led_cdev = &fled_cdev->led_cdev;
 
 	if (led_cdev->flags & LED_DEV_CAP_FLASH) {
-		if (!led_cdev->brightness_set_sync)
+		if (!led_cdev->brightness_set_blocking)
 			return -EINVAL;
 
 		ops = fled_cdev->ops;
diff --git a/drivers/leds/leds-aat1290.c b/drivers/leds/leds-aat1290.c
index ac77d36..c56f9a9 100644
--- a/drivers/leds/leds-aat1290.c
+++ b/drivers/leds/leds-aat1290.c
@@ -510,7 +510,7 @@ static int aat1290_led_probe(struct platform_device *pdev)
 
 	/* Initialize LED Flash class device */
 	led_cdev->brightness_set = aat1290_led_brightness_set;
-	led_cdev->brightness_set_sync = aat1290_led_brightness_set_sync;
+	led_cdev->brightness_set_blocking = aat1290_led_brightness_set_sync;
 	led_cdev->max_brightness = led_cfg.max_brightness;
 	led_cdev->flags |= LED_DEV_CAP_FLASH;
 	INIT_WORK(&led->work_brightness_set, aat1290_brightness_set_work);
diff --git a/drivers/leds/leds-ktd2692.c b/drivers/leds/leds-ktd2692.c
index feca07b..e37de14 100644
--- a/drivers/leds/leds-ktd2692.c
+++ b/drivers/leds/leds-ktd2692.c
@@ -382,7 +382,7 @@ static int ktd2692_probe(struct platform_device *pdev)
 
 	led_cdev->max_brightness = led_cfg.max_brightness;
 	led_cdev->brightness_set = ktd2692_led_brightness_set;
-	led_cdev->brightness_set_sync = ktd2692_led_brightness_set_sync;
+	led_cdev->brightness_set_blocking = ktd2692_led_brightness_set_sync;
 	led_cdev->flags |= LED_CORE_SUSPENDRESUME | LED_DEV_CAP_FLASH;
 
 	mutex_init(&led->lock);
diff --git a/drivers/leds/leds-max77693.c b/drivers/leds/leds-max77693.c
index afbb140..9f7240b 100644
--- a/drivers/leds/leds-max77693.c
+++ b/drivers/leds/leds-max77693.c
@@ -932,7 +932,7 @@ static void max77693_init_fled_cdev(struct max77693_sub_led *sub_led,
 	led_cdev->name = led_cfg->label[fled_id];
 
 	led_cdev->brightness_set = max77693_led_brightness_set;
-	led_cdev->brightness_set_sync = max77693_led_brightness_set_sync;
+	led_cdev->brightness_set_blocking = max77693_led_brightness_set_sync;
 	led_cdev->max_brightness = (led->iout_joint ?
 					led_cfg->iout_torch_max[FLED1] +
 					led_cfg->iout_torch_max[FLED2] :
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index 4238fbc..cf6d448 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -34,7 +34,7 @@ static inline int led_set_brightness_sync(struct led_classdev *led_cdev,
 	led_cdev->brightness = min(value, led_cdev->max_brightness);
 
 	if (!(led_cdev->flags & LED_SUSPENDED))
-		ret = led_cdev->brightness_set_sync(led_cdev,
+		ret = led_cdev->brightness_set_blocking(led_cdev,
 						led_cdev->brightness);
 	return ret;
 }
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 84521e5..b47eff5 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -59,8 +59,8 @@ struct led_classdev {
 	 * 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);
+	int (*brightness_set_blocking)(struct led_classdev *led_cdev,
+				       enum led_brightness brightness);
 	/* Get LED brightness level */
 	enum led_brightness (*brightness_get)(struct led_classdev *led_cdev);
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 03/10] leds: core: Add led_set_brightness_nosleep{nopm} functions
  2015-10-07  9:10 [PATCH v3 00/10] LED core improvements Jacek Anaszewski
  2015-10-07  9:10 ` [PATCH v3 01/10] leds: core: Add two new LED_BLINK_ flags Jacek Anaszewski
  2015-10-07  9:10 ` [PATCH v3 02/10] leds: Rename brightness_set_sync op to brightness_set_blocking Jacek Anaszewski
@ 2015-10-07  9:10 ` Jacek Anaszewski
  2015-10-07  9:10 ` [PATCH v3 04/10] leds: core: Use set_brightness_work for the blocking op Jacek Anaszewski
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Jacek Anaszewski @ 2015-10-07  9:10 UTC (permalink / raw)
  To: linux-leds; +Cc: linux-kernel, pavel, sakari.ailus, andrew, Jacek Anaszewski

This patch adds led_set_brightness_nosleep() and led_set_brightness_nopm()
functions, that guarantee setting LED brightness in a non-blocking way.
The latter is used from pm_ops context and doesn't modify the brightness
cached in the struct led_classdev. Its execution always ends up with
a call to brightness setting op - either directly or through
a set_brightness_work, regardless of LED_SUSPENDED flag state.

The patch also replaces led_set_brightness_async() with
led_set_brightness_nosleep() in all places where the most vital was setting
brightness in a non sleeping way but not necessarily asynchronously, which
is not needed for non-blocking drivers.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
---
 drivers/leds/led-class.c                  |    4 +--
 drivers/leds/led-core.c                   |   40 ++++++++++++++++++++++++-----
 drivers/leds/leds.h                       |    4 +++
 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  |    4 +--
 drivers/leds/trigger/ledtrig-oneshot.c    |    4 +--
 drivers/leds/trigger/ledtrig-transient.c  |    8 +++---
 9 files changed, 56 insertions(+), 24 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 7385f98..83a1dc7 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -109,7 +109,7 @@ static const struct attribute_group *led_groups[] = {
 void led_classdev_suspend(struct led_classdev *led_cdev)
 {
 	led_cdev->flags |= LED_SUSPENDED;
-	led_cdev->brightness_set(led_cdev, 0);
+	led_set_brightness_nopm(led_cdev, 0);
 }
 EXPORT_SYMBOL_GPL(led_classdev_suspend);
 
@@ -119,7 +119,7 @@ EXPORT_SYMBOL_GPL(led_classdev_suspend);
  */
 void led_classdev_resume(struct led_classdev *led_cdev)
 {
-	led_cdev->brightness_set(led_cdev, led_cdev->brightness);
+	led_set_brightness_nopm(led_cdev, led_cdev->brightness);
 
 	if (led_cdev->flash_resume)
 		led_cdev->flash_resume(led_cdev);
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 1ba7fac..a1f53aa 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -32,7 +32,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;
 	}
 
@@ -60,7 +60,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 +
@@ -110,13 +110,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;
 	}
 
@@ -217,7 +218,7 @@ void led_set_brightness(struct led_classdev *led_cdev,
 	}
 
 	if (led_cdev->flags & SET_BRIGHTNESS_ASYNC) {
-		led_set_brightness_async(led_cdev, brightness);
+		led_set_brightness_nosleep(led_cdev, brightness);
 		return;
 	} else if (led_cdev->flags & SET_BRIGHTNESS_SYNC)
 		ret = led_set_brightness_sync(led_cdev, brightness);
@@ -230,6 +231,33 @@ void led_set_brightness(struct led_classdev *led_cdev,
 }
 EXPORT_SYMBOL(led_set_brightness);
 
+void led_set_brightness_nopm(struct led_classdev *led_cdev,
+			      enum led_brightness value)
+{
+	/* Use brightness_set op if available, it is guaranteed not to sleep */
+	if (led_cdev->brightness_set) {
+		led_cdev->brightness_set(led_cdev, value);
+		return;
+	}
+
+	/* If brightness setting can sleep, delegate it to a work queue task */
+	led_cdev->delayed_set_value = value;
+	schedule_work(&led_cdev->set_brightness_work);
+}
+EXPORT_SYMBOL_GPL(led_set_brightness_nopm);
+
+void led_set_brightness_nosleep(struct led_classdev *led_cdev,
+				enum led_brightness value)
+{
+	led_cdev->brightness = min(value, led_cdev->max_brightness);
+
+	if (led_cdev->flags & LED_SUSPENDED)
+		return;
+
+	led_set_brightness_nopm(led_cdev, led_cdev->brightness);
+}
+EXPORT_SYMBOL_GPL(led_set_brightness_nosleep);
+
 int led_update_brightness(struct led_classdev *led_cdev)
 {
 	int ret = 0;
@@ -244,7 +272,7 @@ int led_update_brightness(struct led_classdev *led_cdev)
 
 	return ret;
 }
-EXPORT_SYMBOL(led_update_brightness);
+EXPORT_SYMBOL_GPL(led_update_brightness);
 
 /* Caller must ensure led_cdev->led_access held */
 void led_sysfs_disable(struct led_classdev *led_cdev)
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index cf6d448..8e252a2 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -46,6 +46,10 @@ static inline int led_get_brightness(struct led_classdev *led_cdev)
 
 void led_init_core(struct led_classdev *led_cdev);
 void led_stop_software_blink(struct led_classdev *led_cdev);
+void led_set_brightness_nopm(struct led_classdev *led_cdev,
+				enum led_brightness value);
+void led_set_brightness_nosleep(struct led_classdev *led_cdev,
+				enum led_brightness value);
 
 extern struct rw_semaphore leds_list_lock;
 extern struct list_head leds_list;
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..3dc6f0c 100644
--- a/drivers/leds/trigger/ledtrig-heartbeat.c
+++ b/drivers/leds/trigger/ledtrig-heartbeat.c
@@ -37,7 +37,7 @@ static void led_heartbeat_function(unsigned long data)
 	unsigned long delay = 0;
 
 	if (unlikely(panic_heartbeats)) {
-		led_set_brightness(led_cdev, LED_OFF);
+		led_set_brightness_nosleep(led_cdev, LED_OFF);
 		return;
 	}
 
@@ -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);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 04/10] leds: core: Use set_brightness_work for the blocking op
  2015-10-07  9:10 [PATCH v3 00/10] LED core improvements Jacek Anaszewski
                   ` (2 preceding siblings ...)
  2015-10-07  9:10 ` [PATCH v3 03/10] leds: core: Add led_set_brightness_nosleep{nopm} functions Jacek Anaszewski
@ 2015-10-07  9:10 ` Jacek Anaszewski
  2015-10-16 15:43   ` Sakari Ailus
  2015-10-07  9:10 ` [PATCH v3 05/10] leds: core: Drivers shouldn't enforce SYNC/ASYNC brightness setting Jacek Anaszewski
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Jacek Anaszewski @ 2015-10-07  9:10 UTC (permalink / raw)
  To: linux-leds; +Cc: linux-kernel, pavel, sakari.ailus, andrew, Jacek Anaszewski

This patch makes LED core capable of setting brightness for drivers
that implement brightness_set_blocking op. It removes from LED class
drivers responsibility for using work queues on their own.

In order to achieve this set_brightness_delayed callback is being
modified to directly call one of available ops for brightness setting.

led_set_brightness_async() function didn't set brightness in an
asynchronous way in all cases. It was mistakenly assuming that all
LED subsystem drivers used work queue in their brightness_set op,
whereas only half of them did that. Since it has no users now,
it is being removed.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
---
 drivers/leds/led-core.c |   12 +++++++++++-
 drivers/leds/leds.h     |   10 ----------
 include/linux/leds.h    |    2 +-
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index a1f53aa..7d2512f 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -83,6 +83,7 @@ static void set_brightness_delayed(struct work_struct *ws)
 {
 	struct led_classdev *led_cdev =
 		container_of(ws, struct led_classdev, set_brightness_work);
+	int ret = 0;
 
 	if (led_cdev->flags & LED_BLINK_DISABLE) {
 		led_cdev->delayed_set_value = LED_OFF;
@@ -90,7 +91,16 @@ static void set_brightness_delayed(struct work_struct *ws)
 		led_cdev->flags &= ~LED_BLINK_DISABLE;
 	}
 
-	led_set_brightness_async(led_cdev, led_cdev->delayed_set_value);
+	if (led_cdev->brightness_set)
+		led_cdev->brightness_set(led_cdev, led_cdev->brightness);
+	else if (led_cdev->brightness_set_blocking)
+		ret = led_cdev->brightness_set_blocking(led_cdev,
+							led_cdev->brightness);
+	else
+		ret = -ENOTSUPP;
+	if (ret < 0)
+		dev_err(led_cdev->dev,
+			"Setting an LED's brightness failed (%d)\n", ret);
 }
 
 static void led_set_software_blink(struct led_classdev *led_cdev,
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index 8e252a2..683a605 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -16,16 +16,6 @@
 #include <linux/rwsem.h>
 #include <linux/leds.h>
 
-static inline void led_set_brightness_async(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)
 {
diff --git a/include/linux/leds.h b/include/linux/leds.h
index b47eff5..56d5069 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -158,7 +158,7 @@ extern void led_blink_set_oneshot(struct led_classdev *led_cdev,
  *
  * Set an LED's brightness, and, if necessary, cancel the
  * software blink timer that implements blinking when the
- * hardware doesn't.
+ * hardware doesn't. This function is guaranteed not to sleep.
  */
 extern void led_set_brightness(struct led_classdev *led_cdev,
 			       enum led_brightness brightness);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 05/10] leds: core: Drivers shouldn't enforce SYNC/ASYNC brightness setting
  2015-10-07  9:10 [PATCH v3 00/10] LED core improvements Jacek Anaszewski
                   ` (3 preceding siblings ...)
  2015-10-07  9:10 ` [PATCH v3 04/10] leds: core: Use set_brightness_work for the blocking op Jacek Anaszewski
@ 2015-10-07  9:10 ` Jacek Anaszewski
  2015-10-07  9:10 ` [PATCH v3 06/10] Documentation: leds: Add description of brightness setting API Jacek Anaszewski
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Jacek Anaszewski @ 2015-10-07  9:10 UTC (permalink / raw)
  To: linux-leds; +Cc: linux-kernel, pavel, sakari.ailus, andrew, Jacek Anaszewski

This patch removes SET_BRIGHTNESS_ASYNC and SET_BRIGHTNESS flags.
led_set_brightness() now calls led_set_brightness_nosleep() instead of
choosing between sync and async op basing on the flags defined by the
driver.

>From now on, if a user wants to make sure that brightness will be set
synchronously, they have to use led_set_brightness_sync() API. It is now
being made publicly available since it has become apparent that it is
a caller who should decide whether brightness is to be set in
a synchronous or an asynchronous way.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
---
 drivers/leds/led-class-flash.c |    4 ----
 drivers/leds/led-class.c       |    2 --
 drivers/leds/led-core.c        |   32 +++++++++++++++++++-------------
 drivers/leds/leds.h            |   13 -------------
 include/linux/leds.h           |   19 ++++++++++++++++---
 5 files changed, 35 insertions(+), 35 deletions(-)

diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
index 300a2c9..f53783b 100644
--- a/drivers/leds/led-class-flash.c
+++ b/drivers/leds/led-class-flash.c
@@ -316,10 +316,6 @@ int led_classdev_flash_register(struct device *parent,
 	if (ret < 0)
 		return ret;
 
-	/* Setting a torch brightness needs to have immediate effect */
-	led_cdev->flags &= ~SET_BRIGHTNESS_ASYNC;
-	led_cdev->flags |= SET_BRIGHTNESS_SYNC;
-
 	return 0;
 }
 EXPORT_SYMBOL_GPL(led_classdev_flash_register);
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 83a1dc7..d946991 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -215,8 +215,6 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
 	if (!led_cdev->max_brightness)
 		led_cdev->max_brightness = LED_FULL;
 
-	led_cdev->flags |= SET_BRIGHTNESS_ASYNC;
-
 	led_update_brightness(led_cdev);
 
 	led_init_core(led_cdev);
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 7d2512f..f4b3331 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -205,8 +205,6 @@ EXPORT_SYMBOL_GPL(led_stop_software_blink);
 void led_set_brightness(struct led_classdev *led_cdev,
 			enum led_brightness brightness)
 {
-	int ret = 0;
-
 	/*
 	 * In case blinking is on delay brightness setting
 	 * until the next timer tick.
@@ -227,17 +225,7 @@ void led_set_brightness(struct led_classdev *led_cdev,
 		return;
 	}
 
-	if (led_cdev->flags & SET_BRIGHTNESS_ASYNC) {
-		led_set_brightness_nosleep(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_set_brightness_nosleep(led_cdev, brightness);
 }
 EXPORT_SYMBOL(led_set_brightness);
 
@@ -268,6 +256,24 @@ void led_set_brightness_nosleep(struct led_classdev *led_cdev,
 }
 EXPORT_SYMBOL_GPL(led_set_brightness_nosleep);
 
+int led_set_brightness_sync(struct led_classdev *led_cdev,
+			    enum led_brightness value)
+{
+	if (led_cdev->blink_delay_on || led_cdev->blink_delay_off)
+		return -EBUSY;
+
+	led_cdev->brightness = min(value, led_cdev->max_brightness);
+
+	if (led_cdev->flags & LED_SUSPENDED)
+		return 0;
+
+	if (led_cdev->brightness_set_blocking)
+		return led_cdev->brightness_set_blocking(led_cdev,
+							 led_cdev->brightness);
+	return -ENOTSUPP;
+}
+EXPORT_SYMBOL_GPL(led_set_brightness_sync);
+
 int led_update_brightness(struct led_classdev *led_cdev)
 {
 	int ret = 0;
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index 683a605..db3f20d 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -16,19 +16,6 @@
 #include <linux/rwsem.h>
 #include <linux/leds.h>
 
-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_blocking(led_cdev,
-						led_cdev->brightness);
-	return ret;
-}
-
 static inline int led_get_brightness(struct led_classdev *led_cdev)
 {
 	return led_cdev->brightness;
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 56d5069..5a12950 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -47,9 +47,7 @@ struct led_classdev {
 #define LED_BLINK_BRIGHTNESS_CHANGE (1 << 20)
 #define LED_BLINK_DISABLE	(1 << 21)
 #define LED_SYSFS_DISABLE	(1 << 22)
-#define SET_BRIGHTNESS_ASYNC	(1 << 23)
-#define SET_BRIGHTNESS_SYNC	(1 << 24)
-#define LED_DEV_CAP_FLASH	(1 << 25)
+#define LED_DEV_CAP_FLASH	(1 << 23)
 
 	/* Set LED brightness level */
 	/* Must not sleep, use a workqueue if needed */
@@ -162,6 +160,21 @@ extern void led_blink_set_oneshot(struct led_classdev *led_cdev,
  */
 extern void led_set_brightness(struct led_classdev *led_cdev,
 			       enum led_brightness brightness);
+
+/**
+ * led_set_brightness_sync - set LED brightness synchronously
+ * @led_cdev: the LED to set
+ * @brightness: the brightness to set it to
+ *
+ * Set an LED's brightness immediately. This function will block
+ * the caller for the time required for accessing device registers,
+ * and it can sleep.
+ *
+ * Returns: 0 on success or negative error value on failure
+ */
+extern int led_set_brightness_sync(struct led_classdev *led_cdev,
+				   enum led_brightness value);
+
 /**
  * led_update_brightness - update LED brightness
  * @led_cdev: the LED to query
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 06/10] Documentation: leds: Add description of brightness setting API
  2015-10-07  9:10 [PATCH v3 00/10] LED core improvements Jacek Anaszewski
                   ` (4 preceding siblings ...)
  2015-10-07  9:10 ` [PATCH v3 05/10] leds: core: Drivers shouldn't enforce SYNC/ASYNC brightness setting Jacek Anaszewski
@ 2015-10-07  9:10 ` Jacek Anaszewski
  2015-10-16 15:36   ` Sakari Ailus
  2015-10-07  9:10 ` [PATCH v3 07/10] leds: max77693: Remove work queue Jacek Anaszewski
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Jacek Anaszewski @ 2015-10-07  9:10 UTC (permalink / raw)
  To: linux-leds; +Cc: linux-kernel, pavel, sakari.ailus, andrew, Jacek Anaszewski

This patch adds description of the LED subsystem API for
setting an LED brightness.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
---
 Documentation/leds/leds-class.txt |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/leds/leds-class.txt b/Documentation/leds/leds-class.txt
index 62261c0..d406d98 100644
--- a/Documentation/leds/leds-class.txt
+++ b/Documentation/leds/leds-class.txt
@@ -52,6 +52,19 @@ above leaves scope for further attributes should they be needed. If sections
 of the name don't apply, just leave that section blank.
 
 
+Brightness setting API
+======================
+
+LED subsystem core exposes following API for setting brightness:
+
+    - led_set_brightness : it is guaranteed not to sleep, passing LED_OFF stops
+		blinking,
+    - led_set_brightness_sync : for use cases when immediate effect is desired -
+		it can block the caller for the time required for accessing
+		device registers and can sleep, passing LED_OFF stops hardware
+		blinking, returns -EBUSY if software blink fallback is enabled.
+
+
 Hardware accelerated blink of LEDs
 ==================================
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 07/10] leds: max77693: Remove work queue
  2015-10-07  9:10 [PATCH v3 00/10] LED core improvements Jacek Anaszewski
                   ` (5 preceding siblings ...)
  2015-10-07  9:10 ` [PATCH v3 06/10] Documentation: leds: Add description of brightness setting API Jacek Anaszewski
@ 2015-10-07  9:10 ` Jacek Anaszewski
  2015-10-07  9:10 ` [PATCH v3 08/10] leds: aat1290: " Jacek Anaszewski
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Jacek Anaszewski @ 2015-10-07  9:10 UTC (permalink / raw)
  To: linux-leds; +Cc: linux-kernel, pavel, sakari.ailus, andrew, Jacek Anaszewski

Now the core implements the work queue, remove it from the drivers,
and switch to using brightness_set_blocking op.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/leds/leds-max77693.c |   57 +++++++-----------------------------------
 1 file changed, 9 insertions(+), 48 deletions(-)

diff --git a/drivers/leds/leds-max77693.c b/drivers/leds/leds-max77693.c
index 9f7240b..9401e33 100644
--- a/drivers/leds/leds-max77693.c
+++ b/drivers/leds/leds-max77693.c
@@ -20,7 +20,6 @@
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
-#include <linux/workqueue.h>
 #include <media/v4l2-flash-led-class.h>
 
 #define MODE_OFF		0
@@ -62,8 +61,6 @@ struct max77693_sub_led {
 	int fled_id;
 	/* corresponding LED Flash class device */
 	struct led_classdev_flash fled_cdev;
-	/* assures led-triggers compatibility */
-	struct work_struct work_brightness_set;
 	/* V4L2 Flash device */
 	struct v4l2_flash *v4l2_flash;
 
@@ -463,10 +460,14 @@ static int max77693_setup(struct max77693_led_device *led,
 	return max77693_set_mode_reg(led, MODE_OFF);
 }
 
-static int __max77693_led_brightness_set(struct max77693_led_device *led,
-					int fled_id, enum led_brightness value)
+/* LED subsystem callbacks */
+static int max77693_led_brightness_set(struct led_classdev *led_cdev,
+					enum led_brightness value)
 {
-	int ret;
+	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
+	struct max77693_sub_led *sub_led = flcdev_to_sub_led(fled_cdev);
+	struct max77693_led_device *led = sub_led_to_led(sub_led);
+	int fled_id = sub_led->fled_id, ret;
 
 	mutex_lock(&led->lock);
 
@@ -494,43 +495,8 @@ static int __max77693_led_brightness_set(struct max77693_led_device *led,
 			ret);
 unlock:
 	mutex_unlock(&led->lock);
-	return ret;
-}
 
-static void max77693_led_brightness_set_work(
-					struct work_struct *work)
-{
-	struct max77693_sub_led *sub_led =
-			container_of(work, struct max77693_sub_led,
-					work_brightness_set);
-	struct max77693_led_device *led = sub_led_to_led(sub_led);
-
-	__max77693_led_brightness_set(led, sub_led->fled_id,
-				sub_led->torch_brightness);
-}
-
-/* LED subsystem callbacks */
-
-static int max77693_led_brightness_set_sync(
-				struct led_classdev *led_cdev,
-				enum led_brightness value)
-{
-	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
-	struct max77693_sub_led *sub_led = flcdev_to_sub_led(fled_cdev);
-	struct max77693_led_device *led = sub_led_to_led(sub_led);
-
-	return __max77693_led_brightness_set(led, sub_led->fled_id, value);
-}
-
-static void max77693_led_brightness_set(
-				struct led_classdev *led_cdev,
-				enum led_brightness value)
-{
-	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
-	struct max77693_sub_led *sub_led = flcdev_to_sub_led(fled_cdev);
-
-	sub_led->torch_brightness = value;
-	schedule_work(&sub_led->work_brightness_set);
+	return ret;
 }
 
 static int max77693_led_flash_brightness_set(
@@ -931,16 +897,13 @@ static void max77693_init_fled_cdev(struct max77693_sub_led *sub_led,
 
 	led_cdev->name = led_cfg->label[fled_id];
 
-	led_cdev->brightness_set = max77693_led_brightness_set;
-	led_cdev->brightness_set_blocking = max77693_led_brightness_set_sync;
+	led_cdev->brightness_set_blocking = max77693_led_brightness_set;
 	led_cdev->max_brightness = (led->iout_joint ?
 					led_cfg->iout_torch_max[FLED1] +
 					led_cfg->iout_torch_max[FLED2] :
 					led_cfg->iout_torch_max[fled_id]) /
 				   TORCH_IOUT_STEP;
 	led_cdev->flags |= LED_DEV_CAP_FLASH;
-	INIT_WORK(&sub_led->work_brightness_set,
-			max77693_led_brightness_set_work);
 
 	max77693_init_flash_settings(sub_led, led_cfg);
 
@@ -1062,13 +1025,11 @@ static int max77693_led_remove(struct platform_device *pdev)
 	if (led->iout_joint || max77693_fled_used(led, FLED1)) {
 		v4l2_flash_release(sub_leds[FLED1].v4l2_flash);
 		led_classdev_flash_unregister(&sub_leds[FLED1].fled_cdev);
-		cancel_work_sync(&sub_leds[FLED1].work_brightness_set);
 	}
 
 	if (!led->iout_joint && max77693_fled_used(led, FLED2)) {
 		v4l2_flash_release(sub_leds[FLED2].v4l2_flash);
 		led_classdev_flash_unregister(&sub_leds[FLED2].fled_cdev);
-		cancel_work_sync(&sub_leds[FLED2].work_brightness_set);
 	}
 
 	mutex_destroy(&led->lock);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 08/10] leds: aat1290: Remove work queue
  2015-10-07  9:10 [PATCH v3 00/10] LED core improvements Jacek Anaszewski
                   ` (6 preceding siblings ...)
  2015-10-07  9:10 ` [PATCH v3 07/10] leds: max77693: Remove work queue Jacek Anaszewski
@ 2015-10-07  9:10 ` Jacek Anaszewski
  2015-10-07  9:10 ` [PATCH v3 09/10] leds: ktd2692: " Jacek Anaszewski
  2015-10-07  9:10 ` [PATCH v3 10/10] media: flash: use led_set_brightness_sync for torch brightness Jacek Anaszewski
  9 siblings, 0 replies; 22+ messages in thread
From: Jacek Anaszewski @ 2015-10-07  9:10 UTC (permalink / raw)
  To: linux-leds; +Cc: linux-kernel, pavel, sakari.ailus, andrew, Jacek Anaszewski

Now the core implements the work queue, remove it from the drivers,
and switch to using brightness_set_blocking op.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/leds/leds-aat1290.c |   50 +++++++++++--------------------------------
 1 file changed, 13 insertions(+), 37 deletions(-)

diff --git a/drivers/leds/leds-aat1290.c b/drivers/leds/leds-aat1290.c
index c56f9a9..f225930 100644
--- a/drivers/leds/leds-aat1290.c
+++ b/drivers/leds/leds-aat1290.c
@@ -20,7 +20,6 @@
 #include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
-#include <linux/workqueue.h>
 #include <media/v4l2-flash-led-class.h>
 
 #define AAT1290_MOVIE_MODE_CURRENT_ADDR	17
@@ -82,8 +81,6 @@ struct aat1290_led {
 
 	/* brightness cache */
 	unsigned int torch_brightness;
-	/* assures led-triggers compatibility */
-	struct work_struct work_brightness_set;
 };
 
 static struct aat1290_led *fled_cdev_to_led(
@@ -92,6 +89,12 @@ static struct aat1290_led *fled_cdev_to_led(
 	return container_of(fled_cdev, struct aat1290_led, fled_cdev);
 }
 
+static struct led_classdev_flash *led_cdev_to_fled_cdev(
+				struct led_classdev *led_cdev)
+{
+	return container_of(led_cdev, struct led_classdev_flash, led_cdev);
+}
+
 static void aat1290_as2cwire_write(struct aat1290_led *led, int addr, int value)
 {
 	int i;
@@ -134,9 +137,14 @@ static void aat1290_set_flash_safety_timer(struct aat1290_led *led,
 							flash_tm_reg);
 }
 
-static void aat1290_brightness_set(struct aat1290_led *led,
+/* LED subsystem callbacks */
+
+static int aat1290_led_brightness_set(struct led_classdev *led_cdev,
 					enum led_brightness brightness)
 {
+	struct led_classdev_flash *fled_cdev = led_cdev_to_fled_cdev(led_cdev);
+	struct aat1290_led *led = fled_cdev_to_led(fled_cdev);
+
 	mutex_lock(&led->lock);
 
 	if (brightness == 0) {
@@ -158,35 +166,6 @@ static void aat1290_brightness_set(struct aat1290_led *led,
 	}
 
 	mutex_unlock(&led->lock);
-}
-
-/* LED subsystem callbacks */
-
-static void aat1290_brightness_set_work(struct work_struct *work)
-{
-	struct aat1290_led *led =
-		container_of(work, struct aat1290_led, work_brightness_set);
-
-	aat1290_brightness_set(led, led->torch_brightness);
-}
-
-static void aat1290_led_brightness_set(struct led_classdev *led_cdev,
-					enum led_brightness brightness)
-{
-	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
-	struct aat1290_led *led = fled_cdev_to_led(fled_cdev);
-
-	led->torch_brightness = brightness;
-	schedule_work(&led->work_brightness_set);
-}
-
-static int aat1290_led_brightness_set_sync(struct led_classdev *led_cdev,
-					enum led_brightness brightness)
-{
-	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
-	struct aat1290_led *led = fled_cdev_to_led(fled_cdev);
-
-	aat1290_brightness_set(led, brightness);
 
 	return 0;
 }
@@ -509,11 +488,9 @@ static int aat1290_led_probe(struct platform_device *pdev)
 	mutex_init(&led->lock);
 
 	/* Initialize LED Flash class device */
-	led_cdev->brightness_set = aat1290_led_brightness_set;
-	led_cdev->brightness_set_blocking = aat1290_led_brightness_set_sync;
+	led_cdev->brightness_set_blocking = aat1290_led_brightness_set;
 	led_cdev->max_brightness = led_cfg.max_brightness;
 	led_cdev->flags |= LED_DEV_CAP_FLASH;
-	INIT_WORK(&led->work_brightness_set, aat1290_brightness_set_work);
 
 	aat1290_init_flash_timeout(led, &led_cfg);
 
@@ -548,7 +525,6 @@ static int aat1290_led_remove(struct platform_device *pdev)
 
 	v4l2_flash_release(led->v4l2_flash);
 	led_classdev_flash_unregister(&led->fled_cdev);
-	cancel_work_sync(&led->work_brightness_set);
 
 	mutex_destroy(&led->lock);
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 09/10] leds: ktd2692: Remove work queue
  2015-10-07  9:10 [PATCH v3 00/10] LED core improvements Jacek Anaszewski
                   ` (7 preceding siblings ...)
  2015-10-07  9:10 ` [PATCH v3 08/10] leds: aat1290: " Jacek Anaszewski
@ 2015-10-07  9:10 ` Jacek Anaszewski
  2015-10-07  9:10 ` [PATCH v3 10/10] media: flash: use led_set_brightness_sync for torch brightness Jacek Anaszewski
  9 siblings, 0 replies; 22+ messages in thread
From: Jacek Anaszewski @ 2015-10-07  9:10 UTC (permalink / raw)
  To: linux-leds
  Cc: linux-kernel, pavel, sakari.ailus, andrew, Jacek Anaszewski,
	Ingi Kim

Now the core implements the work queue, remove it from the drivers,
and switch to using brightness_set_blocking op.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Cc: Ingi Kim <ingi2.kim@samsung.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/leds/leds-ktd2692.c |   41 ++++++-----------------------------------
 1 file changed, 6 insertions(+), 35 deletions(-)

diff --git a/drivers/leds/leds-ktd2692.c b/drivers/leds/leds-ktd2692.c
index e37de14..abd04bf 100644
--- a/drivers/leds/leds-ktd2692.c
+++ b/drivers/leds/leds-ktd2692.c
@@ -18,7 +18,6 @@
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
-#include <linux/workqueue.h>
 
 /* Value related the movie mode */
 #define KTD2692_MOVIE_MODE_CURRENT_LEVELS	16
@@ -82,7 +81,6 @@ struct ktd2692_context {
 	/* secures access to the device */
 	struct mutex lock;
 	struct regulator *regulator;
-	struct work_struct work_brightness_set;
 
 	struct gpio_desc *aux_gpio;
 	struct gpio_desc *ctrl_gpio;
@@ -158,9 +156,12 @@ static void ktd2692_expresswire_write(struct ktd2692_context *led, u8 value)
 	ktd2692_expresswire_end(led);
 }
 
-static void ktd2692_brightness_set(struct ktd2692_context *led,
-				   enum led_brightness brightness)
+static int ktd2692_led_brightness_set(struct led_classdev *led_cdev,
+				       enum led_brightness brightness)
 {
+	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
+	struct ktd2692_context *led = fled_cdev_to_led(fled_cdev);
+
 	mutex_lock(&led->lock);
 
 	if (brightness == LED_OFF) {
@@ -174,33 +175,6 @@ static void ktd2692_brightness_set(struct ktd2692_context *led,
 
 	ktd2692_expresswire_write(led, led->mode | KTD2692_REG_MODE_BASE);
 	mutex_unlock(&led->lock);
-}
-
-static void ktd2692_brightness_set_work(struct work_struct *work)
-{
-	struct ktd2692_context *led =
-		container_of(work, struct ktd2692_context, work_brightness_set);
-
-	ktd2692_brightness_set(led, led->torch_brightness);
-}
-
-static void ktd2692_led_brightness_set(struct led_classdev *led_cdev,
-				       enum led_brightness brightness)
-{
-	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
-	struct ktd2692_context *led = fled_cdev_to_led(fled_cdev);
-
-	led->torch_brightness = brightness;
-	schedule_work(&led->work_brightness_set);
-}
-
-static int ktd2692_led_brightness_set_sync(struct led_classdev *led_cdev,
-					   enum led_brightness brightness)
-{
-	struct led_classdev_flash *fled_cdev = lcdev_to_flcdev(led_cdev);
-	struct ktd2692_context *led = fled_cdev_to_led(fled_cdev);
-
-	ktd2692_brightness_set(led, brightness);
 
 	return 0;
 }
@@ -381,12 +355,10 @@ static int ktd2692_probe(struct platform_device *pdev)
 	fled_cdev->ops = &flash_ops;
 
 	led_cdev->max_brightness = led_cfg.max_brightness;
-	led_cdev->brightness_set = ktd2692_led_brightness_set;
-	led_cdev->brightness_set_blocking = ktd2692_led_brightness_set_sync;
+	led_cdev->brightness_set_blocking = ktd2692_led_brightness_set;
 	led_cdev->flags |= LED_CORE_SUSPENDRESUME | LED_DEV_CAP_FLASH;
 
 	mutex_init(&led->lock);
-	INIT_WORK(&led->work_brightness_set, ktd2692_brightness_set_work);
 
 	platform_set_drvdata(pdev, led);
 
@@ -408,7 +380,6 @@ static int ktd2692_remove(struct platform_device *pdev)
 	int ret;
 
 	led_classdev_flash_unregister(&led->fled_cdev);
-	cancel_work_sync(&led->work_brightness_set);
 
 	if (led->regulator) {
 		ret = regulator_disable(led->regulator);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v3 10/10] media: flash: use led_set_brightness_sync for torch brightness
  2015-10-07  9:10 [PATCH v3 00/10] LED core improvements Jacek Anaszewski
                   ` (8 preceding siblings ...)
  2015-10-07  9:10 ` [PATCH v3 09/10] leds: ktd2692: " Jacek Anaszewski
@ 2015-10-07  9:10 ` Jacek Anaszewski
  2015-10-07 10:49   ` Pavel Machek
  2015-11-16  9:35   ` Jacek Anaszewski
  9 siblings, 2 replies; 22+ messages in thread
From: Jacek Anaszewski @ 2015-10-07  9:10 UTC (permalink / raw)
  To: linux-leds
  Cc: linux-kernel, pavel, sakari.ailus, andrew, Jacek Anaszewski,
	linux-media

LED subsystem shifted responsibility for choosing between SYNC or ASYNC
way of setting brightness from drivers to the caller. Adapt the wrapper
to those changes.

Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: linux-media@vger.kernel.org
---
 drivers/media/v4l2-core/v4l2-flash-led-class.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-flash-led-class.c b/drivers/media/v4l2-core/v4l2-flash-led-class.c
index 5bdfb8d..5d67335 100644
--- a/drivers/media/v4l2-core/v4l2-flash-led-class.c
+++ b/drivers/media/v4l2-core/v4l2-flash-led-class.c
@@ -107,10 +107,10 @@ static void v4l2_flash_set_led_brightness(struct v4l2_flash *v4l2_flash,
 		if (ctrls[LED_MODE]->val != V4L2_FLASH_LED_MODE_TORCH)
 			return;
 
-		led_set_brightness(&v4l2_flash->fled_cdev->led_cdev,
+		led_set_brightness_sync(&v4l2_flash->fled_cdev->led_cdev,
 					brightness);
 	} else {
-		led_set_brightness(&v4l2_flash->iled_cdev->led_cdev,
+		led_set_brightness_sync(&v4l2_flash->iled_cdev->led_cdev,
 					brightness);
 	}
 }
@@ -206,11 +206,11 @@ static int v4l2_flash_s_ctrl(struct v4l2_ctrl *c)
 	case V4L2_CID_FLASH_LED_MODE:
 		switch (c->val) {
 		case V4L2_FLASH_LED_MODE_NONE:
-			led_set_brightness(led_cdev, LED_OFF);
+			led_set_brightness_sync(led_cdev, LED_OFF);
 			return led_set_flash_strobe(fled_cdev, false);
 		case V4L2_FLASH_LED_MODE_FLASH:
 			/* Turn the torch LED off */
-			led_set_brightness(led_cdev, LED_OFF);
+			led_set_brightness_sync(led_cdev, LED_OFF);
 			if (ctrls[STROBE_SOURCE]) {
 				external_strobe = (ctrls[STROBE_SOURCE]->val ==
 					V4L2_FLASH_STROBE_SOURCE_EXTERNAL);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 10/10] media: flash: use led_set_brightness_sync for torch brightness
  2015-10-07  9:10 ` [PATCH v3 10/10] media: flash: use led_set_brightness_sync for torch brightness Jacek Anaszewski
@ 2015-10-07 10:49   ` Pavel Machek
  2015-11-16  9:35   ` Jacek Anaszewski
  1 sibling, 0 replies; 22+ messages in thread
From: Pavel Machek @ 2015-10-07 10:49 UTC (permalink / raw)
  To: Jacek Anaszewski
  Cc: linux-leds, linux-kernel, sakari.ailus, andrew, linux-media

On Wed 2015-10-07 11:10:48, Jacek Anaszewski wrote:
> LED subsystem shifted responsibility for choosing between SYNC or ASYNC
> way of setting brightness from drivers to the caller. Adapt the wrapper
> to those changes.
> 
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>

7-10 of the series:

Acked-by: Pavel Machek <pavel@ucw.cz>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 01/10] leds: core: Add two new LED_BLINK_ flags
  2015-10-07  9:10 ` [PATCH v3 01/10] leds: core: Add two new LED_BLINK_ flags Jacek Anaszewski
@ 2015-10-16 15:32   ` Sakari Ailus
  0 siblings, 0 replies; 22+ messages in thread
From: Sakari Ailus @ 2015-10-16 15:32 UTC (permalink / raw)
  To: Jacek Anaszewski; +Cc: linux-leds, linux-kernel, pavel, sakari.ailus, andrew

Hi Jacek,

On Wed, Oct 07, 2015 at 11:10:39AM +0200, Jacek Anaszewski wrote:
> This patch adds LED_BLINK_BRIGHTNESS_CHANGE flag to indicate that blink
> brightness has changed, and LED_BLINK_DISABLE flag to indicate that
> blinking deactivation has been requested. In order to use the flags
> led_timer_function and set_brightness_delayed callbacks as well as
> led_set_brightness() function are being modified. The main goal of these
> modifications is to prepare set_brightness_work for extension of the
> scope of its responsibilities.
> 
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Acked-by: Pavel Machek <pavel@ucw.cz>
> ---
>  drivers/leds/led-core.c |   36 ++++++++++++++++++++++++++----------
>  include/linux/leds.h    |   10 ++++++----
>  2 files changed, 32 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index c1c3af0..1ba7fac 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -44,18 +44,18 @@ 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) {
> -			led_cdev->blink_brightness =
> -					led_cdev->delayed_set_value;
> -			led_cdev->delayed_set_value = 0;
> -		}
>  		brightness = led_cdev->blink_brightness;
>  		delay = led_cdev->blink_delay_on;
>  	} else {
>  		/* Store the current brightness value to be able
>  		 * to restore it when the delay_off period is over.
> +		 * Do it only if there is no pending blink brightness
> +		 * change, to avoid overwriting the new value.
>  		 */
> -		led_cdev->blink_brightness = brightness;
> +		if (!(led_cdev->flags & LED_BLINK_BRIGHTNESS_CHANGE))
> +			led_cdev->blink_brightness = brightness;
> +		else
> +			led_cdev->flags &= ~LED_BLINK_BRIGHTNESS_CHANGE;
>  		brightness = LED_OFF;
>  		delay = led_cdev->blink_delay_off;
>  	}
> @@ -84,7 +84,11 @@ 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_cdev->delayed_set_value = LED_OFF;

LED_OFF is the only value assigned to led_cdev->delayed_set_value. You could
remove the field and use LED_OFF below instead.

The field is being used in later patches as well but I don't think its use
makes any difference from functionality point of view. It doesn't seem to be
a bug but quite confusing nonetheless when you try to figure out how the
different fields are being used. :-)

> +		led_stop_software_blink(led_cdev);
> +		led_cdev->flags &= ~LED_BLINK_DISABLE;
> +	}
>  
>  	led_set_brightness_async(led_cdev, led_cdev->delayed_set_value);
>  }
> @@ -192,11 +196,23 @@ void led_set_brightness(struct led_classdev *led_cdev,
>  {
>  	int ret = 0;
>  
> -	/* delay brightness if soft-blink is active */
> +	/*
> +	 * In case blinking is on delay brightness setting
> +	 * until 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)
> +		/*
> +		 * 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;
>  			schedule_work(&led_cdev->set_brightness_work);
> +		} else {
> +			led_cdev->flags |= LED_BLINK_BRIGHTNESS_CHANGE;
> +			led_cdev->blink_brightness = brightness;
> +		}
>  		return;
>  	}
>  
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index b122eea..84521e5 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -44,10 +44,12 @@ struct led_classdev {
>  #define LED_BLINK_ONESHOT	(1 << 17)
>  #define LED_BLINK_ONESHOT_STOP	(1 << 18)
>  #define LED_BLINK_INVERT	(1 << 19)
> -#define LED_SYSFS_DISABLE	(1 << 20)
> -#define SET_BRIGHTNESS_ASYNC	(1 << 21)
> -#define SET_BRIGHTNESS_SYNC	(1 << 22)
> -#define LED_DEV_CAP_FLASH	(1 << 23)
> +#define LED_BLINK_BRIGHTNESS_CHANGE (1 << 20)
> +#define LED_BLINK_DISABLE	(1 << 21)
> +#define LED_SYSFS_DISABLE	(1 << 22)
> +#define SET_BRIGHTNESS_ASYNC	(1 << 23)
> +#define SET_BRIGHTNESS_SYNC	(1 << 24)
> +#define LED_DEV_CAP_FLASH	(1 << 25)
>  
>  	/* Set LED brightness level */
>  	/* Must not sleep, use a workqueue if needed */

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 02/10] leds: Rename brightness_set_sync op to brightness_set_blocking
  2015-10-07  9:10 ` [PATCH v3 02/10] leds: Rename brightness_set_sync op to brightness_set_blocking Jacek Anaszewski
@ 2015-10-16 15:34   ` Sakari Ailus
  0 siblings, 0 replies; 22+ messages in thread
From: Sakari Ailus @ 2015-10-16 15:34 UTC (permalink / raw)
  To: Jacek Anaszewski; +Cc: linux-leds, linux-kernel, pavel, sakari.ailus, andrew

On Wed, Oct 07, 2015 at 11:10:40AM +0200, Jacek Anaszewski wrote:
> The initial purpose of brightness_set_sync op, introduced along with
> the LED flash class extension, was to add a means for setting torch LED
> brightness as soon as possible, which couldn't have been guaranteed by
> brightness_set op. This patch renames the op to brightness_set_blocking,
> which describes its purpose in a more generic way. It is beneficial
> in view of the prospective changes in the LED core, aiming at removing
> the need for using work queues in LED class drivers that can sleep
> or use delays while setting brightness.

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 06/10] Documentation: leds: Add description of brightness setting API
  2015-10-07  9:10 ` [PATCH v3 06/10] Documentation: leds: Add description of brightness setting API Jacek Anaszewski
@ 2015-10-16 15:36   ` Sakari Ailus
  2015-10-18  7:08     ` Jacek Anaszewski
  0 siblings, 1 reply; 22+ messages in thread
From: Sakari Ailus @ 2015-10-16 15:36 UTC (permalink / raw)
  To: Jacek Anaszewski; +Cc: linux-leds, linux-kernel, pavel, sakari.ailus, andrew

Hi Jacek,

On Wed, Oct 07, 2015 at 11:10:44AM +0200, Jacek Anaszewski wrote:
> This patch adds description of the LED subsystem API for
> setting an LED brightness.
> 
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> ---
>  Documentation/leds/leds-class.txt |   13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/Documentation/leds/leds-class.txt b/Documentation/leds/leds-class.txt
> index 62261c0..d406d98 100644
> --- a/Documentation/leds/leds-class.txt
> +++ b/Documentation/leds/leds-class.txt
> @@ -52,6 +52,19 @@ above leaves scope for further attributes should they be needed. If sections
>  of the name don't apply, just leave that section blank.
>  
>  
> +Brightness setting API
> +======================
> +
> +LED subsystem core exposes following API for setting brightness:
> +
> +    - led_set_brightness : it is guaranteed not to sleep, passing LED_OFF stops
> +		blinking,
> +    - led_set_brightness_sync : for use cases when immediate effect is desired -

Over 80 characters per line.

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

> +		it can block the caller for the time required for accessing
> +		device registers and can sleep, passing LED_OFF stops hardware
> +		blinking, returns -EBUSY if software blink fallback is enabled.
> +
> +
>  Hardware accelerated blink of LEDs
>  ==================================
>  

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 04/10] leds: core: Use set_brightness_work for the blocking op
  2015-10-07  9:10 ` [PATCH v3 04/10] leds: core: Use set_brightness_work for the blocking op Jacek Anaszewski
@ 2015-10-16 15:43   ` Sakari Ailus
  2015-10-18  7:06     ` Jacek Anaszewski
  0 siblings, 1 reply; 22+ messages in thread
From: Sakari Ailus @ 2015-10-16 15:43 UTC (permalink / raw)
  To: Jacek Anaszewski; +Cc: linux-leds, linux-kernel, pavel, sakari.ailus, andrew

Hi Jacek,

I think I'd rearrange patches 3 and 4; delayed_set_brightness is briefly
needed in patch 3 but again no longer here.

I have to say I quite like the set, it cleans up the framework a lot.
Thanks!

On Wed, Oct 07, 2015 at 11:10:42AM +0200, Jacek Anaszewski wrote:
> This patch makes LED core capable of setting brightness for drivers
> that implement brightness_set_blocking op. It removes from LED class
> drivers responsibility for using work queues on their own.
> 
> In order to achieve this set_brightness_delayed callback is being
> modified to directly call one of available ops for brightness setting.
> 
> led_set_brightness_async() function didn't set brightness in an
> asynchronous way in all cases. It was mistakenly assuming that all
> LED subsystem drivers used work queue in their brightness_set op,
> whereas only half of them did that. Since it has no users now,
> it is being removed.
> 
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> ---
>  drivers/leds/led-core.c |   12 +++++++++++-
>  drivers/leds/leds.h     |   10 ----------
>  include/linux/leds.h    |    2 +-
>  3 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index a1f53aa..7d2512f 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -83,6 +83,7 @@ static void set_brightness_delayed(struct work_struct *ws)
>  {
>  	struct led_classdev *led_cdev =
>  		container_of(ws, struct led_classdev, set_brightness_work);
> +	int ret = 0;
>  
>  	if (led_cdev->flags & LED_BLINK_DISABLE) {
>  		led_cdev->delayed_set_value = LED_OFF;
> @@ -90,7 +91,16 @@ static void set_brightness_delayed(struct work_struct *ws)
>  		led_cdev->flags &= ~LED_BLINK_DISABLE;
>  	}
>  
> -	led_set_brightness_async(led_cdev, led_cdev->delayed_set_value);
> +	if (led_cdev->brightness_set)
> +		led_cdev->brightness_set(led_cdev, led_cdev->brightness);
> +	else if (led_cdev->brightness_set_blocking)
> +		ret = led_cdev->brightness_set_blocking(led_cdev,
> +							led_cdev->brightness);
> +	else
> +		ret = -ENOTSUPP;
> +	if (ret < 0)
> +		dev_err(led_cdev->dev,
> +			"Setting an LED's brightness failed (%d)\n", ret);
>  }
>  
>  static void led_set_software_blink(struct led_classdev *led_cdev,
> diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
> index 8e252a2..683a605 100644
> --- a/drivers/leds/leds.h
> +++ b/drivers/leds/leds.h
> @@ -16,16 +16,6 @@
>  #include <linux/rwsem.h>
>  #include <linux/leds.h>
>  
> -static inline void led_set_brightness_async(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)
>  {
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index b47eff5..56d5069 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -158,7 +158,7 @@ extern void led_blink_set_oneshot(struct led_classdev *led_cdev,
>   *
>   * Set an LED's brightness, and, if necessary, cancel the
>   * software blink timer that implements blinking when the
> - * hardware doesn't.
> + * hardware doesn't. This function is guaranteed not to sleep.
>   */
>  extern void led_set_brightness(struct led_classdev *led_cdev,
>  			       enum led_brightness brightness);

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 04/10] leds: core: Use set_brightness_work for the blocking op
  2015-10-16 15:43   ` Sakari Ailus
@ 2015-10-18  7:06     ` Jacek Anaszewski
  0 siblings, 0 replies; 22+ messages in thread
From: Jacek Anaszewski @ 2015-10-18  7:06 UTC (permalink / raw)
  To: Sakari Ailus, Jacek Anaszewski
  Cc: linux-leds, linux-kernel, pavel, sakari.ailus, andrew

Hi Sakari,

Thanks for the review.

On 10/16/2015 05:43 PM, Sakari Ailus wrote:
> Hi Jacek,
>
> I think I'd rearrange patches 3 and 4; delayed_set_brightness is briefly
> needed in patch 3 but again no longer here.

The impression that delayed_set_brightness is not needed is caused
by my mistake in set_brightness_delayed(), please see my explanation
below.

> I have to say I quite like the set, it cleans up the framework a lot.
> Thanks!
>
> On Wed, Oct 07, 2015 at 11:10:42AM +0200, Jacek Anaszewski wrote:
>> This patch makes LED core capable of setting brightness for drivers
>> that implement brightness_set_blocking op. It removes from LED class
>> drivers responsibility for using work queues on their own.
>>
>> In order to achieve this set_brightness_delayed callback is being
>> modified to directly call one of available ops for brightness setting.
>>
>> led_set_brightness_async() function didn't set brightness in an
>> asynchronous way in all cases. It was mistakenly assuming that all
>> LED subsystem drivers used work queue in their brightness_set op,
>> whereas only half of them did that. Since it has no users now,
>> it is being removed.
>>
>> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>> ---
>>   drivers/leds/led-core.c |   12 +++++++++++-
>>   drivers/leds/leds.h     |   10 ----------
>>   include/linux/leds.h    |    2 +-
>>   3 files changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
>> index a1f53aa..7d2512f 100644
>> --- a/drivers/leds/led-core.c
>> +++ b/drivers/leds/led-core.c
>> @@ -83,6 +83,7 @@ static void set_brightness_delayed(struct work_struct *ws)
>>   {
>>   	struct led_classdev *led_cdev =
>>   		container_of(ws, struct led_classdev, set_brightness_work);
>> +	int ret = 0;
>>
>>   	if (led_cdev->flags & LED_BLINK_DISABLE) {
>>   		led_cdev->delayed_set_value = LED_OFF;
>> @@ -90,7 +91,16 @@ static void set_brightness_delayed(struct work_struct *ws)
>>   		led_cdev->flags &= ~LED_BLINK_DISABLE;
>>   	}
>>
>> -	led_set_brightness_async(led_cdev, led_cdev->delayed_set_value);
>> +	if (led_cdev->brightness_set)
>> +		led_cdev->brightness_set(led_cdev, led_cdev->brightness);

s/led_cdev->brightness/led_cdev->delayed_set_value/

>> +	else if (led_cdev->brightness_set_blocking)
>> +		ret = led_cdev->brightness_set_blocking(led_cdev,
>> +							led_cdev->brightness);

s/led_cdev->brightness/led_cdev->delayed_set_value/

led_set_brightness_nopm() wouldn't have effect if we relied here on
led_cdev->brightness. The former, being called from
led_classdev_suspend(), turns the LED off, but without overwriting
led_cdev->brightness, so that led_classdev_resume could restore it.

I will submit the fixed set soon.

-- 
Best Regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 06/10] Documentation: leds: Add description of brightness setting API
  2015-10-16 15:36   ` Sakari Ailus
@ 2015-10-18  7:08     ` Jacek Anaszewski
  0 siblings, 0 replies; 22+ messages in thread
From: Jacek Anaszewski @ 2015-10-18  7:08 UTC (permalink / raw)
  To: Sakari Ailus, Jacek Anaszewski
  Cc: linux-leds, linux-kernel, pavel, sakari.ailus, andrew

Hi Sakari,

Thanks for the review.

On 10/16/2015 05:36 PM, Sakari Ailus wrote:
> Hi Jacek,
>
> On Wed, Oct 07, 2015 at 11:10:44AM +0200, Jacek Anaszewski wrote:
>> This patch adds description of the LED subsystem API for
>> setting an LED brightness.
>>
>> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>> ---
>>   Documentation/leds/leds-class.txt |   13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/Documentation/leds/leds-class.txt b/Documentation/leds/leds-class.txt
>> index 62261c0..d406d98 100644
>> --- a/Documentation/leds/leds-class.txt
>> +++ b/Documentation/leds/leds-class.txt
>> @@ -52,6 +52,19 @@ above leaves scope for further attributes should they be needed. If sections
>>   of the name don't apply, just leave that section blank.
>>
>>
>> +Brightness setting API
>> +======================
>> +
>> +LED subsystem core exposes following API for setting brightness:
>> +
>> +    - led_set_brightness : it is guaranteed not to sleep, passing LED_OFF stops
>> +		blinking,
>> +    - led_set_brightness_sync : for use cases when immediate effect is desired -
>
> Over 80 characters per line.

There are exactly 80 characters, checkpatch.pl doesn't complain. You
probably counted patch line length, which includes leading '+'.

> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>
>> +		it can block the caller for the time required for accessing
>> +		device registers and can sleep, passing LED_OFF stops hardware
>> +		blinking, returns -EBUSY if software blink fallback is enabled.
>> +
>> +
>>   Hardware accelerated blink of LEDs
>>   ==================================
>>
>

-- 
Best Regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 10/10] media: flash: use led_set_brightness_sync for torch brightness
  2015-10-07  9:10 ` [PATCH v3 10/10] media: flash: use led_set_brightness_sync for torch brightness Jacek Anaszewski
  2015-10-07 10:49   ` Pavel Machek
@ 2015-11-16  9:35   ` Jacek Anaszewski
  2015-11-16  9:47     ` Sakari Ailus
  1 sibling, 1 reply; 22+ messages in thread
From: Jacek Anaszewski @ 2015-11-16  9:35 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linux-leds, linux-kernel, pavel, sakari.ailus, andrew,
	linux-media

Hi Mauro,

This patch depends on the preceding LED core improvements patches
from this patch set, and it would be best if it was merged through
the LED tree. Can I get your ack for this? I've already obtained acks
for the whole set from Sakari.

Best Regards,
Jacek Anaszewski

On 10/07/2015 11:10 AM, Jacek Anaszewski wrote:
> LED subsystem shifted responsibility for choosing between SYNC or ASYNC
> way of setting brightness from drivers to the caller. Adapt the wrapper
> to those changes.
>
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: linux-media@vger.kernel.org
> ---
>   drivers/media/v4l2-core/v4l2-flash-led-class.c |    8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-flash-led-class.c b/drivers/media/v4l2-core/v4l2-flash-led-class.c
> index 5bdfb8d..5d67335 100644
> --- a/drivers/media/v4l2-core/v4l2-flash-led-class.c
> +++ b/drivers/media/v4l2-core/v4l2-flash-led-class.c
> @@ -107,10 +107,10 @@ static void v4l2_flash_set_led_brightness(struct v4l2_flash *v4l2_flash,
>   		if (ctrls[LED_MODE]->val != V4L2_FLASH_LED_MODE_TORCH)
>   			return;
>
> -		led_set_brightness(&v4l2_flash->fled_cdev->led_cdev,
> +		led_set_brightness_sync(&v4l2_flash->fled_cdev->led_cdev,
>   					brightness);
>   	} else {
> -		led_set_brightness(&v4l2_flash->iled_cdev->led_cdev,
> +		led_set_brightness_sync(&v4l2_flash->iled_cdev->led_cdev,
>   					brightness);
>   	}
>   }
> @@ -206,11 +206,11 @@ static int v4l2_flash_s_ctrl(struct v4l2_ctrl *c)
>   	case V4L2_CID_FLASH_LED_MODE:
>   		switch (c->val) {
>   		case V4L2_FLASH_LED_MODE_NONE:
> -			led_set_brightness(led_cdev, LED_OFF);
> +			led_set_brightness_sync(led_cdev, LED_OFF);
>   			return led_set_flash_strobe(fled_cdev, false);
>   		case V4L2_FLASH_LED_MODE_FLASH:
>   			/* Turn the torch LED off */
> -			led_set_brightness(led_cdev, LED_OFF);
> +			led_set_brightness_sync(led_cdev, LED_OFF);
>   			if (ctrls[STROBE_SOURCE]) {
>   				external_strobe = (ctrls[STROBE_SOURCE]->val ==
>   					V4L2_FLASH_STROBE_SOURCE_EXTERNAL);
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 10/10] media: flash: use led_set_brightness_sync for torch brightness
  2015-11-16  9:35   ` Jacek Anaszewski
@ 2015-11-16  9:47     ` Sakari Ailus
  2015-11-16 13:34       ` Jacek Anaszewski
  2015-12-03 13:35       ` Mauro Carvalho Chehab
  0 siblings, 2 replies; 22+ messages in thread
From: Sakari Ailus @ 2015-11-16  9:47 UTC (permalink / raw)
  To: Jacek Anaszewski, Mauro Carvalho Chehab
  Cc: linux-leds, linux-kernel, pavel, andrew, linux-media

Jacek Anaszewski wrote:
> This patch depends on the preceding LED core improvements patches
> from this patch set, and it would be best if it was merged through
> the LED tree. Can I get your ack for this? I've already obtained acks
> for the whole set from Sakari.

I agree with this going through the LED tree.

-- 
Sakari Ailus
sakari.ailus@linux.intel.com

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 10/10] media: flash: use led_set_brightness_sync for torch brightness
  2015-11-16  9:47     ` Sakari Ailus
@ 2015-11-16 13:34       ` Jacek Anaszewski
  2015-12-03 13:35       ` Mauro Carvalho Chehab
  1 sibling, 0 replies; 22+ messages in thread
From: Jacek Anaszewski @ 2015-11-16 13:34 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Mauro Carvalho Chehab, linux-leds, linux-kernel, pavel, andrew,
	linux-media

On 11/16/2015 10:47 AM, Sakari Ailus wrote:
> Jacek Anaszewski wrote:
>> This patch depends on the preceding LED core improvements patches
>> from this patch set, and it would be best if it was merged through
>> the LED tree. Can I get your ack for this? I've already obtained acks
>> for the whole set from Sakari.
>
> I agree with this going through the LED tree.
>

Applied this patch set, with fixed version of the patch 4/10 [1],
thanks.

[1] http://www.spinics.net/lists/linux-leds/msg05045.html
-- 
Best Regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v3 10/10] media: flash: use led_set_brightness_sync for torch brightness
  2015-11-16  9:47     ` Sakari Ailus
  2015-11-16 13:34       ` Jacek Anaszewski
@ 2015-12-03 13:35       ` Mauro Carvalho Chehab
  1 sibling, 0 replies; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2015-12-03 13:35 UTC (permalink / raw)
  To: Sakari Ailus
  Cc: Jacek Anaszewski, linux-leds, linux-kernel, pavel, andrew,
	linux-media

Em Mon, 16 Nov 2015 11:47:58 +0200
Sakari Ailus <sakari.ailus@linux.intel.com> escreveu:

> Jacek Anaszewski wrote:
> > This patch depends on the preceding LED core improvements patches
> > from this patch set, and it would be best if it was merged through
> > the LED tree. Can I get your ack for this? I've already obtained acks
> > for the whole set from Sakari.
> 
> I agree with this going through the LED tree.

Feel free to send it via the LED tree.

Acked-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

Regards,
Mauro

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2015-12-03 13:35 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-07  9:10 [PATCH v3 00/10] LED core improvements Jacek Anaszewski
2015-10-07  9:10 ` [PATCH v3 01/10] leds: core: Add two new LED_BLINK_ flags Jacek Anaszewski
2015-10-16 15:32   ` Sakari Ailus
2015-10-07  9:10 ` [PATCH v3 02/10] leds: Rename brightness_set_sync op to brightness_set_blocking Jacek Anaszewski
2015-10-16 15:34   ` Sakari Ailus
2015-10-07  9:10 ` [PATCH v3 03/10] leds: core: Add led_set_brightness_nosleep{nopm} functions Jacek Anaszewski
2015-10-07  9:10 ` [PATCH v3 04/10] leds: core: Use set_brightness_work for the blocking op Jacek Anaszewski
2015-10-16 15:43   ` Sakari Ailus
2015-10-18  7:06     ` Jacek Anaszewski
2015-10-07  9:10 ` [PATCH v3 05/10] leds: core: Drivers shouldn't enforce SYNC/ASYNC brightness setting Jacek Anaszewski
2015-10-07  9:10 ` [PATCH v3 06/10] Documentation: leds: Add description of brightness setting API Jacek Anaszewski
2015-10-16 15:36   ` Sakari Ailus
2015-10-18  7:08     ` Jacek Anaszewski
2015-10-07  9:10 ` [PATCH v3 07/10] leds: max77693: Remove work queue Jacek Anaszewski
2015-10-07  9:10 ` [PATCH v3 08/10] leds: aat1290: " Jacek Anaszewski
2015-10-07  9:10 ` [PATCH v3 09/10] leds: ktd2692: " Jacek Anaszewski
2015-10-07  9:10 ` [PATCH v3 10/10] media: flash: use led_set_brightness_sync for torch brightness Jacek Anaszewski
2015-10-07 10:49   ` Pavel Machek
2015-11-16  9:35   ` Jacek Anaszewski
2015-11-16  9:47     ` Sakari Ailus
2015-11-16 13:34       ` Jacek Anaszewski
2015-12-03 13:35       ` Mauro Carvalho Chehab

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).