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