* [PATCH v4 0/3] leds: Add control of the voltage/current regulator to the LED core @ 2019-09-20 12:25 Jean-Jacques Hiblot 2019-09-20 12:25 ` [PATCH v4 1/3] led: make led_set_brightness_sync() use led_set_brightness_nosleep() Jean-Jacques Hiblot ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Jean-Jacques Hiblot @ 2019-09-20 12:25 UTC (permalink / raw) To: jacek.anaszewski, pavel, daniel.thompson Cc: linux-leds, linux-kernel, dmurphy, tomi.valkeinen, 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 1/3] led: make led_set_brightness_sync() use led_set_brightness_nosleep() 2019-09-20 12:25 [PATCH v4 0/3] leds: Add control of the voltage/current regulator to the LED core Jean-Jacques Hiblot @ 2019-09-20 12:25 ` Jean-Jacques Hiblot 2019-09-20 21:10 ` Jacek Anaszewski 2019-09-21 16:13 ` kbuild test robot 2019-09-20 12:25 ` [PATCH v4 2/3] dt-bindings: leds: document the "power-supply" property Jean-Jacques Hiblot 2019-09-20 12:25 ` [PATCH v4 3/3] leds: Add control of the voltage/current regulator to the LED core Jean-Jacques Hiblot 2 siblings, 2 replies; 9+ messages in thread From: Jean-Jacques Hiblot @ 2019-09-20 12:25 UTC (permalink / raw) To: jacek.anaszewski, pavel, daniel.thompson Cc: linux-leds, linux-kernel, dmurphy, tomi.valkeinen, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/3] led: make led_set_brightness_sync() use led_set_brightness_nosleep() 2019-09-20 12:25 ` [PATCH v4 1/3] led: make led_set_brightness_sync() use led_set_brightness_nosleep() Jean-Jacques Hiblot @ 2019-09-20 21:10 ` Jacek Anaszewski 2019-09-23 9:14 ` Jean-Jacques Hiblot 2019-09-21 16:13 ` kbuild test robot 1 sibling, 1 reply; 9+ messages in thread From: Jacek Anaszewski @ 2019-09-20 21:10 UTC (permalink / raw) To: Jean-Jacques Hiblot, pavel, daniel.thompson Cc: linux-leds, linux-kernel, dmurphy, tomi.valkeinen 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/3] led: make led_set_brightness_sync() use led_set_brightness_nosleep() 2019-09-20 21:10 ` Jacek Anaszewski @ 2019-09-23 9:14 ` Jean-Jacques Hiblot 2019-09-23 21:03 ` Jacek Anaszewski 0 siblings, 1 reply; 9+ messages in thread From: Jean-Jacques Hiblot @ 2019-09-23 9:14 UTC (permalink / raw) To: Jacek Anaszewski, pavel, daniel.thompson Cc: linux-leds, linux-kernel, dmurphy, tomi.valkeinen 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); >> >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/3] led: make led_set_brightness_sync() use led_set_brightness_nosleep() 2019-09-23 9:14 ` Jean-Jacques Hiblot @ 2019-09-23 21:03 ` Jacek Anaszewski 2019-09-24 13:43 ` Jean-Jacques Hiblot 0 siblings, 1 reply; 9+ messages in thread From: Jacek Anaszewski @ 2019-09-23 21:03 UTC (permalink / raw) To: Jean-Jacques Hiblot, pavel, daniel.thompson Cc: linux-leds, linux-kernel, dmurphy, tomi.valkeinen 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/3] led: make led_set_brightness_sync() use led_set_brightness_nosleep() 2019-09-23 21:03 ` Jacek Anaszewski @ 2019-09-24 13:43 ` Jean-Jacques Hiblot 0 siblings, 0 replies; 9+ messages in thread From: Jean-Jacques Hiblot @ 2019-09-24 13:43 UTC (permalink / raw) To: Jacek Anaszewski, pavel, daniel.thompson Cc: linux-leds, linux-kernel, dmurphy, tomi.valkeinen 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); >>>> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v4 1/3] led: make led_set_brightness_sync() use led_set_brightness_nosleep() 2019-09-20 12:25 ` [PATCH v4 1/3] led: make led_set_brightness_sync() use led_set_brightness_nosleep() Jean-Jacques Hiblot 2019-09-20 21:10 ` Jacek Anaszewski @ 2019-09-21 16:13 ` kbuild test robot 1 sibling, 0 replies; 9+ messages in thread From: kbuild test robot @ 2019-09-21 16:13 UTC (permalink / raw) To: Jean-Jacques Hiblot Cc: kbuild-all, jacek.anaszewski, pavel, daniel.thompson, linux-leds, linux-kernel, dmurphy, tomi.valkeinen, Jean-Jacques Hiblot [-- 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 --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v4 2/3] dt-bindings: leds: document the "power-supply" property 2019-09-20 12:25 [PATCH v4 0/3] leds: Add control of the voltage/current regulator to the LED core Jean-Jacques Hiblot 2019-09-20 12:25 ` [PATCH v4 1/3] led: make led_set_brightness_sync() use led_set_brightness_nosleep() Jean-Jacques Hiblot @ 2019-09-20 12:25 ` Jean-Jacques Hiblot 2019-09-20 12:25 ` [PATCH v4 3/3] leds: Add control of the voltage/current regulator to the LED core Jean-Jacques Hiblot 2 siblings, 0 replies; 9+ messages in thread From: Jean-Jacques Hiblot @ 2019-09-20 12:25 UTC (permalink / raw) To: jacek.anaszewski, pavel, daniel.thompson Cc: linux-leds, linux-kernel, dmurphy, tomi.valkeinen, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v4 3/3] leds: Add control of the voltage/current regulator to the LED core 2019-09-20 12:25 [PATCH v4 0/3] leds: Add control of the voltage/current regulator to the LED core Jean-Jacques Hiblot 2019-09-20 12:25 ` [PATCH v4 1/3] led: make led_set_brightness_sync() use led_set_brightness_nosleep() Jean-Jacques Hiblot 2019-09-20 12:25 ` [PATCH v4 2/3] dt-bindings: leds: document the "power-supply" property Jean-Jacques Hiblot @ 2019-09-20 12:25 ` Jean-Jacques Hiblot 2 siblings, 0 replies; 9+ messages in thread From: Jean-Jacques Hiblot @ 2019-09-20 12:25 UTC (permalink / raw) To: jacek.anaszewski, pavel, daniel.thompson Cc: linux-leds, linux-kernel, dmurphy, tomi.valkeinen, 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-09-24 13:43 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-20 12:25 [PATCH v4 0/3] leds: Add control of the voltage/current regulator to the LED core Jean-Jacques Hiblot 2019-09-20 12:25 ` [PATCH v4 1/3] led: make led_set_brightness_sync() use led_set_brightness_nosleep() Jean-Jacques Hiblot 2019-09-20 21:10 ` Jacek Anaszewski 2019-09-23 9:14 ` Jean-Jacques Hiblot 2019-09-23 21:03 ` Jacek Anaszewski 2019-09-24 13:43 ` Jean-Jacques Hiblot 2019-09-21 16:13 ` kbuild test robot 2019-09-20 12:25 ` [PATCH v4 2/3] dt-bindings: leds: document the "power-supply" property Jean-Jacques Hiblot 2019-09-20 12:25 ` [PATCH v4 3/3] leds: Add control of the voltage/current regulator to the LED core Jean-Jacques Hiblot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).