Am 29.02.2016 um 10:57 schrieb Jacek Anaszewski:
> 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.
>
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.
>>>>>> 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-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
