Add generic support for RGB LED's.
Basic idea is to use enum led_brightness also for the hue and saturation
color components.This allows to implement the color extension w/o
changes to struct led_classdev.
Select LEDS_RGB to enable building drivers using the RGB extension.
Flag LED_SET_HUE_SAT allows to specify that hue / saturation
should be overridden even if the provided values are zero.
Some examples for writing values to /sys/class/leds/<xx>/brightness:
(now also hex notation can be used)
255 -> set full brightness and keep existing color if set
0 -> switch LED off but keep existing color so that it can be restored
if the LED is switched on again later
0x1000000 -> switch LED off and set also hue and saturation to 0
0x00ffff -> set full brightness, full saturation and set hue to 0 (red)
Signed-off-by: Heiner Kallweit <[email protected]>
---
v2:
- move extension-specific code into a separate source file and
introduce config symbol LEDS_HSV for it
- create separate patch for the extension to sysfs
- use variable name led_cdev as in the rest if the core
- rename to_hsv to led_validate_brightness
- rename LED_BRIGHTNESS_SET_COLOR to LED_SET_HSV
- introduce helper is_off for checking whether V part
of a HSV value is zero
v3:
- change Kconfig to use depend instead of select, add help message,
and change config symbol to LEDS_COLOR
- change LED core object file name in Makefile
- rename flag LED_SET_HSV to LED_SET_COLOR
- rename is_off to led_is_off
- rename led_validate-brightness to led_confine_brightness
- rename variable in led_confine_brightness
- add dummy enum led_brightness value to enforce 32bit enum
- rename led-hsv-core.c to led-color-core.c
- move check of provided brightness value to led_confine_brightness
v4:
- change config symbol name to LEDS_RGB
- change name of new file to led-rgb-core.c
- factor out part of led_confine_brightness
- change led_is_off to __is_set_brightness
- in led_set_software_blink pass led_cdev->max_brightness instead of LED_FULL
- rename LED_SET_COLOR to LED_SET_HUE_SAT
v5:
- change "RGB Color LED" to "RGB LED" in Kconfig
- move definition of LED_HUE_SAT_MASK to drivers/leds/leds.h
- rename LED_DEV_CAP_HSV to LED_DEV_CAP_RGB
v6:
- no change
v7:
- removed "Color" from RGB Color LED in commit message and title
- don't include linux/kernel.h in led-rgb-core.c
- keep definition of LED_DEV_CAP_[] flags together
---
drivers/leds/Kconfig | 11 +++++++++++
drivers/leds/Makefile | 4 +++-
drivers/leds/led-class.c | 2 +-
drivers/leds/led-core.c | 16 +++++++++-------
drivers/leds/led-rgb-core.c | 39 +++++++++++++++++++++++++++++++++++++++
drivers/leds/leds.h | 18 ++++++++++++++++++
include/linux/leds.h | 11 ++++++++++-
7 files changed, 91 insertions(+), 10 deletions(-)
create mode 100644 drivers/leds/led-rgb-core.c
diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 7f940c2..5b1c852 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -13,6 +13,13 @@ menuconfig NEW_LEDS
if NEW_LEDS
+config LEDS_RGB
+ bool "RGB LED Support"
+ help
+ This option enables support for RGB Color LED devices.
+ Sysfs attribute brightness is interpreted as a HSV color value.
+ For details see Documentation/leds/leds-class.txt.
+
config LEDS_CLASS
tristate "LED Class Support"
help
@@ -29,6 +36,10 @@ config LEDS_CLASS_FLASH
for the flash related features of a LED device. It can be built
as a module.
+if LEDS_RGB
+comment "RGB LED drivers"
+endif # LEDS_RGB
+
comment "LED drivers"
config LEDS_88PM860X
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index e9d5309..cc3676f 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -1,6 +1,8 @@
# LED Core
-obj-$(CONFIG_NEW_LEDS) += led-core.o
+obj-$(CONFIG_NEW_LEDS) += led-core-objs.o
+led-core-objs-y := led-core.o
+led-core-objs-$(CONFIG_LEDS_RGB) += led-rgb-core.o
obj-$(CONFIG_LEDS_CLASS) += led-class.o
obj-$(CONFIG_LEDS_CLASS_FLASH) += led-class-flash.o
obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index aa84e5b..007a5b3 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -53,7 +53,7 @@ static ssize_t brightness_store(struct device *dev,
if (ret)
goto unlock;
- if (state == LED_OFF)
+ if (!__is_brightness_set(state))
led_trigger_remove(led_cdev);
led_set_brightness(led_cdev, state);
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 3495d5d..e75b0c8 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -62,7 +62,7 @@ static void led_timer_function(unsigned long data)
}
brightness = led_get_brightness(led_cdev);
- if (!brightness) {
+ if (!__is_brightness_set(brightness)) {
/* Time to switch the LED on. */
brightness = led_cdev->blink_brightness;
delay = led_cdev->blink_delay_on;
@@ -133,8 +133,9 @@ static void led_set_software_blink(struct led_classdev *led_cdev,
if (current_brightness)
led_cdev->blink_brightness = current_brightness;
if (!led_cdev->blink_brightness)
- led_cdev->blink_brightness = led_cdev->max_brightness;
-
+ led_cdev->blink_brightness =
+ led_confine_brightness(led_cdev,
+ led_cdev->max_brightness);
led_cdev->blink_delay_on = delay_on;
led_cdev->blink_delay_off = delay_off;
@@ -235,12 +236,13 @@ void led_set_brightness(struct led_classdev *led_cdev,
* work queue task to avoid problems in case we are called
* from hard irq context.
*/
- if (brightness == LED_OFF) {
+ if (!__is_brightness_set(brightness)) {
led_cdev->flags |= LED_BLINK_DISABLE;
schedule_work(&led_cdev->set_brightness_work);
} else {
led_cdev->flags |= LED_BLINK_BRIGHTNESS_CHANGE;
- led_cdev->blink_brightness = brightness;
+ led_cdev->blink_brightness =
+ led_confine_brightness(led_cdev, brightness);
}
return;
}
@@ -265,7 +267,7 @@ EXPORT_SYMBOL_GPL(led_set_brightness_nopm);
void led_set_brightness_nosleep(struct led_classdev *led_cdev,
enum led_brightness value)
{
- led_cdev->brightness = min(value, led_cdev->max_brightness);
+ led_cdev->brightness = led_confine_brightness(led_cdev, value);
if (led_cdev->flags & LED_SUSPENDED)
return;
@@ -280,7 +282,7 @@ int led_set_brightness_sync(struct led_classdev *led_cdev,
if (led_cdev->blink_delay_on || led_cdev->blink_delay_off)
return -EBUSY;
- led_cdev->brightness = min(value, led_cdev->max_brightness);
+ led_cdev->brightness = led_confine_brightness(led_cdev, value);
if (led_cdev->flags & LED_SUSPENDED)
return 0;
diff --git a/drivers/leds/led-rgb-core.c b/drivers/leds/led-rgb-core.c
new file mode 100644
index 0000000..cbd8b35
--- /dev/null
+++ b/drivers/leds/led-rgb-core.c
@@ -0,0 +1,39 @@
+/*
+ * LED Class Color Support
+ *
+ * Author: Heiner Kallweit <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/leds.h>
+#include "leds.h"
+
+/*
+ * The color extension handles RGB LEDs but uses a HSV color model internally.
+ * led_rgb_adjust_hue_sat sets hue and saturation part of the HSV color value.
+ */
+static enum led_brightness led_rgb_adjust_hue_sat(struct led_classdev *led_cdev,
+ enum led_brightness value)
+{
+ /* LED_SET_HUE_SAT sets hue and saturation even if both are zero */
+ if (value & LED_SET_HUE_SAT || value > LED_FULL)
+ return value & LED_HUE_SAT_MASK;
+ else
+ return led_cdev->brightness & ~LED_BRIGHTNESS_MASK;
+}
+
+enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
+ enum led_brightness value)
+{
+ enum led_brightness brightness = 0;
+
+ if (led_cdev->flags & LED_DEV_CAP_RGB)
+ brightness = led_rgb_adjust_hue_sat(led_cdev, value);
+
+ return brightness |
+ min(value & LED_BRIGHTNESS_MASK, led_cdev->max_brightness);
+}
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index db3f20d..b853f54 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -16,17 +16,35 @@
#include <linux/rwsem.h>
#include <linux/leds.h>
+#define LED_BRIGHTNESS_MASK 0x000000ff
+#define LED_HUE_SAT_MASK 0x00ffff00
+
static inline int led_get_brightness(struct led_classdev *led_cdev)
{
return led_cdev->brightness;
}
+static inline bool __is_brightness_set(enum led_brightness brightness)
+{
+ return (brightness & LED_BRIGHTNESS_MASK) != LED_OFF;
+}
+
void led_init_core(struct led_classdev *led_cdev);
void led_stop_software_blink(struct led_classdev *led_cdev);
void led_set_brightness_nopm(struct led_classdev *led_cdev,
enum led_brightness value);
void led_set_brightness_nosleep(struct led_classdev *led_cdev,
enum led_brightness value);
+#if IS_ENABLED(CONFIG_LEDS_RGB)
+enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
+ enum led_brightness value);
+#else
+static inline enum led_brightness led_confine_brightness(
+ struct led_classdev *led_cdev, enum led_brightness value)
+{
+ return min(value, led_cdev->max_brightness);
+}
+#endif
extern struct rw_semaphore leds_list_lock;
extern struct list_head leds_list;
diff --git a/include/linux/leds.h b/include/linux/leds.h
index f203a8f..58e8299 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -29,8 +29,16 @@ enum led_brightness {
LED_OFF = 0,
LED_HALF = 127,
LED_FULL = 255,
+ /*
+ * dummy enum value to make gcc use a 32 bit type for the enum
+ * even if compiled with -fshort-enums. This is needed for
+ * the enum to store hsv values.
+ */
+ LED_LEVEL_DUMMY = 0xffffffff,
};
+#define LED_SET_HUE_SAT BIT(24)
+
struct led_classdev {
const char *name;
enum led_brightness brightness;
@@ -49,7 +57,8 @@ struct led_classdev {
#define LED_BLINK_DISABLE (1 << 21)
#define LED_SYSFS_DISABLE (1 << 22)
#define LED_DEV_CAP_FLASH (1 << 23)
-#define LED_HW_PLUGGABLE (1 << 24)
+#define LED_DEV_CAP_RGB (1 << 24)
+#define LED_HW_PLUGGABLE (1 << 25)
/* Set LED brightness level
* Must not sleep. Use brightness_set_blocking for drivers
--
2.7.1
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Heiner Kallweit <[email protected]> wrote:
> Add generic support for RGB LED's.
>
> Basic idea is to use enum led_brightness also for the hue and
> saturation color components.This allows to implement the color
> extension w/o changes to struct led_classdev.
>
> Select LEDS_RGB to enable building drivers using the RGB
> extension.
>
> Flag LED_SET_HUE_SAT allows to specify that hue / saturation
> should be overridden even if the provided values are zero.
>
> Some examples for writing values to
> /sys/class/leds/<xx>/brightness: (now also hex notation can be
> used)
>
> 255 -> set full brightness and keep existing color if set 0 ->
> switch LED off but keep existing color so that it can be
> restored
> if the LED is switched on again later
> 0x1000000 -> switch LED off and set also hue and saturation to
> 0 0x00ffff -> set full brightness, full saturation and set hue
> to 0 (red)
>
I admit I hadn't seen this earlier, and I didn't spend all day
looking at previous attempts, but what's the motivation for
putting all this overloaded syntax into the "brightness" file? I
thought the point was to have a single value in each file, one of
the references I did find was
http://www.spinics.net/lists/linux-leds/msg02995.html Is there
some thread where this was decided as advantageous? Surely 0-255
for _brightness_ is what the brightness entry should do?
I'd like to set the rgb colour of a led (or hsv, that can be it's
own file too) and separately ramp the brightness up and down. I
also think it's substantially simpler and easier to use from the
user's point of view, which is surely the place you want easy
right?
Sincerely,
Karl Palsson
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
iQIcBAEBAgAGBQJW3Jk5AAoJEBmotQ/U1cr2fxYP/AsuQ/x8ky86S9xf9Y8UdRrk
IC7eouBpf07RsDTv2KobRwEH69tk12zxGKmOpNZ5SY8ozVT/VDXMA8Iw/cwKL2t4
EBWTdBhORrOlfxw0sykp4SXYSYBm9n2Z+xZGK9b/fN+2g+XCv4B+W2iDejyvAsIt
c/eH6dGR0PvYdovEh0Tq7qAflpXRAhU0ykRR0Ydq/HrF8Xfxi+MDHC99zTRrHIsV
rPTbPh26cxZ3zyOoUxwgPLNmm4O1BvMsghxuQXV49A95gOlRet+ewDQxBgwWabEp
AUh3fuOl53R/ODJSqjX/JjlO4ynXWgv/9kdCF8QwPUAl13gyhilPvIdI5O3gm3Nr
beiW/rUnvHej3ZxbRUe/Q8ZlQ099WTVH4cEgSxLclC5hiWm4dCjsjskJA1acbnZV
4w5WSqrAqSyNP81Rhy7WV6k8kazDUrASSAl4JFnNJVRC4WNdHQJA4pKkH08mtYyo
5ls3ydMzU2eiTNKCFEze4/cH3MgUWM+L29rLRzev6rT7s32rPzR0JKaKv460pocd
rjpKanbt+zgUVySprVzX4t4GsmDZtKjQkTGooz9BabZP5+WeVvDtEMK3kciZ1d/x
ubtvcMXGbDpZ0FMcQkTQj44Sq3wMdr3P0CoMiDspDGk7XY67gSXsmUgSSh0JTLRL
X4K67h/OUpH0A00XGZCO
=86mG
-----END PGP SIGNATURE-----
Am 06.03.2016 um 21:55 schrieb Karl Palsson:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Heiner Kallweit <[email protected]> wrote:
>> Add generic support for RGB LED's.
>>
>> Basic idea is to use enum led_brightness also for the hue and
>> saturation color components.This allows to implement the color
>> extension w/o changes to struct led_classdev.
>>
>> Select LEDS_RGB to enable building drivers using the RGB
>> extension.
>>
>> Flag LED_SET_HUE_SAT allows to specify that hue / saturation
>> should be overridden even if the provided values are zero.
>>
>> Some examples for writing values to
>> /sys/class/leds/<xx>/brightness: (now also hex notation can be
>> used)
>>
>> 255 -> set full brightness and keep existing color if set 0 ->
>> switch LED off but keep existing color so that it can be
>> restored
>> if the LED is switched on again later
>> 0x1000000 -> switch LED off and set also hue and saturation to
>> 0 0x00ffff -> set full brightness, full saturation and set hue
>> to 0 (red)
>>
>
>
> I admit I hadn't seen this earlier, and I didn't spend all day
> looking at previous attempts, but what's the motivation for
> putting all this overloaded syntax into the "brightness" file? I
> thought the point was to have a single value in each file, one of
> the references I did find was
> http://www.spinics.net/lists/linux-leds/msg02995.html Is there
> some thread where this was decided as advantageous? Surely 0-255
> for _brightness_ is what the brightness entry should do?
>
The referenced mail thread refers to a proposed sysfs attribute
holding a list of space-separated entries. Here it's still about
one numeric value.
Advantage of the approach used now is the full backwards compatibilty.
You can still set brightness to 0..255 and only the brightness will
change (as expected). Or in other words: So far only V was supported,
now we support HSV as a superset.
> I'd like to set the rgb colour of a led (or hsv, that can be it's
> own file too) and separately ramp the brightness up and down. I
> also think it's substantially simpler and easier to use from the
> user's point of view, which is surely the place you want easy
> right?
>
What you describe is perfectly possible with the new approach.
Regards, Heiner
> Sincerely,
> Karl Palsson
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
>
> iQIcBAEBAgAGBQJW3Jk5AAoJEBmotQ/U1cr2fxYP/AsuQ/x8ky86S9xf9Y8UdRrk
> IC7eouBpf07RsDTv2KobRwEH69tk12zxGKmOpNZ5SY8ozVT/VDXMA8Iw/cwKL2t4
> EBWTdBhORrOlfxw0sykp4SXYSYBm9n2Z+xZGK9b/fN+2g+XCv4B+W2iDejyvAsIt
> c/eH6dGR0PvYdovEh0Tq7qAflpXRAhU0ykRR0Ydq/HrF8Xfxi+MDHC99zTRrHIsV
> rPTbPh26cxZ3zyOoUxwgPLNmm4O1BvMsghxuQXV49A95gOlRet+ewDQxBgwWabEp
> AUh3fuOl53R/ODJSqjX/JjlO4ynXWgv/9kdCF8QwPUAl13gyhilPvIdI5O3gm3Nr
> beiW/rUnvHej3ZxbRUe/Q8ZlQ099WTVH4cEgSxLclC5hiWm4dCjsjskJA1acbnZV
> 4w5WSqrAqSyNP81Rhy7WV6k8kazDUrASSAl4JFnNJVRC4WNdHQJA4pKkH08mtYyo
> 5ls3ydMzU2eiTNKCFEze4/cH3MgUWM+L29rLRzev6rT7s32rPzR0JKaKv460pocd
> rjpKanbt+zgUVySprVzX4t4GsmDZtKjQkTGooz9BabZP5+WeVvDtEMK3kciZ1d/x
> ubtvcMXGbDpZ0FMcQkTQj44Sq3wMdr3P0CoMiDspDGk7XY67gSXsmUgSSh0JTLRL
> X4K67h/OUpH0A00XGZCO
> =86mG
> -----END PGP SIGNATURE-----
>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
> > I admit I hadn't seen this earlier, and I didn't spend all day
> > looking at previous attempts, but what's the motivation for
> > putting all this overloaded syntax into the "brightness" file? I
> > thought the point was to have a single value in each file, one of
> > the references I did find was
> > http://www.spinics.net/lists/linux-leds/msg02995.html Is there
> > some thread where this was decided as advantageous? Surely 0-255
> > for _brightness_ is what the brightness entry should do?
> >
> The referenced mail thread refers to a proposed sysfs attribute
> holding a list of space-separated entries. Here it's still
> about one numeric value. Advantage of the approach used now is
> the full backwards compatibilty. You can still set brightness
> to 0..255 and only the brightness will change (as expected). Or
> in other words: So far only V was supported, now we support HSV
> as a superset.
>
> > I'd like to set the rgb colour of a led (or hsv, that can be it's
> > own file too) and separately ramp the brightness up and down. I
> > also think it's substantially simpler and easier to use from the
> > user's point of view, which is surely the place you want easy
> > right?
> >
> What you describe is perfectly possible with the new approach.
>
Only by using magically different formats for what I write to the
file labelled "brightness" I'm really not a fan of a knob called
"brightness" that does _other things_ if you write something
different to it.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
iQIcBAEBAgAGBQJW3LXbAAoJEBmotQ/U1cr2fdUQAKONfj98SS2b4zcHheXG4+ts
PfiyS3nhh3VxlNykJSF13ou6ZBgYs4vuTVFuXO2Vya07TFWGrRM4ZNM0d+3JU7BY
QkDq8nhqYkxBmIvuz3dgmr+HrMShT/ECQoOYSoIq9bAHpXj0v+eCvYnlLseVLBGx
inwgyBme0jSiXvITRDBc0YM0wvpnh492UlruOHGsGkoTSaPaBXU8E+Uv4sv+y448
muYI9M3l+4Lg+B5k9UyXEvwCi2pyLJ2pSGbXyk6flSWYxI92Mny0decOQ+myJqEr
sfIqOczGbXFXiWXo8oX2i/Dm9+b+ChrMEXAtcfMcuB+9+469p/2eoqcCCtMWnAUJ
qmM8h37mNoohwDIv9c4blG2Y2854n6L2R7ZCXGMZj1Thidmn7ANNhnIFp+ojyXsz
O/JSongYWxX4b9pMQ8QhZFRCXfi/V7c/0RDREN5IcjB5nm+1W4Wr/u0jqDGsD7hW
kYgWHLbRzBtjOk7ruyDRBNlUyV+xCwxmYgmHAo3Ko3ZPs0MAPQoD+u6mtG5BPDkY
ek8ek7+Zw1V8MQSKp1LEfVr6GX5rTkaTD13odbC8PcMYACAC6NKFDqS1NNvSclKv
nUyccdaxw0NU4WZtG1dalvJGMwMj6z5MT9BjlE9JvSs4vROYx7RGOQvGvxw9syJT
j5AL5VA86J8n30tDXFuy
=hUju
-----END PGP SIGNATURE-----
Hi Karl,
On 03/06/2016 09:55 PM, Karl Palsson wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Heiner Kallweit <[email protected]> wrote:
>> Add generic support for RGB LED's.
>>
>> Basic idea is to use enum led_brightness also for the hue and
>> saturation color components.This allows to implement the color
>> extension w/o changes to struct led_classdev.
>>
>> Select LEDS_RGB to enable building drivers using the RGB
>> extension.
>>
>> Flag LED_SET_HUE_SAT allows to specify that hue / saturation
>> should be overridden even if the provided values are zero.
>>
>> Some examples for writing values to
>> /sys/class/leds/<xx>/brightness: (now also hex notation can be
>> used)
>>
>> 255 -> set full brightness and keep existing color if set 0 ->
>> switch LED off but keep existing color so that it can be
>> restored
>> if the LED is switched on again later
>> 0x1000000 -> switch LED off and set also hue and saturation to
>> 0 0x00ffff -> set full brightness, full saturation and set hue
>> to 0 (red)
>>
>
>
> I admit I hadn't seen this earlier, and I didn't spend all day
> looking at previous attempts, but what's the motivation for
> putting all this overloaded syntax into the "brightness" file? I
> thought the point was to have a single value in each file, one of
> the references I did find was
With single value per file there would be problems with colour
components setting synchronization.
> http://www.spinics.net/lists/linux-leds/msg02995.html Is there
> some thread where this was decided as advantageous? Surely 0-255
> for _brightness_ is what the brightness entry should do?
You can find a reference to the related discussion in [1].
> I'd like to set the rgb colour of a led (or hsv, that can be it's
> own file too) and separately ramp the brightness up and down. I
> also think it's substantially simpler and easier to use from the
> user's point of view, which is surely the place you want easy
> right?
I'm also not very keen on overloading the brightness attribute
semantics. Nonetheless it seems impossible to add support for
setting three colour components otherwise than through a single
syscall, if we want to make it synchronous and compatible with
LED triggers.
HSV color scheme is also very convenient for adapting monochrome
brightness semantics to the RGB realm.
[1] http://www.spinics.net/lists/linux-leds/msg05477.html
--
Best regards,
Jacek Anaszewski
Hi Heiner,
Thanks for the updated set. I've renamed the feature to RGB LED class
and pushed out to devel branch of linux-leds.git. It will sit there
till the end of the upcoming merge window. There have been some
uncertainties raised related to overloading brightness syntax. so let's
better have it in linux-next through the whole next development cycle
for the people to comment on.
Thanks,
Jacek Anaszewski
On 03/04/2016 10:09 PM, Heiner Kallweit wrote:
> Add generic support for RGB LED's.
>
> Basic idea is to use enum led_brightness also for the hue and saturation
> color components.This allows to implement the color extension w/o
> changes to struct led_classdev.
>
> Select LEDS_RGB to enable building drivers using the RGB extension.
>
> Flag LED_SET_HUE_SAT allows to specify that hue / saturation
> should be overridden even if the provided values are zero.
>
> Some examples for writing values to /sys/class/leds/<xx>/brightness:
> (now also hex notation can be used)
>
> 255 -> set full brightness and keep existing color if set
> 0 -> switch LED off but keep existing color so that it can be restored
> if the LED is switched on again later
> 0x1000000 -> switch LED off and set also hue and saturation to 0
> 0x00ffff -> set full brightness, full saturation and set hue to 0 (red)
>
> Signed-off-by: Heiner Kallweit <[email protected]>
> ---
> v2:
> - move extension-specific code into a separate source file and
> introduce config symbol LEDS_HSV for it
> - create separate patch for the extension to sysfs
> - use variable name led_cdev as in the rest if the core
> - rename to_hsv to led_validate_brightness
> - rename LED_BRIGHTNESS_SET_COLOR to LED_SET_HSV
> - introduce helper is_off for checking whether V part
> of a HSV value is zero
> v3:
> - change Kconfig to use depend instead of select, add help message,
> and change config symbol to LEDS_COLOR
> - change LED core object file name in Makefile
> - rename flag LED_SET_HSV to LED_SET_COLOR
> - rename is_off to led_is_off
> - rename led_validate-brightness to led_confine_brightness
> - rename variable in led_confine_brightness
> - add dummy enum led_brightness value to enforce 32bit enum
> - rename led-hsv-core.c to led-color-core.c
> - move check of provided brightness value to led_confine_brightness
> v4:
> - change config symbol name to LEDS_RGB
> - change name of new file to led-rgb-core.c
> - factor out part of led_confine_brightness
> - change led_is_off to __is_set_brightness
> - in led_set_software_blink pass led_cdev->max_brightness instead of LED_FULL
> - rename LED_SET_COLOR to LED_SET_HUE_SAT
> v5:
> - change "RGB Color LED" to "RGB LED" in Kconfig
> - move definition of LED_HUE_SAT_MASK to drivers/leds/leds.h
> - rename LED_DEV_CAP_HSV to LED_DEV_CAP_RGB
> v6:
> - no change
> v7:
> - removed "Color" from RGB Color LED in commit message and title
> - don't include linux/kernel.h in led-rgb-core.c
> - keep definition of LED_DEV_CAP_[] flags together
> ---
> drivers/leds/Kconfig | 11 +++++++++++
> drivers/leds/Makefile | 4 +++-
> drivers/leds/led-class.c | 2 +-
> drivers/leds/led-core.c | 16 +++++++++-------
> drivers/leds/led-rgb-core.c | 39 +++++++++++++++++++++++++++++++++++++++
> drivers/leds/leds.h | 18 ++++++++++++++++++
> include/linux/leds.h | 11 ++++++++++-
> 7 files changed, 91 insertions(+), 10 deletions(-)
> create mode 100644 drivers/leds/led-rgb-core.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 7f940c2..5b1c852 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -13,6 +13,13 @@ menuconfig NEW_LEDS
>
> if NEW_LEDS
>
> +config LEDS_RGB
> + bool "RGB LED Support"
> + help
> + This option enables support for RGB Color LED devices.
> + Sysfs attribute brightness is interpreted as a HSV color value.
> + For details see Documentation/leds/leds-class.txt.
> +
> config LEDS_CLASS
> tristate "LED Class Support"
> help
> @@ -29,6 +36,10 @@ config LEDS_CLASS_FLASH
> for the flash related features of a LED device. It can be built
> as a module.
>
> +if LEDS_RGB
> +comment "RGB LED drivers"
> +endif # LEDS_RGB
> +
> comment "LED drivers"
>
> config LEDS_88PM860X
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index e9d5309..cc3676f 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -1,6 +1,8 @@
>
> # LED Core
> -obj-$(CONFIG_NEW_LEDS) += led-core.o
> +obj-$(CONFIG_NEW_LEDS) += led-core-objs.o
> +led-core-objs-y := led-core.o
> +led-core-objs-$(CONFIG_LEDS_RGB) += led-rgb-core.o
> obj-$(CONFIG_LEDS_CLASS) += led-class.o
> obj-$(CONFIG_LEDS_CLASS_FLASH) += led-class-flash.o
> obj-$(CONFIG_LEDS_TRIGGERS) += led-triggers.o
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index aa84e5b..007a5b3 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -53,7 +53,7 @@ static ssize_t brightness_store(struct device *dev,
> if (ret)
> goto unlock;
>
> - if (state == LED_OFF)
> + if (!__is_brightness_set(state))
> led_trigger_remove(led_cdev);
> led_set_brightness(led_cdev, state);
>
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index 3495d5d..e75b0c8 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -62,7 +62,7 @@ static void led_timer_function(unsigned long data)
> }
>
> brightness = led_get_brightness(led_cdev);
> - if (!brightness) {
> + if (!__is_brightness_set(brightness)) {
> /* Time to switch the LED on. */
> brightness = led_cdev->blink_brightness;
> delay = led_cdev->blink_delay_on;
> @@ -133,8 +133,9 @@ static void led_set_software_blink(struct led_classdev *led_cdev,
> if (current_brightness)
> led_cdev->blink_brightness = current_brightness;
> if (!led_cdev->blink_brightness)
> - led_cdev->blink_brightness = led_cdev->max_brightness;
> -
> + led_cdev->blink_brightness =
> + led_confine_brightness(led_cdev,
> + led_cdev->max_brightness);
> led_cdev->blink_delay_on = delay_on;
> led_cdev->blink_delay_off = delay_off;
>
> @@ -235,12 +236,13 @@ void led_set_brightness(struct led_classdev *led_cdev,
> * work queue task to avoid problems in case we are called
> * from hard irq context.
> */
> - if (brightness == LED_OFF) {
> + if (!__is_brightness_set(brightness)) {
> led_cdev->flags |= LED_BLINK_DISABLE;
> schedule_work(&led_cdev->set_brightness_work);
> } else {
> led_cdev->flags |= LED_BLINK_BRIGHTNESS_CHANGE;
> - led_cdev->blink_brightness = brightness;
> + led_cdev->blink_brightness =
> + led_confine_brightness(led_cdev, brightness);
> }
> return;
> }
> @@ -265,7 +267,7 @@ EXPORT_SYMBOL_GPL(led_set_brightness_nopm);
> void led_set_brightness_nosleep(struct led_classdev *led_cdev,
> enum led_brightness value)
> {
> - led_cdev->brightness = min(value, led_cdev->max_brightness);
> + led_cdev->brightness = led_confine_brightness(led_cdev, value);
>
> if (led_cdev->flags & LED_SUSPENDED)
> return;
> @@ -280,7 +282,7 @@ int led_set_brightness_sync(struct led_classdev *led_cdev,
> if (led_cdev->blink_delay_on || led_cdev->blink_delay_off)
> return -EBUSY;
>
> - led_cdev->brightness = min(value, led_cdev->max_brightness);
> + led_cdev->brightness = led_confine_brightness(led_cdev, value);
>
> if (led_cdev->flags & LED_SUSPENDED)
> return 0;
> diff --git a/drivers/leds/led-rgb-core.c b/drivers/leds/led-rgb-core.c
> new file mode 100644
> index 0000000..cbd8b35
> --- /dev/null
> +++ b/drivers/leds/led-rgb-core.c
> @@ -0,0 +1,39 @@
> +/*
> + * LED Class Color Support
> + *
> + * Author: Heiner Kallweit <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/leds.h>
> +#include "leds.h"
> +
> +/*
> + * The color extension handles RGB LEDs but uses a HSV color model internally.
> + * led_rgb_adjust_hue_sat sets hue and saturation part of the HSV color value.
> + */
> +static enum led_brightness led_rgb_adjust_hue_sat(struct led_classdev *led_cdev,
> + enum led_brightness value)
> +{
> + /* LED_SET_HUE_SAT sets hue and saturation even if both are zero */
> + if (value & LED_SET_HUE_SAT || value > LED_FULL)
> + return value & LED_HUE_SAT_MASK;
> + else
> + return led_cdev->brightness & ~LED_BRIGHTNESS_MASK;
> +}
> +
> +enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
> + enum led_brightness value)
> +{
> + enum led_brightness brightness = 0;
> +
> + if (led_cdev->flags & LED_DEV_CAP_RGB)
> + brightness = led_rgb_adjust_hue_sat(led_cdev, value);
> +
> + return brightness |
> + min(value & LED_BRIGHTNESS_MASK, led_cdev->max_brightness);
> +}
> diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
> index db3f20d..b853f54 100644
> --- a/drivers/leds/leds.h
> +++ b/drivers/leds/leds.h
> @@ -16,17 +16,35 @@
> #include <linux/rwsem.h>
> #include <linux/leds.h>
>
> +#define LED_BRIGHTNESS_MASK 0x000000ff
> +#define LED_HUE_SAT_MASK 0x00ffff00
> +
> static inline int led_get_brightness(struct led_classdev *led_cdev)
> {
> return led_cdev->brightness;
> }
>
> +static inline bool __is_brightness_set(enum led_brightness brightness)
> +{
> + return (brightness & LED_BRIGHTNESS_MASK) != LED_OFF;
> +}
> +
> void led_init_core(struct led_classdev *led_cdev);
> void led_stop_software_blink(struct led_classdev *led_cdev);
> void led_set_brightness_nopm(struct led_classdev *led_cdev,
> enum led_brightness value);
> void led_set_brightness_nosleep(struct led_classdev *led_cdev,
> enum led_brightness value);
> +#if IS_ENABLED(CONFIG_LEDS_RGB)
> +enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
> + enum led_brightness value);
> +#else
> +static inline enum led_brightness led_confine_brightness(
> + struct led_classdev *led_cdev, enum led_brightness value)
> +{
> + return min(value, led_cdev->max_brightness);
> +}
> +#endif
>
> extern struct rw_semaphore leds_list_lock;
> extern struct list_head leds_list;
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index f203a8f..58e8299 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -29,8 +29,16 @@ enum led_brightness {
> LED_OFF = 0,
> LED_HALF = 127,
> LED_FULL = 255,
> + /*
> + * dummy enum value to make gcc use a 32 bit type for the enum
> + * even if compiled with -fshort-enums. This is needed for
> + * the enum to store hsv values.
> + */
> + LED_LEVEL_DUMMY = 0xffffffff,
> };
>
> +#define LED_SET_HUE_SAT BIT(24)
> +
> struct led_classdev {
> const char *name;
> enum led_brightness brightness;
> @@ -49,7 +57,8 @@ struct led_classdev {
> #define LED_BLINK_DISABLE (1 << 21)
> #define LED_SYSFS_DISABLE (1 << 22)
> #define LED_DEV_CAP_FLASH (1 << 23)
> -#define LED_HW_PLUGGABLE (1 << 24)
> +#define LED_DEV_CAP_RGB (1 << 24)
> +#define LED_HW_PLUGGABLE (1 << 25)
>
> /* Set LED brightness level
> * Must not sleep. Use brightness_set_blocking for drivers
>
--
Best regards,
Jacek Anaszewski
Am 11.03.2016 um 09:38 schrieb Jacek Anaszewski:
> Hi Heiner,
>
> Thanks for the updated set. I've renamed the feature to RGB LED class
> and pushed out to devel branch of linux-leds.git. It will sit there
> till the end of the upcoming merge window. There have been some
> uncertainties raised related to overloading brightness syntax. so let's
> better have it in linux-next through the whole next development cycle
> for the people to comment on.
Thanks Jacek, fine with me. I think in the next days I can come up with
two or three follow-up patches using this RGB LED functionality in triggers.
They would also be candidates for the devel branch then.
Rgds, Heiner
>
> Thanks,
> Jacek Anaszewski
>
> [...]