[PATCH v4 0/3] leds: Add control of the voltage/current regulator to the LED core

[PATCH v4 0/3] leds: Add control of the voltage/current regulator to the LED core

[Jean-Jacques Hiblot]

This series makes it possible for the LED core to manage the power supply
of a LED. It uses the regulator API to disable/enable the power if when the
LED is turned on/off.
This is especially useful in situations where the LED driver/controller is
not supplying the power.
Because updating a regulator state can block, it is always defered to
set_brightness_delayed().

changes in v4:
- Add a new patch to make led_set_brightness_sync() use
  led_set_brightness_nosleep() and then wait the work to be done
- Rework how the core knows how the regulator needs to be updated.

changes in v3:
- reword device-tree description
- reword commit log
- remove regulator updates from functions used in atomic context. If the
  regulator must be updated, it is defered to a workqueue.
- Fix led_set_brightness_sync() to work with the non-blocking function
  __led_set_brightness()

changes in v2:
- use devm_regulator_get_optional() to avoid using the dummy regulator and
  do some unnecessary work

Jean-Jacques Hiblot (3):
  led: make led_set_brightness_sync() use led_set_brightness_nosleep()
  dt-bindings: leds: document the "power-supply" property
  leds: Add control of the voltage/current regulator to the LED core

 .../devicetree/bindings/leds/common.txt       |  6 ++
 drivers/leds/led-class.c                      | 17 ++++
 drivers/leds/led-core.c                       | 77 +++++++++++++++++--
 drivers/leds/leds.h                           |  3 +
 include/linux/leds.h                          |  5 ++
 5 files changed, 101 insertions(+), 7 deletions(-)

-- 
2.17.1

[PATCH v4 1/3] led: make led_set_brightness_sync() use led_set_brightness_nosleep()

[Jean-Jacques Hiblot]

Making led_set_brightness_sync() use led_set_brightness_nosleep() has 2
advantages:
- works for LED controllers that do not provide brightness_set_blocking()
- When the blocking callback is used, it uses the workqueue to update the
  LED state, removing the need for mutual exclusion between
  led_set_brightness_sync() and set_brightness_delayed().

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---
 drivers/leds/led-core.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index f1f718dbe0f8..50e28a8f9357 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -294,15 +294,17 @@ EXPORT_SYMBOL_GPL(led_set_brightness_nosleep);
 int led_set_brightness_sync(struct led_classdev *led_cdev,
 			    enum led_brightness value)
 {
+	int ret;
+
 	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;
+	ret = led_set_brightness_nosleep(led_cdev, value);
+	if (!ret)
+		return ret;
 
-	return __led_set_brightness_blocking(led_cdev, led_cdev->brightness);
+	flush_work(&led_cdev->set_brightness_work);
+	return 0;
 }
 EXPORT_SYMBOL_GPL(led_set_brightness_sync);
 
-- 
2.17.1

[PATCH v4 2/3] dt-bindings: leds: document the "power-supply" property

[Jean-Jacques Hiblot]

Most of the LEDs are powered by a voltage/current regulator. Describing it
in the device-tree makes it possible for the LED core to enable/disable it
when needed.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---
 Documentation/devicetree/bindings/leds/common.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/common.txt b/Documentation/devicetree/bindings/leds/common.txt
index 9fa6f9795d50..54857c16573d 100644
--- a/Documentation/devicetree/bindings/leds/common.txt
+++ b/Documentation/devicetree/bindings/leds/common.txt
@@ -77,6 +77,11 @@ Optional properties for child nodes:
 - panic-indicator : This property specifies that the LED should be used,
 		    if at all possible, as a panic indicator.
 
+- power-supply : A voltage/current regulator used to to power the LED. When a
+		 LED is turned off, the LED core disable its regulator. The
+		 same regulator can power many LED (or other) devices. It is
+		 turned off only when all of its users disabled it.
+
 - trigger-sources : List of devices which should be used as a source triggering
 		    this LED activity. Some LEDs can be related to a specific
 		    device and should somehow indicate its state. E.g. USB 2.0
@@ -124,6 +129,7 @@ led-controller@0 {
 		function = LED_FUNCTION_STATUS;
 		linux,default-trigger = "heartbeat";
 		gpios = <&gpio0 0 GPIO_ACTIVE_HIGH>;
+		power-supply = <&led_regulator>;
 	};
 
 	led1 {
-- 
2.17.1

[PATCH v4 3/3] leds: Add control of the voltage/current regulator to the LED core

[Jean-Jacques Hiblot]

A LED is usually powered by a voltage/current regulator. Let the LED core
know about it. This allows the LED core to turn on or off the power supply
as needed.

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
---
 drivers/leds/led-class.c | 17 +++++++++++
 drivers/leds/led-core.c  | 65 ++++++++++++++++++++++++++++++++++++++--
 drivers/leds/leds.h      |  3 ++
 include/linux/leds.h     |  5 ++++
 4 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index e11177d77b4c..d122b6982efd 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -352,6 +352,7 @@ int led_classdev_register_ext(struct device *parent,
 	char final_name[LED_MAX_NAME_SIZE];
 	const char *proposed_name = composed_name;
 	int ret;
+	struct regulator *regulator;
 
 	if (init_data) {
 		if (init_data->devname_mandatory && !init_data->devicename) {
@@ -387,6 +388,22 @@ int led_classdev_register_ext(struct device *parent,
 		dev_warn(parent, "Led %s renamed to %s due to name collision",
 				led_cdev->name, dev_name(led_cdev->dev));
 
+	regulator = devm_regulator_get_optional(led_cdev->dev, "power");
+	if (IS_ERR(regulator)) {
+		if (regulator != ERR_PTR(-ENODEV)) {
+			dev_err(led_cdev->dev, "Cannot get the power supply for %s\n",
+				led_cdev->name);
+			device_unregister(led_cdev->dev);
+			mutex_unlock(&led_cdev->led_access);
+			return PTR_ERR(regulator);
+		}
+		led_cdev->regulator = NULL;
+	} else {
+		led_cdev->regulator = regulator;
+		led_cdev->regulator_state = REG_OFF;
+		atomic_set(&led_cdev->target_regulator_state, REG_UNKNOWN);
+	}
+
 	if (led_cdev->flags & LED_BRIGHT_HW_CHANGED) {
 		ret = led_add_brightness_hw_changed(led_cdev);
 		if (ret) {
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 50e28a8f9357..287a8a6aad02 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -37,6 +37,43 @@ const char * const led_colors[LED_COLOR_ID_MAX] = {
 };
 EXPORT_SYMBOL_GPL(led_colors);
 
+static int __led_handle_regulator(struct led_classdev *led_cdev)
+{
+	int rc;
+	int target_state = led_cdev->delayed_set_value ?  REG_ON : REG_OFF;
+
+	if (!led_cdev->regulator)
+		return 0;
+
+	/*
+	 * if the current regulator state is not the target state, we
+	 * need to update it.
+	 * note: No need for spinlock or atomic here because
+	 * led_cdev->regulator_state is modified only in the context of
+	 * the worqueue
+	 */
+	if (led_cdev->regulator_state != target_state) {
+
+		if (target_state == REG_ON)
+			rc = regulator_enable(led_cdev->regulator);
+		else
+			rc = regulator_disable(led_cdev->regulator);
+		if (rc) {
+			/*
+			 * If something went wrong with the regulator update,
+			 * Make sure that led_set_brightness_nosleep() assume
+			 * that the regultor is in the right state.
+			 */
+			atomic_set(&led_cdev->target_regulator_state,
+				   REG_UNKNOWN);
+			return rc;
+		}
+
+		led_cdev->regulator_state = target_state;
+	}
+	return 0;
+}
+
 static int __led_set_brightness(struct led_classdev *led_cdev,
 				enum led_brightness value)
 {
@@ -135,6 +172,11 @@ static void set_brightness_delayed(struct work_struct *ws)
 	    (led_cdev->flags & LED_HW_PLUGGABLE)))
 		dev_err(led_cdev->dev,
 			"Setting an LED's brightness failed (%d)\n", ret);
+
+	ret = __led_handle_regulator(led_cdev);
+	if (ret)
+		dev_err(led_cdev->dev,
+			"Updating regulator state failed (%d)\n", ret);
 }
 
 static void led_set_software_blink(struct led_classdev *led_cdev,
@@ -269,8 +311,27 @@ EXPORT_SYMBOL_GPL(led_set_brightness);
 void led_set_brightness_nopm(struct led_classdev *led_cdev,
 			      enum led_brightness value)
 {
-	/* Use brightness_set op if available, it is guaranteed not to sleep */
-	if (!__led_set_brightness(led_cdev, value))
+	bool update_regulator = false;
+	int old, new;
+
+	if (led_cdev->regulator) {
+		/*
+		 * Check if the regulator need to be updated.
+		 * We use an atomic here because multiple threads could
+		 * be calling this function at the same time. Using
+		 * atomic_xchg() ensures the consistency between
+		 * target_regulator_state, value and update_regulator
+		 */
+		new = !!value;
+		old = atomic_xchg(&led_cdev->target_regulator_state, new);
+		update_regulator = (old != new);
+	}
+
+	/*
+	 * If regulator state doesn't need to be changed, use brightness_set
+	 * op if available, it is guaranteed not to sleep
+	 */
+	if (!update_regulator && !__led_set_brightness(led_cdev, value))
 		return;
 
 	/* If brightness setting can sleep, delegate it to a work queue task */
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index 0b577cece8f7..02f261ce77f2 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -11,6 +11,9 @@
 
 #include <linux/rwsem.h>
 #include <linux/leds.h>
+#include <linux/regulator/consumer.h>
+
+enum { REG_OFF = 0, REG_ON, REG_UNKNOWN };
 
 static inline int led_get_brightness(struct led_classdev *led_cdev)
 {
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 88bf2ceaabe6..8ce7cf937192 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -149,6 +149,11 @@ struct led_classdev {
 
 	/* Ensures consistent access to the LED Flash Class device */
 	struct mutex		led_access;
+
+	/* regulator */
+	struct regulator	*regulator;
+	int			regulator_state;
+	atomic_t		target_regulator_state;
 };
 
 /**
-- 
2.17.1

Re: [PATCH v4 1/3] led: make led_set_brightness_sync() use led_set_brightness_nosleep()

[Jacek Anaszewski]

Hi Jean,

On 9/20/19 2:25 PM, Jean-Jacques Hiblot wrote:
> Making led_set_brightness_sync() use led_set_brightness_nosleep() has 2
> advantages:
> - works for LED controllers that do not provide brightness_set_blocking()
> - When the blocking callback is used, it uses the workqueue to update the
>   LED state, removing the need for mutual exclusion between
>   led_set_brightness_sync() and set_brightness_delayed().

And third:

- it compromises the "sync" part of the function name :-)

This function has been introduced specifically to be blocking
and have the immediate effect. Its sole client is
drivers/media/v4l2-core/v4l2-flash-led-class.c.

> 
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
> ---
>  drivers/leds/led-core.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index f1f718dbe0f8..50e28a8f9357 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -294,15 +294,17 @@ EXPORT_SYMBOL_GPL(led_set_brightness_nosleep);
>  int led_set_brightness_sync(struct led_classdev *led_cdev,
>  			    enum led_brightness value)
>  {
> +	int ret;
> +
>  	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;
> +	ret = led_set_brightness_nosleep(led_cdev, value);
> +	if (!ret)
> +		return ret;
>  
> -	return __led_set_brightness_blocking(led_cdev, led_cdev->brightness);
> +	flush_work(&led_cdev->set_brightness_work);
> +	return 0;
>  }
>  EXPORT_SYMBOL_GPL(led_set_brightness_sync);
>  
> 

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v4 1/3] led: make led_set_brightness_sync() use led_set_brightness_nosleep()

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3017 bytes --]

Hi Jean-Jacques,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3 next-20190919]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Jean-Jacques-Hiblot/leds-Add-control-of-the-voltage-current-regulator-to-the-LED-core/20190920-220416
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=sh 
:::::: branch date: 2 hours ago
:::::: commit date: 2 hours ago

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers//leds/led-core.c: In function 'led_set_brightness_sync':
>> drivers//leds/led-core.c:302:6: error: void value not ignored as it ought to be
     ret = led_set_brightness_nosleep(led_cdev, value);
         ^

# https://github.com/0day-ci/linux/commit/54301e6f4e910f292045a1afa62ef732791e1bb5
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 54301e6f4e910f292045a1afa62ef732791e1bb5
vim +302 drivers//leds/led-core.c

81fe8e5b73e3f4 Jacek Anaszewski    2015-10-07  293  
13ae79bbe4c214 Jacek Anaszewski    2015-10-07  294  int led_set_brightness_sync(struct led_classdev *led_cdev,
13ae79bbe4c214 Jacek Anaszewski    2015-10-07  295  			    enum led_brightness value)
13ae79bbe4c214 Jacek Anaszewski    2015-10-07  296  {
54301e6f4e910f Jean-Jacques Hiblot 2019-09-20  297  	int ret;
54301e6f4e910f Jean-Jacques Hiblot 2019-09-20  298  
13ae79bbe4c214 Jacek Anaszewski    2015-10-07  299  	if (led_cdev->blink_delay_on || led_cdev->blink_delay_off)
13ae79bbe4c214 Jacek Anaszewski    2015-10-07  300  		return -EBUSY;
13ae79bbe4c214 Jacek Anaszewski    2015-10-07  301  
54301e6f4e910f Jean-Jacques Hiblot 2019-09-20 @302  	ret = led_set_brightness_nosleep(led_cdev, value);
54301e6f4e910f Jean-Jacques Hiblot 2019-09-20  303  	if (!ret)
54301e6f4e910f Jean-Jacques Hiblot 2019-09-20  304  		return ret;
13ae79bbe4c214 Jacek Anaszewski    2015-10-07  305  
54301e6f4e910f Jean-Jacques Hiblot 2019-09-20  306  	flush_work(&led_cdev->set_brightness_work);
13ae79bbe4c214 Jacek Anaszewski    2015-10-07  307  	return 0;
13ae79bbe4c214 Jacek Anaszewski    2015-10-07  308  }
13ae79bbe4c214 Jacek Anaszewski    2015-10-07  309  EXPORT_SYMBOL_GPL(led_set_brightness_sync);
13ae79bbe4c214 Jacek Anaszewski    2015-10-07  310  

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 52132 bytes --]

Re: [PATCH v4 1/3] led: make led_set_brightness_sync() use led_set_brightness_nosleep()

[Jean-Jacques Hiblot]

Hi Jacek,

On 20/09/2019 23:10, Jacek Anaszewski wrote:
> Hi Jean,
>
> On 9/20/19 2:25 PM, Jean-Jacques Hiblot wrote:
>> Making led_set_brightness_sync() use led_set_brightness_nosleep() has 2
>> advantages:
>> - works for LED controllers that do not provide brightness_set_blocking()
>> - When the blocking callback is used, it uses the workqueue to update the
>>    LED state, removing the need for mutual exclusion between
>>    led_set_brightness_sync() and set_brightness_delayed().
> And third:
>
> - it compromises the "sync" part of the function name :-)

Making it sync is the role of the flush_work() function. It waits until 
the deferred work has been done.

JJ

> This function has been introduced specifically to be blocking
> and have the immediate effect. Its sole client is
> drivers/media/v4l2-core/v4l2-flash-led-class.c.
>
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
>> ---
>>   drivers/leds/led-core.c | 12 +++++++-----
>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
>> index f1f718dbe0f8..50e28a8f9357 100644
>> --- a/drivers/leds/led-core.c
>> +++ b/drivers/leds/led-core.c
>> @@ -294,15 +294,17 @@ EXPORT_SYMBOL_GPL(led_set_brightness_nosleep);
>>   int led_set_brightness_sync(struct led_classdev *led_cdev,
>>   			    enum led_brightness value)
>>   {
>> +	int ret;
>> +
>>   	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;
>> +	ret = led_set_brightness_nosleep(led_cdev, value);
>> +	if (!ret)
>> +		return ret;
>>   
>> -	return __led_set_brightness_blocking(led_cdev, led_cdev->brightness);
>> +	flush_work(&led_cdev->set_brightness_work);
>> +	return 0;
>>   }
>>   EXPORT_SYMBOL_GPL(led_set_brightness_sync);
>>   
>>

Re: [PATCH v4 1/3] led: make led_set_brightness_sync() use led_set_brightness_nosleep()

[Jacek Anaszewski]

Hi Jean,

On 9/23/19 11:14 AM, Jean-Jacques Hiblot wrote:
> Hi Jacek,
> 
> On 20/09/2019 23:10, Jacek Anaszewski wrote:
>> Hi Jean,
>>
>> On 9/20/19 2:25 PM, Jean-Jacques Hiblot wrote:
>>> Making led_set_brightness_sync() use led_set_brightness_nosleep() has 2
>>> advantages:
>>> - works for LED controllers that do not provide
>>> brightness_set_blocking()
>>> - When the blocking callback is used, it uses the workqueue to update
>>> the
>>>    LED state, removing the need for mutual exclusion between
>>>    led_set_brightness_sync() and set_brightness_delayed().
>> And third:
>>
>> - it compromises the "sync" part of the function name :-)
> 
> Making it sync is the role of the flush_work() function. It waits until
> the deferred work has been done.

The thing is not in the blocking character of the function, but rather
in the fastest possible way of setting torch brightness.
led_set_brightness_nosleep() will defer brightness_set_blocking op
to the workqueue so this condition will not be met then.

This function was added specifically for LED class flash v4l2 wrapper:
drivers/media/v4l2-core/v4l2-flash-led-class.c.

It may need an addition of support for brightness_set only drivers,
but we haven't had a use case so far, since all client flash LED
controllers are driven via blocking buses (there are not many of them).

Also, when LED flash class (and thus LED class also as a parent)
is hijacked by v4l2-flash-led wrapper, its sysfs is disabled,
so it is not possible to set e.g. timer trigger which could
interfere with the led_set_brightness_sync() (and it also returns
-EBUSY when blinking is enabled).

>> This function has been introduced specifically to be blocking
>> and have the immediate effect. Its sole client is
>> drivers/media/v4l2-core/v4l2-flash-led-class.c.
>>
>>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
>>> ---
>>>   drivers/leds/led-core.c | 12 +++++++-----
>>>   1 file changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
>>> index f1f718dbe0f8..50e28a8f9357 100644
>>> --- a/drivers/leds/led-core.c
>>> +++ b/drivers/leds/led-core.c
>>> @@ -294,15 +294,17 @@ EXPORT_SYMBOL_GPL(led_set_brightness_nosleep);
>>>   int led_set_brightness_sync(struct led_classdev *led_cdev,
>>>                   enum led_brightness value)
>>>   {
>>> +    int ret;
>>> +
>>>       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;
>>> +    ret = led_set_brightness_nosleep(led_cdev, value);
>>> +    if (!ret)
>>> +        return ret;
>>>   -    return __led_set_brightness_blocking(led_cdev,
>>> led_cdev->brightness);
>>> +    flush_work(&led_cdev->set_brightness_work);
>>> +    return 0;
>>>   }
>>>   EXPORT_SYMBOL_GPL(led_set_brightness_sync);
>>>  
> 

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v4 1/3] led: make led_set_brightness_sync() use led_set_brightness_nosleep()

[Jean-Jacques Hiblot]

On 23/09/2019 23:03, Jacek Anaszewski wrote:
> Hi Jean,
>
> On 9/23/19 11:14 AM, Jean-Jacques Hiblot wrote:
>> Hi Jacek,
>>
>> On 20/09/2019 23:10, Jacek Anaszewski wrote:
>>> Hi Jean,
>>>
>>> On 9/20/19 2:25 PM, Jean-Jacques Hiblot wrote:
>>>> Making led_set_brightness_sync() use led_set_brightness_nosleep() has 2
>>>> advantages:
>>>> - works for LED controllers that do not provide
>>>> brightness_set_blocking()
>>>> - When the blocking callback is used, it uses the workqueue to update
>>>> the
>>>>     LED state, removing the need for mutual exclusion between
>>>>     led_set_brightness_sync() and set_brightness_delayed().
>>> And third:
>>>
>>> - it compromises the "sync" part of the function name :-)
>> Making it sync is the role of the flush_work() function. It waits until
>> the deferred work has been done.
> The thing is not in the blocking character of the function, but rather
> in the fastest possible way of setting torch brightness.
> led_set_brightness_nosleep() will defer brightness_set_blocking op
> to the workqueue so this condition will not be met then.

OK. I see the point there.

>
> This function was added specifically for LED class flash v4l2 wrapper:
> drivers/media/v4l2-core/v4l2-flash-led-class.c.
>
> It may need an addition of support for brightness_set only drivers,
> but we haven't had a use case so far, since all client flash LED
> controllers are driven via blocking buses (there are not many of them).
>
> Also, when LED flash class (and thus LED class also as a parent)
> is hijacked by v4l2-flash-led wrapper, its sysfs is disabled,
> so it is not possible to set e.g. timer trigger which could
> interfere with the led_set_brightness_sync() (and it also returns
> -EBUSY when blinking is enabled).

Then this is a really special use case and we don't really have to  
worry about synchronization with the other ways to set the LED 
brightness. I'll drop any change to this function then.

Thanks for the detailed explanation.

JJ


>
>>> This function has been introduced specifically to be blocking
>>> and have the immediate effect. Its sole client is
>>> drivers/media/v4l2-core/v4l2-flash-led-class.c.
>>>
>>>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@ti.com>
>>>> ---
>>>>    drivers/leds/led-core.c | 12 +++++++-----
>>>>    1 file changed, 7 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
>>>> index f1f718dbe0f8..50e28a8f9357 100644
>>>> --- a/drivers/leds/led-core.c
>>>> +++ b/drivers/leds/led-core.c
>>>> @@ -294,15 +294,17 @@ EXPORT_SYMBOL_GPL(led_set_brightness_nosleep);
>>>>    int led_set_brightness_sync(struct led_classdev *led_cdev,
>>>>                    enum led_brightness value)
>>>>    {
>>>> +    int ret;
>>>> +
>>>>        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;
>>>> +    ret = led_set_brightness_nosleep(led_cdev, value);
>>>> +    if (!ret)
>>>> +        return ret;
>>>>    -    return __led_set_brightness_blocking(led_cdev,
>>>> led_cdev->brightness);
>>>> +    flush_work(&led_cdev->set_brightness_work);
>>>> +    return 0;
>>>>    }
>>>>    EXPORT_SYMBOL_GPL(led_set_brightness_sync);
>>>>