From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1758743AbcC3IHi (ORCPT <rfc822;w@1wt.eu>);
	Wed, 30 Mar 2016 04:07:38 -0400
Received: from mailout4.w1.samsung.com ([210.118.77.14]:32190 "EHLO
	mailout4.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753983AbcC3IHa (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Wed, 30 Mar 2016 04:07:30 -0400
X-AuditID: cbfec7f5-f792a6d000001302-58-56fb893e6fb2
Message-id: <56FB893C.60203@samsung.com>
Date: Wed, 30 Mar 2016 10:07:24 +0200
From: Jacek Anaszewski <j.anaszewski@samsung.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130804
 Thunderbird/17.0.8
MIME-version: 1.0
To: Pavel Machek <pavel@ucw.cz>
Cc: Heiner Kallweit <hkallweit1@gmail.com>, pali.rohar@gmail.com,
        sre@kernel.org, khilman@kernel.org, aaro.koskinen@iki.fi,
        ivo.g.dimitrov.75@gmail.com, patrikbachan@gmail.com, serge@hallyn.com,
        Greg KH <greg@kroah.com>, linux-leds@vger.kernel.org,
        Benjamin Tissoires <benjamin.tissoires@redhat.com>,
        linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org
Subject: Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color LED's
References: <56D608ED.2090406@gmail.com> <20160329100258.GA24964@amd>
 <56FAE7A8.2070302@gmail.com> <20160329214323.GA10455@amd>
In-reply-to: <20160329214323.GA10455@amd>
Content-type: text/plain; charset=ISO-8859-1; format=flowed
Content-transfer-encoding: 7bit
X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupmkeLIzCtJLcpLzFFi42I5/e/4VV27zt9hBv2zeSzWvHCwuD5lM6vF
	uQUzGC0WvZ/BarHkyiF2i6ebHzNZXN41h81i65t1QIllrcwWE0//ZrJYeLOJ2eLuqaNsFucv
	nGO3OL27xIHPY+esu+we13ZHehz+upDFY9OqTjaPtw8DPN7vu8rmsWL1d3aPz5vkAjiiuGxS
	UnMyy1KL9O0SuDL27DrMVtAlV3F9/1eWBsZWiS5GTg4JAROJPW+eM0PYYhIX7q1n62Lk4hAS
	WMoosWRHGzOE84xR4vjiTUAOBwevgIZE3xlbkAYWAVWJ05eOgjWzCRhK/HzxmgnEFhWIkPhz
	eh8riM0rICjxY/I9FhBbREBeYmvfCrCZzALvmCSm9L4EaxAW8Jc4/7ERalkjo8SBzZfAujkF
	NCXWbLoLZjMLWEusnLSNEcKWl9i85i3zBEaBWUiWzEJSNgtJ2QJG5lWMoqmlyQXFSem5RnrF
	ibnFpXnpesn5uZsYIVH0dQfj0mNWhxgFOBiVeHg3SP8OE2JNLCuuzD3EKMHBrCTCa9sBFOJN
	SaysSi3Kjy8qzUktPsQozcGiJM47c9f7ECGB9MSS1OzU1ILUIpgsEwenVANjoXXnRYHnCkZh
	a4/uD1ZnDFKaeWspU8bES1ff1l28318Vw/To3bXOlwtkNnsfS61e8GsRQ/bv316XVk4pWBfH
	HVW3iffal+drV27z+9EzgWO/rPgl3/cC+55011f+2Xm61vZjipvnUaELDRv+7V1pcq1+7a/r
	gpLNfv43fUNPpKtOUZq85P3He0osxRmJhlrMRcWJAK2n2gmeAgAA
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 03/29/2016 11:43 PM, Pavel Machek wrote:
> Hi!
>
>>> First, please Cc me on RGB color support.
>>>
>>>> Add generic support for RGB Color LED's.
>>>>
>>>> Basic idea is to use enum led_brightness also for the hue and saturation
>>>> color components.This allows to implement the color extension w/o
>>>> changes to struct led_classdev.
>>>>
>>>> Select LEDS_RGB to enable building drivers using the RGB extension.
>>>>
>>>> Flag LED_SET_HUE_SAT allows to specify that hue / saturation
>>>> should be overridden even if the provided values are zero.
>>>>
>>>> Some examples for writing values to /sys/class/leds/<xx>/brightness:
>>>> (now also hex notation can be used)
>>>>
>>>> 255 -> set full brightness and keep existing color if set
>>>> 0 -> switch LED off but keep existing color so that it can be restored
>>>>       if the LED is switched on again later
>>>> 0x1000000 -> switch LED off and set also hue and saturation to 0
>>>> 0x00ffff -> set full brightness, full saturation and set hue to 0
>>>> (red)
>>>
>>> Umm, that's rather strange interface -- and three values in single sysfs
>>> file is actually forbidden.
>>>
>>> Plus, it is very much unlike existing interfaces for RGB LEDs, which
>>> we already have supported in the tree. (At least nokia N900 and Sony
>>> motion controller already contain supported three-color LEDs).
>>>
>>> Now... yes, there's work to be done for the 3-color LEDs. Currently,
>>> they are treated as three different LEDs. (Which makes some sense, you
>>> can use "battery charging" trigger for LED, and CPU activity trigger
>>> for green, for example). It would be good to have some kind of
>>> grouping, so that userspace can tell "these 3 leds are actually
>>> combined into one light".
>>>
>> At first thanks for the review comments.
>> Treating the three physical LEDs of a RGB LED as separate LED devices
>> might have been implemented due to the lack of alternatives.
>
> To be fair... they _are_ separate LED devices. In N900 case, you can
> even see light comming from slightly different places if you look closely.
>
>> With one trigger controlling the red LED and another controlling the green
>> LED we may end up with a yellow light. Not sure whether this is what
>> we want.
>
> Well, it should be understandable for most people.
>
>> One driver for this extension was the idea of triggers using color
>> to visualize states etc.
>> Therefore it's not only about userspace controlling the color.
>> As a trigger is bound to a led_classdev we need a led_classdev
>> representing a RGB LED device.
>>
>> And ok: If required the sysfs interface can be splitted into separate
>> attributes for hue, saturation, and (existing) brightness.
>
> Required.
>
> Ok, so:
>
> a) Do we want RGB leds to be handled by existing subsystem, or do we
> need separate layer on top of that?
>
> b) Does RGB make sense, or HSV? RGB is quite widely used in graphics,
> and it is what hardware implements. (But we'd need to do gamma
> correction).
>
> c) My hardware has "acceleration engine", LED is independend from
> CPU. That's rather big deal. Does yours? It seems to be quite common,
> at least in cellphones.
>
> Ideally, I'd like to have "triggers", but different ones. As in: if
> charging, do yellow " .xX" pattern. If fully charged, do green steady
> light. If message is waiting, do blue " x x" pattern. If none of
> above, do slow white blinking. (Plus priorities of events). But that's
> quite different from existing support...)

Please note that HSV colour scheme allows to neatly project monochrome
brightness semantics on the RGB realm. I.e. you can have fixed
hue and saturation, and by changing the brightness component a perceived
colour intensity can be altered.

-- 
Best regards,
Jacek Anaszewski