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

Groups > linux.kernel > #1241259 > unrolled thread

[PATCH v3 00/10] LED core improvements

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 00/10] LED core improvements Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-10-07 11:20 +0200
    [PATCH v3 10/10] media: flash: use led_set_brightness_sync for torch  brightness Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-10-07 11:20 +0200
      Re: [PATCH v3 10/10] media: flash: use led_set_brightness_sync for  torch brightness Pavel Machek <pavel@ucw.cz> - 2015-10-07 12:50 +0200
    [PATCH v3 04/10] leds: core: Use set_brightness_work for the blocking  op Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-10-07 11:20 +0200
      Re: [PATCH v3 04/10] leds: core: Use set_brightness_work for the  blocking op Sakari Ailus <sakari.ailus@iki.fi> - 2015-10-16 17:50 +0200
    [PATCH v3 02/10] leds: Rename brightness_set_sync op to  brightness_set_blocking Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-10-07 11:20 +0200
      Re: [PATCH v3 02/10] leds: Rename brightness_set_sync op to  brightness_set_blocking Sakari Ailus <sakari.ailus@iki.fi> - 2015-10-16 17:40 +0200
    [PATCH v3 05/10] leds: core: Drivers shouldn't enforce SYNC/ASYNC  brightness setting Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-10-07 11:20 +0200
    [PATCH v3 08/10] leds: aat1290: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-10-07 11:20 +0200
    [PATCH v3 07/10] leds: max77693: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-10-07 11:20 +0200
    [PATCH v3 09/10] leds: ktd2692: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-10-07 11:20 +0200

#1241259 — [PATCH v3 00/10] LED core improvements

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

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

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


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

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

Thanks,
Jacek Anaszewski

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

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

 Documentation/leds/leds-class.txt              |   13 +++
 drivers/leds/led-class-flash.c                 |    6 +-
 drivers/leds/led-class.c                       |    6 +-
 drivers/leds/led-core.c                        |  116 ++++++++++++++++++------
 drivers/leds/leds-aat1290.c                    |   50 +++-------
 drivers/leds/leds-ktd2692.c                    |   41 ++-------
 drivers/leds/leds-max77693.c                   |   57 ++----------
 drivers/leds/leds.h                            |   27 +-----
 drivers/leds/trigger/ledtrig-backlight.c       |    8 +-
 drivers/leds/trigger/ledtrig-default-on.c      |    2 +-
 drivers/leds/trigger/ledtrig-gpio.c            |    6 +-
 drivers/leds/trigger/ledtrig-heartbeat.c       |    4 +-
 drivers/leds/trigger/ledtrig-oneshot.c         |    4 +-
 drivers/leds/trigger/ledtrig-transient.c       |    8 +-
 drivers/media/v4l2-core/v4l2-flash-led-class.c |    8 +-
 include/linux/leds.h                           |   27 ++++--
 16 files changed, 177 insertions(+), 206 deletions(-)

-- 
1.7.9.5

--
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] | [next] | [standalone]

#1241262 — [PATCH v3 10/10] media: flash: use led_set_brightness_sync for torch brightness

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

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

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

--
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]

#1241350 — Re: [PATCH v3 10/10] media: flash: use led_set_brightness_sync for torch brightness

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

7-10 of the series:

Acked-by: Pavel Machek <pavel@ucw.cz>
--
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]

#1241263 — [PATCH v3 04/10] leds: core: Use set_brightness_work for the blocking op

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

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

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

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

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

--
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]

#1248921 — Re: [PATCH v3 04/10] leds: core: Use set_brightness_work for the blocking op

Hi Jacek,

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

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

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

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk
--
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]

#1241266 — [PATCH v3 02/10] leds: Rename brightness_set_sync op to brightness_set_blocking

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

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

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

--
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]

#1248908 — Re: [PATCH v3 02/10] leds: Rename brightness_set_sync op to brightness_set_blocking

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

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

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk
--
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]

#1241267 — [PATCH v3 05/10] leds: core: Drivers shouldn't enforce SYNC/ASYNC brightness setting

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

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

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

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

--
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]

#1241269 — [PATCH v3 08/10] 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 <j.anaszewski@samsung.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/leds/leds-aat1290.c |   50 +++++++++++--------------------------------
 1 file changed, 13 insertions(+), 37 deletions(-)

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

--
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]

#1241273 — [PATCH v3 07/10] 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>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/leds/leds-max77693.c |   57 +++++++-----------------------------------
 1 file changed, 9 insertions(+), 48 deletions(-)

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

--
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]

#1241274 — [PATCH v3 09/10] 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 <j.anaszewski@samsung.com>
Cc: Ingi Kim <ingi2.kim@samsung.com>
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/leds/leds-ktd2692.c |   41 ++++++-----------------------------------
 1 file changed, 6 insertions(+), 35 deletions(-)

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

--
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] | [standalone]

Back to top | Article view | linux.kernel

csiph-web