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

Groups > linux.kernel > #1370913 > 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 Jacek Anaszewski <jacek.anaszewski@gmail.com> - 2016-04-04 23:40 +0200
    Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Pavel Machek <pavel@ucw.cz> - 2016-04-05 11:10 +0200
      Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Jacek Anaszewski <jacek.anaszewski@gmail.com> - 2016-04-05 21:50 +0200
        Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Heiner Kallweit <hkallweit1@gmail.com> - 2016-04-05 22:50 +0200
          Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Jacek Anaszewski <jacek.anaszewski@gmail.com> - 2016-04-06 00:20 +0200
            Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Pavel Machek <pavel@ucw.cz> - 2016-04-06 11:20 +0200
          Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Pavel Machek <pavel@ucw.cz> - 2016-04-06 11:20 +0200
        Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Pavel Machek <pavel@ucw.cz> - 2016-04-06 11: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-06 12:00 +0200
            Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Pavel Machek <pavel@ucw.cz> - 2016-04-07 22:50 +0200
              Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Jacek Anaszewski <jacek.anaszewski@gmail.com> - 2016-04-08 20:50 +0200
                Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Pavel Machek <pavel@ucw.cz> - 2016-04-09 18: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-12 09:20 +0200
                    Re: [PATCH v5 1/4] leds: core: add generic support for RGB Color  LED's Pavel Machek <pavel@ucw.cz> - 2016-04-15 14:00 +0200

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

Hi Pavel,

On 04/01/2016 11:18 PM, Pavel Machek wrote:
> 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.

Could you give an example on how to set a color for RGB LED using
this interface? Would it be compatible with LED triggers?
Where the "pattern" class would be implemented?

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

-- 
Best regards,
Jacek Anaszewski

[toc] | [next] | [standalone]

#1371332

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.
> 
> Could you give an example on how to set a color for RGB LED using
> this interface? Would it be compatible with LED triggers?
> Where the "pattern" class would be implemented?

Well, 'echo "50 60 70" > /sys/class/pattern/lp5533::0/color' should
set the color for the led. 'echo "trigger-name" > trigger' would set
the trigger, probably just toggling between LED off and set color for
the old triggers.

Where to implement the patterns is different question, but for example
drivers/leds/pattern?

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]

#1371912

On 04/05/2016 11:01 AM, Pavel Machek wrote:
> 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.
>>
>> Could you give an example on how to set a color for RGB LED using
>> this interface? Would it be compatible with LED triggers?
>> Where the "pattern" class would be implemented?
>
> Well, 'echo "50 60 70" > /sys/class/pattern/lp5533::0/color' should
> set the color for the led. 'echo "trigger-name" > trigger' would set
> the trigger, probably just toggling between LED off and set color for
> the old triggers.
>
> Where to implement the patterns is different question, but for example
> drivers/leds/pattern?

I'd rather leave the pattern issue for now, since it seems to be
different from the problem Heiner was trying to solve with his LED RGB
extension. Moreover, hardware patterns are device specific and it could
be hard to propose a generic interface.
Drivers can always expose their custom sysfs attributes for configuring
the patterns.

Regardless of the above, some of your considerations brought me an idea
on how to add generic and backwards compatible support for setting RGB
color at one go.

Currently LED class drivers of RGB LED controllers expose three LED
class devices - one per R, G and B color component. I propose that
such drivers set LED_DEV_CAP_RGB flag for each LED class device they
register. LED core, seeing the flag, would create a generic "color"
sysfs attribute for each of the three LED class devices.

The "color" attribute would contain "R G B" values. Setting the "color"
attribute of any of the three LED class devices would affect brightness
properties (i.e. constituent colors) of the remaining two ones.
It would result in disabling any active triggers and writing all the
three color settings to the RGB LED controller at one go.

We would probably need additional op in the LED core : color_set.

Having the color set to nonzero value would signify the the three LED
class devices are in sync and that setting a trigger on any of them
applies to the remaining two ones. It would have to be considered
whether existing triggers could be made compatible with synchronized
RGB LED class devices.

I'm curious what do you think about the idea.

Pavel, Heiner, others?

-- 
Best regards,
Jacek Anaszewski

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

#1371932

Am 05.04.2016 um 21:45 schrieb Jacek Anaszewski:
> On 04/05/2016 11:01 AM, Pavel Machek wrote:
>> 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.
>>>
>>> Could you give an example on how to set a color for RGB LED using
>>> this interface? Would it be compatible with LED triggers?
>>> Where the "pattern" class would be implemented?
>>
>> Well, 'echo "50 60 70" > /sys/class/pattern/lp5533::0/color' should
>> set the color for the led. 'echo "trigger-name" > trigger' would set
>> the trigger, probably just toggling between LED off and set color for
>> the old triggers.
>>
>> Where to implement the patterns is different question, but for example
>> drivers/leds/pattern?
> 
> I'd rather leave the pattern issue for now, since it seems to be
> different from the problem Heiner was trying to solve with his LED RGB
> extension. Moreover, hardware patterns are device specific and it could
> be hard to propose a generic interface.
> Drivers can always expose their custom sysfs attributes for configuring
> the patterns.
> 
> Regardless of the above, some of your considerations brought me an idea
> on how to add generic and backwards compatible support for setting RGB
> color at one go.
> 
> Currently LED class drivers of RGB LED controllers expose three LED
> class devices - one per R, G and B color component. I propose that
> such drivers set LED_DEV_CAP_RGB flag for each LED class device they
> register. LED core, seeing the flag, would create a generic "color"
> sysfs attribute for each of the three LED class devices.
> 
> The "color" attribute would contain "R G B" values. Setting the "color"
> attribute of any of the three LED class devices would affect brightness
> properties (i.e. constituent colors) of the remaining two ones.
> It would result in disabling any active triggers and writing all the
> three color settings to the RGB LED controller at one go.
> 
> We would probably need additional op in the LED core : color_set.
> 
> Having the color set to nonzero value would signify the the three LED
> class devices are in sync and that setting a trigger on any of them
> applies to the remaining two ones. It would have to be considered
> whether existing triggers could be made compatible with synchronized
> RGB LED class devices.
> 
> I'm curious what do you think about the idea.
> 
> Pavel, Heiner, others?
> 
Exposing "coupled LED devices" as separate LED devices most likely is ok
when accessed from user space as the name of the led_classdev's indicates
that they belong together.
But how about a trigger wanting to set a RGB LED to a specific color?
(That's not available yet but one possible use case for RGB LED's)
A trigger is bound to a led_classdev currently. In addition we'd need
to introduce some kind of super_led_classdev having links to the respective
R/G/B led_classdev's (+ trigger functions dealing with this super_led_classdev).

These changes / extensions are not needed if a RGB LED is exposed as one
led_classdev, just with flag LED_DEV_CAP_RGB set.
OK, we'd still have to change the sysfs interface as obviously setting
hue/sat/brightness via one "brightness" attribute is not acceptable.
However this constraint might not affect the kernel-internal trigger API
(usage of parameter brightness in led_trigger_event).

I see Pavel's point that there might be different types of multi-color LED's.
At least we have:
- multi-color LED's where each single LED is visible even if all are switched on
- multi-color LED's like RGB LED's where you usually just see a uniform color

Last but not least regarding the patterns:
Something like proposed by Pavel is e.g. (partially) supported by the blink(1)
firmware. That would be an example of such a "hardware-accelerated" pattern.

As I see it the current blinking support then would be one special case of a pattern.
As a consequence once having pattern support we might be able to switch users of blinking
to pattern and remove the blinking support.

Regards, Heiner

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

#1372026

Hi Heiner,

Thanks for the feedback.

On 04/05/2016 10:43 PM, Heiner Kallweit wrote:
> Am 05.04.2016 um 21:45 schrieb Jacek Anaszewski:
>> On 04/05/2016 11:01 AM, Pavel Machek wrote:
>>> 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.
>>>>
>>>> Could you give an example on how to set a color for RGB LED using
>>>> this interface? Would it be compatible with LED triggers?
>>>> Where the "pattern" class would be implemented?
>>>
>>> Well, 'echo "50 60 70" > /sys/class/pattern/lp5533::0/color' should
>>> set the color for the led. 'echo "trigger-name" > trigger' would set
>>> the trigger, probably just toggling between LED off and set color for
>>> the old triggers.
>>>
>>> Where to implement the patterns is different question, but for example
>>> drivers/leds/pattern?
>>
>> I'd rather leave the pattern issue for now, since it seems to be
>> different from the problem Heiner was trying to solve with his LED RGB
>> extension. Moreover, hardware patterns are device specific and it could
>> be hard to propose a generic interface.
>> Drivers can always expose their custom sysfs attributes for configuring
>> the patterns.
>>
>> Regardless of the above, some of your considerations brought me an idea
>> on how to add generic and backwards compatible support for setting RGB
>> color at one go.
>>
>> Currently LED class drivers of RGB LED controllers expose three LED
>> class devices - one per R, G and B color component. I propose that
>> such drivers set LED_DEV_CAP_RGB flag for each LED class device they
>> register. LED core, seeing the flag, would create a generic "color"
>> sysfs attribute for each of the three LED class devices.
>>
>> The "color" attribute would contain "R G B" values. Setting the "color"
>> attribute of any of the three LED class devices would affect brightness
>> properties (i.e. constituent colors) of the remaining two ones.
>> It would result in disabling any active triggers and writing all the
>> three color settings to the RGB LED controller at one go.
>>
>> We would probably need additional op in the LED core : color_set.
>>
>> Having the color set to nonzero value would signify the the three LED
>> class devices are in sync and that setting a trigger on any of them
>> applies to the remaining two ones. It would have to be considered
>> whether existing triggers could be made compatible with synchronized
>> RGB LED class devices.
>>
>> I'm curious what do you think about the idea.
>>
>> Pavel, Heiner, others?
>>
 >
> Exposing "coupled LED devices" as separate LED devices most likely is ok
> when accessed from user space as the name of the led_classdev's indicates
> that they belong together.
> But how about a trigger wanting to set a RGB LED to a specific color?

RGB triggers would use a new color_set op. It means that currently
implemented triggers would be unable to set arbitrary color, but
they could be used only in a monochrome context.

> (That's not available yet but one possible use case for RGB LED's)
> A trigger is bound to a led_classdev currently. In addition we'd need
> to introduce some kind of super_led_classdev having links to the respective
> R/G/B led_classdev's (+ trigger functions dealing with this super_led_classdev).
>
> These changes / extensions are not needed if a RGB LED is exposed as one
> led_classdev, just with flag LED_DEV_CAP_RGB set.
> OK, we'd still have to change the sysfs interface as obviously setting
> hue/sat/brightness via one "brightness" attribute is not acceptable.
> However this constraint might not affect the kernel-internal trigger API
> (usage of parameter brightness in led_trigger_event).

We would still have to abuse brightness parameter semantics.

> I see Pavel's point that there might be different types of multi-color LED's.
> At least we have:
> - multi-color LED's where each single LED is visible even if all are switched on
> - multi-color LED's like RGB LED's where you usually just see a uniform color

I think that if we are talking about RGB LEDs it is always the second
case.

> Last but not least regarding the patterns:
> Something like proposed by Pavel is e.g. (partially) supported by the blink(1)
> firmware. That would be an example of such a "hardware-accelerated" pattern.
>
> As I see it the current blinking support then would be one special case of a pattern.
> As a consequence once having pattern support we might be able to switch users of blinking
> to pattern and remove the blinking support.

Let's split patterns related discussion into a separate thread.
It would be best if it began with a patch.

-- 
Best regards,
Jacek Anaszewski

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

#1372316

Hi!

> >As I see it the current blinking support then would be one special case of a pattern.
> >As a consequence once having pattern support we might be able to switch users of blinking
> >to pattern and remove the blinking support.
> 
> Let's split patterns related discussion into a separate thread.
> It would be best if it began with a patch.

Lets design userland interface first, then decide how to implement it
in the kernel. Patches are useless at this point.

And actually... without patterns, existing interface works just
fine. Even if you can't "atomically" write values to three different
files, operation is so fast that user will not see the intermediate
state, anyway. So solving the "atomic" issue without solving the rest
is pretty much useless.

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]

#1372317

Hi!

> > We would probably need additional op in the LED core : color_set.
> > 
> > Having the color set to nonzero value would signify the the three LED
> > class devices are in sync and that setting a trigger on any of them
> > applies to the remaining two ones. It would have to be considered
> > whether existing triggers could be made compatible with synchronized
> > RGB LED class devices.
> > 
> > I'm curious what do you think about the idea.
> > 
> > Pavel, Heiner, others?
> > 
> Exposing "coupled LED devices" as separate LED devices most likely is ok
> when accessed from user space as the name of the led_classdev's indicates
> that they belong together.
> But how about a trigger wanting to set a RGB LED to a specific color?
> (That's not available yet but one possible use case for RGB LED's)
> A trigger is bound to a led_classdev currently. In addition we'd need
> to introduce some kind of super_led_classdev having links to the respective
> R/G/B led_classdev's (+ trigger functions dealing with this super_led_classdev).
> 
> These changes / extensions are not needed if a RGB LED is exposed as one
> led_classdev, just with flag LED_DEV_CAP_RGB set.
> OK, we'd still have to change the sysfs interface as obviously setting
> hue/sat/brightness via one "brightness" attribute is not acceptable.
> However this constraint might not affect the kernel-internal trigger API
> (usage of parameter brightness in led_trigger_event).

Your proposal would break existing hardware. We already have RGB LEDs
exposed as three LEDs. It is too late to change interface there.

> I see Pavel's point that there might be different types of multi-color LED's.
> At least we have:
> - multi-color LED's where each single LED is visible even if all are switched on
> - multi-color LED's like RGB LED's where you usually just see a
> uniform color

Well, I suggest we ignore that distinction. Yes, I can see different
colors coming from different directions, but the LED was clearly
designed to look like single light. 

> Last but not least regarding the patterns:
> Something like proposed by Pavel is e.g. (partially) supported by the blink(1)
> firmware. That would be an example of such a "hardware-accelerated" pattern.
> 
> As I see it the current blinking support then would be one special case of a pattern.
> As a consequence once having pattern support we might be able to switch users of blinking
> to pattern and remove the blinking support.

No, you can't remove existing blinking support, due to backwards
compatibility reasons.

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

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

#1372304

Hi!

> >>>>>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.
> >>
> >>Could you give an example on how to set a color for RGB LED using
> >>this interface? Would it be compatible with LED triggers?
> >>Where the "pattern" class would be implemented?
> >
> >Well, 'echo "50 60 70" > /sys/class/pattern/lp5533::0/color' should
> >set the color for the led. 'echo "trigger-name" > trigger' would set
> >the trigger, probably just toggling between LED off and set color for
> >the old triggers.
> >
> >Where to implement the patterns is different question, but for example
> >drivers/leds/pattern?
> 
> I'd rather leave the pattern issue for now, since it seems to be
> different from the problem Heiner was trying to solve with his LED RGB
> extension. Moreover, hardware patterns are device specific and it could
> be hard to propose a generic interface.

Well, RGB leds are basically useless without pattern support. And I
believe we can do generic interface.

> Drivers can always expose their custom sysfs attributes for configuring
> the patterns.
> 
> Regardless of the above, some of your considerations brought me an idea
> on how to add generic and backwards compatible support for setting RGB
> color at one go.
> 
> Currently LED class drivers of RGB LED controllers expose three LED
> class devices - one per R, G and B color component. I propose that
> such drivers set LED_DEV_CAP_RGB flag for each LED class device they
> register. LED core, seeing the flag, would create a generic "color"
> sysfs attribute for each of the three LED class devices.
> 
> The "color" attribute would contain "R G B" values. Setting the "color"
> attribute of any of the three LED class devices would affect brightness
> properties (i.e. constituent colors) of the remaining two ones.
> It would result in disabling any active triggers and writing all the
> three color settings to the RGB LED controller at one go.

Having one attribute across three devices is rather ugly. And we'll
need to solve the pattern issue one day.

What's tricky about patterns is that you need to control 3 (or more)
leds at a time. Problem you are trying to solve here is ... control of
3 leds, at the same time.

So let's solve them together.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

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

#1372349

On 04/06/2016 10:52 AM, Pavel Machek wrote:
> Hi!
>
>>>>>>> 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.
>>>>
>>>> Could you give an example on how to set a color for RGB LED using
>>>> this interface? Would it be compatible with LED triggers?
>>>> Where the "pattern" class would be implemented?
>>>
>>> Well, 'echo "50 60 70" > /sys/class/pattern/lp5533::0/color' should
>>> set the color for the led. 'echo "trigger-name" > trigger' would set
>>> the trigger, probably just toggling between LED off and set color for
>>> the old triggers.
>>>
>>> Where to implement the patterns is different question, but for example
>>> drivers/leds/pattern?
>>
>> I'd rather leave the pattern issue for now, since it seems to be
>> different from the problem Heiner was trying to solve with his LED RGB
>> extension. Moreover, hardware patterns are device specific and it could
>> be hard to propose a generic interface.
>
> Well, RGB leds are basically useless without pattern support. And I
> believe we can do generic interface.
>
>> Drivers can always expose their custom sysfs attributes for configuring
>> the patterns.
>>
>> Regardless of the above, some of your considerations brought me an idea
>> on how to add generic and backwards compatible support for setting RGB
>> color at one go.
>>
>> Currently LED class drivers of RGB LED controllers expose three LED
>> class devices - one per R, G and B color component. I propose that
>> such drivers set LED_DEV_CAP_RGB flag for each LED class device they
>> register. LED core, seeing the flag, would create a generic "color"
>> sysfs attribute for each of the three LED class devices.
>>
>> The "color" attribute would contain "R G B" values. Setting the "color"
>> attribute of any of the three LED class devices would affect brightness
>> properties (i.e. constituent colors) of the remaining two ones.
>> It would result in disabling any active triggers and writing all the
>> three color settings to the RGB LED controller at one go.
>
> Having one attribute across three devices is rather ugly. And we'll
> need to solve the pattern issue one day.
>
> What's tricky about patterns is that you need to control 3 (or more)
> leds at a time. Problem you are trying to solve here is ... control of
> 3 leds, at the same time.
>
> So let's solve them together.

OK, now I've got your point. So we'd need to have a means for defining
patterns. The interface could be located at /sys/class/leds/patterns.

We'd need to have a flexible way for defining LED class devices involved
in a pattern. Since we cannot guarantee no space in a LED class device
name, then a single attribute containing space separated list is not an
option. We'd have to create a predefined set of attributes that would
contain LED class device name. Predefined implies that it would be
a fixed number, i.e. either some attributes would always remain unused
or, which is even worse, we could run out of free attributes for some
use cases.

The same constraints would appear if we wanted to be able to define
more than one pattern.

It would be best to work out more flexible solution. I wonder if
ioctl interface isn't the only option.

-- 
Best regards,
Jacek Anaszewski

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

#1373750

Hi!

> >>The "color" attribute would contain "R G B" values. Setting the "color"
> >>attribute of any of the three LED class devices would affect brightness
> >>properties (i.e. constituent colors) of the remaining two ones.
> >>It would result in disabling any active triggers and writing all the
> >>three color settings to the RGB LED controller at one go.
> >
> >Having one attribute across three devices is rather ugly. And we'll
> >need to solve the pattern issue one day.
> >
> >What's tricky about patterns is that you need to control 3 (or more)
> >leds at a time. Problem you are trying to solve here is ... control of
> >3 leds, at the same time.
> >
> >So let's solve them together.
> 
> OK, now I've got your point. So we'd need to have a means for defining
> patterns. The interface could be located at /sys/class/leds/patterns.
> 
> We'd need to have a flexible way for defining LED class devices involved
> in a pattern. Since we cannot guarantee no space in a LED class device
> name, then a single attribute containing space separated list is not an
> option. We'd have to create a predefined set of attributes that would
> contain LED class device name. Predefined implies that it would be
> a fixed number, i.e. either some attributes would always remain unused
> or, which is even worse, we could run out of free attributes for some
> use cases.

There's a better solution: make pattern behave as a trigger for leds
it controls.

So we'd have

/sys/class/leds/patterns/lp5523

then we'd have

/sys/class/leds/lp5523::red/trigger = "lp5523:1"
/sys/class/leds/lp5523::green/trigger = "lp5523:2"
/sys/class/leds/lp5523::blue/trigger = "lp5523:3"

(or something similar, I'd have to boot the n900 to see the exact
names).

That means that we don't need space-separated lists. (And actually
gives us more flexibility; Maemo for example used the pattern engine
not for RGB led, but for 6 keyboard backlight leds.)

> The same constraints would appear if we wanted to be able to define
> more than one pattern.

We'd like to have more than one pattern _engine_, but it should be
enough to have one pattern per pattern engine at a time. 

> It would be best to work out more flexible solution. I wonder if
> ioctl interface isn't the only option.

Well, there's configs, which is more flexible, but...

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]

#1374417

On 04/07/2016 10:45 PM, Pavel Machek wrote:
> Hi!
>
>>>> The "color" attribute would contain "R G B" values. Setting the "color"
>>>> attribute of any of the three LED class devices would affect brightness
>>>> properties (i.e. constituent colors) of the remaining two ones.
>>>> It would result in disabling any active triggers and writing all the
>>>> three color settings to the RGB LED controller at one go.
>>>
>>> Having one attribute across three devices is rather ugly. And we'll
>>> need to solve the pattern issue one day.
>>>
>>> What's tricky about patterns is that you need to control 3 (or more)
>>> leds at a time. Problem you are trying to solve here is ... control of
>>> 3 leds, at the same time.
>>>
>>> So let's solve them together.
>>
>> OK, now I've got your point. So we'd need to have a means for defining
>> patterns. The interface could be located at /sys/class/leds/patterns.
>>
>> We'd need to have a flexible way for defining LED class devices involved
>> in a pattern. Since we cannot guarantee no space in a LED class device
>> name, then a single attribute containing space separated list is not an
>> option. We'd have to create a predefined set of attributes that would
>> contain LED class device name. Predefined implies that it would be
>> a fixed number, i.e. either some attributes would always remain unused
>> or, which is even worse, we could run out of free attributes for some
>> use cases.
>
> There's a better solution: make pattern behave as a trigger for leds
> it controls.
>
> So we'd have
>
> /sys/class/leds/patterns/lp5523
>
> then we'd have
>
> /sys/class/leds/lp5523::red/trigger = "lp5523:1"
> /sys/class/leds/lp5523::green/trigger = "lp5523:2"
> /sys/class/leds/lp5523::blue/trigger = "lp5523:3"
>
> (or something similar, I'd have to boot the n900 to see the exact
> names).

How about implementing patterns as a specific typer of triggers?
Let's say we have ledtrig-rgb-pattern:

After setting a trigger following sysfs attribute would appear
in a LED class device sysfs interface:

$cat /sys/class/lp5523::red/rgb_color
red green blue [none]

$echo "red" > /sys/class/leds/lp5523::red/rgb_color

and similarly

$echo "green" > /sys/class/leds/lp5523::green/rgb_color
$echo "blue" > /sys/class/leds/lp5523::blue/rgb_color

Similar approach could be applied for blink patterns:
There could be additional attributes provided for defining
the position in a blink sequence, or/and blink period.

Now it has become apparent to me that triggers in fact assure
LED class device synchronization.

> That means that we don't need space-separated lists. (And actually
> gives us more flexibility; Maemo for example used the pattern engine
> not for RGB led, but for 6 keyboard backlight leds.)
>
>> The same constraints would appear if we wanted to be able to define
>> more than one pattern.
>
> We'd like to have more than one pattern _engine_, but it should be
> enough to have one pattern per pattern engine at a time.
>
>> It would be best to work out more flexible solution. I wonder if
>> ioctl interface isn't the only option.
>
> Well, there's configs, which is more flexible, but...
>
> Best regards,
>
> 									Pavel
>

-- 
Best regards,
Jacek Anaszewski

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

#1374646

Hi!

> >>>What's tricky about patterns is that you need to control 3 (or more)
> >>>leds at a time. Problem you are trying to solve here is ... control of
> >>>3 leds, at the same time.
> >>>
> >>>So let's solve them together.
> >>
> >>OK, now I've got your point. So we'd need to have a means for defining
> >>patterns. The interface could be located at /sys/class/leds/patterns.
> >>
> >>We'd need to have a flexible way for defining LED class devices involved
> >>in a pattern. Since we cannot guarantee no space in a LED class device
> >>name, then a single attribute containing space separated list is not an
> >>option. We'd have to create a predefined set of attributes that would
> >>contain LED class device name. Predefined implies that it would be
> >>a fixed number, i.e. either some attributes would always remain unused
> >>or, which is even worse, we could run out of free attributes for some
> >>use cases.
> >
> >There's a better solution: make pattern behave as a trigger for leds
> >it controls.
> >
> >So we'd have
> >
> >/sys/class/leds/patterns/lp5523
> >
> >then we'd have
> >
> >/sys/class/leds/lp5523::red/trigger = "lp5523:1"
> >/sys/class/leds/lp5523::green/trigger = "lp5523:2"
> >/sys/class/leds/lp5523::blue/trigger = "lp5523:3"
> >
> >(or something similar, I'd have to boot the n900 to see the exact
> >names).
> 
> How about implementing patterns as a specific typer of triggers?
> Let's say we have ledtrig-rgb-pattern:

Well, we'd need ledtrig-rgb-pattern-1, ledtrig-rgb-pattern-2, ... , as we
can have more than one rgb led. But yes.

> After setting a trigger following sysfs attribute would appear
> in a LED class device sysfs interface:
> 
> $cat /sys/class/lp5523::red/rgb_color
> red green blue [none]
> 
> $echo "red" > /sys/class/leds/lp5523::red/rgb_color
> 
> and similarly
> 
> $echo "green" > /sys/class/leds/lp5523::green/rgb_color
> $echo "blue" > /sys/class/leds/lp5523::blue/rgb_color

Yes, that would work -- selecting channels from the pattern.

> Similar approach could be applied for blink patterns:
> There could be additional attributes provided for defining
> the position in a blink sequence, or/and blink period.

For patterns, I'd suggest array of (r g b time) values.

Pattern engines can do stuff like "slowly turn LED from off to red, then switch color to
white, then slowly turn it to yellow, then turn it off at once" with defined speeds
for "slowly" and option of either linear on non-linear brightness ramping.

The last option might be a bit too much, but I believe we should support the rest.

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]

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

On 04/09/2016 06:01 PM, Pavel Machek wrote:
> Hi!
>
>>>>> What's tricky about patterns is that you need to control 3 (or more)
>>>>> leds at a time. Problem you are trying to solve here is ... control of
>>>>> 3 leds, at the same time.
>>>>>
>>>>> So let's solve them together.
>>>>
>>>> OK, now I've got your point. So we'd need to have a means for defining
>>>> patterns. The interface could be located at /sys/class/leds/patterns.
>>>>
>>>> We'd need to have a flexible way for defining LED class devices involved
>>>> in a pattern. Since we cannot guarantee no space in a LED class device
>>>> name, then a single attribute containing space separated list is not an
>>>> option. We'd have to create a predefined set of attributes that would
>>>> contain LED class device name. Predefined implies that it would be
>>>> a fixed number, i.e. either some attributes would always remain unused
>>>> or, which is even worse, we could run out of free attributes for some
>>>> use cases.
>>>
>>> There's a better solution: make pattern behave as a trigger for leds
>>> it controls.
>>>
>>> So we'd have
>>>
>>> /sys/class/leds/patterns/lp5523
>>>
>>> then we'd have
>>>
>>> /sys/class/leds/lp5523::red/trigger = "lp5523:1"
>>> /sys/class/leds/lp5523::green/trigger = "lp5523:2"
>>> /sys/class/leds/lp5523::blue/trigger = "lp5523:3"
>>>
>>> (or something similar, I'd have to boot the n900 to see the exact
>>> names).
>>
>> How about implementing patterns as a specific typer of triggers?
>> Let's say we have ledtrig-rgb-pattern:
>
> Well, we'd need ledtrig-rgb-pattern-1, ledtrig-rgb-pattern-2, ... , as we
> can have more than one rgb led. But yes.

Triggers can have many listeners, i.e. led_trigger_event() sets
brightness on all LED class devices registered on given trigger.
We could have led_trigger_rgb_event() that would set brightness
on all groups-of-three LEDs registered on given rgb-trigger.

I agree that ledtrig-rgb-pattern-1, ledtrig-rgb-pattern-2, etc. would
be also needed to add a capability of setting different colors on
different LED devices.

>> After setting a trigger following sysfs attribute would appear
>> in a LED class device sysfs interface:
>>
>> $cat /sys/class/lp5523::red/rgb_color
>> red green blue [none]
>>
>> $echo "red" > /sys/class/leds/lp5523::red/rgb_color
>>
>> and similarly
>>
>> $echo "green" > /sys/class/leds/lp5523::green/rgb_color
>> $echo "blue" > /sys/class/leds/lp5523::blue/rgb_color
>
> Yes, that would work -- selecting channels from the pattern.
>
>> Similar approach could be applied for blink patterns:
>> There could be additional attributes provided for defining
>> the position in a blink sequence, or/and blink period.
>
> For patterns, I'd suggest array of (r g b time) values.
>
> Pattern engines can do stuff like "slowly turn LED from off to red, then switch color to
> white, then slowly turn it to yellow, then turn it off at once" with defined speeds
> for "slowly" and option of either linear on non-linear brightness ramping.
>
> The last option might be a bit too much, but I believe we should support the rest.

Yes, that's an interesting idea. It also turns out that trigger based
patterns could be also used for defining generic patterns for a group
of monochrome LEDs.

-- 
Best regards,
Jacek Anaszewski

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

#1379769

Hi!

> >>How about implementing patterns as a specific typer of triggers?
> >>Let's say we have ledtrig-rgb-pattern:
> >
> >Well, we'd need ledtrig-rgb-pattern-1, ledtrig-rgb-pattern-2, ... , as we
> >can have more than one rgb led. But yes.
> 
> Triggers can have many listeners, i.e. led_trigger_event() sets
> brightness on all LED class devices registered on given trigger.
> We could have led_trigger_rgb_event() that would set brightness
> on all groups-of-three LEDs registered on given rgb-trigger.

I do not understand that.

> I agree that ledtrig-rgb-pattern-1, ledtrig-rgb-pattern-2, etc. would
> be also needed to add a capability of setting different colors on
> different LED devices.

Ok.

> >For patterns, I'd suggest array of (r g b time) values.
> >
> >Pattern engines can do stuff like "slowly turn LED from off to red, then switch color to
> >white, then slowly turn it to yellow, then turn it off at once" with defined speeds
> >for "slowly" and option of either linear on non-linear brightness ramping.
> >
> >The last option might be a bit too much, but I believe we should support the rest.
> 
> Yes, that's an interesting idea. It also turns out that trigger based
> patterns could be also used for defining generic patterns for a group
> of monochrome LEDs.

Yes, controlling monochrome LEDs synchronously is another task for
patterns. Actually, N900 uses that to control 6 keyboard backlight
LEDs synchronously... and yes, it would be somehow nice to preserve
this functionality.

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web