Skip to contentSkip to conversation

leds: core: add support for RGB LED's

PATCH Superseded Heiner Kallweit 3 participants 7 messages Feb 18, 2016 – Feb 29, 2016 1 message not in the archive LED

v3 of 8 Superseded Latest is v8 →

lore
Share & tools
lore

Heiner Kallweit

author
#0 lore raw

PATCH v3 4/4 leds: core: add support for RGB LED's

Add support for RGB LED's. Flag LED_DEV_CAP_HSV is used to instruct the core to convert HSV to RGB on output.

Signed-off-by Heiner Kallweit <[email protected]>

---
v2:
- move hsv -> rgb conversion to separate file
- remove flag LED_DEV_CAP_RGB
v3:
- call led_hsv_to_rgb only if LED_DEV_CAP_HSV is set This is needed in cases when we have monochrome and color LEDs as well in a system.

---
 drivers/leds/led-color-core.c | 35 +++++++++++++++++++++++++++++++++++
 drivers/leds/led-core.c       | 17 ++++++++++++++++-
 drivers/leds/leds.h           |  1 +
 3 files changed, 52 insertions(+), 1 deletion(-)
drivers/leds/led-color-core.c +35 -0
index b101f73..467beeb 100644
@@ -31,3 +31,38 @@ enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
 	return brightness |
 	       min(value & LED_BRIGHTNESS_MASK, led_cdev->max_brightness);
 }
+
+enum led_brightness led_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;
+}
drivers/leds/led-core.c +16 -1
index 525d566..255bdd7 100644
@@ -31,6 +31,11 @@ static int __led_set_brightness(struct led_classdev *led_cdev,
 	if (!led_cdev->brightness_set)
 		return -ENOTSUPP;
 
+#if IS_ENABLED(CONFIG_LEDS_COLOR)
+	if (led_cdev->flags & LED_DEV_CAP_HSV)
+		value = led_hsv_to_rgb(value);
+#endif
+
 	led_cdev->brightness_set(led_cdev, value);
 
 	return 0;

@@ -42,6 +47,11 @@ static int __led_set_brightness_blocking(struct led_classdev *led_cdev,
 	if (!led_cdev->brightness_set_blocking)
 		return -ENOTSUPP;
 
+#if IS_ENABLED(CONFIG_LEDS_COLOR)
+	if (led_cdev->flags & LED_DEV_CAP_HSV)
+		value = led_hsv_to_rgb(value);
+#endif
+
 	return led_cdev->brightness_set_blocking(led_cdev, value);
 }
 

@@ -294,7 +304,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_HSV)) {
 		ret = led_cdev->brightness_get(led_cdev);
 		if (ret >= 0) {
 			led_cdev->brightness = ret;
drivers/leds/leds.h +1 -0
index 094707f..763f19a 100644
@@ -37,6 +37,7 @@ void led_set_brightness_nosleep(struct led_classdev *led_cdev,
 #if IS_ENABLED(CONFIG_LEDS_COLOR)
 enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
 					    enum led_brightness value);
+enum led_brightness led_hsv_to_rgb(enum led_brightness hsv);
 #else
 static inline enum led_brightness led_confine_brightness(
 		struct led_classdev *led_cdev, enum led_brightness value)
Signature · 1 line
2.7.1
#1 lore raw

Hi Heiner,

Quoted 51 lines · Heiner Kallweit

Add support for RGB LED's. Flag LED_DEV_CAP_HSV is used to instruct the core to convert HSV to RGB on output.

Signed-off-by Heiner Kallweit <[email protected]>

---
v2:
- move hsv -> rgb conversion to separate file
- remove flag LED_DEV_CAP_RGB
v3:
- call led_hsv_to_rgb only if LED_DEV_CAP_HSV is set This is needed in cases when we have monochrome and color LEDs as well in a system.

---
  drivers/leds/led-color-core.c | 35 +++++++++++++++++++++++++++++++++++
  drivers/leds/led-core.c       | 17 ++++++++++++++++-
  drivers/leds/leds.h           |  1 +
  3 files changed, 52 insertions(+), 1 deletion(-)
drivers/leds/led-color-core.c +25 -0
index b101f73..467beeb 100644
@@ -31,3 +31,38 @@ enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
  	return brightness |
  	       min(value & LED_BRIGHTNESS_MASK, led_cdev->max_brightness);
  }
+
+enum led_brightness led_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;

drivers/leds/led-color-core.c enum led_brightness led_confine_brightness(struct led_classdev *led_cdev, · line 68

Quoted 10 lines
drivers/leds/led-color-core.c +10 -0
+	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;
+}

Do you plan to add a driver using Color LEDs Support? If so, it would be good to add it to this patch set, which would make testing this code easier.

drivers/leds/led-core.c static int __led_set_brightness(struct led_classdev *led_cdev, · line 38

Quoted 13 lines
drivers/leds/led-core.c +5 -0
index 525d566..255bdd7 100644
@@ -31,6 +31,11 @@ static int __led_set_brightness(struct led_classdev *led_cdev,
  	if (!led_cdev->brightness_set)
  		return -ENOTSUPP;
 
+#if IS_ENABLED(CONFIG_LEDS_COLOR)
+	if (led_cdev->flags & LED_DEV_CAP_HSV)
+		value = led_hsv_to_rgb(value);
+#endif
+

Please drop this changes. led_hsv_to_rgb() can be used internally by RGB LED class drivers.

drivers/leds/led-core.c static int __led_set_brightness_blocking(struct led_classdev *led_cdev, · line 54

Quoted 12 lines
drivers/leds/led-core.c +5 -0
  	led_cdev->brightness_set(led_cdev, value);
 
  	return 0;

@@ -42,6 +47,11 @@ static int __led_set_brightness_blocking(struct led_classdev *led_cdev,
  	if (!led_cdev->brightness_set_blocking)
  		return -ENOTSUPP;
 
+#if IS_ENABLED(CONFIG_LEDS_COLOR)
+	if (led_cdev->flags & LED_DEV_CAP_HSV)
+		value = led_hsv_to_rgb(value);
+#endif
+

Ditto.

Quoted 30 lines
drivers/leds/led-core.c +6 -1
  	return led_cdev->brightness_set_blocking(led_cdev, value);
  }
 

@@ -294,7 +304,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_HSV)) {
  		ret = led_cdev->brightness_get(led_cdev);
  		if (ret >= 0) {
  			led_cdev->brightness = ret;
drivers/leds/leds.h +1 -0
index 094707f..763f19a 100644
@@ -37,6 +37,7 @@ void led_set_brightness_nosleep(struct led_classdev *led_cdev,
  #if IS_ENABLED(CONFIG_LEDS_COLOR)
  enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
  					    enum led_brightness value);
+enum led_brightness led_hsv_to_rgb(enum led_brightness hsv);
  #else
  static inline enum led_brightness led_confine_brightness(
  		struct led_classdev *led_cdev, enum led_brightness value)
Signature · 2 lines
Best regards,
Jacek Anaszewski
author
#2 lore raw

Am 19.02.2016 um 14:59 schrieb Jacek Anaszewski:

Quoted 60 lines

Hi Heiner,

Quoted 57 lines · Heiner Kallweit

Add support for RGB LED's. Flag LED_DEV_CAP_HSV is used to instruct the core to convert HSV to RGB on output.

Signed-off-by Heiner Kallweit <[email protected]>

---
v2:
- move hsv -> rgb conversion to separate file
- remove flag LED_DEV_CAP_RGB
v3:
- call led_hsv_to_rgb only if LED_DEV_CAP_HSV is set This is needed in cases when we have monochrome and color LEDs as well in a system.

---
  drivers/leds/led-color-core.c | 35 +++++++++++++++++++++++++++++++++++
  drivers/leds/led-core.c       | 17 ++++++++++++++++-
  drivers/leds/leds.h           |  1 +
  3 files changed, 52 insertions(+), 1 deletion(-)
drivers/leds/led-color-core.c +31 -0
index b101f73..467beeb 100644
@@ -31,3 +31,38 @@ enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
      return brightness |
             min(value & LED_BRIGHTNESS_MASK, led_cdev->max_brightness);
  }
+
+enum led_brightness led_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-color-core.c enum led_brightness led_confine_brightness(struct led_classdev *led_cdev, · line 68

Quoted 4 lines
drivers/leds/led-color-core.c +4 -0
+    }
+
+    return (r << 16) + (g << 8) + b;
+}

Do you plan to add a driver using Color LEDs Support? If so, it would be good to add it to this patch set, which would make testing this code easier.

I own a Thingm Blink(1) dual RGB LED USB device which I use for testing. With the RGB extension in the kernel the driver could be significantly simplified so that the changes are more or less a rewrite of the driver.
Because the USB device uses a HID interface the driver currently is located under drivers/hid. IMHO that's questionable because the driver implements LED functionality only. Moving this driver to drivers/leds could be an option. When sending the patches for this driver to you for testing I'll set the maintainer of the HID subsystem on cc.

Quoted 8 lines
Quoted 8 lines
drivers/leds/led-core.c +0 -0
index 525d566..255bdd7 100644
@@ -31,6 +31,11 @@ static int __led_set_brightness(struct led_classdev *led_cdev,
      if (!led_cdev->brightness_set)
          return -ENOTSUPP;
Quoted 9 lines

drivers/leds/led-core.c static int __led_set_brightness(struct led_classdev *led_cdev, · line 38

Quoted 5 lines
drivers/leds/led-core.c +5 -0
+#if IS_ENABLED(CONFIG_LEDS_COLOR)
+    if (led_cdev->flags & LED_DEV_CAP_HSV)
+        value = led_hsv_to_rgb(value);
+#endif
+

Please drop this changes. led_hsv_to_rgb() can be used internally by RGB LED class drivers.

OK

Quoted 47 lines

drivers/leds/led-core.c static int __led_set_brightness_blocking(struct led_classdev *led_cdev, · line 54

Quoted 12 lines
drivers/leds/led-core.c +5 -0
      led_cdev->brightness_set(led_cdev, value);
 
      return 0;

@@ -42,6 +47,11 @@ static int __led_set_brightness_blocking(struct led_classdev *led_cdev,
      if (!led_cdev->brightness_set_blocking)
          return -ENOTSUPP;
 
+#if IS_ENABLED(CONFIG_LEDS_COLOR)
+    if (led_cdev->flags & LED_DEV_CAP_HSV)
+        value = led_hsv_to_rgb(value);
+#endif
+

Ditto.

Quoted 30 lines
drivers/leds/led-core.c +6 -1
      return led_cdev->brightness_set_blocking(led_cdev, value);
  }
 

@@ -294,7 +304,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_HSV)) {
          ret = led_cdev->brightness_get(led_cdev);
          if (ret >= 0) {
              led_cdev->brightness = ret;
drivers/leds/leds.h +1 -0
index 094707f..763f19a 100644
@@ -37,6 +37,7 @@ void led_set_brightness_nosleep(struct led_classdev *led_cdev,
  #if IS_ENABLED(CONFIG_LEDS_COLOR)
  enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
                          enum led_brightness value);
+enum led_brightness led_hsv_to_rgb(enum led_brightness hsv);
  #else
  static inline enum led_brightness led_confine_brightness(
          struct led_classdev *led_cdev, enum led_brightness value)
#3 lore raw
Quoted 70 lines · Heiner Kallweit

Am 19.02.2016 um 14:59 schrieb Jacek Anaszewski:

Quoted 60 lines

Hi Heiner,

Quoted 57 lines · Heiner Kallweit

Add support for RGB LED's. Flag LED_DEV_CAP_HSV is used to instruct the core to convert HSV to RGB on output.

Signed-off-by Heiner Kallweit <[email protected]>

---
v2:
- move hsv -> rgb conversion to separate file
- remove flag LED_DEV_CAP_RGB
v3:
- call led_hsv_to_rgb only if LED_DEV_CAP_HSV is set This is needed in cases when we have monochrome and color LEDs as well in a system.

---
   drivers/leds/led-color-core.c | 35 +++++++++++++++++++++++++++++++++++
   drivers/leds/led-core.c       | 17 ++++++++++++++++-
   drivers/leds/leds.h           |  1 +
   3 files changed, 52 insertions(+), 1 deletion(-)
drivers/leds/led-color-core.c +31 -0
index b101f73..467beeb 100644
@@ -31,3 +31,38 @@ enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
       return brightness |
              min(value & LED_BRIGHTNESS_MASK, led_cdev->max_brightness);
   }
+
+enum led_brightness led_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-color-core.c enum led_brightness led_confine_brightness(struct led_classdev *led_cdev, · line 68

Quoted 4 lines
drivers/leds/led-color-core.c +4 -0
+    }
+
+    return (r << 16) + (g << 8) + b;
+}

Do you plan to add a driver using Color LEDs Support? If so, it would be good to add it to this patch set, which would make testing this code easier.

Quoted 8 lines

I own a Thingm Blink(1) dual RGB LED USB device which I use for testing. With the RGB extension in the kernel the driver could be significantly simplified so that the changes are more or less a rewrite of the driver.
Because the USB device uses a HID interface the driver currently is located under drivers/hid. IMHO that's questionable because the driver implements LED functionality only. Moving this driver to drivers/leds could be an option. When sending the patches for this driver to you for testing I'll set the maintainer of the HID subsystem on cc.

But it will be still USB driver. Please target the driver for usb subsystem and add myself on cc.

Quoted 69 lines
Quoted 8 lines
Quoted 8 lines
drivers/leds/led-core.c +0 -0
index 525d566..255bdd7 100644
@@ -31,6 +31,11 @@ static int __led_set_brightness(struct led_classdev *led_cdev,
       if (!led_cdev->brightness_set)
           return -ENOTSUPP;
Quoted 9 lines

drivers/leds/led-core.c static int __led_set_brightness(struct led_classdev *led_cdev, · line 38

Quoted 5 lines
drivers/leds/led-core.c +5 -0
+#if IS_ENABLED(CONFIG_LEDS_COLOR)
+    if (led_cdev->flags & LED_DEV_CAP_HSV)
+        value = led_hsv_to_rgb(value);
+#endif
+

Please drop this changes. led_hsv_to_rgb() can be used internally by RGB LED class drivers.

OK

Quoted 47 lines

drivers/leds/led-core.c static int __led_set_brightness_blocking(struct led_classdev *led_cdev, · line 54

Quoted 12 lines
drivers/leds/led-core.c +5 -0
       led_cdev->brightness_set(led_cdev, value);
 
       return 0;

@@ -42,6 +47,11 @@ static int __led_set_brightness_blocking(struct led_classdev *led_cdev,
       if (!led_cdev->brightness_set_blocking)
           return -ENOTSUPP;
 
+#if IS_ENABLED(CONFIG_LEDS_COLOR)
+    if (led_cdev->flags & LED_DEV_CAP_HSV)
+        value = led_hsv_to_rgb(value);
+#endif
+

Ditto.

Quoted 30 lines
drivers/leds/led-core.c +6 -1
       return led_cdev->brightness_set_blocking(led_cdev, value);
   }
 

@@ -294,7 +304,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_HSV)) {
           ret = led_cdev->brightness_get(led_cdev);
           if (ret >= 0) {
               led_cdev->brightness = ret;
drivers/leds/leds.h +1 -0
index 094707f..763f19a 100644
@@ -37,6 +37,7 @@ void led_set_brightness_nosleep(struct led_classdev *led_cdev,
   #if IS_ENABLED(CONFIG_LEDS_COLOR)
   enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
                           enum led_brightness value);
+enum led_brightness led_hsv_to_rgb(enum led_brightness hsv);
   #else
   static inline enum led_brightness led_confine_brightness(
           struct led_classdev *led_cdev, enum led_brightness value)
Signature · 2 lines
Best regards,
Jacek Anaszewski
author
#4 lore raw

Am 25.02.2016 um 13:40 schrieb Jacek Anaszewski:

Quoted 80 lines · Heiner Kallweit
Quoted 70 lines

Am 19.02.2016 um 14:59 schrieb Jacek Anaszewski:

Quoted 60 lines

Hi Heiner,

Quoted 57 lines · Heiner Kallweit

Add support for RGB LED's. Flag LED_DEV_CAP_HSV is used to instruct the core to convert HSV to RGB on output.

Signed-off-by Heiner Kallweit <[email protected]>

---
v2:
- move hsv -> rgb conversion to separate file
- remove flag LED_DEV_CAP_RGB
v3:
- call led_hsv_to_rgb only if LED_DEV_CAP_HSV is set This is needed in cases when we have monochrome and color LEDs as well in a system.

---
   drivers/leds/led-color-core.c | 35 +++++++++++++++++++++++++++++++++++
   drivers/leds/led-core.c       | 17 ++++++++++++++++-
   drivers/leds/leds.h           |  1 +
   3 files changed, 52 insertions(+), 1 deletion(-)
drivers/leds/led-color-core.c +31 -0
index b101f73..467beeb 100644
@@ -31,3 +31,38 @@ enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
       return brightness |
              min(value & LED_BRIGHTNESS_MASK, led_cdev->max_brightness);
   }
+
+enum led_brightness led_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-color-core.c enum led_brightness led_confine_brightness(struct led_classdev *led_cdev, · line 68

Quoted 4 lines
drivers/leds/led-color-core.c +4 -0
+    }
+
+    return (r << 16) + (g << 8) + b;
+}

Do you plan to add a driver using Color LEDs Support? If so, it would be good to add it to this patch set, which would make testing this code easier.

Quoted 8 lines

I own a Thingm Blink(1) dual RGB LED USB device which I use for testing. With the RGB extension in the kernel the driver could be significantly simplified so that the changes are more or less a rewrite of the driver.
Because the USB device uses a HID interface the driver currently is located under drivers/hid. IMHO that's questionable because the driver implements LED functionality only. Moving this driver to drivers/leds could be an option. When sending the patches for this driver to you for testing I'll set the maintainer of the HID subsystem on cc.

Quoted 3 lines

But it will be still USB driver. Please target the driver for usb subsystem and add myself on cc.

I'm afraid the maintainer of the HID subsystem will reject the driver patches because he can't even apply them as the RGB LED extension is not yet in his tree. Would it make sense that I send the driver patches to you first for testing the RGB extension (not as official patches)?
If everything goes well and the RGB extension shows up in the linux-next tree eventually then the driver changes can be submitted officially to the HID maintainer.

Quoted 71 lines
Quoted 69 lines
Quoted 8 lines
Quoted 8 lines
drivers/leds/led-core.c +0 -0
index 525d566..255bdd7 100644
@@ -31,6 +31,11 @@ static int __led_set_brightness(struct led_classdev *led_cdev,
       if (!led_cdev->brightness_set)
           return -ENOTSUPP;
Quoted 9 lines

drivers/leds/led-core.c static int __led_set_brightness(struct led_classdev *led_cdev, · line 38

Quoted 5 lines
drivers/leds/led-core.c +5 -0
+#if IS_ENABLED(CONFIG_LEDS_COLOR)
+    if (led_cdev->flags & LED_DEV_CAP_HSV)
+        value = led_hsv_to_rgb(value);
+#endif
+

Please drop this changes. led_hsv_to_rgb() can be used internally by RGB LED class drivers.

OK

Quoted 47 lines

drivers/leds/led-core.c static int __led_set_brightness_blocking(struct led_classdev *led_cdev, · line 54

Quoted 12 lines
drivers/leds/led-core.c +5 -0
       led_cdev->brightness_set(led_cdev, value);
 
       return 0;

@@ -42,6 +47,11 @@ static int __led_set_brightness_blocking(struct led_classdev *led_cdev,
       if (!led_cdev->brightness_set_blocking)
           return -ENOTSUPP;
 
+#if IS_ENABLED(CONFIG_LEDS_COLOR)
+    if (led_cdev->flags & LED_DEV_CAP_HSV)
+        value = led_hsv_to_rgb(value);
+#endif
+

Ditto.

Quoted 30 lines
drivers/leds/led-core.c +6 -1
       return led_cdev->brightness_set_blocking(led_cdev, value);
   }
 

@@ -294,7 +304,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_HSV)) {
           ret = led_cdev->brightness_get(led_cdev);
           if (ret >= 0) {
               led_cdev->brightness = ret;
drivers/leds/leds.h +1 -0
index 094707f..763f19a 100644
@@ -37,6 +37,7 @@ void led_set_brightness_nosleep(struct led_classdev *led_cdev,
   #if IS_ENABLED(CONFIG_LEDS_COLOR)
   enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
                           enum led_brightness value);
+enum led_brightness led_hsv_to_rgb(enum led_brightness hsv);
   #else
   static inline enum led_brightness led_confine_brightness(
           struct led_classdev *led_cdev, enum led_brightness value)
#5 lore raw
Quoted 84 lines · Heiner Kallweit

Am 25.02.2016 um 13:40 schrieb Jacek Anaszewski:

Quoted 80 lines · Heiner Kallweit
Quoted 70 lines

Am 19.02.2016 um 14:59 schrieb Jacek Anaszewski:

Quoted 60 lines

Hi Heiner,

Quoted 57 lines · Heiner Kallweit

Add support for RGB LED's. Flag LED_DEV_CAP_HSV is used to instruct the core to convert HSV to RGB on output.

Signed-off-by Heiner Kallweit <[email protected]>

---
v2:
- move hsv -> rgb conversion to separate file
- remove flag LED_DEV_CAP_RGB
v3:
- call led_hsv_to_rgb only if LED_DEV_CAP_HSV is set This is needed in cases when we have monochrome and color LEDs as well in a system.

---
    drivers/leds/led-color-core.c | 35 +++++++++++++++++++++++++++++++++++
    drivers/leds/led-core.c       | 17 ++++++++++++++++-
    drivers/leds/leds.h           |  1 +
    3 files changed, 52 insertions(+), 1 deletion(-)
drivers/leds/led-color-core.c +31 -0
index b101f73..467beeb 100644
@@ -31,3 +31,38 @@ enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
        return brightness |
               min(value & LED_BRIGHTNESS_MASK, led_cdev->max_brightness);
    }
+
+enum led_brightness led_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-color-core.c enum led_brightness led_confine_brightness(struct led_classdev *led_cdev, · line 68

Quoted 4 lines
drivers/leds/led-color-core.c +4 -0
+    }
+
+    return (r << 16) + (g << 8) + b;
+}

Do you plan to add a driver using Color LEDs Support? If so, it would be good to add it to this patch set, which would make testing this code easier.

Quoted 8 lines

I own a Thingm Blink(1) dual RGB LED USB device which I use for testing. With the RGB extension in the kernel the driver could be significantly simplified so that the changes are more or less a rewrite of the driver.
Because the USB device uses a HID interface the driver currently is located under drivers/hid. IMHO that's questionable because the driver implements LED functionality only. Moving this driver to drivers/leds could be an option. When sending the patches for this driver to you for testing I'll set the maintainer of the HID subsystem on cc.

Quoted 3 lines

But it will be still USB driver. Please target the driver for usb subsystem and add myself on cc.

Quoted 6 lines

I'm afraid the maintainer of the HID subsystem will reject the driver patches because he can't even apply them as the RGB LED extension is not yet in his tree. Would it make sense that I send the driver patches to you first for testing the RGB extension (not as official patches)?
If everything goes well and the RGB extension shows up in the linux-next tree eventually then the driver changes can be submitted officially to the HID maintainer.

USB maintainer could only ack it and I would merge it through the LED tree. I propose to cc the whole patch set also to linux-usb list, which would also have the benefit that more people could look at the whole idea of using hsv interface for RGB LED. The traffic on linux-leds isn't too heavy. Please cc the patch set also to linux-kernel next time.

Quoted 74 lines
Quoted 71 lines
Quoted 69 lines
Quoted 8 lines
Quoted 8 lines
drivers/leds/led-core.c +0 -0
index 525d566..255bdd7 100644
@@ -31,6 +31,11 @@ static int __led_set_brightness(struct led_classdev *led_cdev,
        if (!led_cdev->brightness_set)
            return -ENOTSUPP;
Quoted 9 lines

drivers/leds/led-core.c static int __led_set_brightness(struct led_classdev *led_cdev, · line 38

Quoted 5 lines
drivers/leds/led-core.c +5 -0
+#if IS_ENABLED(CONFIG_LEDS_COLOR)
+    if (led_cdev->flags & LED_DEV_CAP_HSV)
+        value = led_hsv_to_rgb(value);
+#endif
+

Please drop this changes. led_hsv_to_rgb() can be used internally by RGB LED class drivers.

OK

Quoted 47 lines

drivers/leds/led-core.c static int __led_set_brightness_blocking(struct led_classdev *led_cdev, · line 54

Quoted 12 lines
drivers/leds/led-core.c +5 -0
        led_cdev->brightness_set(led_cdev, value);
 
        return 0;

@@ -42,6 +47,11 @@ static int __led_set_brightness_blocking(struct led_classdev *led_cdev,
        if (!led_cdev->brightness_set_blocking)
            return -ENOTSUPP;
 
+#if IS_ENABLED(CONFIG_LEDS_COLOR)
+    if (led_cdev->flags & LED_DEV_CAP_HSV)
+        value = led_hsv_to_rgb(value);
+#endif
+

Ditto.

Quoted 30 lines
drivers/leds/led-core.c +6 -1
        return led_cdev->brightness_set_blocking(led_cdev, value);
    }
 

@@ -294,7 +304,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_HSV)) {
            ret = led_cdev->brightness_get(led_cdev);
            if (ret >= 0) {
                led_cdev->brightness = ret;
drivers/leds/leds.h +1 -0
index 094707f..763f19a 100644
@@ -37,6 +37,7 @@ void led_set_brightness_nosleep(struct led_classdev *led_cdev,
    #if IS_ENABLED(CONFIG_LEDS_COLOR)
    enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
                            enum led_brightness value);
+enum led_brightness led_hsv_to_rgb(enum led_brightness hsv);
    #else
    static inline enum led_brightness led_confine_brightness(
            struct led_classdev *led_cdev, enum led_brightness value)
Signature · 2 lines
Best regards,
Jacek Anaszewski
#7 lore raw
In reply to a message not in the archive

Am 29.02.2016 um 10:57 schrieb Jacek Anaszewski:

Quoted 92 lines · Heiner Kallweit
Quoted 84 lines

Am 25.02.2016 um 13:40 schrieb Jacek Anaszewski:

Quoted 80 lines · Heiner Kallweit
Quoted 70 lines

Am 19.02.2016 um 14:59 schrieb Jacek Anaszewski:

Quoted 60 lines

Hi Heiner,

Quoted 57 lines · Heiner Kallweit

Add support for RGB LED's. Flag LED_DEV_CAP_HSV is used to instruct the core to convert HSV to RGB on output.

Signed-off-by Heiner Kallweit <[email protected]>

---
v2:
- move hsv -> rgb conversion to separate file
- remove flag LED_DEV_CAP_RGB
v3:
- call led_hsv_to_rgb only if LED_DEV_CAP_HSV is set This is needed in cases when we have monochrome and color LEDs as well in a system.

---
    drivers/leds/led-color-core.c | 35 +++++++++++++++++++++++++++++++++++
    drivers/leds/led-core.c       | 17 ++++++++++++++++-
    drivers/leds/leds.h           |  1 +
    3 files changed, 52 insertions(+), 1 deletion(-)
drivers/leds/led-color-core.c +31 -0
index b101f73..467beeb 100644
@@ -31,3 +31,38 @@ enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
        return brightness |
               min(value & LED_BRIGHTNESS_MASK, led_cdev->max_brightness);
    }
+
+enum led_brightness led_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-color-core.c enum led_brightness led_confine_brightness(struct led_classdev *led_cdev, · line 68

Quoted 4 lines
drivers/leds/led-color-core.c +4 -0
+    }
+
+    return (r << 16) + (g << 8) + b;
+}

Do you plan to add a driver using Color LEDs Support? If so, it would be good to add it to this patch set, which would make testing this code easier.

Quoted 8 lines

I own a Thingm Blink(1) dual RGB LED USB device which I use for testing. With the RGB extension in the kernel the driver could be significantly simplified so that the changes are more or less a rewrite of the driver.
Because the USB device uses a HID interface the driver currently is located under drivers/hid. IMHO that's questionable because the driver implements LED functionality only. Moving this driver to drivers/leds could be an option. When sending the patches for this driver to you for testing I'll set the maintainer of the HID subsystem on cc.

Quoted 3 lines

But it will be still USB driver. Please target the driver for usb subsystem and add myself on cc.

Quoted 6 lines

I'm afraid the maintainer of the HID subsystem will reject the driver patches because he can't even apply them as the RGB LED extension is not yet in his tree. Would it make sense that I send the driver patches to you first for testing the RGB extension (not as official patches)?
If everything goes well and the RGB extension shows up in the linux-next tree eventually then the driver changes can be submitted officially to the HID maintainer.

Quoted 7 lines

USB maintainer could only ack it and I would merge it through the LED tree. I propose to cc the whole patch set also to linux-usb list, which would also have the benefit that more people could look at the whole idea of using hsv interface for RGB LED. The traffic on linux-leds isn't too heavy. Please cc the patch set also to linux-kernel next time.

OK, I cc Jiri as HID USB maintainer and will send the next version of the patch series also to linux-usb and linux-kernel.

Quoted 66 lines
Quoted 66 lines
Quoted 66 lines
Quoted 66 lines
Quoted 8 lines
Quoted 8 lines
drivers/leds/led-core.c +0 -0
index 525d566..255bdd7 100644
@@ -31,6 +31,11 @@ static int __led_set_brightness(struct led_classdev *led_cdev,
        if (!led_cdev->brightness_set)
            return -ENOTSUPP;
Quoted 9 lines

drivers/leds/led-core.c static int __led_set_brightness(struct led_classdev *led_cdev, · line 38

Quoted 5 lines
drivers/leds/led-core.c +5 -0
+#if IS_ENABLED(CONFIG_LEDS_COLOR)
+    if (led_cdev->flags & LED_DEV_CAP_HSV)
+        value = led_hsv_to_rgb(value);
+#endif
+

Please drop this changes. led_hsv_to_rgb() can be used internally by RGB LED class drivers.

OK

Quoted 47 lines

drivers/leds/led-core.c static int __led_set_brightness_blocking(struct led_classdev *led_cdev, · line 54

Quoted 12 lines
drivers/leds/led-core.c +5 -0
        led_cdev->brightness_set(led_cdev, value);
 
        return 0;

@@ -42,6 +47,11 @@ static int __led_set_brightness_blocking(struct led_classdev *led_cdev,
        if (!led_cdev->brightness_set_blocking)
            return -ENOTSUPP;
 
+#if IS_ENABLED(CONFIG_LEDS_COLOR)
+    if (led_cdev->flags & LED_DEV_CAP_HSV)
+        value = led_hsv_to_rgb(value);
+#endif
+

Ditto.

Quoted 30 lines
drivers/leds/led-core.c +6 -1
        return led_cdev->brightness_set_blocking(led_cdev, value);
    }
 

@@ -294,7 +304,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_HSV)) {
            ret = led_cdev->brightness_get(led_cdev);
            if (ret >= 0) {
                led_cdev->brightness = ret;
drivers/leds/leds.h +1 -0
index 094707f..763f19a 100644
@@ -37,6 +37,7 @@ void led_set_brightness_nosleep(struct led_classdev *led_cdev,
    #if IS_ENABLED(CONFIG_LEDS_COLOR)
    enum led_brightness led_confine_brightness(struct led_classdev *led_cdev,
                            enum led_brightness value);
+enum led_brightness led_hsv_to_rgb(enum led_brightness hsv);
    #else
    static inline enum led_brightness led_confine_brightness(
            struct led_classdev *led_cdev, enum led_brightness value)
Quoted 10 lines
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html