linux-kernel - Re: [PATCH v5 4/4] leds: core: add support for RGB LED's

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-id: <56FB8787.7090808@samsung.com>
Date:	Wed, 30 Mar 2016 10:00:07 +0200
From:	Jacek Anaszewski <j.anaszewski@...sung.com>
To:	Heiner Kallweit <hkallweit1@...il.com>
Cc:	Pavel Machek <pavel@....cz>, linux-leds@...r.kernel.org,
	Benjamin Tissoires <benjamin.tissoires@...hat.com>,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 4/4] leds: core: add support for RGB LED's

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@...il.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