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

From: Pavel Machek
Date: Wed Apr 06 2016 - 05:12:37 EST


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