Re: [PATCH RFC] leds: Add status code trigger

Re: [PATCH RFC] leds: Add status code trigger

[Bryan Wu]

On Thu, Feb 19, 2015 at 12:26 AM, Raphaël Teysseyre
<rteysseyre@gmail.com> wrote:
> Hi all,
>
> Here is a new trigger. It would allow userspace to make an LED
> blink with a pattern to show a status (or error) code to a human.
> This is useful for embedded systems without screen or network
> access. Obviously, the LED will keep on blinking even if userspace
> crashes.
>

Thanks for bringing up this. We did get similar idea and patches
before. The problem is people are seeking more complicated
applications like define a script language to do blink such as
"~~~xxxxx~x~x" and there is a decoder to translate this script to
program the leds.

It was pointed out we need another API between kernel and user space,
but I don't have such time to do that.

I will add more people to review this patch and discuss this new feature.

-Bryan


> This trigger exports three properties to sysfs when activated:
> Nblink, period, and pause. It makes the LED blink "Nblink" times
> with a period of "period" milliseconds, then pause for "pause"
> milliseconds, and repeat.
>
> It's been tested on an ARM architecture (Xilinx Zynq 7010 SoC,
> which CPU is a dual ARM Cortex-A9), with a non-mainline
> kernel (xilinx-v2014.4, based of a 3.17.0 kernel).
> Unfortunately, I don't have other hardware to test it on.
> It compiles fine in a 3.19.0-rc1 source tree.
>
> Additional testing and comments would be appreciated.
> Do the properties names seem appropriate to you ? I hesitated
> between Nblink, N_blink, nblink, and n_blink.
>
> Best regards,
>
> Signed-off-by: Raphaël Teysseyre <rteysseyre@gmail.com>
> ---
>  Documentation/leds/ledtrig-statuscode.txt |   41 ++++++
>  drivers/leds/trigger/Kconfig              |    9 ++
>  drivers/leds/trigger/Makefile             |    1 +
>  drivers/leds/trigger/ledtrig-statuscode.c |  194 +++++++++++++++++++++++++++++
>  4 files changed, 245 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/leds/ledtrig-statuscode.txt
>  create mode 100644 drivers/leds/trigger/ledtrig-statuscode.c
>
> diff --git a/Documentation/leds/ledtrig-statuscode.txt b/Documentation/leds/ledtrig-statuscode.txt
> new file mode 100644
> index 0000000..f5ea3ac
> --- /dev/null
> +++ b/Documentation/leds/ledtrig-statuscode.txt
> @@ -0,0 +1,41 @@
> +LED Status code Trigger
> +=======================
> +
> +This trigger allows userspace to make a LED blink with a pattern
> +to show a status (or error) code to a human.
> +
> +Use case : this can be useful for an embedded system without screen
> +or network access. An LED with this trigger can be used to convey
> +a basic "status code" to the final user. If user space crashes,
> +the LED will keep on blinking, which might help debugging the system.
> +
> +This trigger exports three properties : period, pause, and Nblink.
> +The LED will blink "Nblink" times with a period of "period" milliseconds,
> +then pause for "pause" milliseconds, and repeat.
> +
> +When the trigger is activated, its properties are set to default values.
> +
> +       Nblink : Number of LED blinks per cycle. Default value : 1.
> +
> +       period : Period length (in milliseconds). The LED will blink
> +               with this period, with a 50% duty cycle.
> +               Default value : 1000.
> +
> +       pause :  Pause duration (in milliseconds) between each cycle.
> +               Every Nblink blinks, the LED will stay off for this
> +               duration. Default value : 1000.
> +
> +For example, if Nblink = 3, period = 1000, and pause = 2000,
> +the LED will blink with the following pattern :
> +
> + ...OFF  |  ON  |  OFF |  ON  |  OFF |  ON  |         OFF               |   ON  |  OFF...
> +          <     1s    > <     1s    > <0.5s> <            2s           >
> +
> +To get this pattern, the user will have to do :
> +       echo statuscode > trigger
> +       echo 3 > Nblink
> +       echo 1000 > period
> +       echo 2000 > pause
> +
> +When ON the LED will be driven at its max_brightness property. When the trigger
> +is deactivated, the LED is returned to the OFF state.
> diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
> index 49794b4..09f0df5 100644
> --- a/drivers/leds/trigger/Kconfig
> +++ b/drivers/leds/trigger/Kconfig
> @@ -108,4 +108,13 @@ config LEDS_TRIGGER_CAMERA
>           This enables direct flash/torch on/off by the driver, kernel space.
>           If unsure, say Y.
>
> +config LEDS_TRIGGER_STATUSCODE
> +       tristate "LED StatusCode Trigger"
> +       depends on LEDS_TRIGGERS
> +       help
> +         This allows LEDs blinking with a pattern. Can be useful on embedded
> +         systems with no screen to give out a status code to a human.
> +
> +         If unsure, say N.
> +
>  endif # LEDS_TRIGGERS
> diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
> index 1abf48d..8c09e77 100644
> --- a/drivers/leds/trigger/Makefile
> +++ b/drivers/leds/trigger/Makefile
> @@ -8,3 +8,4 @@ obj-$(CONFIG_LEDS_TRIGGER_CPU)          += ledtrig-cpu.o
>  obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON)  += ledtrig-default-on.o
>  obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT)   += ledtrig-transient.o
>  obj-$(CONFIG_LEDS_TRIGGER_CAMERA)      += ledtrig-camera.o
> +obj-$(CONFIG_LEDS_TRIGGER_STATUSCODE)   += ledtrig-statuscode.o
> diff --git a/drivers/leds/trigger/ledtrig-statuscode.c b/drivers/leds/trigger/ledtrig-statuscode.c
> new file mode 100644
> index 0000000..7f0e7ae
> --- /dev/null
> +++ b/drivers/leds/trigger/ledtrig-statuscode.c
> @@ -0,0 +1,194 @@
> +/*
> + * Status code trigger
> + *
> + * Copyright 2015, Epsiline
> + *
> + * Author : Raphaël Teysseyre <rteysseyre@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/leds.h>
> +#include <linux/module.h>
> +#include "../leds.h"
> +
> +struct statuscode_trig_data {
> +       unsigned int period;
> +       unsigned int pause;
> +       unsigned int Nblink;
> +       unsigned int phase;
> +       struct timer_list timer;
> +};
> +
> +static const struct statuscode_trig_data STATUSCODE_TRIG_DATA_DEFAULT = {
> +       .period = 1000,
> +       .pause = 1000,
> +       .Nblink = 1,
> +       .phase = (unsigned int) -1, /* The first call to
> +                                      statuscode_trig_update()
> +                                      will increment this to zero */
> +       .timer = TIMER_INITIALIZER(NULL, 0, 0),
> +};
> +
> +/* --- Sysfs handling --- */
> +
> +#define statuscode_trig_create_attribute(name)                         \
> +                                                                       \
> +       static ssize_t statuscode_trig_show_ ## name                    \
> +       (struct device *dev, struct device_attribute *attr, char *buf)  \
> +       {                                                               \
> +               struct led_classdev *led_cdev = dev_get_drvdata(dev);   \
> +               struct statuscode_trig_data *data = led_cdev->trigger_data; \
> +               return scnprintf(buf, PAGE_SIZE, "%u\n", data->name);   \
> +       }                                                               \
> +                                                                       \
> +       static ssize_t statuscode_trig_store_ ## name                   \
> +       (struct device *dev, struct device_attribute *attr,             \
> +               const char *buf, size_t count)                          \
> +       {                                                               \
> +               struct led_classdev *led_cdev = dev_get_drvdata(dev);   \
> +               struct statuscode_trig_data *data = led_cdev->trigger_data; \
> +               unsigned int res;                                       \
> +               int err = kstrtouint(buf, 10, &res);                    \
> +               if (err)                                                \
> +                       return err;                                     \
> +               data->name = res;                                       \
> +               return count;                                           \
> +       }                                                               \
> +                                                                       \
> +       DEVICE_ATTR(name, S_IRUGO | S_IWUSR,                            \
> +               statuscode_trig_show_ ## name, statuscode_trig_store_ ## name) \
> +
> +statuscode_trig_create_attribute(period);
> +statuscode_trig_create_attribute(pause);
> +statuscode_trig_create_attribute(Nblink);
> +
> +static int statuscode_trig_create_sysfs_files(struct device *dev)
> +{
> +       int err;
> +
> +       err = device_create_file(dev, &dev_attr_period);
> +       if (err)
> +               return err;
> +
> +       err = device_create_file(dev, &dev_attr_pause);
> +       if (err)
> +               goto err_pause;
> +
> +       err = device_create_file(dev, &dev_attr_Nblink);
> +       if (err)
> +               goto err_Nblink;
> +
> +       return 0;
> +
> +err_Nblink:
> +       device_remove_file(dev, &dev_attr_pause);
> +err_pause:
> +       device_remove_file(dev, &dev_attr_period);
> +       return err;
> +}
> +
> +static void statuscode_trig_remove_sysfs_files(struct device *dev)
> +{
> +       device_remove_file(dev, &dev_attr_Nblink);
> +       device_remove_file(dev, &dev_attr_pause);
> +       device_remove_file(dev, &dev_attr_period);
> +}
> +
> +/* --- Led intensity updating --- */
> +
> +/*
> + * For Nblink = 3 :
> + * LED intensity :
> + *       +---+   +---+   +---+         +---+   +---+   +---+
> + * ------+   +---+   +---+   +---------+   +---+   +---+   +------
> + * Phase :
> + *    0  | 1 | 2 | 3 | 4 | 5 |    0    | 1 | 2 | 3 | 4 | 5 |   0
> + *
> + */
> +
> +static void statuscode_trig_update(unsigned long led_cdev_ptr)
> +{
> +       struct led_classdev *led_cdev = (struct led_classdev *) led_cdev_ptr;
> +       struct statuscode_trig_data *data = led_cdev->trigger_data;
> +
> +       data->phase++;
> +       if (data->phase >= 2 * data->Nblink)
> +               data->phase = 0;
> +
> +       if (data->phase % 2)
> +               led_set_brightness(led_cdev, led_cdev->max_brightness);
> +       else
> +               led_set_brightness(led_cdev, LED_OFF);
> +
> +       if (data->phase == 0)
> +               mod_timer(&data->timer, jiffies +
> +                       msecs_to_jiffies(data->pause));
> +       else
> +               mod_timer(&data->timer, jiffies +
> +                       msecs_to_jiffies(data->period / 2));
> +}
> +
> +/* --- Activate / deactivate trigger --- */
> +
> +static void statuscode_trig_activate(struct led_classdev *led_cdev)
> +{
> +       struct statuscode_trig_data *data = NULL;
> +
> +       data = kzalloc(sizeof(*data), GFP_KERNEL);
> +       if (!data)
> +               return;
> +
> +       led_cdev->trigger_data = data;
> +
> +       *data = STATUSCODE_TRIG_DATA_DEFAULT;
> +       setup_timer(&data->timer,
> +               statuscode_trig_update,
> +               (unsigned long) led_cdev);
> +
> +       statuscode_trig_create_sysfs_files(led_cdev->dev);
> +       statuscode_trig_update(data->timer.data);
> +}
> +
> +static void statuscode_trig_deactivate(struct led_classdev *led_cdev)
> +{
> +       struct statuscode_trig_data *data = led_cdev->trigger_data;
> +
> +       if (data) {
> +               statuscode_trig_remove_sysfs_files(led_cdev->dev);
> +               del_timer_sync(&data->timer);
> +               led_set_brightness(led_cdev, LED_OFF);
> +               kfree(data);
> +               led_cdev->trigger_data = NULL;
> +       }
> +}
> +
> +/* --- Module loading/unloading --- */
> +
> +static struct led_trigger statuscode_led_trigger = {
> +       .name = "statuscode",
> +       .activate = statuscode_trig_activate,
> +       .deactivate = statuscode_trig_deactivate,
> +};
> +
> +static int __init statuscode_trig_init(void)
> +{
> +       return led_trigger_register(&statuscode_led_trigger);
> +}
> +
> +static void __exit statuscode_trig_exit(void)
> +{
> +       led_trigger_unregister(&statuscode_led_trigger);
> +}
> +
> +module_init(statuscode_trig_init);
> +module_exit(statuscode_trig_exit);
> +
> +MODULE_AUTHOR("Raphael Teysseyre <rteysseyre@gmail.com");
> +MODULE_DESCRIPTION("Statuscode LED trigger");
> +MODULE_LICENSE("GPL");
> --
> 1.7.1
>
>
>

Re: [PATCH RFC] leds: Add status code trigger

[Pavel Machek]

Hi!

> > Here is a new trigger. It would allow userspace to make an LED
> > blink with a pattern to show a status (or error) code to a human.
> > This is useful for embedded systems without screen or network
> > access. Obviously, the LED will keep on blinking even if userspace
> > crashes.
> 
> Thanks for bringing up this. We did get similar idea and patches
> before. The problem is people are seeking more complicated
> applications like define a script language to do blink such as
> "~~~xxxxx~x~x" and there is a decoder to translate this script to
> program the leds.
> 
> It was pointed out we need another API between kernel and user space,
> but I don't have such time to do that.
> 
> I will add more people to review this patch and discuss this new
> feature.

And then there are LEDs that can do smooth brightness transitions, and
LEDs that provide RGB colors, and programmable LED engines.

I think I have interface to handle most of the issues: array of
integers in "brightness, length in miliseconds".

So for example slowly blinking LED would be "0 1000msec, 255 1000
msec". On off pattern would be "0 1000msec, 255 0msec, 255 1000msec, 0
0msec".

Rapahael, this should be able to do all the stuff you want to do. It
can also do morse code, slow blinks, and combination of both. Do you
want to try implementing it?

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

Re: [PATCH RFC] leds: Add status code trigger

[Pavel Machek]

Hi!

> This patch is very similar to mine patten trigger and all features are covered by my patch.
> If you are considering to take this feature, could you consider my patch instead?
> http://thread.gmane.org/gmane.linux.documentation/19116/focus=19135

That one is indeed better than this patch (as it supports almost
arbitrary blinking pattern on single LED, it will not help much with
smooth brightness control.

Would solution below work for you?

Best regards,
									Pavel

> > I think I have interface to handle most of the issues: array of
> > integers in "brightness, length in miliseconds".
> > 
> > So for example slowly blinking LED would be "0 1000msec, 255 1000
> > msec". On off pattern would be "0 1000msec, 255 0msec, 255 1000msec, 0
> > 0msec".
> > 
> > Rapahael, this should be able to do all the stuff you want to do. It
> > can also do morse code, slow blinks, and combination of both. Do you
> > want to try implementing it?
>  		 	   		  

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

Re: [PATCH RFC] leds: Add status code trigger

[Raphaël Teysseyre]

Le mardi 10 mars 2015 à 20:22 +0100, Pavel Machek a écrit :
> Hi!
> 
> > This patch is very similar to mine patten trigger and all features are covered by my patch.
> > If you are considering to take this feature, could you consider my patch instead?
> > http://thread.gmane.org/gmane.linux.documentation/19116/focus=19135
> 
> That one is indeed better than this patch (as it supports almost
> arbitrary blinking pattern on single LED, it will not help much with
> smooth brightness control.

I agree, that patch supports more features than mine, I didn't find it
while browsing the mailing list archive.

> 
> Would solution below work for you?
> 
> Best regards,
> 									Pavel
> 
> > > I think I have interface to handle most of the issues: array of
> > > integers in "brightness, length in miliseconds".
> > > 
> > > So for example slowly blinking LED would be "0 1000msec, 255 1000
> > > msec". On off pattern would be "0 1000msec, 255 0msec, 255 1000msec, 0
> > > 0msec".
> > > 
> > > Rapahael, this should be able to do all the stuff you want to do. It
> > > can also do morse code, slow blinks, and combination of both. Do you
> > > want to try implementing it?
> >  		 	   		  
> 

I could implement it. This is how I'd do it :
When activated, the trigger would export three
sysfs attributes : pattern, pattern_repeat, and pattern_once.

	pattern : list of brightness/time couple, separated by newlines
		For the slowly blinking LED of your example,
		this attribute would contain :
		0 1000
		255 1000

		For a heartbeat-like pattern :
		0 1000
		255 100
		0 100
		255 100

	pattern_repeat : repeat this pattern indefinitely ?
		1 -> yes, 0 -> no

	pattern_once : if pattern_repeat = 0, writing 1 to this
		file makes the pattern run once .

Does that seem OK to you?

Regards,
Raphaël

Re: [PATCH RFC] leds: Add status code trigger

[Pavel Machek]

Hi!

> > > This patch is very similar to mine patten trigger and all features are covered by my patch.
> > > If you are considering to take this feature, could you consider my patch instead?
> > > http://thread.gmane.org/gmane.linux.documentation/19116/focus=19135
> > 
> > That one is indeed better than this patch (as it supports almost
> > arbitrary blinking pattern on single LED, it will not help much with
> > smooth brightness control.
> 
> I agree, that patch supports more features than mine, I didn't find it
> while browsing the mailing list archive.

> > Would solution below work for you?

> > > > I think I have interface to handle most of the issues: array of
> > > > integers in "brightness, length in miliseconds".
> > > > 
> > > > So for example slowly blinking LED would be "0 1000msec, 255 1000
> > > > msec". On off pattern would be "0 1000msec, 255 0msec, 255 1000msec, 0
> > > > 0msec".
> > > > 
> > > > Rapahael, this should be able to do all the stuff you want to do. It
> > > > can also do morse code, slow blinks, and combination of both. Do you
> > > > want to try implementing it?
> > >  		 	   		  
> > 
> 
> I could implement it. This is how I'd do it :
> When activated, the trigger would export three
> sysfs attributes : pattern, pattern_repeat, and pattern_once.
> 
> 	pattern : list of brightness/time couple, separated by newlines
> 		For the slowly blinking LED of your example,
> 		this attribute would contain :
> 		0 1000
> 		255 1000
> 
> 		For a heartbeat-like pattern :
> 		0 1000
> 		255 100
> 		0 100
> 		255 100

That would be "slowly go to zero intensity, quickly go to max
intensity, to zero, to maximum". If you wanted to turn the LED on/off
without going through partial brightnesses, you'd do

	        0 1000
		255 0
		255 100
		0 0
		0 100
		255 0
		255 100
		0 0

> 	pattern_repeat : repeat this pattern indefinitely ?
> 		1 -> yes, 0 -> no
> 
> 	pattern_once : if pattern_repeat = 0, writing 1 to this
> 		file makes the pattern run once .

I'd do attribute "repeat" with number of repetitions, or -1 == "repeat
forever" default value.

Best regards,
									Pavel

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

Re: [PATCH RFC] leds: Add status code trigger

[Raphaël Teysseyre]

> > 		For a heartbeat-like pattern :
> > 		0 1000
> > 		255 100
> > 		0 100
> > 		255 100
> 
> That would be "slowly go to zero intensity, quickly go to max
> intensity, to zero, to maximum". If you wanted to turn the LED on/off
> without going through partial brightnesses, you'd do
> 
> 	        0 1000
> 		255 0
> 		255 100
> 		0 0
> 		0 100
> 		255 0
> 		255 100
> 		0 0
> 
Okay, now I get why there were zero-time lines in your second example.

> > 	pattern_repeat : repeat this pattern indefinitely ?
> > 		1 -> yes, 0 -> no
> > 
> > 	pattern_once : if pattern_repeat = 0, writing 1 to this
> > 		file makes the pattern run once .
> 
> I'd do attribute "repeat" with number of repetitions, or -1 == "repeat
> forever" default value.
> 

Yes that's better.

Best regards,
Raphaël

[PATCH] leds: Add arbitrary pattern trigger

[Raphaël Teysseyre]

Hi all,

Following our discussion about [PATCH RFC] leds: Add status code trigger,
here is a patch allowing arbitrary patterns. This is intended for
embedded systems without screen or network access to show a status (or
error) code to a human.

It's been tested on an ARM architecture (Xilinx Zynq 7010 SoC,
which CPU is a dual ARM Cortex-A9), with a non-mainline
kernel (xilinx-v2014.4, based of a 3.17.0 kernel).
Unfortunately, I don't have other hardware to test it on.
It compiles fine in a 3.19.0-rc1 source tree.
Additional testing and comments would be appreciated.




Add a new led trigger supporting arbitrary patterns.
This is useful for embedded systems with no screen or network access.

Export two sysfs attributes: "pattern", and "repeat".

When "repeat"=-1, repeat the pattern indefinitely. Otherwise,
repeat it the specified number of times. "pattern" specifies
the pattern as comma separated couples of "brightness duration"
values. See detailed documentation in patch.

Signed-off-by: Raphaël Teysseyre <rteysseyre@gmail.com>
---
 Documentation/leds/ledtrig-pattern.txt |   80 +++++++
 drivers/leds/trigger/Kconfig           |   10 +
 drivers/leds/trigger/Makefile          |    1 +
 drivers/leds/trigger/ledtrig-pattern.c |  388 ++++++++++++++++++++++++++++++++
 4 files changed, 479 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/leds/ledtrig-pattern.txt
 create mode 100644 drivers/leds/trigger/ledtrig-pattern.c

diff --git a/Documentation/leds/ledtrig-pattern.txt b/Documentation/leds/ledtrig-pattern.txt
new file mode 100644
index 0000000..68d11d2
--- /dev/null
+++ b/Documentation/leds/ledtrig-pattern.txt
@@ -0,0 +1,80 @@
+LED Pattern Trigger
+===================
+
+This is a LED trigger allowing arbitrary pattern execution. It can do gradual
+dimming. This trigger can be configured to repeat the pattern a number of
+times or indefinitely. This is intended as a way of communication for embedded
+systems with no screen.
+
+The trigger can be activated from user space on led class devices as shown
+below:
+
+  echo pattern > trigger
+
+This adds the following sysfs attributes to the LED:
+
+  pattern - specifies the pattern. See syntax below.
+
+  repeat - number of times the pattern must be repeated.
+	writing -1 to this file will make the pattern
+	repeat indefinitely.
+
+The pattern will be restarted each time a new value is written to
+the pattern or repeat attribute. When dimming, the LED brightness
+is set every 50 ms.
+
+pattern syntax:
+The pattern is specified in the pattern attribute with an array of comma-
+separated "brightness/length in miliseconds" values. The two components
+of each value are to be separated by a space.
+
+For example, assuming the driven LED supports
+intensity value from 0 to 255:
+
+  echo 0 1000, 255 2000 > pattern
+
+Will make the LED go gradually from zero-intensity to max (255) intensity
+in 1000 milliseconds, then back to zero intensity in 2000 milliseconds:
+
+LED brightness
+    ^
+255-|       / \            / \            /
+    |      /    \         /    \         /
+    |     /       \      /       \      /
+    |    /          \   /          \   /
+  0-|   /             \/             \/
+    +---0----1----2----3----4----5----6------------> time (s)
+
+
+
+To make the LED go instantly from one brigntess value to another,
+use zero-time lengths. For example:
+
+  echo 0 1000, 0 0, 255 2000, 255 0 > pattern
+
+Will make the LED stay off for one second, then stay at max brightness
+for two seconds:
+
+LED brightness
+    ^
+255-|        +---------+    +---------+
+    |        |         |    |         |
+    |        |         |    |         |
+    |        |         |    |         |
+  0-|   -----+         +----+         +----
+    +---0----1----2----3----4----5----6------------> time (s)
+
+
+
+Patterns with less than two values, no value with time length > 50
+milliseconds, or no two values with differing brightnesses will
+result in the LED being set at the brightness of the first value,
+or zero if the pattern contains no value.
+
+Patterns with invalid syntax are silently swallowed and will result
+in undefined behavior of the LED brightness.
+
+Because sysfs is used to define the pattern, patterns that need more than
+PAGE_SIZE characters to describe aren't supported. PAGE_SIZE is system
+dependent.
+
diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index 49794b4..ce27bdb 100644
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -108,4 +108,14 @@ config LEDS_TRIGGER_CAMERA
 	  This enables direct flash/torch on/off by the driver, kernel space.
 	  If unsure, say Y.
 
+config LEDS_TRIGGER_PATTERN
+	tristate "LED Pattern Trigger"
+	depends on LEDS_TRIGGERS
+	help
+	  This allows LEDs blinking with an arbitrary pattern. Can be useful
+	  on embedded systems with no screen to give out a status code to
+	  a human.
+
+	  If unsure, say N
+
 endif # LEDS_TRIGGERS
diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
index 1abf48d..a739429 100644
--- a/drivers/leds/trigger/Makefile
+++ b/drivers/leds/trigger/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_LEDS_TRIGGER_CPU)		+= ledtrig-cpu.o
 obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON)	+= ledtrig-default-on.o
 obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT)	+= ledtrig-transient.o
 obj-$(CONFIG_LEDS_TRIGGER_CAMERA)	+= ledtrig-camera.o
+obj-$(CONFIG_LEDS_TRIGGER_PATTERN)	+= ledtrig-pattern.o
diff --git a/drivers/leds/trigger/ledtrig-pattern.c b/drivers/leds/trigger/ledtrig-pattern.c
new file mode 100644
index 0000000..da290b3
--- /dev/null
+++ b/drivers/leds/trigger/ledtrig-pattern.c
@@ -0,0 +1,388 @@
+/*
+ * Arbitrary pattern trigger
+ *
+ * Copyright 2015, Epsiline
+ *
+ * Author : Raphaël Teysseyre <rteysseyre@gmail.com>
+ *
+ * Idea discussed with Pavel Machek <pavel@ucw.cz> on
+ * <linux-leds@vger.kernel.org> (march 2015, thread title
+ * [PATCH RFC] leds: Add status code trigger)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include "../leds.h"
+
+struct pattern_step {
+	int brightness;
+	int time_ms;
+};
+
+struct pattern_trig_data {
+	struct pattern_step *steps; /* Array describing the pattern */
+	struct mutex lock;
+	char is_sane;
+	struct pattern_step *curr;
+	struct pattern_step *next;
+	int time_ms; /* Time in current step */
+	int nsteps;  /* Number of steps */
+	int repeat;  /* < 0 means repeat indefinitely */
+	struct timer_list timer; /* Used for scheduling updates to
+				    the led brightness */
+};
+
+#define MAX_NSTEPS (PAGE_SIZE/4)
+/* The "pattern" attribute contains at most PAGE_SIZE characters.
+   Each line in this attribute is at least 4 characters long
+   (a 1-digit number for the led brighntess, a space,
+   a 1-digit number for the time, a semi-colon).
+   Therefore, there is at most PAGE_SIZE/4 steps. */
+
+#define UPDATE_INTERVAL 50
+/* When doing gradual dimming, the led brightness
+   will be updated every UPDATE_INTERVAL milliseconds */
+
+#define PATTERN_SEPARATOR ","
+
+static int pattern_trig_initialize_data(struct pattern_trig_data *data)
+{
+	mutex_init(&data->lock);
+	mutex_lock(&data->lock);
+
+	data->is_sane = 0;
+	data->steps = kzalloc(MAX_NSTEPS*sizeof(struct pattern_step),
+			GFP_KERNEL);
+	if (!data->steps)
+		return -ENOMEM;
+
+	data->curr = NULL;
+	data->next = NULL;
+	data->time_ms = 0;
+	data->nsteps = 0;
+	data->repeat = -1;
+	init_timer(&data->timer);
+
+	mutex_unlock(&data->lock);
+	return 0;
+}
+
+static void pattern_trig_clear_data(struct pattern_trig_data *data)
+{
+	data->is_sane = 0;
+	kfree(data->steps);
+}
+
+/*
+ *  is_sane : pattern checking.
+ *  A pattern satisfaying these three conditions is reported as sane :
+ *    - At least two steps
+ *    - At least one step with time >= UPDATE_INTERVAL
+ *    - At least two steps with differing brightnesses
+ *  When @data isn't sane, a sensible brightness
+ *  default is suggested in @brightness
+ *
+ * DO NOT call pattern_trig_update on a not-sane pattern,
+ * you'll be punished with an infinite loop in the kernel.
+ */
+static int is_sane(struct pattern_trig_data *data, int *brightness)
+{
+	int i;
+	char stept_ok = 0;
+	char stepb_ok = 0;
+
+	*brightness = 0;
+	if (data->nsteps < 1)
+		return 0;
+
+	*brightness = data->steps[0].brightness;
+	if (data->nsteps < 2)
+		return 0;
+
+	for (i = 0; i < data->nsteps; i++) {
+		if (data->steps[i].time_ms >= UPDATE_INTERVAL) {
+			if (stepb_ok)
+				return 1;
+			stept_ok = 1;
+		}
+		if (data->steps[i].brightness != data->steps[0].brightness) {
+			if (stept_ok)
+				return 1;
+			stepb_ok = 1;
+		}
+	}
+
+	return 0;
+}
+
+static void reset_pattern(struct pattern_trig_data *data,
+			struct led_classdev *led_cdev)
+{
+	int brightness;
+
+	del_timer_sync(&data->timer);
+
+	if (is_sane(data, &brightness)) {
+		data->curr = data->steps;
+		data->next = data->steps + 1;
+		data->time_ms = 0;
+		data->is_sane = 1;
+
+		data->timer.expires = jiffies;
+		add_timer(&data->timer);
+	} else
+		led_set_brightness(led_cdev, brightness);
+}
+
+/* --- Sysfs handling --- */
+
+static ssize_t pattern_trig_show_repeat(
+	struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct pattern_trig_data *data = led_cdev->trigger_data;
+
+	return scnprintf(buf, PAGE_SIZE, "%d\n", data->repeat);
+}
+
+static ssize_t pattern_trig_store_repeat(
+	struct device *dev, struct device_attribute *attr,
+	const char *buf, size_t count)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct pattern_trig_data *data = led_cdev->trigger_data;
+	long res;
+	int err;
+
+	err = kstrtol(buf, 10, &res);
+	if (err)
+		return err;
+
+	data->repeat = res < 0 ? -1 : res;
+	reset_pattern(data, led_cdev);
+
+	return count;
+}
+
+DEVICE_ATTR(repeat, S_IRUGO | S_IWUSR,
+	pattern_trig_show_repeat, pattern_trig_store_repeat);
+
+static ssize_t pattern_trig_show_pattern(
+	struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct pattern_trig_data *data = led_cdev->trigger_data;
+	ssize_t count = 0;
+	int i;
+
+	if (!data->steps || !data->nsteps)
+		return 0;
+
+	for (i = 0; i < data->nsteps; i++)
+		count += scnprintf(buf + count, PAGE_SIZE - count,
+				"%d %d" PATTERN_SEPARATOR,
+				data->steps[i].brightness,
+				data->steps[i].time_ms);
+	buf[count - 1] = '\n';
+	buf[count] = '\0';
+
+	return count + 1;
+}
+
+static ssize_t pattern_trig_store_pattern(
+	struct device *dev, struct device_attribute *attr,
+	const char *buf, size_t count)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct pattern_trig_data *data = led_cdev->trigger_data;
+	int cr = 0; /* Characters read on one conversion */
+	int tcr = 0; /* Total characters read */
+	int ccount; /* Number of successful conversions */
+
+	mutex_lock(&data->lock);
+	data->is_sane = 0;
+
+	for (data->nsteps = 0; data->nsteps < MAX_NSTEPS; data->nsteps++) {
+		cr = 0;
+		ccount = sscanf(buf + tcr, "%d %d " PATTERN_SEPARATOR "%n",
+			&data->steps[data->nsteps].brightness,
+			&data->steps[data->nsteps].time_ms, &cr);
+
+		if (!cr) { /* Invalid syntax or end of pattern */
+			if (ccount == 2)
+				data->nsteps++;
+			mutex_unlock(&data->lock);
+			reset_pattern(data, led_cdev);
+			return count;
+		}
+
+		tcr += cr;
+	}
+
+	/* Shouldn't reach that */
+	WARN(1, "MAX_NSTEP too small. Please report\n");
+	mutex_unlock(&data->lock);
+	return count;
+}
+
+DEVICE_ATTR(pattern, S_IRUGO | S_IWUSR,
+	pattern_trig_show_pattern, pattern_trig_store_pattern);
+
+static int pattern_trig_create_sysfs_files(struct device *dev)
+{
+	int err;
+
+	err = device_create_file(dev, &dev_attr_repeat);
+	if (err)
+		return err;
+
+	err = device_create_file(dev, &dev_attr_pattern);
+	if (err)
+		device_remove_file(dev, &dev_attr_repeat);
+
+	return err;
+}
+
+static void pattern_trig_remove_sysfs_files(struct device *dev)
+{
+	device_remove_file(dev, &dev_attr_pattern);
+	device_remove_file(dev, &dev_attr_repeat);
+}
+
+/* --- Led intensity updating --- */
+
+static int compute_brightness(struct pattern_trig_data *data)
+{
+	if (data->time_ms == 0)
+		return data->curr->brightness;
+
+	if (data->curr->time_ms == 0)
+		return data->next->brightness;
+
+	return data->curr->brightness + data->time_ms
+		* (data->next->brightness - data->curr->brightness)
+		/ data->curr->time_ms;
+}
+
+static void update_to_next_step(struct pattern_trig_data *data)
+{
+	data->curr = data->next;
+	if (data->curr == data->steps)
+		data->repeat--;
+
+	if (data->next == data->steps + data->nsteps - 1)
+		data->next = data->steps;
+	else
+		data->next++;
+
+	data->time_ms = 0;
+}
+
+static void pattern_trig_update(unsigned long led_cdev_ptr)
+{
+	struct led_classdev *led_cdev = (struct led_classdev *) led_cdev_ptr;
+	struct pattern_trig_data *data = led_cdev->trigger_data;
+
+	mutex_lock(&data->lock);
+
+	if (!data->is_sane || !data->repeat) {
+		mutex_unlock(&data->lock);
+		return;
+	}
+
+	if (data->time_ms > data->curr->time_ms)
+		update_to_next_step(data);
+
+	/* is_sane() checked that there is at least
+	   one step with time_ms >= UPDATE_INTERVAL
+	   so we won't go in an infinite loop */
+	while (data->curr->time_ms < UPDATE_INTERVAL)
+		update_to_next_step(data);
+
+	if (data->next->brightness == data->curr->brightness) {
+		/* Constant brightness for this step */
+		led_set_brightness(led_cdev, data->curr->brightness);
+		mod_timer(&data->timer, jiffies
+			+ msecs_to_jiffies(data->curr->time_ms));
+		update_to_next_step(data);
+	} else {
+		/* Gradual dimming */
+		led_set_brightness(led_cdev, compute_brightness(data));
+		data->time_ms += UPDATE_INTERVAL;
+		mod_timer(&data->timer, jiffies
+			+ msecs_to_jiffies(UPDATE_INTERVAL));
+	}
+
+	mutex_unlock(&data->lock);
+}
+
+/* --- Trigger activation --- */
+
+static void pattern_trig_activate(struct led_classdev *led_cdev)
+{
+	struct pattern_trig_data *data = NULL;
+	int err;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return;
+
+	err = pattern_trig_initialize_data(data);
+	if (err) {
+		kfree(data);
+		return;
+	}
+
+	led_cdev->trigger_data = data;
+	setup_timer(&data->timer, pattern_trig_update,
+		(unsigned long) led_cdev);
+	pattern_trig_create_sysfs_files(led_cdev->dev);
+}
+
+static void pattern_trig_deactivate(struct led_classdev *led_cdev)
+{
+	struct pattern_trig_data *data = led_cdev->trigger_data;
+
+	if (data) {
+		pattern_trig_remove_sysfs_files(led_cdev->dev);
+		del_timer_sync(&data->timer);
+		led_set_brightness(led_cdev, LED_OFF);
+		pattern_trig_clear_data(data);
+		kfree(data);
+		led_cdev->trigger_data = NULL;
+	}
+}
+
+static struct led_trigger pattern_led_trigger = {
+	.name = "pattern",
+	.activate = pattern_trig_activate,
+	.deactivate = pattern_trig_deactivate,
+};
+
+/* --- Module loading/unloading --- */
+
+static int __init pattern_trig_init(void)
+{
+	return led_trigger_register(&pattern_led_trigger);
+}
+
+static void __exit pattern_trig_exit(void)
+{
+	led_trigger_unregister(&pattern_led_trigger);
+}
+
+module_init(pattern_trig_init);
+module_exit(pattern_trig_exit);
+
+MODULE_AUTHOR("Raphael Teysseyre <rteysseyre@gmail.com");
+MODULE_DESCRIPTION("Statuscode LED trigger");
+MODULE_LICENSE("GPL");
-- 
1.7.1

Re: [PATCH] leds: Add arbitrary pattern trigger

[Pavel Machek]

Hi!

> Additional testing and comments would be appreciated.

git complains about newline at EOF.

Nice ascii art ;-).

Got this while playing with my hid-sony patches:

[  460.025901] snd_hda_intel 0000:00:1b.0: IRQ timing workaround is
activated for card #0. Suggest a bigger bdl_pos_adj.
[  826.164068] ------------[ cut here ]------------
[  826.164088] WARNING: CPU: 0 PID: 0 at kernel/locking/mutex.c:535
mutex_lock_nested+0x2ba/0x2d0()
[  826.164092] DEBUG_LOCKS_WARN_ON(in_interrupt())
[  826.164095] Modules linked in:

[  826.164105] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W
4.0.0-rc3+ #57
[  826.164109] Hardware name: LENOVO 17097HU/17097HU, BIOS 7BETD8WW
(2.19 ) 03/31/2011
[  826.164112]  00000000 00000000 f5c1bee0 c486806a f5c1bf0c f5c1befc
c403a42b 00000217
[  826.164130]  c486d8fa ecfb3f44 ecfb3f44 f48b6a10 f5c1bf14 c403a486
00000009 f5c1bf0c
[  826.164146]  c4aa13a9 f5c1bf28 f5c1bf54 c486d8fa c4aa13c1 00000217
c4aa13a9 c4aa139a
[  826.164162] Call Trace:
[  826.164171]  [<c486806a>] dump_stack+0x41/0x52
[  826.164182]  [<c403a42b>] warn_slowpath_common+0x6b/0xa0
[  826.164189]  [<c486d8fa>] ? mutex_lock_nested+0x2ba/0x2d0
[  826.164196]  [<c403a486>] warn_slowpath_fmt+0x26/0x30
[  826.164204]  [<c486d8fa>] mutex_lock_nested+0x2ba/0x2d0
[  826.164213]  [<c461af4d>] pattern_trig_update+0x1d/0xe0
[  826.164221]  [<c408ef20>] ? process_timeout+0x10/0x10
[  826.164228]  [<c461af30>] ? pattern_trig_deactivate+0x60/0x60
[  826.164235]  [<c408ef6f>] call_timer_fn+0x4f/0xc0
[  826.164242]  [<c408ef20>] ? process_timeout+0x10/0x10
[  826.164248]  [<c461af30>] ? pattern_trig_deactivate+0x60/0x60
[  826.164255]  [<c408f781>] run_timer_softirq+0x171/0x210
[  826.164262]  [<c403d384>] __do_softirq+0xd4/0x1c0
[  826.164269]  [<c403d2b0>] ? cpu_callback+0x120/0x120
[  826.164276]  [<c400409d>] do_softirq_own_stack+0x1d/0x30
[  826.164280]  <IRQ>  [<c403d57e>] irq_exit+0x6e/0x90
[  826.164291]  [<c4003a70>] do_IRQ+0x40/0xb0
[  826.164299]  [<c48706b0>] common_interrupt+0x30/0x38
[  826.164307]  [<c45ff30a>] ? cpuidle_enter_state+0x3a/0xe0
[  826.164314]  [<c45ff3cf>] cpuidle_enter+0xf/0x20
[  826.164322]  [<c406c114>] cpu_startup_entry+0x234/0x320
[  826.164331]  [<c4865092>] rest_init+0x92/0xa0
[  826.164341]  [<c4c49ab4>] start_kernel+0x36b/0x372
[  826.164348]  [<c4c492d4>] i386_start_kernel+0x82/0x86
[  826.164352] ---[ end trace a011c72f14dcd310 ]---
[  942.239519] Sony: init

I'd suggest following fix for the documentation.

Thanks,
							Pavel

diff --git a/Documentation/leds/ledtrig-pattern.txt b/Documentation/leds/ledtrig-pattern.txt
index 2db2021..cf14381 100644
--- a/Documentation/leds/ledtrig-pattern.txt
+++ b/Documentation/leds/ledtrig-pattern.txt
@@ -6,7 +6,7 @@ dimming. This trigger can be configured to repeat the pattern a number of
 times or indefinitely. This is intended as a way of communication for embedded
 systems with no screen.
 
-The trigger can be activated from user space on led class devices as shown
+The trigger can be activated from user space on LED class devices as shown
 below:
 
   echo pattern > trigger

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

Re: [PATCH] leds: Add arbitrary pattern trigger

[Raphaël Teysseyre]

> Got this while playing with my hid-sony patches:

Sorry about that, it's me being dumb and using a mutex in an atomic context...

Raphaël

Re: [PATCH] leds: Add arbitrary pattern trigger

[Pavel Machek]

Hi!

I'd really return error if we can't parse user's input.

Small spelling fixes:

--- drivers/leds/trigger/ledtrig-pattern.c~	2015-03-16 12:07:06.897408622 +0100
+++ drivers/leds/trigger/ledtrig-pattern.c	2015-03-16 13:08:25.801864534 +0100
@@ -38,7 +38,7 @@
 	int nsteps;  /* Number of steps */
 	int repeat;  /* < 0 means repeat indefinitely */
 	struct timer_list timer; /* Used for scheduling updates to
-				    the led brightness */
+				    the LED brightness */
 };
 
 #define MAX_NSTEPS (PAGE_SIZE/4)
@@ -84,7 +84,7 @@
 
 /*
  *  is_sane : pattern checking.
- *  A pattern satisfaying these three conditions is reported as sane :
+ *  A pattern satisfying these three conditions is reported as sane :
  *    - At least two steps
  *    - At least one step with time >= UPDATE_INTERVAL
  *    - At least two steps with differing brightnesses


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

Re: [PATCH] leds: Add arbitrary pattern trigger

[Paul Bolle]

Just a license nit.

On Mon, 2015-03-16 at 11:06 +0100, Raphaël Teysseyre wrote:
> --- /dev/null
> +++ b/drivers/leds/trigger/ledtrig-pattern.c

> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.

This states that the license is GPL v2.

> +MODULE_LICENSE("GPL");

And
    MODULE_LICENSE("GPL v2");

would match that statement.


Paul Bolle