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

From: Jacek Anaszewski
Date: Mon Apr 18 2016 - 05:12:43 EST


Hi Pavel,

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

Triggers are defined as kernel source of led events.

Currently we have two types of triggers as far as the source of
led event is concerned:
- triggers that are created per LED class device and therefore each
LED class device has their own source of kernel event,
initialized on trigger activation (e.g. ledtrig-timer,
ledtrig-heartbeat, ledtrig-oneshot),
- triggers that propagate kernel events from one source to many
listeners (e.g.ledtrig-ide-disk, ledtrig-cpu) - they internally
use led_trigger_event(), which iterates through all LED class devices
registered on a trigger and applies the same brightness.

In case of RGB trigger we'd like to have a common source of kernel
events for three LED class devices. After deeper analysis I'd modify
the approach a bit, in order to add a capability of generating kernel
led events from user space.

Let's say that we have LED RGB driver that exposes three LED class
devices: lp5523:red, lp5523:green, lp5523 blue.

$echo "rgb-pattern-lp5523" > /sys/class/leds/lp5523::red/trigger
$echo "red" > /sys/class/leds/lp5523::red/rgb_color
$echo "rgb-pattern-lp5523" > /sys/class/leds/lp5523::green/trigger
$echo "green" > /sys/class/leds/lp5523::green/rgb_color
$echo "rgb-pattern-lp5523" > /sys/class/leds/lp5523::blue/trigger
$echo "blue" > /sys/class/leds/lp5523::blue/rgb_color

led-rgb-pattern trigger would create a new trigger each time a non
existing rgp-pattern-* suffix is passed. In order to make it possible
for the user space to generate trigger events, a dedicated sysfs
interface would have to be exposed. How about creating a new LED RGB
class device that would expose "color" sysfs attribute with three space
separated R G B values? The device would appear in the sysfs after
rgb-pattern trigger is created.

Internally the LED RGB class device would use a new
led_trigger_rgb_event() to set the color:


void led_trigger_rgb_event(struct led_trigger *trig,
enum led_brightness red,
enum led_brightness green,
enum led_brightness blue,
{
struct led_classdev *led_cdev;

if (!trig)
return;

read_lock(&trig->leddev_list_lock);
list_for_each_entry(led_cdev, &trig->led_cdevs, trig_list)
if (led_cdev>flags & LED_RGB_COLOR_R)
led_set_brightness(led_cdev, red);
else if (led_cdev>flags & LED_RGB_COLOR_G)
led_set_brightness(led_cdev, green);
else if (led_cdev>flags & LED_RGB_COLOR_B)
led_set_brightness(led_cdev, blue);
read_unlock(&trig->leddev_list_lock);
}




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,
Jacek Anaszewski