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

Groups > linux.kernel > #1229435 > unrolled thread

[PATCH 0/5] LED flash: Set brightness in a sync way on demand

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] LED flash: Set brightness in a sync way on demand Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-09-21 16:40 +0200
    [PATCH 1/5] leds: core: Drivers shouldn't enforce SYNC/ASYNC  brightness setting Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-09-21 16:40 +0200
      Re: [PATCH 1/5] leds: core: Drivers shouldn't enforce SYNC/ASYNC  brightness setting Sakari Ailus <sakari.ailus@linux.intel.com> - 2015-09-22 08:50 +0200
        Re: [PATCH 1/5] leds: core: Drivers shouldn't enforce SYNC/ASYNC  brightness setting Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-09-22 09:20 +0200
      Re: [PATCH 1/5] leds: core: Drivers shouldn't enforce SYNC/ASYNC  brightness setting Andrew Lunn <andrew@lunn.ch> - 2015-09-22 21:30 +0200
    [PATCH 3/5] leds: max77693: Remove work queue Jacek Anaszewski <j.anaszewski@samsung.com> - 2015-09-21 16:40 +0200
      Re: [PATCH 3/5] leds: max77693: Remove work queue Sakari Ailus <sakari.ailus@linux.intel.com> - 2015-09-22 12:30 +0200

#1229435 — [PATCH 0/5] LED flash: Set brightness in a sync way on demand

Since it has become apparent that user should decide whether brightness
is to be set in a synchronous or an asynchronousi way, export internal
led_set_brightness_sync API and remove SET_BRIGHTNESS_SYNC and
SET_BRIGHTNESS_ASYNC flags from the LED core. After patch set [1] flash
LED drivers don't longer have to implement brightness_set op and use work
queues internally. Remove the redundant code from them.

Thanks,
Jacek Anaszewski

[1] https://lkml.org/lkml/2015/9/16/299 

Jacek Anaszewski (5):
  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

 Documentation/leds/leds-class.txt |   21 ++++++++++++++
 drivers/leds/led-class-flash.c    |    4 ---
 drivers/leds/led-class.c          |    2 --
 drivers/leds/led-core.c           |   33 ++++++++++++---------
 drivers/leds/leds-aat1290.c       |   50 +++++++++-----------------------
 drivers/leds/leds-ktd2692.c       |   41 ++++----------------------
 drivers/leds/leds-max77693.c      |   57 ++++++-------------------------------
 drivers/leds/leds.h               |   13 ---------
 include/linux/leds.h              |   19 +++++++++++--
 9 files changed, 84 insertions(+), 156 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]

#1229437 — [PATCH 1/5] 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        |   33 +++++++++++++++++++--------------
 drivers/leds/leds.h            |   13 -------------
 include/linux/leds.h           |   19 ++++++++++++++++---
 5 files changed, 35 insertions(+), 36 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 7385f98..37eec1a 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 d8649f1..d0eb838 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -215,8 +215,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.
@@ -235,19 +233,9 @@ void led_set_brightness(struct led_classdev *led_cdev,
 			led_cdev->blink_brightness = brightness;
 		}
 		return;
-	}
-
-	if (led_cdev->flags & SET_BRIGHTNESS_ASYNC) {
+	} else {
 		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);
+	}
 }
 EXPORT_SYMBOL(led_set_brightness);
 
@@ -269,6 +257,23 @@ void led_set_brightness_nosleep(struct led_classdev *led_cdev,
 }
 EXPORT_SYMBOL(led_set_brightness_nosleep);
 
+int led_set_brightness_sync(struct led_classdev *led_cdev,
+			    enum led_brightness value)
+{
+	WARN_ON(led_cdev->blink_delay_on || led_cdev->blink_delay_off);
+
+	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 -EINVAL;
+}
+EXPORT_SYMBOL(led_set_brightness_sync);
+
 int led_update_brightness(struct led_classdev *led_cdev)
 {
 	int ret = 0;
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index 04b8e41..62d9eba 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 ae3c178..2019929 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -47,9 +47,7 @@ struct led_classdev {
 #define LED_BLINK_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 */
@@ -159,6 +157,21 @@ extern void led_blink_set_oneshot(struct led_classdev *led_cdev,
  */
 extern void led_set_brightness(struct led_classdev *led_cdev,
 			       enum led_brightness brightness);
+
+/**
+ * led_set_brightness_sync - set LED brightness synchronously
+ * @led_cdev: the LED to set
+ * @brightness: the brightness to set it to
+ *
+ * Set an LED's brightness immediately. This function will block
+ * the caller for the time required for accessing device register,
+ * and it can sleep.
+ *
+ * Returns: 0 on success or negative error value on failure
+ */
+extern int led_set_brightness_sync(struct led_classdev *led_cdev,
+				   enum led_brightness value);
+
 /**
  * led_update_brightness - update LED brightness
  * @led_cdev: the LED to query
-- 
1.7.9.5

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

#1229930 — Re: [PATCH 1/5] leds: core: Drivers shouldn't enforce SYNC/ASYNC brightness setting

Hi Jacek,

Thanks for the patchset. A few comments below.

Jacek Anaszewski wrote:
> 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        |   33 +++++++++++++++++++--------------
>  drivers/leds/leds.h            |   13 -------------
>  include/linux/leds.h           |   19 ++++++++++++++++---
>  5 files changed, 35 insertions(+), 36 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 7385f98..37eec1a 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 d8649f1..d0eb838 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -215,8 +215,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.
> @@ -235,19 +233,9 @@ void led_set_brightness(struct led_classdev *led_cdev,
>  			led_cdev->blink_brightness = brightness;
>  		}
>  		return;
> -	}
> -
> -	if (led_cdev->flags & SET_BRIGHTNESS_ASYNC) {
> +	} else {

Useless use of else.

>  		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);
> +	}
>  }
>  EXPORT_SYMBOL(led_set_brightness);
>  
> @@ -269,6 +257,23 @@ void led_set_brightness_nosleep(struct led_classdev *led_cdev,
>  }
>  EXPORT_SYMBOL(led_set_brightness_nosleep);
>  
> +int led_set_brightness_sync(struct led_classdev *led_cdev,
> +			    enum led_brightness value)
> +{
> +	WARN_ON(led_cdev->blink_delay_on || led_cdev->blink_delay_off);

How about simply returning an error instead?

> +
> +	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 -EINVAL;
> +}
> +EXPORT_SYMBOL(led_set_brightness_sync);
> +
>  int led_update_brightness(struct led_classdev *led_cdev)
>  {
>  	int ret = 0;
> diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
> index 04b8e41..62d9eba 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 ae3c178..2019929 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -47,9 +47,7 @@ struct led_classdev {
>  #define LED_BLINK_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 */
> @@ -159,6 +157,21 @@ extern void led_blink_set_oneshot(struct led_classdev *led_cdev,
>   */
>  extern void led_set_brightness(struct led_classdev *led_cdev,
>  			       enum led_brightness brightness);
> +
> +/**
> + * led_set_brightness_sync - set LED brightness synchronously
> + * @led_cdev: the LED to set
> + * @brightness: the brightness to set it to
> + *
> + * Set an LED's brightness immediately. This function will block
> + * the caller for the time required for accessing device register,
> + * and it can sleep.
> + *
> + * Returns: 0 on success or negative error value on failure
> + */
> +extern int led_set_brightness_sync(struct led_classdev *led_cdev,
> +				   enum led_brightness value);
> +
>  /**
>   * led_update_brightness - update LED brightness
>   * @led_cdev: the LED to query
> 

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com
--
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]

#1229937 — Re: [PATCH 1/5] leds: core: Drivers shouldn't enforce SYNC/ASYNC brightness setting

Hi Sakari,

Thanks for the review.
Just to avoid a confusion - this patch set depends on [1], which is
not merged yet and also needs a review. I reimplemented and split [2]
into two patch sets, so that [1] contained only modifications required
for removing work queues from LED class drivers, but yesterday decided
to send also the remaining part.

[1] https://lkml.org/lkml/2015/9/16/299
[2] https://lkml.org/lkml/2015/8/20/426

On 09/22/2015 08:42 AM, Sakari Ailus wrote:
> Hi Jacek,
>
> Thanks for the patchset. A few comments below.
>
> Jacek Anaszewski wrote:
>> 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        |   33 +++++++++++++++++++--------------
>>   drivers/leds/leds.h            |   13 -------------
>>   include/linux/leds.h           |   19 ++++++++++++++++---
>>   5 files changed, 35 insertions(+), 36 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 7385f98..37eec1a 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 d8649f1..d0eb838 100644
>> --- a/drivers/leds/led-core.c
>> +++ b/drivers/leds/led-core.c
>> @@ -215,8 +215,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.
>> @@ -235,19 +233,9 @@ void led_set_brightness(struct led_classdev *led_cdev,
>>   			led_cdev->blink_brightness = brightness;
>>   		}
>>   		return;
>> -	}
>> -
>> -	if (led_cdev->flags & SET_BRIGHTNESS_ASYNC) {
>> +	} else {
>
> Useless use of else.

Right.

>>   		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);
>> +	}
>>   }
>>   EXPORT_SYMBOL(led_set_brightness);
>>
>> @@ -269,6 +257,23 @@ void led_set_brightness_nosleep(struct led_classdev *led_cdev,
>>   }
>>   EXPORT_SYMBOL(led_set_brightness_nosleep);
>>
>> +int led_set_brightness_sync(struct led_classdev *led_cdev,
>> +			    enum led_brightness value)
>> +{
>> +	WARN_ON(led_cdev->blink_delay_on || led_cdev->blink_delay_off);
>
> How about simply returning an error instead?

Indeed, it will be more relevant here.

>> +
>> +	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 -EINVAL;
>> +}
>> +EXPORT_SYMBOL(led_set_brightness_sync);
>> +
>>   int led_update_brightness(struct led_classdev *led_cdev)
>>   {
>>   	int ret = 0;
>> diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
>> index 04b8e41..62d9eba 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 ae3c178..2019929 100644
>> --- a/include/linux/leds.h
>> +++ b/include/linux/leds.h
>> @@ -47,9 +47,7 @@ struct led_classdev {
>>   #define LED_BLINK_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 */
>> @@ -159,6 +157,21 @@ extern void led_blink_set_oneshot(struct led_classdev *led_cdev,
>>    */
>>   extern void led_set_brightness(struct led_classdev *led_cdev,
>>   			       enum led_brightness brightness);
>> +
>> +/**
>> + * led_set_brightness_sync - set LED brightness synchronously
>> + * @led_cdev: the LED to set
>> + * @brightness: the brightness to set it to
>> + *
>> + * Set an LED's brightness immediately. This function will block
>> + * the caller for the time required for accessing device register,
>> + * and it can sleep.
>> + *
>> + * Returns: 0 on success or negative error value on failure
>> + */
>> +extern int led_set_brightness_sync(struct led_classdev *led_cdev,
>> +				   enum led_brightness value);
>> +
>>   /**
>>    * led_update_brightness - update LED brightness
>>    * @led_cdev: the LED to query
>>
>


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

#1230852 — Re: [PATCH 1/5] leds: core: Drivers shouldn't enforce SYNC/ASYNC brightness setting

> +int led_set_brightness_sync(struct led_classdev *led_cdev,
> +			    enum led_brightness value)
> +{
> +	WARN_ON(led_cdev->blink_delay_on || led_cdev->blink_delay_off);
> +
> +	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 -EINVAL;

Maybe -ENOTSUP would be better. It is not an invalid argument, the
driver does not support it.


> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index ae3c178..2019929 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -47,9 +47,7 @@ struct led_classdev {
>  #define LED_BLINK_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 */
> @@ -159,6 +157,21 @@ extern void led_blink_set_oneshot(struct led_classdev *led_cdev,
>   */
>  extern void led_set_brightness(struct led_classdev *led_cdev,
>  			       enum led_brightness brightness);
> +
> +/**
> + * led_set_brightness_sync - set LED brightness synchronously
> + * @led_cdev: the LED to set
> + * @brightness: the brightness to set it to
> + *
> + * Set an LED's brightness immediately. This function will block
> + * the caller for the time required for accessing device register,

registers - plural.

	  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]

#1229439 — [PATCH 3/5] 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>
---
 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 f69bcb4..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,43 +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 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(
@@ -930,16 +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_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);
 
@@ -1061,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]

#1230058 — Re: [PATCH 3/5] leds: max77693: Remove work queue

Jacek Anaszewski wrote:
> 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>

Nice patches! For patches 3--5:

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

-- 
Sakari Ailus
sakari.ailus@linux.intel.com
--
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