All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 4/4] leds: core: add support for RGB LED's
@ 2016-03-01 21:36 Heiner Kallweit
  2016-03-04  9:05 ` Jacek Anaszewski
       [not found] ` <56D60B4C.5000506-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 2 replies; 7+ messages in thread
From: Heiner Kallweit @ 2016-03-01 21:36 UTC (permalink / raw)
  To: Jacek Anaszewski; +Cc: linux-leds, Benjamin Tissoires, linux-usb, linux-kernel

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 <hkallweit1@gmail.com>
---
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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 4/4] leds: core: add support for RGB LED's
  2016-03-01 21:36 [PATCH v5 4/4] leds: core: add support for RGB LED's Heiner Kallweit
@ 2016-03-04  9:05 ` Jacek Anaszewski
  2016-03-04 21:21   ` Heiner Kallweit
       [not found] ` <56D60B4C.5000506-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  1 sibling, 1 reply; 7+ messages in thread
From: Jacek Anaszewski @ 2016-03-04  9:05 UTC (permalink / raw)
  To: Heiner Kallweit; +Cc: linux-leds, Benjamin Tissoires, linux-usb, linux-kernel

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 <hkallweit1@gmail.com>
> ---
> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 4/4] leds: core: add support for RGB LED's
  2016-03-04  9:05 ` Jacek Anaszewski
@ 2016-03-04 21:21   ` Heiner Kallweit
  0 siblings, 0 replies; 7+ messages in thread
From: Heiner Kallweit @ 2016-03-04 21:21 UTC (permalink / raw)
  To: Jacek Anaszewski; +Cc: linux-leds, Benjamin Tissoires, linux-usb, linux-kernel

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 <hkallweit1@gmail.com>
>> ---
>> 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
>>    */
>>
> 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 4/4] leds: core: add support for RGB LED's
  2016-03-01 21:36 [PATCH v5 4/4] leds: core: add support for RGB LED's Heiner Kallweit
@ 2016-03-29 10:05     ` Pavel Machek
       [not found] ` <56D60B4C.5000506-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  1 sibling, 0 replies; 7+ messages in thread
From: Pavel Machek @ 2016-03-29 10:05 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Jacek Anaszewski, linux-leds-u79uwXL29TY76Z2rM5mHXA,
	Benjamin Tissoires, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

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 <hkallweit1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 4/4] leds: core: add support for RGB LED's
@ 2016-03-29 10:05     ` Pavel Machek
  0 siblings, 0 replies; 7+ messages in thread
From: Pavel Machek @ 2016-03-29 10:05 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Jacek Anaszewski, linux-leds, Benjamin Tissoires, linux-usb,
	linux-kernel

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 <hkallweit1@gmail.com>
> ---
> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 4/4] leds: core: add support for RGB LED's
  2016-03-29 10:05     ` Pavel Machek
  (?)
@ 2016-03-29 20:42     ` Heiner Kallweit
  2016-03-30  8:00       ` Jacek Anaszewski
  -1 siblings, 1 reply; 7+ messages in thread
From: Heiner Kallweit @ 2016-03-29 20:42 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Jacek Anaszewski, linux-leds, Benjamin Tissoires, linux-usb,
	linux-kernel

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 <hkallweit1@gmail.com>
>> ---
>> 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
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v5 4/4] leds: core: add support for RGB LED's
  2016-03-29 20:42     ` Heiner Kallweit
@ 2016-03-30  8:00       ` Jacek Anaszewski
  0 siblings, 0 replies; 7+ messages in thread
From: Jacek Anaszewski @ 2016-03-30  8:00 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Pavel Machek, linux-leds, Benjamin Tissoires, linux-usb, linux-kernel

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 <hkallweit1@gmail.com>
>>> ---
>>> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-03-30  8:00 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-01 21:36 [PATCH v5 4/4] leds: core: add support for RGB LED's Heiner Kallweit
2016-03-04  9:05 ` Jacek Anaszewski
2016-03-04 21:21   ` Heiner Kallweit
     [not found] ` <56D60B4C.5000506-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-03-29 10:05   ` Pavel Machek
2016-03-29 10:05     ` Pavel Machek
2016-03-29 20:42     ` Heiner Kallweit
2016-03-30  8:00       ` Jacek Anaszewski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.