Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1210531 > unrolled thread

[PATCH/RFC v6 00/36] Remove work queues from LED class drivers

Back to article view | Back to linux.kernel

Contents

  [PATCH/RFC v6 00/36] Remove work queues from LED class drivers Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 31/36] leds: mc13783: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 14/36] leds: bd2802: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 07/36] Documentation: leds: Add description of  brightness_set* ops Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 02/36] leds: Add led_set_brightness_sync to the public  LED subsystem API Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
      Re: [PATCH/RFC v6 02/36] leds: Add led_set_brightness_sync to the  public LED subsystem API Andrew Lunn <andrew@lunn.ch> - 2015-08-20 18:00 +0200
    [PATCH/RFC v6 27/36] leds: da9052: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 01/36] leds: Add brightness_set_blocking op Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
      Re: [PATCH/RFC v6 01/36] leds: Add brightness_set_blocking op Andrew Lunn <andrew@lunn.ch> - 2015-08-20 17:30 +0200
      Re: [PATCH/RFC v6 01/36] leds: Add brightness_set_blocking op Pavel Machek <pavel@ucw.cz> - 2015-08-24 13:50 +0200
    [PATCH/RFC v6 17/36] leds: lm3642: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 06/36] leds: Drivers shouldn't enforce SYNC/ASYNC  brightness setting Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 34/36] leds: gpio: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 09/36] leds: aat1290: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 03/36] leds: Improve asynchronous path of setting  brightness Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 16/36] leds: lm3533: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 36/36] leds: lm355x: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 08/36] leds: ktd2692: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 33/36] leds: wm8350: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 18/36] leds: pca9532: Remove work queue for LEDs. Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 13/36] leds: adp5520: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 05/36] leds: Improve setting brightness in a non  sleeping way Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
      Re: [PATCH/RFC v6 05/36] leds: Improve setting brightness in a non  sleeping way Andrew Lunn <andrew@lunn.ch> - 2015-08-20 18:20 +0200
        Re: [PATCH/RFC v6 05/36] leds: Improve setting brightness in a non  sleeping way Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-21 11:30 +0200
          Re: [PATCH/RFC v6 05/36] leds: Improve setting brightness in a non  sleeping way Andrew Lunn <andrew@lunn.ch> - 2015-08-21 20:00 +0200
            Re: [PATCH/RFC v6 05/36] leds: Improve setting brightness in a non  sleeping way Jacek Anaszewski <jacek.anaszewski@gmail.com> - 2015-08-21 22:40 +0200
              Re: [PATCH/RFC v6 05/36] leds: Improve setting brightness in a non  sleeping way Andrew Lunn <andrew@lunn.ch> - 2015-08-21 22:50 +0200
                Re: [PATCH/RFC v6 05/36] leds: Improve setting brightness in a non  sleeping way Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-24 10:20 +0200
    [PATCH/RFC v6 12/36] leds: 88pm860x: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 21/36] leds: lp8788: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 29/36] leds: lt3593: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 30/36] leds: max8997: Remove unneeded workqueue include Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 10/36] leds: max77693: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 15/36] leds: blinkm: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 22/36] leds: lp8860: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 11/36] leds: tlc591xx: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 32/36] leds: regulator: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
      Re: [PATCH/RFC v6 32/36] leds: regulator: Remove work queue Antonio Ospite <ao2@ao2.it> - 2015-08-21 09:50 +0200
    [PATCH/RFC v6 19/36] leds: lp3944: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
      Re: [PATCH/RFC v6 19/36] leds: lp3944: Remove work queue Antonio Ospite <ao2@ao2.it> - 2015-08-21 09:50 +0200
    [PATCH/RFC v6 20/36] leds: lp55xx: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 04/36] leds: Add an internal led_set_brightness_nosleep  function Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 35/36] leds: pwm: remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 16:50 +0200
    [PATCH/RFC v6 28/36] leds: dac124d085: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 17:00 +0200
    [PATCH/RFC v6 25/36] leds: wm831x: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 17:00 +0200
    [PATCH/RFC v6 26/36] leds: da903x: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 17:00 +0200
    [PATCH/RFC v6 24/36] leds: pca963x: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 17:00 +0200
    [PATCH/RFC v6 23/36] leds: pca955x: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-08-20 17:00 +0200

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

#1210551 — [PATCH/RFC v6 13/36] leds: adp5520: Remove work queue

From: Andrew Lunn <andrew@lunn.ch>

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 <andrew@lunn.ch>
Cc: Michael Hennerich <michael.hennerich@analog.com>
Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1210552 — [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 <j.anaszewski@samsung.com>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Stas Sergeev <stsp@users.sourceforge.net>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1210612 — 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1211041 — 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1211283 — 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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1211321 — 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1211326 — 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1211898 — 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1210553 — [PATCH/RFC v6 12/36] leds: 88pm860x: Remove work queue

From: Andrew Lunn <andrew@lunn.ch>

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 <andrew@lunn.ch>
Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1210554 — [PATCH/RFC v6 21/36] leds: lp8788: Remove work queue

From: Andrew Lunn <andrew@lunn.ch>

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 <andrew@lunn.ch>
Cc: Milo Kim <milo.kim@ti.com>
Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1210555 — [PATCH/RFC v6 29/36] leds: lt3593: Remove work queue

From: Andrew Lunn <andrew@lunn.ch>

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 <andrew@lunn.ch>
Cc: Daniel Mack <daniel@caiaq.de>
Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1210556 — [PATCH/RFC v6 30/36] leds: max8997: Remove unneeded workqueue include

From: Andrew Lunn <andrew@lunn.ch>

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

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1210557 — [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 <j.anaszewski@samsung.com>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1210560 — [PATCH/RFC v6 15/36] leds: blinkm: Remove work queue

From: Andrew Lunn <andrew@lunn.ch>

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 <andrew@lunn.ch>
Cc: Jan-Simon Moeller <dl9pf@gmx.de>
Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1210561 — [PATCH/RFC v6 22/36] leds: lp8860: Remove work queue

From: Andrew Lunn <andrew@lunn.ch>

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 <andrew@lunn.ch>
Cc: Dan Murphy <dmurphy@ti.com>
Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1210562 — [PATCH/RFC v6 11/36] leds: tlc591xx: Remove work queue

From: Andrew Lunn <andrew@lunn.ch>

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 <andrew@lunn.ch>
Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1210563 — [PATCH/RFC v6 32/36] leds: regulator: Remove work queue

From: Andrew Lunn <andrew@lunn.ch>

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 <andrew@lunn.ch>
Cc: Antonio Ospite <ospite@studenti.unina.it>
Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1210979 — Re: [PATCH/RFC v6 32/36] leds: regulator: Remove work queue

On Thu, 20 Aug 2015 16:44:02 +0200
Jacek Anaszewski <j.anaszewski@samsung.com> wrote:

> From: Andrew Lunn <andrew@lunn.ch>
> 
> 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 <andrew@lunn.ch>
> Cc: Antonio Ospite <ospite@studenti.unina.it>
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>

Acked-by: Antonio Ospite <ao2@ao2.it>

> ---
>  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?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1210564 — [PATCH/RFC v6 19/36] leds: lp3944: Remove work queue

From: Andrew Lunn <andrew@lunn.ch>

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 <andrew@lunn.ch>
Cc: Antonio Ospite <ospite@studenti.unina.it>
Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1210982 — Re: [PATCH/RFC v6 19/36] leds: lp3944: Remove work queue

On Thu, 20 Aug 2015 16:43:49 +0200
Jacek Anaszewski <j.anaszewski@samsung.com> wrote:

> From: Andrew Lunn <andrew@lunn.ch>
> 
> 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 <andrew@lunn.ch>
> Cc: Antonio Ospite <ospite@studenti.unina.it>
> Signed-off-by: Jacek Anaszewski <j.anaszewski@samsung.com>
>

Acked-by: Antonio Ospite <ao2@ao2.it>

> ---
>  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?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | linux.kernel

csiph-web