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

[Jacek Anaszewski]

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