[PATCH v1 0/4] upstream led blinking rework

[PATCH v1 0/4] upstream led blinking rework

[Oleksij Rempel]

Sascha Hauer (4):
  led: Allow blinking/flashing on led level
  led: Add blink/flash to led command
  led: trigger: Use led triggers
  led-trigger: rework

 commands/led.c             |  44 ++++++++++++-
 commands/trigger.c         |  54 ++++++++--------
 drivers/led/core.c         | 102 ++++++++++++++++++++++++++++-
 drivers/led/led-triggers.c | 157 ++++++++++++++++++++++++++++-----------------
 include/led.h              |  17 ++++-
 5 files changed, 287 insertions(+), 87 deletions(-)

-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v1 1/4] led: Allow blinking/flashing on led level

[Oleksij Rempel]

From: Sascha Hauer <s.hauer@pengutronix.de>

So far blinking/flashing LEDs is only supported on led-trigger level.
Even without triggers it useful to be able to blink/flash LEDs, so
add this functionality to the LED core.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

led: Introduce led_blink_pattern for flexible LED blinking

A pattern consists of a number of on and off-periods which are described
in an array. Using such an array you can encode nearly every blink
pattern you need.

led_blink and led_flash are rewritten to use the new mechanism.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/led/core.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 include/led.h      |  11 ++++++
 2 files changed, 112 insertions(+), 1 deletion(-)

diff --git a/drivers/led/core.c b/drivers/led/core.c
index 30b016bb3..ab293b562 100644
--- a/drivers/led/core.c
+++ b/drivers/led/core.c
@@ -23,6 +23,7 @@
 #include <linux/list.h>
 #include <errno.h>
 #include <led.h>
+#include <init.h>
 #include <poller.h>
 #include <clock.h>
 #include <linux/ctype.h>
@@ -101,7 +102,7 @@ struct led *led_by_name_or_number(const char *str)
  * @param led	the led
  * @param value	the value of the LED (0 is disabled)
  */
-int led_set(struct led *led, unsigned int value)
+static int __led_set(struct led *led, unsigned int value)
 {
 	if (value > led->max_value)
 		value = led->max_value;
@@ -114,6 +115,105 @@ int led_set(struct led *led, unsigned int value)
 	return 0;
 }
 
+int led_set(struct led *led, unsigned int value)
+{
+	led->blink = 0;
+	led->flash = 0;
+	return __led_set(led, value);
+}
+
+static void led_blink_func(struct poller_struct *poller)
+{
+	struct led *led;
+
+	list_for_each_entry(led, &leds, list) {
+		bool on;
+
+		if (!led->blink && !led->flash)
+			continue;
+
+		if (led->blink_next_event > get_time_ns()) {
+			continue;
+		}
+
+		on = !(led->blink_next_state % 2);
+
+		led->blink_next_event = get_time_ns() +
+			(led->blink_states[led->blink_next_state] * MSECOND);
+		led->blink_next_state = (led->blink_next_state + 1) %
+					led->blink_nr_states;
+
+		if (led->flash && !on)
+			led->flash = 0;
+
+		__led_set(led, on);
+	}
+}
+
+/**
+ * led_blink_pattern - Blink a led with flexible timings.
+ * @led LED used
+ * @pattern Array of millisecond intervals describing the on and off periods of
+ * the pattern. At the end of the array/pattern it is repeated. The array
+ * starts with an on-period. In general every array item with even index
+ * describes an on-period, every item with odd index an off-period.
+ * @pattern_len Length of the pattern array.
+ *
+ * Returns 0 on success.
+ *
+ * Example:
+ * 	pattern = {500, 1000};
+ * 	This will enable the LED for 500ms and disable it for 1000ms after
+ * 	that. This is repeated forever.
+ */
+int led_blink_pattern(struct led *led, const unsigned int *pattern,
+		      unsigned int pattern_len)
+{
+	if (led->blink_states)
+		free(led->blink_states);
+	led->blink_states = xmemdup(pattern,
+				    pattern_len * sizeof(*led->blink_states));
+	led->blink_nr_states = pattern_len;
+	led->blink_next_state = 0;
+	led->blink_next_event = get_time_ns();
+	led->blink = 1;
+	led->flash = 0;
+
+	return 0;
+}
+
+int led_blink(struct led *led, unsigned int on_ms, unsigned int off_ms)
+{
+	unsigned int pattern[] = {on_ms, off_ms};
+
+	return led_blink_pattern(led, pattern, 2);
+}
+
+int led_flash(struct led *led, unsigned int duration_ms)
+{
+	unsigned int pattern[] = {duration_ms, 0};
+	int ret;
+
+	ret = led_blink_pattern(led, pattern, 2);
+	if (ret)
+		return ret;
+
+	led->flash = 1;
+	led->blink = 0;
+
+	return 0;
+}
+
+static struct poller_struct led_poller = {
+	.func = led_blink_func,
+};
+
+static int led_blink_init(void)
+{
+	return poller_register(&led_poller);
+}
+late_initcall(led_blink_init);
+
 /**
  * led_set_num - set the value of a LED
  * @param num	the number of the LED
diff --git a/include/led.h b/include/led.h
index 000267cdc..fb5c48b9e 100644
--- a/include/led.h
+++ b/include/led.h
@@ -12,6 +12,13 @@ struct led {
 	char *name;
 	int num;
 	struct list_head list;
+
+	int blink;
+	int flash;
+	unsigned int *blink_states;
+	int blink_nr_states;
+	int blink_next_state;
+	uint64_t blink_next_event;
 };
 
 struct led *led_by_number(int no);
@@ -25,6 +32,10 @@ static inline int led_get_number(struct led *led)
 
 int led_set_num(int num, unsigned int value);
 int led_set(struct led *led, unsigned int value);
+int led_blink(struct led *led, unsigned int on_ms, unsigned int off_ms);
+int led_blink_pattern(struct led *led, const unsigned int *pattern,
+		      unsigned int pattern_len);
+int led_flash(struct led *led, unsigned int duration_ms);
 int led_register(struct led *led);
 void led_unregister(struct led *led);
 void led_unregister(struct led *led);
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v1 2/4] led: Add blink/flash to led command

[Oleksij Rempel]

From: Sascha Hauer <s.hauer@pengutronix.de>

The LED framework now supports blinking/flashing LEDs, so
add this functionality to the led command.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 commands/led.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/commands/led.c b/commands/led.c
index 354f74df8..a53f0df6a 100644
--- a/commands/led.c
+++ b/commands/led.c
@@ -29,6 +29,44 @@ static int do_led(int argc, char *argv[])
 	unsigned long value;
 	struct led *led;
 	int ret;
+	int opt;
+	int flash = 0, blink = 0;
+	int blink_on_ms = 500;
+	int blink_off_ms = 500;
+
+	while ((opt = getopt(argc, argv, "fb")) > 0) {
+		switch(opt) {
+		case 'f':
+			flash = 1;
+			break;
+		case 'b':
+			blink = 1;
+			break;
+		}
+	}
+
+	if (flash || blink) {
+		int args = argc - optind;
+
+		if (!args || (flash && blink))
+			return COMMAND_ERROR_USAGE;
+
+		led = led_by_name_or_number(argv[optind]);
+		if (!led) {
+			printf("no such LED: %s\n", argv[optind]);
+			return 1;
+		}
+
+		if (args > 1)
+			blink_on_ms = simple_strtoul(argv[optind + 1], NULL, 0);
+		if (args > 2)
+			blink_off_ms = simple_strtoul(argv[optind + 2], NULL, 0);
+
+		if (flash)
+			return led_flash(led, blink_on_ms);
+		if (blink)
+			return led_blink(led, blink_on_ms, blink_off_ms);
+	}
 
 	if (argc == 1) {
 		int i = 0;
@@ -73,9 +111,13 @@ static int do_led(int argc, char *argv[])
 BAREBOX_CMD_HELP_START(led)
 BAREBOX_CMD_HELP_TEXT("Control the value of a LED. The exact meaning of VALUE is unspecified,")
 BAREBOX_CMD_HELP_TEXT("it can be a brightness, or a color. Most often a value of '1' means on")
-BAREBOX_CMD_HELP_TEXT("and '0' means off.")
+BAREBOX_CMD_HELP_TEXT("and '0' means off. Basic usage is 'led <led> <value>'. LEDs can be given")
+BAREBOX_CMD_HELP_TEXT("by name or number.")
 BAREBOX_CMD_HELP_TEXT("")
 BAREBOX_CMD_HELP_TEXT("Without arguments the available LEDs are listed.")
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT ("-b <led> [duration-on-ms] [duration-off-ms]",  "blink a LED")
+BAREBOX_CMD_HELP_OPT ("-f <led> [duration-ms]",  "flash a LED")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(led)
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v1 3/4] led: trigger: Use led triggers

[Oleksij Rempel]

From: Sascha Hauer <s.hauer@pengutronix.de>

Since the LED framework now supports blinking/flashing, use this
functionality in the LED triggers.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/led/led-triggers.c | 40 +++++-----------------------------------
 1 file changed, 5 insertions(+), 35 deletions(-)

diff --git a/drivers/led/led-triggers.c b/drivers/led/led-triggers.c
index dee936739..6c05cc5bb 100644
--- a/drivers/led/led-triggers.c
+++ b/drivers/led/led-triggers.c
@@ -48,34 +48,12 @@
 
 struct led_trigger_struct {
 	struct led *led;
-	uint64_t flash_start;
-	int flash;
+	struct list_head list;
+	enum led_trigger trigger;
 };
 
 static struct led_trigger_struct triggers[LED_TRIGGER_MAX];
 
-static void trigger_func(struct poller_struct *poller)
-{
-	int i;
-
-	for (i = 0; i < LED_TRIGGER_MAX; i++) {
-		if (triggers[i].led &&
-		    triggers[i].flash &&
-		    is_timeout(triggers[i].flash_start, 200 * MSECOND)) {
-			led_set(triggers[i].led, 0);
-			triggers[i].flash = 0;
-		}
-	}
-
-	if (triggers[LED_TRIGGER_HEARTBEAT].led &&
-			is_timeout(triggers[LED_TRIGGER_HEARTBEAT].flash_start, SECOND))
-		led_trigger(LED_TRIGGER_HEARTBEAT, TRIGGER_FLASH);
-}
-
-static struct poller_struct trigger_poller = {
-	.func = trigger_func,
-};
-
 /**
  * led_trigger - triggers a trigger
  * @param trigger	The trigger to enable/disable
@@ -91,11 +69,7 @@ void led_trigger(enum led_trigger trigger, enum trigger_type type)
 		return;
 
 	if (type == TRIGGER_FLASH) {
-		if (is_timeout(triggers[trigger].flash_start, 400 * MSECOND)) {
-			led_set(triggers[trigger].led, triggers[trigger].led->max_value);
-			triggers[trigger].flash_start = get_time_ns();
-			triggers[trigger].flash = 1;
-		}
+		led_flash(triggers[trigger].led, 200);
 		return;
 	}
 
@@ -129,6 +103,8 @@ int led_set_trigger(enum led_trigger trigger, struct led *led)
 
 	if (led && trigger == LED_TRIGGER_DEFAULT_ON)
 		led_set(triggers[trigger].led, triggers[trigger].led->max_value);
+	if (led && trigger == LED_TRIGGER_HEARTBEAT)
+		led_blink(led, 200, 1000);
 
 	return 0;
 }
@@ -147,9 +123,3 @@ int led_get_trigger(enum led_trigger trigger)
 		return -ENODEV;
 	return led_get_number(triggers[trigger].led);
 }
-
-static int trigger_init(void)
-{
-	return poller_register(&trigger_poller);
-}
-late_initcall(trigger_init);
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v1 4/4] led-trigger: rework

[Oleksij Rempel]

From: Sascha Hauer <s.hauer@pengutronix.de>

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 commands/trigger.c         |  54 +++++++++++---------
 drivers/led/led-triggers.c | 125 +++++++++++++++++++++++++++++++++++----------
 include/led.h              |   6 ++-
 3 files changed, 131 insertions(+), 54 deletions(-)

diff --git a/commands/trigger.c b/commands/trigger.c
index 2758ce74e..0dd3b346f 100644
--- a/commands/trigger.c
+++ b/commands/trigger.c
@@ -28,59 +28,62 @@
 #define LED_COMMAND_SHOW_INFO		2
 #define	LED_COMMAND_DISABLE_TRIGGER	3
 
-static char *trigger_names[] = {
-	[LED_TRIGGER_PANIC] = "panic",
-	[LED_TRIGGER_HEARTBEAT] = "heartbeat",
-	[LED_TRIGGER_NET_RX] = "net rx",
-	[LED_TRIGGER_NET_TX] = "net tx",
-	[LED_TRIGGER_NET_TXRX] = "net",
-	[LED_TRIGGER_DEFAULT_ON] = "default on",
-};
 
 static int do_trigger(int argc, char *argv[])
 {
-	struct led *led;
-	int i, opt, ret = 0;
+	struct led *led = NULL;
+	int opt, ret = 0;
 	int cmd = LED_COMMAND_SHOW_INFO;
-	unsigned long trigger = 0;
+	enum led_trigger trigger;
+	const char *led_name = NULL;
+	const char *trigger_name = NULL;
 
 	while((opt = getopt(argc, argv, "t:d:")) > 0) {
 		switch(opt) {
 		case 't':
-			trigger = simple_strtoul(optarg, NULL, 0);
+			trigger_name = optarg;
 			cmd = LED_COMMAND_SET_TRIGGER;
 			break;
 		case 'd':
-			trigger = simple_strtoul(optarg, NULL, 0);
+			led_name = optarg;
 			cmd = LED_COMMAND_DISABLE_TRIGGER;
 		}
 	}
 
+	if (optind < argc)
+		led = led_by_name_or_number(argv[optind]);
+
 	switch (cmd) {
 	case LED_COMMAND_SHOW_INFO:
-		for (i = 0; i < LED_TRIGGER_MAX; i++) {
-			int led = led_get_trigger(i);
-			printf("%d: %s", i, trigger_names[i]);
-			if (led >= 0)
-				printf(" (led %d)", led);
-			printf("\n");
-		}
+		led_triggers_show_info();
 		break;
 
 	case LED_COMMAND_DISABLE_TRIGGER:
-		ret = led_set_trigger(trigger, NULL);
+		led = led_by_name_or_number(led_name);
+		if (!led) {
+			printf("no such led: %s\n", led_name);
+			return 1;
+		}
+
+		led_trigger_disable(led);
 		break;
 
 	case LED_COMMAND_SET_TRIGGER:
 		if (argc - optind != 1)
 			return COMMAND_ERROR_USAGE;
-		led = led_by_name_or_number(argv[optind]);
 
+		led = led_by_name_or_number(argv[optind]);
 		if (!led) {
 			printf("no such led: %s\n", argv[optind]);
 			return 1;
 		}
 
+		trigger = trigger_by_name(trigger_name);
+		if (trigger == LED_TRIGGER_INVALID) {
+			printf("no such trigger: %s\n", trigger_name);
+			return 1;
+		}
+
 		ret = led_set_trigger(trigger, led);
 		break;
 	}
@@ -92,16 +95,17 @@ static int do_trigger(int argc, char *argv[])
 
 BAREBOX_CMD_HELP_START(trigger)
 BAREBOX_CMD_HELP_TEXT("Control a LED trigger. Without options assigned triggers are shown.")
+BAREBOX_CMD_HELP_TEXT("triggers are given with their name, LEDs are given with their name or number")
 BAREBOX_CMD_HELP_TEXT("")
 BAREBOX_CMD_HELP_TEXT("Options:")
-BAREBOX_CMD_HELP_OPT ("-t",  "set a trigger (needs LED argument)")
-BAREBOX_CMD_HELP_OPT ("-d",  "disable a trigger")
+BAREBOX_CMD_HELP_OPT ("-t <trigger> <led>",  "set a trigger")
+BAREBOX_CMD_HELP_OPT ("-d <led>",  "disable a trigger")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(trigger)
 	.cmd		= do_trigger,
 	BAREBOX_CMD_DESC("handle LED triggers")
-	BAREBOX_CMD_OPTS("[-td] TRIGGER [LED]")
+	BAREBOX_CMD_OPTS("[-td] [LED]")
 	BAREBOX_CMD_GROUP(CMD_GRP_HWMANIP)
 	BAREBOX_CMD_HELP(cmd_trigger_help)
 BAREBOX_CMD_END
diff --git a/drivers/led/led-triggers.c b/drivers/led/led-triggers.c
index 6c05cc5bb..76a1481e1 100644
--- a/drivers/led/led-triggers.c
+++ b/drivers/led/led-triggers.c
@@ -52,7 +52,7 @@ struct led_trigger_struct {
 	enum led_trigger trigger;
 };
 
-static struct led_trigger_struct triggers[LED_TRIGGER_MAX];
+static LIST_HEAD(led_triggers);
 
 /**
  * led_trigger - triggers a trigger
@@ -63,17 +63,53 @@ static struct led_trigger_struct triggers[LED_TRIGGER_MAX];
  */
 void led_trigger(enum led_trigger trigger, enum trigger_type type)
 {
+	struct led_trigger_struct *led_trigger;
+
 	if (trigger >= LED_TRIGGER_MAX)
 		return;
-	if (!triggers[trigger].led)
-		return;
 
-	if (type == TRIGGER_FLASH) {
-		led_flash(triggers[trigger].led, 200);
-		return;
+	list_for_each_entry(led_trigger, &led_triggers, list) {
+		if (led_trigger->trigger != trigger)
+			continue;
+
+		switch (type) {
+		case TRIGGER_FLASH:
+			led_flash(led_trigger->led, 200);
+			break;
+		case TRIGGER_ENABLE:
+			led_set(led_trigger->led, led_trigger->led->max_value);
+			break;
+		case TRIGGER_DISABLE:
+			led_set(led_trigger->led, 0);
+			break;
+		}
 	}
+}
+
+static struct led_trigger_struct *led_find_trigger(struct led *led)
+{
+	struct led_trigger_struct *led_trigger;
+
+	list_for_each_entry(led_trigger, &led_triggers, list)
+		if (led_trigger->led == led)
+			return led_trigger;
+
+	return NULL;
+}
 
-	led_set(triggers[trigger].led, type == TRIGGER_ENABLE ? triggers[trigger].led->max_value : 0);
+void led_trigger_disable(struct led *led)
+{
+	struct led_trigger_struct *led_trigger;
+
+	led_trigger = led_find_trigger(led);
+	if (!led_trigger)
+		return;
+
+	list_del(&led_trigger->list);
+
+	led_set(led, 0);
+
+	free(led_trigger);
 }
 
 /**
@@ -86,40 +122,73 @@ void led_trigger(enum led_trigger trigger, enum trigger_type type)
  */
 int led_set_trigger(enum led_trigger trigger, struct led *led)
 {
-	int i;
+	struct led_trigger_struct *led_trigger;
 
 	if (trigger >= LED_TRIGGER_MAX)
 		return -EINVAL;
 
-	if (led)
-		for (i = 0; i < LED_TRIGGER_MAX; i++)
-			if (triggers[i].led == led)
-				return -EBUSY;
+	led_trigger_disable(led);
 
-	if (triggers[trigger].led && !led)
-		led_set(triggers[trigger].led, 0);
+	led_trigger = xzalloc(sizeof(*led_trigger));
 
-	triggers[trigger].led = led;
+	led_trigger->led = led;
+	led_trigger->trigger = trigger;
+	list_add_tail(&led_trigger->list, &led_triggers);
 
-	if (led && trigger == LED_TRIGGER_DEFAULT_ON)
-		led_set(triggers[trigger].led, triggers[trigger].led->max_value);
-	if (led && trigger == LED_TRIGGER_HEARTBEAT)
+	if (trigger == LED_TRIGGER_DEFAULT_ON)
+		led_set(led, led->max_value);
+	if (trigger == LED_TRIGGER_HEARTBEAT)
 		led_blink(led, 200, 1000);
 
 	return 0;
 }
 
+static char *trigger_names[] = {
+	[LED_TRIGGER_PANIC] = "panic",
+	[LED_TRIGGER_HEARTBEAT] = "heartbeat",
+	[LED_TRIGGER_NET_RX] = "net-rx",
+	[LED_TRIGGER_NET_TX] = "net-tx",
+	[LED_TRIGGER_NET_TXRX] = "net",
+	[LED_TRIGGER_DEFAULT_ON] = "default-on",
+};
+
+const char *trigger_name(enum led_trigger trigger)
+{
+	return trigger_names[trigger];
+}
+
+enum led_trigger trigger_by_name(const char *name)
+{
+	int i;
+
+	for (i = 0; i < LED_TRIGGER_MAX; i++)
+		if (!strcmp(name, trigger_names[i]))
+			return i;
+
+	return LED_TRIGGER_MAX;
+}
+
 /**
- * led_get_trigger - get the LED for a trigger
- * @param trigger	The trigger to set a LED for
- *
- * return the LED number of a trigger.
+ * led_triggers_show_info - Show information about all registered
+ * triggers
  */
-int led_get_trigger(enum led_trigger trigger)
+void led_triggers_show_info(void)
 {
-	if (trigger >= LED_TRIGGER_MAX)
-		return -EINVAL;
-	if (!triggers[trigger].led)
-		return -ENODEV;
-	return led_get_number(triggers[trigger].led);
+	struct led_trigger_struct *led_trigger;
+	int i;
+
+	for (i = 0; i < LED_TRIGGER_MAX; i++) {
+		printf("%s", trigger_name(i));
+
+		list_for_each_entry(led_trigger, &led_triggers, list) {
+			struct led *led = led_trigger->led;
+
+			if (led_trigger->trigger != i)
+				continue;
+
+			printf("\n  LED %d (%s)", led->num, led->name);
+		}
+
+		printf("\n");
+	}
 }
diff --git a/include/led.h b/include/led.h
index fb5c48b9e..0ce857129 100644
--- a/include/led.h
+++ b/include/led.h
@@ -49,6 +49,7 @@ enum led_trigger {
 	LED_TRIGGER_NET_TXRX,
 	LED_TRIGGER_DEFAULT_ON,
 	LED_TRIGGER_MAX,
+	LED_TRIGGER_INVALID = LED_TRIGGER_MAX,
 };
 
 enum trigger_type {
@@ -59,6 +60,7 @@ enum trigger_type {
 
 #ifdef CONFIG_LED_TRIGGERS
 int led_set_trigger(enum led_trigger trigger, struct led *led);
+void led_trigger_disable(struct led *led);
 void led_trigger(enum led_trigger trigger, enum trigger_type);
 #else
 static inline int led_set_trigger(enum led_trigger trigger, struct led *led)
@@ -71,7 +73,9 @@ static inline void led_trigger(enum led_trigger trigger, enum trigger_type type)
 }
 #endif
 
-int led_get_trigger(enum led_trigger trigger);
+void led_triggers_show_info(void);
+const char *trigger_name(enum led_trigger trigger);
+enum led_trigger trigger_by_name(const char *name);
 
 void led_of_parse_trigger(struct led *led, struct device_node *np);
 
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 0/4] upstream led blinking rework

[Oleksij Rempel]

changes v1:
 - merge blinking/flashing and led_blink_pattern patches
 - remove new unsued variables from structs
 - add sanity check in the led command to avoid simultan -b anf -f options

changes v2:
 - reword commit message in "led: add blinking/flashing..." patch

Sascha Hauer (4):
  led: add blinking/flashing and led_blink_pattern interface
  led: Add blink/flash to led command
  led: trigger: Use led triggers
  led-trigger: rework

 commands/led.c             |  44 ++++++++++++-
 commands/trigger.c         |  54 ++++++++--------
 drivers/led/core.c         | 102 ++++++++++++++++++++++++++++-
 drivers/led/led-triggers.c | 157 ++++++++++++++++++++++++++++-----------------
 include/led.h              |  17 ++++-
 5 files changed, 287 insertions(+), 87 deletions(-)

-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 1/4] led: add blinking/flashing and led_blink_pattern interface

[Oleksij Rempel]

From: Sascha Hauer <s.hauer@pengutronix.de>

So far blinking/flashing LEDs is only supported on led-trigger level.
Even without triggers it useful to be able to blink/flash LEDs, so
add this functionality to the LED core.

A led_blink_pattern consists of a number of on and off-periods which
are described in an array. Using such an array you can encode nearly
every blink pattern you need.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/led/core.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 include/led.h      |  11 ++++++
 2 files changed, 112 insertions(+), 1 deletion(-)

diff --git a/drivers/led/core.c b/drivers/led/core.c
index 30b016bb3..ab293b562 100644
--- a/drivers/led/core.c
+++ b/drivers/led/core.c
@@ -23,6 +23,7 @@
 #include <linux/list.h>
 #include <errno.h>
 #include <led.h>
+#include <init.h>
 #include <poller.h>
 #include <clock.h>
 #include <linux/ctype.h>
@@ -101,7 +102,7 @@ struct led *led_by_name_or_number(const char *str)
  * @param led	the led
  * @param value	the value of the LED (0 is disabled)
  */
-int led_set(struct led *led, unsigned int value)
+static int __led_set(struct led *led, unsigned int value)
 {
 	if (value > led->max_value)
 		value = led->max_value;
@@ -114,6 +115,105 @@ int led_set(struct led *led, unsigned int value)
 	return 0;
 }
 
+int led_set(struct led *led, unsigned int value)
+{
+	led->blink = 0;
+	led->flash = 0;
+	return __led_set(led, value);
+}
+
+static void led_blink_func(struct poller_struct *poller)
+{
+	struct led *led;
+
+	list_for_each_entry(led, &leds, list) {
+		bool on;
+
+		if (!led->blink && !led->flash)
+			continue;
+
+		if (led->blink_next_event > get_time_ns()) {
+			continue;
+		}
+
+		on = !(led->blink_next_state % 2);
+
+		led->blink_next_event = get_time_ns() +
+			(led->blink_states[led->blink_next_state] * MSECOND);
+		led->blink_next_state = (led->blink_next_state + 1) %
+					led->blink_nr_states;
+
+		if (led->flash && !on)
+			led->flash = 0;
+
+		__led_set(led, on);
+	}
+}
+
+/**
+ * led_blink_pattern - Blink a led with flexible timings.
+ * @led LED used
+ * @pattern Array of millisecond intervals describing the on and off periods of
+ * the pattern. At the end of the array/pattern it is repeated. The array
+ * starts with an on-period. In general every array item with even index
+ * describes an on-period, every item with odd index an off-period.
+ * @pattern_len Length of the pattern array.
+ *
+ * Returns 0 on success.
+ *
+ * Example:
+ * 	pattern = {500, 1000};
+ * 	This will enable the LED for 500ms and disable it for 1000ms after
+ * 	that. This is repeated forever.
+ */
+int led_blink_pattern(struct led *led, const unsigned int *pattern,
+		      unsigned int pattern_len)
+{
+	if (led->blink_states)
+		free(led->blink_states);
+	led->blink_states = xmemdup(pattern,
+				    pattern_len * sizeof(*led->blink_states));
+	led->blink_nr_states = pattern_len;
+	led->blink_next_state = 0;
+	led->blink_next_event = get_time_ns();
+	led->blink = 1;
+	led->flash = 0;
+
+	return 0;
+}
+
+int led_blink(struct led *led, unsigned int on_ms, unsigned int off_ms)
+{
+	unsigned int pattern[] = {on_ms, off_ms};
+
+	return led_blink_pattern(led, pattern, 2);
+}
+
+int led_flash(struct led *led, unsigned int duration_ms)
+{
+	unsigned int pattern[] = {duration_ms, 0};
+	int ret;
+
+	ret = led_blink_pattern(led, pattern, 2);
+	if (ret)
+		return ret;
+
+	led->flash = 1;
+	led->blink = 0;
+
+	return 0;
+}
+
+static struct poller_struct led_poller = {
+	.func = led_blink_func,
+};
+
+static int led_blink_init(void)
+{
+	return poller_register(&led_poller);
+}
+late_initcall(led_blink_init);
+
 /**
  * led_set_num - set the value of a LED
  * @param num	the number of the LED
diff --git a/include/led.h b/include/led.h
index 000267cdc..fb5c48b9e 100644
--- a/include/led.h
+++ b/include/led.h
@@ -12,6 +12,13 @@ struct led {
 	char *name;
 	int num;
 	struct list_head list;
+
+	int blink;
+	int flash;
+	unsigned int *blink_states;
+	int blink_nr_states;
+	int blink_next_state;
+	uint64_t blink_next_event;
 };
 
 struct led *led_by_number(int no);
@@ -25,6 +32,10 @@ static inline int led_get_number(struct led *led)
 
 int led_set_num(int num, unsigned int value);
 int led_set(struct led *led, unsigned int value);
+int led_blink(struct led *led, unsigned int on_ms, unsigned int off_ms);
+int led_blink_pattern(struct led *led, const unsigned int *pattern,
+		      unsigned int pattern_len);
+int led_flash(struct led *led, unsigned int duration_ms);
 int led_register(struct led *led);
 void led_unregister(struct led *led);
 void led_unregister(struct led *led);
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 1/4] led: add blinking/flashing and led_blink_pattern interface

[Sascha Hauer]

On Sun, Mar 12, 2017 at 09:19:21AM +0100, Oleksij Rempel wrote:
> From: Sascha Hauer <s.hauer@pengutronix.de>
> 
> So far blinking/flashing LEDs is only supported on led-trigger level.
> Even without triggers it useful to be able to blink/flash LEDs, so
> add this functionality to the LED core.
> 
> A led_blink_pattern consists of a number of on and off-periods which
> are described in an array. Using such an array you can encode nearly
> every blink pattern you need.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/led/core.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  include/led.h      |  11 ++++++
>  2 files changed, 112 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/led/core.c b/drivers/led/core.c
> index 30b016bb3..ab293b562 100644
> --- a/drivers/led/core.c
> +++ b/drivers/led/core.c
> @@ -23,6 +23,7 @@
>  #include <linux/list.h>
>  #include <errno.h>
>  #include <led.h>
> +#include <init.h>
>  #include <poller.h>
>  #include <clock.h>
>  #include <linux/ctype.h>
> @@ -101,7 +102,7 @@ struct led *led_by_name_or_number(const char *str)
>   * @param led	the led
>   * @param value	the value of the LED (0 is disabled)
>   */
> -int led_set(struct led *led, unsigned int value)
> +static int __led_set(struct led *led, unsigned int value)
>  {
>  	if (value > led->max_value)
>  		value = led->max_value;
> @@ -114,6 +115,105 @@ int led_set(struct led *led, unsigned int value)
>  	return 0;
>  }
>  
> +int led_set(struct led *led, unsigned int value)
> +{
> +	led->blink = 0;
> +	led->flash = 0;
> +	return __led_set(led, value);
> +}
> +
> +static void led_blink_func(struct poller_struct *poller)
> +{
> +	struct led *led;
> +
> +	list_for_each_entry(led, &leds, list) {
> +		bool on;
> +
> +		if (!led->blink && !led->flash)
> +			continue;
> +
> +		if (led->blink_next_event > get_time_ns()) {
> +			continue;
> +		}
> +
> +		on = !(led->blink_next_state % 2);
> +
> +		led->blink_next_event = get_time_ns() +
> +			(led->blink_states[led->blink_next_state] * MSECOND);
> +		led->blink_next_state = (led->blink_next_state + 1) %
> +					led->blink_nr_states;
> +
> +		if (led->flash && !on)
> +			led->flash = 0;
> +
> +		__led_set(led, on);
> +	}
> +}
> +
> +/**
> + * led_blink_pattern - Blink a led with flexible timings.
> + * @led LED used
> + * @pattern Array of millisecond intervals describing the on and off periods of
> + * the pattern. At the end of the array/pattern it is repeated. The array
> + * starts with an on-period. In general every array item with even index
> + * describes an on-period, every item with odd index an off-period.
> + * @pattern_len Length of the pattern array.
> + *
> + * Returns 0 on success.
> + *
> + * Example:
> + * 	pattern = {500, 1000};
> + * 	This will enable the LED for 500ms and disable it for 1000ms after
> + * 	that. This is repeated forever.
> + */
> +int led_blink_pattern(struct led *led, const unsigned int *pattern,
> +		      unsigned int pattern_len)
> +{
> +	if (led->blink_states)
> +		free(led->blink_states);

The test is unnecessary.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 2/4] led: Add blink/flash to led command

[Oleksij Rempel]

From: Sascha Hauer <s.hauer@pengutronix.de>

The LED framework now supports blinking/flashing LEDs, so
add this functionality to the led command.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 commands/led.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/commands/led.c b/commands/led.c
index 354f74df8..a53f0df6a 100644
--- a/commands/led.c
+++ b/commands/led.c
@@ -29,6 +29,44 @@ static int do_led(int argc, char *argv[])
 	unsigned long value;
 	struct led *led;
 	int ret;
+	int opt;
+	int flash = 0, blink = 0;
+	int blink_on_ms = 500;
+	int blink_off_ms = 500;
+
+	while ((opt = getopt(argc, argv, "fb")) > 0) {
+		switch(opt) {
+		case 'f':
+			flash = 1;
+			break;
+		case 'b':
+			blink = 1;
+			break;
+		}
+	}
+
+	if (flash || blink) {
+		int args = argc - optind;
+
+		if (!args || (flash && blink))
+			return COMMAND_ERROR_USAGE;
+
+		led = led_by_name_or_number(argv[optind]);
+		if (!led) {
+			printf("no such LED: %s\n", argv[optind]);
+			return 1;
+		}
+
+		if (args > 1)
+			blink_on_ms = simple_strtoul(argv[optind + 1], NULL, 0);
+		if (args > 2)
+			blink_off_ms = simple_strtoul(argv[optind + 2], NULL, 0);
+
+		if (flash)
+			return led_flash(led, blink_on_ms);
+		if (blink)
+			return led_blink(led, blink_on_ms, blink_off_ms);
+	}
 
 	if (argc == 1) {
 		int i = 0;
@@ -73,9 +111,13 @@ static int do_led(int argc, char *argv[])
 BAREBOX_CMD_HELP_START(led)
 BAREBOX_CMD_HELP_TEXT("Control the value of a LED. The exact meaning of VALUE is unspecified,")
 BAREBOX_CMD_HELP_TEXT("it can be a brightness, or a color. Most often a value of '1' means on")
-BAREBOX_CMD_HELP_TEXT("and '0' means off.")
+BAREBOX_CMD_HELP_TEXT("and '0' means off. Basic usage is 'led <led> <value>'. LEDs can be given")
+BAREBOX_CMD_HELP_TEXT("by name or number.")
 BAREBOX_CMD_HELP_TEXT("")
 BAREBOX_CMD_HELP_TEXT("Without arguments the available LEDs are listed.")
+BAREBOX_CMD_HELP_TEXT("Options:")
+BAREBOX_CMD_HELP_OPT ("-b <led> [duration-on-ms] [duration-off-ms]",  "blink a LED")
+BAREBOX_CMD_HELP_OPT ("-f <led> [duration-ms]",  "flash a LED")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(led)
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 3/4] led: trigger: Use led triggers

[Oleksij Rempel]

From: Sascha Hauer <s.hauer@pengutronix.de>

Since the LED framework now supports blinking/flashing, use this
functionality in the LED triggers.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/led/led-triggers.c | 40 +++++-----------------------------------
 1 file changed, 5 insertions(+), 35 deletions(-)

diff --git a/drivers/led/led-triggers.c b/drivers/led/led-triggers.c
index dee936739..6c05cc5bb 100644
--- a/drivers/led/led-triggers.c
+++ b/drivers/led/led-triggers.c
@@ -48,34 +48,12 @@
 
 struct led_trigger_struct {
 	struct led *led;
-	uint64_t flash_start;
-	int flash;
+	struct list_head list;
+	enum led_trigger trigger;
 };
 
 static struct led_trigger_struct triggers[LED_TRIGGER_MAX];
 
-static void trigger_func(struct poller_struct *poller)
-{
-	int i;
-
-	for (i = 0; i < LED_TRIGGER_MAX; i++) {
-		if (triggers[i].led &&
-		    triggers[i].flash &&
-		    is_timeout(triggers[i].flash_start, 200 * MSECOND)) {
-			led_set(triggers[i].led, 0);
-			triggers[i].flash = 0;
-		}
-	}
-
-	if (triggers[LED_TRIGGER_HEARTBEAT].led &&
-			is_timeout(triggers[LED_TRIGGER_HEARTBEAT].flash_start, SECOND))
-		led_trigger(LED_TRIGGER_HEARTBEAT, TRIGGER_FLASH);
-}
-
-static struct poller_struct trigger_poller = {
-	.func = trigger_func,
-};
-
 /**
  * led_trigger - triggers a trigger
  * @param trigger	The trigger to enable/disable
@@ -91,11 +69,7 @@ void led_trigger(enum led_trigger trigger, enum trigger_type type)
 		return;
 
 	if (type == TRIGGER_FLASH) {
-		if (is_timeout(triggers[trigger].flash_start, 400 * MSECOND)) {
-			led_set(triggers[trigger].led, triggers[trigger].led->max_value);
-			triggers[trigger].flash_start = get_time_ns();
-			triggers[trigger].flash = 1;
-		}
+		led_flash(triggers[trigger].led, 200);
 		return;
 	}
 
@@ -129,6 +103,8 @@ int led_set_trigger(enum led_trigger trigger, struct led *led)
 
 	if (led && trigger == LED_TRIGGER_DEFAULT_ON)
 		led_set(triggers[trigger].led, triggers[trigger].led->max_value);
+	if (led && trigger == LED_TRIGGER_HEARTBEAT)
+		led_blink(led, 200, 1000);
 
 	return 0;
 }
@@ -147,9 +123,3 @@ int led_get_trigger(enum led_trigger trigger)
 		return -ENODEV;
 	return led_get_number(triggers[trigger].led);
 }
-
-static int trigger_init(void)
-{
-	return poller_register(&trigger_poller);
-}
-late_initcall(trigger_init);
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH v2 4/4] led-trigger: rework

[Oleksij Rempel]

From: Sascha Hauer <s.hauer@pengutronix.de>

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 commands/trigger.c         |  54 +++++++++++---------
 drivers/led/led-triggers.c | 125 +++++++++++++++++++++++++++++++++++----------
 include/led.h              |   6 ++-
 3 files changed, 131 insertions(+), 54 deletions(-)

diff --git a/commands/trigger.c b/commands/trigger.c
index 2758ce74e..0dd3b346f 100644
--- a/commands/trigger.c
+++ b/commands/trigger.c
@@ -28,59 +28,62 @@
 #define LED_COMMAND_SHOW_INFO		2
 #define	LED_COMMAND_DISABLE_TRIGGER	3
 
-static char *trigger_names[] = {
-	[LED_TRIGGER_PANIC] = "panic",
-	[LED_TRIGGER_HEARTBEAT] = "heartbeat",
-	[LED_TRIGGER_NET_RX] = "net rx",
-	[LED_TRIGGER_NET_TX] = "net tx",
-	[LED_TRIGGER_NET_TXRX] = "net",
-	[LED_TRIGGER_DEFAULT_ON] = "default on",
-};
 
 static int do_trigger(int argc, char *argv[])
 {
-	struct led *led;
-	int i, opt, ret = 0;
+	struct led *led = NULL;
+	int opt, ret = 0;
 	int cmd = LED_COMMAND_SHOW_INFO;
-	unsigned long trigger = 0;
+	enum led_trigger trigger;
+	const char *led_name = NULL;
+	const char *trigger_name = NULL;
 
 	while((opt = getopt(argc, argv, "t:d:")) > 0) {
 		switch(opt) {
 		case 't':
-			trigger = simple_strtoul(optarg, NULL, 0);
+			trigger_name = optarg;
 			cmd = LED_COMMAND_SET_TRIGGER;
 			break;
 		case 'd':
-			trigger = simple_strtoul(optarg, NULL, 0);
+			led_name = optarg;
 			cmd = LED_COMMAND_DISABLE_TRIGGER;
 		}
 	}
 
+	if (optind < argc)
+		led = led_by_name_or_number(argv[optind]);
+
 	switch (cmd) {
 	case LED_COMMAND_SHOW_INFO:
-		for (i = 0; i < LED_TRIGGER_MAX; i++) {
-			int led = led_get_trigger(i);
-			printf("%d: %s", i, trigger_names[i]);
-			if (led >= 0)
-				printf(" (led %d)", led);
-			printf("\n");
-		}
+		led_triggers_show_info();
 		break;
 
 	case LED_COMMAND_DISABLE_TRIGGER:
-		ret = led_set_trigger(trigger, NULL);
+		led = led_by_name_or_number(led_name);
+		if (!led) {
+			printf("no such led: %s\n", led_name);
+			return 1;
+		}
+
+		led_trigger_disable(led);
 		break;
 
 	case LED_COMMAND_SET_TRIGGER:
 		if (argc - optind != 1)
 			return COMMAND_ERROR_USAGE;
-		led = led_by_name_or_number(argv[optind]);
 
+		led = led_by_name_or_number(argv[optind]);
 		if (!led) {
 			printf("no such led: %s\n", argv[optind]);
 			return 1;
 		}
 
+		trigger = trigger_by_name(trigger_name);
+		if (trigger == LED_TRIGGER_INVALID) {
+			printf("no such trigger: %s\n", trigger_name);
+			return 1;
+		}
+
 		ret = led_set_trigger(trigger, led);
 		break;
 	}
@@ -92,16 +95,17 @@ static int do_trigger(int argc, char *argv[])
 
 BAREBOX_CMD_HELP_START(trigger)
 BAREBOX_CMD_HELP_TEXT("Control a LED trigger. Without options assigned triggers are shown.")
+BAREBOX_CMD_HELP_TEXT("triggers are given with their name, LEDs are given with their name or number")
 BAREBOX_CMD_HELP_TEXT("")
 BAREBOX_CMD_HELP_TEXT("Options:")
-BAREBOX_CMD_HELP_OPT ("-t",  "set a trigger (needs LED argument)")
-BAREBOX_CMD_HELP_OPT ("-d",  "disable a trigger")
+BAREBOX_CMD_HELP_OPT ("-t <trigger> <led>",  "set a trigger")
+BAREBOX_CMD_HELP_OPT ("-d <led>",  "disable a trigger")
 BAREBOX_CMD_HELP_END
 
 BAREBOX_CMD_START(trigger)
 	.cmd		= do_trigger,
 	BAREBOX_CMD_DESC("handle LED triggers")
-	BAREBOX_CMD_OPTS("[-td] TRIGGER [LED]")
+	BAREBOX_CMD_OPTS("[-td] [LED]")
 	BAREBOX_CMD_GROUP(CMD_GRP_HWMANIP)
 	BAREBOX_CMD_HELP(cmd_trigger_help)
 BAREBOX_CMD_END
diff --git a/drivers/led/led-triggers.c b/drivers/led/led-triggers.c
index 6c05cc5bb..76a1481e1 100644
--- a/drivers/led/led-triggers.c
+++ b/drivers/led/led-triggers.c
@@ -52,7 +52,7 @@ struct led_trigger_struct {
 	enum led_trigger trigger;
 };
 
-static struct led_trigger_struct triggers[LED_TRIGGER_MAX];
+static LIST_HEAD(led_triggers);
 
 /**
  * led_trigger - triggers a trigger
@@ -63,17 +63,53 @@ static struct led_trigger_struct triggers[LED_TRIGGER_MAX];
  */
 void led_trigger(enum led_trigger trigger, enum trigger_type type)
 {
+	struct led_trigger_struct *led_trigger;
+
 	if (trigger >= LED_TRIGGER_MAX)
 		return;
-	if (!triggers[trigger].led)
-		return;
 
-	if (type == TRIGGER_FLASH) {
-		led_flash(triggers[trigger].led, 200);
-		return;
+	list_for_each_entry(led_trigger, &led_triggers, list) {
+		if (led_trigger->trigger != trigger)
+			continue;
+
+		switch (type) {
+		case TRIGGER_FLASH:
+			led_flash(led_trigger->led, 200);
+			break;
+		case TRIGGER_ENABLE:
+			led_set(led_trigger->led, led_trigger->led->max_value);
+			break;
+		case TRIGGER_DISABLE:
+			led_set(led_trigger->led, 0);
+			break;
+		}
 	}
+}
+
+static struct led_trigger_struct *led_find_trigger(struct led *led)
+{
+	struct led_trigger_struct *led_trigger;
+
+	list_for_each_entry(led_trigger, &led_triggers, list)
+		if (led_trigger->led == led)
+			return led_trigger;
+
+	return NULL;
+}
 
-	led_set(triggers[trigger].led, type == TRIGGER_ENABLE ? triggers[trigger].led->max_value : 0);
+void led_trigger_disable(struct led *led)
+{
+	struct led_trigger_struct *led_trigger;
+
+	led_trigger = led_find_trigger(led);
+	if (!led_trigger)
+		return;
+
+	list_del(&led_trigger->list);
+
+	led_set(led, 0);
+
+	free(led_trigger);
 }
 
 /**
@@ -86,40 +122,73 @@ void led_trigger(enum led_trigger trigger, enum trigger_type type)
  */
 int led_set_trigger(enum led_trigger trigger, struct led *led)
 {
-	int i;
+	struct led_trigger_struct *led_trigger;
 
 	if (trigger >= LED_TRIGGER_MAX)
 		return -EINVAL;
 
-	if (led)
-		for (i = 0; i < LED_TRIGGER_MAX; i++)
-			if (triggers[i].led == led)
-				return -EBUSY;
+	led_trigger_disable(led);
 
-	if (triggers[trigger].led && !led)
-		led_set(triggers[trigger].led, 0);
+	led_trigger = xzalloc(sizeof(*led_trigger));
 
-	triggers[trigger].led = led;
+	led_trigger->led = led;
+	led_trigger->trigger = trigger;
+	list_add_tail(&led_trigger->list, &led_triggers);
 
-	if (led && trigger == LED_TRIGGER_DEFAULT_ON)
-		led_set(triggers[trigger].led, triggers[trigger].led->max_value);
-	if (led && trigger == LED_TRIGGER_HEARTBEAT)
+	if (trigger == LED_TRIGGER_DEFAULT_ON)
+		led_set(led, led->max_value);
+	if (trigger == LED_TRIGGER_HEARTBEAT)
 		led_blink(led, 200, 1000);
 
 	return 0;
 }
 
+static char *trigger_names[] = {
+	[LED_TRIGGER_PANIC] = "panic",
+	[LED_TRIGGER_HEARTBEAT] = "heartbeat",
+	[LED_TRIGGER_NET_RX] = "net-rx",
+	[LED_TRIGGER_NET_TX] = "net-tx",
+	[LED_TRIGGER_NET_TXRX] = "net",
+	[LED_TRIGGER_DEFAULT_ON] = "default-on",
+};
+
+const char *trigger_name(enum led_trigger trigger)
+{
+	return trigger_names[trigger];
+}
+
+enum led_trigger trigger_by_name(const char *name)
+{
+	int i;
+
+	for (i = 0; i < LED_TRIGGER_MAX; i++)
+		if (!strcmp(name, trigger_names[i]))
+			return i;
+
+	return LED_TRIGGER_MAX;
+}
+
 /**
- * led_get_trigger - get the LED for a trigger
- * @param trigger	The trigger to set a LED for
- *
- * return the LED number of a trigger.
+ * led_triggers_show_info - Show information about all registered
+ * triggers
  */
-int led_get_trigger(enum led_trigger trigger)
+void led_triggers_show_info(void)
 {
-	if (trigger >= LED_TRIGGER_MAX)
-		return -EINVAL;
-	if (!triggers[trigger].led)
-		return -ENODEV;
-	return led_get_number(triggers[trigger].led);
+	struct led_trigger_struct *led_trigger;
+	int i;
+
+	for (i = 0; i < LED_TRIGGER_MAX; i++) {
+		printf("%s", trigger_name(i));
+
+		list_for_each_entry(led_trigger, &led_triggers, list) {
+			struct led *led = led_trigger->led;
+
+			if (led_trigger->trigger != i)
+				continue;
+
+			printf("\n  LED %d (%s)", led->num, led->name);
+		}
+
+		printf("\n");
+	}
 }
diff --git a/include/led.h b/include/led.h
index fb5c48b9e..0ce857129 100644
--- a/include/led.h
+++ b/include/led.h
@@ -49,6 +49,7 @@ enum led_trigger {
 	LED_TRIGGER_NET_TXRX,
 	LED_TRIGGER_DEFAULT_ON,
 	LED_TRIGGER_MAX,
+	LED_TRIGGER_INVALID = LED_TRIGGER_MAX,
 };
 
 enum trigger_type {
@@ -59,6 +60,7 @@ enum trigger_type {
 
 #ifdef CONFIG_LED_TRIGGERS
 int led_set_trigger(enum led_trigger trigger, struct led *led);
+void led_trigger_disable(struct led *led);
 void led_trigger(enum led_trigger trigger, enum trigger_type);
 #else
 static inline int led_set_trigger(enum led_trigger trigger, struct led *led)
@@ -71,7 +73,9 @@ static inline void led_trigger(enum led_trigger trigger, enum trigger_type type)
 }
 #endif
 
-int led_get_trigger(enum led_trigger trigger);
+void led_triggers_show_info(void);
+const char *trigger_name(enum led_trigger trigger);
+enum led_trigger trigger_by_name(const char *name);
 
 void led_of_parse_trigger(struct led *led, struct device_node *np);
 
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v2 4/4] led-trigger: rework

[Sascha Hauer]

On Sun, Mar 12, 2017 at 09:19:24AM +0100, Oleksij Rempel wrote:
> From: Sascha Hauer <s.hauer@pengutronix.de>
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  commands/trigger.c         |  54 +++++++++++---------
>  drivers/led/led-triggers.c | 125 +++++++++++++++++++++++++++++++++++----------
>  include/led.h              |   6 ++-
>  3 files changed, 131 insertions(+), 54 deletions(-)

Sascha, please write a proper commit message :)

Ok, here we go:

led-trigger: Allow multiple led triggers of the same type

We used to have a static array of trigger types which, allows only one led
per trigger. While this is enough for panic and heartbeat, it falls
short when multiple leds are associated to the default-on trigger. The
default-on trigger is used to turn on a led statically from devicetree
and may be used multiple times.
This patch reworks the led triggers so that a trigger struct is
allocated dynamically when needed and put onto a list.

Sascha

> 
> diff --git a/commands/trigger.c b/commands/trigger.c
> index 2758ce74e..0dd3b346f 100644
> --- a/commands/trigger.c
> +++ b/commands/trigger.c
> @@ -28,59 +28,62 @@
>  #define LED_COMMAND_SHOW_INFO		2
>  #define	LED_COMMAND_DISABLE_TRIGGER	3
>  
> -static char *trigger_names[] = {
> -	[LED_TRIGGER_PANIC] = "panic",
> -	[LED_TRIGGER_HEARTBEAT] = "heartbeat",
> -	[LED_TRIGGER_NET_RX] = "net rx",
> -	[LED_TRIGGER_NET_TX] = "net tx",
> -	[LED_TRIGGER_NET_TXRX] = "net",
> -	[LED_TRIGGER_DEFAULT_ON] = "default on",
> -};
>  
>  static int do_trigger(int argc, char *argv[])
>  {
> -	struct led *led;
> -	int i, opt, ret = 0;
> +	struct led *led = NULL;
> +	int opt, ret = 0;
>  	int cmd = LED_COMMAND_SHOW_INFO;
> -	unsigned long trigger = 0;
> +	enum led_trigger trigger;
> +	const char *led_name = NULL;
> +	const char *trigger_name = NULL;
>  
>  	while((opt = getopt(argc, argv, "t:d:")) > 0) {
>  		switch(opt) {
>  		case 't':
> -			trigger = simple_strtoul(optarg, NULL, 0);
> +			trigger_name = optarg;
>  			cmd = LED_COMMAND_SET_TRIGGER;
>  			break;
>  		case 'd':
> -			trigger = simple_strtoul(optarg, NULL, 0);
> +			led_name = optarg;
>  			cmd = LED_COMMAND_DISABLE_TRIGGER;
>  		}
>  	}
>  
> +	if (optind < argc)
> +		led = led_by_name_or_number(argv[optind]);
> +
>  	switch (cmd) {
>  	case LED_COMMAND_SHOW_INFO:
> -		for (i = 0; i < LED_TRIGGER_MAX; i++) {
> -			int led = led_get_trigger(i);
> -			printf("%d: %s", i, trigger_names[i]);
> -			if (led >= 0)
> -				printf(" (led %d)", led);
> -			printf("\n");
> -		}
> +		led_triggers_show_info();
>  		break;
>  
>  	case LED_COMMAND_DISABLE_TRIGGER:
> -		ret = led_set_trigger(trigger, NULL);
> +		led = led_by_name_or_number(led_name);
> +		if (!led) {
> +			printf("no such led: %s\n", led_name);
> +			return 1;
> +		}
> +
> +		led_trigger_disable(led);
>  		break;
>  
>  	case LED_COMMAND_SET_TRIGGER:
>  		if (argc - optind != 1)
>  			return COMMAND_ERROR_USAGE;
> -		led = led_by_name_or_number(argv[optind]);
>  
> +		led = led_by_name_or_number(argv[optind]);
>  		if (!led) {
>  			printf("no such led: %s\n", argv[optind]);
>  			return 1;
>  		}
>  
> +		trigger = trigger_by_name(trigger_name);
> +		if (trigger == LED_TRIGGER_INVALID) {
> +			printf("no such trigger: %s\n", trigger_name);
> +			return 1;
> +		}
> +
>  		ret = led_set_trigger(trigger, led);
>  		break;
>  	}
> @@ -92,16 +95,17 @@ static int do_trigger(int argc, char *argv[])
>  
>  BAREBOX_CMD_HELP_START(trigger)
>  BAREBOX_CMD_HELP_TEXT("Control a LED trigger. Without options assigned triggers are shown.")
> +BAREBOX_CMD_HELP_TEXT("triggers are given with their name, LEDs are given with their name or number")
>  BAREBOX_CMD_HELP_TEXT("")
>  BAREBOX_CMD_HELP_TEXT("Options:")
> -BAREBOX_CMD_HELP_OPT ("-t",  "set a trigger (needs LED argument)")
> -BAREBOX_CMD_HELP_OPT ("-d",  "disable a trigger")
> +BAREBOX_CMD_HELP_OPT ("-t <trigger> <led>",  "set a trigger")
> +BAREBOX_CMD_HELP_OPT ("-d <led>",  "disable a trigger")
>  BAREBOX_CMD_HELP_END
>  
>  BAREBOX_CMD_START(trigger)
>  	.cmd		= do_trigger,
>  	BAREBOX_CMD_DESC("handle LED triggers")
> -	BAREBOX_CMD_OPTS("[-td] TRIGGER [LED]")
> +	BAREBOX_CMD_OPTS("[-td] [LED]")
>  	BAREBOX_CMD_GROUP(CMD_GRP_HWMANIP)
>  	BAREBOX_CMD_HELP(cmd_trigger_help)
>  BAREBOX_CMD_END
> diff --git a/drivers/led/led-triggers.c b/drivers/led/led-triggers.c
> index 6c05cc5bb..76a1481e1 100644
> --- a/drivers/led/led-triggers.c
> +++ b/drivers/led/led-triggers.c
> @@ -52,7 +52,7 @@ struct led_trigger_struct {
>  	enum led_trigger trigger;
>  };
>  
> -static struct led_trigger_struct triggers[LED_TRIGGER_MAX];
> +static LIST_HEAD(led_triggers);
>  
>  /**
>   * led_trigger - triggers a trigger
> @@ -63,17 +63,53 @@ static struct led_trigger_struct triggers[LED_TRIGGER_MAX];
>   */
>  void led_trigger(enum led_trigger trigger, enum trigger_type type)
>  {
> +	struct led_trigger_struct *led_trigger;
> +
>  	if (trigger >= LED_TRIGGER_MAX)
>  		return;
> -	if (!triggers[trigger].led)
> -		return;
>  
> -	if (type == TRIGGER_FLASH) {
> -		led_flash(triggers[trigger].led, 200);
> -		return;
> +	list_for_each_entry(led_trigger, &led_triggers, list) {
> +		if (led_trigger->trigger != trigger)
> +			continue;
> +
> +		switch (type) {
> +		case TRIGGER_FLASH:
> +			led_flash(led_trigger->led, 200);
> +			break;
> +		case TRIGGER_ENABLE:
> +			led_set(led_trigger->led, led_trigger->led->max_value);
> +			break;
> +		case TRIGGER_DISABLE:
> +			led_set(led_trigger->led, 0);
> +			break;
> +		}
>  	}
> +}
> +
> +static struct led_trigger_struct *led_find_trigger(struct led *led)
> +{
> +	struct led_trigger_struct *led_trigger;
> +
> +	list_for_each_entry(led_trigger, &led_triggers, list)
> +		if (led_trigger->led == led)
> +			return led_trigger;
> +
> +	return NULL;
> +}
>  
> -	led_set(triggers[trigger].led, type == TRIGGER_ENABLE ? triggers[trigger].led->max_value : 0);
> +void led_trigger_disable(struct led *led)
> +{
> +	struct led_trigger_struct *led_trigger;
> +
> +	led_trigger = led_find_trigger(led);
> +	if (!led_trigger)
> +		return;
> +
> +	list_del(&led_trigger->list);
> +
> +	led_set(led, 0);
> +
> +	free(led_trigger);
>  }
>  
>  /**
> @@ -86,40 +122,73 @@ void led_trigger(enum led_trigger trigger, enum trigger_type type)
>   */
>  int led_set_trigger(enum led_trigger trigger, struct led *led)
>  {
> -	int i;
> +	struct led_trigger_struct *led_trigger;
>  
>  	if (trigger >= LED_TRIGGER_MAX)
>  		return -EINVAL;
>  
> -	if (led)
> -		for (i = 0; i < LED_TRIGGER_MAX; i++)
> -			if (triggers[i].led == led)
> -				return -EBUSY;
> +	led_trigger_disable(led);
>  
> -	if (triggers[trigger].led && !led)
> -		led_set(triggers[trigger].led, 0);
> +	led_trigger = xzalloc(sizeof(*led_trigger));
>  
> -	triggers[trigger].led = led;
> +	led_trigger->led = led;
> +	led_trigger->trigger = trigger;
> +	list_add_tail(&led_trigger->list, &led_triggers);
>  
> -	if (led && trigger == LED_TRIGGER_DEFAULT_ON)
> -		led_set(triggers[trigger].led, triggers[trigger].led->max_value);
> -	if (led && trigger == LED_TRIGGER_HEARTBEAT)
> +	if (trigger == LED_TRIGGER_DEFAULT_ON)
> +		led_set(led, led->max_value);
> +	if (trigger == LED_TRIGGER_HEARTBEAT)
>  		led_blink(led, 200, 1000);
>  
>  	return 0;
>  }
>  
> +static char *trigger_names[] = {
> +	[LED_TRIGGER_PANIC] = "panic",
> +	[LED_TRIGGER_HEARTBEAT] = "heartbeat",
> +	[LED_TRIGGER_NET_RX] = "net-rx",
> +	[LED_TRIGGER_NET_TX] = "net-tx",
> +	[LED_TRIGGER_NET_TXRX] = "net",
> +	[LED_TRIGGER_DEFAULT_ON] = "default-on",
> +};
> +
> +const char *trigger_name(enum led_trigger trigger)
> +{
> +	return trigger_names[trigger];
> +}
> +
> +enum led_trigger trigger_by_name(const char *name)
> +{
> +	int i;
> +
> +	for (i = 0; i < LED_TRIGGER_MAX; i++)
> +		if (!strcmp(name, trigger_names[i]))
> +			return i;
> +
> +	return LED_TRIGGER_MAX;
> +}
> +
>  /**
> - * led_get_trigger - get the LED for a trigger
> - * @param trigger	The trigger to set a LED for
> - *
> - * return the LED number of a trigger.
> + * led_triggers_show_info - Show information about all registered
> + * triggers
>   */
> -int led_get_trigger(enum led_trigger trigger)
> +void led_triggers_show_info(void)
>  {
> -	if (trigger >= LED_TRIGGER_MAX)
> -		return -EINVAL;
> -	if (!triggers[trigger].led)
> -		return -ENODEV;
> -	return led_get_number(triggers[trigger].led);
> +	struct led_trigger_struct *led_trigger;
> +	int i;
> +
> +	for (i = 0; i < LED_TRIGGER_MAX; i++) {
> +		printf("%s", trigger_name(i));
> +
> +		list_for_each_entry(led_trigger, &led_triggers, list) {
> +			struct led *led = led_trigger->led;
> +
> +			if (led_trigger->trigger != i)
> +				continue;
> +
> +			printf("\n  LED %d (%s)", led->num, led->name);
> +		}
> +
> +		printf("\n");
> +	}
>  }
> diff --git a/include/led.h b/include/led.h
> index fb5c48b9e..0ce857129 100644
> --- a/include/led.h
> +++ b/include/led.h
> @@ -49,6 +49,7 @@ enum led_trigger {
>  	LED_TRIGGER_NET_TXRX,
>  	LED_TRIGGER_DEFAULT_ON,
>  	LED_TRIGGER_MAX,
> +	LED_TRIGGER_INVALID = LED_TRIGGER_MAX,
>  };
>  
>  enum trigger_type {
> @@ -59,6 +60,7 @@ enum trigger_type {
>  
>  #ifdef CONFIG_LED_TRIGGERS
>  int led_set_trigger(enum led_trigger trigger, struct led *led);
> +void led_trigger_disable(struct led *led);
>  void led_trigger(enum led_trigger trigger, enum trigger_type);
>  #else
>  static inline int led_set_trigger(enum led_trigger trigger, struct led *led)
> @@ -71,7 +73,9 @@ static inline void led_trigger(enum led_trigger trigger, enum trigger_type type)
>  }
>  #endif
>  
> -int led_get_trigger(enum led_trigger trigger);
> +void led_triggers_show_info(void);
> +const char *trigger_name(enum led_trigger trigger);
> +enum led_trigger trigger_by_name(const char *name);
>  
>  void led_of_parse_trigger(struct led *led, struct device_node *np);
>  
> -- 
> 2.11.0
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox