2015-08-20 14:44:28

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 00/36] Remove work queues from LED class drivers

This is sixth version of the RFC aiming at removing work queues
from LED class drivers, as well as getting rid of complimentary
functionalities introduced along with addition of LED flash class
extension.

======================
Changes from version 5
======================
- avoided changing semantics of brightness_set op in favour
of introducing brightness_set_blocking op, which also allowed
to get rid of LED_BRIGHTNESS_BLOCKING flag - Andrew thanks for
the hints
- made new brightness_set_blocking op returning int, which entailed
the need for extending the scope of modifications in the drivers
from which work queues are removed. Those affected drivers now
implement the new op instead of brightness_set, which returns void
- since LED_BRIGHTNESS_FAST flags is not needed with the new
approach, its introduction has been postponed

======================
Changes from version 4
======================
- switched to using two separate ops for blocking and non-blocking way
of setting brightness as requested by Pavel Machek.
- improved patches for leds-lm3533, leds-regulator, leds-lp3944
and leds-ipaq-micro drivers in response to review remarks

======================
Changes from version 3
======================
- fixed return value in one of intermediary patches
- changed the comment over the brightness_set op member
of struct led_classdev
- added patches adjusting LED subsystem drivers to the introduced
modifications - they have been only compile-tested

======================
Changes from version 2
======================

- split changes to several incremental patches
- removed SET_BRIGHTNESS_SYNC and SET_BRIGHTNESS_ASYNC flags
- fixed led_set_brightness_async function instead of renaming it

======================
Changes from version 1
======================

V2 includes also patches for one LED class driver
and two LED flash class drivers, that show how the
drivers will benefit from the optimization being
introduced in the first patch of this patch set.

I was able to test only the LED Flash class drivers.

Original message from the patch 483a3122 ("leds: Use set_brightness_work for
brightness_set ops that can sleep") that was sent previously as a single one:

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

Andrew Lunn (23):
leds: tlc591xx: Remove work queue
leds: 88pm860x: Remove work queue
leds: adp5520: Remove work queue
leds: bd2802: Remove work queue
leds: blinkm: Remove work queue
leds: lm3533: Remove work queue
leds: lm3642: Remove work queue
leds: pca9532: Remove work queue for LEDs.
leds: lp3944: Remove work queue
leds: lp55xx: Remove work queue
leds: lp8788: Remove work queue
leds: lp8860: Remove work queue
leds: pca955x: Remove work queue
leds: pca963x: Remove work queue
leds: wm831x: Remove work queue
leds: da903x: Remove work queue
leds: da9052: Remove work queue
leds: dac124d085: Remove work queue
leds: lt3593: Remove work queue
leds: max8997: Remove unneeded workqueue include
leds: mc13783: Remove work queue
leds: regulator: Remove work queue
leds: wm8350: Remove work queue

Jacek Anaszewski (13):
leds: Add brightness_set_blocking op
leds: Add led_set_brightness_sync to the public LED subsystem API
leds: Improve asynchronous path of setting brightness
leds: Add an internal led_set_brightness_nosleep function
leds: Improve setting brightness in a non sleeping way
leds: Drivers shouldn't enforce SYNC/ASYNC brightness setting
Documentation: leds: Add description of brightness_set* ops
leds: ktd2692: Remove work queue
leds: aat1290: Remove work queue
leds: max77693: Remove work queue
leds: gpio: Remove work queue
leds: pwm: remove work queue
leds: lm355x: Remove work queue

Documentation/leds/leds-class.txt | 11 ++++
drivers/leds/led-class-flash.c | 7 ---
drivers/leds/led-class.c | 22 +++++---
drivers/leds/led-core.c | 79 ++++++++++++++++++++------
drivers/leds/leds-88pm860x.c | 23 +++-----
drivers/leds/leds-aat1290.c | 50 +++++------------
drivers/leds/leds-adp5520.c | 26 ++-------
drivers/leds/leds-bd2802.c | 39 +++++--------
drivers/leds/leds-blinkm.c | 87 ++++++-----------------------
drivers/leds/leds-da903x.c | 46 ++++++---------
drivers/leds/leds-da9052.c | 39 ++++---------
drivers/leds/leds-dac124s085.c | 38 ++++---------
drivers/leds/leds-gpio.c | 64 +++++++--------------
drivers/leds/leds-ktd2692.c | 41 ++------------
drivers/leds/leds-lm3533.c | 30 ++--------
drivers/leds/leds-lm355x.c | 85 +++++++++-------------------
drivers/leds/leds-lm3642.c | 73 ++++++++----------------
drivers/leds/leds-lp3944.c | 32 +++--------
drivers/leds/leds-lp5521.c | 11 ++--
drivers/leds/leds-lp5523.c | 10 ++--
drivers/leds/leds-lp5562.c | 11 ++--
drivers/leds/leds-lp55xx-common.c | 12 ++--
drivers/leds/leds-lp55xx-common.h | 6 +-
drivers/leds/leds-lp8501.c | 11 ++--
drivers/leds/leds-lp8788.c | 48 +++++++---------
drivers/leds/leds-lp8860.c | 27 +++------
drivers/leds/leds-lt3593.c | 33 ++++-------
drivers/leds/leds-max77693.c | 57 +++----------------
drivers/leds/leds-max8997.c | 1 -
drivers/leds/leds-mc13783.c | 35 +++---------
drivers/leds/leds-pca9532.c | 28 ++++------
drivers/leds/leds-pca955x.c | 39 +++----------
drivers/leds/leds-pca963x.c | 80 ++++++++------------------
drivers/leds/leds-pwm.c | 34 +++++------
drivers/leds/leds-regulator.c | 46 ++++-----------
drivers/leds/leds-tlc591xx.c | 31 +++-------
drivers/leds/leds-wm831x-status.c | 25 +++------
drivers/leds/leds-wm8350.c | 64 +++++++++------------
drivers/leds/leds.h | 26 +++------
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 +--
include/linux/leds.h | 31 +++++++---
include/linux/mfd/wm8350/pmic.h | 1 -
47 files changed, 512 insertions(+), 979 deletions(-)

--
1.7.9.5


2015-08-20 14:44:33

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 01/36] leds: Add brightness_set_blocking op

This patch adds a new brightness_set_blocking op to the LED subsystem.
The op is intended for drivers that set brightness in a blocking way,
i.e. they either can sleep or use delays while setting brightness.

Signed-off-by: Jacek Anaszewski <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Sakari Ailus <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Stas Sergeev <[email protected]>
---
drivers/leds/led-class.c | 3 +++
include/linux/leds.h | 6 ++++++
2 files changed, 9 insertions(+)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index ca51d58..93a2414 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -267,6 +267,9 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
dev_warn(parent, "Led %s renamed to %s due to name collision",
led_cdev->name, dev_name(led_cdev->dev));

+ WARN_ON(led_cdev->brightness_set &&
+ led_cdev->brightness_set_blocking);
+
#ifdef CONFIG_LEDS_TRIGGERS
init_rwsem(&led_cdev->trigger_lock);
#endif
diff --git a/include/linux/leds.h b/include/linux/leds.h
index b122eea..85fad6b 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -54,6 +54,12 @@ struct led_classdev {
void (*brightness_set)(struct led_classdev *led_cdev,
enum led_brightness brightness);
/*
+ * Intended for drivers that either can sleep or use delays while
+ * setting brightness.
+ */
+ int (*brightness_set_blocking)(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.
*/
--
1.7.9.5

2015-08-20 14:44:38

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 02/36] leds: Add led_set_brightness_sync to the public LED subsystem API

led_set_brightness_sync function was visible only internally to the
LED subsystem. It is now being made publicly available since it has
become apparent that this is a caller who should decide whether
brightness is to be set in a synchronous or an asynchronous way.

The function is modified to use brightness_set_blocking or
brightness_set op, depending on which of them is implemented, in case
brightness_set_sync op wasn't been provided.

Signed-off-by: Jacek Anaszewski <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Sakari Ailus <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Stas Sergeev <[email protected]>
---
drivers/leds/led-core.c | 25 +++++++++++++++++++++++++
drivers/leds/leds.h | 13 -------------
include/linux/leds.h | 15 +++++++++++++++
3 files changed, 40 insertions(+), 13 deletions(-)

diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 549de7e..3f3b71d 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -141,6 +141,31 @@ void led_set_brightness(struct led_classdev *led_cdev,
}
EXPORT_SYMBOL(led_set_brightness);

+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)
+ return 0;
+
+ if (led_cdev->brightness_set_sync)
+ ret = led_cdev->brightness_set_sync(led_cdev,
+ led_cdev->brightness);
+ else if (led_cdev->brightness_set_blocking)
+ led_cdev->brightness_set_blocking(led_cdev,
+ led_cdev->brightness);
+ else if (led_cdev->brightness_set)
+ led_cdev->brightness_set(led_cdev, led_cdev->brightness);
+ else
+ ret = -EINVAL;
+
+ return ret;
+}
+EXPORT_SYMBOL(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 bc89d7a..1c026c9 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -26,19 +26,6 @@ static inline void led_set_brightness_async(struct led_classdev *led_cdev,
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;
-}
-
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 85fad6b..2377e0f 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -166,6 +166,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 register,
+ * 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

2015-08-20 14:44:46

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 03/36] leds: Improve asynchronous path of setting brightness

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. Modify the function to assure
setting brightness asynchronously in all cases by using existing
set_brightness_work.

Aforementioned modifications change the initial purpose of
set_brightness_work which was used for setting brightness only if blink
timer was active. In order to keep this functionality LED_BLINK_DISABLE
flag is being introduced, as well as a 'new_brightness_value' field is
being added to the struct led_classdev. set_brightness_delayed callback
now needs to use led_set_brightness_sync for setting brightness.
All these improvements entail changes in the led_brightness_set function.

Signed-off-by: Jacek Anaszewski <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Sakari Ailus <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Stas Sergeev <[email protected]>
---
drivers/leds/led-class.c | 13 ++++++++-----
drivers/leds/led-core.c | 20 ++++++++++++++++----
drivers/leds/leds.h | 9 +++------
include/linux/leds.h | 2 ++
4 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 93a2414..fe11ed8 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -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;
@@ -161,9 +161,12 @@ 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;
+ }

- led_set_brightness_async(led_cdev, led_cdev->delayed_set_value);
+ led_set_brightness_sync(led_cdev, led_cdev->delayed_set_value);
}

/**
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 3f3b71d..b69271f 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -119,11 +119,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)
- schedule_work(&led_cdev->set_brightness_work);
+ 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;
+ led_set_brightness_async(led_cdev, brightness);
return;
}

diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index 1c026c9..ca38f6a 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -17,13 +17,10 @@
#include <linux/leds.h>

static inline void led_set_brightness_async(struct led_classdev *led_cdev,
- enum led_brightness value)
+ 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);
+ led_cdev->delayed_set_value = value;
+ schedule_work(&led_cdev->set_brightness_work);
}

static inline int led_get_brightness(struct led_classdev *led_cdev)
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 2377e0f..8fefe72 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -48,6 +48,7 @@ struct led_classdev {
#define SET_BRIGHTNESS_ASYNC (1 << 21)
#define SET_BRIGHTNESS_SYNC (1 << 22)
#define LED_DEV_CAP_FLASH (1 << 23)
+#define LED_BLINK_DISABLE (1 << 24)

/* Set LED brightness level */
/* Must not sleep, use a workqueue if needed */
@@ -93,6 +94,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

2015-08-20 14:44:55

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 04/36] leds: Add an internal led_set_brightness_nosleep function

This patch adds led_set_brightness_nosleep function. It guarantees setting
LED brightness in a non-blocking way.

Signed-off-by: Jacek Anaszewski <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Sakari Ailus <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Stas Sergeev <[email protected]>
---
drivers/leds/led-core.c | 21 +++++++++++++++++++++
drivers/leds/leds.h | 4 ++++
2 files changed, 25 insertions(+)

diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index b69271f..0200407 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -114,6 +114,27 @@ void led_stop_software_blink(struct led_classdev *led_cdev)
}
EXPORT_SYMBOL_GPL(led_stop_software_blink);

+void led_set_brightness_nosleep(struct led_classdev *led_cdev,
+ enum led_brightness value)
+{
+ int ret;
+
+ /*
+ * Drivers that implement brightness_set op are guaranteed
+ * not to sleep while setting brightness.
+ */
+ if (led_cdev->brightness_set) {
+ ret = led_set_brightness_sync(led_cdev, value);
+ if (ret < 0)
+ dev_err(led_cdev->dev,
+ "cannot set led brightness %d\n", ret);
+ return;
+ }
+
+ led_set_brightness_async(led_cdev, value);
+}
+EXPORT_SYMBOL_GPL(led_set_brightness_nosleep);
+
void led_set_brightness(struct led_classdev *led_cdev,
enum led_brightness brightness)
{
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index ca38f6a..cd251be 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -2,8 +2,10 @@
* LED Core
*
* Copyright 2005 Openedhand Ltd.
+ * Copyright 2014, 2015 Samsung Electronics Co., Ltd.
*
* Author: Richard Purdie <[email protected]>
+ * Author: Jacek Anaszewski <[email protected]>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -28,6 +30,8 @@ static inline int led_get_brightness(struct led_classdev *led_cdev)
return led_cdev->brightness;
}

+void led_set_brightness_nosleep(struct led_classdev *led_cdev,
+ enum led_brightness value);
void led_stop_software_blink(struct led_classdev *led_cdev);

extern struct rw_semaphore leds_list_lock;
--
1.7.9.5

2015-08-20 14:45:07

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 05/36] leds: Improve setting brightness in a non sleeping way

This patch 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 <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Sakari Ailus <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Stas Sergeev <[email protected]>
---
drivers/leds/led-class.c | 4 ++--
drivers/leds/led-core.c | 5 +++--
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 ++++----
8 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index fe11ed8..241059d 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;
}

@@ -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 +
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 0200407..956c8d8 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;
}

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

2015-08-20 14:45:19

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 06/36] leds: Drivers shouldn't enforce SYNC/ASYNC brightness setting

This patch removes brightness_set_sync op as well as SET_BRIGHTNESS_ASYNC
and SET_BRIGHTNESS sync flags. led_set_brightness now calls
led_set_brightness_nosleep rather then choosing between sync and async op
basing on the flags defined by the driver. Now if a user wants to make
sure that brightness will be set synchronously they have to use
led_set_brightness_sync API.

Signed-off-by: Jacek Anaszewski <[email protected]>
Cc: Ingi Kim <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Sakari Ailus <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Stas Sergeev <[email protected]>
---
drivers/leds/led-class-flash.c | 7 -------
drivers/leds/led-class.c | 2 --
drivers/leds/led-core.c | 24 ++++--------------------
drivers/leds/leds-aat1290.c | 12 ------------
drivers/leds/leds-ktd2692.c | 12 ------------
drivers/leds/leds-max77693.c | 12 ------------
include/linux/leds.h | 14 +++-----------
7 files changed, 7 insertions(+), 76 deletions(-)

diff --git a/drivers/leds/led-class-flash.c b/drivers/leds/led-class-flash.c
index 3b25734..68c3f36 100644
--- a/drivers/leds/led-class-flash.c
+++ b/drivers/leds/led-class-flash.c
@@ -298,9 +298,6 @@ 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)
- return -EINVAL;
-
ops = fled_cdev->ops;
if (!ops || !ops->strobe_set)
return -EINVAL;
@@ -316,10 +313,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 241059d..8e9ed00 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -285,8 +285,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);

INIT_WORK(&led_cdev->set_brightness_work, set_brightness_delayed);
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 956c8d8..dc10c54 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -139,8 +139,6 @@ EXPORT_SYMBOL_GPL(led_set_brightness_nosleep);
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.
@@ -158,20 +156,9 @@ void led_set_brightness(struct led_classdev *led_cdev,
*/
led_cdev->flags |= LED_BLINK_DISABLE;
led_set_brightness_async(led_cdev, brightness);
- return;
+ } else {
+ led_set_brightness_nosleep(led_cdev, brightness);
}
-
- 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);
}
EXPORT_SYMBOL(led_set_brightness);

@@ -185,12 +172,9 @@ int led_set_brightness_sync(struct led_classdev *led_cdev,
if (led_cdev->flags & LED_SUSPENDED)
return 0;

- if (led_cdev->brightness_set_sync)
- ret = led_cdev->brightness_set_sync(led_cdev,
- led_cdev->brightness);
- else if (led_cdev->brightness_set_blocking)
+ if (led_cdev->brightness_set_blocking)
led_cdev->brightness_set_blocking(led_cdev,
- led_cdev->brightness);
+ led_cdev->brightness);
else if (led_cdev->brightness_set)
led_cdev->brightness_set(led_cdev, led_cdev->brightness);
else
diff --git a/drivers/leds/leds-aat1290.c b/drivers/leds/leds-aat1290.c
index fd7c25f..0ecaf76 100644
--- a/drivers/leds/leds-aat1290.c
+++ b/drivers/leds/leds-aat1290.c
@@ -180,17 +180,6 @@ static void aat1290_led_brightness_set(struct led_classdev *led_cdev,
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;
-}
-
static int aat1290_led_flash_strobe_set(struct led_classdev_flash *fled_cdev,
bool state)

@@ -510,7 +499,6 @@ 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->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 2ae8c4d..7f6758d 100644
--- a/drivers/leds/leds-ktd2692.c
+++ b/drivers/leds/leds-ktd2692.c
@@ -194,17 +194,6 @@ static void ktd2692_led_brightness_set(struct led_classdev *led_cdev,
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;
-}
-
static int ktd2692_led_flash_strobe_set(struct led_classdev_flash *fled_cdev,
bool state)
{
@@ -382,7 +371,6 @@ 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->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 b8b0eec..0eade1ff 100644
--- a/drivers/leds/leds-max77693.c
+++ b/drivers/leds/leds-max77693.c
@@ -510,17 +510,6 @@ static void max77693_led_brightness_set_work(

/* 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)
@@ -931,7 +920,6 @@ 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->max_brightness = (led->iout_joint ?
led_cfg->iout_torch_max[FLED1] +
led_cfg->iout_torch_max[FLED2] :
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 8fefe72..804cbe1 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -45,10 +45,8 @@ struct led_classdev {
#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_DISABLE (1 << 24)
+#define LED_DEV_CAP_FLASH (1 << 21)
+#define LED_BLINK_DISABLE (1 << 22)

/* Set LED brightness level */
/* Must not sleep, use a workqueue if needed */
@@ -60,12 +58,6 @@ struct led_classdev {
*/
int (*brightness_set_blocking)(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);

@@ -164,7 +156,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

2015-08-20 14:45:30

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 07/36] Documentation: leds: Add description of brightness_set* ops

This patch adds description of brightness_set and a recently introduced
brightness_set_nonblocking op.

Signed-off-by: Jacek Anaszewski <[email protected]>
Cc: Andrew Lunn <[email protected]>
Cc: Sakari Ailus <[email protected]>
Cc: Pavel Machek <[email protected]>
Cc: Stas Sergeev <[email protected]>
---
Documentation/leds/leds-class.txt | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/Documentation/leds/leds-class.txt b/Documentation/leds/leds-class.txt
index 62261c0..c36a6b8 100644
--- a/Documentation/leds/leds-class.txt
+++ b/Documentation/leds/leds-class.txt
@@ -52,6 +52,17 @@ 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 callbacks
+============================
+
+LED subsystem core exposes two function pointers for setting brightness:
+
+ - brightness_set : Intended for drivers that set brightness in
+ a non-blocking way,
+ - brightness_set_blocking : Intended for drivers that either sleep or
+ use delays while setting brightness.
+
+
Hardware accelerated blink of LEDs
==================================

--
1.7.9.5

2015-08-20 14:45:42

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 08/36] leds: ktd2692: Remove work queue

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 <[email protected]>
Cc: Sakari Ailus <[email protected]>
Cc: Ingi Kim <[email protected]>
---
drivers/leds/leds-ktd2692.c | 31 +++++++------------------------
1 file changed, 7 insertions(+), 24 deletions(-)

diff --git a/drivers/leds/leds-ktd2692.c b/drivers/leds/leds-ktd2692.c
index 7f6758d..d70af1a 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,24 +175,8 @@ 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);
+ return 0;
}

static int ktd2692_led_flash_strobe_set(struct led_classdev_flash *fled_cdev,
@@ -370,11 +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;
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);

@@ -396,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

2015-08-20 14:45:48

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 09/36] leds: aat1290: Remove work queue

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 <[email protected]>
Cc: Sakari Ailus <[email protected]>
Cc: Bryan Wu <[email protected]>
---
drivers/leds/leds-aat1290.c | 40 ++++++++++++++--------------------------
1 file changed, 14 insertions(+), 26 deletions(-)

diff --git a/drivers/leds/leds-aat1290.c b/drivers/leds/leds-aat1290.c
index 0ecaf76..d802fbb 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,26 +166,8 @@ 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);
+ return 0;
}

static int aat1290_led_flash_strobe_set(struct led_classdev_flash *fled_cdev,
@@ -498,10 +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;
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);

@@ -536,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

2015-08-20 14:45:55

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 10/36] leds: max77693: Remove work queue

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 <[email protected]>
Cc: Sakari Ailus <[email protected]>
---
drivers/leds/leds-max77693.c | 45 +++++++++---------------------------------
1 file changed, 9 insertions(+), 36 deletions(-)

diff --git a/drivers/leds/leds-max77693.c b/drivers/leds/leds-max77693.c
index 0eade1ff..28f3d05 100644
--- a/drivers/leds/leds-max77693.c
+++ b/drivers/leds/leds-max77693.c
@@ -19,7 +19,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
@@ -61,8 +60,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;

@@ -462,10 +459,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);

@@ -493,32 +494,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 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(
@@ -919,15 +896,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;
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);

@@ -1049,13 +1024,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

2015-08-20 14:46:07

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 11/36] leds: tlc591xx: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
drivers/leds/leds-tlc591xx.c | 31 +++++++------------------------
1 file changed, 7 insertions(+), 24 deletions(-)

diff --git a/drivers/leds/leds-tlc591xx.c b/drivers/leds/leds-tlc591xx.c
index b806eca..30453164 100644
--- a/drivers/leds/leds-tlc591xx.c
+++ b/drivers/leds/leds-tlc591xx.c
@@ -14,7 +14,6 @@
#include <linux/of_device.h>
#include <linux/regmap.h>
#include <linux/slab.h>
-#include <linux/workqueue.h>

#define TLC591XX_MAX_LEDS 16

@@ -42,13 +41,11 @@
#define LEDOUT_MASK 0x3

#define ldev_to_led(c) container_of(c, struct tlc591xx_led, ldev)
-#define work_to_led(work) container_of(work, struct tlc591xx_led, work)

struct tlc591xx_led {
bool active;
unsigned int led_no;
struct led_classdev ldev;
- struct work_struct work;
struct tlc591xx_priv *priv;
};

@@ -110,12 +107,12 @@ tlc591xx_set_pwm(struct tlc591xx_priv *priv, struct tlc591xx_led *led,
return regmap_write(priv->regmap, pwm, brightness);
}

-static void
-tlc591xx_led_work(struct work_struct *work)
+static int
+tlc591xx_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
{
- struct tlc591xx_led *led = work_to_led(work);
+ struct tlc591xx_led *led = ldev_to_led(led_cdev);
struct tlc591xx_priv *priv = led->priv;
- enum led_brightness brightness = led->ldev.brightness;
int err;

switch (brightness) {
@@ -131,18 +128,7 @@ tlc591xx_led_work(struct work_struct *work)
err = tlc591xx_set_pwm(priv, led, brightness);
}

- if (err)
- dev_err(led->ldev.dev, "Failed setting brightness\n");
-}
-
-static void
-tlc591xx_brightness_set(struct led_classdev *led_cdev,
- enum led_brightness brightness)
-{
- struct tlc591xx_led *led = ldev_to_led(led_cdev);
-
- led->ldev.brightness = brightness;
- schedule_work(&led->work);
+ return err;
}

static void
@@ -151,10 +137,8 @@ tlc591xx_destroy_devices(struct tlc591xx_priv *priv, unsigned int j)
int i = j;

while (--i >= 0) {
- if (priv->leds[i].active) {
+ if (priv->leds[i].active)
led_classdev_unregister(&priv->leds[i].ldev);
- cancel_work_sync(&priv->leds[i].work);
- }
}
}

@@ -175,9 +159,8 @@ tlc591xx_configure(struct device *dev,

led->priv = priv;
led->led_no = i;
- led->ldev.brightness_set = tlc591xx_brightness_set;
+ led->ldev.brightness_set_blocking = tlc591xx_brightness_set;
led->ldev.max_brightness = LED_FULL;
- INIT_WORK(&led->work, tlc591xx_led_work);
err = led_classdev_register(dev, &led->ldev);
if (err < 0) {
dev_err(dev, "couldn't register LED %s\n",
--
1.7.9.5

2015-08-20 14:46:14

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 12/36] leds: 88pm860x: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
drivers/leds/leds-88pm860x.c | 23 +++++++----------------
1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/leds/leds-88pm860x.c b/drivers/leds/leds-88pm860x.c
index 1497a09..c9fcf49 100644
--- a/drivers/leds/leds-88pm860x.c
+++ b/drivers/leds/leds-88pm860x.c
@@ -16,7 +16,6 @@
#include <linux/i2c.h>
#include <linux/leds.h>
#include <linux/slab.h>
-#include <linux/workqueue.h>
#include <linux/mfd/88pm860x.h>
#include <linux/module.h>

@@ -33,7 +32,6 @@
struct pm860x_led {
struct led_classdev cdev;
struct i2c_client *i2c;
- struct work_struct work;
struct pm860x_chip *chip;
struct mutex lock;
char name[MFD_NAME_SIZE];
@@ -69,17 +67,18 @@ static int led_power_set(struct pm860x_chip *chip, int port, int on)
return ret;
}

-static void pm860x_led_work(struct work_struct *work)
+static int pm860x_led_set(struct led_classdev *cdev,
+ enum led_brightness value)
{
-
- struct pm860x_led *led;
+ struct pm860x_led *led = container_of(cdev, struct pm860x_led, cdev);
struct pm860x_chip *chip;
unsigned char buf[3];
int ret;

- led = container_of(work, struct pm860x_led, work);
chip = led->chip;
mutex_lock(&led->lock);
+ led->brightness = value >> 3;
+
if ((led->current_brightness == 0) && led->brightness) {
led_power_set(chip, led->port, 1);
if (led->iset) {
@@ -112,15 +111,8 @@ static void pm860x_led_work(struct work_struct *work)
dev_dbg(chip->dev, "Update LED. (reg:%d, brightness:%d)\n",
led->reg_control, led->brightness);
mutex_unlock(&led->lock);
-}

-static void pm860x_led_set(struct led_classdev *cdev,
- enum led_brightness value)
-{
- struct pm860x_led *data = container_of(cdev, struct pm860x_led, cdev);
-
- data->brightness = value >> 3;
- schedule_work(&data->work);
+ return 0;
}

#ifdef CONFIG_OF
@@ -212,9 +204,8 @@ static int pm860x_led_probe(struct platform_device *pdev)

data->current_brightness = 0;
data->cdev.name = data->name;
- data->cdev.brightness_set = pm860x_led_set;
+ data->cdev.brightness_set_blocking = pm860x_led_set;
mutex_init(&data->lock);
- INIT_WORK(&data->work, pm860x_led_work);

ret = led_classdev_register(chip->dev, &data->cdev);
if (ret < 0) {
--
1.7.9.5

2015-08-20 14:46:22

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 13/36] leds: adp5520: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Cc: Michael Hennerich <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
drivers/leds/leds-adp5520.c | 26 +++++---------------------
1 file changed, 5 insertions(+), 21 deletions(-)

diff --git a/drivers/leds/leds-adp5520.c b/drivers/leds/leds-adp5520.c
index 07e66ca..853b2d3 100644
--- a/drivers/leds/leds-adp5520.c
+++ b/drivers/leds/leds-adp5520.c
@@ -17,34 +17,24 @@
#include <linux/kernel.h>
#include <linux/platform_device.h>
#include <linux/leds.h>
-#include <linux/workqueue.h>
#include <linux/mfd/adp5520.h>
#include <linux/slab.h>

struct adp5520_led {
struct led_classdev cdev;
- struct work_struct work;
struct device *master;
- enum led_brightness new_brightness;
int id;
int flags;
};

-static void adp5520_led_work(struct work_struct *work)
-{
- struct adp5520_led *led = container_of(work, struct adp5520_led, work);
- adp5520_write(led->master, ADP5520_LED1_CURRENT + led->id - 1,
- led->new_brightness >> 2);
-}
-
-static void adp5520_led_set(struct led_classdev *led_cdev,
+static int adp5520_led_set(struct led_classdev *led_cdev,
enum led_brightness value)
{
struct adp5520_led *led;

led = container_of(led_cdev, struct adp5520_led, cdev);
- led->new_brightness = value;
- schedule_work(&led->work);
+ return adp5520_write(led->master, ADP5520_LED1_CURRENT + led->id - 1,
+ value >> 2);
}

static int adp5520_led_setup(struct adp5520_led *led)
@@ -135,7 +125,7 @@ static int adp5520_led_probe(struct platform_device *pdev)

led_dat->cdev.name = cur_led->name;
led_dat->cdev.default_trigger = cur_led->default_trigger;
- led_dat->cdev.brightness_set = adp5520_led_set;
+ led_dat->cdev.brightness_set_blocking = adp5520_led_set;
led_dat->cdev.brightness = LED_OFF;

if (cur_led->flags & ADP5520_FLAG_LED_MASK)
@@ -146,9 +136,6 @@ static int adp5520_led_probe(struct platform_device *pdev)
led_dat->id = led_dat->flags & ADP5520_FLAG_LED_MASK;

led_dat->master = pdev->dev.parent;
- led_dat->new_brightness = LED_OFF;
-
- INIT_WORK(&led_dat->work, adp5520_led_work);

ret = led_classdev_register(led_dat->master, &led_dat->cdev);
if (ret) {
@@ -170,10 +157,8 @@ static int adp5520_led_probe(struct platform_device *pdev)

err:
if (i > 0) {
- for (i = i - 1; i >= 0; i--) {
+ for (i = i - 1; i >= 0; i--)
led_classdev_unregister(&led[i].cdev);
- cancel_work_sync(&led[i].work);
- }
}

return ret;
@@ -192,7 +177,6 @@ static int adp5520_led_remove(struct platform_device *pdev)

for (i = 0; i < pdata->num_leds; i++) {
led_classdev_unregister(&led[i].cdev);
- cancel_work_sync(&led[i].work);
}

return 0;
--
1.7.9.5

2015-08-20 14:46:32

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 14/36] leds: bd2802: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Cc: Kim Kyuwon <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
drivers/leds/leds-bd2802.c | 39 ++++++++++++++-------------------------
1 file changed, 14 insertions(+), 25 deletions(-)

diff --git a/drivers/leds/leds-bd2802.c b/drivers/leds/leds-bd2802.c
index 6078c15d..6b4de76 100644
--- a/drivers/leds/leds-bd2802.c
+++ b/drivers/leds/leds-bd2802.c
@@ -72,7 +72,6 @@ struct bd2802_led {
struct bd2802_led_platform_data *pdata;
struct i2c_client *client;
struct rw_semaphore rwsem;
- struct work_struct work;

struct led_state led[2];

@@ -518,29 +517,22 @@ static struct device_attribute *bd2802_attributes[] = {
&bd2802_rgb_current_attr,
};

-static void bd2802_led_work(struct work_struct *work)
-{
- struct bd2802_led *led = container_of(work, struct bd2802_led, work);
-
- if (led->state)
- bd2802_turn_on(led, led->led_id, led->color, led->state);
- else
- bd2802_turn_off(led, led->led_id, led->color);
-}
-
#define BD2802_CONTROL_RGBS(name, id, clr) \
-static void bd2802_set_##name##_brightness(struct led_classdev *led_cdev,\
+static int bd2802_set_##name##_brightness(struct led_classdev *led_cdev,\
enum led_brightness value) \
{ \
struct bd2802_led *led = \
container_of(led_cdev, struct bd2802_led, cdev_##name); \
led->led_id = id; \
led->color = clr; \
- if (value == LED_OFF) \
+ if (value == LED_OFF) { \
led->state = BD2802_OFF; \
- else \
+ bd2802_turn_off(led, led->led_id, led->color); \
+ } else { \
led->state = BD2802_ON; \
- schedule_work(&led->work); \
+ bd2802_turn_on(led, led->led_id, led->color, BD2802_ON);\
+ } \
+ return 0; \
} \
static int bd2802_set_##name##_blink(struct led_classdev *led_cdev, \
unsigned long *delay_on, unsigned long *delay_off) \
@@ -552,7 +544,7 @@ static int bd2802_set_##name##_blink(struct led_classdev *led_cdev, \
led->led_id = id; \
led->color = clr; \
led->state = BD2802_BLINK; \
- schedule_work(&led->work); \
+ bd2802_turn_on(led, led->led_id, led->color, BD2802_BLINK); \
return 0; \
}

@@ -567,11 +559,9 @@ static int bd2802_register_led_classdev(struct bd2802_led *led)
{
int ret;

- INIT_WORK(&led->work, bd2802_led_work);
-
led->cdev_led1r.name = "led1_R";
led->cdev_led1r.brightness = LED_OFF;
- led->cdev_led1r.brightness_set = bd2802_set_led1r_brightness;
+ led->cdev_led1r.brightness_set_blocking = bd2802_set_led1r_brightness;
led->cdev_led1r.blink_set = bd2802_set_led1r_blink;

ret = led_classdev_register(&led->client->dev, &led->cdev_led1r);
@@ -583,7 +573,7 @@ static int bd2802_register_led_classdev(struct bd2802_led *led)

led->cdev_led1g.name = "led1_G";
led->cdev_led1g.brightness = LED_OFF;
- led->cdev_led1g.brightness_set = bd2802_set_led1g_brightness;
+ led->cdev_led1g.brightness_set_blocking = bd2802_set_led1g_brightness;
led->cdev_led1g.blink_set = bd2802_set_led1g_blink;

ret = led_classdev_register(&led->client->dev, &led->cdev_led1g);
@@ -595,7 +585,7 @@ static int bd2802_register_led_classdev(struct bd2802_led *led)

led->cdev_led1b.name = "led1_B";
led->cdev_led1b.brightness = LED_OFF;
- led->cdev_led1b.brightness_set = bd2802_set_led1b_brightness;
+ led->cdev_led1b.brightness_set_blocking = bd2802_set_led1b_brightness;
led->cdev_led1b.blink_set = bd2802_set_led1b_blink;

ret = led_classdev_register(&led->client->dev, &led->cdev_led1b);
@@ -607,7 +597,7 @@ static int bd2802_register_led_classdev(struct bd2802_led *led)

led->cdev_led2r.name = "led2_R";
led->cdev_led2r.brightness = LED_OFF;
- led->cdev_led2r.brightness_set = bd2802_set_led2r_brightness;
+ led->cdev_led2r.brightness_set_blocking = bd2802_set_led2r_brightness;
led->cdev_led2r.blink_set = bd2802_set_led2r_blink;

ret = led_classdev_register(&led->client->dev, &led->cdev_led2r);
@@ -619,7 +609,7 @@ static int bd2802_register_led_classdev(struct bd2802_led *led)

led->cdev_led2g.name = "led2_G";
led->cdev_led2g.brightness = LED_OFF;
- led->cdev_led2g.brightness_set = bd2802_set_led2g_brightness;
+ led->cdev_led2g.brightness_set_blocking = bd2802_set_led2g_brightness;
led->cdev_led2g.blink_set = bd2802_set_led2g_blink;

ret = led_classdev_register(&led->client->dev, &led->cdev_led2g);
@@ -631,7 +621,7 @@ static int bd2802_register_led_classdev(struct bd2802_led *led)

led->cdev_led2b.name = "led2_B";
led->cdev_led2b.brightness = LED_OFF;
- led->cdev_led2b.brightness_set = bd2802_set_led2b_brightness;
+ led->cdev_led2b.brightness_set_blocking = bd2802_set_led2b_brightness;
led->cdev_led2b.blink_set = bd2802_set_led2b_blink;
led->cdev_led2b.flags |= LED_CORE_SUSPENDRESUME;

@@ -661,7 +651,6 @@ failed_unregister_led1_R:

static void bd2802_unregister_led_classdev(struct bd2802_led *led)
{
- cancel_work_sync(&led->work);
led_classdev_unregister(&led->cdev_led2b);
led_classdev_unregister(&led->cdev_led2g);
led_classdev_unregister(&led->cdev_led2r);
--
1.7.9.5

2015-08-20 14:46:40

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 15/36] leds: blinkm: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Cc: Jan-Simon Moeller <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
drivers/leds/leds-blinkm.c | 87 +++++++++-----------------------------------
1 file changed, 17 insertions(+), 70 deletions(-)

diff --git a/drivers/leds/leds-blinkm.c b/drivers/leds/leds-blinkm.c
index d0452b0..617fe97 100644
--- a/drivers/leds/leds-blinkm.c
+++ b/drivers/leds/leds-blinkm.c
@@ -39,16 +39,9 @@ struct blinkm_led {
struct i2c_client *i2c_client;
struct led_classdev led_cdev;
int id;
- atomic_t active;
-};
-
-struct blinkm_work {
- struct blinkm_led *blinkm_led;
- struct work_struct work;
};

#define cdev_to_blmled(c) container_of(c, struct blinkm_led, led_cdev)
-#define work_to_blmwork(c) container_of(c, struct blinkm_work, work)

struct blinkm_data {
struct i2c_client *i2c_client;
@@ -439,65 +432,30 @@ static int blinkm_transfer_hw(struct i2c_client *client, int cmd)
return 0;
}

-static void led_work(struct work_struct *work)
-{
- int ret;
- struct blinkm_led *led;
- struct blinkm_data *data;
- struct blinkm_work *blm_work = work_to_blmwork(work);
-
- led = blm_work->blinkm_led;
- data = i2c_get_clientdata(led->i2c_client);
- ret = blinkm_transfer_hw(led->i2c_client, BLM_GO_RGB);
- atomic_dec(&led->active);
- dev_dbg(&led->i2c_client->dev,
- "# DONE # next_red = %d, next_green = %d,"
- " next_blue = %d, active = %d\n",
- data->next_red, data->next_green,
- data->next_blue, atomic_read(&led->active));
- kfree(blm_work);
-}
-
static int blinkm_led_common_set(struct led_classdev *led_cdev,
enum led_brightness value, int color)
{
/* led_brightness is 0, 127 or 255 - we just use it here as-is */
struct blinkm_led *led = cdev_to_blmled(led_cdev);
struct blinkm_data *data = i2c_get_clientdata(led->i2c_client);
- struct blinkm_work *bl_work;

switch (color) {
case RED:
/* bail out if there's no change */
if (data->next_red == (u8) value)
return 0;
- /* we assume a quite fast sequence here ([off]->on->off)
- * think of network led trigger - we cannot blink that fast, so
- * in case we already have a off->on->off transition queued up,
- * we refuse to queue up more.
- * Revisit: fast-changing brightness. */
- if (atomic_read(&led->active) > 1)
- return 0;
data->next_red = (u8) value;
break;
case GREEN:
/* bail out if there's no change */
if (data->next_green == (u8) value)
return 0;
- /* we assume a quite fast sequence here ([off]->on->off)
- * Revisit: fast-changing brightness. */
- if (atomic_read(&led->active) > 1)
- return 0;
data->next_green = (u8) value;
break;
case BLUE:
/* bail out if there's no change */
if (data->next_blue == (u8) value)
return 0;
- /* we assume a quite fast sequence here ([off]->on->off)
- * Revisit: fast-changing brightness. */
- if (atomic_read(&led->active) > 1)
- return 0;
data->next_blue = (u8) value;
break;

@@ -506,42 +464,31 @@ static int blinkm_led_common_set(struct led_classdev *led_cdev,
return -EINVAL;
}

- bl_work = kzalloc(sizeof(*bl_work), GFP_ATOMIC);
- if (!bl_work)
- return -ENOMEM;
-
- atomic_inc(&led->active);
+ blinkm_transfer_hw(led->i2c_client, BLM_GO_RGB);
dev_dbg(&led->i2c_client->dev,
- "#TO_SCHED# next_red = %d, next_green = %d,"
- " next_blue = %d, active = %d\n",
+ "# DONE # next_red = %d, next_green = %d,"
+ " next_blue = %d\n",
data->next_red, data->next_green,
- data->next_blue, atomic_read(&led->active));
-
- /* a fresh work _item_ for each change */
- bl_work->blinkm_led = led;
- INIT_WORK(&bl_work->work, led_work);
- /* queue work in own queue for easy sync on exit*/
- schedule_work(&bl_work->work);
-
+ data->next_blue);
return 0;
}

-static void blinkm_led_red_set(struct led_classdev *led_cdev,
+static int blinkm_led_red_set(struct led_classdev *led_cdev,
enum led_brightness value)
{
- blinkm_led_common_set(led_cdev, value, RED);
+ return blinkm_led_common_set(led_cdev, value, RED);
}

-static void blinkm_led_green_set(struct led_classdev *led_cdev,
+static int blinkm_led_green_set(struct led_classdev *led_cdev,
enum led_brightness value)
{
- blinkm_led_common_set(led_cdev, value, GREEN);
+ return blinkm_led_common_set(led_cdev, value, GREEN);
}

-static void blinkm_led_blue_set(struct led_classdev *led_cdev,
+static int blinkm_led_blue_set(struct led_classdev *led_cdev,
enum led_brightness value)
{
- blinkm_led_common_set(led_cdev, value, BLUE);
+ return blinkm_led_common_set(led_cdev, value, BLUE);
}

static void blinkm_init_hw(struct i2c_client *client)
@@ -669,7 +616,6 @@ static int blinkm_probe(struct i2c_client *client,
led[i]->id = i;
led[i]->led_cdev.max_brightness = 255;
led[i]->led_cdev.flags = LED_CORE_SUSPENDRESUME;
- atomic_set(&led[i]->active, 0);
switch (i) {
case RED:
snprintf(blinkm_led_name, sizeof(blinkm_led_name),
@@ -677,7 +623,8 @@ static int blinkm_probe(struct i2c_client *client,
client->adapter->nr,
client->addr);
led[i]->led_cdev.name = blinkm_led_name;
- led[i]->led_cdev.brightness_set = blinkm_led_red_set;
+ led[i]->led_cdev.brightness_set_blocking =
+ blinkm_led_red_set;
err = led_classdev_register(&client->dev,
&led[i]->led_cdev);
if (err < 0) {
@@ -693,7 +640,8 @@ static int blinkm_probe(struct i2c_client *client,
client->adapter->nr,
client->addr);
led[i]->led_cdev.name = blinkm_led_name;
- led[i]->led_cdev.brightness_set = blinkm_led_green_set;
+ led[i]->led_cdev.brightness_set_blocking =
+ blinkm_led_green_set;
err = led_classdev_register(&client->dev,
&led[i]->led_cdev);
if (err < 0) {
@@ -709,7 +657,8 @@ static int blinkm_probe(struct i2c_client *client,
client->adapter->nr,
client->addr);
led[i]->led_cdev.name = blinkm_led_name;
- led[i]->led_cdev.brightness_set = blinkm_led_blue_set;
+ led[i]->led_cdev.brightness_set_blocking =
+ blinkm_led_blue_set;
err = led_classdev_register(&client->dev,
&led[i]->led_cdev);
if (err < 0) {
@@ -746,10 +695,8 @@ static int blinkm_remove(struct i2c_client *client)
int i;

/* make sure no workqueue entries are pending */
- for (i = 0; i < 3; i++) {
- flush_scheduled_work();
+ for (i = 0; i < 3; i++)
led_classdev_unregister(&data->blinkm_leds[i].led_cdev);
- }

/* reset rgb */
data->next_red = 0x00;
--
1.7.9.5

2015-08-20 14:46:44

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 16/36] leds: lm3533: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
Cc: Johan Hovold <[email protected]>
---
drivers/leds/leds-lm3533.c | 30 ++++++------------------------
1 file changed, 6 insertions(+), 24 deletions(-)

diff --git a/drivers/leds/leds-lm3533.c b/drivers/leds/leds-lm3533.c
index 6e2e020..196dcb5 100644
--- a/drivers/leds/leds-lm3533.c
+++ b/drivers/leds/leds-lm3533.c
@@ -17,7 +17,6 @@
#include <linux/mutex.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
-#include <linux/workqueue.h>

#include <linux/mfd/lm3533.h>

@@ -53,9 +52,6 @@ struct lm3533_led {

struct mutex mutex;
unsigned long flags;
-
- struct work_struct work;
- u8 new_brightness;
};


@@ -123,27 +119,17 @@ out:
return ret;
}

-static void lm3533_led_work(struct work_struct *work)
-{
- struct lm3533_led *led = container_of(work, struct lm3533_led, work);
-
- dev_dbg(led->cdev.dev, "%s - %u\n", __func__, led->new_brightness);
-
- if (led->new_brightness == 0)
- lm3533_led_pattern_enable(led, 0); /* disable blink */
-
- lm3533_ctrlbank_set_brightness(&led->cb, led->new_brightness);
-}
-
-static void lm3533_led_set(struct led_classdev *cdev,
+static int lm3533_led_set(struct led_classdev *cdev,
enum led_brightness value)
{
struct lm3533_led *led = to_lm3533_led(cdev);

dev_dbg(led->cdev.dev, "%s - %d\n", __func__, value);

- led->new_brightness = value;
- schedule_work(&led->work);
+ if (value == 0)
+ lm3533_led_pattern_enable(led, 0); /* disable blink */
+
+ return lm3533_ctrlbank_set_brightness(&led->cb, value);
}

static enum led_brightness lm3533_led_get(struct led_classdev *cdev)
@@ -693,7 +679,7 @@ static int lm3533_led_probe(struct platform_device *pdev)
led->lm3533 = lm3533;
led->cdev.name = pdata->name;
led->cdev.default_trigger = pdata->default_trigger;
- led->cdev.brightness_set = lm3533_led_set;
+ led->cdev.brightness_set_blocking = lm3533_led_set;
led->cdev.brightness_get = lm3533_led_get;
led->cdev.blink_set = lm3533_led_blink_set;
led->cdev.brightness = LED_OFF;
@@ -701,7 +687,6 @@ static int lm3533_led_probe(struct platform_device *pdev)
led->id = pdev->id;

mutex_init(&led->mutex);
- INIT_WORK(&led->work, lm3533_led_work);

/* The class framework makes a callback to get brightness during
* registration so use parent device (for error reporting) until
@@ -733,7 +718,6 @@ static int lm3533_led_probe(struct platform_device *pdev)

err_unregister:
led_classdev_unregister(&led->cdev);
- flush_work(&led->work);

return ret;
}
@@ -746,7 +730,6 @@ static int lm3533_led_remove(struct platform_device *pdev)

lm3533_ctrlbank_disable(&led->cb);
led_classdev_unregister(&led->cdev);
- flush_work(&led->work);

return 0;
}
@@ -760,7 +743,6 @@ static void lm3533_led_shutdown(struct platform_device *pdev)

lm3533_ctrlbank_disable(&led->cb);
lm3533_led_set(&led->cdev, LED_OFF); /* disable blink */
- flush_work(&led->work);
}

static struct platform_driver lm3533_led_driver = {
--
1.7.9.5

2015-08-20 14:46:47

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 17/36] leds: lm3642: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Cc: Daniel Jeong <[email protected]>
Cc: G.Shark Jeong <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
drivers/leds/leds-lm3642.c | 73 +++++++++++++-------------------------------
1 file changed, 22 insertions(+), 51 deletions(-)

diff --git a/drivers/leds/leds-lm3642.c b/drivers/leds/leds-lm3642.c
index 02ebe34..cada084 100644
--- a/drivers/leds/leds-lm3642.c
+++ b/drivers/leds/leds-lm3642.c
@@ -15,7 +15,6 @@
#include <linux/platform_device.h>
#include <linux/fs.h>
#include <linux/regmap.h>
-#include <linux/workqueue.h>
#include <linux/platform_data/leds-lm3642.h>

#define REG_FILT_TIME (0x0)
@@ -73,10 +72,6 @@ struct lm3642_chip_data {
struct led_classdev cdev_torch;
struct led_classdev cdev_indicator;

- struct work_struct work_flash;
- struct work_struct work_torch;
- struct work_struct work_indicator;
-
u8 br_flash;
u8 br_torch;
u8 br_indicator;
@@ -209,24 +204,18 @@ out_strtoint:

static DEVICE_ATTR(torch_pin, S_IWUSR, NULL, lm3642_torch_pin_store);

-static void lm3642_deferred_torch_brightness_set(struct work_struct *work)
-{
- struct lm3642_chip_data *chip =
- container_of(work, struct lm3642_chip_data, work_torch);
-
- mutex_lock(&chip->lock);
- lm3642_control(chip, chip->br_torch, MODES_TORCH);
- mutex_unlock(&chip->lock);
-}
-
-static void lm3642_torch_brightness_set(struct led_classdev *cdev,
+static int lm3642_torch_brightness_set(struct led_classdev *cdev,
enum led_brightness brightness)
{
struct lm3642_chip_data *chip =
container_of(cdev, struct lm3642_chip_data, cdev_torch);
+ int ret;

+ mutex_lock(&chip->lock);
chip->br_torch = brightness;
- schedule_work(&chip->work_torch);
+ ret = lm3642_control(chip, chip->br_torch, MODES_TORCH);
+ mutex_unlock(&chip->lock);
+ return ret;
}

/* flash */
@@ -266,45 +255,33 @@ out_strtoint:

static DEVICE_ATTR(strobe_pin, S_IWUSR, NULL, lm3642_strobe_pin_store);

-static void lm3642_deferred_strobe_brightness_set(struct work_struct *work)
-{
- struct lm3642_chip_data *chip =
- container_of(work, struct lm3642_chip_data, work_flash);
-
- mutex_lock(&chip->lock);
- lm3642_control(chip, chip->br_flash, MODES_FLASH);
- mutex_unlock(&chip->lock);
-}
-
-static void lm3642_strobe_brightness_set(struct led_classdev *cdev,
+static int lm3642_strobe_brightness_set(struct led_classdev *cdev,
enum led_brightness brightness)
{
struct lm3642_chip_data *chip =
container_of(cdev, struct lm3642_chip_data, cdev_flash);
-
- chip->br_flash = brightness;
- schedule_work(&chip->work_flash);
-}
-
-/* indicator */
-static void lm3642_deferred_indicator_brightness_set(struct work_struct *work)
-{
- struct lm3642_chip_data *chip =
- container_of(work, struct lm3642_chip_data, work_indicator);
+ int ret;

mutex_lock(&chip->lock);
- lm3642_control(chip, chip->br_indicator, MODES_INDIC);
+ chip->br_flash = brightness;
+ ret = lm3642_control(chip, chip->br_flash, MODES_FLASH);
mutex_unlock(&chip->lock);
+ return ret;
}

-static void lm3642_indicator_brightness_set(struct led_classdev *cdev,
+/* indicator */
+static int lm3642_indicator_brightness_set(struct led_classdev *cdev,
enum led_brightness brightness)
{
struct lm3642_chip_data *chip =
container_of(cdev, struct lm3642_chip_data, cdev_indicator);
+ int ret;

+ mutex_lock(&chip->lock);
chip->br_indicator = brightness;
- schedule_work(&chip->work_indicator);
+ ret = lm3642_control(chip, chip->br_indicator, MODES_INDIC);
+ mutex_unlock(&chip->lock);
+ return ret;
}

static const struct regmap_config lm3642_regmap = {
@@ -371,10 +348,9 @@ static int lm3642_probe(struct i2c_client *client,
goto err_out;

/* flash */
- INIT_WORK(&chip->work_flash, lm3642_deferred_strobe_brightness_set);
chip->cdev_flash.name = "flash";
chip->cdev_flash.max_brightness = 16;
- chip->cdev_flash.brightness_set = lm3642_strobe_brightness_set;
+ chip->cdev_flash.brightness_set_blocking = lm3642_strobe_brightness_set;
chip->cdev_flash.default_trigger = "flash";
chip->cdev_flash.groups = lm3642_flash_groups,
err = led_classdev_register((struct device *)
@@ -385,10 +361,9 @@ static int lm3642_probe(struct i2c_client *client,
}

/* torch */
- INIT_WORK(&chip->work_torch, lm3642_deferred_torch_brightness_set);
chip->cdev_torch.name = "torch";
chip->cdev_torch.max_brightness = 8;
- chip->cdev_torch.brightness_set = lm3642_torch_brightness_set;
+ chip->cdev_torch.brightness_set_blocking = lm3642_torch_brightness_set;
chip->cdev_torch.default_trigger = "torch";
chip->cdev_torch.groups = lm3642_torch_groups,
err = led_classdev_register((struct device *)
@@ -399,11 +374,10 @@ static int lm3642_probe(struct i2c_client *client,
}

/* indicator */
- INIT_WORK(&chip->work_indicator,
- lm3642_deferred_indicator_brightness_set);
chip->cdev_indicator.name = "indicator";
chip->cdev_indicator.max_brightness = 8;
- chip->cdev_indicator.brightness_set = lm3642_indicator_brightness_set;
+ chip->cdev_indicator.brightness_set_blocking =
+ lm3642_indicator_brightness_set;
err = led_classdev_register((struct device *)
&client->dev, &chip->cdev_indicator);
if (err < 0) {
@@ -427,11 +401,8 @@ static int lm3642_remove(struct i2c_client *client)
struct lm3642_chip_data *chip = i2c_get_clientdata(client);

led_classdev_unregister(&chip->cdev_indicator);
- flush_work(&chip->work_indicator);
led_classdev_unregister(&chip->cdev_torch);
- flush_work(&chip->work_torch);
led_classdev_unregister(&chip->cdev_flash);
- flush_work(&chip->work_flash);
regmap_write(chip->regmap, REG_ENABLE, 0);
return 0;
}
--
1.7.9.5

2015-08-20 14:46:50

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 18/36] leds: pca9532: Remove work queue for LEDs.

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
drivers/leds/leds-pca9532.c | 28 ++++++++++++----------------
1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c
index 5a6363d..17c63ec 100644
--- a/drivers/leds/leds-pca9532.c
+++ b/drivers/leds/leds-pca9532.c
@@ -158,7 +158,7 @@ static void pca9532_setled(struct pca9532_led *led)
mutex_unlock(&data->update_lock);
}

-static void pca9532_set_brightness(struct led_classdev *led_cdev,
+static int pca9532_set_brightness(struct led_classdev *led_cdev,
enum led_brightness value)
{
int err = 0;
@@ -172,9 +172,12 @@ static void pca9532_set_brightness(struct led_classdev *led_cdev,
led->state = PCA9532_PWM0; /* Thecus: hardcode one pwm */
err = pca9532_calcpwm(led->client, 0, 0, value);
if (err)
- return; /* XXX: led api doesn't allow error code? */
+ return err;
}
- schedule_work(&led->work);
+ if (led->state == PCA9532_PWM0)
+ pca9532_setpwm(led->client, 0);
+ pca9532_setled(led);
+ return err;
}

static int pca9532_set_blink(struct led_classdev *led_cdev,
@@ -198,7 +201,10 @@ static int pca9532_set_blink(struct led_classdev *led_cdev,
err = pca9532_calcpwm(client, 0, psc, led_cdev->brightness);
if (err)
return err;
- schedule_work(&led->work);
+ if (led->state == PCA9532_PWM0)
+ pca9532_setpwm(led->client, 0);
+ pca9532_setled(led);
+
return 0;
}

@@ -233,15 +239,6 @@ static void pca9532_input_work(struct work_struct *work)
mutex_unlock(&data->update_lock);
}

-static void pca9532_led_work(struct work_struct *work)
-{
- struct pca9532_led *led;
- led = container_of(work, struct pca9532_led, work);
- if (led->state == PCA9532_PWM0)
- pca9532_setpwm(led->client, 0);
- pca9532_setled(led);
-}
-
#ifdef CONFIG_LEDS_PCA9532_GPIO
static int pca9532_gpio_request_pin(struct gpio_chip *gc, unsigned offset)
{
@@ -307,7 +304,6 @@ static int pca9532_destroy_devices(struct pca9532_data *data, int n_devs)
break;
case PCA9532_TYPE_LED:
led_classdev_unregister(&data->leds[i].ldev);
- cancel_work_sync(&data->leds[i].work);
break;
case PCA9532_TYPE_N2100_BEEP:
if (data->idev != NULL) {
@@ -359,9 +355,9 @@ static int pca9532_configure(struct i2c_client *client,
led->name = pled->name;
led->ldev.name = led->name;
led->ldev.brightness = LED_OFF;
- led->ldev.brightness_set = pca9532_set_brightness;
+ led->ldev.brightness_set_blocking =
+ pca9532_set_brightness;
led->ldev.blink_set = pca9532_set_blink;
- INIT_WORK(&led->work, pca9532_led_work);
err = led_classdev_register(&client->dev, &led->ldev);
if (err < 0) {
dev_err(&client->dev,
--
1.7.9.5

2015-08-20 14:46:55

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 19/36] leds: lp3944: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Cc: Antonio Ospite <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
drivers/leds/leds-lp3944.c | 32 ++++++++------------------------
1 file changed, 8 insertions(+), 24 deletions(-)

diff --git a/drivers/leds/leds-lp3944.c b/drivers/leds/leds-lp3944.c
index 53144fb..6c758ae 100644
--- a/drivers/leds/leds-lp3944.c
+++ b/drivers/leds/leds-lp3944.c
@@ -31,7 +31,6 @@
#include <linux/slab.h>
#include <linux/leds.h>
#include <linux/mutex.h>
-#include <linux/workqueue.h>
#include <linux/leds-lp3944.h>

/* Read Only Registers */
@@ -68,10 +67,8 @@
struct lp3944_led_data {
u8 id;
enum lp3944_type type;
- enum lp3944_status status;
struct led_classdev ldev;
struct i2c_client *client;
- struct work_struct work;
};

struct lp3944_data {
@@ -275,13 +272,12 @@ static int lp3944_led_set_blink(struct led_classdev *led_cdev,
dev_dbg(&led->client->dev, "%s: OK hardware accelerated blink!\n",
__func__);

- led->status = LP3944_LED_STATUS_DIM0;
- schedule_work(&led->work);
+ lp3944_led_set(led, LP3944_LED_STATUS_DIM0);

return 0;
}

-static void lp3944_led_set_brightness(struct led_classdev *led_cdev,
+static int lp3944_led_set_brightness(struct led_classdev *led_cdev,
enum led_brightness brightness)
{
struct lp3944_led_data *led = ldev_to_led(led_cdev);
@@ -289,16 +285,7 @@ static void lp3944_led_set_brightness(struct led_classdev *led_cdev,
dev_dbg(&led->client->dev, "%s: %s, %d\n",
__func__, led_cdev->name, brightness);

- led->status = !!brightness;
- schedule_work(&led->work);
-}
-
-static void lp3944_led_work(struct work_struct *work)
-{
- struct lp3944_led_data *led;
-
- led = container_of(work, struct lp3944_led_data, work);
- lp3944_led_set(led, led->status);
+ return lp3944_led_set(led, !!brightness);
}

static int lp3944_configure(struct i2c_client *client,
@@ -318,14 +305,13 @@ static int lp3944_configure(struct i2c_client *client,
case LP3944_LED_TYPE_LED:
case LP3944_LED_TYPE_LED_INVERTED:
led->type = pled->type;
- led->status = pled->status;
led->ldev.name = pled->name;
led->ldev.max_brightness = 1;
- led->ldev.brightness_set = lp3944_led_set_brightness;
+ led->ldev.brightness_set_blocking =
+ lp3944_led_set_brightness;
led->ldev.blink_set = lp3944_led_set_blink;
led->ldev.flags = LED_CORE_SUSPENDRESUME;

- INIT_WORK(&led->work, lp3944_led_work);
err = led_classdev_register(&client->dev, &led->ldev);
if (err < 0) {
dev_err(&client->dev,
@@ -336,14 +322,14 @@ static int lp3944_configure(struct i2c_client *client,

/* to expose the default value to userspace */
led->ldev.brightness =
- (enum led_brightness) led->status;
+ (enum led_brightness) pled->status;

/* Set the default led status */
- err = lp3944_led_set(led, led->status);
+ err = lp3944_led_set(led, pled->status);
if (err < 0) {
dev_err(&client->dev,
"%s couldn't set STATUS %d\n",
- led->ldev.name, led->status);
+ led->ldev.name, pled->status);
goto exit;
}
break;
@@ -364,7 +350,6 @@ exit:
case LP3944_LED_TYPE_LED:
case LP3944_LED_TYPE_LED_INVERTED:
led_classdev_unregister(&data->leds[i].ldev);
- cancel_work_sync(&data->leds[i].work);
break;

case LP3944_LED_TYPE_NONE:
@@ -424,7 +409,6 @@ static int lp3944_remove(struct i2c_client *client)
case LP3944_LED_TYPE_LED:
case LP3944_LED_TYPE_LED_INVERTED:
led_classdev_unregister(&data->leds[i].ldev);
- cancel_work_sync(&data->leds[i].work);
break;

case LP3944_LED_TYPE_NONE:
--
1.7.9.5

2015-08-20 14:46:59

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 20/36] leds: lp55xx: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
Cc: Milo Kim <[email protected]>
---
drivers/leds/leds-lp5521.c | 11 ++++++-----
drivers/leds/leds-lp5523.c | 10 +++++-----
drivers/leds/leds-lp5562.c | 11 ++++++-----
drivers/leds/leds-lp55xx-common.c | 12 +++++-------
drivers/leds/leds-lp55xx-common.h | 6 ++----
drivers/leds/leds-lp8501.c | 11 ++++++-----
6 files changed, 30 insertions(+), 31 deletions(-)

diff --git a/drivers/leds/leds-lp5521.c b/drivers/leds/leds-lp5521.c
index 8ca197a..86c9882 100644
--- a/drivers/leds/leds-lp5521.c
+++ b/drivers/leds/leds-lp5521.c
@@ -362,16 +362,17 @@ static int lp5521_run_selftest(struct lp55xx_chip *chip, char *buf)
return 0;
}

-static void lp5521_led_brightness_work(struct work_struct *work)
+static int lp5521_led_brightness(struct lp55xx_led *led)
{
- struct lp55xx_led *led = container_of(work, struct lp55xx_led,
- brightness_work);
struct lp55xx_chip *chip = led->chip;
+ int ret;

mutex_lock(&chip->lock);
- lp55xx_write(chip, LP5521_REG_LED_PWM_BASE + led->chan_nr,
+ ret = lp55xx_write(chip, LP5521_REG_LED_PWM_BASE + led->chan_nr,
led->brightness);
mutex_unlock(&chip->lock);
+
+ return ret;
}

static ssize_t show_engine_mode(struct device *dev,
@@ -501,7 +502,7 @@ static struct lp55xx_device_config lp5521_cfg = {
},
.max_channel = LP5521_MAX_LEDS,
.post_init_device = lp5521_post_init_device,
- .brightness_work_fn = lp5521_led_brightness_work,
+ .brightness_fn = lp5521_led_brightness,
.set_led_current = lp5521_set_led_current,
.firmware_cb = lp5521_firmware_loaded,
.run_engine = lp5521_run_engine,
diff --git a/drivers/leds/leds-lp5523.c b/drivers/leds/leds-lp5523.c
index 584dbbc..e81694a 100644
--- a/drivers/leds/leds-lp5523.c
+++ b/drivers/leds/leds-lp5523.c
@@ -802,16 +802,16 @@ leave:
return ret;
}

-static void lp5523_led_brightness_work(struct work_struct *work)
+static int lp5523_led_brightness(struct lp55xx_led *led)
{
- struct lp55xx_led *led = container_of(work, struct lp55xx_led,
- brightness_work);
struct lp55xx_chip *chip = led->chip;
+ int ret;

mutex_lock(&chip->lock);
- lp55xx_write(chip, LP5523_REG_LED_PWM_BASE + led->chan_nr,
+ ret = lp55xx_write(chip, LP5523_REG_LED_PWM_BASE + led->chan_nr,
led->brightness);
mutex_unlock(&chip->lock);
+ return ret;
}

static LP55XX_DEV_ATTR_RW(engine1_mode, show_engine1_mode, store_engine1_mode);
@@ -867,7 +867,7 @@ static struct lp55xx_device_config lp5523_cfg = {
},
.max_channel = LP5523_MAX_LEDS,
.post_init_device = lp5523_post_init_device,
- .brightness_work_fn = lp5523_led_brightness_work,
+ .brightness_fn = lp5523_led_brightness,
.set_led_current = lp5523_set_led_current,
.firmware_cb = lp5523_firmware_loaded,
.run_engine = lp5523_run_engine,
diff --git a/drivers/leds/leds-lp5562.c b/drivers/leds/leds-lp5562.c
index ca85724..d7ec706 100644
--- a/drivers/leds/leds-lp5562.c
+++ b/drivers/leds/leds-lp5562.c
@@ -311,10 +311,8 @@ static int lp5562_post_init_device(struct lp55xx_chip *chip)
return 0;
}

-static void lp5562_led_brightness_work(struct work_struct *work)
+static int lp5562_led_brightness(struct lp55xx_led *led)
{
- struct lp55xx_led *led = container_of(work, struct lp55xx_led,
- brightness_work);
struct lp55xx_chip *chip = led->chip;
u8 addr[] = {
LP5562_REG_R_PWM,
@@ -322,10 +320,13 @@ static void lp5562_led_brightness_work(struct work_struct *work)
LP5562_REG_B_PWM,
LP5562_REG_W_PWM,
};
+ int ret;

mutex_lock(&chip->lock);
- lp55xx_write(chip, addr[led->chan_nr], led->brightness);
+ ret = lp55xx_write(chip, addr[led->chan_nr], led->brightness);
mutex_unlock(&chip->lock);
+
+ return ret;
}

static void lp5562_write_program_memory(struct lp55xx_chip *chip,
@@ -503,7 +504,7 @@ static struct lp55xx_device_config lp5562_cfg = {
},
.post_init_device = lp5562_post_init_device,
.set_led_current = lp5562_set_led_current,
- .brightness_work_fn = lp5562_led_brightness_work,
+ .brightness_fn = lp5562_led_brightness,
.run_engine = lp5562_run_engine,
.firmware_cb = lp5562_firmware_loaded,
.dev_attr_group = &lp5562_group,
diff --git a/drivers/leds/leds-lp55xx-common.c b/drivers/leds/leds-lp55xx-common.c
index 96d51e9..95fd28e 100644
--- a/drivers/leds/leds-lp55xx-common.c
+++ b/drivers/leds/leds-lp55xx-common.c
@@ -134,13 +134,14 @@ static struct attribute *lp55xx_led_attrs[] = {
};
ATTRIBUTE_GROUPS(lp55xx_led);

-static void lp55xx_set_brightness(struct led_classdev *cdev,
+static int lp55xx_set_brightness(struct led_classdev *cdev,
enum led_brightness brightness)
{
struct lp55xx_led *led = cdev_to_lp55xx_led(cdev);
+ struct lp55xx_device_config *cfg = led->chip->cfg;

led->brightness = (u8)brightness;
- schedule_work(&led->brightness_work);
+ return cfg->brightness_fn(led);
}

static int lp55xx_init_led(struct lp55xx_led *led,
@@ -172,7 +173,7 @@ static int lp55xx_init_led(struct lp55xx_led *led,
return -EINVAL;
}

- led->cdev.brightness_set = lp55xx_set_brightness;
+ led->cdev.brightness_set_blocking = lp55xx_set_brightness;
led->cdev.groups = lp55xx_led_groups;

if (pdata->led_config[chan].name) {
@@ -464,7 +465,7 @@ int lp55xx_register_leds(struct lp55xx_led *led, struct lp55xx_chip *chip)
int ret;
int i;

- if (!cfg->brightness_work_fn) {
+ if (!cfg->brightness_fn) {
dev_err(&chip->cl->dev, "empty brightness configuration\n");
return -EINVAL;
}
@@ -481,8 +482,6 @@ int lp55xx_register_leds(struct lp55xx_led *led, struct lp55xx_chip *chip)
if (ret)
goto err_init_led;

- INIT_WORK(&each->brightness_work, cfg->brightness_work_fn);
-
chip->num_leds++;
each->chip = chip;

@@ -507,7 +506,6 @@ void lp55xx_unregister_leds(struct lp55xx_led *led, struct lp55xx_chip *chip)
for (i = 0; i < chip->num_leds; i++) {
each = led + i;
led_classdev_unregister(&each->cdev);
- flush_work(&each->brightness_work);
}
}
EXPORT_SYMBOL_GPL(lp55xx_unregister_leds);
diff --git a/drivers/leds/leds-lp55xx-common.h b/drivers/leds/leds-lp55xx-common.h
index cceab48..22e7882 100644
--- a/drivers/leds/leds-lp55xx-common.h
+++ b/drivers/leds/leds-lp55xx-common.h
@@ -95,7 +95,7 @@ struct lp55xx_reg {
* @enable : Chip specific enable command
* @max_channel : Maximum number of channels
* @post_init_device : Chip specific initialization code
- * @brightness_work_fn : Brightness work function
+ * @brightness_fn : Brightness function
* @set_led_current : LED current set function
* @firmware_cb : Call function when the firmware is loaded
* @run_engine : Run internal engine for pattern
@@ -110,7 +110,7 @@ struct lp55xx_device_config {
int (*post_init_device) (struct lp55xx_chip *chip);

/* access brightness register */
- void (*brightness_work_fn)(struct work_struct *work);
+ int (*brightness_fn)(struct lp55xx_led *led);

/* current setting function */
void (*set_led_current) (struct lp55xx_led *led, u8 led_current);
@@ -164,7 +164,6 @@ struct lp55xx_chip {
* @cdev : LED class device
* @led_current : Current setting at each led channel
* @max_current : Maximun current at each led channel
- * @brightness_work : Workqueue for brightness control
* @brightness : Brightness value
* @chip : The lp55xx chip data
*/
@@ -173,7 +172,6 @@ struct lp55xx_led {
struct led_classdev cdev;
u8 led_current;
u8 max_current;
- struct work_struct brightness_work;
u8 brightness;
struct lp55xx_chip *chip;
};
diff --git a/drivers/leds/leds-lp8501.c b/drivers/leds/leds-lp8501.c
index d3098e3..b94210b 100644
--- a/drivers/leds/leds-lp8501.c
+++ b/drivers/leds/leds-lp8501.c
@@ -272,16 +272,17 @@ static void lp8501_firmware_loaded(struct lp55xx_chip *chip)
lp8501_update_program_memory(chip, fw->data, fw->size);
}

-static void lp8501_led_brightness_work(struct work_struct *work)
+static int lp8501_led_brightness(struct lp55xx_led *led)
{
- struct lp55xx_led *led = container_of(work, struct lp55xx_led,
- brightness_work);
struct lp55xx_chip *chip = led->chip;
+ int ret;

mutex_lock(&chip->lock);
- lp55xx_write(chip, LP8501_REG_LED_PWM_BASE + led->chan_nr,
+ ret = lp55xx_write(chip, LP8501_REG_LED_PWM_BASE + led->chan_nr,
led->brightness);
mutex_unlock(&chip->lock);
+
+ return ret;
}

/* Chip specific configurations */
@@ -296,7 +297,7 @@ static struct lp55xx_device_config lp8501_cfg = {
},
.max_channel = LP8501_MAX_LEDS,
.post_init_device = lp8501_post_init_device,
- .brightness_work_fn = lp8501_led_brightness_work,
+ .brightness_fn = lp8501_led_brightness,
.set_led_current = lp8501_set_led_current,
.firmware_cb = lp8501_firmware_loaded,
.run_engine = lp8501_run_engine,
--
1.7.9.5

2015-08-20 14:47:09

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 21/36] leds: lp8788: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Cc: Milo Kim <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
drivers/leds/leds-lp8788.c | 48 +++++++++++++++++++-------------------------
1 file changed, 21 insertions(+), 27 deletions(-)

diff --git a/drivers/leds/leds-lp8788.c b/drivers/leds/leds-lp8788.c
index 3409f03..0eee38f 100644
--- a/drivers/leds/leds-lp8788.c
+++ b/drivers/leds/leds-lp8788.c
@@ -26,10 +26,8 @@
struct lp8788_led {
struct lp8788 *lp;
struct mutex lock;
- struct work_struct work;
struct led_classdev led_dev;
enum lp8788_isink_number isink_num;
- enum led_brightness brightness;
int on;
};

@@ -76,24 +74,29 @@ static int lp8788_led_init_device(struct lp8788_led *led,
return lp8788_update_bits(led->lp, addr, mask, val);
}

-static void lp8788_led_enable(struct lp8788_led *led,
+static int lp8788_led_enable(struct lp8788_led *led,
enum lp8788_isink_number num, int on)
{
+ int ret;
+
u8 mask = 1 << num;
u8 val = on << num;

- if (lp8788_update_bits(led->lp, LP8788_ISINK_CTRL, mask, val))
- return;
+ ret = lp8788_update_bits(led->lp, LP8788_ISINK_CTRL, mask, val);
+ if (ret == 0)
+ led->on = on;

- led->on = on;
+ return ret;
}

-static void lp8788_led_work(struct work_struct *work)
+static int lp8788_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness val)
{
- struct lp8788_led *led = container_of(work, struct lp8788_led, work);
+ struct lp8788_led *led =
+ container_of(led_cdev, struct lp8788_led, led_dev);
+
enum lp8788_isink_number num = led->isink_num;
- int enable;
- u8 val = led->brightness;
+ int enable, ret;

mutex_lock(&led->lock);

@@ -101,28 +104,21 @@ static void lp8788_led_work(struct work_struct *work)
case LP8788_ISINK_1:
case LP8788_ISINK_2:
case LP8788_ISINK_3:
- lp8788_write_byte(led->lp, lp8788_pwm_addr[num], val);
+ ret = lp8788_write_byte(led->lp, lp8788_pwm_addr[num], val);
+ if (ret < 0)
+ goto unlock;
break;
default:
mutex_unlock(&led->lock);
- return;
+ return -EINVAL;
}

enable = (val > 0) ? 1 : 0;
if (enable != led->on)
- lp8788_led_enable(led, num, enable);
-
+ ret = lp8788_led_enable(led, num, enable);
+unlock:
mutex_unlock(&led->lock);
-}
-
-static void lp8788_brightness_set(struct led_classdev *led_cdev,
- enum led_brightness brt_val)
-{
- struct lp8788_led *led =
- container_of(led_cdev, struct lp8788_led, led_dev);
-
- led->brightness = brt_val;
- schedule_work(&led->work);
+ return ret;
}

static int lp8788_led_probe(struct platform_device *pdev)
@@ -139,7 +135,7 @@ static int lp8788_led_probe(struct platform_device *pdev)

led->lp = lp;
led->led_dev.max_brightness = MAX_BRIGHTNESS;
- led->led_dev.brightness_set = lp8788_brightness_set;
+ led->led_dev.brightness_set_blocking = lp8788_brightness_set;

led_pdata = lp->pdata ? lp->pdata->led_pdata : NULL;

@@ -149,7 +145,6 @@ static int lp8788_led_probe(struct platform_device *pdev)
led->led_dev.name = led_pdata->name;

mutex_init(&led->lock);
- INIT_WORK(&led->work, lp8788_led_work);

platform_set_drvdata(pdev, led);

@@ -173,7 +168,6 @@ static int lp8788_led_remove(struct platform_device *pdev)
struct lp8788_led *led = platform_get_drvdata(pdev);

led_classdev_unregister(&led->led_dev);
- flush_work(&led->work);

return 0;
}
--
1.7.9.5

2015-08-20 14:47:17

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 22/36] leds: lp8860: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Cc: Dan Murphy <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
drivers/leds/leds-lp8860.c | 27 +++++++--------------------
1 file changed, 7 insertions(+), 20 deletions(-)

diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
index 79f0843..3e70775 100644
--- a/drivers/leds/leds-lp8860.c
+++ b/drivers/leds/leds-lp8860.c
@@ -91,26 +91,22 @@
/**
* struct lp8860_led -
* @lock - Lock for reading/writing the device
- * @work - Work item used to off load the brightness register writes
* @client - Pointer to the I2C client
* @led_dev - led class device pointer
* @regmap - Devices register map
* @eeprom_regmap - EEPROM register map
* @enable_gpio - VDDIO/EN gpio to enable communication interface
* @regulator - LED supply regulator pointer
- * @brightness - Current brightness value requested
* @label - LED label
**/
struct lp8860_led {
struct mutex lock;
- struct work_struct work;
struct i2c_client *client;
struct led_classdev led_dev;
struct regmap *regmap;
struct regmap *eeprom_regmap;
struct gpio_desc *enable_gpio;
struct regulator *regulator;
- enum led_brightness brightness;
const char *label;
};

@@ -212,11 +208,13 @@ out:
return ret;
}

-static void lp8860_led_brightness_work(struct work_struct *work)
+static int lp8860_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness brt_val)
{
- struct lp8860_led *led = container_of(work, struct lp8860_led, work);
+ struct lp8860_led *led =
+ container_of(led_cdev, struct lp8860_led, led_dev);
+ int disp_brightness = brt_val * 255;
int ret;
- int disp_brightness = led->brightness * 255;

mutex_lock(&led->lock);

@@ -241,16 +239,7 @@ static void lp8860_led_brightness_work(struct work_struct *work)
}
out:
mutex_unlock(&led->lock);
-}
-
-static void lp8860_brightness_set(struct led_classdev *led_cdev,
- enum led_brightness brt_val)
-{
- struct lp8860_led *led =
- container_of(led_cdev, struct lp8860_led, led_dev);
-
- led->brightness = brt_val;
- schedule_work(&led->work);
+ return ret;
}

static int lp8860_init(struct lp8860_led *led)
@@ -406,10 +395,9 @@ static int lp8860_probe(struct i2c_client *client,
led->client = client;
led->led_dev.name = led->label;
led->led_dev.max_brightness = LED_FULL;
- led->led_dev.brightness_set = lp8860_brightness_set;
+ led->led_dev.brightness_set_blocking = lp8860_brightness_set;

mutex_init(&led->lock);
- INIT_WORK(&led->work, lp8860_led_brightness_work);

i2c_set_clientdata(client, led);

@@ -448,7 +436,6 @@ static int lp8860_remove(struct i2c_client *client)
int ret;

led_classdev_unregister(&led->led_dev);
- cancel_work_sync(&led->work);

if (led->enable_gpio)
gpiod_direction_output(led->enable_gpio, 0);
--
1.7.9.5

2015-08-20 14:47:19

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 23/36] leds: pca955x: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Cc: Nate Case <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
drivers/leds/leds-pca955x.c | 39 +++++++++------------------------------
1 file changed, 9 insertions(+), 30 deletions(-)

diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
index b775e1e..840401a 100644
--- a/drivers/leds/leds-pca955x.c
+++ b/drivers/leds/leds-pca955x.c
@@ -47,7 +47,6 @@
#include <linux/leds.h>
#include <linux/err.h>
#include <linux/i2c.h>
-#include <linux/workqueue.h>
#include <linux/slab.h>

/* LED select registers determine the source that drives LED outputs */
@@ -110,8 +109,6 @@ struct pca955x {

struct pca955x_led {
struct pca955x *pca955x;
- struct work_struct work;
- enum led_brightness brightness;
struct led_classdev led_cdev;
int led_num; /* 0 .. 15 potentially */
char name[32];
@@ -193,7 +190,8 @@ static u8 pca955x_read_ls(struct i2c_client *client, int n)
pca95xx_num_input_regs(pca955x->chipdef->bits) + 4 + n);
}

-static void pca955x_led_work(struct work_struct *work)
+static int pca955x_led_set(struct led_classdev *led_cdev,
+ enum led_brightness value)
{
struct pca955x_led *pca955x_led;
struct pca955x *pca955x;
@@ -201,7 +199,7 @@ static void pca955x_led_work(struct work_struct *work)
int chip_ls; /* which LSx to use (0-3 potentially) */
int ls_led; /* which set of bits within LSx to use (0-3) */

- pca955x_led = container_of(work, struct pca955x_led, work);
+ pca955x_led = container_of(led_cdev, struct pca955x_led, led_cdev);
pca955x = pca955x_led->pca955x;

chip_ls = pca955x_led->led_num / 4;
@@ -211,7 +209,7 @@ static void pca955x_led_work(struct work_struct *work)

ls = pca955x_read_ls(pca955x->client, chip_ls);

- switch (pca955x_led->brightness) {
+ switch (value) {
case LED_FULL:
ls = pca955x_ledsel(ls, ls_led, PCA955X_LS_LED_ON);
break;
@@ -230,7 +228,7 @@ static void pca955x_led_work(struct work_struct *work)
* just turning off for all other values.
*/
pca955x_write_pwm(pca955x->client, 1,
- 255 - pca955x_led->brightness);
+ 255 - value);
ls = pca955x_ledsel(ls, ls_led, PCA955X_LS_BLINK1);
break;
}
@@ -238,21 +236,8 @@ static void pca955x_led_work(struct work_struct *work)
pca955x_write_ls(pca955x->client, chip_ls, ls);

mutex_unlock(&pca955x->lock);
-}
-
-static void pca955x_led_set(struct led_classdev *led_cdev, enum led_brightness value)
-{
- struct pca955x_led *pca955x;
-
- pca955x = container_of(led_cdev, struct pca955x_led, led_cdev);
-
- pca955x->brightness = value;

- /*
- * Must use workqueue for the actual I/O since I2C operations
- * can sleep.
- */
- schedule_work(&pca955x->work);
+ return 0;
}

static int pca955x_probe(struct i2c_client *client,
@@ -328,9 +313,7 @@ static int pca955x_probe(struct i2c_client *client,
}

pca955x_led->led_cdev.name = pca955x_led->name;
- pca955x_led->led_cdev.brightness_set = pca955x_led_set;
-
- INIT_WORK(&pca955x_led->work, pca955x_led_work);
+ pca955x_led->led_cdev.brightness_set_blocking = pca955x_led_set;

err = led_classdev_register(&client->dev,
&pca955x_led->led_cdev);
@@ -355,10 +338,8 @@ static int pca955x_probe(struct i2c_client *client,
return 0;

exit:
- while (i--) {
+ while (i--)
led_classdev_unregister(&pca955x->leds[i].led_cdev);
- cancel_work_sync(&pca955x->leds[i].work);
- }

return err;
}
@@ -368,10 +349,8 @@ static int pca955x_remove(struct i2c_client *client)
struct pca955x *pca955x = i2c_get_clientdata(client);
int i;

- for (i = 0; i < pca955x->chipdef->bits; i++) {
+ for (i = 0; i < pca955x->chipdef->bits; i++)
led_classdev_unregister(&pca955x->leds[i].led_cdev);
- cancel_work_sync(&pca955x->leds[i].work);
- }

return 0;
}
--
1.7.9.5

2015-08-20 14:51:49

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 24/36] leds: pca963x: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
Cc: Peter Meerwald <[email protected]>
Cc: Ricardo Ribalda <[email protected]>
---
drivers/leds/leds-pca963x.c | 80 +++++++++++++------------------------------
1 file changed, 23 insertions(+), 57 deletions(-)

diff --git a/drivers/leds/leds-pca963x.c b/drivers/leds/leds-pca963x.c
index 3f63a1b..f31495d 100644
--- a/drivers/leds/leds-pca963x.c
+++ b/drivers/leds/leds-pca963x.c
@@ -32,7 +32,6 @@
#include <linux/leds.h>
#include <linux/err.h>
#include <linux/i2c.h>
-#include <linux/workqueue.h>
#include <linux/slab.h>
#include <linux/of.h>
#include <linux/platform_data/leds-pca963x.h>
@@ -96,11 +95,6 @@ static const struct i2c_device_id pca963x_id[] = {
};
MODULE_DEVICE_TABLE(i2c, pca963x_id);

-enum pca963x_cmd {
- BRIGHTNESS_SET,
- BLINK_SET,
-};
-
struct pca963x_led;

struct pca963x {
@@ -112,47 +106,52 @@ struct pca963x {

struct pca963x_led {
struct pca963x *chip;
- struct work_struct work;
- enum led_brightness brightness;
struct led_classdev led_cdev;
int led_num; /* 0 .. 15 potentially */
- enum pca963x_cmd cmd;
char name[32];
u8 gdc;
u8 gfrq;
};

-static void pca963x_brightness_work(struct pca963x_led *pca963x)
+static int pca963x_brightness(struct pca963x_led *pca963x,
+ enum led_brightness brightness)
{
u8 ledout_addr = pca963x->chip->chipdef->ledout_base
+ (pca963x->led_num / 4);
u8 ledout;
int shift = 2 * (pca963x->led_num % 4);
u8 mask = 0x3 << shift;
+ int ret;

mutex_lock(&pca963x->chip->mutex);
ledout = i2c_smbus_read_byte_data(pca963x->chip->client, ledout_addr);
- switch (pca963x->brightness) {
+ switch (brightness) {
case LED_FULL:
- i2c_smbus_write_byte_data(pca963x->chip->client, ledout_addr,
+ ret = i2c_smbus_write_byte_data(pca963x->chip->client,
+ ledout_addr,
(ledout & ~mask) | (PCA963X_LED_ON << shift));
break;
case LED_OFF:
- i2c_smbus_write_byte_data(pca963x->chip->client, ledout_addr,
- ledout & ~mask);
+ ret = i2c_smbus_write_byte_data(pca963x->chip->client,
+ ledout_addr, ledout & ~mask);
break;
default:
- i2c_smbus_write_byte_data(pca963x->chip->client,
+ ret = i2c_smbus_write_byte_data(pca963x->chip->client,
PCA963X_PWM_BASE + pca963x->led_num,
- pca963x->brightness);
- i2c_smbus_write_byte_data(pca963x->chip->client, ledout_addr,
+ brightness);
+ if (ret < 0)
+ goto unlock;
+ ret = i2c_smbus_write_byte_data(pca963x->chip->client,
+ ledout_addr,
(ledout & ~mask) | (PCA963X_LED_PWM << shift));
break;
}
+unlock:
mutex_unlock(&pca963x->chip->mutex);
+ return ret;
}

-static void pca963x_blink_work(struct pca963x_led *pca963x)
+static void pca963x_blink(struct pca963x_led *pca963x)
{
u8 ledout_addr = pca963x->chip->chipdef->ledout_base +
(pca963x->led_num / 4);
@@ -180,36 +179,14 @@ static void pca963x_blink_work(struct pca963x_led *pca963x)
mutex_unlock(&pca963x->chip->mutex);
}

-static void pca963x_work(struct work_struct *work)
-{
- struct pca963x_led *pca963x = container_of(work,
- struct pca963x_led, work);
-
- switch (pca963x->cmd) {
- case BRIGHTNESS_SET:
- pca963x_brightness_work(pca963x);
- break;
- case BLINK_SET:
- pca963x_blink_work(pca963x);
- break;
- }
-}
-
-static void pca963x_led_set(struct led_classdev *led_cdev,
+static int pca963x_led_set(struct led_classdev *led_cdev,
enum led_brightness value)
{
struct pca963x_led *pca963x;

pca963x = container_of(led_cdev, struct pca963x_led, led_cdev);

- pca963x->cmd = BRIGHTNESS_SET;
- pca963x->brightness = value;
-
- /*
- * Must use workqueue for the actual I/O since I2C operations
- * can sleep.
- */
- schedule_work(&pca963x->work);
+ return pca963x_brightness(pca963x, value);
}

static int pca963x_blink_set(struct led_classdev *led_cdev,
@@ -254,15 +231,10 @@ static int pca963x_blink_set(struct led_classdev *led_cdev,
*/
gfrq = (period * 24 / 1000) - 1;

- pca963x->cmd = BLINK_SET;
pca963x->gdc = gdc;
pca963x->gfrq = gfrq;

- /*
- * Must use workqueue for the actual I/O since I2C operations
- * can sleep.
- */
- schedule_work(&pca963x->work);
+ pca963x_blink(pca963x);

*delay_on = time_on;
*delay_off = time_off;
@@ -408,13 +380,11 @@ static int pca963x_probe(struct i2c_client *client,
client->addr, i);

pca963x[i].led_cdev.name = pca963x[i].name;
- pca963x[i].led_cdev.brightness_set = pca963x_led_set;
+ pca963x[i].led_cdev.brightness_set_blocking = pca963x_led_set;

if (pdata && pdata->blink_type == PCA963X_HW_BLINK)
pca963x[i].led_cdev.blink_set = pca963x_blink_set;

- INIT_WORK(&pca963x[i].work, pca963x_work);
-
err = led_classdev_register(&client->dev, &pca963x[i].led_cdev);
if (err < 0)
goto exit;
@@ -434,10 +404,8 @@ static int pca963x_probe(struct i2c_client *client,
return 0;

exit:
- while (i--) {
+ while (i--)
led_classdev_unregister(&pca963x[i].led_cdev);
- cancel_work_sync(&pca963x[i].work);
- }

return err;
}
@@ -447,10 +415,8 @@ static int pca963x_remove(struct i2c_client *client)
struct pca963x *pca963x = i2c_get_clientdata(client);
int i;

- for (i = 0; i < pca963x->chipdef->n_leds; i++) {
+ for (i = 0; i < pca963x->chipdef->n_leds; i++)
led_classdev_unregister(&pca963x->leds[i].led_cdev);
- cancel_work_sync(&pca963x->leds[i].work);
- }

return 0;
}
--
1.7.9.5

2015-08-20 14:51:22

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 25/36] leds: wm831x: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Cc: Mark Brown <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
drivers/leds/leds-wm831x-status.c | 25 +++++++++----------------
1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/leds/leds-wm831x-status.c b/drivers/leds/leds-wm831x-status.c
index 56027ef..64a2226 100644
--- a/drivers/leds/leds-wm831x-status.c
+++ b/drivers/leds/leds-wm831x-status.c
@@ -23,7 +23,6 @@
struct wm831x_status {
struct led_classdev cdev;
struct wm831x *wm831x;
- struct work_struct work;
struct mutex mutex;

spinlock_t value_lock;
@@ -40,10 +39,8 @@ struct wm831x_status {
#define to_wm831x_status(led_cdev) \
container_of(led_cdev, struct wm831x_status, cdev)

-static void wm831x_status_work(struct work_struct *work)
+static void wm831x_status_set(struct wm831x_status *led)
{
- struct wm831x_status *led = container_of(work, struct wm831x_status,
- work);
unsigned long flags;

mutex_lock(&led->mutex);
@@ -70,8 +67,8 @@ static void wm831x_status_work(struct work_struct *work)
mutex_unlock(&led->mutex);
}

-static void wm831x_status_set(struct led_classdev *led_cdev,
- enum led_brightness value)
+static int wm831x_status_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness value)
{
struct wm831x_status *led = to_wm831x_status(led_cdev);
unsigned long flags;
@@ -80,8 +77,10 @@ static void wm831x_status_set(struct led_classdev *led_cdev,
led->brightness = value;
if (value == LED_OFF)
led->blink = 0;
- schedule_work(&led->work);
spin_unlock_irqrestore(&led->value_lock, flags);
+ wm831x_status_set(led);
+
+ return 0;
}

static int wm831x_status_blink_set(struct led_classdev *led_cdev,
@@ -147,11 +146,8 @@ static int wm831x_status_blink_set(struct led_classdev *led_cdev,
else
led->blink = 0;

- /* Always update; if we fail turn off blinking since we expect
- * a software fallback. */
- schedule_work(&led->work);
-
spin_unlock_irqrestore(&led->value_lock, flags);
+ wm831x_status_set(led);

return ret;
}
@@ -206,11 +202,9 @@ static ssize_t wm831x_status_src_store(struct device *dev,
for (i = 0; i < ARRAY_SIZE(led_src_texts); i++) {
if (!strcmp(name, led_src_texts[i])) {
mutex_lock(&led->mutex);
-
led->src = i;
- schedule_work(&led->work);
-
mutex_unlock(&led->mutex);
+ wm831x_status_set(led);
}
}

@@ -262,7 +256,6 @@ static int wm831x_status_probe(struct platform_device *pdev)
pdata.name = dev_name(&pdev->dev);

mutex_init(&drvdata->mutex);
- INIT_WORK(&drvdata->work, wm831x_status_work);
spin_lock_init(&drvdata->value_lock);

/* We cache the configuration register and read startup values
@@ -287,7 +280,7 @@ static int wm831x_status_probe(struct platform_device *pdev)

drvdata->cdev.name = pdata.name;
drvdata->cdev.default_trigger = pdata.default_trigger;
- drvdata->cdev.brightness_set = wm831x_status_set;
+ drvdata->cdev.brightness_set_blocking = wm831x_status_brightness_set;
drvdata->cdev.blink_set = wm831x_status_blink_set;
drvdata->cdev.groups = wm831x_status_groups;

--
1.7.9.5

2015-08-20 14:50:52

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 26/36] leds: da903x: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
drivers/leds/leds-da903x.c | 46 +++++++++++++++++---------------------------
1 file changed, 18 insertions(+), 28 deletions(-)

diff --git a/drivers/leds/leds-da903x.c b/drivers/leds/leds-da903x.c
index 952ba96e..4752a2b 100644
--- a/drivers/leds/leds-da903x.c
+++ b/drivers/leds/leds-da903x.c
@@ -16,7 +16,6 @@
#include <linux/kernel.h>
#include <linux/platform_device.h>
#include <linux/leds.h>
-#include <linux/workqueue.h>
#include <linux/mfd/da903x.h>
#include <linux/slab.h>

@@ -33,9 +32,7 @@

struct da903x_led {
struct led_classdev cdev;
- struct work_struct work;
struct device *master;
- enum led_brightness new_brightness;
int id;
int flags;
};
@@ -43,11 +40,13 @@ struct da903x_led {
#define DA9030_LED_OFFSET(id) ((id) - DA9030_ID_LED_1)
#define DA9034_LED_OFFSET(id) ((id) - DA9034_ID_LED_1)

-static void da903x_led_work(struct work_struct *work)
+static int da903x_led_set(struct led_classdev *led_cdev,
+ enum led_brightness value)
{
- struct da903x_led *led = container_of(work, struct da903x_led, work);
+ struct da903x_led *led =
+ container_of(led_cdev, struct da903x_led, cdev);
uint8_t val;
- int offset;
+ int offset, ret = -EINVAL;

switch (led->id) {
case DA9030_ID_LED_1:
@@ -57,37 +56,31 @@ static void da903x_led_work(struct work_struct *work)
case DA9030_ID_LED_PC:
offset = DA9030_LED_OFFSET(led->id);
val = led->flags & ~0x87;
- val |= (led->new_brightness) ? 0x80 : 0; /* EN bit */
- val |= (0x7 - (led->new_brightness >> 5)) & 0x7; /* PWM<2:0> */
- da903x_write(led->master, DA9030_LED1_CONTROL + offset, val);
+ val |= value ? 0x80 : 0; /* EN bit */
+ val |= (0x7 - (value >> 5)) & 0x7; /* PWM<2:0> */
+ ret = da903x_write(led->master, DA9030_LED1_CONTROL + offset,
+ val);
break;
case DA9030_ID_VIBRA:
val = led->flags & ~0x80;
- val |= (led->new_brightness) ? 0x80 : 0; /* EN bit */
- da903x_write(led->master, DA9030_MISC_CONTROL_A, val);
+ val |= value ? 0x80 : 0; /* EN bit */
+ ret = da903x_write(led->master, DA9030_MISC_CONTROL_A, val);
break;
case DA9034_ID_LED_1:
case DA9034_ID_LED_2:
offset = DA9034_LED_OFFSET(led->id);
- val = (led->new_brightness * 0x5f / LED_FULL) & 0x7f;
+ val = (value * 0x5f / LED_FULL) & 0x7f;
val |= (led->flags & DA9034_LED_RAMP) ? 0x80 : 0;
- da903x_write(led->master, DA9034_LED1_CONTROL + offset, val);
+ ret = da903x_write(led->master, DA9034_LED1_CONTROL + offset,
+ val);
break;
case DA9034_ID_VIBRA:
- val = led->new_brightness & 0xfe;
- da903x_write(led->master, DA9034_VIBRA, val);
+ val = value & 0xfe;
+ ret = da903x_write(led->master, DA9034_VIBRA, val);
break;
}
-}

-static void da903x_led_set(struct led_classdev *led_cdev,
- enum led_brightness value)
-{
- struct da903x_led *led;
-
- led = container_of(led_cdev, struct da903x_led, cdev);
- led->new_brightness = value;
- schedule_work(&led->work);
+ return ret;
}

static int da903x_led_probe(struct platform_device *pdev)
@@ -113,15 +106,12 @@ static int da903x_led_probe(struct platform_device *pdev)

led->cdev.name = pdata->name;
led->cdev.default_trigger = pdata->default_trigger;
- led->cdev.brightness_set = da903x_led_set;
+ led->cdev.brightness_set_blocking = da903x_led_set;
led->cdev.brightness = LED_OFF;

led->id = id;
led->flags = pdata->flags;
led->master = pdev->dev.parent;
- led->new_brightness = LED_OFF;
-
- INIT_WORK(&led->work, da903x_led_work);

ret = led_classdev_register(led->master, &led->cdev);
if (ret) {
--
1.7.9.5

2015-08-20 14:47:26

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 27/36] leds: da9052: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
drivers/leds/leds-da9052.c | 39 ++++++++++++---------------------------
1 file changed, 12 insertions(+), 27 deletions(-)

diff --git a/drivers/leds/leds-da9052.c b/drivers/leds/leds-da9052.c
index 28291b6..f8c7d82 100644
--- a/drivers/leds/leds-da9052.c
+++ b/drivers/leds/leds-da9052.c
@@ -16,7 +16,6 @@
#include <linux/kernel.h>
#include <linux/platform_device.h>
#include <linux/leds.h>
-#include <linux/workqueue.h>
#include <linux/slab.h>

#include <linux/mfd/da9052/reg.h>
@@ -32,11 +31,9 @@

struct da9052_led {
struct led_classdev cdev;
- struct work_struct work;
struct da9052 *da9052;
unsigned char led_index;
unsigned char id;
- int brightness;
};

static unsigned char led_reg[] = {
@@ -44,12 +41,13 @@ static unsigned char led_reg[] = {
DA9052_LED_CONT_5_REG,
};

-static int da9052_set_led_brightness(struct da9052_led *led)
+static int da9052_set_led_brightness(struct da9052_led *led,
+ enum led_brightness brightness)
{
u8 val;
int error;

- val = (led->brightness & 0x7f) | DA9052_LED_CONT_DIM;
+ val = (brightness & 0x7f) | DA9052_LED_CONT_DIM;

error = da9052_reg_write(led->da9052, led_reg[led->led_index], val);
if (error < 0)
@@ -58,21 +56,13 @@ static int da9052_set_led_brightness(struct da9052_led *led)
return error;
}

-static void da9052_led_work(struct work_struct *work)
-{
- struct da9052_led *led = container_of(work, struct da9052_led, work);
-
- da9052_set_led_brightness(led);
-}
-
-static void da9052_led_set(struct led_classdev *led_cdev,
+static int da9052_led_set(struct led_classdev *led_cdev,
enum led_brightness value)
{
- struct da9052_led *led;
+ struct da9052_led *led =
+ container_of(led_cdev, struct da9052_led, cdev);

- led = container_of(led_cdev, struct da9052_led, cdev);
- led->brightness = value;
- schedule_work(&led->work);
+ return da9052_set_led_brightness(led, value);
}

static int da9052_configure_leds(struct da9052 *da9052)
@@ -133,13 +123,11 @@ static int da9052_led_probe(struct platform_device *pdev)

for (i = 0; i < pled->num_leds; i++) {
led[i].cdev.name = pled->leds[i].name;
- led[i].cdev.brightness_set = da9052_led_set;
+ led[i].cdev.brightness_set_blocking = da9052_led_set;
led[i].cdev.brightness = LED_OFF;
led[i].cdev.max_brightness = DA9052_MAX_BRIGHTNESS;
- led[i].brightness = LED_OFF;
led[i].led_index = pled->leds[i].flags;
led[i].da9052 = dev_get_drvdata(pdev->dev.parent);
- INIT_WORK(&led[i].work, da9052_led_work);

error = led_classdev_register(pdev->dev.parent, &led[i].cdev);
if (error) {
@@ -148,7 +136,8 @@ static int da9052_led_probe(struct platform_device *pdev)
goto err_register;
}

- error = da9052_set_led_brightness(&led[i]);
+ error = da9052_set_led_brightness(&led[i],
+ led[i].cdev.brightness);
if (error) {
dev_err(&pdev->dev, "Unable to init led %d\n",
led[i].led_index);
@@ -166,10 +155,8 @@ static int da9052_led_probe(struct platform_device *pdev)
return 0;

err_register:
- for (i = i - 1; i >= 0; i--) {
+ for (i = i - 1; i >= 0; i--)
led_classdev_unregister(&led[i].cdev);
- cancel_work_sync(&led[i].work);
- }
err:
return error;
}
@@ -187,10 +174,8 @@ static int da9052_led_remove(struct platform_device *pdev)
pled = pdata->pled;

for (i = 0; i < pled->num_leds; i++) {
- led[i].brightness = 0;
- da9052_set_led_brightness(&led[i]);
+ da9052_set_led_brightness(&led[i], LED_OFF);
led_classdev_unregister(&led[i].cdev);
- cancel_work_sync(&led[i].work);
}

return 0;
--
1.7.9.5

2015-08-20 14:47:32

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 28/36] leds: dac124d085: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Cc: Guennadi Liakhovetski <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
drivers/leds/leds-dac124s085.c | 38 ++++++++++----------------------------
1 file changed, 10 insertions(+), 28 deletions(-)

diff --git a/drivers/leds/leds-dac124s085.c b/drivers/leds/leds-dac124s085.c
index db3ba8b..1e6a3ee 100644
--- a/drivers/leds/leds-dac124s085.c
+++ b/drivers/leds/leds-dac124s085.c
@@ -13,20 +13,15 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/slab.h>
-#include <linux/spinlock.h>
-#include <linux/workqueue.h>
#include <linux/spi/spi.h>

struct dac124s085_led {
struct led_classdev ldev;
struct spi_device *spi;
int id;
- int brightness;
char name[sizeof("dac124s085-3")];

struct mutex mutex;
- struct work_struct work;
- spinlock_t lock;
};

struct dac124s085 {
@@ -38,29 +33,21 @@ struct dac124s085 {
#define ALL_WRITE_UPDATE (2 << 12)
#define POWER_DOWN_OUTPUT (3 << 12)

-static void dac124s085_led_work(struct work_struct *work)
+static int dac124s085_set_brightness(struct led_classdev *ldev,
+ enum led_brightness brightness)
{
- struct dac124s085_led *led = container_of(work, struct dac124s085_led,
- work);
+ struct dac124s085_led *led = container_of(ldev, struct dac124s085_led,
+ ldev);
u16 word;
+ int ret;

mutex_lock(&led->mutex);
word = cpu_to_le16(((led->id) << 14) | REG_WRITE_UPDATE |
- (led->brightness & 0xfff));
- spi_write(led->spi, (const u8 *)&word, sizeof(word));
+ (brightness & 0xfff));
+ ret = spi_write(led->spi, (const u8 *)&word, sizeof(word));
mutex_unlock(&led->mutex);
-}
-
-static void dac124s085_set_brightness(struct led_classdev *ldev,
- enum led_brightness brightness)
-{
- struct dac124s085_led *led = container_of(ldev, struct dac124s085_led,
- ldev);

- spin_lock(&led->lock);
- led->brightness = brightness;
- schedule_work(&led->work);
- spin_unlock(&led->lock);
+ return ret;
}

static int dac124s085_probe(struct spi_device *spi)
@@ -78,16 +65,13 @@ static int dac124s085_probe(struct spi_device *spi)
for (i = 0; i < ARRAY_SIZE(dac->leds); i++) {
led = dac->leds + i;
led->id = i;
- led->brightness = LED_OFF;
led->spi = spi;
snprintf(led->name, sizeof(led->name), "dac124s085-%d", i);
- spin_lock_init(&led->lock);
- INIT_WORK(&led->work, dac124s085_led_work);
mutex_init(&led->mutex);
led->ldev.name = led->name;
led->ldev.brightness = LED_OFF;
led->ldev.max_brightness = 0xfff;
- led->ldev.brightness_set = dac124s085_set_brightness;
+ led->ldev.brightness_set_blocking = dac124s085_set_brightness;
ret = led_classdev_register(&spi->dev, &led->ldev);
if (ret < 0)
goto eledcr;
@@ -109,10 +93,8 @@ static int dac124s085_remove(struct spi_device *spi)
struct dac124s085 *dac = spi_get_drvdata(spi);
int i;

- for (i = 0; i < ARRAY_SIZE(dac->leds); i++) {
+ for (i = 0; i < ARRAY_SIZE(dac->leds); i++)
led_classdev_unregister(&dac->leds[i].ldev);
- cancel_work_sync(&dac->leds[i].work);
- }

return 0;
}
--
1.7.9.5

2015-08-20 14:47:37

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 29/36] leds: lt3593: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Cc: Daniel Mack <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
drivers/leds/leds-lt3593.c | 33 ++++++++++-----------------------
1 file changed, 10 insertions(+), 23 deletions(-)

diff --git a/drivers/leds/leds-lt3593.c b/drivers/leds/leds-lt3593.c
index 9f41124..a7ff510 100644
--- a/drivers/leds/leds-lt3593.c
+++ b/drivers/leds/leds-lt3593.c
@@ -19,7 +19,6 @@
#include <linux/kernel.h>
#include <linux/platform_device.h>
#include <linux/leds.h>
-#include <linux/workqueue.h>
#include <linux/delay.h>
#include <linux/gpio.h>
#include <linux/slab.h>
@@ -28,15 +27,14 @@
struct lt3593_led_data {
struct led_classdev cdev;
unsigned gpio;
- struct work_struct work;
- u8 new_level;
};

-static void lt3593_led_work(struct work_struct *work)
+static int lt3593_led_set(struct led_classdev *led_cdev,
+ enum led_brightness value)
{
- int pulses;
struct lt3593_led_data *led_dat =
- container_of(work, struct lt3593_led_data, work);
+ container_of(led_cdev, struct lt3593_led_data, cdev);
+ int pulses;

/*
* The LT3593 resets its internal current level register to the maximum
@@ -47,18 +45,18 @@ static void lt3593_led_work(struct work_struct *work)
* applied is to the output driver.
*/

- if (led_dat->new_level == 0) {
+ if (value == 0) {
gpio_set_value_cansleep(led_dat->gpio, 0);
- return;
+ return 0;
}

- pulses = 32 - (led_dat->new_level * 32) / 255;
+ pulses = 32 - (value * 32) / 255;

if (pulses == 0) {
gpio_set_value_cansleep(led_dat->gpio, 0);
mdelay(1);
gpio_set_value_cansleep(led_dat->gpio, 1);
- return;
+ return 0;
}

gpio_set_value_cansleep(led_dat->gpio, 1);
@@ -69,16 +67,8 @@ static void lt3593_led_work(struct work_struct *work)
gpio_set_value_cansleep(led_dat->gpio, 1);
udelay(1);
}
-}

-static void lt3593_led_set(struct led_classdev *led_cdev,
- enum led_brightness value)
-{
- struct lt3593_led_data *led_dat =
- container_of(led_cdev, struct lt3593_led_data, cdev);
-
- led_dat->new_level = value;
- schedule_work(&led_dat->work);
+ return 0;
}

static int create_lt3593_led(const struct gpio_led *template,
@@ -97,7 +87,7 @@ static int create_lt3593_led(const struct gpio_led *template,
led_dat->cdev.default_trigger = template->default_trigger;
led_dat->gpio = template->gpio;

- led_dat->cdev.brightness_set = lt3593_led_set;
+ led_dat->cdev.brightness_set_blocking = lt3593_led_set;

state = (template->default_state == LEDS_GPIO_DEFSTATE_ON);
led_dat->cdev.brightness = state ? LED_FULL : LED_OFF;
@@ -111,8 +101,6 @@ static int create_lt3593_led(const struct gpio_led *template,
if (ret < 0)
return ret;

- INIT_WORK(&led_dat->work, lt3593_led_work);
-
ret = led_classdev_register(parent, &led_dat->cdev);
if (ret < 0)
return ret;
@@ -129,7 +117,6 @@ static void delete_lt3593_led(struct lt3593_led_data *led)
return;

led_classdev_unregister(&led->cdev);
- cancel_work_sync(&led->work);
}

static int lt3593_led_probe(struct platform_device *pdev)
--
1.7.9.5

2015-08-20 14:48:19

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 30/36] leds: max8997: Remove unneeded workqueue include

From: Andrew Lunn <[email protected]>

Work queues are not used in this driver, so remove the include.

Signed-off-by: Andrew Lunn <[email protected]>
---
drivers/leds/leds-max8997.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/leds/leds-max8997.c b/drivers/leds/leds-max8997.c
index c592aa5..01b45906 100644
--- a/drivers/leds/leds-max8997.c
+++ b/drivers/leds/leds-max8997.c
@@ -13,7 +13,6 @@
#include <linux/module.h>
#include <linux/err.h>
#include <linux/slab.h>
-#include <linux/workqueue.h>
#include <linux/leds.h>
#include <linux/mfd/max8997.h>
#include <linux/mfd/max8997-private.h>
--
1.7.9.5

2015-08-20 14:47:45

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 31/36] leds: mc13783: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
drivers/leds/leds-mc13783.c | 35 +++++++++--------------------------
1 file changed, 9 insertions(+), 26 deletions(-)

diff --git a/drivers/leds/leds-mc13783.c b/drivers/leds/leds-mc13783.c
index e2b847f..a2e4c17 100644
--- a/drivers/leds/leds-mc13783.c
+++ b/drivers/leds/leds-mc13783.c
@@ -20,7 +20,6 @@
#include <linux/platform_device.h>
#include <linux/leds.h>
#include <linux/of.h>
-#include <linux/workqueue.h>
#include <linux/mfd/mc13xxx.h>

struct mc13xxx_led_devtype {
@@ -32,8 +31,6 @@ struct mc13xxx_led_devtype {

struct mc13xxx_led {
struct led_classdev cdev;
- struct work_struct work;
- enum led_brightness new_brightness;
int id;
struct mc13xxx_leds *leds;
};
@@ -55,9 +52,11 @@ static unsigned int mc13xxx_max_brightness(int id)
return 0x3f;
}

-static void mc13xxx_led_work(struct work_struct *work)
+static int mc13xxx_led_set(struct led_classdev *led_cdev,
+ enum led_brightness value)
{
- struct mc13xxx_led *led = container_of(work, struct mc13xxx_led, work);
+ struct mc13xxx_led *led =
+ container_of(led_cdev, struct mc13xxx_led, cdev);
struct mc13xxx_leds *leds = led->leds;
unsigned int reg, bank, off, shift;

@@ -105,19 +104,9 @@ static void mc13xxx_led_work(struct work_struct *work)
BUG();
}

- mc13xxx_reg_rmw(leds->master, leds->devtype->ledctrl_base + reg,
+ return mc13xxx_reg_rmw(leds->master, leds->devtype->ledctrl_base + reg,
mc13xxx_max_brightness(led->id) << shift,
- led->new_brightness << shift);
-}
-
-static void mc13xxx_led_set(struct led_classdev *led_cdev,
- enum led_brightness value)
-{
- struct mc13xxx_led *led =
- container_of(led_cdev, struct mc13xxx_led, cdev);
-
- led->new_brightness = value;
- schedule_work(&led->work);
+ value << shift);
}

#ifdef CONFIG_OF
@@ -257,11 +246,9 @@ static int __init mc13xxx_led_probe(struct platform_device *pdev)
leds->led[i].cdev.name = name;
leds->led[i].cdev.default_trigger = trig;
leds->led[i].cdev.flags = LED_CORE_SUSPENDRESUME;
- leds->led[i].cdev.brightness_set = mc13xxx_led_set;
+ leds->led[i].cdev.brightness_set_blocking = mc13xxx_led_set;
leds->led[i].cdev.max_brightness = mc13xxx_max_brightness(id);

- INIT_WORK(&leds->led[i].work, mc13xxx_led_work);
-
ret = led_classdev_register(dev->parent, &leds->led[i].cdev);
if (ret) {
dev_err(dev, "Failed to register LED %i\n", id);
@@ -270,10 +257,8 @@ static int __init mc13xxx_led_probe(struct platform_device *pdev)
}

if (ret)
- while (--i >= 0) {
+ while (--i >= 0)
led_classdev_unregister(&leds->led[i].cdev);
- cancel_work_sync(&leds->led[i].work);
- }

return ret;
}
@@ -283,10 +268,8 @@ static int mc13xxx_led_remove(struct platform_device *pdev)
struct mc13xxx_leds *leds = platform_get_drvdata(pdev);
int i;

- for (i = 0; i < leds->num_leds; i++) {
+ for (i = 0; i < leds->num_leds; i++)
led_classdev_unregister(&leds->led[i].cdev);
- cancel_work_sync(&leds->led[i].work);
- }

return 0;
}
--
1.7.9.5

2015-08-20 14:47:49

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 32/36] leds: regulator: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Cc: Antonio Ospite <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
---
drivers/leds/leds-regulator.c | 46 +++++++++++------------------------------
1 file changed, 12 insertions(+), 34 deletions(-)

diff --git a/drivers/leds/leds-regulator.c b/drivers/leds/leds-regulator.c
index ffc2139..acf77ca 100644
--- a/drivers/leds/leds-regulator.c
+++ b/drivers/leds/leds-regulator.c
@@ -14,7 +14,6 @@
#include <linux/module.h>
#include <linux/err.h>
#include <linux/slab.h>
-#include <linux/workqueue.h>
#include <linux/leds.h>
#include <linux/leds-regulator.h>
#include <linux/platform_device.h>
@@ -25,10 +24,8 @@

struct regulator_led {
struct led_classdev cdev;
- enum led_brightness value;
int enabled;
struct mutex mutex;
- struct work_struct work;

struct regulator *vcc;
};
@@ -94,22 +91,24 @@ static void regulator_led_disable(struct regulator_led *led)
led->enabled = 0;
}

-static void regulator_led_set_value(struct regulator_led *led)
+static int regulator_led_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness value)
{
+ struct regulator_led *led = to_regulator_led(led_cdev);
int voltage;
- int ret;
+ int ret = 0;

mutex_lock(&led->mutex);

- if (led->value == LED_OFF) {
+ if (value == LED_OFF) {
regulator_led_disable(led);
goto out;
}

if (led->cdev.max_brightness > 1) {
- voltage = led_regulator_get_voltage(led->vcc, led->value);
+ voltage = led_regulator_get_voltage(led->vcc, value);
dev_dbg(led->cdev.dev, "brightness: %d voltage: %d\n",
- led->value, voltage);
+ value, voltage);

ret = regulator_set_voltage(led->vcc, voltage, voltage);
if (ret != 0)
@@ -121,23 +120,7 @@ static void regulator_led_set_value(struct regulator_led *led)

out:
mutex_unlock(&led->mutex);
-}
-
-static void led_work(struct work_struct *work)
-{
- struct regulator_led *led;
-
- led = container_of(work, struct regulator_led, work);
- regulator_led_set_value(led);
-}
-
-static void regulator_led_brightness_set(struct led_classdev *led_cdev,
- enum led_brightness value)
-{
- struct regulator_led *led = to_regulator_led(led_cdev);
-
- led->value = value;
- schedule_work(&led->work);
+ return ret;
}

static int regulator_led_probe(struct platform_device *pdev)
@@ -169,9 +152,8 @@ static int regulator_led_probe(struct platform_device *pdev)
pdata->brightness);
return -EINVAL;
}
- led->value = pdata->brightness;

- led->cdev.brightness_set = regulator_led_brightness_set;
+ led->cdev.brightness_set_blocking = regulator_led_brightness_set;
led->cdev.name = pdata->name;
led->cdev.flags |= LED_CORE_SUSPENDRESUME;
led->vcc = vcc;
@@ -181,21 +163,18 @@ static int regulator_led_probe(struct platform_device *pdev)
led->enabled = 1;

mutex_init(&led->mutex);
- INIT_WORK(&led->work, led_work);

platform_set_drvdata(pdev, led);

ret = led_classdev_register(&pdev->dev, &led->cdev);
- if (ret < 0) {
- cancel_work_sync(&led->work);
+ if (ret < 0)
return ret;
- }

/* to expose the default value to userspace */
- led->cdev.brightness = led->value;
+ led->cdev.brightness = pdata->brightness;

/* Set the default led status */
- regulator_led_set_value(led);
+ regulator_led_brightness_set(&led->cdev, led->cdev.brightness);

return 0;
}
@@ -205,7 +184,6 @@ static int regulator_led_remove(struct platform_device *pdev)
struct regulator_led *led = platform_get_drvdata(pdev);

led_classdev_unregister(&led->cdev);
- cancel_work_sync(&led->work);
regulator_led_disable(led);
return 0;
}
--
1.7.9.5

2015-08-20 14:47:56

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 33/36] leds: wm8350: Remove work queue

From: Andrew Lunn <[email protected]>

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

Signed-off-by: Andrew Lunn <[email protected]>
Signed-off-by: Jacek Anaszewski <[email protected]>
Cc: Mark Brown <[email protected]>
---
drivers/leds/leds-wm8350.c | 64 +++++++++++++++------------------------
include/linux/mfd/wm8350/pmic.h | 1 -
2 files changed, 25 insertions(+), 40 deletions(-)

diff --git a/drivers/leds/leds-wm8350.c b/drivers/leds/leds-wm8350.c
index 0d12183..e1e4e9d 100644
--- a/drivers/leds/leds-wm8350.c
+++ b/drivers/leds/leds-wm8350.c
@@ -89,40 +89,42 @@ static const int isink_cur[] = {
#define to_wm8350_led(led_cdev) \
container_of(led_cdev, struct wm8350_led, cdev)

-static void wm8350_led_enable(struct wm8350_led *led)
+static int wm8350_led_enable(struct wm8350_led *led)
{
- int ret;
+ int ret = 0;

if (led->enabled)
- return;
+ return ret;

ret = regulator_enable(led->isink);
if (ret != 0) {
dev_err(led->cdev.dev, "Failed to enable ISINK: %d\n", ret);
- return;
+ return ret;
}

ret = regulator_enable(led->dcdc);
if (ret != 0) {
dev_err(led->cdev.dev, "Failed to enable DCDC: %d\n", ret);
regulator_disable(led->isink);
- return;
+ return ret;
}

led->enabled = 1;
+
+ return ret;
}

-static void wm8350_led_disable(struct wm8350_led *led)
+static int wm8350_led_disable(struct wm8350_led *led)
{
- int ret;
+ int ret = 0;

if (!led->enabled)
- return;
+ return ret;

ret = regulator_disable(led->dcdc);
if (ret != 0) {
dev_err(led->cdev.dev, "Failed to disable DCDC: %d\n", ret);
- return;
+ return ret;
}

ret = regulator_disable(led->isink);
@@ -132,27 +134,29 @@ static void wm8350_led_disable(struct wm8350_led *led)
if (ret != 0)
dev_err(led->cdev.dev, "Failed to reenable DCDC: %d\n",
ret);
- return;
+ return ret;
}

led->enabled = 0;
+
+ return ret;
}

-static void led_work(struct work_struct *work)
+static int wm8350_led_set(struct led_classdev *led_cdev,
+ enum led_brightness value)
{
- struct wm8350_led *led = container_of(work, struct wm8350_led, work);
+ struct wm8350_led *led = to_wm8350_led(led_cdev);
+ unsigned long flags;
int ret;
int uA;
- unsigned long flags;

- mutex_lock(&led->mutex);
+ led->value = value;

spin_lock_irqsave(&led->value_lock, flags);

if (led->value == LED_OFF) {
spin_unlock_irqrestore(&led->value_lock, flags);
- wm8350_led_disable(led);
- goto out;
+ return wm8350_led_disable(led);
}

/* This scales linearly into the index of valid current
@@ -166,36 +170,21 @@ static void led_work(struct work_struct *work)

ret = regulator_set_current_limit(led->isink, isink_cur[uA],
isink_cur[uA]);
- if (ret != 0)
+ if (ret != 0) {
dev_err(led->cdev.dev, "Failed to set %duA: %d\n",
isink_cur[uA], ret);
+ return ret;
+ }

- wm8350_led_enable(led);
-
-out:
- mutex_unlock(&led->mutex);
-}
-
-static void wm8350_led_set(struct led_classdev *led_cdev,
- enum led_brightness value)
-{
- struct wm8350_led *led = to_wm8350_led(led_cdev);
- unsigned long flags;
-
- spin_lock_irqsave(&led->value_lock, flags);
- led->value = value;
- schedule_work(&led->work);
- spin_unlock_irqrestore(&led->value_lock, flags);
+ return wm8350_led_enable(led);
}

static void wm8350_led_shutdown(struct platform_device *pdev)
{
struct wm8350_led *led = platform_get_drvdata(pdev);

- mutex_lock(&led->mutex);
led->value = LED_OFF;
wm8350_led_disable(led);
- mutex_unlock(&led->mutex);
}

static int wm8350_led_probe(struct platform_device *pdev)
@@ -232,7 +221,7 @@ static int wm8350_led_probe(struct platform_device *pdev)
if (led == NULL)
return -ENOMEM;

- led->cdev.brightness_set = wm8350_led_set;
+ led->cdev.brightness_set_blocking = wm8350_led_set;
led->cdev.default_trigger = pdata->default_trigger;
led->cdev.name = pdata->name;
led->cdev.flags |= LED_CORE_SUSPENDRESUME;
@@ -251,8 +240,6 @@ static int wm8350_led_probe(struct platform_device *pdev)
pdata->max_uA);

spin_lock_init(&led->value_lock);
- mutex_init(&led->mutex);
- INIT_WORK(&led->work, led_work);
led->value = LED_OFF;
platform_set_drvdata(pdev, led);

@@ -264,7 +251,6 @@ static int wm8350_led_remove(struct platform_device *pdev)
struct wm8350_led *led = platform_get_drvdata(pdev);

led_classdev_unregister(&led->cdev);
- flush_work(&led->work);
wm8350_led_disable(led);
return 0;
}
diff --git a/include/linux/mfd/wm8350/pmic.h b/include/linux/mfd/wm8350/pmic.h
index 579b50c..7a09e7f 100644
--- a/include/linux/mfd/wm8350/pmic.h
+++ b/include/linux/mfd/wm8350/pmic.h
@@ -715,7 +715,6 @@ struct wm8350_led_platform_data {

struct wm8350_led {
struct platform_device *pdev;
- struct mutex mutex;
struct work_struct work;
spinlock_t value_lock;
enum led_brightness value;
--
1.7.9.5

2015-08-20 14:49:34

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 34/36] leds: gpio: Remove work queue

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 <[email protected]>
Cc: Raphael Assenat <[email protected]>
---
drivers/leds/leds-gpio.c | 64 +++++++++++++++-------------------------------
1 file changed, 21 insertions(+), 43 deletions(-)

diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index af1876a..3282ad3 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -20,32 +20,16 @@
#include <linux/platform_device.h>
#include <linux/property.h>
#include <linux/slab.h>
-#include <linux/workqueue.h>

struct gpio_led_data {
struct led_classdev cdev;
struct gpio_desc *gpiod;
- struct work_struct work;
- u8 new_level;
u8 can_sleep;
u8 blinking;
int (*platform_gpio_blink_set)(struct gpio_desc *desc, int state,
unsigned long *delay_on, unsigned long *delay_off);
};

-static void gpio_led_work(struct work_struct *work)
-{
- struct gpio_led_data *led_dat =
- container_of(work, struct gpio_led_data, work);
-
- if (led_dat->blinking) {
- led_dat->platform_gpio_blink_set(led_dat->gpiod,
- led_dat->new_level, NULL, NULL);
- led_dat->blinking = 0;
- } else
- gpiod_set_value_cansleep(led_dat->gpiod, led_dat->new_level);
-}
-
static void gpio_led_set(struct led_classdev *led_cdev,
enum led_brightness value)
{
@@ -58,21 +42,19 @@ static void gpio_led_set(struct led_classdev *led_cdev,
else
level = 1;

- /* Setting GPIOs with I2C/etc requires a task context, and we don't
- * seem to have a reliable way to know if we're already in one; so
- * let's just assume the worst.
- */
- if (led_dat->can_sleep) {
- led_dat->new_level = level;
- schedule_work(&led_dat->work);
- } else {
- if (led_dat->blinking) {
- led_dat->platform_gpio_blink_set(led_dat->gpiod, level,
- NULL, NULL);
- led_dat->blinking = 0;
- } else
- gpiod_set_value(led_dat->gpiod, level);
- }
+ if (led_dat->blinking) {
+ led_dat->platform_gpio_blink_set(led_dat->gpiod, level,
+ NULL, NULL);
+ led_dat->blinking = 0;
+ } else
+ gpiod_set_value(led_dat->gpiod, level);
+}
+
+static int gpio_led_set_blocking(struct led_classdev *led_cdev,
+ enum led_brightness value)
+{
+ gpio_led_set(led_cdev, value);
+ return 0;
}

static int gpio_blink_set(struct led_classdev *led_cdev,
@@ -125,12 +107,15 @@ static int create_gpio_led(const struct gpio_led *template,
led_dat->cdev.name = template->name;
led_dat->cdev.default_trigger = template->default_trigger;
led_dat->can_sleep = gpiod_cansleep(led_dat->gpiod);
+ if (!led_dat->can_sleep)
+ led_dat->cdev.brightness_set = gpio_led_set;
+ else
+ led_dat->cdev.brightness_set_blocking = gpio_led_set_blocking;
led_dat->blinking = 0;
if (blink_set) {
led_dat->platform_gpio_blink_set = blink_set;
led_dat->cdev.blink_set = gpio_blink_set;
}
- led_dat->cdev.brightness_set = gpio_led_set;
if (template->default_state == LEDS_GPIO_DEFSTATE_KEEP)
state = !!gpiod_get_value_cansleep(led_dat->gpiod);
else
@@ -143,17 +128,9 @@ static int create_gpio_led(const struct gpio_led *template,
if (ret < 0)
return ret;

- INIT_WORK(&led_dat->work, gpio_led_work);
-
return led_classdev_register(parent, &led_dat->cdev);
}

-static void delete_gpio_led(struct gpio_led_data *led)
-{
- led_classdev_unregister(&led->cdev);
- cancel_work_sync(&led->work);
-}
-
struct gpio_leds_priv {
int num_leds;
struct gpio_led_data leds[];
@@ -233,7 +210,7 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)

err:
for (count = priv->num_leds - 1; count >= 0; count--)
- delete_gpio_led(&priv->leds[count]);
+ led_classdev_unregister(&priv->leds[count].cdev);
return ERR_PTR(ret);
}

@@ -265,7 +242,8 @@ static int gpio_led_probe(struct platform_device *pdev)
if (ret < 0) {
/* On failure: unwind the led creations */
for (i = i - 1; i >= 0; i--)
- delete_gpio_led(&priv->leds[i]);
+ led_classdev_unregister(
+ &priv->leds[i].cdev);
return ret;
}
}
@@ -286,7 +264,7 @@ static int gpio_led_remove(struct platform_device *pdev)
int i;

for (i = 0; i < priv->num_leds; i++)
- delete_gpio_led(&priv->leds[i]);
+ led_classdev_unregister(&priv->leds[i].cdev);

return 0;
}
--
1.7.9.5

2015-08-20 14:47:59

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 35/36] leds: pwm: remove work queue

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 <[email protected]>
Cc: Raphael Assenat <[email protected]>
---
drivers/leds/leds-pwm.c | 34 +++++++++++++---------------------
1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 1d07e3e..bc50193 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -22,12 +22,10 @@
#include <linux/pwm.h>
#include <linux/leds_pwm.h>
#include <linux/slab.h>
-#include <linux/workqueue.h>

struct led_pwm_data {
struct led_classdev cdev;
struct pwm_device *pwm;
- struct work_struct work;
unsigned int active_low;
unsigned int period;
int duty;
@@ -51,14 +49,6 @@ static void __led_pwm_set(struct led_pwm_data *led_dat)
pwm_enable(led_dat->pwm);
}

-static void led_pwm_work(struct work_struct *work)
-{
- struct led_pwm_data *led_dat =
- container_of(work, struct led_pwm_data, work);
-
- __led_pwm_set(led_dat);
-}
-
static void led_pwm_set(struct led_classdev *led_cdev,
enum led_brightness brightness)
{
@@ -75,10 +65,14 @@ static void led_pwm_set(struct led_classdev *led_cdev,

led_dat->duty = duty;

- if (led_dat->can_sleep)
- schedule_work(&led_dat->work);
- else
- __led_pwm_set(led_dat);
+ __led_pwm_set(led_dat);
+}
+
+static int led_pwm_set_blocking(struct led_classdev *led_cdev,
+ enum led_brightness brightness)
+{
+ led_pwm_set(led_cdev, brightness);
+ return 0;
}

static inline size_t sizeof_pwm_leds_priv(int num_leds)
@@ -89,11 +83,8 @@ static inline size_t sizeof_pwm_leds_priv(int num_leds)

static void led_pwm_cleanup(struct led_pwm_priv *priv)
{
- while (priv->num_leds--) {
+ while (priv->num_leds--)
led_classdev_unregister(&priv->leds[priv->num_leds].cdev);
- if (priv->leds[priv->num_leds].can_sleep)
- cancel_work_sync(&priv->leds[priv->num_leds].work);
- }
}

static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
@@ -105,7 +96,6 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
led_data->active_low = led->active_low;
led_data->cdev.name = led->name;
led_data->cdev.default_trigger = led->default_trigger;
- led_data->cdev.brightness_set = led_pwm_set;
led_data->cdev.brightness = LED_OFF;
led_data->cdev.max_brightness = led->max_brightness;
led_data->cdev.flags = LED_CORE_SUSPENDRESUME;
@@ -122,8 +112,10 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
}

led_data->can_sleep = pwm_can_sleep(led_data->pwm);
- if (led_data->can_sleep)
- INIT_WORK(&led_data->work, led_pwm_work);
+ if (!led_data->can_sleep)
+ led_data->cdev.brightness_set = led_pwm_set;
+ else
+ led_data->cdev.brightness_set_blocking = led_pwm_set_blocking;

led_data->period = pwm_get_period(led_data->pwm);
if (!led_data->period && (led->pwm_period_ns > 0))
--
1.7.9.5

2015-08-20 14:48:01

by Jacek Anaszewski

[permalink] [raw]
Subject: [PATCH/RFC v6 36/36] leds: lm355x: Remove work queue

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 <[email protected]>
Cc: Daniel Jeong <[email protected]>
Cc: G.Shark Jeong <[email protected]>
---
drivers/leds/leds-lm355x.c | 85 ++++++++++++++------------------------------
1 file changed, 26 insertions(+), 59 deletions(-)

diff --git a/drivers/leds/leds-lm355x.c b/drivers/leds/leds-lm355x.c
index 48872997..6cb94f9 100644
--- a/drivers/leds/leds-lm355x.c
+++ b/drivers/leds/leds-lm355x.c
@@ -16,7 +16,6 @@
#include <linux/platform_device.h>
#include <linux/fs.h>
#include <linux/regmap.h>
-#include <linux/workqueue.h>
#include <linux/platform_data/leds-lm355x.h>

enum lm355x_type {
@@ -59,14 +58,6 @@ struct lm355x_chip_data {
struct led_classdev cdev_torch;
struct led_classdev cdev_indicator;

- struct work_struct work_flash;
- struct work_struct work_torch;
- struct work_struct work_indicator;
-
- u8 br_flash;
- u8 br_torch;
- u8 br_indicator;
-
struct lm355x_platform_data *pdata;
struct regmap *regmap;
struct mutex lock;
@@ -204,7 +195,7 @@ out:
}

/* chip control */
-static void lm355x_control(struct lm355x_chip_data *chip,
+static int lm355x_control(struct lm355x_chip_data *chip,
u8 brightness, enum lm355x_mode opmode)
{
int ret;
@@ -301,7 +292,7 @@ static void lm355x_control(struct lm355x_chip_data *chip,
case MODE_SHDN:
break;
default:
- return;
+ return -EINVAL;
}
/* operation mode control */
ret = regmap_update_bits(chip->regmap, preg[REG_OPMODE].regno,
@@ -309,73 +300,55 @@ static void lm355x_control(struct lm355x_chip_data *chip,
opmode << preg[REG_OPMODE].shift);
if (ret < 0)
goto out;
- return;
+ return ret;
out:
dev_err(chip->dev, "%s:i2c access fail to register\n", __func__);
- return;
+ return ret;
}

/* torch */
-static void lm355x_deferred_torch_brightness_set(struct work_struct *work)
-{
- struct lm355x_chip_data *chip =
- container_of(work, struct lm355x_chip_data, work_torch);

- mutex_lock(&chip->lock);
- lm355x_control(chip, chip->br_torch, MODE_TORCH);
- mutex_unlock(&chip->lock);
-}
-
-static void lm355x_torch_brightness_set(struct led_classdev *cdev,
+static int lm355x_torch_brightness_set(struct led_classdev *cdev,
enum led_brightness brightness)
{
struct lm355x_chip_data *chip =
container_of(cdev, struct lm355x_chip_data, cdev_torch);
-
- chip->br_torch = brightness;
- schedule_work(&chip->work_torch);
-}
-
-/* flash */
-static void lm355x_deferred_strobe_brightness_set(struct work_struct *work)
-{
- struct lm355x_chip_data *chip =
- container_of(work, struct lm355x_chip_data, work_flash);
+ int ret;

mutex_lock(&chip->lock);
- lm355x_control(chip, chip->br_flash, MODE_FLASH);
+ ret = lm355x_control(chip, brightness, MODE_TORCH);
mutex_unlock(&chip->lock);
+ return ret;
}

-static void lm355x_strobe_brightness_set(struct led_classdev *cdev,
+/* flash */
+
+static int lm355x_strobe_brightness_set(struct led_classdev *cdev,
enum led_brightness brightness)
{
struct lm355x_chip_data *chip =
container_of(cdev, struct lm355x_chip_data, cdev_flash);
-
- chip->br_flash = brightness;
- schedule_work(&chip->work_flash);
-}
-
-/* indicator */
-static void lm355x_deferred_indicator_brightness_set(struct work_struct *work)
-{
- struct lm355x_chip_data *chip =
- container_of(work, struct lm355x_chip_data, work_indicator);
+ int ret;

mutex_lock(&chip->lock);
- lm355x_control(chip, chip->br_indicator, MODE_INDIC);
+ ret = lm355x_control(chip, brightness, MODE_FLASH);
mutex_unlock(&chip->lock);
+ return ret;
}

-static void lm355x_indicator_brightness_set(struct led_classdev *cdev,
+/* indicator */
+
+static int lm355x_indicator_brightness_set(struct led_classdev *cdev,
enum led_brightness brightness)
{
struct lm355x_chip_data *chip =
container_of(cdev, struct lm355x_chip_data, cdev_indicator);
+ int ret;

- chip->br_indicator = brightness;
- schedule_work(&chip->work_indicator);
+ mutex_lock(&chip->lock);
+ ret = lm355x_control(chip, brightness, MODE_INDIC);
+ mutex_unlock(&chip->lock);
+ return ret;
}

/* indicator pattern only for lm3556*/
@@ -479,34 +452,31 @@ static int lm355x_probe(struct i2c_client *client,
goto err_out;

/* flash */
- INIT_WORK(&chip->work_flash, lm355x_deferred_strobe_brightness_set);
chip->cdev_flash.name = "flash";
chip->cdev_flash.max_brightness = 16;
- chip->cdev_flash.brightness_set = lm355x_strobe_brightness_set;
+ chip->cdev_flash.brightness_set_blocking = lm355x_strobe_brightness_set;
chip->cdev_flash.default_trigger = "flash";
err = led_classdev_register((struct device *)
&client->dev, &chip->cdev_flash);
if (err < 0)
goto err_out;
/* torch */
- INIT_WORK(&chip->work_torch, lm355x_deferred_torch_brightness_set);
chip->cdev_torch.name = "torch";
chip->cdev_torch.max_brightness = 8;
- chip->cdev_torch.brightness_set = lm355x_torch_brightness_set;
+ chip->cdev_torch.brightness_set_blocking = lm355x_torch_brightness_set;
chip->cdev_torch.default_trigger = "torch";
err = led_classdev_register((struct device *)
&client->dev, &chip->cdev_torch);
if (err < 0)
goto err_create_torch_file;
/* indicator */
- INIT_WORK(&chip->work_indicator,
- lm355x_deferred_indicator_brightness_set);
chip->cdev_indicator.name = "indicator";
if (id->driver_data == CHIP_LM3554)
chip->cdev_indicator.max_brightness = 4;
else
chip->cdev_indicator.max_brightness = 8;
- chip->cdev_indicator.brightness_set = lm355x_indicator_brightness_set;
+ chip->cdev_indicator.brightness_set_blocking =
+ lm355x_indicator_brightness_set;
/* indicator pattern control only for LM3556 */
if (id->driver_data == CHIP_LM3556)
chip->cdev_indicator.groups = lm355x_indicator_groups;
@@ -534,11 +504,8 @@ static int lm355x_remove(struct i2c_client *client)

regmap_write(chip->regmap, preg[REG_OPMODE].regno, 0);
led_classdev_unregister(&chip->cdev_indicator);
- flush_work(&chip->work_indicator);
led_classdev_unregister(&chip->cdev_torch);
- flush_work(&chip->work_torch);
led_classdev_unregister(&chip->cdev_flash);
- flush_work(&chip->work_flash);
dev_info(&client->dev, "%s is removed\n", lm355x_name[chip->type]);

return 0;
--
1.7.9.5

2015-08-20 15:29:20

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH/RFC v6 01/36] leds: Add brightness_set_blocking op

On Thu, Aug 20, 2015 at 04:43:31PM +0200, Jacek Anaszewski wrote:
> This patch adds a new brightness_set_blocking op to the LED subsystem.
> The op is intended for drivers that set brightness in a blocking way,
> i.e. they either can sleep or use delays while setting brightness.

Thanks for adding this. This is the correct way to go.

Acked-by: Andrew Lunn <[email protected]>

Andrew

> Signed-off-by: Jacek Anaszewski <[email protected]>
> Cc: Andrew Lunn <[email protected]>
> Cc: Sakari Ailus <[email protected]>
> Cc: Pavel Machek <[email protected]>
> Cc: Stas Sergeev <[email protected]>
> ---
> drivers/leds/led-class.c | 3 +++
> include/linux/leds.h | 6 ++++++
> 2 files changed, 9 insertions(+)
>
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index ca51d58..93a2414 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -267,6 +267,9 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
> dev_warn(parent, "Led %s renamed to %s due to name collision",
> led_cdev->name, dev_name(led_cdev->dev));
>
> + WARN_ON(led_cdev->brightness_set &&
> + led_cdev->brightness_set_blocking);
> +
> #ifdef CONFIG_LEDS_TRIGGERS
> init_rwsem(&led_cdev->trigger_lock);
> #endif
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index b122eea..85fad6b 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -54,6 +54,12 @@ struct led_classdev {
> void (*brightness_set)(struct led_classdev *led_cdev,
> enum led_brightness brightness);
> /*
> + * Intended for drivers that either can sleep or use delays while
> + * setting brightness.
> + */
> + int (*brightness_set_blocking)(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.
> */
> --
> 1.7.9.5
>

2015-08-20 15:50:49

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH/RFC v6 02/36] leds: Add led_set_brightness_sync to the public LED subsystem API

On Thu, Aug 20, 2015 at 04:43:32PM +0200, Jacek Anaszewski wrote:
> led_set_brightness_sync function was visible only internally to the
> LED subsystem. It is now being made publicly available since it has
> become apparent that this is a caller who should decide whether
> brightness is to be set in a synchronous or an asynchronous way.
>
> The function is modified to use brightness_set_blocking or
> brightness_set op, depending on which of them is implemented, in case
> brightness_set_sync op wasn't been provided.
>
> Signed-off-by: Jacek Anaszewski <[email protected]>
> Cc: Andrew Lunn <[email protected]>
> Cc: Sakari Ailus <[email protected]>
> Cc: Pavel Machek <[email protected]>
> Cc: Stas Sergeev <[email protected]>
> ---
> drivers/leds/led-core.c | 25 +++++++++++++++++++++++++
> drivers/leds/leds.h | 13 -------------
> include/linux/leds.h | 15 +++++++++++++++
> 3 files changed, 40 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index 549de7e..3f3b71d 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -141,6 +141,31 @@ void led_set_brightness(struct led_classdev *led_cdev,
> }
> EXPORT_SYMBOL(led_set_brightness);
>
> +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)
> + return 0;
> +
> + if (led_cdev->brightness_set_sync)
> + ret = led_cdev->brightness_set_sync(led_cdev,
> + led_cdev->brightness);
> + else if (led_cdev->brightness_set_blocking)
> + led_cdev->brightness_set_blocking(led_cdev,
> + led_cdev->brightness);
> + else if (led_cdev->brightness_set)
> + led_cdev->brightness_set(led_cdev, led_cdev->brightness);

I don't see how this can be correct, when you say:

> +/**
> + * 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 register,
> + * and it can sleep.

led_cdev->brightness_set_sync() is fine, it is defined to the
synchronous.

led_cdev->brightness_set_blocking() is O.K.

But led_cdev->brightness_set() only guarantees:

/* Set LED brightness level */
/* Must not sleep, use a workqueue if needed */
void (*brightness_set)(struct led_classdev *led_cdev,
enum led_brightness brightness);


and it seems like 50% of the current drivers use work queues. It has
never been defined to by synchronous. If brightness_set() is your only
choice, you should return -ENOSUP.

With time, you could review all the drivers and move those that are
synchronous to brightness_set_sync().

Andrew

2015-08-20 16:17:01

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH/RFC v6 05/36] leds: Improve setting brightness in a non sleeping way

On Thu, Aug 20, 2015 at 04:43:35PM +0200, Jacek Anaszewski wrote:
> This patch 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.

O.K, so i've lost the plot. _sync, _asymc, _nosleep, etc. Too many
changes without a clearly documented vision of what you are trying to
achieve.

How about splitting this up into at least two patch sets.

1) Add the brightness_set_blocking op and the minimum of changes
needed to the core to make it work, and the driver changes taking out
the work queue.

2) A set of patches cleaning up the core and its API. We want a well
documented linux/leds.h and drivers/led/leds.h defining the APIs which
users and triggers should be using.

Andrew

2015-08-21 07:47:31

by Antonio Ospite

[permalink] [raw]
Subject: Re: [PATCH/RFC v6 19/36] leds: lp3944: Remove work queue

On Thu, 20 Aug 2015 16:43:49 +0200
Jacek Anaszewski <[email protected]> wrote:

> From: Andrew Lunn <[email protected]>
>
> Now the core implements the work queue, remove it from the driver,
> and switch to using brightness_set_blocking op.
>
> Signed-off-by: Andrew Lunn <[email protected]>
> Cc: Antonio Ospite <[email protected]>
> Signed-off-by: Jacek Anaszewski <[email protected]>
>

Acked-by: Antonio Ospite <[email protected]>

> ---
> drivers/leds/leds-lp3944.c | 32 ++++++++------------------------
> 1 file changed, 8 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/leds/leds-lp3944.c b/drivers/leds/leds-lp3944.c
> index 53144fb..6c758ae 100644
> --- a/drivers/leds/leds-lp3944.c
> +++ b/drivers/leds/leds-lp3944.c
> @@ -31,7 +31,6 @@
> #include <linux/slab.h>
> #include <linux/leds.h>
> #include <linux/mutex.h>
> -#include <linux/workqueue.h>
> #include <linux/leds-lp3944.h>
>
> /* Read Only Registers */
> @@ -68,10 +67,8 @@
> struct lp3944_led_data {
> u8 id;
> enum lp3944_type type;
> - enum lp3944_status status;
> struct led_classdev ldev;
> struct i2c_client *client;
> - struct work_struct work;
> };
>
> struct lp3944_data {
> @@ -275,13 +272,12 @@ static int lp3944_led_set_blink(struct led_classdev *led_cdev,
> dev_dbg(&led->client->dev, "%s: OK hardware accelerated blink!\n",
> __func__);
>
> - led->status = LP3944_LED_STATUS_DIM0;
> - schedule_work(&led->work);
> + lp3944_led_set(led, LP3944_LED_STATUS_DIM0);
>
> return 0;
> }
>
> -static void lp3944_led_set_brightness(struct led_classdev *led_cdev,
> +static int lp3944_led_set_brightness(struct led_classdev *led_cdev,
> enum led_brightness brightness)
> {
> struct lp3944_led_data *led = ldev_to_led(led_cdev);
> @@ -289,16 +285,7 @@ static void lp3944_led_set_brightness(struct led_classdev *led_cdev,
> dev_dbg(&led->client->dev, "%s: %s, %d\n",
> __func__, led_cdev->name, brightness);
>
> - led->status = !!brightness;
> - schedule_work(&led->work);
> -}
> -
> -static void lp3944_led_work(struct work_struct *work)
> -{
> - struct lp3944_led_data *led;
> -
> - led = container_of(work, struct lp3944_led_data, work);
> - lp3944_led_set(led, led->status);
> + return lp3944_led_set(led, !!brightness);
> }
>
> static int lp3944_configure(struct i2c_client *client,
> @@ -318,14 +305,13 @@ static int lp3944_configure(struct i2c_client *client,
> case LP3944_LED_TYPE_LED:
> case LP3944_LED_TYPE_LED_INVERTED:
> led->type = pled->type;
> - led->status = pled->status;
> led->ldev.name = pled->name;
> led->ldev.max_brightness = 1;
> - led->ldev.brightness_set = lp3944_led_set_brightness;
> + led->ldev.brightness_set_blocking =
> + lp3944_led_set_brightness;
> led->ldev.blink_set = lp3944_led_set_blink;
> led->ldev.flags = LED_CORE_SUSPENDRESUME;
>
> - INIT_WORK(&led->work, lp3944_led_work);
> err = led_classdev_register(&client->dev, &led->ldev);
> if (err < 0) {
> dev_err(&client->dev,
> @@ -336,14 +322,14 @@ static int lp3944_configure(struct i2c_client *client,
>
> /* to expose the default value to userspace */
> led->ldev.brightness =
> - (enum led_brightness) led->status;
> + (enum led_brightness) pled->status;
>
> /* Set the default led status */
> - err = lp3944_led_set(led, led->status);
> + err = lp3944_led_set(led, pled->status);
> if (err < 0) {
> dev_err(&client->dev,
> "%s couldn't set STATUS %d\n",
> - led->ldev.name, led->status);
> + led->ldev.name, pled->status);
> goto exit;
> }
> break;
> @@ -364,7 +350,6 @@ exit:
> case LP3944_LED_TYPE_LED:
> case LP3944_LED_TYPE_LED_INVERTED:
> led_classdev_unregister(&data->leds[i].ldev);
> - cancel_work_sync(&data->leds[i].work);
> break;
>
> case LP3944_LED_TYPE_NONE:
> @@ -424,7 +409,6 @@ static int lp3944_remove(struct i2c_client *client)
> case LP3944_LED_TYPE_LED:
> case LP3944_LED_TYPE_LED_INVERTED:
> led_classdev_unregister(&data->leds[i].ldev);
> - cancel_work_sync(&data->leds[i].work);
> break;
>
> case LP3944_LED_TYPE_NONE:
> --
> 1.7.9.5
>


--
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

2015-08-21 07:49:48

by Antonio Ospite

[permalink] [raw]
Subject: Re: [PATCH/RFC v6 32/36] leds: regulator: Remove work queue

On Thu, 20 Aug 2015 16:44:02 +0200
Jacek Anaszewski <[email protected]> wrote:

> From: Andrew Lunn <[email protected]>
>
> Now the core implements the work queue, remove it from the drivers,
> and switch to using brightness_set_blocking op.
>
> Signed-off-by: Andrew Lunn <[email protected]>
> Cc: Antonio Ospite <[email protected]>
> Signed-off-by: Jacek Anaszewski <[email protected]>
>

Acked-by: Antonio Ospite <[email protected]>

> ---
> drivers/leds/leds-regulator.c | 46 +++++++++++------------------------------
> 1 file changed, 12 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/leds/leds-regulator.c b/drivers/leds/leds-regulator.c
> index ffc2139..acf77ca 100644
> --- a/drivers/leds/leds-regulator.c
> +++ b/drivers/leds/leds-regulator.c
> @@ -14,7 +14,6 @@
> #include <linux/module.h>
> #include <linux/err.h>
> #include <linux/slab.h>
> -#include <linux/workqueue.h>
> #include <linux/leds.h>
> #include <linux/leds-regulator.h>
> #include <linux/platform_device.h>
> @@ -25,10 +24,8 @@
>
> struct regulator_led {
> struct led_classdev cdev;
> - enum led_brightness value;
> int enabled;
> struct mutex mutex;
> - struct work_struct work;
>
> struct regulator *vcc;
> };
> @@ -94,22 +91,24 @@ static void regulator_led_disable(struct regulator_led *led)
> led->enabled = 0;
> }
>
> -static void regulator_led_set_value(struct regulator_led *led)
> +static int regulator_led_brightness_set(struct led_classdev *led_cdev,
> + enum led_brightness value)
> {
> + struct regulator_led *led = to_regulator_led(led_cdev);
> int voltage;
> - int ret;
> + int ret = 0;
>
> mutex_lock(&led->mutex);
>
> - if (led->value == LED_OFF) {
> + if (value == LED_OFF) {
> regulator_led_disable(led);
> goto out;
> }
>
> if (led->cdev.max_brightness > 1) {
> - voltage = led_regulator_get_voltage(led->vcc, led->value);
> + voltage = led_regulator_get_voltage(led->vcc, value);
> dev_dbg(led->cdev.dev, "brightness: %d voltage: %d\n",
> - led->value, voltage);
> + value, voltage);
>
> ret = regulator_set_voltage(led->vcc, voltage, voltage);
> if (ret != 0)
> @@ -121,23 +120,7 @@ static void regulator_led_set_value(struct regulator_led *led)
>
> out:
> mutex_unlock(&led->mutex);
> -}
> -
> -static void led_work(struct work_struct *work)
> -{
> - struct regulator_led *led;
> -
> - led = container_of(work, struct regulator_led, work);
> - regulator_led_set_value(led);
> -}
> -
> -static void regulator_led_brightness_set(struct led_classdev *led_cdev,
> - enum led_brightness value)
> -{
> - struct regulator_led *led = to_regulator_led(led_cdev);
> -
> - led->value = value;
> - schedule_work(&led->work);
> + return ret;
> }
>
> static int regulator_led_probe(struct platform_device *pdev)
> @@ -169,9 +152,8 @@ static int regulator_led_probe(struct platform_device *pdev)
> pdata->brightness);
> return -EINVAL;
> }
> - led->value = pdata->brightness;
>
> - led->cdev.brightness_set = regulator_led_brightness_set;
> + led->cdev.brightness_set_blocking = regulator_led_brightness_set;
> led->cdev.name = pdata->name;
> led->cdev.flags |= LED_CORE_SUSPENDRESUME;
> led->vcc = vcc;
> @@ -181,21 +163,18 @@ static int regulator_led_probe(struct platform_device *pdev)
> led->enabled = 1;
>
> mutex_init(&led->mutex);
> - INIT_WORK(&led->work, led_work);
>
> platform_set_drvdata(pdev, led);
>
> ret = led_classdev_register(&pdev->dev, &led->cdev);
> - if (ret < 0) {
> - cancel_work_sync(&led->work);
> + if (ret < 0)
> return ret;
> - }
>
> /* to expose the default value to userspace */
> - led->cdev.brightness = led->value;
> + led->cdev.brightness = pdata->brightness;
>
> /* Set the default led status */
> - regulator_led_set_value(led);
> + regulator_led_brightness_set(&led->cdev, led->cdev.brightness);
>
> return 0;
> }
> @@ -205,7 +184,6 @@ static int regulator_led_remove(struct platform_device *pdev)
> struct regulator_led *led = platform_get_drvdata(pdev);
>
> led_classdev_unregister(&led->cdev);
> - cancel_work_sync(&led->work);
> regulator_led_disable(led);
> return 0;
> }
> --
> 1.7.9.5
>


--
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

2015-08-21 09:22:44

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [PATCH/RFC v6 05/36] leds: Improve setting brightness in a non sleeping way

Hi Andrew,

Thanks for the review.

On 08/20/2015 06:09 PM, Andrew Lunn wrote:
> On Thu, Aug 20, 2015 at 04:43:35PM +0200, Jacek Anaszewski wrote:
>> This patch 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.
>
> O.K, so i've lost the plot. _sync, _asymc, _nosleep, etc. Too many
> changes without a clearly documented vision of what you are trying to
> achieve.
>
> How about splitting this up into at least two patch sets.
>
> 1) Add the brightness_set_blocking op and the minimum of changes
> needed to the core to make it work, and the driver changes taking out
> the work queue.

The minimum of changes needed includes harnessing existing
set_brightness_work for setting brightness instead of the work queues
in the drivers. First three patches in the patch set are indispensable
to implement this and avoid breakage of blinking feature. Especially
patch 3/36 is quite complex, but I didn't have better idea on how to
tackle this problem without breaking bisect,

> 2) A set of patches cleaning up the core and its API. We want a well
> documented linux/leds.h and drivers/led/leds.h defining the APIs which
> users and triggers should be using.

Of course, I will document led_set_brightness_async and
led_set_brightness_nosleep functions.

--
Best Regards,
Jacek Anaszewski

2015-08-21 17:52:32

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH/RFC v6 05/36] leds: Improve setting brightness in a non sleeping way

On Fri, Aug 21, 2015 at 11:22:33AM +0200, Jacek Anaszewski wrote:
> Hi Andrew,
>
> Thanks for the review.
>
> On 08/20/2015 06:09 PM, Andrew Lunn wrote:
> >On Thu, Aug 20, 2015 at 04:43:35PM +0200, Jacek Anaszewski wrote:
> >>This patch 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.
> >
> >O.K, so i've lost the plot. _sync, _asymc, _nosleep, etc. Too many
> >changes without a clearly documented vision of what you are trying to
> >achieve.
> >
> >How about splitting this up into at least two patch sets.
> >
> >1) Add the brightness_set_blocking op and the minimum of changes
> >needed to the core to make it work, and the driver changes taking out
> >the work queue.
>
> The minimum of changes needed includes harnessing existing
> set_brightness_work for setting brightness instead of the work queues
> in the drivers.

I'm not sure that is the correct architecture.

The work queue is in the class, not the core. So you need to define
the core API to not need this work queue. What exactly is the core
API? What does it say about blocking and non-blocking, synchronous and
non-synchronous?

Adding the work queue to the core is the quick and simple way of
removing it from the drivers. Maybe that is the way forward. You can
then later come back and sort out the core API and the class API, and
clean up the documentation.

Andrew

2015-08-21 20:31:13

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [PATCH/RFC v6 05/36] leds: Improve setting brightness in a non sleeping way

On 08/21/2015 07:45 PM, Andrew Lunn wrote:
> On Fri, Aug 21, 2015 at 11:22:33AM +0200, Jacek Anaszewski wrote:
>> Hi Andrew,
>>
>> Thanks for the review.
>>
>> On 08/20/2015 06:09 PM, Andrew Lunn wrote:
>>> On Thu, Aug 20, 2015 at 04:43:35PM +0200, Jacek Anaszewski wrote:
>>>> This patch 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.
>>>
>>> O.K, so i've lost the plot. _sync, _asymc, _nosleep, etc. Too many
>>> changes without a clearly documented vision of what you are trying to
>>> achieve.
>>>
>>> How about splitting this up into at least two patch sets.
>>>
>>> 1) Add the brightness_set_blocking op and the minimum of changes
>>> needed to the core to make it work, and the driver changes taking out
>>> the work queue.
>>
>> The minimum of changes needed includes harnessing existing
>> set_brightness_work for setting brightness instead of the work queues
>> in the drivers.
>
> I'm not sure that is the correct architecture.
>
> The work queue is in the class, not the core. So you need to define
> the core API to not need this work queue.

If we wanted to follow this logic then we should also ask
if led_timer_function shouldn't be placed in the core too.
set_brightness_work was introduced only because of out-of-tree
user which called led_set_brightness from hard irq context,
which caused problems related to locking between hard and
softirq, when timer trigger was enabled.

> What exactly is the core
> API? What does it say about blocking and non-blocking, synchronous and
> non-synchronous?

The core API is everything in linux/leds.h not prefixed with
led_classdev_, i.e functions for controlling brightness, blinking,
and triggers. Until the addition of LED flash class extension
things like sync/async, blocking/non_blocking weren't considered
neither by the API, nor by documentation. There was only a comment
over brightness_set op declaration, that it mustn't sleep.
This requirement stems from the fact that some triggers, e.g. timer,
set brightness from soft irq context.

While implementing LED flash extension we noticed that LED subsystem
doesn't provide a means for setting brightness synchronously,
and we added led_set_brightness_sync API for this, SET_BRIGHTNESS_SYNC,
and SET_BRIGHTNESS_ASYNC flags. Recently we agreed that this is not
a driver that should decide about sync/async way of brightness setting,
but the caller. That's why I am removing the flags and modifying the
sync API. Actually I tried to tell this story in the commit messages of
the patches making up my recent patch set.

> Adding the work queue to the core is the quick and simple way of
> removing it from the drivers. Maybe that is the way forward. You can
> then later come back and sort out the core API and the class API, and
> clean up the documentation.

This work queue from led-class.c is used for setting brightness, when
blink timer is on. Blinking is the functionality from the LED core, so
the work queue should also belong to the core. It should be moved
there along with led_timer_function, for consistency reasons. In view
of the above, using it for setting brightness by blocking drivers
would be correct from the architectural point of view.

--
Best Regards,
Jacek Anaszewski

2015-08-21 20:49:26

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH/RFC v6 05/36] leds: Improve setting brightness in a non sleeping way

> This work queue from led-class.c is used for setting brightness, when
> blink timer is on. Blinking is the functionality from the LED core,
> so the work queue should also belong to the core. It should be moved
> there along with led_timer_function, for consistency reasons. In view
> of the above, using it for setting brightness by blocking drivers
> would be correct from the architectural point of view.

So lets assume we are not doing all the changes in a single patch
set. Its too complex. You say the work queue should be in the
core. Lets put a work queue in the core to handle the blocking op, and
strip it out of the drivers. That allows you to get a lot of driver
patches merged.

You can then work on moving led_timer_function into the core, etc,
with a much smaller patch set, and hopefully in a number of small
steps which are easy to review.

Andrew

2015-08-24 08:11:53

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [PATCH/RFC v6 05/36] leds: Improve setting brightness in a non sleeping way

On 08/21/2015 10:42 PM, Andrew Lunn wrote:
>> This work queue from led-class.c is used for setting brightness, when
>> blink timer is on. Blinking is the functionality from the LED core,
>> so the work queue should also belong to the core. It should be moved
>> there along with led_timer_function, for consistency reasons. In view
>> of the above, using it for setting brightness by blocking drivers
>> would be correct from the architectural point of view.
>
> So lets assume we are not doing all the changes in a single patch
> set. Its too complex. You say the work queue should be in the
> core. Lets put a work queue in the core to handle the blocking op, and
> strip it out of the drivers. That allows you to get a lot of driver
> patches merged.

Moving the work queue to the core is rather a cosmetic change and it is
not required for adapting it to setting brightness for blocking drivers.
I think that we could postpone the cosmetic changes to the moment when
other essential patches are merged.

Like I explained in the previous message, patches from 1 to 4 are
the minimum of the changes required for removing work queues from
drivers. I am aware that the changes may be hard to analyze, but the
things are entangled together and changes must be done in a few places
simultaneously. Those crucial changes are gathered in the patch [1].
I tried to explain them in the commit messages. Maybe my explanations
weren't comprehensive enough, if so, I'd be glad if you could indicate
what could be added/modified in the description to make it more clear.

> You can then work on moving led_timer_function into the core, etc,
> with a much smaller patch set, and hopefully in a number of small
> steps which are easy to review.
>
> Andrew
>

[1] https://lkml.org/lkml/2015/8/11/191
--
Best Regards,
Jacek Anaszewski

2015-08-24 11:40:20

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH/RFC v6 01/36] leds: Add brightness_set_blocking op

On Thu 2015-08-20 16:43:31, Jacek Anaszewski wrote:
> This patch adds a new brightness_set_blocking op to the LED subsystem.
> The op is intended for drivers that set brightness in a blocking way,
> i.e. they either can sleep or use delays while setting brightness.
>
> Signed-off-by: Jacek Anaszewski <[email protected]>
> Cc: Andrew Lunn <[email protected]>
> Cc: Sakari Ailus <[email protected]>

Acked-by: Pavel Machek <[email protected]>

Thanks!
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2015-11-16 13:39:50

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [PATCH/RFC v6 00/36] Remove work queues from LED class drivers

On 08/20/2015 04:43 PM, Jacek Anaszewski wrote:
> This is sixth version of the RFC aiming at removing work queues
> from LED class drivers, as well as getting rid of complimentary
> functionalities introduced along with addition of LED flash class
> extension.
>
> ======================
> Changes from version 5
> ======================
> - avoided changing semantics of brightness_set op in favour
> of introducing brightness_set_blocking op, which also allowed
> to get rid of LED_BRIGHTNESS_BLOCKING flag - Andrew thanks for
> the hints
> - made new brightness_set_blocking op returning int, which entailed
> the need for extending the scope of modifications in the drivers
> from which work queues are removed. Those affected drivers now
> implement the new op instead of brightness_set, which returns void
> - since LED_BRIGHTNESS_FAST flags is not needed with the new
> approach, its introduction has been postponed
>
> ======================
> Changes from version 4
> ======================
> - switched to using two separate ops for blocking and non-blocking way
> of setting brightness as requested by Pavel Machek.
> - improved patches for leds-lm3533, leds-regulator, leds-lp3944
> and leds-ipaq-micro drivers in response to review remarks
>
> ======================
> Changes from version 3
> ======================
> - fixed return value in one of intermediary patches
> - changed the comment over the brightness_set op member
> of struct led_classdev
> - added patches adjusting LED subsystem drivers to the introduced
> modifications - they have been only compile-tested
>
> ======================
> Changes from version 2
> ======================
>
> - split changes to several incremental patches
> - removed SET_BRIGHTNESS_SYNC and SET_BRIGHTNESS_ASYNC flags
> - fixed led_set_brightness_async function instead of renaming it
>
> ======================
> Changes from version 1
> ======================
>
> V2 includes also patches for one LED class driver
> and two LED flash class drivers, that show how the
> drivers will benefit from the optimization being
> introduced in the first patch of this patch set.
>
> I was able to test only the LED Flash class drivers.
>
> Original message from the patch 483a3122 ("leds: Use set_brightness_work for
> brightness_set ops that can sleep") that was sent previously as a single one:
>
> 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
>
> Andrew Lunn (23):
> leds: tlc591xx: Remove work queue
> leds: 88pm860x: Remove work queue
> leds: adp5520: Remove work queue
> leds: bd2802: Remove work queue
> leds: blinkm: Remove work queue
> leds: lm3533: Remove work queue
> leds: lm3642: Remove work queue
> leds: pca9532: Remove work queue for LEDs.
> leds: lp3944: Remove work queue
> leds: lp55xx: Remove work queue
> leds: lp8788: Remove work queue
> leds: lp8860: Remove work queue
> leds: pca955x: Remove work queue
> leds: pca963x: Remove work queue
> leds: wm831x: Remove work queue
> leds: da903x: Remove work queue
> leds: da9052: Remove work queue
> leds: dac124d085: Remove work queue
> leds: lt3593: Remove work queue
> leds: max8997: Remove unneeded workqueue include
> leds: mc13783: Remove work queue
> leds: regulator: Remove work queue
> leds: wm8350: Remove work queue
>
> Jacek Anaszewski (13):
> leds: Add brightness_set_blocking op
> leds: Add led_set_brightness_sync to the public LED subsystem API
> leds: Improve asynchronous path of setting brightness
> leds: Add an internal led_set_brightness_nosleep function
> leds: Improve setting brightness in a non sleeping way
> leds: Drivers shouldn't enforce SYNC/ASYNC brightness setting
> Documentation: leds: Add description of brightness_set* ops
> leds: ktd2692: Remove work queue
> leds: aat1290: Remove work queue
> leds: max77693: Remove work queue
> leds: gpio: Remove work queue
> leds: pwm: remove work queue
> leds: lm355x: Remove work queue
>
> Documentation/leds/leds-class.txt | 11 ++++
> drivers/leds/led-class-flash.c | 7 ---
> drivers/leds/led-class.c | 22 +++++---
> drivers/leds/led-core.c | 79 ++++++++++++++++++++------
> drivers/leds/leds-88pm860x.c | 23 +++-----
> drivers/leds/leds-aat1290.c | 50 +++++------------
> drivers/leds/leds-adp5520.c | 26 ++-------
> drivers/leds/leds-bd2802.c | 39 +++++--------
> drivers/leds/leds-blinkm.c | 87 ++++++-----------------------
> drivers/leds/leds-da903x.c | 46 ++++++---------
> drivers/leds/leds-da9052.c | 39 ++++---------
> drivers/leds/leds-dac124s085.c | 38 ++++---------
> drivers/leds/leds-gpio.c | 64 +++++++--------------
> drivers/leds/leds-ktd2692.c | 41 ++------------
> drivers/leds/leds-lm3533.c | 30 ++--------
> drivers/leds/leds-lm355x.c | 85 +++++++++-------------------
> drivers/leds/leds-lm3642.c | 73 ++++++++----------------
> drivers/leds/leds-lp3944.c | 32 +++--------
> drivers/leds/leds-lp5521.c | 11 ++--
> drivers/leds/leds-lp5523.c | 10 ++--
> drivers/leds/leds-lp5562.c | 11 ++--
> drivers/leds/leds-lp55xx-common.c | 12 ++--
> drivers/leds/leds-lp55xx-common.h | 6 +-
> drivers/leds/leds-lp8501.c | 11 ++--
> drivers/leds/leds-lp8788.c | 48 +++++++---------
> drivers/leds/leds-lp8860.c | 27 +++------
> drivers/leds/leds-lt3593.c | 33 ++++-------
> drivers/leds/leds-max77693.c | 57 +++----------------
> drivers/leds/leds-max8997.c | 1 -
> drivers/leds/leds-mc13783.c | 35 +++---------
> drivers/leds/leds-pca9532.c | 28 ++++------
> drivers/leds/leds-pca955x.c | 39 +++----------
> drivers/leds/leds-pca963x.c | 80 ++++++++------------------
> drivers/leds/leds-pwm.c | 34 +++++------
> drivers/leds/leds-regulator.c | 46 ++++-----------
> drivers/leds/leds-tlc591xx.c | 31 +++-------
> drivers/leds/leds-wm831x-status.c | 25 +++------
> drivers/leds/leds-wm8350.c | 64 +++++++++------------
> drivers/leds/leds.h | 26 +++------
> 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 +--
> include/linux/leds.h | 31 +++++++---
> include/linux/mfd/wm8350/pmic.h | 1 -
> 47 files changed, 512 insertions(+), 979 deletions(-)
>

Applied patches 11-36 from this set.

--
Best Regards,
Jacek Anaszewski