leds: core: add support for RGB LED's
PATCH 2/2 leds: core: add support for RGB LED's
Add support for RGB LED's. Flag LED_DEV_CAP_RGB is used to instruct the core to convert HSV to RGB on output.
--- drivers/leds/led-class.c | 3 +++ drivers/leds/led-core.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++- include/linux/leds.h | 1 + 3 files changed, 51 insertions(+), 1 deletion(-)
drivers/leds/led-class.c +3 -0
@@ -193,6 +193,9 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)char name[64 ];int ret; +if (led_cdev->flags &LED_DEV_CAP_RGB ) + led_cdev->flags |=LED_DEV_CAP_COLOR ; + ret =led_classdev_next_name (led_cdev->name, name,sizeof (name));if (ret <0 )return ret;
drivers/leds/led-core.c +47 -1
72 lines not on this page · raw message
include/linux/leds.h +1 -0
@@ -51,6 +51,7 @@ struct led_classdev {#define LED_DEV_CAP_FLASH (1 << 23)#define LED_HW_PLUGGABLE (1 << 24)#define LED_DEV_CAP_COLOR (1 << 25) +#define LED_DEV_CAP_RGB (1 << 26)/* Set LED brightness level * Must not sleep. Use brightness_set_blocking for drivers
Signature · 1 line
2.7.0
Quoted 57 lines · Heiner Kallweit
Add support for RGB LED's. Flag
LED_DEV_CAP_RGBis used to instruct the core to convert HSV to RGB on output.Signed-off-by Heiner Kallweit <[email protected]>--- drivers/leds/led-class.c | 3 +++ drivers/leds/led-core.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++- include/linux/leds.h | 1 + 3 files changed, 51 insertions(+), 1 deletion(-)drivers/leds/led-class.c +3
-0@@ -193,6 +193,9 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)char name[64 ];int ret; +if (led_cdev->flags &LED_DEV_CAP_RGB ) + led_cdev->flags |=LED_DEV_CAP_COLOR ; + ret =led_classdev_next_name (led_cdev->name, name,sizeof (name));if (ret <0 )return ret;drivers/leds/led-core.c +25
-0@@ -45,12 +45,50 @@ static inline enum led_brightness to_hsv(struct led_classdev *cdev,return ret |min (value &LED_BRIGHTNESS_MASK , cdev->max_brightness); } +static enum led_brightness hsv_to_rgb (enum led_brightness hsv) +{ +int h =min_t (int, (hsv >>16 ) &0xff ,251 ); +int s = (hsv >>8 ) &0xff ; +int v = hsv &0xff ; +int f, p, q, t, r, g, b; + +if (!v) +return 0 ; +if (!s) +return (v <<16 ) + (v <<8 ) + v; + + f =DIV_ROUND_CLOSEST ((h %42 ) *255 ,42 ); + p = v -DIV_ROUND_CLOSEST (s * v,255 ); + q = v -DIV_ROUND_CLOSEST (f * s * v,255 *255 ); + t = v -DIV_ROUND_CLOSEST ((255 - f) * s * v,255 *255 ); + +switch (h /42 ) { +case 0 : + r = v; g = t; b = p;break ; +case 1 : + r = q; g = v; b = p;break ; +case 2 : + r = p; g = v; b = t;break ; +case 3 :
drivers/leds/led-core.c static inline enum led_brightness to_hsv(struct led_classdev *cdev, · line 82
Quoted 10 lines
drivers/leds/led-core.c +10
-0+ r = p; g = q; b = v;break ; +case 4 : + r = t; g = p; b = v;break ; +case 5 : + r = v; g = p; b = q;break ; + } + +return (r <<16 ) + (g <<8 ) + b; +} +
This can be moved to led-hvs-core.c. Could you also share a reference to the algorithm of hsv -> rgb conversion you implemented here?
Quoted 41 lines
drivers/leds/led-core.c +12
-1static int led_set_output (struct led_classdev *cdev,enum led_brightness value) {if (!cdev->brightness_set)return -ENOTSUPP ; +if (cdev->flags &LED_DEV_CAP_RGB ) + value =hsv_to_rgb (value); + cdev->brightness_set (cdev, value);return 0 ;@@ -62,6 +100,9 @@ static int led_set_output_blocking(struct led_classdev *cdev,if (!cdev->brightness_set_blocking)return -ENOTSUPP ; +if (cdev->flags &LED_DEV_CAP_RGB ) + value =hsv_to_rgb (value); +return cdev->brightness_set_blocking (cdev, value); }@@ -320,7 +361,12 @@ int led_update_brightness(struct led_classdev *led_cdev) {int ret =0 ; -if (led_cdev->brightness_get) { +/* +* for now reading back the color is not supported as multiple +* HSV -> RGB -> HSV conversions may distort the color due to +* rounding issues in the conversion algorithm +*/ +if (led_cdev->brightness_get && !(led_cdev->flags &LED_DEV_CAP_RGB )) { ret = led_cdev->brightness_get (led_cdev);if (ret >=0 ) { led_cdev->brightness = ret;include/linux/leds.h +0
-0
include/linux/leds.h struct led_classdev { · line 54
Quoted 4 lines
include/linux/leds.h +1
-0@@ -51,6 +51,7 @@ struct led_classdev {#define LED_DEV_CAP_FLASH (1 << 23)#define LED_HW_PLUGGABLE (1 << 24)#define LED_DEV_CAP_COLOR (1 << 25) +#define LED_DEV_CAP_RGB (1 << 26)
Why two flags - COLOR and RGB? Their meaning is ambiguous.
Quoted 4 lines
include/linux/leds.h +0
-0/* Set LED brightness level * Must not sleep. Use brightness_set_blocking for drivers
Signature · 2 lines
Best Regards, Jacek Anaszewski
Am 12.02.2016 um 17:12 schrieb Jacek Anaszewski:
Quoted 63 lines · Heiner Kallweit
Quoted 62 lines
Add support for RGB LED's. Flag
LED_DEV_CAP_RGBis used to instruct the core to convert HSV to RGB on output.Signed-off-by Heiner Kallweit <[email protected]>--- drivers/leds/led-class.c | 3 +++ drivers/leds/led-core.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++- include/linux/leds.h | 1 + 3 files changed, 51 insertions(+), 1 deletion(-)drivers/leds/led-class.c +3
-0@@ -193,6 +193,9 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)char name[64 ];int ret; +if (led_cdev->flags &LED_DEV_CAP_RGB ) + led_cdev->flags |=LED_DEV_CAP_COLOR ; + ret =led_classdev_next_name (led_cdev->name, name,sizeof (name));if (ret <0 )return ret;drivers/leds/led-core.c +30
-0@@ -45,12 +45,50 @@ static inline enum led_brightness to_hsv(struct led_classdev *cdev,return ret |min (value &LED_BRIGHTNESS_MASK , cdev->max_brightness); } +static enum led_brightness hsv_to_rgb (enum led_brightness hsv) +{ +int h =min_t (int, (hsv >>16 ) &0xff ,251 ); +int s = (hsv >>8 ) &0xff ; +int v = hsv &0xff ; +int f, p, q, t, r, g, b; + +if (!v) +return 0 ; +if (!s) +return (v <<16 ) + (v <<8 ) + v; + + f =DIV_ROUND_CLOSEST ((h %42 ) *255 ,42 ); + p = v -DIV_ROUND_CLOSEST (s * v,255 ); + q = v -DIV_ROUND_CLOSEST (f * s * v,255 *255 ); + t = v -DIV_ROUND_CLOSEST ((255 - f) * s * v,255 *255 ); + +switch (h /42 ) { +case 0 : + r = v; g = t; b = p;break ; +case 1 : + r = q; g = v; b = p;break ; +case 2 : + r = p; g = v; b = t;break ; +case 3 : + r = p; g = q; b = v;break ; +case 4 : + r = t; g = p; b = v;break ; +case 5 : + r = v; g = p; b = q;break ;
Quoted 9 lines
drivers/leds/led-core.c static inline enum led_brightness to_hsv(struct led_classdev *cdev, · line 82
Quoted 5 lines
drivers/leds/led-core.c +5
-0+ } + +return (r <<16 ) + (g <<8 ) + b; +} +This can be moved to
led-hvs-core.c. Could you also share a reference to the algorithm of hsv -> rgb conversion you implemented here?
OK, will move it. The HSV -> RGB algorithm is the one from Gonzalez / Woods and I took it from the German wikipedia.
https://de.wikipedia.org/wiki/HSV-Farbraum#Umrechnung_HSV_in_RGB
The calculation was slightly adjusted to deal with intervals 0-255 instead of [0,1] and for hue 0-251 instead of [0, 360)
Quoted 39 lines
Quoted 39 lines
drivers/leds/led-core.c +12
-1static int led_set_output (struct led_classdev *cdev,enum led_brightness value) {if (!cdev->brightness_set)return -ENOTSUPP ; +if (cdev->flags &LED_DEV_CAP_RGB ) + value =hsv_to_rgb (value); + cdev->brightness_set (cdev, value);return 0 ;@@ -62,6 +100,9 @@ static int led_set_output_blocking(struct led_classdev *cdev,if (!cdev->brightness_set_blocking)return -ENOTSUPP ; +if (cdev->flags &LED_DEV_CAP_RGB ) + value =hsv_to_rgb (value); +return cdev->brightness_set_blocking (cdev, value); }@@ -320,7 +361,12 @@ int led_update_brightness(struct led_classdev *led_cdev) {int ret =0 ; -if (led_cdev->brightness_get) { +/* +* for now reading back the color is not supported as multiple +* HSV -> RGB -> HSV conversions may distort the color due to +* rounding issues in the conversion algorithm +*/ +if (led_cdev->brightness_get && !(led_cdev->flags &LED_DEV_CAP_RGB )) { ret = led_cdev->brightness_get (led_cdev);if (ret >=0 ) { led_cdev->brightness = ret;include/linux/leds.h +0
-1--- a/include/linux/leds.h
Quoted 9 lines
include/linux/leds.h struct led_classdev { · line 54
Quoted 6 lines
+++
b/include/linux/leds.hinclude/linux/leds.h +1
-0@@ -51,6 +51,7 @@ struct led_classdev {#define LED_DEV_CAP_FLASH (1 << 23)#define LED_HW_PLUGGABLE (1 << 24)#define LED_DEV_CAP_COLOR (1 << 25) +#define LED_DEV_CAP_RGB (1 << 26)Why two flags - COLOR and RGB? Their meaning is ambiguous.
There might be color LED's with HSV interface and setting the COLOR flag but not the RGB flag would support such LED's w/o additional code. Not sure whether such devices exist, but one additional flag might be a small enough price for decoupling internal color handling and RGB output conversion.
Quoted 6 lines
Quoted 4 lines
include/linux/leds.h +0
-0/* Set LED brightness level * Must not sleep. Use brightness_set_blocking for drivers
Quoted 73 lines · Heiner Kallweit
Am 12.02.2016 um 17:12 schrieb Jacek Anaszewski:
Quoted 63 lines · Heiner Kallweit
Quoted 62 lines
Add support for RGB LED's. Flag
LED_DEV_CAP_RGBis used to instruct the core to convert HSV to RGB on output.Signed-off-by Heiner Kallweit <[email protected]>--- drivers/leds/led-class.c | 3 +++ drivers/leds/led-core.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++- include/linux/leds.h | 1 + 3 files changed, 51 insertions(+), 1 deletion(-)drivers/leds/led-class.c +3
-0@@ -193,6 +193,9 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)char name[64 ];int ret; +if (led_cdev->flags &LED_DEV_CAP_RGB ) + led_cdev->flags |=LED_DEV_CAP_COLOR ; + ret =led_classdev_next_name (led_cdev->name, name,sizeof (name));if (ret <0 )return ret;drivers/leds/led-core.c +30
-0@@ -45,12 +45,50 @@ static inline enum led_brightness to_hsv(struct led_classdev *cdev,return ret |min (value &LED_BRIGHTNESS_MASK , cdev->max_brightness); } +static enum led_brightness hsv_to_rgb (enum led_brightness hsv) +{ +int h =min_t (int, (hsv >>16 ) &0xff ,251 ); +int s = (hsv >>8 ) &0xff ; +int v = hsv &0xff ; +int f, p, q, t, r, g, b; + +if (!v) +return 0 ; +if (!s) +return (v <<16 ) + (v <<8 ) + v; + + f =DIV_ROUND_CLOSEST ((h %42 ) *255 ,42 ); + p = v -DIV_ROUND_CLOSEST (s * v,255 ); + q = v -DIV_ROUND_CLOSEST (f * s * v,255 *255 ); + t = v -DIV_ROUND_CLOSEST ((255 - f) * s * v,255 *255 ); + +switch (h /42 ) { +case 0 : + r = v; g = t; b = p;break ; +case 1 : + r = q; g = v; b = p;break ; +case 2 : + r = p; g = v; b = t;break ; +case 3 : + r = p; g = q; b = v;break ; +case 4 : + r = t; g = p; b = v;break ; +case 5 : + r = v; g = p; b = q;break ;Quoted 9 lines
drivers/leds/led-core.c static inline enum led_brightness to_hsv(struct led_classdev *cdev, · line 82
Quoted 5 lines
drivers/leds/led-core.c +5
-0+ } + +return (r <<16 ) + (g <<8 ) + b; +} +This can be moved to
led-hvs-core.c. Could you also share a reference to the algorithm of hsv -> rgb conversion you implemented here?
Quoted 5 lines
OK, will move it. The HSV -> RGB algorithm is the one from Gonzalez / Woods and I took it from the German wikipedia.
https://de.wikipedia.org/wiki/HSV-Farbraum#Umrechnung_HSV_in_RGB
The calculation was slightly adjusted to deal with intervals 0-255 instead of [0,1] and for hue 0-251 instead of [0, 360)
Thanks for the link. It is possible that I'll have some comments to this piece of code after getting acquainted with the subject, but it may take a while.
Quoted 48 lines
Quoted 39 lines
Quoted 39 lines
drivers/leds/led-core.c +12
-1static int led_set_output (struct led_classdev *cdev,enum led_brightness value) {if (!cdev->brightness_set)return -ENOTSUPP ; +if (cdev->flags &LED_DEV_CAP_RGB ) + value =hsv_to_rgb (value); + cdev->brightness_set (cdev, value);return 0 ;@@ -62,6 +100,9 @@ static int led_set_output_blocking(struct led_classdev *cdev,if (!cdev->brightness_set_blocking)return -ENOTSUPP ; +if (cdev->flags &LED_DEV_CAP_RGB ) + value =hsv_to_rgb (value); +return cdev->brightness_set_blocking (cdev, value); }@@ -320,7 +361,12 @@ int led_update_brightness(struct led_classdev *led_cdev) {int ret =0 ; -if (led_cdev->brightness_get) { +/* +* for now reading back the color is not supported as multiple +* HSV -> RGB -> HSV conversions may distort the color due to +* rounding issues in the conversion algorithm +*/ +if (led_cdev->brightness_get && !(led_cdev->flags &LED_DEV_CAP_RGB )) { ret = led_cdev->brightness_get (led_cdev);if (ret >=0 ) { led_cdev->brightness = ret;include/linux/leds.h +0
-1--- a/include/linux/leds.hQuoted 9 lines
include/linux/leds.h struct led_classdev { · line 54
Quoted 6 lines
+++
b/include/linux/leds.hinclude/linux/leds.h +1
-0@@ -51,6 +51,7 @@ struct led_classdev {#define LED_DEV_CAP_FLASH (1 << 23)#define LED_HW_PLUGGABLE (1 << 24)#define LED_DEV_CAP_COLOR (1 << 25) +#define LED_DEV_CAP_RGB (1 << 26)Why two flags - COLOR and RGB? Their meaning is ambiguous.
Quoted 5 lines
There might be color LED's with HSV interface and setting the COLOR flag but not the RGB flag would support such LED's w/o additional code. Not sure whether such devices exist, but one additional flag might be a small enough price for decoupling internal color handling and RGB output conversion.
For now let's keep the code as simple as possible. We can always add the flag when needed.
Please also rename LED_DEV_CAP_COLOR to LED_DEV_CAP_HSV.
We'll also have to change the type of brightness and max_brightness properties of struct led_classdev to u32. By default it is signed int type, but using -fshort-enums GCC option causes the smallest possible type to be used [1].
Quoted 7 lines
Quoted 6 lines
include/linux/leds.h struct led_classdev {
Quoted 4 lines
include/linux/leds.h +0
-0/* Set LED brightness level * Must not sleep. Use brightness_set_blocking for drivers
Quoted 6 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-leds" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[1] http://www.gnu.org/software/gnu-c-manual/gnu-c-manual.html#Enumerations
Signature · 2 lines
Best regards, Jacek Anaszewski
Am 15.02.2016 um 11:19 schrieb Jacek Anaszewski:
Quoted 141 lines · Heiner Kallweit
Quoted 73 lines
Am 12.02.2016 um 17:12 schrieb Jacek Anaszewski:
Quoted 63 lines · Heiner Kallweit
Quoted 62 lines
Add support for RGB LED's. Flag
LED_DEV_CAP_RGBis used to instruct the core to convert HSV to RGB on output.Signed-off-by Heiner Kallweit <[email protected]>--- drivers/leds/led-class.c | 3 +++ drivers/leds/led-core.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++- include/linux/leds.h | 1 + 3 files changed, 51 insertions(+), 1 deletion(-)drivers/leds/led-class.c +3
-0@@ -193,6 +193,9 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)char name[64 ];int ret; +if (led_cdev->flags &LED_DEV_CAP_RGB ) + led_cdev->flags |=LED_DEV_CAP_COLOR ; + ret =led_classdev_next_name (led_cdev->name, name,sizeof (name));if (ret <0 )return ret;drivers/leds/led-core.c +30
-0@@ -45,12 +45,50 @@ static inline enum led_brightness to_hsv(struct led_classdev *cdev,return ret |min (value &LED_BRIGHTNESS_MASK , cdev->max_brightness); } +static enum led_brightness hsv_to_rgb (enum led_brightness hsv) +{ +int h =min_t (int, (hsv >>16 ) &0xff ,251 ); +int s = (hsv >>8 ) &0xff ; +int v = hsv &0xff ; +int f, p, q, t, r, g, b; + +if (!v) +return 0 ; +if (!s) +return (v <<16 ) + (v <<8 ) + v; + + f =DIV_ROUND_CLOSEST ((h %42 ) *255 ,42 ); + p = v -DIV_ROUND_CLOSEST (s * v,255 ); + q = v -DIV_ROUND_CLOSEST (f * s * v,255 *255 ); + t = v -DIV_ROUND_CLOSEST ((255 - f) * s * v,255 *255 ); + +switch (h /42 ) { +case 0 : + r = v; g = t; b = p;break ; +case 1 : + r = q; g = v; b = p;break ; +case 2 : + r = p; g = v; b = t;break ; +case 3 : + r = p; g = q; b = v;break ; +case 4 : + r = t; g = p; b = v;break ; +case 5 : + r = v; g = p; b = q;break ;Quoted 9 lines
drivers/leds/led-core.c static inline enum led_brightness to_hsv(struct led_classdev *cdev, · line 82
Quoted 5 lines
drivers/leds/led-core.c +5
-0+ } + +return (r <<16 ) + (g <<8 ) + b; +} +This can be moved to
led-hvs-core.c. Could you also share a reference to the algorithm of hsv -> rgb conversion you implemented here?Quoted 5 lines
OK, will move it. The HSV -> RGB algorithm is the one from Gonzalez / Woods and I took it from the German wikipedia.
https://de.wikipedia.org/wiki/HSV-Farbraum#Umrechnung_HSV_in_RGB
The calculation was slightly adjusted to deal with intervals 0-255 instead of [0,1] and for hue 0-251 instead of [0, 360)Thanks for the link. It is possible that I'll have some comments to this piece of code after getting acquainted with the subject, but it may take a while.
Quoted 48 lines
Quoted 39 lines
Quoted 39 lines
drivers/leds/led-core.c +12
-1static int led_set_output (struct led_classdev *cdev,enum led_brightness value) {if (!cdev->brightness_set)return -ENOTSUPP ; +if (cdev->flags &LED_DEV_CAP_RGB ) + value =hsv_to_rgb (value); + cdev->brightness_set (cdev, value);return 0 ;@@ -62,6 +100,9 @@ static int led_set_output_blocking(struct led_classdev *cdev,if (!cdev->brightness_set_blocking)return -ENOTSUPP ; +if (cdev->flags &LED_DEV_CAP_RGB ) + value =hsv_to_rgb (value); +return cdev->brightness_set_blocking (cdev, value); }@@ -320,7 +361,12 @@ int led_update_brightness(struct led_classdev *led_cdev) {int ret =0 ; -if (led_cdev->brightness_get) { +/* +* for now reading back the color is not supported as multiple +* HSV -> RGB -> HSV conversions may distort the color due to +* rounding issues in the conversion algorithm +*/ +if (led_cdev->brightness_get && !(led_cdev->flags &LED_DEV_CAP_RGB )) { ret = led_cdev->brightness_get (led_cdev);if (ret >=0 ) { led_cdev->brightness = ret;include/linux/leds.h +0
-1--- a/include/linux/leds.hQuoted 9 lines
include/linux/leds.h struct led_classdev { · line 54
Quoted 6 lines
+++
b/include/linux/leds.hinclude/linux/leds.h +1
-0@@ -51,6 +51,7 @@ struct led_classdev {#define LED_DEV_CAP_FLASH (1 << 23)#define LED_HW_PLUGGABLE (1 << 24)#define LED_DEV_CAP_COLOR (1 << 25) +#define LED_DEV_CAP_RGB (1 << 26)Why two flags - COLOR and RGB? Their meaning is ambiguous.
Quoted 5 lines
There might be color LED's with HSV interface and setting the COLOR flag but not the RGB flag would support such LED's w/o additional code. Not sure whether such devices exist, but one additional flag might be a small enough price for decoupling internal color handling and RGB output conversion.
For now let's keep the code as simple as possible. We can always add the flag when needed.
Quoted 7 lines
Please also rename
LED_DEV_CAP_COLORtoLED_DEV_CAP_HSV.We'll also have to change the type of brightness and
max_brightnessproperties of structled_classdevto u32. By default it is signed int type, but using -fshort-enums GCC option causes the smallest possible type to be used [1].
Interesting. I think we want to avoid having to change existing drivers, therefore the callbacks in struct led_classdev will have to take enum led_brightness arguments also further on.
But we need it to be at least 32 bit for the hsv extension. To ensure this (even if short-enums is used) we could add a dummy value of 0x70000000 to the enum. What do you think?
Quoted 17 lines
Quoted 14 lines
Quoted 7 lines
Quoted 5 lines
include/linux/leds.h +0
-0/* Set LED brightness level * Must not sleep. Use brightness_set_blocking for drivers Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-leds" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html[1] http://www.gnu.org/software/gnu-c-manual/gnu-c-manual.html#Enumerations
Quoted 149 lines · Heiner Kallweit
Am 15.02.2016 um 11:19 schrieb Jacek Anaszewski:
Quoted 141 lines · Heiner Kallweit
Quoted 73 lines
Am 12.02.2016 um 17:12 schrieb Jacek Anaszewski:
Quoted 63 lines · Heiner Kallweit
Quoted 62 lines
Add support for RGB LED's. Flag
LED_DEV_CAP_RGBis used to instruct the core to convert HSV to RGB on output.Signed-off-by Heiner Kallweit <[email protected]>--- drivers/leds/led-class.c | 3 +++ drivers/leds/led-core.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++- include/linux/leds.h | 1 + 3 files changed, 51 insertions(+), 1 deletion(-)drivers/leds/led-class.c +3
-0@@ -193,6 +193,9 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)char name[64 ];int ret; +if (led_cdev->flags &LED_DEV_CAP_RGB ) + led_cdev->flags |=LED_DEV_CAP_COLOR ; + ret =led_classdev_next_name (led_cdev->name, name,sizeof (name));if (ret <0 )return ret;drivers/leds/led-core.c +30
-0@@ -45,12 +45,50 @@ static inline enum led_brightness to_hsv(struct led_classdev *cdev,return ret |min (value &LED_BRIGHTNESS_MASK , cdev->max_brightness); } +static enum led_brightness hsv_to_rgb (enum led_brightness hsv) +{ +int h =min_t (int, (hsv >>16 ) &0xff ,251 ); +int s = (hsv >>8 ) &0xff ; +int v = hsv &0xff ; +int f, p, q, t, r, g, b; + +if (!v) +return 0 ; +if (!s) +return (v <<16 ) + (v <<8 ) + v; + + f =DIV_ROUND_CLOSEST ((h %42 ) *255 ,42 ); + p = v -DIV_ROUND_CLOSEST (s * v,255 ); + q = v -DIV_ROUND_CLOSEST (f * s * v,255 *255 ); + t = v -DIV_ROUND_CLOSEST ((255 - f) * s * v,255 *255 ); + +switch (h /42 ) { +case 0 : + r = v; g = t; b = p;break ; +case 1 : + r = q; g = v; b = p;break ; +case 2 : + r = p; g = v; b = t;break ; +case 3 : + r = p; g = q; b = v;break ; +case 4 : + r = t; g = p; b = v;break ; +case 5 : + r = v; g = p; b = q;break ;Quoted 9 lines
drivers/leds/led-core.c static inline enum led_brightness to_hsv(struct led_classdev *cdev, · line 82
Quoted 5 lines
drivers/leds/led-core.c +5
-0+ } + +return (r <<16 ) + (g <<8 ) + b; +} +This can be moved to
led-hvs-core.c. Could you also share a reference to the algorithm of hsv -> rgb conversion you implemented here?Quoted 5 lines
OK, will move it. The HSV -> RGB algorithm is the one from Gonzalez / Woods and I took it from the German wikipedia.
https://de.wikipedia.org/wiki/HSV-Farbraum#Umrechnung_HSV_in_RGB
The calculation was slightly adjusted to deal with intervals 0-255 instead of [0,1] and for hue 0-251 instead of [0, 360)Thanks for the link. It is possible that I'll have some comments to this piece of code after getting acquainted with the subject, but it may take a while.
Quoted 48 lines
Quoted 39 lines
Quoted 39 lines
drivers/leds/led-core.c +12
-1static int led_set_output (struct led_classdev *cdev,enum led_brightness value) {if (!cdev->brightness_set)return -ENOTSUPP ; +if (cdev->flags &LED_DEV_CAP_RGB ) + value =hsv_to_rgb (value); + cdev->brightness_set (cdev, value);return 0 ;@@ -62,6 +100,9 @@ static int led_set_output_blocking(struct led_classdev *cdev,if (!cdev->brightness_set_blocking)return -ENOTSUPP ; +if (cdev->flags &LED_DEV_CAP_RGB ) + value =hsv_to_rgb (value); +return cdev->brightness_set_blocking (cdev, value); }@@ -320,7 +361,12 @@ int led_update_brightness(struct led_classdev *led_cdev) {int ret =0 ; -if (led_cdev->brightness_get) { +/* +* for now reading back the color is not supported as multiple +* HSV -> RGB -> HSV conversions may distort the color due to +* rounding issues in the conversion algorithm +*/ +if (led_cdev->brightness_get && !(led_cdev->flags &LED_DEV_CAP_RGB )) { ret = led_cdev->brightness_get (led_cdev);if (ret >=0 ) { led_cdev->brightness = ret;include/linux/leds.h +0
-1--- a/include/linux/leds.hQuoted 9 lines
include/linux/leds.h struct led_classdev { · line 54
Quoted 6 lines
+++
b/include/linux/leds.hinclude/linux/leds.h +1
-0@@ -51,6 +51,7 @@ struct led_classdev {#define LED_DEV_CAP_FLASH (1 << 23)#define LED_HW_PLUGGABLE (1 << 24)#define LED_DEV_CAP_COLOR (1 << 25) +#define LED_DEV_CAP_RGB (1 << 26)Why two flags - COLOR and RGB? Their meaning is ambiguous.
Quoted 5 lines
There might be color LED's with HSV interface and setting the COLOR flag but not the RGB flag would support such LED's w/o additional code. Not sure whether such devices exist, but one additional flag might be a small enough price for decoupling internal color handling and RGB output conversion.
For now let's keep the code as simple as possible. We can always add the flag when needed.
Quoted 7 lines
Please also rename
LED_DEV_CAP_COLORtoLED_DEV_CAP_HSV.We'll also have to change the type of brightness and
max_brightnessproperties of structled_classdevto u32. By default it is signed int type, but using -fshort-enums GCC option causes the smallest possible type to be used [1].
Quoted 6 lines
Interesting. I think we want to avoid having to change existing drivers, therefore the callbacks in struct
led_classdevwill have to take enumled_brightnessarguments also further on.
But we need it to be at least 32 bit for the hsv extension. To ensure this (even if short-enums is used) we could add a dummy value of0x70000000to the enum. What do you think?
Originally the type was int. It was changed to enum led_brightness with patch [1], but it wasn't a good move, which turns out now. With -fshort-enums and current definition of enum led_brightness gcc can choose char for brightness type.
This is clear bug because default maximum allowed brightness value can be overridden with max_brightness property. With -fshort-enums we end up with truncated brightness for levels over 255. Brightness value can be truncated also when passing to LED API.
I agree that adding an enum value requiring four bytes seems to be the best solution. I'd prefer that the value was 0xffffffff, and a suitable comment should be placed next to it, explaining why LED_FULL is not the last value in the enum.
[1] http://www.spinics.net/lists/linux-leds/msg02070.html
Signature · 2 lines
Best regards, Jacek Anaszewski