Export a function to convert HSV color values to RGB.
It's intended to be called by drivers for RGB LEDs.
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.
v4:
- Export led_hsv_to_rgb and let the device driver call it instead
of doing the conversion in the core
v5:
- don't ignore led_cdev->brightness_get silently if LED_DEV_CAP_RGB
is set but warn
---
drivers/leds/led-class.c | 7 +++++++
drivers/leds/led-rgb-core.c | 36 ++++++++++++++++++++++++++++++++++++
include/linux/leds.h | 8 ++++++++
3 files changed, 51 insertions(+)
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 8a3748a..a4b144e 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -193,6 +193,13 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
char name[64];
int ret;
+ /*
+ * 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
+ */
+ WARN_ON(led_cdev->flags & LED_DEV_CAP_RGB && led_cdev->brightness_get);
+
ret = led_classdev_next_name(led_cdev->name, name, sizeof(name));
if (ret < 0)
return ret;
diff --git a/drivers/leds/led-rgb-core.c b/drivers/leds/led-rgb-core.c
index f6591f1..2b18d4c 100644
--- a/drivers/leds/led-rgb-core.c
+++ b/drivers/leds/led-rgb-core.c
@@ -38,3 +38,39 @@ 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;
+}
+EXPORT_SYMBOL_GPL(led_hsv_to_rgb);
diff --git a/include/linux/leds.h b/include/linux/leds.h
index bbf24bb..82b3477 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -226,6 +226,14 @@ static inline bool led_sysfs_is_disabled(struct led_classdev *led_cdev)
return led_cdev->flags & LED_SYSFS_DISABLE;
}
+/**
+ * led_hsv_to_rgb - convert a hsv color value to rgb color model
+ * @hsv: the hsv value to convert
+ *
+ * Returns: the resulting rgb value
+ */
+enum led_brightness led_hsv_to_rgb(enum led_brightness hsv);
+
/*
* LED Triggers
*/
--
2.7.1
On 03/01/2016 10:36 PM, Heiner Kallweit wrote:
> Export a function to convert HSV color values to RGB.
> It's intended to be called by drivers for RGB LEDs.
>
> 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.
> v4:
> - Export led_hsv_to_rgb and let the device driver call it instead
> of doing the conversion in the core
> v5:
> - don't ignore led_cdev->brightness_get silently if LED_DEV_CAP_RGB
> is set but warn
> ---
> drivers/leds/led-class.c | 7 +++++++
> drivers/leds/led-rgb-core.c | 36 ++++++++++++++++++++++++++++++++++++
> include/linux/leds.h | 8 ++++++++
> 3 files changed, 51 insertions(+)
>
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 8a3748a..a4b144e 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -193,6 +193,13 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
> char name[64];
> int ret;
>
> + /*
> + * 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
> + */
Does the "for now" mean that you plan on supporting it in the future?
> + WARN_ON(led_cdev->flags & LED_DEV_CAP_RGB && led_cdev->brightness_get);
> +
Could you move this warning to the line 216?
> ret = led_classdev_next_name(led_cdev->name, name, sizeof(name));
> if (ret < 0)
> return ret;
> diff --git a/drivers/leds/led-rgb-core.c b/drivers/leds/led-rgb-core.c
> index f6591f1..2b18d4c 100644
> --- a/drivers/leds/led-rgb-core.c
> +++ b/drivers/leds/led-rgb-core.c
> @@ -38,3 +38,39 @@ 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;
> +}
> +EXPORT_SYMBOL_GPL(led_hsv_to_rgb);
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index bbf24bb..82b3477 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -226,6 +226,14 @@ static inline bool led_sysfs_is_disabled(struct led_classdev *led_cdev)
> return led_cdev->flags & LED_SYSFS_DISABLE;
> }
>
> +/**
> + * led_hsv_to_rgb - convert a hsv color value to rgb color model
> + * @hsv: the hsv value to convert
> + *
> + * Returns: the resulting rgb value
> + */
> +enum led_brightness led_hsv_to_rgb(enum led_brightness hsv);
> +
> /*
> * LED Triggers
> */
>
--
Best regards,
Jacek Anaszewski
Am 04.03.2016 um 10:05 schrieb Jacek Anaszewski:
> On 03/01/2016 10:36 PM, Heiner Kallweit wrote:
>> Export a function to convert HSV color values to RGB.
>> It's intended to be called by drivers for RGB LEDs.
>>
>> 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.
>> v4:
>> - Export led_hsv_to_rgb and let the device driver call it instead
>> of doing the conversion in the core
>> v5:
>> - don't ignore led_cdev->brightness_get silently if LED_DEV_CAP_RGB
>> is set but warn
>> ---
>> drivers/leds/led-class.c | 7 +++++++
>> drivers/leds/led-rgb-core.c | 36 ++++++++++++++++++++++++++++++++++++
>> include/linux/leds.h | 8 ++++++++
>> 3 files changed, 51 insertions(+)
>>
>> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
>> index 8a3748a..a4b144e 100644
>> --- a/drivers/leds/led-class.c
>> +++ b/drivers/leds/led-class.c
>> @@ -193,6 +193,13 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
>> char name[64];
>> int ret;
>>
>> + /*
>> + * 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
>> + */
>
> Does the "for now" mean that you plan on supporting it in the future?
>
It was meant as "until somebody has an idea how to implement the conversion
HSV -> RGB -> HSV in a way that always results in the original values".
I'll remove the "for now".
>> + WARN_ON(led_cdev->flags & LED_DEV_CAP_RGB && led_cdev->brightness_get);
>> +
>
> Could you move this warning to the line 216?
>
>> ret = led_classdev_next_name(led_cdev->name, name, sizeof(name));
>> if (ret < 0)
>> return ret;
>> diff --git a/drivers/leds/led-rgb-core.c b/drivers/leds/led-rgb-core.c
>> index f6591f1..2b18d4c 100644
>> --- a/drivers/leds/led-rgb-core.c
>> +++ b/drivers/leds/led-rgb-core.c
>> @@ -38,3 +38,39 @@ 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;
>> +}
>> +EXPORT_SYMBOL_GPL(led_hsv_to_rgb);
>> diff --git a/include/linux/leds.h b/include/linux/leds.h
>> index bbf24bb..82b3477 100644
>> --- a/include/linux/leds.h
>> +++ b/include/linux/leds.h
>> @@ -226,6 +226,14 @@ static inline bool led_sysfs_is_disabled(struct led_classdev *led_cdev)
>> return led_cdev->flags & LED_SYSFS_DISABLE;
>> }
>>
>> +/**
>> + * led_hsv_to_rgb - convert a hsv color value to rgb color model
>> + * @hsv: the hsv value to convert
>> + *
>> + * Returns: the resulting rgb value
>> + */
>> +enum led_brightness led_hsv_to_rgb(enum led_brightness hsv);
>> +
>> /*
>> * LED Triggers
>> */
>>
>
>
On Tue 2016-03-01 22:36:12, Heiner Kallweit wrote:
> Export a function to convert HSV color values to RGB.
> It's intended to be called by drivers for RGB LEDs.
>
> 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.
> v4:
> - Export led_hsv_to_rgb and let the device driver call it instead
> of doing the conversion in the core
> v5:
> - don't ignore led_cdev->brightness_get silently if LED_DEV_CAP_RGB
> is set but warn
> ---
> drivers/leds/led-class.c | 7 +++++++
> drivers/leds/led-rgb-core.c | 36 ++++++++++++++++++++++++++++++++++++
> include/linux/leds.h | 8 ++++++++
> 3 files changed, 51 insertions(+)
>
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 8a3748a..a4b144e 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -193,6 +193,13 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
> char name[64];
> int ret;
>
> + /*
> + * 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
> + */
> + WARN_ON(led_cdev->flags & LED_DEV_CAP_RGB && led_cdev->brightness_get);
> +
Backtrace is useless here, you may want to add some ()s and you don't
really want user to be causing messages in syslog this easily.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Am 29.03.2016 um 12:05 schrieb Pavel Machek:
> On Tue 2016-03-01 22:36:12, Heiner Kallweit wrote:
>> Export a function to convert HSV color values to RGB.
>> It's intended to be called by drivers for RGB LEDs.
>>
>> 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.
>> v4:
>> - Export led_hsv_to_rgb and let the device driver call it instead
>> of doing the conversion in the core
>> v5:
>> - don't ignore led_cdev->brightness_get silently if LED_DEV_CAP_RGB
>> is set but warn
>> ---
>> drivers/leds/led-class.c | 7 +++++++
>> drivers/leds/led-rgb-core.c | 36 ++++++++++++++++++++++++++++++++++++
>> include/linux/leds.h | 8 ++++++++
>> 3 files changed, 51 insertions(+)
>>
>> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
>> index 8a3748a..a4b144e 100644
>> --- a/drivers/leds/led-class.c
>> +++ b/drivers/leds/led-class.c
>> @@ -193,6 +193,13 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
>> char name[64];
>> int ret;
>>
>> + /*
>> + * 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
>> + */
>> + WARN_ON(led_cdev->flags & LED_DEV_CAP_RGB && led_cdev->brightness_get);
>> +
>
> Backtrace is useless here, you may want to add some ()s and you don't
> really want user to be causing messages in syslog this easily.
>
I agree that the backtrace doesn't provide a benefit here.
However I don't see how a user could create syslog entries.
The warn condition can be true only for drivers implementing the RGB extension
in a not supported way (by setting flag LED_DEV_CAP_RGB and implementing
brightness_get).
Pavel
>
On 03/29/2016 10:42 PM, Heiner Kallweit wrote:
> Am 29.03.2016 um 12:05 schrieb Pavel Machek:
>> On Tue 2016-03-01 22:36:12, Heiner Kallweit wrote:
>>> Export a function to convert HSV color values to RGB.
>>> It's intended to be called by drivers for RGB LEDs.
>>>
>>> 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.
>>> v4:
>>> - Export led_hsv_to_rgb and let the device driver call it instead
>>> of doing the conversion in the core
>>> v5:
>>> - don't ignore led_cdev->brightness_get silently if LED_DEV_CAP_RGB
>>> is set but warn
>>> ---
>>> drivers/leds/led-class.c | 7 +++++++
>>> drivers/leds/led-rgb-core.c | 36 ++++++++++++++++++++++++++++++++++++
>>> include/linux/leds.h | 8 ++++++++
>>> 3 files changed, 51 insertions(+)
>>>
>>> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
>>> index 8a3748a..a4b144e 100644
>>> --- a/drivers/leds/led-class.c
>>> +++ b/drivers/leds/led-class.c
>>> @@ -193,6 +193,13 @@ int led_classdev_register(struct device *parent, struct led_classdev *led_cdev)
>>> char name[64];
>>> int ret;
>>>
>>> + /*
>>> + * 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
>>> + */
>>> + WARN_ON(led_cdev->flags & LED_DEV_CAP_RGB && led_cdev->brightness_get);
>>> +
>>
>> Backtrace is useless here, you may want to add some ()s and you don't
>> really want user to be causing messages in syslog this easily.
>>
> I agree that the backtrace doesn't provide a benefit here.
I requested it to provide a verbose notification for a LED RGB class
driver developer, in case they try to implement a brightness_get
op, which would be harmful in case of HSV interface for RGB LEDs.
> However I don't see how a user could create syslog entries.
> The warn condition can be true only for drivers implementing the RGB extension
> in a not supported way (by setting flag LED_DEV_CAP_RGB and implementing
> brightness_get).
>
> Pavel
>>
>
>
>
--
Best regards,
Jacek Anaszewski