linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] leds: ledtrig-morse: send out morse code
@ 2018-06-28 13:42 Andreas Klinger
  2018-06-28 13:54 ` Greg KH
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: Andreas Klinger @ 2018-06-28 13:42 UTC (permalink / raw)
  To: jacek.anaszewski, pavel, ben.whitten, geert+renesas, w, ak,
	pombredanne, gregkh, linux-kernel, linux-leds

Send out a morse code by using LEDs.

This is useful especially on embedded systems without displays to tell the
user about error conditions and status information.

The trigger will be called "morse"

The string to be send is written into the file morse_string and sent out
with a workqueue. Supported are letters and digits.

With the file dot_unit the minimal time unit can be adjusted in
milliseconds.

Signed-off-by: Andreas Klinger <ak@it-klinger.de>
---
 drivers/leds/trigger/Kconfig         |  10 ++
 drivers/leds/trigger/Makefile        |   1 +
 drivers/leds/trigger/ledtrig-morse.c | 298 +++++++++++++++++++++++++++++++++++
 3 files changed, 309 insertions(+)
 create mode 100644 drivers/leds/trigger/ledtrig-morse.c

diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index a2559b4fdfff..ea706ef2354c 100644
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -142,4 +142,14 @@ config LEDS_TRIGGER_NETDEV
 	  This allows LEDs to be controlled by network device activity.
 	  If unsure, say Y.
 
+config LEDS_TRIGGER_MORSE
+	tristate "LED Morse Trigger"
+	depends on LEDS_TRIGGERS
+	help
+	  This allows to send a morse code through LEDs.
+	  It is useful especially in embedded systems when there is only
+	  little interface to tell the user error or status codes. Sending
+	  a morse code can be an alternative here.
+	  If unsure, say Y.
+
 endif # LEDS_TRIGGERS
diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
index f3cfe1950538..5735381cc3d3 100644
--- a/drivers/leds/trigger/Makefile
+++ b/drivers/leds/trigger/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT)	+= ledtrig-transient.o
 obj-$(CONFIG_LEDS_TRIGGER_CAMERA)	+= ledtrig-camera.o
 obj-$(CONFIG_LEDS_TRIGGER_PANIC)	+= ledtrig-panic.o
 obj-$(CONFIG_LEDS_TRIGGER_NETDEV)	+= ledtrig-netdev.o
+obj-$(CONFIG_LEDS_TRIGGER_MORSE)	+= ledtrig-morse.o
diff --git a/drivers/leds/trigger/ledtrig-morse.c b/drivers/leds/trigger/ledtrig-morse.c
new file mode 100644
index 000000000000..77f6ee502ebe
--- /dev/null
+++ b/drivers/leds/trigger/ledtrig-morse.c
@@ -0,0 +1,298 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ledtrig-morse: LED Morse Trigger
+ *
+ * send a string as morse code out through LEDs
+ *
+ * can be used to send error codes or messages
+ *
+ * string to be send is written into morse_string
+ * supported are letters and digits
+ *
+ * Author: Andreas Klinger <ak@it-klinger.de>
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/ctype.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/workqueue.h>
+#include <linux/leds.h>
+
+
+#define MORSE_DOT_UNIT_DEFAULT	500
+#define MORSE_TELEGRAM_SIZE	100
+
+struct morse_data {
+	unsigned int		dot_unit;
+	struct led_classdev	*led_cdev;
+	struct work_struct	work;
+	char			telegram[MORSE_TELEGRAM_SIZE];
+	unsigned int		telegram_size;
+	struct mutex		lock;
+};
+
+struct morse_char {
+	char	c;
+	char	*z;
+};
+
+static struct morse_char morse_table[] = {
+	{'a', ".-S"},
+	{'b', "-...S"},
+	{'c', "-.-.S"},
+	{'d', "-..S"},
+	{'e', ".S"},
+	{'f', "..-.S"},
+	{'g', "--.S"},
+	{'h', "....S"},
+	{'i', "..S"},
+	{'j', ".---S"},
+	{'k', "-.-S"},
+	{'l', ".-..S"},
+	{'m', "--S"},
+	{'n', "-.S"},
+	{'o', "---S"},
+	{'p', ".--.S"},
+	{'q', "--.-S"},
+	{'r', ".-.S"},
+	{'s', "...S"},
+	{'t', "-S"},
+	{'u', "..-S"},
+	{'v', "...-S"},
+	{'w', ".--S"},
+	{'x', "-..-S"},
+	{'y', "-.--S"},
+	{'z', "--..S"},
+	{'1', ".----S"},
+	{'2', "..---S"},
+	{'3', "...--S"},
+	{'4', "....-S"},
+	{'5', ".....S"},
+	{'6', "-....S"},
+	{'7', "--...S"},
+	{'8', "---..S"},
+	{'9', "----.S"},
+	{'0', "-----S"},
+	{0, NULL},
+};
+
+static void morse_long(struct led_classdev *led_cdev)
+{
+	struct morse_data *data = led_cdev->trigger_data;
+
+	led_set_brightness(led_cdev, LED_ON);
+	msleep(3 * data->dot_unit);
+	led_set_brightness(led_cdev, LED_OFF);
+	msleep(data->dot_unit);
+}
+
+static void morse_short(struct led_classdev *led_cdev)
+{
+	struct morse_data *data = led_cdev->trigger_data;
+
+	led_set_brightness(led_cdev, LED_ON);
+	msleep(data->dot_unit);
+	led_set_brightness(led_cdev, LED_OFF);
+	msleep(data->dot_unit);
+}
+
+static void morse_letter_space(struct led_classdev *led_cdev)
+{
+	struct morse_data *data = led_cdev->trigger_data;
+	/*
+	 * Pause: 3 dot spaces
+	 * 1 dot space already there from morse character
+	 */
+	msleep(2 * data->dot_unit);
+}
+
+static void morse_word_space(struct led_classdev *led_cdev)
+{
+	struct morse_data *data = led_cdev->trigger_data;
+	/*
+	 * Pause: 7 dot spaces
+	 * 1 dot space already there from morse character
+	 * 2 dot spaces already there from letter space
+	 */
+	msleep(4 * data->dot_unit);
+}
+
+static void morse_send_char(struct led_classdev *led_cdev, char ch)
+{
+	int i = 0;
+
+	while ((morse_table[i].c) && (morse_table[i].c != tolower(ch)))
+		i++;
+
+	if (morse_table[i].c) {
+		int j = 0;
+
+		while (morse_table[i].z[j] != 'S') {
+			switch (morse_table[i].z[j]) {
+			case '.':
+				morse_short(led_cdev);
+				break;
+			case '-':
+				morse_long(led_cdev);
+				break;
+			}
+			j++;
+		}
+		morse_letter_space(led_cdev);
+	} else {
+		/*
+		 * keep it simple:
+		 * whenever there is an unrecognized character make a word
+		 * space
+		 */
+		morse_word_space(led_cdev);
+	}
+}
+
+static void morse_work(struct work_struct *work)
+{
+	struct morse_data *data = container_of(work, struct morse_data, work);
+	int i;
+
+	mutex_lock(&data->lock);
+
+	for (i = 0; i < data->telegram_size; i++)
+		morse_send_char(data->led_cdev, data->telegram[i]);
+
+	mutex_unlock(&data->lock);
+}
+
+static ssize_t morse_string_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t size)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct morse_data *data = led_cdev->trigger_data;
+
+	if (size >= sizeof(data->telegram))
+		return -E2BIG;
+
+	mutex_lock(&data->lock);
+
+	memcpy(data->telegram, buf, size);
+	data->telegram_size = size;
+
+	mutex_unlock(&data->lock);
+
+	schedule_work(&data->work);
+
+	return size;
+}
+
+static DEVICE_ATTR_WO(morse_string);
+
+static ssize_t dot_unit_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct morse_data *data = led_cdev->trigger_data;
+
+	return sprintf(buf, "%u\n", data->dot_unit);
+}
+
+static ssize_t dot_unit_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t size)
+{
+	struct led_classdev *led_cdev = dev_get_drvdata(dev);
+	struct morse_data *data = led_cdev->trigger_data;
+	unsigned long dot_unit;
+	ssize_t ret = -EINVAL;
+
+	ret = kstrtoul(buf, 10, &dot_unit);
+	if (ret)
+		return ret;
+
+	data->dot_unit = dot_unit;
+
+	return size;
+}
+
+static DEVICE_ATTR_RW(dot_unit);
+
+static void morse_trig_activate(struct led_classdev *led_cdev)
+{
+	int rc;
+	struct morse_data *data;
+
+	data = kzalloc(sizeof(struct morse_data), GFP_KERNEL);
+	if (!data) {
+		dev_err(led_cdev->dev, "unable to allocate morse trigger\n");
+		return;
+	}
+
+	led_cdev->trigger_data = data;
+	data->led_cdev = led_cdev;
+	data->dot_unit = MORSE_DOT_UNIT_DEFAULT;
+
+	rc = device_create_file(led_cdev->dev, &dev_attr_morse_string);
+	if (rc)
+		goto err_out_data;
+
+	rc = device_create_file(led_cdev->dev, &dev_attr_dot_unit);
+	if (rc)
+		goto err_out_morse_string;
+
+	INIT_WORK(&data->work, morse_work);
+
+	mutex_init(&data->lock);
+
+	led_set_brightness(led_cdev, LED_OFF);
+	led_cdev->activated = true;
+
+	return;
+
+err_out_data:
+	kfree(data);
+err_out_morse_string:
+	device_remove_file(led_cdev->dev, &dev_attr_morse_string);
+}
+
+static void morse_trig_deactivate(struct led_classdev *led_cdev)
+{
+	struct morse_data *data = led_cdev->trigger_data;
+
+	if (led_cdev->activated) {
+
+		cancel_work_sync(&data->work);
+
+		device_remove_file(led_cdev->dev, &dev_attr_morse_string);
+		device_remove_file(led_cdev->dev, &dev_attr_dot_unit);
+
+		kfree(data);
+
+		led_cdev->trigger_data = NULL;
+		led_cdev->activated = false;
+	}
+}
+
+static struct led_trigger morse_led_trigger = {
+	.name     = "morse",
+	.activate = morse_trig_activate,
+	.deactivate = morse_trig_deactivate,
+};
+
+static int __init morse_trig_init(void)
+{
+	return led_trigger_register(&morse_led_trigger);
+}
+
+static void __exit morse_trig_exit(void)
+{
+	led_trigger_unregister(&morse_led_trigger);
+}
+
+module_init(morse_trig_init);
+module_exit(morse_trig_exit);
+
+MODULE_AUTHOR("Andreas Klinger <ak@it-klinger.de>");
+MODULE_DESCRIPTION("Morse code LED trigger");
+MODULE_LICENSE("GPL");
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] leds: ledtrig-morse: send out morse code
  2018-06-28 13:42 [PATCH] leds: ledtrig-morse: send out morse code Andreas Klinger
@ 2018-06-28 13:54 ` Greg KH
  2018-06-28 15:36 ` Geert Uytterhoeven
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2018-06-28 13:54 UTC (permalink / raw)
  To: Andreas Klinger
  Cc: jacek.anaszewski, pavel, ben.whitten, geert+renesas, w,
	pombredanne, linux-kernel, linux-leds

On Thu, Jun 28, 2018 at 03:42:27PM +0200, Andreas Klinger wrote:
> Send out a morse code by using LEDs.
> 
> This is useful especially on embedded systems without displays to tell the
> user about error conditions and status information.
> 
> The trigger will be called "morse"
> 
> The string to be send is written into the file morse_string and sent out
> with a workqueue. Supported are letters and digits.
> 
> With the file dot_unit the minimal time unit can be adjusted in
> milliseconds.

This is great stuff, nice work.

But you need to document your new sysfs files in Documentation/ABI/
somewhere, so that people know how to use this.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] leds: ledtrig-morse: send out morse code
  2018-06-28 13:42 [PATCH] leds: ledtrig-morse: send out morse code Andreas Klinger
  2018-06-28 13:54 ` Greg KH
@ 2018-06-28 15:36 ` Geert Uytterhoeven
  2018-06-28 18:21 ` Willy Tarreau
  2018-06-28 18:56 ` Pavel Machek
  3 siblings, 0 replies; 16+ messages in thread
From: Geert Uytterhoeven @ 2018-06-28 15:36 UTC (permalink / raw)
  To: ak
  Cc: Jacek Anaszewski, Pavel Machek, Ben Whitten, Geert Uytterhoeven,
	Willy Tarreau, Philippe Ombredanne, Greg KH,
	Linux Kernel Mailing List, linux-leds

Hi Andreas,

On Thu, Jun 28, 2018 at 3:42 PM Andreas Klinger <ak@it-klinger.de> wrote:
> Send out a morse code by using LEDs.
>
> This is useful especially on embedded systems without displays to tell the
> user about error conditions and status information.
>
> The trigger will be called "morse"
>
> The string to be send is written into the file morse_string and sent out
> with a workqueue. Supported are letters and digits.
>
> With the file dot_unit the minimal time unit can be adjusted in
> milliseconds.
>
> Signed-off-by: Andreas Klinger <ak@it-klinger.de>\

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/leds/trigger/ledtrig-morse.c
> @@ -0,0 +1,298 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ledtrig-morse: LED Morse Trigger
> + *
> + * send a string as morse code out through LEDs
> + *
> + * can be used to send error codes or messages
> + *
> + * string to be send is written into morse_string
> + * supported are letters and digits
> + *
> + * Author: Andreas Klinger <ak@it-klinger.de>
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/device.h>
> +#include <linux/ctype.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/workqueue.h>
> +#include <linux/leds.h>
> +
> +
> +#define MORSE_DOT_UNIT_DEFAULT 500
> +#define MORSE_TELEGRAM_SIZE    100
> +
> +struct morse_data {
> +       unsigned int            dot_unit;
> +       struct led_classdev     *led_cdev;
> +       struct work_struct      work;
> +       char                    telegram[MORSE_TELEGRAM_SIZE];
> +       unsigned int            telegram_size;
> +       struct mutex            lock;
> +};
> +
> +struct morse_char {
> +       char    c;
> +       char    *z;
> +};
> +
> +static struct morse_char morse_table[] = {
> +       {'a', ".-S"},

What's the added value of the trailing "S", which is present in each string,
over the standard NUL terminator, which is also present?

Given no character uses more than 5 symbols, a more compact encoding
(e.g. 3 bits for length, 5 bits for symbols) could be used.
But it may not be worth doing that optimization. And you may want to add
support for more characters later.

> +static void morse_send_char(struct led_classdev *led_cdev, char ch)
> +{
> +       int i = 0;

unsigned int

> +
> +       while ((morse_table[i].c) && (morse_table[i].c != tolower(ch)))
> +               i++;
> +
> +       if (morse_table[i].c) {
> +               int j = 0;

unsigned int

> +
> +               while (morse_table[i].z[j] != 'S') {

Without the trailing "S"es, you could just check for "morse_table[i].z[j]".

> +                       switch (morse_table[i].z[j]) {
> +                       case '.':
> +                               morse_short(led_cdev);
> +                               break;
> +                       case '-':
> +                               morse_long(led_cdev);
> +                               break;
> +                       }
> +                       j++;
> +               }
> +               morse_letter_space(led_cdev);
> +       } else {
> +               /*
> +                * keep it simple:
> +                * whenever there is an unrecognized character make a word
> +                * space
> +                */
> +               morse_word_space(led_cdev);
> +       }
> +}
> +
> +static void morse_work(struct work_struct *work)
> +{
> +       struct morse_data *data = container_of(work, struct morse_data, work);
> +       int i;

unsigned int i;

> +
> +       mutex_lock(&data->lock);
> +
> +       for (i = 0; i < data->telegram_size; i++)
> +               morse_send_char(data->led_cdev, data->telegram[i]);
> +
> +       mutex_unlock(&data->lock);
> +}

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] leds: ledtrig-morse: send out morse code
  2018-06-28 13:42 [PATCH] leds: ledtrig-morse: send out morse code Andreas Klinger
  2018-06-28 13:54 ` Greg KH
  2018-06-28 15:36 ` Geert Uytterhoeven
@ 2018-06-28 18:21 ` Willy Tarreau
  2018-06-28 18:56 ` Pavel Machek
  3 siblings, 0 replies; 16+ messages in thread
From: Willy Tarreau @ 2018-06-28 18:21 UTC (permalink / raw)
  To: Andreas Klinger
  Cc: jacek.anaszewski, pavel, ben.whitten, geert+renesas, pombredanne,
	gregkh, linux-kernel, linux-leds

On Thu, Jun 28, 2018 at 03:42:27PM +0200, Andreas Klinger wrote:
> Send out a morse code by using LEDs.
> 
> This is useful especially on embedded systems without displays to tell the
> user about error conditions and status information.
> 
> The trigger will be called "morse"
> 
> The string to be send is written into the file morse_string and sent out
> with a workqueue. Supported are letters and digits.
> 
> With the file dot_unit the minimal time unit can be adjusted in
> milliseconds.

Wow, I really love the idea!

Willy

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] leds: ledtrig-morse: send out morse code
  2018-06-28 13:42 [PATCH] leds: ledtrig-morse: send out morse code Andreas Klinger
                   ` (2 preceding siblings ...)
  2018-06-28 18:21 ` Willy Tarreau
@ 2018-06-28 18:56 ` Pavel Machek
  2018-06-28 20:29   ` Andreas Klinger
  2018-06-28 20:33   ` Andreas Klinger
  3 siblings, 2 replies; 16+ messages in thread
From: Pavel Machek @ 2018-06-28 18:56 UTC (permalink / raw)
  To: Andreas Klinger
  Cc: jacek.anaszewski, ben.whitten, geert+renesas, w, pombredanne,
	gregkh, linux-kernel, linux-leds


[-- Attachment #1.1: Type: text/plain, Size: 856 bytes --]

Hi!

> Send out a morse code by using LEDs.
> 
> This is useful especially on embedded systems without displays to tell the
> user about error conditions and status information.
> 
> The trigger will be called "morse"
> 
> The string to be send is written into the file morse_string and sent out
> with a workqueue. Supported are letters and digits.
> 
> With the file dot_unit the minimal time unit can be adjusted in
> milliseconds.
> 
> Signed-off-by: Andreas Klinger <ak@it-klinger.de>

Can we get more general "pattern" trigger? Some LEDs can do that in
hardware, and it is more general than plain morse.

Ouch and it already was implemented :-). Patch is in attachment.

									Pavel

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

[-- Attachment #1.2: delme --]
[-- Type: text/plain, Size: 20486 bytes --]

From rteysseyre@gmail.com Fri Mar 20 15:40:22 2015
Return-Path: <rteysseyre@gmail.com>
X-Original-To: pavel@atrey.karlin.mff.cuni.cz
Delivered-To: pavel@atrey.karlin.mff.cuni.cz
Received: from jabberwock.ucw.cz (jabberwock.ucw.cz [46.255.230.98])
	by atrey.karlin.mff.cuni.cz (Postfix) with ESMTP id 3275281ECB
	for <pavel@atrey.karlin.mff.cuni.cz>; Fri, 20 Mar 2015 15:40:22 +0100 (CET)
Received: by jabberwock.ucw.cz (Postfix)
	id 3053A1C00FF; Fri, 20 Mar 2015 15:40:22 +0100 (CET)
Delivered-To: pavel@jabberwock.ucw.cz
Received: from mail-wg0-f46.google.com (mail-wg0-f46.google.com [74.125.82.46])
	(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))
	(Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (not verified))
	by jabberwock.ucw.cz (Postfix) with ESMTPS id 18EB61C00F9
	for <pavel@ucw.cz>; Fri, 20 Mar 2015 15:40:22 +0100 (CET)
Received: by wgbcc7 with SMTP id cc7so91022009wgb.0
        for <pavel@ucw.cz>; Fri, 20 Mar 2015 07:40:21 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
        d=gmail.com; s=20120113;
        h=subject:from:to:cc:content-type:date:message-id:mime-version
         :content-transfer-encoding;
        bh=UdT+u6nHVkpFGm2AqZ0XFePjLX83mCQTkcoTCraWWvc=;
        b=b9VbjaS+0McMAtdeSJv1Lhb6RDg/noWghgIsbIZD/DpMnIXJ05Dyi160Jx28uS0YzT
         zyPmtmEv3IrnmkcCp7g6S6f04ATgi9q3tI+5qooaQS65V7bRtlpcTMoB8KEA38SsBtap
         8Ng+65MHchSpmGyR/h0WxVg934ieNpySSLCQ/vSUu2lOxi0SLmyr93KqfZXQ6Yk5FCA3
         QOcCuiXpcLfmGbLwR2YT0uu3/60LOmWL6xt+J/bkWjo38qwqtH5phoa5ydjcUy5MZJSZ
         tn4qM8mQ8YH9EUJ+VjcxQX4nYdY+Mf3tsnSOieHNzUVt+2z1GmLcDi1Jfe8MtyPh8+9m
         dp+w==
X-Received: by 10.194.108.137 with SMTP id hk9mr118158926wjb.112.1426862421666;
        Fri, 20 Mar 2015 07:40:21 -0700 (PDT)
Received: from [192.168.244.132] (fs-141-0-203-237.fullsave.info. [141.0.203.237])
        by mx.google.com with ESMTPSA id ha10sm6624401wjc.37.2015.03.20.07.40.20
        (version=SSLv3 cipher=RC4-SHA bits=128/128);
        Fri, 20 Mar 2015 07:40:20 -0700 (PDT)
Subject: [PATCH v2] leds: Add arbitrary pattern trigger
From: =?ISO-8859-1?Q?Rapha=EBl?= Teysseyre <rteysseyre@gmail.com>
To: Pavel Machek <pavel@ucw.cz>
Cc: Joe Xue <lgxue@hotmail.com>, Bryan Wu <cooloney@gmail.com>, 
 "rpurdie@rpsys.net" <rpurdie@rpsys.net>, Linux LED Subsystem
 <linux-leds@vger.kernel.org>, lkml <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Date: Fri, 20 Mar 2015 15:42:02 +0100
Message-ID: <1426862522.48232.3.camel@localhost>
Mime-Version: 1.0
X-Mailer: Evolution 2.32.3 (2.32.3-34.el6) 
Content-Transfer-Encoding: 8bit
X-CRM114-Status: Good  ( pR: 191.7191 )
Status: RO
Content-Length: 17146

Hi all,

Following your comments about [PATCH] leds: Add arbitrary pattern trigger,
here is a revised version of this patch.

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.

Changes since version 1:
 - Fixed typos in documentation and comments.
 - Fixed MODULE_LICENSE string.
 - No more mutex in atomic context.
 - Return EINVAL when invalid patterns are written to the "pattern" attribute.



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 detailled documentation in patch.

Signed-off-by: Raphaël Teysseyre <rteysseyre@gmail.com>
---
 Documentation/leds/ledtrig-pattern.txt |   86 +++++++
 drivers/leds/trigger/Kconfig           |   10 +
 drivers/leds/trigger/Makefile          |    1 +
 drivers/leds/trigger/ledtrig-pattern.c |  432 ++++++++++++++++++++++++++++++++
 4 files changed, 529 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..579295e
--- /dev/null
+++ b/Documentation/leds/ledtrig-pattern.txt
@@ -0,0 +1,86 @@
+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
+
+Or:
+
+  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)
+
+
+Notes:
+
+Patterns with invalid syntax are reported with EINVAL, the
+resulting LED brightness is undefined. Reading the pattern
+back returns an empty string.
+
+Patterns with less than two values, no value with time length > 50
+milliseconds, or no two values with differing brightnesses
+result in the LED being set at the brightness of the first value,
+or zero if the pattern contains no value. EINVAL is returned,
+and reading the pattern back returns an empty string.
+
+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..fdf231d
--- /dev/null
+++ b/drivers/leds/trigger/ledtrig-pattern.c
@@ -0,0 +1,432 @@
+/*
+ * 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 workqueue_struct *queue;
+	struct delayed_work dwork;
+	struct led_classdev *led_cdev; /* Needed by pattern_trig_update() */
+};
+
+#define UPDATE_INTERVAL 50
+/* When doing gradual dimming, the led brightness
+   will be updated every UPDATE_INTERVAL milliseconds */
+
+#define PATTERN_SEPARATOR ","
+
+#define MAX_NSTEPS (PAGE_SIZE/4)
+/* The "pattern" attribute contains at most PAGE_SIZE characters.
+   Each pattern step in this attribute needs at least 4 characters
+   (a 1-digit number for the led brighntess, a space,
+   a 1-digit number for the time, a PATTERN_SEPARATOR).
+   Therefore, there is at most PAGE_SIZE/4 steps. */
+
+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;
+	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 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
+ *  When @data->pattern isn't sane, a sensible brightness
+ *  default is suggested in @brightness
+ *
+ * DO NOT call pattern_trig_update() on a not-sane pattern
+ * with is_sane = 1, 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 int reset_pattern(struct pattern_trig_data *data,
+			struct led_classdev *led_cdev)
+{
+	int brightness;
+
+	mutex_lock(&data->lock);
+	data->is_sane = 0; /* Prevent pattern_trig_update()
+			      from scheduling new work */
+	mutex_unlock(&data->lock);
+
+	flush_workqueue(data->queue);
+
+	mutex_lock(&data->lock);
+	if (is_sane(data, &brightness)) {
+		data->curr = data->steps;
+		data->next = data->steps + 1;
+		data->time_ms = 0;
+		data->is_sane = 1;
+		queue_delayed_work(data->queue, &data->dwork, 0);
+		mutex_unlock(&data->lock);
+		return 0;
+	}
+
+	mutex_unlock(&data->lock);
+	led_set_brightness(led_cdev, brightness);
+	return -EINVAL;
+}
+
+/* --- 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;
+
+	mutex_lock(&data->lock);
+	data->repeat = res < 0 ? -1 : res;
+	mutex_unlock(&data->lock);
+
+	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 after a successful sscanf call */
+	int ccr = 0; /* Characters read before looking for PATTERN_SEPARATOR */
+	int tcr = 0; /* Total characters read in buf */
+	int ccount;  /* Number of successful conversions in a sscanf call */
+
+	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 %n" PATTERN_SEPARATOR " %n",
+				&data->steps[data->nsteps].brightness,
+				&data->steps[data->nsteps].time_ms, &ccr, &cr);
+
+		if (!cr) { /* PATTERN_SEPARATOR not reached */
+			if (ccount == 2) {
+				/* Successful conversion before
+				   looking for PATTERN_SEPARATOR. */
+				data->nsteps++;
+				tcr += ccr;
+			}
+
+			if (tcr != count) {
+				/* Invalid syntax */
+				data->nsteps = 0;
+				mutex_unlock(&data->lock);
+				return -EINVAL;
+			}
+
+			mutex_unlock(&data->lock);
+
+			if (reset_pattern(data, led_cdev)) {
+				/* Invalid pattern */
+				data->nsteps = 0;
+				return -EINVAL;
+			}
+
+			return count;
+		}
+
+		tcr += cr;
+	}
+
+	/* Shouldn't reach that */
+	WARN(1, "MAX_NSTEP too small. Please report\n");
+	data->nsteps = 0;
+	mutex_unlock(&data->lock);
+	return -EINVAL;
+}
+
+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) /* Don't divide by zero */
+		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(struct work_struct *work)
+{
+	struct delayed_work *dwork = to_delayed_work(work);
+	struct pattern_trig_data *data =
+		container_of(dwork, struct pattern_trig_data, dwork);
+
+	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(data->led_cdev, data->curr->brightness);
+		queue_delayed_work(data->queue, &data->dwork,
+				msecs_to_jiffies(data->curr->time_ms));
+		update_to_next_step(data);
+	} else {
+		/* Gradual dimming */
+		led_set_brightness(data->led_cdev, compute_brightness(data));
+		data->time_ms += UPDATE_INTERVAL;
+		queue_delayed_work(data->queue, &data->dwork,
+				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;
+	data->led_cdev = led_cdev;
+	data->queue = alloc_ordered_workqueue("pattern trigger on %s", 0,
+					led_cdev->name ? led_cdev->name : "?");
+	INIT_DELAYED_WORK(&data->dwork, pattern_trig_update);
+	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);
+
+		mutex_lock(&data->lock);
+		data->is_sane = 0; /* Prevent pattern_trig_update()
+				      from scheduling new work */
+		mutex_unlock(&data->lock);
+
+		flush_workqueue(data->queue);
+		destroy_workqueue(data->queue);
+
+		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 v2");
-- 
1.7.1




[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] leds: ledtrig-morse: send out morse code
  2018-06-28 18:56 ` Pavel Machek
@ 2018-06-28 20:29   ` Andreas Klinger
  2018-06-28 20:45     ` Pavel Machek
  2018-06-28 20:33   ` Andreas Klinger
  1 sibling, 1 reply; 16+ messages in thread
From: Andreas Klinger @ 2018-06-28 20:29 UTC (permalink / raw)
  To: Pavel Machek
  Cc: jacek.anaszewski, ben.whitten, geert+renesas, w, pombredanne,
	gregkh, linux-kernel, linux-leds

Hi Pavel,

Pavel Machek <pavel@ucw.cz> schrieb am Thu, 28. Jun 20:56:
> Hi!
> 
> > Send out a morse code by using LEDs.
> > 
> > This is useful especially on embedded systems without displays to tell the
> > user about error conditions and status information.
> > 
> > The trigger will be called "morse"
> > 
> > The string to be send is written into the file morse_string and sent out
> > with a workqueue. Supported are letters and digits.
> > 
> > With the file dot_unit the minimal time unit can be adjusted in
> > milliseconds.
> > 
> > Signed-off-by: Andreas Klinger <ak@it-klinger.de>
> 
> Can we get more general "pattern" trigger? Some LEDs can do that in
> hardware, and it is more general than plain morse.
> 
> Ouch and it already was implemented :-). Patch is in attachment.
> 
> 									Pavel

The idea of the morse trigger is so be able so send out a short error
code or if needed also a complete sentence. The morse code don't need
extra explanation to the user. Decoding can be found everywhere.

With the pattern trigger one need a translation of the letters and
numbers into morse code. This is what the morse trigger is doing.

So from my perspective the pattern trigger is something different than
the morse one.

Another question:
The pattern trigger is not in mainline. Do you know why?

Andreas


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

> From rteysseyre@gmail.com Fri Mar 20 15:40:22 2015
> Return-Path: <rteysseyre@gmail.com>
> X-Original-To: pavel@atrey.karlin.mff.cuni.cz
> Delivered-To: pavel@atrey.karlin.mff.cuni.cz
> Received: from jabberwock.ucw.cz (jabberwock.ucw.cz [46.255.230.98])
> 	by atrey.karlin.mff.cuni.cz (Postfix) with ESMTP id 3275281ECB
> 	for <pavel@atrey.karlin.mff.cuni.cz>; Fri, 20 Mar 2015 15:40:22 +0100 (CET)
> Received: by jabberwock.ucw.cz (Postfix)
> 	id 3053A1C00FF; Fri, 20 Mar 2015 15:40:22 +0100 (CET)
> Delivered-To: pavel@jabberwock.ucw.cz
> Received: from mail-wg0-f46.google.com (mail-wg0-f46.google.com [74.125.82.46])
> 	(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))
> 	(Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (not verified))
> 	by jabberwock.ucw.cz (Postfix) with ESMTPS id 18EB61C00F9
> 	for <pavel@ucw.cz>; Fri, 20 Mar 2015 15:40:22 +0100 (CET)
> Received: by wgbcc7 with SMTP id cc7so91022009wgb.0
>         for <pavel@ucw.cz>; Fri, 20 Mar 2015 07:40:21 -0700 (PDT)
> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
>         d=gmail.com; s=20120113;
>         h=subject:from:to:cc:content-type:date:message-id:mime-version
>          :content-transfer-encoding;
>         bh=UdT+u6nHVkpFGm2AqZ0XFePjLX83mCQTkcoTCraWWvc=;
>         b=b9VbjaS+0McMAtdeSJv1Lhb6RDg/noWghgIsbIZD/DpMnIXJ05Dyi160Jx28uS0YzT
>          zyPmtmEv3IrnmkcCp7g6S6f04ATgi9q3tI+5qooaQS65V7bRtlpcTMoB8KEA38SsBtap
>          8Ng+65MHchSpmGyR/h0WxVg934ieNpySSLCQ/vSUu2lOxi0SLmyr93KqfZXQ6Yk5FCA3
>          QOcCuiXpcLfmGbLwR2YT0uu3/60LOmWL6xt+J/bkWjo38qwqtH5phoa5ydjcUy5MZJSZ
>          tn4qM8mQ8YH9EUJ+VjcxQX4nYdY+Mf3tsnSOieHNzUVt+2z1GmLcDi1Jfe8MtyPh8+9m
>          dp+w==
> X-Received: by 10.194.108.137 with SMTP id hk9mr118158926wjb.112.1426862421666;
>         Fri, 20 Mar 2015 07:40:21 -0700 (PDT)
> Received: from [192.168.244.132] (fs-141-0-203-237.fullsave.info. [141.0.203.237])
>         by mx.google.com with ESMTPSA id ha10sm6624401wjc.37.2015.03.20.07.40.20
>         (version=SSLv3 cipher=RC4-SHA bits=128/128);
>         Fri, 20 Mar 2015 07:40:20 -0700 (PDT)
> Subject: [PATCH v2] leds: Add arbitrary pattern trigger
> From: =?ISO-8859-1?Q?Rapha=EBl?= Teysseyre <rteysseyre@gmail.com>
> To: Pavel Machek <pavel@ucw.cz>
> Cc: Joe Xue <lgxue@hotmail.com>, Bryan Wu <cooloney@gmail.com>, 
>  "rpurdie@rpsys.net" <rpurdie@rpsys.net>, Linux LED Subsystem
>  <linux-leds@vger.kernel.org>, lkml <linux-kernel@vger.kernel.org>
> Content-Type: text/plain; charset="UTF-8"
> Date: Fri, 20 Mar 2015 15:42:02 +0100
> Message-ID: <1426862522.48232.3.camel@localhost>
> Mime-Version: 1.0
> X-Mailer: Evolution 2.32.3 (2.32.3-34.el6) 
> Content-Transfer-Encoding: 8bit
> X-CRM114-Status: Good  ( pR: 191.7191 )
> Status: RO
> Content-Length: 17146
> 
> Hi all,
> 
> Following your comments about [PATCH] leds: Add arbitrary pattern trigger,
> here is a revised version of this patch.
> 
> 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.
> 
> Changes since version 1:
>  - Fixed typos in documentation and comments.
>  - Fixed MODULE_LICENSE string.
>  - No more mutex in atomic context.
>  - Return EINVAL when invalid patterns are written to the "pattern" attribute.
> 
> 
> 
> 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 detailled documentation in patch.
> 
> Signed-off-by: Raphaël Teysseyre <rteysseyre@gmail.com>
> ---
>  Documentation/leds/ledtrig-pattern.txt |   86 +++++++
>  drivers/leds/trigger/Kconfig           |   10 +
>  drivers/leds/trigger/Makefile          |    1 +
>  drivers/leds/trigger/ledtrig-pattern.c |  432 ++++++++++++++++++++++++++++++++
>  4 files changed, 529 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..579295e
> --- /dev/null
> +++ b/Documentation/leds/ledtrig-pattern.txt
> @@ -0,0 +1,86 @@
> +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
> +
> +Or:
> +
> +  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)
> +
> +
> +Notes:
> +
> +Patterns with invalid syntax are reported with EINVAL, the
> +resulting LED brightness is undefined. Reading the pattern
> +back returns an empty string.
> +
> +Patterns with less than two values, no value with time length > 50
> +milliseconds, or no two values with differing brightnesses
> +result in the LED being set at the brightness of the first value,
> +or zero if the pattern contains no value. EINVAL is returned,
> +and reading the pattern back returns an empty string.
> +
> +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..fdf231d
> --- /dev/null
> +++ b/drivers/leds/trigger/ledtrig-pattern.c
> @@ -0,0 +1,432 @@
> +/*
> + * 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 workqueue_struct *queue;
> +	struct delayed_work dwork;
> +	struct led_classdev *led_cdev; /* Needed by pattern_trig_update() */
> +};
> +
> +#define UPDATE_INTERVAL 50
> +/* When doing gradual dimming, the led brightness
> +   will be updated every UPDATE_INTERVAL milliseconds */
> +
> +#define PATTERN_SEPARATOR ","
> +
> +#define MAX_NSTEPS (PAGE_SIZE/4)
> +/* The "pattern" attribute contains at most PAGE_SIZE characters.
> +   Each pattern step in this attribute needs at least 4 characters
> +   (a 1-digit number for the led brighntess, a space,
> +   a 1-digit number for the time, a PATTERN_SEPARATOR).
> +   Therefore, there is at most PAGE_SIZE/4 steps. */
> +
> +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;
> +	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 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
> + *  When @data->pattern isn't sane, a sensible brightness
> + *  default is suggested in @brightness
> + *
> + * DO NOT call pattern_trig_update() on a not-sane pattern
> + * with is_sane = 1, 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 int reset_pattern(struct pattern_trig_data *data,
> +			struct led_classdev *led_cdev)
> +{
> +	int brightness;
> +
> +	mutex_lock(&data->lock);
> +	data->is_sane = 0; /* Prevent pattern_trig_update()
> +			      from scheduling new work */
> +	mutex_unlock(&data->lock);
> +
> +	flush_workqueue(data->queue);
> +
> +	mutex_lock(&data->lock);
> +	if (is_sane(data, &brightness)) {
> +		data->curr = data->steps;
> +		data->next = data->steps + 1;
> +		data->time_ms = 0;
> +		data->is_sane = 1;
> +		queue_delayed_work(data->queue, &data->dwork, 0);
> +		mutex_unlock(&data->lock);
> +		return 0;
> +	}
> +
> +	mutex_unlock(&data->lock);
> +	led_set_brightness(led_cdev, brightness);
> +	return -EINVAL;
> +}
> +
> +/* --- 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;
> +
> +	mutex_lock(&data->lock);
> +	data->repeat = res < 0 ? -1 : res;
> +	mutex_unlock(&data->lock);
> +
> +	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 after a successful sscanf call */
> +	int ccr = 0; /* Characters read before looking for PATTERN_SEPARATOR */
> +	int tcr = 0; /* Total characters read in buf */
> +	int ccount;  /* Number of successful conversions in a sscanf call */
> +
> +	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 %n" PATTERN_SEPARATOR " %n",
> +				&data->steps[data->nsteps].brightness,
> +				&data->steps[data->nsteps].time_ms, &ccr, &cr);
> +
> +		if (!cr) { /* PATTERN_SEPARATOR not reached */
> +			if (ccount == 2) {
> +				/* Successful conversion before
> +				   looking for PATTERN_SEPARATOR. */
> +				data->nsteps++;
> +				tcr += ccr;
> +			}
> +
> +			if (tcr != count) {
> +				/* Invalid syntax */
> +				data->nsteps = 0;
> +				mutex_unlock(&data->lock);
> +				return -EINVAL;
> +			}
> +
> +			mutex_unlock(&data->lock);
> +
> +			if (reset_pattern(data, led_cdev)) {
> +				/* Invalid pattern */
> +				data->nsteps = 0;
> +				return -EINVAL;
> +			}
> +
> +			return count;
> +		}
> +
> +		tcr += cr;
> +	}
> +
> +	/* Shouldn't reach that */
> +	WARN(1, "MAX_NSTEP too small. Please report\n");
> +	data->nsteps = 0;
> +	mutex_unlock(&data->lock);
> +	return -EINVAL;
> +}
> +
> +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) /* Don't divide by zero */
> +		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(struct work_struct *work)
> +{
> +	struct delayed_work *dwork = to_delayed_work(work);
> +	struct pattern_trig_data *data =
> +		container_of(dwork, struct pattern_trig_data, dwork);
> +
> +	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(data->led_cdev, data->curr->brightness);
> +		queue_delayed_work(data->queue, &data->dwork,
> +				msecs_to_jiffies(data->curr->time_ms));
> +		update_to_next_step(data);
> +	} else {
> +		/* Gradual dimming */
> +		led_set_brightness(data->led_cdev, compute_brightness(data));
> +		data->time_ms += UPDATE_INTERVAL;
> +		queue_delayed_work(data->queue, &data->dwork,
> +				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;
> +	data->led_cdev = led_cdev;
> +	data->queue = alloc_ordered_workqueue("pattern trigger on %s", 0,
> +					led_cdev->name ? led_cdev->name : "?");
> +	INIT_DELAYED_WORK(&data->dwork, pattern_trig_update);
> +	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);
> +
> +		mutex_lock(&data->lock);
> +		data->is_sane = 0; /* Prevent pattern_trig_update()
> +				      from scheduling new work */
> +		mutex_unlock(&data->lock);
> +
> +		flush_workqueue(data->queue);
> +		destroy_workqueue(data->queue);
> +
> +		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 v2");
> -- 
> 1.7.1
> 
> 
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] leds: ledtrig-morse: send out morse code
  2018-06-28 18:56 ` Pavel Machek
  2018-06-28 20:29   ` Andreas Klinger
@ 2018-06-28 20:33   ` Andreas Klinger
  1 sibling, 0 replies; 16+ messages in thread
From: Andreas Klinger @ 2018-06-28 20:33 UTC (permalink / raw)
  To: Pavel Machek
  Cc: jacek.anaszewski, ben.whitten, geert+renesas, w, pombredanne,
	gregkh, linux-kernel, linux-leds

Hi Pavel,

> Hi!
> 
> > Send out a morse code by using LEDs.
> > 
> > This is useful especially on embedded systems without displays to tell the
> > user about error conditions and status information.
> > 
> > The trigger will be called "morse"
> > 
> > The string to be send is written into the file morse_string and sent out
> > with a workqueue. Supported are letters and digits.
> > 
> > With the file dot_unit the minimal time unit can be adjusted in
> > milliseconds.
> > 
> > Signed-off-by: Andreas Klinger <ak@it-klinger.de>
> 
> Can we get more general "pattern" trigger? Some LEDs can do that in
> hardware, and it is more general than plain morse.
> 
> Ouch and it already was implemented :-). Patch is in attachment.
> 
> 									Pavel

The idea of the morse trigger is so be able so send out a short error
code or if needed also a complete sentence. The morse code don't need
extra explanation to the user. Decoding can be found everywhere.

With the pattern trigger one need a translation of the letters and
numbers into morse code. This is what the morse trigger is doing.

So from my perspective the pattern trigger is something different than
the morse one.

Another question:
The pattern trigger is not in mainline. Do you know why?

Andreas



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

> From rteysseyre@gmail.com Fri Mar 20 15:40:22 2015
> Return-Path: <rteysseyre@gmail.com>
> X-Original-To: pavel@atrey.karlin.mff.cuni.cz
> Delivered-To: pavel@atrey.karlin.mff.cuni.cz
> Received: from jabberwock.ucw.cz (jabberwock.ucw.cz [46.255.230.98])
> 	by atrey.karlin.mff.cuni.cz (Postfix) with ESMTP id 3275281ECB
> 	for <pavel@atrey.karlin.mff.cuni.cz>; Fri, 20 Mar 2015 15:40:22 +0100 (CET)
> Received: by jabberwock.ucw.cz (Postfix)
> 	id 3053A1C00FF; Fri, 20 Mar 2015 15:40:22 +0100 (CET)
> Delivered-To: pavel@jabberwock.ucw.cz
> Received: from mail-wg0-f46.google.com (mail-wg0-f46.google.com [74.125.82.46])
> 	(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))
> 	(Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (not verified))
> 	by jabberwock.ucw.cz (Postfix) with ESMTPS id 18EB61C00F9
> 	for <pavel@ucw.cz>; Fri, 20 Mar 2015 15:40:22 +0100 (CET)
> Received: by wgbcc7 with SMTP id cc7so91022009wgb.0
>         for <pavel@ucw.cz>; Fri, 20 Mar 2015 07:40:21 -0700 (PDT)
> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
>         d=gmail.com; s=20120113;
>         h=subject:from:to:cc:content-type:date:message-id:mime-version
>          :content-transfer-encoding;
>         bh=UdT+u6nHVkpFGm2AqZ0XFePjLX83mCQTkcoTCraWWvc=;
>         b=b9VbjaS+0McMAtdeSJv1Lhb6RDg/noWghgIsbIZD/DpMnIXJ05Dyi160Jx28uS0YzT
>          zyPmtmEv3IrnmkcCp7g6S6f04ATgi9q3tI+5qooaQS65V7bRtlpcTMoB8KEA38SsBtap
>          8Ng+65MHchSpmGyR/h0WxVg934ieNpySSLCQ/vSUu2lOxi0SLmyr93KqfZXQ6Yk5FCA3
>          QOcCuiXpcLfmGbLwR2YT0uu3/60LOmWL6xt+J/bkWjo38qwqtH5phoa5ydjcUy5MZJSZ
>          tn4qM8mQ8YH9EUJ+VjcxQX4nYdY+Mf3tsnSOieHNzUVt+2z1GmLcDi1Jfe8MtyPh8+9m
>          dp+w==
> X-Received: by 10.194.108.137 with SMTP id hk9mr118158926wjb.112.1426862421666;
>         Fri, 20 Mar 2015 07:40:21 -0700 (PDT)
> Received: from [192.168.244.132] (fs-141-0-203-237.fullsave.info. [141.0.203.237])
>         by mx.google.com with ESMTPSA id ha10sm6624401wjc.37.2015.03.20.07.40.20
>         (version=SSLv3 cipher=RC4-SHA bits=128/128);
>         Fri, 20 Mar 2015 07:40:20 -0700 (PDT)
> Subject: [PATCH v2] leds: Add arbitrary pattern trigger
> From: =?ISO-8859-1?Q?Rapha=EBl?= Teysseyre <rteysseyre@gmail.com>
> To: Pavel Machek <pavel@ucw.cz>
> Cc: Joe Xue <lgxue@hotmail.com>, Bryan Wu <cooloney@gmail.com>, 
>  "rpurdie@rpsys.net" <rpurdie@rpsys.net>, Linux LED Subsystem
>  <linux-leds@vger.kernel.org>, lkml <linux-kernel@vger.kernel.org>
> Content-Type: text/plain; charset="UTF-8"
> Date: Fri, 20 Mar 2015 15:42:02 +0100
> Message-ID: <1426862522.48232.3.camel@localhost>
> Mime-Version: 1.0
> X-Mailer: Evolution 2.32.3 (2.32.3-34.el6) 
> Content-Transfer-Encoding: 8bit
> X-CRM114-Status: Good  ( pR: 191.7191 )
> Status: RO
> Content-Length: 17146
> 
> Hi all,
> 
> Following your comments about [PATCH] leds: Add arbitrary pattern trigger,
> here is a revised version of this patch.
> 
> 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.
> 
> Changes since version 1:
>  - Fixed typos in documentation and comments.
>  - Fixed MODULE_LICENSE string.
>  - No more mutex in atomic context.
>  - Return EINVAL when invalid patterns are written to the "pattern" attribute.
> 
> 
> 
> 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 detailled documentation in patch.
> 
> Signed-off-by: Raphaël Teysseyre <rteysseyre@gmail.com>
> ---
>  Documentation/leds/ledtrig-pattern.txt |   86 +++++++
>  drivers/leds/trigger/Kconfig           |   10 +
>  drivers/leds/trigger/Makefile          |    1 +
>  drivers/leds/trigger/ledtrig-pattern.c |  432 ++++++++++++++++++++++++++++++++
>  4 files changed, 529 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..579295e
> --- /dev/null
> +++ b/Documentation/leds/ledtrig-pattern.txt
> @@ -0,0 +1,86 @@
> +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
> +
> +Or:
> +
> +  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)
> +
> +
> +Notes:
> +
> +Patterns with invalid syntax are reported with EINVAL, the
> +resulting LED brightness is undefined. Reading the pattern
> +back returns an empty string.
> +
> +Patterns with less than two values, no value with time length > 50
> +milliseconds, or no two values with differing brightnesses
> +result in the LED being set at the brightness of the first value,
> +or zero if the pattern contains no value. EINVAL is returned,
> +and reading the pattern back returns an empty string.
> +
> +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..fdf231d
> --- /dev/null
> +++ b/drivers/leds/trigger/ledtrig-pattern.c
> @@ -0,0 +1,432 @@
> +/*
> + * 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 workqueue_struct *queue;
> +	struct delayed_work dwork;
> +	struct led_classdev *led_cdev; /* Needed by pattern_trig_update() */
> +};
> +
> +#define UPDATE_INTERVAL 50
> +/* When doing gradual dimming, the led brightness
> +   will be updated every UPDATE_INTERVAL milliseconds */
> +
> +#define PATTERN_SEPARATOR ","
> +
> +#define MAX_NSTEPS (PAGE_SIZE/4)
> +/* The "pattern" attribute contains at most PAGE_SIZE characters.
> +   Each pattern step in this attribute needs at least 4 characters
> +   (a 1-digit number for the led brighntess, a space,
> +   a 1-digit number for the time, a PATTERN_SEPARATOR).
> +   Therefore, there is at most PAGE_SIZE/4 steps. */
> +
> +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;
> +	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 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
> + *  When @data->pattern isn't sane, a sensible brightness
> + *  default is suggested in @brightness
> + *
> + * DO NOT call pattern_trig_update() on a not-sane pattern
> + * with is_sane = 1, 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 int reset_pattern(struct pattern_trig_data *data,
> +			struct led_classdev *led_cdev)
> +{
> +	int brightness;
> +
> +	mutex_lock(&data->lock);
> +	data->is_sane = 0; /* Prevent pattern_trig_update()
> +			      from scheduling new work */
> +	mutex_unlock(&data->lock);
> +
> +	flush_workqueue(data->queue);
> +
> +	mutex_lock(&data->lock);
> +	if (is_sane(data, &brightness)) {
> +		data->curr = data->steps;
> +		data->next = data->steps + 1;
> +		data->time_ms = 0;
> +		data->is_sane = 1;
> +		queue_delayed_work(data->queue, &data->dwork, 0);
> +		mutex_unlock(&data->lock);
> +		return 0;
> +	}
> +
> +	mutex_unlock(&data->lock);
> +	led_set_brightness(led_cdev, brightness);
> +	return -EINVAL;
> +}
> +
> +/* --- 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;
> +
> +	mutex_lock(&data->lock);
> +	data->repeat = res < 0 ? -1 : res;
> +	mutex_unlock(&data->lock);
> +
> +	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 after a successful sscanf call */
> +	int ccr = 0; /* Characters read before looking for PATTERN_SEPARATOR */
> +	int tcr = 0; /* Total characters read in buf */
> +	int ccount;  /* Number of successful conversions in a sscanf call */
> +
> +	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 %n" PATTERN_SEPARATOR " %n",
> +				&data->steps[data->nsteps].brightness,
> +				&data->steps[data->nsteps].time_ms, &ccr, &cr);
> +
> +		if (!cr) { /* PATTERN_SEPARATOR not reached */
> +			if (ccount == 2) {
> +				/* Successful conversion before
> +				   looking for PATTERN_SEPARATOR. */
> +				data->nsteps++;
> +				tcr += ccr;
> +			}
> +
> +			if (tcr != count) {
> +				/* Invalid syntax */
> +				data->nsteps = 0;
> +				mutex_unlock(&data->lock);
> +				return -EINVAL;
> +			}
> +
> +			mutex_unlock(&data->lock);
> +
> +			if (reset_pattern(data, led_cdev)) {
> +				/* Invalid pattern */
> +				data->nsteps = 0;
> +				return -EINVAL;
> +			}
> +
> +			return count;
> +		}
> +
> +		tcr += cr;
> +	}
> +
> +	/* Shouldn't reach that */
> +	WARN(1, "MAX_NSTEP too small. Please report\n");
> +	data->nsteps = 0;
> +	mutex_unlock(&data->lock);
> +	return -EINVAL;
> +}
> +
> +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) /* Don't divide by zero */
> +		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(struct work_struct *work)
> +{
> +	struct delayed_work *dwork = to_delayed_work(work);
> +	struct pattern_trig_data *data =
> +		container_of(dwork, struct pattern_trig_data, dwork);
> +
> +	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(data->led_cdev, data->curr->brightness);
> +		queue_delayed_work(data->queue, &data->dwork,
> +				msecs_to_jiffies(data->curr->time_ms));
> +		update_to_next_step(data);
> +	} else {
> +		/* Gradual dimming */
> +		led_set_brightness(data->led_cdev, compute_brightness(data));
> +		data->time_ms += UPDATE_INTERVAL;
> +		queue_delayed_work(data->queue, &data->dwork,
> +				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;
> +	data->led_cdev = led_cdev;
> +	data->queue = alloc_ordered_workqueue("pattern trigger on %s", 0,
> +					led_cdev->name ? led_cdev->name : "?");
> +	INIT_DELAYED_WORK(&data->dwork, pattern_trig_update);
> +	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);
> +
> +		mutex_lock(&data->lock);
> +		data->is_sane = 0; /* Prevent pattern_trig_update()
> +				      from scheduling new work */
> +		mutex_unlock(&data->lock);
> +
> +		flush_workqueue(data->queue);
> +		destroy_workqueue(data->queue);
> +
> +		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 v2");
> -- 
> 1.7.1
> 
> 
> 




-- 
Andreas Klinger
Grabenreith 27
84508 Burgkirchen
+49 8623 919966
ak@it-klinger.de
www.it-klinger.de
www.grabenreith.de

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] leds: ledtrig-morse: send out morse code
  2018-06-28 20:29   ` Andreas Klinger
@ 2018-06-28 20:45     ` Pavel Machek
  2018-06-29  6:41       ` Andreas Klinger
  0 siblings, 1 reply; 16+ messages in thread
From: Pavel Machek @ 2018-06-28 20:45 UTC (permalink / raw)
  To: Andreas Klinger
  Cc: jacek.anaszewski, ben.whitten, geert+renesas, w, pombredanne,
	gregkh, linux-kernel, linux-leds

[-- Attachment #1: Type: text/plain, Size: 4777 bytes --]

On Thu 2018-06-28 22:29:57, Andreas Klinger wrote:
> Hi Pavel,
> 
> Pavel Machek <pavel@ucw.cz> schrieb am Thu, 28. Jun 20:56:
> > Hi!
> > 
> > > Send out a morse code by using LEDs.
> > > 
> > > This is useful especially on embedded systems without displays to tell the
> > > user about error conditions and status information.
> > > 
> > > The trigger will be called "morse"
> > > 
> > > The string to be send is written into the file morse_string and sent out
> > > with a workqueue. Supported are letters and digits.
> > > 
> > > With the file dot_unit the minimal time unit can be adjusted in
> > > milliseconds.
> > > 
> > > Signed-off-by: Andreas Klinger <ak@it-klinger.de>
> > 
> > Can we get more general "pattern" trigger? Some LEDs can do that in
> > hardware, and it is more general than plain morse.
> > 
> > Ouch and it already was implemented :-). Patch is in attachment.
> 
> The idea of the morse trigger is so be able so send out a short error
> code or if needed also a complete sentence. The morse code don't need
> extra explanation to the user. Decoding can be found everywhere.

Yeah, well... I don't think decoding sentences in morse code is going
to be much fun.

And we don't really want encoder in kernel. Just do encoding in
userspace, and use pattern trigger to display it.

For many uses, morse code is "too geeky", and other patterns will be
used.

Like " X    " for error 1, " X X     " for error 2, " .xX .xX " for
charging, " .xXx. " for everything okay...

> With the pattern trigger one need a translation of the letters and
> numbers into morse code. This is what the morse trigger is doing.
> 
> So from my perspective the pattern trigger is something different than
> the morse one.

Well, pattern trigger is more generic, and can do everything morse
trigger can do. Lets do that instead.

> Another question:
> The pattern trigger is not in mainline. Do you know why?

Needs to be resubmitted, I'd say.

								Pavel

> > +++ b/Documentation/leds/ledtrig-pattern.txt
> > @@ -0,0 +1,86 @@
> > +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
> > +
> > +Or:
> > +
> > +  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)
> > +

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] leds: ledtrig-morse: send out morse code
  2018-06-28 20:45     ` Pavel Machek
@ 2018-06-29  6:41       ` Andreas Klinger
  2018-06-29  7:17         ` Pavel Machek
  0 siblings, 1 reply; 16+ messages in thread
From: Andreas Klinger @ 2018-06-29  6:41 UTC (permalink / raw)
  To: pavel, jacek.anaszewski, ben.whitten, geert+renesas, w,
	pombredanne, gregkh, linux-kernel, linux-leds

Hi,

Pavel Machek <pavel@ucw.cz> schrieb am Thu, 28. Jun 22:45:
> On Thu 2018-06-28 22:29:57, Andreas Klinger wrote:
> > Hi Pavel,
> > 
> > Pavel Machek <pavel@ucw.cz> schrieb am Thu, 28. Jun 20:56:
> > > Hi!
> > > 
> > > > Send out a morse code by using LEDs.
> > > > 
> > > > This is useful especially on embedded systems without displays to tell the
> > > > user about error conditions and status information.
> > > > 
> > > > The trigger will be called "morse"
> > > > 
> > > > The string to be send is written into the file morse_string and sent out
> > > > with a workqueue. Supported are letters and digits.
> > > > 
> > > > With the file dot_unit the minimal time unit can be adjusted in
> > > > milliseconds.
> > > > 
> > > > Signed-off-by: Andreas Klinger <ak@it-klinger.de>
> > > 
> > > Can we get more general "pattern" trigger? Some LEDs can do that in
> > > hardware, and it is more general than plain morse.
> > > 
> > > Ouch and it already was implemented :-). Patch is in attachment.
> > 
> > The idea of the morse trigger is so be able so send out a short error
> > code or if needed also a complete sentence. The morse code don't need
> > extra explanation to the user. Decoding can be found everywhere.
> 
> Yeah, well... I don't think decoding sentences in morse code is going
> to be much fun.
> 
> And we don't really want encoder in kernel. Just do encoding in
> userspace, and use pattern trigger to display it.
> 
> For many uses, morse code is "too geeky", and other patterns will be
> used.
> 
> Like " X    " for error 1, " X X     " for error 2, " .xX .xX " for
> charging, " .xXx. " for everything okay...
> 

The hardware i'm using is not able to adjust brightness. It can just switch the
LED on or off. That's it.

If anybody is interested i can submit a version 2 of the morse trigger with the
improvements suggested by Greg and Geert. Please let me know.

Andreas


> > With the pattern trigger one need a translation of the letters and
> > numbers into morse code. This is what the morse trigger is doing.
> > 
> > So from my perspective the pattern trigger is something different than
> > the morse one.
> 
> Well, pattern trigger is more generic, and can do everything morse
> trigger can do. Lets do that instead.
> 
> > Another question:
> > The pattern trigger is not in mainline. Do you know why?
> 
> Needs to be resubmitted, I'd say.
> 
> 								Pavel
> 
> > > +++ b/Documentation/leds/ledtrig-pattern.txt
> > > @@ -0,0 +1,86 @@
> > > +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
> > > +
> > > +Or:
> > > +
> > > +  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)
> > > +
> 
> -- 
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] leds: ledtrig-morse: send out morse code
  2018-06-29  6:41       ` Andreas Klinger
@ 2018-06-29  7:17         ` Pavel Machek
  2018-06-29  7:21           ` Geert Uytterhoeven
  0 siblings, 1 reply; 16+ messages in thread
From: Pavel Machek @ 2018-06-29  7:17 UTC (permalink / raw)
  To: Andreas Klinger
  Cc: jacek.anaszewski, ben.whitten, geert+renesas, w, pombredanne,
	gregkh, linux-kernel, linux-leds

[-- Attachment #1: Type: text/plain, Size: 930 bytes --]


> > Yeah, well... I don't think decoding sentences in morse code is going
> > to be much fun.
> > 
> > And we don't really want encoder in kernel. Just do encoding in
> > userspace, and use pattern trigger to display it.
> > 
> > For many uses, morse code is "too geeky", and other patterns will be
> > used.
> > 
> > Like " X    " for error 1, " X X     " for error 2, " .xX .xX " for
> > charging, " .xXx. " for everything okay...
> > 
> 
> The hardware i'm using is not able to adjust brightness. It can just switch the
> LED on or off. That's it.
> 
> If anybody is interested i can submit a version 2 of the morse trigger with the
> improvements suggested by Greg and Geert. Please let me know.

I'd prefer more general pattern trigger.
									Pavel
									
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] leds: ledtrig-morse: send out morse code
  2018-06-29  7:17         ` Pavel Machek
@ 2018-06-29  7:21           ` Geert Uytterhoeven
  2018-06-29  7:29             ` Pavel Machek
  0 siblings, 1 reply; 16+ messages in thread
From: Geert Uytterhoeven @ 2018-06-29  7:21 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Andreas Klinger, Jacek Anaszewski, Ben Whitten,
	Geert Uytterhoeven, Willy Tarreau, Philippe Ombredanne, Greg KH,
	Linux Kernel Mailing List, linux-leds

Hi Pavel,

On Fri, Jun 29, 2018 at 9:17 AM Pavel Machek <pavel@ucw.cz> wrote:
> > > Yeah, well... I don't think decoding sentences in morse code is going
> > > to be much fun.
> > >
> > > And we don't really want encoder in kernel. Just do encoding in
> > > userspace, and use pattern trigger to display it.
> > >
> > > For many uses, morse code is "too geeky", and other patterns will be
> > > used.
> > >
> > > Like " X    " for error 1, " X X     " for error 2, " .xX .xX " for
> > > charging, " .xXx. " for everything okay...
> > >
> >
> > The hardware i'm using is not able to adjust brightness. It can just switch the
> > LED on or off. That's it.
> >
> > If anybody is interested i can submit a version 2 of the morse trigger with the
> > improvements suggested by Greg and Geert. Please let me know.
>
> I'd prefer more general pattern trigger.

There are two parts here:
  1. The general pattern trigger (which is not yet upstream),
  2. A way to supply a string, convert it into morse code, and feed it to
     the LED subsystem.

Given 1 is not yet upstream, I think Andreas solution is fine.
Once 1 is upstream, 2 can be modified to feed into 1, if available.

Do you agree?
Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] leds: ledtrig-morse: send out morse code
  2018-06-29  7:21           ` Geert Uytterhoeven
@ 2018-06-29  7:29             ` Pavel Machek
  2018-06-29  7:48               ` Geert Uytterhoeven
  0 siblings, 1 reply; 16+ messages in thread
From: Pavel Machek @ 2018-06-29  7:29 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Andreas Klinger, Jacek Anaszewski, Ben Whitten,
	Geert Uytterhoeven, Willy Tarreau, Philippe Ombredanne, Greg KH,
	Linux Kernel Mailing List, linux-leds

[-- Attachment #1: Type: text/plain, Size: 1684 bytes --]

On Fri 2018-06-29 09:21:24, Geert Uytterhoeven wrote:
> Hi Pavel,
> 
> On Fri, Jun 29, 2018 at 9:17 AM Pavel Machek <pavel@ucw.cz> wrote:
> > > > Yeah, well... I don't think decoding sentences in morse code is going
> > > > to be much fun.
> > > >
> > > > And we don't really want encoder in kernel. Just do encoding in
> > > > userspace, and use pattern trigger to display it.
> > > >
> > > > For many uses, morse code is "too geeky", and other patterns will be
> > > > used.
> > > >
> > > > Like " X    " for error 1, " X X     " for error 2, " .xX .xX " for
> > > > charging, " .xXx. " for everything okay...
> > > >
> > >
> > > The hardware i'm using is not able to adjust brightness. It can just switch the
> > > LED on or off. That's it.
> > >
> > > If anybody is interested i can submit a version 2 of the morse trigger with the
> > > improvements suggested by Greg and Geert. Please let me know.
> >
> > I'd prefer more general pattern trigger.
> 
> There are two parts here:
>   1. The general pattern trigger (which is not yet upstream),
>   2. A way to supply a string, convert it into morse code, and feed it to
>      the LED subsystem.
> 
> Given 1 is not yet upstream, I think Andreas solution is fine.
> Once 1 is upstream, 2 can be modified to feed into 1, if available.

There's no reason 2. should be in kernel ("mechanism, not policy").

Given 1. is not really more complicated than the morse
trigger... please just help with pattern trigger if you want this to
happen.

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] leds: ledtrig-morse: send out morse code
  2018-06-29  7:29             ` Pavel Machek
@ 2018-06-29  7:48               ` Geert Uytterhoeven
  2018-06-29  8:07                 ` Pavel Machek
  0 siblings, 1 reply; 16+ messages in thread
From: Geert Uytterhoeven @ 2018-06-29  7:48 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Andreas Klinger, Jacek Anaszewski, Ben Whitten,
	Geert Uytterhoeven, Willy Tarreau, Philippe Ombredanne, Greg KH,
	Linux Kernel Mailing List, linux-leds

Hi Pavel.

On Fri, Jun 29, 2018 at 9:29 AM Pavel Machek <pavel@ucw.cz> wrote:
> On Fri 2018-06-29 09:21:24, Geert Uytterhoeven wrote:
> > On Fri, Jun 29, 2018 at 9:17 AM Pavel Machek <pavel@ucw.cz> wrote:
> > > > > Yeah, well... I don't think decoding sentences in morse code is going
> > > > > to be much fun.
> > > > >
> > > > > And we don't really want encoder in kernel. Just do encoding in
> > > > > userspace, and use pattern trigger to display it.
> > > > >
> > > > > For many uses, morse code is "too geeky", and other patterns will be
> > > > > used.
> > > > >
> > > > > Like " X    " for error 1, " X X     " for error 2, " .xX .xX " for
> > > > > charging, " .xXx. " for everything okay...
> > > > >
> > > >
> > > > The hardware i'm using is not able to adjust brightness. It can just switch the
> > > > LED on or off. That's it.
> > > >
> > > > If anybody is interested i can submit a version 2 of the morse trigger with the
> > > > improvements suggested by Greg and Geert. Please let me know.
> > >
> > > I'd prefer more general pattern trigger.
> >
> > There are two parts here:
> >   1. The general pattern trigger (which is not yet upstream),
> >   2. A way to supply a string, convert it into morse code, and feed it to
> >      the LED subsystem.
> >
> > Given 1 is not yet upstream, I think Andreas solution is fine.
> > Once 1 is upstream, 2 can be modified to feed into 1, if available.
>
> There's no reason 2. should be in kernel ("mechanism, not policy").
>
> Given 1. is not really more complicated than the morse
> trigger... please just help with pattern trigger if you want this to
> happen.

Ah, you want to do the conversion to morse code in userspace?
Yes, that makes sense.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] leds: ledtrig-morse: send out morse code
  2018-06-29  7:48               ` Geert Uytterhoeven
@ 2018-06-29  8:07                 ` Pavel Machek
  2018-06-29  8:24                   ` Greg KH
  0 siblings, 1 reply; 16+ messages in thread
From: Pavel Machek @ 2018-06-29  8:07 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Andreas Klinger, Jacek Anaszewski, Ben Whitten,
	Geert Uytterhoeven, Willy Tarreau, Philippe Ombredanne, Greg KH,
	Linux Kernel Mailing List, linux-leds

[-- Attachment #1: Type: text/plain, Size: 527 bytes --]

Hi!

> > There's no reason 2. should be in kernel ("mechanism, not policy").
> >
> > Given 1. is not really more complicated than the morse
> > trigger... please just help with pattern trigger if you want this to
> > happen.
> 
> Ah, you want to do the conversion to morse code in userspace?
> Yes, that makes sense.

Yes, that's what I want.

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] leds: ledtrig-morse: send out morse code
  2018-06-29  8:07                 ` Pavel Machek
@ 2018-06-29  8:24                   ` Greg KH
  2018-06-29  9:23                     ` Pavel Machek
  0 siblings, 1 reply; 16+ messages in thread
From: Greg KH @ 2018-06-29  8:24 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Geert Uytterhoeven, Andreas Klinger, Jacek Anaszewski,
	Ben Whitten, Geert Uytterhoeven, Willy Tarreau,
	Philippe Ombredanne, Linux Kernel Mailing List, linux-leds

On Fri, Jun 29, 2018 at 10:07:17AM +0200, Pavel Machek wrote:
> Hi!
> 
> > > There's no reason 2. should be in kernel ("mechanism, not policy").
> > >
> > > Given 1. is not really more complicated than the morse
> > > trigger... please just help with pattern trigger if you want this to
> > > happen.
> > 
> > Ah, you want to do the conversion to morse code in userspace?
> > Yes, that makes sense.
> 
> Yes, that's what I want.

We all want a pony :)

Seriously, this is a very simple module, works well, and makes some
system's life much easier as there is no external dependancy on a
text-to-morse-code library needed.

If you don't want this cluttering up your kernel, don't built it in.

I think it should be taken as-is (well, with the few changes already
recommended).

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] leds: ledtrig-morse: send out morse code
  2018-06-29  8:24                   ` Greg KH
@ 2018-06-29  9:23                     ` Pavel Machek
  0 siblings, 0 replies; 16+ messages in thread
From: Pavel Machek @ 2018-06-29  9:23 UTC (permalink / raw)
  To: Greg KH
  Cc: Geert Uytterhoeven, Andreas Klinger, Jacek Anaszewski,
	Ben Whitten, Geert Uytterhoeven, Willy Tarreau,
	Philippe Ombredanne, Linux Kernel Mailing List, linux-leds

[-- Attachment #1: Type: text/plain, Size: 1123 bytes --]

On Fri 2018-06-29 04:24:39, Greg KH wrote:
> On Fri, Jun 29, 2018 at 10:07:17AM +0200, Pavel Machek wrote:
> > Hi!
> > 
> > > > There's no reason 2. should be in kernel ("mechanism, not policy").
> > > >
> > > > Given 1. is not really more complicated than the morse
> > > > trigger... please just help with pattern trigger if you want this to
> > > > happen.
> > > 
> > > Ah, you want to do the conversion to morse code in userspace?
> > > Yes, that makes sense.
> > 
> > Yes, that's what I want.
> 
> We all want a pony :)

I already have a pony :-).

> Seriously, this is a very simple module, works well, and makes some
> system's life much easier as there is no external dependancy on a
> text-to-morse-code library needed.

It still does not belong into kernel.

It is interesting. You deleted bluetooth driver people used from
staging because of "not enough cleanups" and you want me to maintain
text-to-morse-code in the kernel?!

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2018-06-29  9:23 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-28 13:42 [PATCH] leds: ledtrig-morse: send out morse code Andreas Klinger
2018-06-28 13:54 ` Greg KH
2018-06-28 15:36 ` Geert Uytterhoeven
2018-06-28 18:21 ` Willy Tarreau
2018-06-28 18:56 ` Pavel Machek
2018-06-28 20:29   ` Andreas Klinger
2018-06-28 20:45     ` Pavel Machek
2018-06-29  6:41       ` Andreas Klinger
2018-06-29  7:17         ` Pavel Machek
2018-06-29  7:21           ` Geert Uytterhoeven
2018-06-29  7:29             ` Pavel Machek
2018-06-29  7:48               ` Geert Uytterhoeven
2018-06-29  8:07                 ` Pavel Machek
2018-06-29  8:24                   ` Greg KH
2018-06-29  9:23                     ` Pavel Machek
2018-06-28 20:33   ` Andreas Klinger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).