[RFC PATCH] leds: trigger: add activity LED trigger

[RFC PATCH] leds: trigger: add activity LED trigger

[Tobias Doerffel]

This is an initial attempt to unify all kinds of activity LED
functionality in various kernel drivers by implementing an additional
layer between simple LED triggers and device drivers.

I started this project as I'm working on activity LED support for serial
ports in parallel where I mainly duplicated code from the CAN stack.
Once serial port LED support has been mainlined, there's going to be
at least one more device driver taking advantage of ledtrig-activity
besides arcnet and can.

[RFC PATCH 1/3] leds: trigger: add activity LED trigger

[Tobias Doerffel]

The LED activity trigger allows device drivers to indicate device
activity by blinking LEDs. A typical use case is to signal RX and/or
TX activity on e.g. CAN busses or serial ports.

This is meant to replace similar code in various device drivers which
register all their activity LEDs individually.

Signed-off-by: Tobias Doerffel <tobias.doerffel@ed-chemnitz.de>
---
 drivers/leds/trigger/Kconfig            |  10 +++
 drivers/leds/trigger/Makefile           |   1 +
 drivers/leds/trigger/ledtrig-activity.c | 147 ++++++++++++++++++++++++++++++++
 include/linux/leds.h                    |  71 +++++++++++++++
 4 files changed, 229 insertions(+)
 create mode 100644 drivers/leds/trigger/ledtrig-activity.c

diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index 3f9ddb9..ca69bac 100644
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -33,6 +33,16 @@ config LEDS_TRIGGER_ONESHOT
 
 	  If unsure, say Y.
 
+config LEDS_TRIGGER_ACTIVITY
+	tristate "LED Activity Trigger"
+	depends on LEDS_TRIGGERS
+	help
+	  This allows device drivers to indicate device activity by blinking
+	  LEDs. A typical use case is to signal RX and/or TX activity on e.g.
+	  CAN busses or serial ports
+
+	  If unsure, say Y.
+
 config LEDS_TRIGGER_DISK
 	bool "LED Disk Trigger"
 	depends on IDE_GD_ATA || ATA
diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
index a72c43c..8431f18 100644
--- a/drivers/leds/trigger/Makefile
+++ b/drivers/leds/trigger/Makefile
@@ -1,5 +1,6 @@
 obj-$(CONFIG_LEDS_TRIGGER_TIMER)	+= ledtrig-timer.o
 obj-$(CONFIG_LEDS_TRIGGER_ONESHOT)	+= ledtrig-oneshot.o
+obj-$(CONFIG_LEDS_TRIGGER_ACTIVITY)	+= ledtrig-activity.o
 obj-$(CONFIG_LEDS_TRIGGER_DISK)		+= ledtrig-disk.o
 obj-$(CONFIG_LEDS_TRIGGER_MTD)		+= ledtrig-mtd.o
 obj-$(CONFIG_LEDS_TRIGGER_HEARTBEAT)	+= ledtrig-heartbeat.o
diff --git a/drivers/leds/trigger/ledtrig-activity.c b/drivers/leds/trigger/ledtrig-activity.c
new file mode 100644
index 0000000..fb71738
--- /dev/null
+++ b/drivers/leds/trigger/ledtrig-activity.c
@@ -0,0 +1,147 @@
+/*
+ * Activity LED Trigger
+ *
+ * Copyright 2016 EDC Electronic Design Chemnitz GmbH
+ *
+ * Author: Tobias Doerffel <tobias.doerffel@ed-chemnitz.de>
+ *
+ * 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/module.h>
+#include <linux/slab.h>
+#include <linux/leds.h>
+
+static const char *activity_suffixes[ACTIVITY_LED_COUNT] = {
+	[ACTIVITY_LED_RX] = "rx",
+	[ACTIVITY_LED_TX] = "tx",
+	[ACTIVITY_LED_RXTX] = "rxtx",
+	[ACTIVITY_LED_RECONNECT] = "recon"
+};
+
+void ledtrig_activity_event(struct activity_led_trigger *trig,
+			    enum activity_led_event event)
+{
+	int i;
+	int event_leds = 0;
+	unsigned long led_delay = 0;
+
+	switch (event) {
+	case ACTIVITY_LED_EVENT_START:
+		for (i = 0; i < ACTIVITY_LED_COUNT; i++) {
+			if (trig->triggers[i].led_trig) {
+				led_trigger_event(trig->triggers[i].led_trig,
+						  LED_FULL);
+			}
+		}
+		break;
+	case ACTIVITY_LED_EVENT_STOP:
+		for (i = 0; i < ACTIVITY_LED_COUNT; i++) {
+			if (trig->triggers[i].led_trig) {
+				led_trigger_event(trig->triggers[i].led_trig,
+						  LED_OFF);
+			}
+		}
+		break;
+	case ACTIVITY_LED_EVENT_RX:
+		event_leds = (ACTIVITY_LEDS_RX | ACTIVITY_LEDS_RXTX);
+		break;
+	case ACTIVITY_LED_EVENT_TX:
+		event_leds = (ACTIVITY_LEDS_TX | ACTIVITY_LEDS_RXTX);
+		break;
+	case ACTIVITY_LED_EVENT_RXTX:
+		event_leds = ACTIVITY_LEDS_RXTX;
+		break;
+	case ACTIVITY_LED_EVENT_RECONNECT:
+		event_leds = ACTIVITY_LEDS_RECONNECT;
+		break;
+	default:
+		break;
+	}
+
+	for (i = 0; i < ACTIVITY_LED_COUNT; i++) {
+		led_delay = trig->triggers[i].delay;
+		if (led_delay > 0 &&
+		    (1 << trig->triggers[i].led) & event_leds) {
+			led_trigger_blink_oneshot(trig->triggers[i].led_trig,
+						  &led_delay, &led_delay, 1);
+		}
+	}
+}
+EXPORT_SYMBOL_GPL(ledtrig_activity_event);
+
+void ledtrig_activity_register(const char *name, int leds,
+			       struct activity_led_trigger **tp)
+{
+	int i;
+	struct activity_led_trigger *trig;
+
+	trig = kzalloc(sizeof(struct activity_led_trigger), GFP_KERNEL);
+
+	for (i = 0; i < ACTIVITY_LED_COUNT; i++) {
+		if (leds & (1 << i)) {
+			trig->triggers[i].led = i;
+			trig->triggers[i].delay = ACTIVITY_LED_DEFAULT_DELAY;
+			snprintf(trig->triggers[i].led_trig_name,
+				 sizeof(trig->triggers[i].led_trig_name),
+				 "%s-%s", name, activity_suffixes[i]);
+			led_trigger_register_simple(
+						trig->triggers[i].led_trig_name,
+						&trig->triggers[i].led_trig);
+		}
+	}
+
+	*tp = trig;
+}
+EXPORT_SYMBOL_GPL(ledtrig_activity_register);
+
+void ledtrig_activity_unregister(struct activity_led_trigger *trig)
+{
+	int i;
+
+	for (i = 0; i < ACTIVITY_LED_COUNT; i++) {
+		if (trig->triggers[i].led_trig) {
+			led_trigger_unregister_simple(
+						trig->triggers[i].led_trig);
+		}
+	}
+
+	kfree(trig);
+}
+EXPORT_SYMBOL_GPL(ledtrig_activity_unregister);
+
+void ledtrig_activity_rename(const char *name,
+			     struct activity_led_trigger *trig)
+{
+	int i;
+	char trig_name[TRIG_NAME_MAX];
+
+	for (i = 0; i < ACTIVITY_LED_COUNT; i++) {
+		if (trig->triggers[i].led_trig) {
+			snprintf(trig_name, sizeof(trig_name),
+				 "%s-%s", name, activity_suffixes[i]);
+			led_trigger_rename_static(trig_name,
+						  trig->triggers[i].led_trig);
+		}
+	}
+}
+EXPORT_SYMBOL_GPL(ledtrig_activity_rename);
+
+void ledtrig_activity_set_delay(struct activity_led_trigger *trig, int leds,
+				unsigned long delay)
+{
+	int i;
+
+	for (i = 0; i < ACTIVITY_LED_COUNT; i++) {
+		if (leds & (1 << i))
+			trig->triggers[i].delay = delay;
+	}
+}
+EXPORT_SYMBOL_GPL(ledtrig_activity_set_delay);
+
+MODULE_AUTHOR("Tobias Doerffel <tobias.doerffel@ed-chemnitz.de>");
+MODULE_DESCRIPTION("Activity LED trigger");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/leds.h b/include/linux/leds.h
index 8a3b5d2..8a37f97 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -414,4 +414,75 @@ static inline void ledtrig_cpu(enum cpu_led_event evt)
 }
 #endif
 
+/*
+ * Activity LED trigger
+ */
+
+enum activity_led_event {
+	ACTIVITY_LED_EVENT_START,
+	ACTIVITY_LED_EVENT_STOP,
+	ACTIVITY_LED_EVENT_RX,
+	ACTIVITY_LED_EVENT_TX,
+	ACTIVITY_LED_EVENT_RXTX,
+	ACTIVITY_LED_EVENT_RECONNECT,
+};
+
+enum activity_led {
+	ACTIVITY_LED_RX,
+	ACTIVITY_LED_TX,
+	ACTIVITY_LED_RXTX,
+	ACTIVITY_LED_RECONNECT,
+	ACTIVITY_LED_COUNT,
+};
+
+#define	ACTIVITY_LEDS_RX		(1 << ACTIVITY_LED_RX)
+#define	ACTIVITY_LEDS_TX		(1 << ACTIVITY_LED_TX)
+#define	ACTIVITY_LEDS_RXTX		(1 << ACTIVITY_LED_RXTX)
+#define	ACTIVITY_LEDS_RECONNECT	(1 << ACTIVITY_LED_RECONNECT)
+
+#define ACTIVITY_LED_DEFAULT_DELAY	50
+
+struct activity_led_event_trigger {
+	struct led_trigger *led_trig;
+	char led_trig_name[TRIG_NAME_MAX];
+	int led;
+	unsigned long delay;
+};
+
+struct activity_led_trigger {
+	struct activity_led_event_trigger triggers[ACTIVITY_LED_COUNT];
+};
+
+#ifdef CONFIG_LEDS_TRIGGER_ACTIVITY
+void ledtrig_activity_event(struct activity_led_trigger *trig,
+			    enum activity_led_event event);
+void ledtrig_activity_register(const char *name, int leds,
+			       struct activity_led_trigger **tp);
+void ledtrig_activity_unregister(struct activity_led_trigger *trig);
+void ledtrig_activity_rename(const char *name,
+			     struct activity_led_trigger *trig);
+void ledtrig_activity_set_delay(struct activity_led_trigger *trig, int leds,
+				unsigned long delay);
+#else
+void ledtrig_activity_event(struct activity_led_trigger *trig,
+			    enum activity_led_event event)
+{
+}
+void ledtrig_activity_register(const char *name, int leds,
+			       struct activity_led_trigger **tp)
+{
+}
+void ledtrig_activity_unregister(struct activity_led_trigger *trig)
+{
+}
+void ledtrig_activity_rename(const char *name,
+			     struct activity_led_trigger *trig)
+{
+}
+void ledtrig_activity_set_delay(struct activity_led_trigger *trig, int leds,
+				unsigned long delay)
+{
+}
+#endif
+
 #endif		/* __LINUX_LEDS_H_INCLUDED */
-- 
2.1.4

[RFC PATCH 2/3] arcnet: make use of ledtrig-activity

[Tobias Doerffel]

Instead of registering and controlling activity LEDs individually use
the new activity LED trigger.

Signed-off-by: Tobias Doerffel <tobias.doerffel@ed-chemnitz.de>
---
 drivers/net/arcnet/arcdevice.h |  7 +------
 drivers/net/arcnet/arcnet.c    | 34 +++++++++++++---------------------
 2 files changed, 14 insertions(+), 27 deletions(-)

diff --git a/drivers/net/arcnet/arcdevice.h b/drivers/net/arcnet/arcdevice.h
index 20bfb9b..3ca76ec 100644
--- a/drivers/net/arcnet/arcdevice.h
+++ b/drivers/net/arcnet/arcdevice.h
@@ -237,8 +237,6 @@ struct Outgoing {
 		numsegs;	/* number of segments */
 };
 
-#define ARCNET_LED_NAME_SZ (IFNAMSIZ + 6)
-
 struct arcnet_local {
 	uint8_t config,		/* current value of CONFIG register */
 		timeout,	/* Extended timeout for COM20020 */
@@ -262,10 +260,7 @@ struct arcnet_local {
 	/* On preemtive and SMB a lock is needed */
 	spinlock_t lock;
 
-	struct led_trigger *tx_led_trig;
-	char tx_led_trig_name[ARCNET_LED_NAME_SZ];
-	struct led_trigger *recon_led_trig;
-	char recon_led_trig_name[ARCNET_LED_NAME_SZ];
+	struct activity_led_trigger *activity_led_trig;
 
 	struct timer_list	timer;
 
diff --git a/drivers/net/arcnet/arcnet.c b/drivers/net/arcnet/arcnet.c
index 6ea963e..49b9061 100644
--- a/drivers/net/arcnet/arcnet.c
+++ b/drivers/net/arcnet/arcnet.c
@@ -195,25 +195,18 @@ static void arcnet_dump_packet(struct net_device *dev, int bufnum,
 void arcnet_led_event(struct net_device *dev, enum arcnet_led_event event)
 {
 	struct arcnet_local *lp = netdev_priv(dev);
-	unsigned long led_delay = 350;
-	unsigned long tx_delay = 50;
+	struct activity_led_trigger *trig = lp->activity_led_trig;
 
 	switch (event) {
 	case ARCNET_LED_EVENT_RECON:
-		led_trigger_blink_oneshot(lp->recon_led_trig,
-					  &led_delay, &led_delay, 0);
+		ledtrig_activity_event(trig, ACTIVITY_LED_EVENT_RECONNECT);
 		break;
 	case ARCNET_LED_EVENT_OPEN:
-		led_trigger_event(lp->tx_led_trig, LED_OFF);
-		led_trigger_event(lp->recon_led_trig, LED_OFF);
-		break;
 	case ARCNET_LED_EVENT_STOP:
-		led_trigger_event(lp->tx_led_trig, LED_OFF);
-		led_trigger_event(lp->recon_led_trig, LED_OFF);
+		ledtrig_activity_event(trig, ACTIVITY_LED_EVENT_STOP);
 		break;
 	case ARCNET_LED_EVENT_TX:
-		led_trigger_blink_oneshot(lp->tx_led_trig,
-					  &tx_delay, &tx_delay, 0);
+		ledtrig_activity_event(trig, ACTIVITY_LED_EVENT_TX);
 		break;
 	}
 }
@@ -223,8 +216,7 @@ static void arcnet_led_release(struct device *gendev, void *res)
 {
 	struct arcnet_local *lp = netdev_priv(to_net_dev(gendev));
 
-	led_trigger_unregister_simple(lp->tx_led_trig);
-	led_trigger_unregister_simple(lp->recon_led_trig);
+	ledtrig_activity_unregister(lp->activity_led_trig);
 }
 
 /* Register ARCNET LED triggers for a arcnet device
@@ -234,6 +226,7 @@ static void arcnet_led_release(struct device *gendev, void *res)
 void devm_arcnet_led_init(struct net_device *netdev, int index, int subid)
 {
 	struct arcnet_local *lp = netdev_priv(netdev);
+	char name[TRIG_NAME_MAX];
 	void *res;
 
 	res = devres_alloc(arcnet_led_release, 0, GFP_KERNEL);
@@ -242,15 +235,14 @@ void devm_arcnet_led_init(struct net_device *netdev, int index, int subid)
 		return;
 	}
 
-	snprintf(lp->tx_led_trig_name, sizeof(lp->tx_led_trig_name),
-		 "arc%d-%d-tx", index, subid);
-	snprintf(lp->recon_led_trig_name, sizeof(lp->recon_led_trig_name),
-		 "arc%d-%d-recon", index, subid);
+	snprintf(name, sizeof(name), "arc%d-%d", index, subid);
+
+	ledtrig_activity_register(name, ACTIVITY_LEDS_TX |
+				  ACTIVITY_LEDS_RECONNECT,
+				  &lp->activity_led_trig);
 
-	led_trigger_register_simple(lp->tx_led_trig_name,
-				    &lp->tx_led_trig);
-	led_trigger_register_simple(lp->recon_led_trig_name,
-				    &lp->recon_led_trig);
+	ledtrig_activity_set_delay(lp->activity_led_trig,
+				   ACTIVITY_LEDS_RECONNECT, 350);
 
 	devres_add(&netdev->dev, res);
 }
-- 
2.1.4

[RFC PATCH 3/3] can: make use of ledtrig-activity

[Tobias Doerffel]

Instead of registering and controlling activity LEDs individually use
the new activity LED trigger.

Signed-off-by: Tobias Doerffel <tobias.doerffel@ed-chemnitz.de>
---
 drivers/net/can/led.c   | 58 ++++++++++---------------------------------------
 include/linux/can/dev.h |  7 +-----
 include/linux/can/led.h |  2 --
 3 files changed, 12 insertions(+), 55 deletions(-)

diff --git a/drivers/net/can/led.c b/drivers/net/can/led.c
index c1b6676..cd2b99c 100644
--- a/drivers/net/can/led.c
+++ b/drivers/net/can/led.c
@@ -25,33 +25,20 @@ MODULE_PARM_DESC(led_delay,
 void can_led_event(struct net_device *netdev, enum can_led_event event)
 {
 	struct can_priv *priv = netdev_priv(netdev);
+	struct activity_led_trigger *trig = priv->activity_led_trig;
 
 	switch (event) {
 	case CAN_LED_EVENT_OPEN:
-		led_trigger_event(priv->tx_led_trig, LED_FULL);
-		led_trigger_event(priv->rx_led_trig, LED_FULL);
-		led_trigger_event(priv->rxtx_led_trig, LED_FULL);
+		ledtrig_activity_event(trig, ACTIVITY_LED_EVENT_START);
 		break;
 	case CAN_LED_EVENT_STOP:
-		led_trigger_event(priv->tx_led_trig, LED_OFF);
-		led_trigger_event(priv->rx_led_trig, LED_OFF);
-		led_trigger_event(priv->rxtx_led_trig, LED_OFF);
+		ledtrig_activity_event(trig, ACTIVITY_LED_EVENT_STOP);
 		break;
 	case CAN_LED_EVENT_TX:
-		if (led_delay) {
-			led_trigger_blink_oneshot(priv->tx_led_trig,
-						  &led_delay, &led_delay, 1);
-			led_trigger_blink_oneshot(priv->rxtx_led_trig,
-						  &led_delay, &led_delay, 1);
-		}
+		ledtrig_activity_event(trig, ACTIVITY_LED_EVENT_TX);
 		break;
 	case CAN_LED_EVENT_RX:
-		if (led_delay) {
-			led_trigger_blink_oneshot(priv->rx_led_trig,
-						  &led_delay, &led_delay, 1);
-			led_trigger_blink_oneshot(priv->rxtx_led_trig,
-						  &led_delay, &led_delay, 1);
-		}
+		ledtrig_activity_event(trig, ACTIVITY_LED_EVENT_RX);
 		break;
 	}
 }
@@ -61,9 +48,7 @@ static void can_led_release(struct device *gendev, void *res)
 {
 	struct can_priv *priv = netdev_priv(to_net_dev(gendev));
 
-	led_trigger_unregister_simple(priv->tx_led_trig);
-	led_trigger_unregister_simple(priv->rx_led_trig);
-	led_trigger_unregister_simple(priv->rxtx_led_trig);
+	ledtrig_activity_unregister(priv->activity_led_trig);
 }
 
 /* Register CAN LED triggers for a CAN device
@@ -81,19 +66,9 @@ void devm_can_led_init(struct net_device *netdev)
 		return;
 	}
 
-	snprintf(priv->tx_led_trig_name, sizeof(priv->tx_led_trig_name),
-		 "%s-tx", netdev->name);
-	snprintf(priv->rx_led_trig_name, sizeof(priv->rx_led_trig_name),
-		 "%s-rx", netdev->name);
-	snprintf(priv->rxtx_led_trig_name, sizeof(priv->rxtx_led_trig_name),
-		 "%s-rxtx", netdev->name);
-
-	led_trigger_register_simple(priv->tx_led_trig_name,
-				    &priv->tx_led_trig);
-	led_trigger_register_simple(priv->rx_led_trig_name,
-				    &priv->rx_led_trig);
-	led_trigger_register_simple(priv->rxtx_led_trig_name,
-				    &priv->rxtx_led_trig);
+	ledtrig_activity_register(netdev->name, ACTIVITY_LEDS_RX |
+				  ACTIVITY_LEDS_TX | ACTIVITY_LEDS_RXTX,
+				  &priv->activity_led_trig);
 
 	devres_add(&netdev->dev, res);
 }
@@ -105,23 +80,12 @@ static int can_led_notifier(struct notifier_block *nb, unsigned long msg,
 {
 	struct net_device *netdev = netdev_notifier_info_to_dev(ptr);
 	struct can_priv *priv = safe_candev_priv(netdev);
-	char name[CAN_LED_NAME_SZ];
-
-	if (!priv)
-		return NOTIFY_DONE;
 
-	if (!priv->tx_led_trig || !priv->rx_led_trig || !priv->rxtx_led_trig)
+	if (!priv || !priv->activity_led_trig)
 		return NOTIFY_DONE;
 
 	if (msg == NETDEV_CHANGENAME) {
-		snprintf(name, sizeof(name), "%s-tx", netdev->name);
-		led_trigger_rename_static(name, priv->tx_led_trig);
-
-		snprintf(name, sizeof(name), "%s-rx", netdev->name);
-		led_trigger_rename_static(name, priv->rx_led_trig);
-
-		snprintf(name, sizeof(name), "%s-rxtx", netdev->name);
-		led_trigger_rename_static(name, priv->rxtx_led_trig);
+		ledtrig_activity_rename(netdev->name, priv->activity_led_trig);
 	}
 
 	return NOTIFY_DONE;
diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h
index 5261751..f75e7a6 100644
--- a/include/linux/can/dev.h
+++ b/include/linux/can/dev.h
@@ -61,12 +61,7 @@ struct can_priv {
 	struct sk_buff **echo_skb;
 
 #ifdef CONFIG_CAN_LEDS
-	struct led_trigger *tx_led_trig;
-	char tx_led_trig_name[CAN_LED_NAME_SZ];
-	struct led_trigger *rx_led_trig;
-	char rx_led_trig_name[CAN_LED_NAME_SZ];
-	struct led_trigger *rxtx_led_trig;
-	char rxtx_led_trig_name[CAN_LED_NAME_SZ];
+	struct activity_led_trigger *activity_led_trig;
 #endif
 };
 
diff --git a/include/linux/can/led.h b/include/linux/can/led.h
index 2746f7c..96b58c0 100644
--- a/include/linux/can/led.h
+++ b/include/linux/can/led.h
@@ -25,8 +25,6 @@ enum can_led_event {
 /* keep space for interface name + "-tx"/"-rx"/"-rxtx"
  * suffix and null terminator
  */
-#define CAN_LED_NAME_SZ (IFNAMSIZ + 6)
-
 void can_led_event(struct net_device *netdev, enum can_led_event event);
 void devm_can_led_init(struct net_device *netdev);
 int __init can_led_notifier_init(void);
-- 
2.1.4

Re: [RFC PATCH 1/3] leds: trigger: add activity LED trigger

[Jacek Anaszewski]

Hi Tobias,

Thanks for the patch. Please find my comment below.

On 09/06/2016 02:10 PM, Tobias Doerffel wrote:
> The LED activity trigger allows device drivers to indicate device
> activity by blinking LEDs. A typical use case is to signal RX and/or
> TX activity on e.g. CAN busses or serial ports.
>
> This is meant to replace similar code in various device drivers which
> register all their activity LEDs individually.
>
> Signed-off-by: Tobias Doerffel <tobias.doerffel@ed-chemnitz.de>
> ---
>  drivers/leds/trigger/Kconfig            |  10 +++
>  drivers/leds/trigger/Makefile           |   1 +
>  drivers/leds/trigger/ledtrig-activity.c | 147 ++++++++++++++++++++++++++++++++
>  include/linux/leds.h                    |  71 +++++++++++++++
>  4 files changed, 229 insertions(+)
>  create mode 100644 drivers/leds/trigger/ledtrig-activity.c
>
> diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
> index 3f9ddb9..ca69bac 100644
> --- a/drivers/leds/trigger/Kconfig
> +++ b/drivers/leds/trigger/Kconfig
> @@ -33,6 +33,16 @@ config LEDS_TRIGGER_ONESHOT
>
>  	  If unsure, say Y.
>
> +config LEDS_TRIGGER_ACTIVITY
> +	tristate "LED Activity Trigger"
> +	depends on LEDS_TRIGGERS
> +	help
> +	  This allows device drivers to indicate device activity by blinking
> +	  LEDs. A typical use case is to signal RX and/or TX activity on e.g.
> +	  CAN busses or serial ports
> +
> +	  If unsure, say Y.
> +
>  config LEDS_TRIGGER_DISK
>  	bool "LED Disk Trigger"
>  	depends on IDE_GD_ATA || ATA
> diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
> index a72c43c..8431f18 100644
> --- a/drivers/leds/trigger/Makefile
> +++ b/drivers/leds/trigger/Makefile
> @@ -1,5 +1,6 @@
>  obj-$(CONFIG_LEDS_TRIGGER_TIMER)	+= ledtrig-timer.o
>  obj-$(CONFIG_LEDS_TRIGGER_ONESHOT)	+= ledtrig-oneshot.o
> +obj-$(CONFIG_LEDS_TRIGGER_ACTIVITY)	+= ledtrig-activity.o
>  obj-$(CONFIG_LEDS_TRIGGER_DISK)		+= ledtrig-disk.o
>  obj-$(CONFIG_LEDS_TRIGGER_MTD)		+= ledtrig-mtd.o
>  obj-$(CONFIG_LEDS_TRIGGER_HEARTBEAT)	+= ledtrig-heartbeat.o
> diff --git a/drivers/leds/trigger/ledtrig-activity.c b/drivers/leds/trigger/ledtrig-activity.c
> new file mode 100644
> index 0000000..fb71738
> --- /dev/null
> +++ b/drivers/leds/trigger/ledtrig-activity.c
> @@ -0,0 +1,147 @@
> +/*
> + * Activity LED Trigger
> + *
> + * Copyright 2016 EDC Electronic Design Chemnitz GmbH
> + *
> + * Author: Tobias Doerffel <tobias.doerffel@ed-chemnitz.de>
> + *
> + * 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/module.h>
> +#include <linux/slab.h>
> +#include <linux/leds.h>
> +
> +static const char *activity_suffixes[ACTIVITY_LED_COUNT] = {
> +	[ACTIVITY_LED_RX] = "rx",
> +	[ACTIVITY_LED_TX] = "tx",
> +	[ACTIVITY_LED_RXTX] = "rxtx",
> +	[ACTIVITY_LED_RECONNECT] = "recon"
> +};
> +
> +void ledtrig_activity_event(struct activity_led_trigger *trig,
> +			    enum activity_led_event event)
> +{
> +	int i;
> +	int event_leds = 0;
> +	unsigned long led_delay = 0;
> +
> +	switch (event) {
> +	case ACTIVITY_LED_EVENT_START:
> +		for (i = 0; i < ACTIVITY_LED_COUNT; i++) {
> +			if (trig->triggers[i].led_trig) {
> +				led_trigger_event(trig->triggers[i].led_trig,
> +						  LED_FULL);
> +			}
> +		}
> +		break;
> +	case ACTIVITY_LED_EVENT_STOP:
> +		for (i = 0; i < ACTIVITY_LED_COUNT; i++) {
> +			if (trig->triggers[i].led_trig) {
> +				led_trigger_event(trig->triggers[i].led_trig,
> +						  LED_OFF);
> +			}
> +		}
> +		break;
> +	case ACTIVITY_LED_EVENT_RX:
> +		event_leds = (ACTIVITY_LEDS_RX | ACTIVITY_LEDS_RXTX);
> +		break;
> +	case ACTIVITY_LED_EVENT_TX:
> +		event_leds = (ACTIVITY_LEDS_TX | ACTIVITY_LEDS_RXTX);
> +		break;
> +	case ACTIVITY_LED_EVENT_RXTX:
> +		event_leds = ACTIVITY_LEDS_RXTX;
> +		break;
> +	case ACTIVITY_LED_EVENT_RECONNECT:
> +		event_leds = ACTIVITY_LEDS_RECONNECT;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	for (i = 0; i < ACTIVITY_LED_COUNT; i++) {
> +		led_delay = trig->triggers[i].delay;
> +		if (led_delay > 0 &&
> +		    (1 << trig->triggers[i].led) & event_leds) {
> +			led_trigger_blink_oneshot(trig->triggers[i].led_trig,
> +						  &led_delay, &led_delay, 1);
> +		}
> +	}
> +}
> +EXPORT_SYMBOL_GPL(ledtrig_activity_event);
> +
> +void ledtrig_activity_register(const char *name, int leds,
> +			       struct activity_led_trigger **tp)
> +{
> +	int i;
> +	struct activity_led_trigger *trig;
> +
> +	trig = kzalloc(sizeof(struct activity_led_trigger), GFP_KERNEL);
> +
> +	for (i = 0; i < ACTIVITY_LED_COUNT; i++) {
> +		if (leds & (1 << i)) {
> +			trig->triggers[i].led = i;
> +			trig->triggers[i].delay = ACTIVITY_LED_DEFAULT_DELAY;
> +			snprintf(trig->triggers[i].led_trig_name,
> +				 sizeof(trig->triggers[i].led_trig_name),
> +				 "%s-%s", name, activity_suffixes[i]);
> +			led_trigger_register_simple(
> +						trig->triggers[i].led_trig_name,
> +						&trig->triggers[i].led_trig);
> +		}
> +	}

This will result in registering as many triggers of this type, as many
drivers will call this API. It will be in the contrary with the concept
of generic trigger, that all the triggers in drivers/leds/triggers
adhere to. Those modules register only one instance of the trigger and
this creates a layer through which drivers can notify all the LEDs
listening to the given trigger notifications.

This trigger is just not generic enough IMHO.

> +
> +	*tp = trig;
> +}
> +EXPORT_SYMBOL_GPL(ledtrig_activity_register);
> +
> +void ledtrig_activity_unregister(struct activity_led_trigger *trig)
> +{
> +	int i;
> +
> +	for (i = 0; i < ACTIVITY_LED_COUNT; i++) {
> +		if (trig->triggers[i].led_trig) {
> +			led_trigger_unregister_simple(
> +						trig->triggers[i].led_trig);
> +		}
> +	}
> +
> +	kfree(trig);
> +}
> +EXPORT_SYMBOL_GPL(ledtrig_activity_unregister);
> +
> +void ledtrig_activity_rename(const char *name,
> +			     struct activity_led_trigger *trig)
> +{
> +	int i;
> +	char trig_name[TRIG_NAME_MAX];
> +
> +	for (i = 0; i < ACTIVITY_LED_COUNT; i++) {
> +		if (trig->triggers[i].led_trig) {
> +			snprintf(trig_name, sizeof(trig_name),
> +				 "%s-%s", name, activity_suffixes[i]);
> +			led_trigger_rename_static(trig_name,
> +						  trig->triggers[i].led_trig);
> +		}
> +	}
> +}
> +EXPORT_SYMBOL_GPL(ledtrig_activity_rename);
> +
> +void ledtrig_activity_set_delay(struct activity_led_trigger *trig, int leds,
> +				unsigned long delay)
> +{
> +	int i;
> +
> +	for (i = 0; i < ACTIVITY_LED_COUNT; i++) {
> +		if (leds & (1 << i))
> +			trig->triggers[i].delay = delay;
> +	}
> +}
> +EXPORT_SYMBOL_GPL(ledtrig_activity_set_delay);
> +
> +MODULE_AUTHOR("Tobias Doerffel <tobias.doerffel@ed-chemnitz.de>");
> +MODULE_DESCRIPTION("Activity LED trigger");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/leds.h b/include/linux/leds.h
> index 8a3b5d2..8a37f97 100644
> --- a/include/linux/leds.h
> +++ b/include/linux/leds.h
> @@ -414,4 +414,75 @@ static inline void ledtrig_cpu(enum cpu_led_event evt)
>  }
>  #endif
>
> +/*
> + * Activity LED trigger
> + */
> +
> +enum activity_led_event {
> +	ACTIVITY_LED_EVENT_START,
> +	ACTIVITY_LED_EVENT_STOP,
> +	ACTIVITY_LED_EVENT_RX,
> +	ACTIVITY_LED_EVENT_TX,
> +	ACTIVITY_LED_EVENT_RXTX,
> +	ACTIVITY_LED_EVENT_RECONNECT,
> +};
> +
> +enum activity_led {
> +	ACTIVITY_LED_RX,
> +	ACTIVITY_LED_TX,
> +	ACTIVITY_LED_RXTX,
> +	ACTIVITY_LED_RECONNECT,
> +	ACTIVITY_LED_COUNT,
> +};
> +
> +#define	ACTIVITY_LEDS_RX		(1 << ACTIVITY_LED_RX)
> +#define	ACTIVITY_LEDS_TX		(1 << ACTIVITY_LED_TX)
> +#define	ACTIVITY_LEDS_RXTX		(1 << ACTIVITY_LED_RXTX)
> +#define	ACTIVITY_LEDS_RECONNECT	(1 << ACTIVITY_LED_RECONNECT)
> +
> +#define ACTIVITY_LED_DEFAULT_DELAY	50
> +
> +struct activity_led_event_trigger {
> +	struct led_trigger *led_trig;
> +	char led_trig_name[TRIG_NAME_MAX];
> +	int led;
> +	unsigned long delay;
> +};
> +
> +struct activity_led_trigger {
> +	struct activity_led_event_trigger triggers[ACTIVITY_LED_COUNT];
> +};
> +
> +#ifdef CONFIG_LEDS_TRIGGER_ACTIVITY
> +void ledtrig_activity_event(struct activity_led_trigger *trig,
> +			    enum activity_led_event event);
> +void ledtrig_activity_register(const char *name, int leds,
> +			       struct activity_led_trigger **tp);
> +void ledtrig_activity_unregister(struct activity_led_trigger *trig);
> +void ledtrig_activity_rename(const char *name,
> +			     struct activity_led_trigger *trig);
> +void ledtrig_activity_set_delay(struct activity_led_trigger *trig, int leds,
> +				unsigned long delay);
> +#else
> +void ledtrig_activity_event(struct activity_led_trigger *trig,
> +			    enum activity_led_event event)
> +{
> +}
> +void ledtrig_activity_register(const char *name, int leds,
> +			       struct activity_led_trigger **tp)
> +{
> +}
> +void ledtrig_activity_unregister(struct activity_led_trigger *trig)
> +{
> +}
> +void ledtrig_activity_rename(const char *name,
> +			     struct activity_led_trigger *trig)
> +{
> +}
> +void ledtrig_activity_set_delay(struct activity_led_trigger *trig, int leds,
> +				unsigned long delay)
> +{
> +}
> +#endif
> +
>  #endif		/* __LINUX_LEDS_H_INCLUDED */
>


-- 
Best regards,
Jacek Anaszewski