On 02/26/2016 11:36 PM, Heiner Kallweit wrote:
Am 25.02.2016 um 13:40 schrieb Jacek Anaszewski:
On 02/23/2016 09:27 PM, Heiner Kallweit wrote:
Am 19.02.2016 um 14:59 schrieb Jacek Anaszewski:
Hi Heiner,
On 02/18/2016 10:31 PM, Heiner Kallweit wrote:
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 <hkallweit1@xxxxxxxxx>
---
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(-)
diff --git a/drivers/leds/led-color-core.c b/drivers/leds/led-color-core.c
index b101f73..467beeb 100644
--- a/drivers/leds/led-color-core.c
+++ b/drivers/leds/led-color-core.c
@@ -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;
+}
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.
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.
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.
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index 525d566..255bdd7 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -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.
OK
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.
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;
diff --git a/drivers/leds/leds.h b/drivers/leds/leds.h
index 094707f..763f19a 100644
--- a/drivers/leds/leds.h
+++ b/drivers/leds/leds.h
@@ -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)
--
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
