Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1366102 > unrolled thread

Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color LED's

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Pavel Machek <pavel@ucw.cz> - 2016-03-29 12:10 +0200
    Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Heiner Kallweit <hkallweit1@gmail.com> - 2016-03-29 22:50 +0200
      Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Pavel Machek <pavel@ucw.cz> - 2016-03-29 23:50 +0200
        Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Pavel Machek <pavel@ucw.cz> - 2016-03-30 00:10 +0200
        Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Heiner Kallweit <hkallweit1@gmail.com> - 2016-03-30 08:00 +0200
          Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Pavel Machek <pavel@ucw.cz> - 2016-04-01 15:00 +0200
        Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color LED's Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-03-30 10:10 +0200
          Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Pavel Machek <pavel@ucw.cz> - 2016-03-30 15:10 +0200
            Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color LED's Heiner Kallweit <hkallweit1@gmail.com> - 2016-03-30 16:10 +0200
              Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color LED's Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-03-31 10:20 +0200
                Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Pavel Machek <pavel@ucw.cz> - 2016-04-01 16:00 +0200
                  Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Jacek Anaszewski <jacek.anaszewski@gmail.com> - 2016-04-01 21:00 +0200
                    Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Pavel Machek <pavel@ucw.cz> - 2016-04-01 23:20 +0200
              Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Pavel Machek <pavel@ucw.cz> - 2016-04-01 15:00 +0200
                Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color LED's Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-04-01 15:30 +0200
                  Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Pavel Machek <pavel@ucw.cz> - 2016-04-01 16:10 +0200
                    Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color LED's Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-04-01 16:30 +0200
                      Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Pavel Machek <pavel@ucw.cz> - 2016-04-01 17:10 +0200
          Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Pavel Machek <pavel@ucw.cz> - 2016-04-01 15:00 +0200
      Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color LED's Jacek Anaszewski <j.anaszewski@samsung.com> - 2016-03-30 10:00 +0200

#1366102 — Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color LED's

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

Second, we should define that LED brightness has similar gamma to the
monitor, so that expected colors are displayed when user requests
them.

(And then.. I guess we should talk about more advanced stuff, like
hardware that can drive the LED changes independently of the main
CPU.)

Best regards,
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [next] | [standalone]

#1366696

Am 29.03.2016 um 12:02 schrieb Pavel Machek:
> 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.
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.

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.

Rgds, Heiner

> 
> Second, we should define that LED brightness has similar gamma to the
> monitor, so that expected colors are displayed when user requests
> them.
> 
> (And then.. I guess we should talk about more advanced stuff, like
> hardware that can drive the LED changes independently of the main
> CPU.)
> 
> Best regards,
> 								Pavel
> 

[toc] | [prev] | [next] | [standalone]

#1366732

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

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1366739

Hi!

> > 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?

And subquestion: if using existing subsystem, should the RGB led be
one led, or three?

Kernel currently uses three leds for one RGB led, and even before
that, there were leds such as "charging::yellow", "charging::green"
that were as close as leds in RGB module are.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1366903

Am 29.03.2016 um 23:43 schrieb Pavel Machek:
> 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.
> 
I mainly work with encapsulated USB HID LED devices like Thingm blink(1).
Due to the diffuse plastic cover you don't see the individual LEDs on the chip.

>> 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).
> 
HSV has the charme that the current monochrome V-only is a subset.
Therefore the current API can be used also with color LEDs.
However there might be good reasons for using RGB too.

> 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.
> 
Devices like blink(1) support storing and re-playing patterns, fading etc.

> 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...)
> 
I think for this a separate layer would be helpful.
Your primary intention is to e.g. display "charging" on the RGB LED
device. Most likely you don't want to split yellow into its red + green
component and then write to the respective (sub-)LEDs.
Also just think of the case that later you might decide that orange
is nicer than yellow.

> 								Pavel
> 

[toc] | [prev] | [next] | [standalone]

#1369275

Hi!

> > 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.
> > 
> I mainly work with encapsulated USB HID LED devices like Thingm blink(1).
> Due to the diffuse plastic cover you don't see the individual LEDs on the chip.

Yeah, so on N900, you can't really see the individual LEDs, either.
But white is not uniform white.

On PS/3 motion controller (another device that is already supported),
diffusing works a bit better. 

> > 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).
> > 
> HSV has the charme that the current monochrome V-only is a subset.
> Therefore the current API can be used also with color LEDs.
> However there might be good reasons for using RGB too.

Yes, nice, but we already have RGB LED support in kernel, and it looks
different from what is proposed here. And quite incompatible. 

> > 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.
> > 
> Devices like blink(1) support storing and re-playing patterns, fading etc.
> 
> > 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...)
> > 
> I think for this a separate layer would be helpful.
> Your primary intention is to e.g. display "charging" on the RGB LED
> device. Most likely you don't want to split yellow into its red + green
> component and then write to the respective (sub-)LEDs.
> Also just think of the case that later you might decide that orange
> is nicer than yellow.

Well, so what about keeping existing red/green/blue LED devices (to
stay backward compatible) and then add separate device that links to
these, and controls patterns and colors?

Small complication is that (at least on N900) the pattern capability
can control keyboard backlight LEDs, too. It has nine channels, and
you select 3 channels that are connected to pattern generator.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1366980 — Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color LED's

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

[toc] | [prev] | [next] | [standalone]

#1367191

Hi!

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

I see HSV has some advantages. But we already have LEDs with multiple
colors, and kernel already handles them:

pavel@duo:~$ ls -1 /sys/class/leds/
tpacpi:green:batt
tpacpi:orange:batt

This is physically 2 leds but hidden under one indicator, so you got
"off", "green", "orange" and "green+orange".
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1367238 — Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color LED's

On Wed, Mar 30, 2016 at 3:03 PM, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> >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.
>
> I see HSV has some advantages. But we already have LEDs with multiple
> colors, and kernel already handles them:
>
> pavel@duo:~$ ls -1 /sys/class/leds/
> tpacpi:green:batt
> tpacpi:orange:batt
>
> This is physically 2 leds but hidden under one indicator, so you got
> "off", "green", "orange" and "green+orange".

That's a good example. As long as you can recognize green+orange as
separate lights/colors
(w/o magnifying glass) I wouldn't call it "a LED with multiple colors"
but "multiple
LED devices".

In my use case we talk about RGB LEDs like the commonly used 5050 SMD RGB LEDs.
And it's not only about using a handful of discrete colors but about
displaying any arbitrary
color.
So far the kernel exposes the physical RGB LEDs as separate LEDs only
and I can't use
a trigger to e.g. set "magenta with 50% brightness".

Heiner

>                                                                         Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1367954 — Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color LED's

Hi Heiner,

On 03/30/2016 03:59 PM, Heiner Kallweit wrote:
> On Wed, Mar 30, 2016 at 3:03 PM, Pavel Machek <pavel@ucw.cz> wrote:
>> Hi!
>>
>>>> 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.
>>
>> I see HSV has some advantages. But we already have LEDs with multiple
>> colors, and kernel already handles them:
>>
>> pavel@duo:~$ ls -1 /sys/class/leds/
>> tpacpi:green:batt
>> tpacpi:orange:batt
>>
>> This is physically 2 leds but hidden under one indicator, so you got
>> "off", "green", "orange" and "green+orange".
>
> That's a good example. As long as you can recognize green+orange as
> separate lights/colors
> (w/o magnifying glass) I wouldn't call it "a LED with multiple colors"
> but "multiple
> LED devices".
>
> In my use case we talk about RGB LEDs like the commonly used 5050 SMD RGB LEDs.
> And it's not only about using a handful of discrete colors but about
> displaying any arbitrary
> color.
> So far the kernel exposes the physical RGB LEDs as separate LEDs only
> and I can't use
> a trigger to e.g. set "magenta with 50% brightness".

I think that we should consult more people before pushing the solution
upstream. Would you mind writing a message with an explanation of the
issue to linux-api list?

Please keep in mind also the information from the "Attributes" section
of Documentation/filesystems/sysfs.txt.

-- 
Best regards,
Jacek Anaszewski

[toc] | [prev] | [next] | [standalone]

#1369332

Hi!
On Wed 2016-03-30 09:57:38, Jacek Anaszewski wrote:
> Hi Heiner and Pavel,
> 
> On 03/29/2016 10:38 PM, Heiner Kallweit wrote:
> >Am 29.03.2016 um 12:02 schrieb Pavel Machek:
> >>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".
Hi!

> >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.
> >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.
> >
> >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.
> 
> It would have the same downsides as in case of having r, g and b in
> separate attributes, i.e. - problems with setting LED colour in
> a consistent way. This way LED blinking in whatever colour couldn't
> be supported reliably. It was one of your primary rationale standing
> behind this design, if I remember correctly. Second - what about
> triggers? We've had a long discussion about it and this design turned
> out to be most fitting.

Are on/off triggers really that useful for a LED that can produce 16
million colors?

I believe we should support patterns for RGB LEDs. Something like
[ (time, r, g, b), ... ] . Ok, what about this one?

Lets say we have

/sys/class/pattern/lp5533::0
/sys/class/pattern/software::0

/sys/class/led/n900::red ; default trigger "lp5533::0:0"
/sys/class/led/n900::green ; default trigger "lp5533::0:1"
/sys/class/led/n900::blue ; default trigger "lp5533::0:2"

Normally, pattern would correspond to one RGB LED. We could have
attribute "/sys/class/pattern/lp5533::0/color" containing R,G,B for
this pattern. Then we could have normal "trigger" mechanism, working
with the color used. Probably recognizing "none" for manual control,
and "pattern" for pattern control. (Pattern would be controlled as
described above).

> It's hard to address these requirements by having the settings in
> separate attributes, due to synchronization issues, and LED trigger
> mechanism specificity.
> 
> There is a question whether we can bend the sysfs "one value per sysfs
> file" rule down to RGB LEDs needs.
> 
> Of course other brilliant ideas on how to approach the problem are
> more than expected.

linux-api sounds like interesting idea, please cc me if you do that.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1369537

On 04/01/2016 03:57 PM, Pavel Machek wrote:
> Hi!
> On Wed 2016-03-30 09:57:38, Jacek Anaszewski wrote:
>> Hi Heiner and Pavel,
>>
>> On 03/29/2016 10:38 PM, Heiner Kallweit wrote:
>>> Am 29.03.2016 um 12:02 schrieb Pavel Machek:
>>>> 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".
> Hi!
>
>>> 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.
>>> 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.
>>>
>>> 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.
>>
>> It would have the same downsides as in case of having r, g and b in
>> separate attributes, i.e. - problems with setting LED colour in
>> a consistent way. This way LED blinking in whatever colour couldn't
>> be supported reliably. It was one of your primary rationale standing
>> behind this design, if I remember correctly. Second - what about
>> triggers? We've had a long discussion about it and this design turned
>> out to be most fitting.
>
> Are on/off triggers really that useful for a LED that can produce 16
> million colors?
>
> I believe we should support patterns for RGB LEDs. Something like
> [ (time, r, g, b), ... ] . Ok, what about this one?
>
> Lets say we have
>
> /sys/class/pattern/lp5533::0
> /sys/class/pattern/software::0
>
> /sys/class/led/n900::red ; default trigger "lp5533::0:0"
> /sys/class/led/n900::green ; default trigger "lp5533::0:1"
> /sys/class/led/n900::blue ; default trigger "lp5533::0:2"
>
> Normally, pattern would correspond to one RGB LED. We could have
> attribute "/sys/class/pattern/lp5533::0/color" containing R,G,B for
> this pattern.

This involves the same issue you were opposed to: three values per
sysfs attribute.

> Then we could have normal "trigger" mechanism, working
> with the color used. Probably recognizing "none" for manual control,
> and "pattern" for pattern control. (Pattern would be controlled as
> described above).
>
>> It's hard to address these requirements by having the settings in
>> separate attributes, due to synchronization issues, and LED trigger
>> mechanism specificity.
>>
>> There is a question whether we can bend the sysfs "one value per sysfs
>> file" rule down to RGB LEDs needs.
>>
>> Of course other brilliant ideas on how to approach the problem are
>> more than expected.
>
> linux-api sounds like interesting idea, please cc me if you do that.
>
> Best regards,
> 									Pavel
>

-- 
Best regards,
Jacek Anaszewski

[toc] | [prev] | [next] | [standalone]

#1369595

Hi!

> >>It would have the same downsides as in case of having r, g and b in
> >>separate attributes, i.e. - problems with setting LED colour in
> >>a consistent way. This way LED blinking in whatever colour couldn't
> >>be supported reliably. It was one of your primary rationale standing
> >>behind this design, if I remember correctly. Second - what about
> >>triggers? We've had a long discussion about it and this design turned
> >>out to be most fitting.
> >
> >Are on/off triggers really that useful for a LED that can produce 16
> >million colors?
> >
> >I believe we should support patterns for RGB LEDs. Something like
> >[ (time, r, g, b), ... ] . Ok, what about this one?
> >
> >Lets say we have
> >
> >/sys/class/pattern/lp5533::0
> >/sys/class/pattern/software::0
> >
> >/sys/class/led/n900::red ; default trigger "lp5533::0:0"
> >/sys/class/led/n900::green ; default trigger "lp5533::0:1"
> >/sys/class/led/n900::blue ; default trigger "lp5533::0:2"
> >
> >Normally, pattern would correspond to one RGB LED. We could have
> >attribute "/sys/class/pattern/lp5533::0/color" containing R,G,B for
> >this pattern.
> 
> This involves the same issue you were opposed to: three values per
> sysfs attribute.

And solves a lot of other things. Like actually being backwards
compatible.

And yes, it involves three values in a file, but now it is array of
led brightnesses, and that might actually be acceptable. (At least the
values have uniform meaning).

Plus, it is not "issue you were opposed to" it is "something that is
not permitted by sysfs maintainers".

Thanks,
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1369278

Hi!

> > pavel@duo:~$ ls -1 /sys/class/leds/
> > tpacpi:green:batt
> > tpacpi:orange:batt
> >
> > This is physically 2 leds but hidden under one indicator, so you got
> > "off", "green", "orange" and "green+orange".
> 
> That's a good example. As long as you can recognize green+orange as
> separate lights/colors
> (w/o magnifying glass) I wouldn't call it "a LED with multiple colors"
> but "multiple
> LED devices".

Well, that's how it is currently handled. But for the user, it looks
as a LED with multiple colors.

> In my use case we talk about RGB LEDs like the commonly used 5050 SMD RGB LEDs.
> And it's not only about using a handful of discrete colors but about
> displaying any arbitrary
> color.
> So far the kernel exposes the physical RGB LEDs as separate LEDs only
> and I can't use
> a trigger to e.g. set "magenta with 50% brightness".

Why not?

What do you do if you want to display magenta on your LCD?

You compute RGB values, then display them.

What would you do for the LEDs? Same thing.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1369307 — Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color LED's

On 04/01/2016 02:55 PM, Pavel Machek wrote:
> Hi!
>
>>> pavel@duo:~$ ls -1 /sys/class/leds/
>>> tpacpi:green:batt
>>> tpacpi:orange:batt
>>>
>>> This is physically 2 leds but hidden under one indicator, so you got
>>> "off", "green", "orange" and "green+orange".
>>
>> That's a good example. As long as you can recognize green+orange as
>> separate lights/colors
>> (w/o magnifying glass) I wouldn't call it "a LED with multiple colors"
>> but "multiple
>> LED devices".
>
> Well, that's how it is currently handled. But for the user, it looks
> as a LED with multiple colors.
>
>> In my use case we talk about RGB LEDs like the commonly used 5050 SMD RGB LEDs.
>> And it's not only about using a handful of discrete colors but about
>> displaying any arbitrary
>> color.
>> So far the kernel exposes the physical RGB LEDs as separate LEDs only
>> and I can't use
>> a trigger to e.g. set "magenta with 50% brightness".
>
> Why not?
>
> What do you do if you want to display magenta on your LCD?
>
> You compute RGB values, then display them.

The main drawback is that you can't set the colour at one go,
but have to set brightness of each LED class device (R,G,B)
separately. It incurs delays between setting each colour component.

It is also impossible to set arbitrary colour from a trigger.
Similarly blinking with arbitrarily chosen colour from RGB palette
is impossible if separate colour components are treated as
separate LEDs.

-- 
Best regards,
Jacek Anaszewski

[toc] | [prev] | [next] | [standalone]

#1369343

Hi!

> >>>pavel@duo:~$ ls -1 /sys/class/leds/
> >>>tpacpi:green:batt
> >>>tpacpi:orange:batt
> >>>
> >>>This is physically 2 leds but hidden under one indicator, so you got
> >>>"off", "green", "orange" and "green+orange".
> >>
> >>That's a good example. As long as you can recognize green+orange as
> >>separate lights/colors
> >>(w/o magnifying glass) I wouldn't call it "a LED with multiple colors"
> >>but "multiple
> >>LED devices".
> >
> >Well, that's how it is currently handled. But for the user, it looks
> >as a LED with multiple colors.
> >
> >>In my use case we talk about RGB LEDs like the commonly used 5050 SMD RGB LEDs.
> >>And it's not only about using a handful of discrete colors but about
> >>displaying any arbitrary
> >>color.
> >>So far the kernel exposes the physical RGB LEDs as separate LEDs only
> >>and I can't use
> >>a trigger to e.g. set "magenta with 50% brightness".
> >
> >Why not?
> >
> >What do you do if you want to display magenta on your LCD?
> >
> >You compute RGB values, then display them.
> 
> The main drawback is that you can't set the colour at one go,
> but have to set brightness of each LED class device (R,G,B)
> separately. It incurs delays between setting each colour component.

Yeah. Well, on some hardware, that's just the way it is. If the leds
are separate devices on i2c, you can't really set them in one go.

But some hardware has hardware pattern controls, and it can set them
atomically. 

> It is also impossible to set arbitrary colour from a trigger.
> Similarly blinking with arbitrarily chosen colour from RGB palette
> is impossible if separate colour components are treated as
> separate LEDs.

Yes, see the proposal in the other mail. I believe we should have
separate R, G, B LED devices, and separate pattern controller.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1369369 — Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color LED's

On 04/01/2016 04:07 PM, Pavel Machek wrote:
> Hi!
>
>>>>> pavel@duo:~$ ls -1 /sys/class/leds/
>>>>> tpacpi:green:batt
>>>>> tpacpi:orange:batt
>>>>>
>>>>> This is physically 2 leds but hidden under one indicator, so you got
>>>>> "off", "green", "orange" and "green+orange".
>>>>
>>>> That's a good example. As long as you can recognize green+orange as
>>>> separate lights/colors
>>>> (w/o magnifying glass) I wouldn't call it "a LED with multiple colors"
>>>> but "multiple
>>>> LED devices".
>>>
>>> Well, that's how it is currently handled. But for the user, it looks
>>> as a LED with multiple colors.
>>>
>>>> In my use case we talk about RGB LEDs like the commonly used 5050 SMD RGB LEDs.
>>>> And it's not only about using a handful of discrete colors but about
>>>> displaying any arbitrary
>>>> color.
>>>> So far the kernel exposes the physical RGB LEDs as separate LEDs only
>>>> and I can't use
>>>> a trigger to e.g. set "magenta with 50% brightness".
>>>
>>> Why not?
>>>
>>> What do you do if you want to display magenta on your LCD?
>>>
>>> You compute RGB values, then display them.
>>
>> The main drawback is that you can't set the colour at one go,
>> but have to set brightness of each LED class device (R,G,B)
>> separately. It incurs delays between setting each colour component.
>
> Yeah. Well, on some hardware, that's just the way it is. If the leds
> are separate devices on i2c, you can't really set them in one go.

Delays can occur even if the LEDs are controlled by the same device.
Brightness of each LED class device is set with separate system
call and there will be always some time shift between particular I2C
transmissions that set the brightness for each sub-LED.

If the three sub-LEDs were controlled by a single LED class device
then we could setup the brightness of each sub-LED with single I2C
transmission.

> But some hardware has hardware pattern controls, and it can set them
> atomically.
>
>> It is also impossible to set arbitrary colour from a trigger.
>> Similarly blinking with arbitrarily chosen colour from RGB palette
>> is impossible if separate colour components are treated as
>> separate LEDs.
>
> Yes, see the proposal in the other mail. I believe we should have
> separate R, G, B LED devices, and separate pattern controller.
>
> Best regards,
> 									Pavel
>


-- 
Best regards,
Jacek Anaszewski

[toc] | [prev] | [next] | [standalone]

#1369404

Hi!

> >>The main drawback is that you can't set the colour at one go,
> >>but have to set brightness of each LED class device (R,G,B)
> >>separately. It incurs delays between setting each colour component.
> >
> >Yeah. Well, on some hardware, that's just the way it is. If the leds
> >are separate devices on i2c, you can't really set them in one go.
> 
> Delays can occur even if the LEDs are controlled by the same device.
> Brightness of each LED class device is set with separate system
> call and there will be always some time shift between particular I2C
> transmissions that set the brightness for each sub-LED.
> 
> If the three sub-LEDs were controlled by a single LED class device
> then we could setup the brightness of each sub-LED with single I2C
> transmission.

Ok, well, yes, maybe you could.

You can still do that with the proposed interface, but yes, it will be
trickier.

OTOH proposed interface will also help with the hardware pattern
support, will work with existing leds, and matches the way hardware
works. So I believe it is worth it.

Best regards,

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1369268

Hi!

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

Yes, that's nice, but it is incompatible with already existing RGB
support in kernel. Plus, echoing hue into file called brightness is
extremely ugly, and it violates sysfs rules.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[toc] | [prev] | [next] | [standalone]

#1366970 — Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color LED's

Hi Heiner and Pavel,

On 03/29/2016 10:38 PM, Heiner Kallweit wrote:
> Am 29.03.2016 um 12:02 schrieb Pavel Machek:
>> 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.
> 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.
>
> 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.

It would have the same downsides as in case of having r, g and b in
separate attributes, i.e. - problems with setting LED colour in
a consistent way. This way LED blinking in whatever colour couldn't
be supported reliably. It was one of your primary rationale standing
behind this design, if I remember correctly. Second - what about
triggers? We've had a long discussion about it and this design turned
out to be most fitting.

It's hard to address these requirements by having the settings in
separate attributes, due to synchronization issues, and LED trigger
mechanism specificity.

There is a question whether we can bend the sysfs "one value per sysfs
file" rule down to RGB LEDs needs.

Of course other brilliant ideas on how to approach the problem are
more than expected.

-- 
Best regards,
Jacek Anaszewski

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web