Re: [PATCH 2/2] leds: core: add support for RGB LED's

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/13/2016 07:31 PM, Heiner Kallweit wrote:
Am 12.02.2016 um 17:12 schrieb Jacek Anaszewski:
On 02/07/2016 11:45 AM, Heiner Kallweit wrote:
Add support for RGB LED's. Flag LED_DEV_CAP_RGB is used to instruct
the core to convert HSV to RGB on output.

Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
---
   drivers/leds/led-class.c |  3 +++
   drivers/leds/led-core.c  | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
   include/linux/leds.h     |  1 +
   3 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 18a4558..f04efd2 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -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;
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 798e31e..6e1adc5 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -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;
+    }
+
+    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)

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.

   static 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;
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 8e7db72..24eae4b 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -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.

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



       /* Set LED brightness level
        * Must not sleep. Use brightness_set_blocking for drivers




--
To unsubscribe from this list: send the line "unsubscribe linux-leds" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
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

--
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-leds" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux