[PATCH] Route kbd leds through the generic leds layer

[PATCH] Route kbd leds through the generic leds layer

[Samuel Thibault]

Route keyboard leds through the generic leds layer.

This permits to reassign keyboard LEDs to something else than keyboard
"leds" state, and also permits to fix #7063 from userland by using a
modifier to implement proper CapsLock behavior and have the keyboard
caps lock led show that caps lock state.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---

Hello,

Here is a second version of the patch and should now be fine for
inclusion e.g. in the mm tree and eventually pushed to 2.6.34. The main
difference with the first version is that now only input leds actually
available on some device are registered.

The first version was acked by Pavel Machek.

Samuel

diff -ur linux-2.6.32-orig/Documentation/leds-class.txt linux-2.6.32-perso/Documentation/leds-class.txt
--- linux-2.6.32-orig/Documentation/leds-class.txt	2009-12-03 13:41:42.000000000 +0100
+++ linux-2.6.32-perso/Documentation/leds-class.txt	2010-02-21 04:12:59.000000000 +0100
@@ -2,9 +2,6 @@
 LED handling under Linux
 ========================
 
-If you're reading this and thinking about keyboard leds, these are
-handled by the input subsystem and the led class is *not* needed.
-
 In its simplest form, the LED class just allows control of LEDs from
 userspace. LEDs appear in /sys/class/leds/. The maximum brightness of the
 LED is defined in max_brightness file. The brightness file will set the brightness
diff -ur linux-2.6.32-orig/drivers/char/keyboard.c linux-2.6.32-perso/drivers/char/keyboard.c
--- linux-2.6.32-orig/drivers/char/keyboard.c	2009-12-03 13:42:46.000000000 +0100
+++ linux-2.6.32-perso/drivers/char/keyboard.c	2010-02-23 20:41:07.000000000 +0100
@@ -34,6 +34,7 @@
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/irq.h>
+#include <linux/leds.h>
 
 #include <linux/kbd_kern.h>
 #include <linux/kbd_diacr.h>
@@ -140,6 +141,9 @@
 static char rep;					/* flag telling character repeat */
 
 static unsigned char ledstate = 0xff;			/* undefined */
+#ifdef CONFIG_LEDS_INPUT
+static unsigned char lockstate = 0xff;			/* undefined */
+#endif
 static unsigned char ledioctl;
 
 static struct ledptr {
@@ -997,6 +1001,23 @@
 	return leds;
 }
 
+#ifdef CONFIG_LEDS_INPUT
+/* When input-based leds are enabled, we route keyboard "leds" through triggers
+ */
+DEFINE_LED_TRIGGER(ledtrig_scrolllock);
+DEFINE_LED_TRIGGER(ledtrig_numlock);
+DEFINE_LED_TRIGGER(ledtrig_capslock);
+DEFINE_LED_TRIGGER(ledtrig_kanalock);
+DEFINE_LED_TRIGGER(ledtrig_shiftlock);
+DEFINE_LED_TRIGGER(ledtrig_altgrlock);
+DEFINE_LED_TRIGGER(ledtrig_ctrllock);
+DEFINE_LED_TRIGGER(ledtrig_altlock);
+DEFINE_LED_TRIGGER(ledtrig_shiftllock);
+DEFINE_LED_TRIGGER(ledtrig_shiftrlock);
+DEFINE_LED_TRIGGER(ledtrig_ctrlllock);
+DEFINE_LED_TRIGGER(ledtrig_ctrlrlock);
+#endif
+
 /*
  * This routine is the bottom half of the keyboard interrupt
  * routine, and runs with all interrupts enabled. It does
@@ -1013,19 +1034,63 @@
 
 static void kbd_bh(unsigned long dummy)
 {
-	struct list_head *node;
 	unsigned char leds = getleds();
 
 	if (leds != ledstate) {
+#ifdef CONFIG_LEDS_INPUT
+		led_trigger_event(ledtrig_scrolllock,
+				leds & (1 << VC_SCROLLOCK) ? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_numlock,
+				leds & (1 << VC_NUMLOCK)   ? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_capslock,
+				leds & (1 << VC_CAPSLOCK)  ? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_kanalock,
+				leds & (1 << VC_KANALOCK)  ? INT_MAX : LED_OFF);
+#else
+		struct list_head *node;
 		list_for_each(node, &kbd_handler.h_list) {
 			struct input_handle *handle = to_handle_h(node);
-			input_inject_event(handle, EV_LED, LED_SCROLLL, !!(leds & 0x01));
-			input_inject_event(handle, EV_LED, LED_NUML,    !!(leds & 0x02));
-			input_inject_event(handle, EV_LED, LED_CAPSL,   !!(leds & 0x04));
+			input_inject_event(handle, EV_LED, LED_SCROLLL,
+					!!(leds & (1 << VC_SCROLLOCK)));
+			input_inject_event(handle, EV_LED, LED_NUML,
+					!!(leds & (1 << VC_NUMLOCK)));
+			input_inject_event(handle, EV_LED, LED_CAPSL,
+					!!(leds & (1 << VC_CAPSLOCK)));
 			input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
 		}
+#endif
 	}
 
+#ifdef CONFIG_LEDS_INPUT
+	if (kbd->lockstate != lockstate) {
+		led_trigger_event(ledtrig_shiftlock,
+			kbd->lockstate & (1<<VC_SHIFTLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_altgrlock,
+			kbd->lockstate & (1<<VC_ALTGRLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_ctrllock,
+			kbd->lockstate & (1<<VC_CTRLLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_altlock,
+			kbd->lockstate & (1<<VC_ALTLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_shiftllock,
+			kbd->lockstate & (1<<VC_SHIFTLLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_shiftrlock,
+			kbd->lockstate & (1<<VC_SHIFTRLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_ctrlllock,
+			kbd->lockstate & (1<<VC_CTRLLLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_ctrlrlock,
+			kbd->lockstate & (1<<VC_CTRLRLOCK)
+				? INT_MAX : LED_OFF);
+	}
+	lockstate = kbd->lockstate;
+#endif
+
 	ledstate = leds;
 }
 
@@ -1357,6 +1422,7 @@
 	kfree(handle);
 }
 
+#ifndef CONFIG_LEDS_INPUT
 /*
  * Start keyboard handler on the new keyboard by refreshing LED state to
  * match the rest of the system.
@@ -1367,13 +1433,17 @@
 
 	tasklet_disable(&keyboard_tasklet);
 	if (leds != 0xff) {
-		input_inject_event(handle, EV_LED, LED_SCROLLL, !!(leds & 0x01));
-		input_inject_event(handle, EV_LED, LED_NUML,    !!(leds & 0x02));
-		input_inject_event(handle, EV_LED, LED_CAPSL,   !!(leds & 0x04));
+		input_inject_event(handle, EV_LED, LED_SCROLLL,
+				!!(leds & (1 << VC_SCROLLOCK)));
+		input_inject_event(handle, EV_LED, LED_NUML,
+				!!(leds & (1 << VC_NUMLOCK)));
+		input_inject_event(handle, EV_LED, LED_CAPSL,
+				!!(leds & (1 << VC_CAPSLOCK)));
 		input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
 	}
 	tasklet_enable(&keyboard_tasklet);
 }
+#endif
 
 static const struct input_device_id kbd_ids[] = {
 	{
@@ -1395,7 +1465,9 @@
 	.event		= kbd_event,
 	.connect	= kbd_connect,
 	.disconnect	= kbd_disconnect,
+#ifndef CONFIG_LEDS_INPUT
 	.start		= kbd_start,
+#endif
 	.name		= "kbd",
 	.id_table	= kbd_ids,
 };
@@ -1419,6 +1491,21 @@
 	if (error)
 		return error;
 
+#ifdef CONFIG_LEDS_INPUT
+	led_trigger_register_simple("scrolllock", &ledtrig_scrolllock);
+	led_trigger_register_simple("numlock", &ledtrig_numlock);
+	led_trigger_register_simple("capslock", &ledtrig_capslock);
+	led_trigger_register_simple("kanalock", &ledtrig_kanalock);
+	led_trigger_register_simple("shiftlock", &ledtrig_shiftlock);
+	led_trigger_register_simple("altgrlock", &ledtrig_altgrlock);
+	led_trigger_register_simple("ctrllock", &ledtrig_ctrllock);
+	led_trigger_register_simple("altlock", &ledtrig_altlock);
+	led_trigger_register_simple("shiftllock", &ledtrig_shiftllock);
+	led_trigger_register_simple("shiftrlock", &ledtrig_shiftrlock);
+	led_trigger_register_simple("ctrlllock", &ledtrig_ctrlllock);
+	led_trigger_register_simple("ctrlrlock", &ledtrig_ctrlrlock);
+#endif
+
 	tasklet_enable(&keyboard_tasklet);
 	tasklet_schedule(&keyboard_tasklet);
 
diff -ur linux-2.6.32-orig/drivers/leds/Kconfig linux-2.6.32-perso/drivers/leds/Kconfig
--- linux-2.6.32-orig/drivers/leds/Kconfig	2009-12-03 13:42:57.000000000 +0100
+++ linux-2.6.32-perso/drivers/leds/Kconfig	2010-02-24 00:52:55.000000000 +0100
@@ -4,9 +4,6 @@
 	  Say Y to enable Linux LED support.  This allows control of supported
 	  LEDs from both userspace and optionally, by kernel events (triggers).
 
-	  This is not related to standard keyboard LEDs which are controlled
-	  via the input system.
-
 if NEW_LEDS
 
 config LEDS_CLASS
@@ -17,6 +14,13 @@
 
 comment "LED drivers"
 
+config LEDS_INPUT
+	tristate "LED Support using input keyboards"
+	depends on LEDS_CLASS
+	help
+	  This option enables support for the LEDs on keyboard managed
+	  by the input layer.
+
 config LEDS_ATMEL_PWM
 	tristate "LED Support using Atmel PWM outputs"
 	depends on LEDS_CLASS && ATMEL_PWM
diff -ur linux-2.6.32-orig/drivers/leds/leds-input.c linux-2.6.32-perso/drivers/leds/leds-input.c
--- linux-2.6.32-orig/drivers/leds/leds-input.c	2010-02-21 04:13:41.000000000 +0100
+++ linux-2.6.32-perso/drivers/leds/leds-input.c	2010-02-24 01:04:09.000000000 +0100
@@ -0,0 +1,186 @@
+/*
+ * LED support for the input layer
+ *
+ * Copyright 2010 Samuel Thibault <samuel.thibault@ens-lyon.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include <linux/input.h>
+
+/* Protects concurrency of led state and registration */
+static DEFINE_SPINLOCK(input_led_lock);
+/* Current state */
+static unsigned long input_led_leds[BITS_TO_LONGS(LED_CNT)];
+/* array of all led classes */
+static struct led_classdev input_leds[LED_CNT];
+/* which led classes are registered */
+static unsigned long input_led_registered[BITS_TO_LONGS(LED_CNT)];
+/* our handler */
+static struct input_handler input_led_handler;
+
+/* Led state change, update all keyboards */
+static void input_led_set(struct led_classdev *cdev,
+			  enum led_brightness brightness)
+{
+	int led = cdev - input_leds;
+	unsigned long flags;
+	struct input_handle *handle;
+
+	spin_lock_irqsave(&input_led_lock, flags);
+	list_for_each_entry(handle, &input_led_handler.h_list, h_node) {
+		input_inject_event(handle, EV_LED, led, !!brightness);
+		input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
+	}
+	if (brightness)
+		set_bit(led, input_led_leds);
+	else
+		clear_bit(led, input_led_leds);
+	spin_unlock_irqrestore(&input_led_lock, flags);
+}
+
+static struct led_classdev input_leds[LED_CNT] = {
+#define DEFINE_INPUT_LED(input_led, nam, deftrig) \
+	[input_led] = { \
+		.name = "input::"nam, \
+		.max_brightness = 1, \
+		.brightness_set = input_led_set, \
+		.default_trigger = deftrig, \
+	}
+DEFINE_INPUT_LED(LED_NUML, "numlock", "numlock"),
+DEFINE_INPUT_LED(LED_CAPSL, "capslock", "capslock"),
+DEFINE_INPUT_LED(LED_SCROLLL, "scrolllock", "scrolllock"),
+DEFINE_INPUT_LED(LED_COMPOSE, "compose", NULL),
+DEFINE_INPUT_LED(LED_KANA, "kana", "kanalock"),
+DEFINE_INPUT_LED(LED_SLEEP, "sleep", NULL),
+DEFINE_INPUT_LED(LED_SUSPEND, "suspend", NULL),
+DEFINE_INPUT_LED(LED_MUTE, "mute", NULL),
+DEFINE_INPUT_LED(LED_MISC, "misc", NULL),
+DEFINE_INPUT_LED(LED_MAIL, "mail", NULL),
+DEFINE_INPUT_LED(LED_CHARGING, "charging", NULL),
+};
+
+static int input_led_connect(struct input_handler *handler,
+			      struct input_dev *dev,
+			      const struct input_device_id *id)
+{
+	struct input_handle *handle;
+	int i, error;
+	unsigned long flags;
+
+	if (!test_bit(EV_LED, dev->keybit))
+		return -ENODEV;
+
+	handle = kzalloc(sizeof(struct input_handle), GFP_KERNEL);
+	if (!handle)
+		return -ENOMEM;
+
+	handle->dev = dev;
+	handle->handler = handler;
+	handle->name = "input leds";
+
+	error = input_register_handle(handle);
+	if (error) {
+		kfree(handle);
+		return error;
+	}
+
+	spin_lock_irqsave(&input_led_lock, flags);
+	for (i = 0; i < LED_CNT; i++)
+		if (input_leds[i].name
+				&& !test_bit(i, input_led_registered)
+				&& test_bit(i, dev->ledbit))
+			/* This keyboard has led i, try to register it */
+			if (!led_classdev_register(NULL, &input_leds[i]))
+				set_bit(i, input_led_registered);
+	spin_unlock_irqrestore(&input_led_lock, flags);
+
+	return 0;
+}
+
+static void input_led_disconnect(struct input_handle *handle)
+{
+	int unregister,i;
+	input_unregister_handle(handle);
+	kfree(handle);
+
+	for (i = 0; i < LED_CNT; i++) {
+		if (!test_bit(i, input_led_registered))
+			continue;
+
+		unregister = 1;
+		list_for_each_entry(handle, &input_led_handler.h_list, h_node) {
+			if (test_bit(i, handle->dev->ledbit)) {
+				unregister = 0;
+				break;
+			}
+		}
+		if (!unregister)
+			continue;
+
+		led_classdev_unregister(&input_leds[i]);
+		clear_bit(i, input_led_registered);
+	}
+}
+
+/* New keyboard, update its leds */
+static void input_led_start(struct input_handle *handle)
+{
+	unsigned long flags;
+	int i;
+
+	spin_lock_irqsave(&input_led_lock, flags);
+	for (i = 0; i < LED_CNT; i++)
+		if (input_leds[i].name && test_bit(i, handle->dev->ledbit))
+			input_inject_event(handle, EV_LED, i,
+					test_bit(i, input_led_leds));
+	input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
+	spin_unlock_irqrestore(&input_led_lock, flags);
+}
+
+static const struct input_device_id input_led_ids[] = {
+	{
+		.flags = INPUT_DEVICE_ID_MATCH_EVBIT,
+		.evbit = { BIT_MASK(EV_LED) },
+	},
+
+	{ },	/* Terminating entry */
+};
+
+static struct input_handler input_led_handler = {
+	.connect	= input_led_connect,
+	.disconnect	= input_led_disconnect,
+	.start		= input_led_start,
+	.name		= "input leds",
+	.id_table	= input_led_ids,
+};
+
+static int __init input_led_init(void)
+{
+	return input_register_handler(&input_led_handler);
+}
+
+static void __exit input_led_exit(void)
+{
+	int i;
+
+	input_unregister_handler(&input_led_handler);
+
+	for (i = 0; i < LED_CNT; i++)
+		if (test_bit(i, input_led_registered)) {
+			led_classdev_unregister(&input_leds[i]);
+			clear_bit(i, input_led_registered);
+		}
+}
+
+module_init(input_led_init);
+module_exit(input_led_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("User LED support for input layer");
+MODULE_AUTHOR("Samuel Thibault <samuel.thibault@ens-lyon.org>");
diff -ur linux-2.6.32-orig/drivers/leds/Makefile linux-2.6.32-perso/drivers/leds/Makefile
--- linux-2.6.32-orig/drivers/leds/Makefile	2009-12-03 13:42:57.000000000 +0100
+++ linux-2.6.32-perso/drivers/leds/Makefile	2010-02-21 03:37:08.000000000 +0100
@@ -5,6 +5,7 @@
 obj-$(CONFIG_LEDS_TRIGGERS)		+= led-triggers.o
 
 # LED Platform Drivers
+obj-$(CONFIG_LEDS_INPUT)		+= leds-input.o
 obj-$(CONFIG_LEDS_ATMEL_PWM)		+= leds-atmel-pwm.o
 obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
 obj-$(CONFIG_LEDS_LOCOMO)		+= leds-locomo.o

[PATCH] Route kbd leds through the generic leds layer (3rd version)

[Samuel Thibault]

Route keyboard leds through the generic leds layer.

This permits to reassign keyboard LEDs to something else than keyboard
"leds" state, and also permits to fix #7063 from userland by using a
modifier to implement proper CapsLock behavior and have the keyboard
caps lock led show that caps lock state.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---

Hello,

Here is a third version of the patch and should now be fine for
inclusion e.g. in the mm tree and eventually pushed to 2.6.34. The main
difference with the first version is that now only input leds actually
available on some device are registered. The difference with the second
version is a refresh against 2.6.33 and a locking fix.

The first version was acked by Pavel Machek.

Samuel

diff -ur linux-2.6.33-orig/Documentation/leds-class.txt linux-2.6.33-perso/Documentation/leds-class.txt
--- linux-2.6.33-orig/Documentation/leds-class.txt	2009-12-03 13:41:42.000000000 +0100
+++ linux-2.6.33-perso/Documentation/leds-class.txt	2010-02-25 01:45:28.000000000 +0100
@@ -2,9 +2,6 @@
 LED handling under Linux
 ========================
 
-If you're reading this and thinking about keyboard leds, these are
-handled by the input subsystem and the led class is *not* needed.
-
 In its simplest form, the LED class just allows control of LEDs from
 userspace. LEDs appear in /sys/class/leds/. The maximum brightness of the
 LED is defined in max_brightness file. The brightness file will set the brightness
diff -ur linux-2.6.33-orig/drivers/char/keyboard.c linux-2.6.33-perso/drivers/char/keyboard.c
--- linux-2.6.33-orig/drivers/char/keyboard.c	2010-02-25 01:41:19.000000000 +0100
+++ linux-2.6.33-perso/drivers/char/keyboard.c	2010-02-25 01:52:11.000000000 +0100
@@ -34,6 +34,7 @@
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/irq.h>
+#include <linux/leds.h>
 
 #include <linux/kbd_kern.h>
 #include <linux/kbd_diacr.h>
@@ -139,6 +140,9 @@
 static char rep;					/* flag telling character repeat */
 
 static unsigned char ledstate = 0xff;			/* undefined */
+#ifdef CONFIG_LEDS_INPUT
+static unsigned char lockstate = 0xff;			/* undefined */
+#endif
 static unsigned char ledioctl;
 
 static struct ledptr {
@@ -971,6 +975,23 @@
 	}
 }
 
+#ifdef CONFIG_LEDS_INPUT
+/* When input-based leds are enabled, we route keyboard "leds" through triggers
+ */
+DEFINE_LED_TRIGGER(ledtrig_scrolllock);
+DEFINE_LED_TRIGGER(ledtrig_numlock);
+DEFINE_LED_TRIGGER(ledtrig_capslock);
+DEFINE_LED_TRIGGER(ledtrig_kanalock);
+DEFINE_LED_TRIGGER(ledtrig_shiftlock);
+DEFINE_LED_TRIGGER(ledtrig_altgrlock);
+DEFINE_LED_TRIGGER(ledtrig_ctrllock);
+DEFINE_LED_TRIGGER(ledtrig_altlock);
+DEFINE_LED_TRIGGER(ledtrig_shiftllock);
+DEFINE_LED_TRIGGER(ledtrig_shiftrlock);
+DEFINE_LED_TRIGGER(ledtrig_ctrlllock);
+DEFINE_LED_TRIGGER(ledtrig_ctrlrlock);
+#endif
+
 /*
  * The leds display either (i) the status of NumLock, CapsLock, ScrollLock,
  * or (ii) whatever pattern of lights people want to show using KDSETLED,
@@ -1019,9 +1040,12 @@
 	unsigned char leds = *(unsigned char *)data;
 
 	if (test_bit(EV_LED, handle->dev->evbit)) {
-		input_inject_event(handle, EV_LED, LED_SCROLLL, !!(leds & 0x01));
-		input_inject_event(handle, EV_LED, LED_NUML,    !!(leds & 0x02));
-		input_inject_event(handle, EV_LED, LED_CAPSL,   !!(leds & 0x04));
+		input_inject_event(handle, EV_LED, LED_SCROLLL,
+				!!(leds & VC_SCROLLOCK));
+		input_inject_event(handle, EV_LED, LED_NUML,
+				!!(leds & VC_NUMLOCK));
+		input_inject_event(handle, EV_LED, LED_CAPSL,
+				!!(leds & VC_CAPSLOCK));
 		input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
 	}
 
@@ -1040,10 +1064,51 @@
 	unsigned char leds = getleds();
 
 	if (leds != ledstate) {
+#ifdef CONFIG_LEDS_INPUT
+		led_trigger_event(ledtrig_scrolllock,
+				leds & (1 << VC_SCROLLOCK) ? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_numlock,
+				leds & (1 << VC_NUMLOCK)   ? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_capslock,
+				leds & (1 << VC_CAPSLOCK)  ? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_kanalock,
+				leds & (1 << VC_KANALOCK)  ? INT_MAX : LED_OFF);
+#else
 		input_handler_for_each_handle(&kbd_handler, &leds,
 					      kbd_update_leds_helper);
+#endif
 		ledstate = leds;
 	}
+
+#ifdef CONFIG_LEDS_INPUT
+	if (kbd->lockstate != lockstate) {
+		led_trigger_event(ledtrig_shiftlock,
+			kbd->lockstate & (1<<VC_SHIFTLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_altgrlock,
+			kbd->lockstate & (1<<VC_ALTGRLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_ctrllock,
+			kbd->lockstate & (1<<VC_CTRLLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_altlock,
+			kbd->lockstate & (1<<VC_ALTLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_shiftllock,
+			kbd->lockstate & (1<<VC_SHIFTLLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_shiftrlock,
+			kbd->lockstate & (1<<VC_SHIFTRLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_ctrlllock,
+			kbd->lockstate & (1<<VC_CTRLLLOCK)
+				? INT_MAX : LED_OFF);
+		led_trigger_event(ledtrig_ctrlrlock,
+			kbd->lockstate & (1<<VC_CTRLRLOCK)
+				? INT_MAX : LED_OFF);
+		lockstate = kbd->lockstate;
+	}
+#endif
 }
 
 DECLARE_TASKLET_DISABLED(keyboard_tasklet, kbd_bh, 0);
@@ -1380,6 +1445,7 @@
 	kfree(handle);
 }
 
+#ifndef CONFIG_LEDS_INPUT
 /*
  * Start keyboard handler on the new keyboard by refreshing LED state to
  * match the rest of the system.
@@ -1393,6 +1459,7 @@
 
 	tasklet_enable(&keyboard_tasklet);
 }
+#endif
 
 static const struct input_device_id kbd_ids[] = {
 	{
@@ -1414,7 +1481,9 @@
 	.event		= kbd_event,
 	.connect	= kbd_connect,
 	.disconnect	= kbd_disconnect,
+#ifndef CONFIG_LEDS_INPUT
 	.start		= kbd_start,
+#endif
 	.name		= "kbd",
 	.id_table	= kbd_ids,
 };
@@ -1438,6 +1507,21 @@
 	if (error)
 		return error;
 
+#ifdef CONFIG_LEDS_INPUT
+	led_trigger_register_simple("scrolllock", &ledtrig_scrolllock);
+	led_trigger_register_simple("numlock", &ledtrig_numlock);
+	led_trigger_register_simple("capslock", &ledtrig_capslock);
+	led_trigger_register_simple("kanalock", &ledtrig_kanalock);
+	led_trigger_register_simple("shiftlock", &ledtrig_shiftlock);
+	led_trigger_register_simple("altgrlock", &ledtrig_altgrlock);
+	led_trigger_register_simple("ctrllock", &ledtrig_ctrllock);
+	led_trigger_register_simple("altlock", &ledtrig_altlock);
+	led_trigger_register_simple("shiftllock", &ledtrig_shiftllock);
+	led_trigger_register_simple("shiftrlock", &ledtrig_shiftrlock);
+	led_trigger_register_simple("ctrlllock", &ledtrig_ctrlllock);
+	led_trigger_register_simple("ctrlrlock", &ledtrig_ctrlrlock);
+#endif
+
 	tasklet_enable(&keyboard_tasklet);
 	tasklet_schedule(&keyboard_tasklet);
 
diff -ur linux-2.6.33-orig/drivers/leds/Kconfig linux-2.6.33-perso/drivers/leds/Kconfig
--- linux-2.6.33-orig/drivers/leds/Kconfig	2010-02-25 01:41:27.000000000 +0100
+++ linux-2.6.33-perso/drivers/leds/Kconfig	2010-02-25 01:45:28.000000000 +0100
@@ -4,9 +4,6 @@
 	  Say Y to enable Linux LED support.  This allows control of supported
 	  LEDs from both userspace and optionally, by kernel events (triggers).
 
-	  This is not related to standard keyboard LEDs which are controlled
-	  via the input system.
-
 if NEW_LEDS
 
 config LEDS_CLASS
@@ -17,6 +14,13 @@
 
 comment "LED drivers"
 
+config LEDS_INPUT
+	tristate "LED Support using input keyboards"
+	depends on LEDS_CLASS
+	help
+	  This option enables support for the LEDs on keyboard managed
+	  by the input layer.
+
 config LEDS_ATMEL_PWM
 	tristate "LED Support using Atmel PWM outputs"
 	depends on LEDS_CLASS && ATMEL_PWM
diff -ur linux-2.6.33-orig/drivers/leds/leds-input.c linux-2.6.33-perso/drivers/leds/leds-input.c
--- linux-2.6.33-orig/drivers/leds/leds-input.c	2010-02-21 04:13:41.000000000 +0100
+++ linux-2.6.33-perso/drivers/leds/leds-input.c	2010-02-25 01:45:28.000000000 +0100
@@ -0,0 +1,195 @@
+/*
+ * LED support for the input layer
+ *
+ * Copyright 2010 Samuel Thibault <samuel.thibault@ens-lyon.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include <linux/input.h>
+
+/* Protects concurrency of led state */
+static DEFINE_SPINLOCK(input_leds_lock);
+/* Current state */
+static unsigned long input_led_leds[BITS_TO_LONGS(LED_CNT)];
+/* array of all led classes */
+static struct led_classdev input_leds[LED_CNT];
+/* Protects concurrency of led registration */
+static DEFINE_SPINLOCK(input_led_registered_lock);
+/* which led classes are registered */
+static unsigned long input_led_registered[BITS_TO_LONGS(LED_CNT)];
+/* our handler */
+static struct input_handler input_led_handler;
+
+/* Led state change, update all keyboards */
+static void input_led_set(struct led_classdev *cdev,
+			  enum led_brightness brightness)
+{
+	int led = cdev - input_leds;
+	unsigned long flags;
+	struct input_handle *handle;
+
+	spin_lock_irqsave(&input_leds_lock, flags);
+	list_for_each_entry(handle, &input_led_handler.h_list, h_node) {
+		input_inject_event(handle, EV_LED, led, !!brightness);
+		input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
+	}
+	if (brightness)
+		set_bit(led, input_led_leds);
+	else
+		clear_bit(led, input_led_leds);
+	spin_unlock_irqrestore(&input_leds_lock, flags);
+}
+
+static struct led_classdev input_leds[LED_CNT] = {
+#define DEFINE_INPUT_LED(input_led, nam, deftrig) \
+	[input_led] = { \
+		.name = "input::"nam, \
+		.max_brightness = 1, \
+		.brightness_set = input_led_set, \
+		.default_trigger = deftrig, \
+	}
+DEFINE_INPUT_LED(LED_NUML, "numlock", "numlock"),
+DEFINE_INPUT_LED(LED_CAPSL, "capslock", "capslock"),
+DEFINE_INPUT_LED(LED_SCROLLL, "scrolllock", "scrolllock"),
+DEFINE_INPUT_LED(LED_COMPOSE, "compose", NULL),
+DEFINE_INPUT_LED(LED_KANA, "kana", "kanalock"),
+DEFINE_INPUT_LED(LED_SLEEP, "sleep", NULL),
+DEFINE_INPUT_LED(LED_SUSPEND, "suspend", NULL),
+DEFINE_INPUT_LED(LED_MUTE, "mute", NULL),
+DEFINE_INPUT_LED(LED_MISC, "misc", NULL),
+DEFINE_INPUT_LED(LED_MAIL, "mail", NULL),
+DEFINE_INPUT_LED(LED_CHARGING, "charging", NULL),
+};
+
+static int input_led_connect(struct input_handler *handler,
+			      struct input_dev *dev,
+			      const struct input_device_id *id)
+{
+	struct input_handle *handle;
+	int i, error;
+	unsigned long flags;
+
+	if (!test_bit(EV_LED, dev->keybit))
+		return -ENODEV;
+
+	handle = kzalloc(sizeof(struct input_handle), GFP_KERNEL);
+	if (!handle)
+		return -ENOMEM;
+
+	handle->dev = dev;
+	handle->handler = handler;
+	handle->name = "input leds";
+
+	error = input_register_handle(handle);
+	if (error) {
+		kfree(handle);
+		return error;
+	}
+
+	spin_lock_irqsave(&input_led_registered_lock, flags);
+	for (i = 0; i < LED_CNT; i++)
+		if (input_leds[i].name
+				&& !test_bit(i, input_led_registered)
+				&& test_bit(i, dev->ledbit))
+			/* This keyboard has led i, try to register it */
+			if (!led_classdev_register(NULL, &input_leds[i]))
+				set_bit(i, input_led_registered);
+	spin_unlock_irqrestore(&input_led_registered_lock, flags);
+
+	return 0;
+}
+
+static void input_led_disconnect(struct input_handle *handle)
+{
+	int unregister, i;
+	unsigned long flags;
+
+	input_unregister_handle(handle);
+	kfree(handle);
+
+	spin_lock_irqsave(&input_led_registered_lock, flags);
+	for (i = 0; i < LED_CNT; i++) {
+		if (!test_bit(i, input_led_registered))
+			continue;
+
+		unregister = 1;
+		list_for_each_entry(handle, &input_led_handler.h_list, h_node) {
+			if (test_bit(i, handle->dev->ledbit)) {
+				unregister = 0;
+				break;
+			}
+		}
+		if (!unregister)
+			continue;
+
+		led_classdev_unregister(&input_leds[i]);
+		clear_bit(i, input_led_registered);
+	}
+	spin_unlock_irqrestore(&input_led_registered_lock, flags);
+}
+
+/* New keyboard, update its leds */
+static void input_led_start(struct input_handle *handle)
+{
+	unsigned long flags;
+	int i;
+
+	spin_lock_irqsave(&input_leds_lock, flags);
+	for (i = 0; i < LED_CNT; i++)
+		if (input_leds[i].name && test_bit(i, handle->dev->ledbit))
+			input_inject_event(handle, EV_LED, i,
+					test_bit(i, input_led_leds));
+	input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
+	spin_unlock_irqrestore(&input_leds_lock, flags);
+}
+
+static const struct input_device_id input_led_ids[] = {
+	{
+		.flags = INPUT_DEVICE_ID_MATCH_EVBIT,
+		.evbit = { BIT_MASK(EV_LED) },
+	},
+
+	{ },	/* Terminating entry */
+};
+
+static struct input_handler input_led_handler = {
+	.connect	= input_led_connect,
+	.disconnect	= input_led_disconnect,
+	.start		= input_led_start,
+	.name		= "input leds",
+	.id_table	= input_led_ids,
+};
+
+static int __init input_led_init(void)
+{
+	return input_register_handler(&input_led_handler);
+}
+
+static void __exit input_led_exit(void)
+{
+	int i;
+	unsigned long flags;
+
+	input_unregister_handler(&input_led_handler);
+
+	spin_lock_irqsave(&input_leds_lock, flags);
+	for (i = 0; i < LED_CNT; i++)
+		if (test_bit(i, input_led_registered)) {
+			led_classdev_unregister(&input_leds[i]);
+			clear_bit(i, input_led_registered);
+		}
+	spin_unlock_irqrestore(&input_leds_lock, flags);
+}
+
+module_init(input_led_init);
+module_exit(input_led_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("User LED support for input layer");
+MODULE_AUTHOR("Samuel Thibault <samuel.thibault@ens-lyon.org>");
diff -ur linux-2.6.33-orig/drivers/leds/Makefile linux-2.6.33-perso/drivers/leds/Makefile
--- linux-2.6.33-orig/drivers/leds/Makefile	2010-02-25 01:41:27.000000000 +0100
+++ linux-2.6.33-perso/drivers/leds/Makefile	2010-02-25 01:45:28.000000000 +0100
@@ -5,6 +5,7 @@
 obj-$(CONFIG_LEDS_TRIGGERS)		+= led-triggers.o
 
 # LED Platform Drivers
+obj-$(CONFIG_LEDS_INPUT)		+= leds-input.o
 obj-$(CONFIG_LEDS_ATMEL_PWM)		+= leds-atmel-pwm.o
 obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
 obj-$(CONFIG_LEDS_LOCOMO)		+= leds-locomo.o

Re: [PATCH] Route kbd leds through the generic leds layer (3rd version)

[Dmitry Torokhov]

On Thu, Feb 25, 2010 at 02:38:40AM +0100, Samuel Thibault wrote:
> Route keyboard leds through the generic leds layer.
> 
> This permits to reassign keyboard LEDs to something else than keyboard
> "leds" state, and also permits to fix #7063 from userland by using a
> modifier to implement proper CapsLock behavior and have the keyboard
> caps lock led show that caps lock state.
> 

I am aunsure about this patch. It ties all LEDs together whereas
currently it is possible to operate keyboards and their leds
independently (as long as legacy keyboard driver is out of picture, i.e.
in X or otherwise when we have console in raw mode).

> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> 
> Hello,
> 
> Here is a third version of the patch and should now be fine for
> inclusion e.g. in the mm tree and eventually pushed to 2.6.34. The main
> difference with the first version is that now only input leds actually
> available on some device are registered. The difference with the second
> version is a refresh against 2.6.33 and a locking fix.
> 
> The first version was acked by Pavel Machek.
> 
> Samuel
> 
> diff -ur linux-2.6.33-orig/Documentation/leds-class.txt linux-2.6.33-perso/Documentation/leds-class.txt
> --- linux-2.6.33-orig/Documentation/leds-class.txt	2009-12-03 13:41:42.000000000 +0100
> +++ linux-2.6.33-perso/Documentation/leds-class.txt	2010-02-25 01:45:28.000000000 +0100
> @@ -2,9 +2,6 @@
>  LED handling under Linux
>  ========================
>  
> -If you're reading this and thinking about keyboard leds, these are
> -handled by the input subsystem and the led class is *not* needed.
> -
>  In its simplest form, the LED class just allows control of LEDs from
>  userspace. LEDs appear in /sys/class/leds/. The maximum brightness of the
>  LED is defined in max_brightness file. The brightness file will set the brightness
> diff -ur linux-2.6.33-orig/drivers/char/keyboard.c linux-2.6.33-perso/drivers/char/keyboard.c
> --- linux-2.6.33-orig/drivers/char/keyboard.c	2010-02-25 01:41:19.000000000 +0100
> +++ linux-2.6.33-perso/drivers/char/keyboard.c	2010-02-25 01:52:11.000000000 +0100
> @@ -34,6 +34,7 @@
>  #include <linux/init.h>
>  #include <linux/slab.h>
>  #include <linux/irq.h>
> +#include <linux/leds.h>
>  
>  #include <linux/kbd_kern.h>
>  #include <linux/kbd_diacr.h>
> @@ -139,6 +140,9 @@
>  static char rep;					/* flag telling character repeat */
>  
>  static unsigned char ledstate = 0xff;			/* undefined */
> +#ifdef CONFIG_LEDS_INPUT
> +static unsigned char lockstate = 0xff;			/* undefined */
> +#endif
>  static unsigned char ledioctl;
>  
>  static struct ledptr {
> @@ -971,6 +975,23 @@
>  	}
>  }
>  
> +#ifdef CONFIG_LEDS_INPUT
> +/* When input-based leds are enabled, we route keyboard "leds" through triggers
> + */
> +DEFINE_LED_TRIGGER(ledtrig_scrolllock);
> +DEFINE_LED_TRIGGER(ledtrig_numlock);
> +DEFINE_LED_TRIGGER(ledtrig_capslock);
> +DEFINE_LED_TRIGGER(ledtrig_kanalock);
> +DEFINE_LED_TRIGGER(ledtrig_shiftlock);
> +DEFINE_LED_TRIGGER(ledtrig_altgrlock);
> +DEFINE_LED_TRIGGER(ledtrig_ctrllock);
> +DEFINE_LED_TRIGGER(ledtrig_altlock);
> +DEFINE_LED_TRIGGER(ledtrig_shiftllock);
> +DEFINE_LED_TRIGGER(ledtrig_shiftrlock);
> +DEFINE_LED_TRIGGER(ledtrig_ctrlllock);
> +DEFINE_LED_TRIGGER(ledtrig_ctrlrlock);
> +#endif
> +
>  /*
>   * The leds display either (i) the status of NumLock, CapsLock, ScrollLock,
>   * or (ii) whatever pattern of lights people want to show using KDSETLED,
> @@ -1019,9 +1040,12 @@
>  	unsigned char leds = *(unsigned char *)data;
>  
>  	if (test_bit(EV_LED, handle->dev->evbit)) {
> -		input_inject_event(handle, EV_LED, LED_SCROLLL, !!(leds & 0x01));
> -		input_inject_event(handle, EV_LED, LED_NUML,    !!(leds & 0x02));
> -		input_inject_event(handle, EV_LED, LED_CAPSL,   !!(leds & 0x04));
> +		input_inject_event(handle, EV_LED, LED_SCROLLL,
> +				!!(leds & VC_SCROLLOCK));
> +		input_inject_event(handle, EV_LED, LED_NUML,
> +				!!(leds & VC_NUMLOCK));
> +		input_inject_event(handle, EV_LED, LED_CAPSL,
> +				!!(leds & VC_CAPSLOCK));
>  		input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
>  	}
>  
> @@ -1040,10 +1064,51 @@
>  	unsigned char leds = getleds();
>  
>  	if (leds != ledstate) {
> +#ifdef CONFIG_LEDS_INPUT
> +		led_trigger_event(ledtrig_scrolllock,
> +				leds & (1 << VC_SCROLLOCK) ? INT_MAX : LED_OFF);
> +		led_trigger_event(ledtrig_numlock,
> +				leds & (1 << VC_NUMLOCK)   ? INT_MAX : LED_OFF);
> +		led_trigger_event(ledtrig_capslock,
> +				leds & (1 << VC_CAPSLOCK)  ? INT_MAX : LED_OFF);
> +		led_trigger_event(ledtrig_kanalock,
> +				leds & (1 << VC_KANALOCK)  ? INT_MAX : LED_OFF);
> +#else
>  		input_handler_for_each_handle(&kbd_handler, &leds,
>  					      kbd_update_leds_helper);
> +#endif
>  		ledstate = leds;
>  	}
> +
> +#ifdef CONFIG_LEDS_INPUT
> +	if (kbd->lockstate != lockstate) {
> +		led_trigger_event(ledtrig_shiftlock,
> +			kbd->lockstate & (1<<VC_SHIFTLOCK)
> +				? INT_MAX : LED_OFF);
> +		led_trigger_event(ledtrig_altgrlock,
> +			kbd->lockstate & (1<<VC_ALTGRLOCK)
> +				? INT_MAX : LED_OFF);
> +		led_trigger_event(ledtrig_ctrllock,
> +			kbd->lockstate & (1<<VC_CTRLLOCK)
> +				? INT_MAX : LED_OFF);
> +		led_trigger_event(ledtrig_altlock,
> +			kbd->lockstate & (1<<VC_ALTLOCK)
> +				? INT_MAX : LED_OFF);
> +		led_trigger_event(ledtrig_shiftllock,
> +			kbd->lockstate & (1<<VC_SHIFTLLOCK)
> +				? INT_MAX : LED_OFF);
> +		led_trigger_event(ledtrig_shiftrlock,
> +			kbd->lockstate & (1<<VC_SHIFTRLOCK)
> +				? INT_MAX : LED_OFF);
> +		led_trigger_event(ledtrig_ctrlllock,
> +			kbd->lockstate & (1<<VC_CTRLLLOCK)
> +				? INT_MAX : LED_OFF);
> +		led_trigger_event(ledtrig_ctrlrlock,
> +			kbd->lockstate & (1<<VC_CTRLRLOCK)
> +				? INT_MAX : LED_OFF);
> +		lockstate = kbd->lockstate;
> +	}
> +#endif
>  }
>  
>  DECLARE_TASKLET_DISABLED(keyboard_tasklet, kbd_bh, 0);
> @@ -1380,6 +1445,7 @@
>  	kfree(handle);
>  }
>  
> +#ifndef CONFIG_LEDS_INPUT
>  /*
>   * Start keyboard handler on the new keyboard by refreshing LED state to
>   * match the rest of the system.
> @@ -1393,6 +1459,7 @@
>  
>  	tasklet_enable(&keyboard_tasklet);
>  }
> +#endif
>  
>  static const struct input_device_id kbd_ids[] = {
>  	{
> @@ -1414,7 +1481,9 @@
>  	.event		= kbd_event,
>  	.connect	= kbd_connect,
>  	.disconnect	= kbd_disconnect,
> +#ifndef CONFIG_LEDS_INPUT
>  	.start		= kbd_start,
> +#endif
>  	.name		= "kbd",
>  	.id_table	= kbd_ids,
>  };
> @@ -1438,6 +1507,21 @@
>  	if (error)
>  		return error;
>  
> +#ifdef CONFIG_LEDS_INPUT
> +	led_trigger_register_simple("scrolllock", &ledtrig_scrolllock);
> +	led_trigger_register_simple("numlock", &ledtrig_numlock);
> +	led_trigger_register_simple("capslock", &ledtrig_capslock);
> +	led_trigger_register_simple("kanalock", &ledtrig_kanalock);
> +	led_trigger_register_simple("shiftlock", &ledtrig_shiftlock);
> +	led_trigger_register_simple("altgrlock", &ledtrig_altgrlock);
> +	led_trigger_register_simple("ctrllock", &ledtrig_ctrllock);
> +	led_trigger_register_simple("altlock", &ledtrig_altlock);
> +	led_trigger_register_simple("shiftllock", &ledtrig_shiftllock);
> +	led_trigger_register_simple("shiftrlock", &ledtrig_shiftrlock);
> +	led_trigger_register_simple("ctrlllock", &ledtrig_ctrlllock);
> +	led_trigger_register_simple("ctrlrlock", &ledtrig_ctrlrlock);
> +#endif
> +
>  	tasklet_enable(&keyboard_tasklet);
>  	tasklet_schedule(&keyboard_tasklet);
>  
> diff -ur linux-2.6.33-orig/drivers/leds/Kconfig linux-2.6.33-perso/drivers/leds/Kconfig
> --- linux-2.6.33-orig/drivers/leds/Kconfig	2010-02-25 01:41:27.000000000 +0100
> +++ linux-2.6.33-perso/drivers/leds/Kconfig	2010-02-25 01:45:28.000000000 +0100
> @@ -4,9 +4,6 @@
>  	  Say Y to enable Linux LED support.  This allows control of supported
>  	  LEDs from both userspace and optionally, by kernel events (triggers).
>  
> -	  This is not related to standard keyboard LEDs which are controlled
> -	  via the input system.
> -
>  if NEW_LEDS
>  
>  config LEDS_CLASS
> @@ -17,6 +14,13 @@
>  
>  comment "LED drivers"
>  
> +config LEDS_INPUT
> +	tristate "LED Support using input keyboards"
> +	depends on LEDS_CLASS
> +	help
> +	  This option enables support for the LEDs on keyboard managed
> +	  by the input layer.
> +
>  config LEDS_ATMEL_PWM
>  	tristate "LED Support using Atmel PWM outputs"
>  	depends on LEDS_CLASS && ATMEL_PWM
> diff -ur linux-2.6.33-orig/drivers/leds/leds-input.c linux-2.6.33-perso/drivers/leds/leds-input.c
> --- linux-2.6.33-orig/drivers/leds/leds-input.c	2010-02-21 04:13:41.000000000 +0100
> +++ linux-2.6.33-perso/drivers/leds/leds-input.c	2010-02-25 01:45:28.000000000 +0100
> @@ -0,0 +1,195 @@
> +/*
> + * LED support for the input layer
> + *
> + * Copyright 2010 Samuel Thibault <samuel.thibault@ens-lyon.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/leds.h>
> +#include <linux/input.h>
> +
> +/* Protects concurrency of led state */
> +static DEFINE_SPINLOCK(input_leds_lock);
> +/* Current state */
> +static unsigned long input_led_leds[BITS_TO_LONGS(LED_CNT)];
> +/* array of all led classes */
> +static struct led_classdev input_leds[LED_CNT];
> +/* Protects concurrency of led registration */
> +static DEFINE_SPINLOCK(input_led_registered_lock);
> +/* which led classes are registered */
> +static unsigned long input_led_registered[BITS_TO_LONGS(LED_CNT)];
> +/* our handler */
> +static struct input_handler input_led_handler;
> +
> +/* Led state change, update all keyboards */
> +static void input_led_set(struct led_classdev *cdev,
> +			  enum led_brightness brightness)
> +{
> +	int led = cdev - input_leds;
> +	unsigned long flags;
> +	struct input_handle *handle;
> +
> +	spin_lock_irqsave(&input_leds_lock, flags);
> +	list_for_each_entry(handle, &input_led_handler.h_list, h_node) {
> +		input_inject_event(handle, EV_LED, led, !!brightness);
> +		input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
> +	}
> +	if (brightness)
> +		set_bit(led, input_led_leds);
> +	else
> +		clear_bit(led, input_led_leds);
> +	spin_unlock_irqrestore(&input_leds_lock, flags);
> +}
> +
> +static struct led_classdev input_leds[LED_CNT] = {
> +#define DEFINE_INPUT_LED(input_led, nam, deftrig) \
> +	[input_led] = { \
> +		.name = "input::"nam, \
> +		.max_brightness = 1, \
> +		.brightness_set = input_led_set, \
> +		.default_trigger = deftrig, \
> +	}
> +DEFINE_INPUT_LED(LED_NUML, "numlock", "numlock"),
> +DEFINE_INPUT_LED(LED_CAPSL, "capslock", "capslock"),
> +DEFINE_INPUT_LED(LED_SCROLLL, "scrolllock", "scrolllock"),
> +DEFINE_INPUT_LED(LED_COMPOSE, "compose", NULL),
> +DEFINE_INPUT_LED(LED_KANA, "kana", "kanalock"),
> +DEFINE_INPUT_LED(LED_SLEEP, "sleep", NULL),
> +DEFINE_INPUT_LED(LED_SUSPEND, "suspend", NULL),
> +DEFINE_INPUT_LED(LED_MUTE, "mute", NULL),
> +DEFINE_INPUT_LED(LED_MISC, "misc", NULL),
> +DEFINE_INPUT_LED(LED_MAIL, "mail", NULL),
> +DEFINE_INPUT_LED(LED_CHARGING, "charging", NULL),
> +};
> +
> +static int input_led_connect(struct input_handler *handler,
> +			      struct input_dev *dev,
> +			      const struct input_device_id *id)
> +{
> +	struct input_handle *handle;
> +	int i, error;
> +	unsigned long flags;
> +
> +	if (!test_bit(EV_LED, dev->keybit))
> +		return -ENODEV;
> +
> +	handle = kzalloc(sizeof(struct input_handle), GFP_KERNEL);
> +	if (!handle)
> +		return -ENOMEM;
> +
> +	handle->dev = dev;
> +	handle->handler = handler;
> +	handle->name = "input leds";
> +
> +	error = input_register_handle(handle);
> +	if (error) {
> +		kfree(handle);
> +		return error;
> +	}
> +
> +	spin_lock_irqsave(&input_led_registered_lock, flags);
> +	for (i = 0; i < LED_CNT; i++)
> +		if (input_leds[i].name
> +				&& !test_bit(i, input_led_registered)
> +				&& test_bit(i, dev->ledbit))
> +			/* This keyboard has led i, try to register it */
> +			if (!led_classdev_register(NULL, &input_leds[i]))
> +				set_bit(i, input_led_registered);
> +	spin_unlock_irqrestore(&input_led_registered_lock, flags);
> +
> +	return 0;
> +}
> +
> +static void input_led_disconnect(struct input_handle *handle)
> +{
> +	int unregister, i;
> +	unsigned long flags;
> +
> +	input_unregister_handle(handle);
> +	kfree(handle);
> +
> +	spin_lock_irqsave(&input_led_registered_lock, flags);
> +	for (i = 0; i < LED_CNT; i++) {
> +		if (!test_bit(i, input_led_registered))
> +			continue;
> +
> +		unregister = 1;
> +		list_for_each_entry(handle, &input_led_handler.h_list, h_node) {
> +			if (test_bit(i, handle->dev->ledbit)) {
> +				unregister = 0;
> +				break;
> +			}
> +		}
> +		if (!unregister)
> +			continue;
> +
> +		led_classdev_unregister(&input_leds[i]);
> +		clear_bit(i, input_led_registered);
> +	}
> +	spin_unlock_irqrestore(&input_led_registered_lock, flags);
> +}
> +
> +/* New keyboard, update its leds */
> +static void input_led_start(struct input_handle *handle)
> +{
> +	unsigned long flags;
> +	int i;
> +
> +	spin_lock_irqsave(&input_leds_lock, flags);
> +	for (i = 0; i < LED_CNT; i++)
> +		if (input_leds[i].name && test_bit(i, handle->dev->ledbit))
> +			input_inject_event(handle, EV_LED, i,
> +					test_bit(i, input_led_leds));
> +	input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
> +	spin_unlock_irqrestore(&input_leds_lock, flags);
> +}
> +
> +static const struct input_device_id input_led_ids[] = {
> +	{
> +		.flags = INPUT_DEVICE_ID_MATCH_EVBIT,
> +		.evbit = { BIT_MASK(EV_LED) },
> +	},
> +
> +	{ },	/* Terminating entry */
> +};
> +
> +static struct input_handler input_led_handler = {
> +	.connect	= input_led_connect,
> +	.disconnect	= input_led_disconnect,
> +	.start		= input_led_start,
> +	.name		= "input leds",
> +	.id_table	= input_led_ids,
> +};
> +
> +static int __init input_led_init(void)
> +{
> +	return input_register_handler(&input_led_handler);
> +}
> +
> +static void __exit input_led_exit(void)
> +{
> +	int i;
> +	unsigned long flags;
> +
> +	input_unregister_handler(&input_led_handler);
> +
> +	spin_lock_irqsave(&input_leds_lock, flags);
> +	for (i = 0; i < LED_CNT; i++)
> +		if (test_bit(i, input_led_registered)) {
> +			led_classdev_unregister(&input_leds[i]);
> +			clear_bit(i, input_led_registered);
> +		}
> +	spin_unlock_irqrestore(&input_leds_lock, flags);
> +}
> +
> +module_init(input_led_init);
> +module_exit(input_led_exit);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("User LED support for input layer");
> +MODULE_AUTHOR("Samuel Thibault <samuel.thibault@ens-lyon.org>");
> diff -ur linux-2.6.33-orig/drivers/leds/Makefile linux-2.6.33-perso/drivers/leds/Makefile
> --- linux-2.6.33-orig/drivers/leds/Makefile	2010-02-25 01:41:27.000000000 +0100
> +++ linux-2.6.33-perso/drivers/leds/Makefile	2010-02-25 01:45:28.000000000 +0100
> @@ -5,6 +5,7 @@
>  obj-$(CONFIG_LEDS_TRIGGERS)		+= led-triggers.o
>  
>  # LED Platform Drivers
> +obj-$(CONFIG_LEDS_INPUT)		+= leds-input.o
>  obj-$(CONFIG_LEDS_ATMEL_PWM)		+= leds-atmel-pwm.o
>  obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
>  obj-$(CONFIG_LEDS_LOCOMO)		+= leds-locomo.o

-- 
Dmitry

Re: [PATCH] Route kbd leds through the generic leds layer (3rd version)

[Richard Purdie]

On Thu, 2010-02-25 at 02:20 -0800, Dmitry Torokhov wrote:
> On Thu, Feb 25, 2010 at 02:38:40AM +0100, Samuel Thibault wrote:
> > Route keyboard leds through the generic leds layer.
> > 
> > This permits to reassign keyboard LEDs to something else than keyboard
> > "leds" state, and also permits to fix #7063 from userland by using a
> > modifier to implement proper CapsLock behavior and have the keyboard
> > caps lock led show that caps lock state.
> > 
> 
> I am aunsure about this patch. It ties all LEDs together whereas
> currently it is possible to operate keyboards and their leds
> independently (as long as legacy keyboard driver is out of picture, i.e.
> in X or otherwise when we have console in raw mode).

I know what you mean, I'm wary too.

Looking at the patch I think it might make sense to split the patch into
two parts under separate kconfig options. The first would be the
triggers themselves which are independent in their own right and
shouldn't be too controversial.

The second part would be the input LEDs themselves. My input susbsystem
memory is fuzzy, I assume this sets a set of LEDs per input device and
attaches the appropriate default triggers?

Is there a set of states we can detect (like raw mode) when we could
just know to disconnect the default triggers?


> > Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> > ---
> > 
> > Hello,
> > 
> > Here is a third version of the patch and should now be fine for
> > inclusion e.g. in the mm tree and eventually pushed to 2.6.34. The main
> > difference with the first version is that now only input leds actually
> > available on some device are registered. The difference with the second
> > version is a refresh against 2.6.33 and a locking fix.
> > 
> > The first version was acked by Pavel Machek.
> > 
> > Samuel
> > 
> > diff -ur linux-2.6.33-orig/Documentation/leds-class.txt linux-2.6.33-perso/Documentation/leds-class.txt
> > --- linux-2.6.33-orig/Documentation/leds-class.txt	2009-12-03 13:41:42.000000000 +0100
> > +++ linux-2.6.33-perso/Documentation/leds-class.txt	2010-02-25 01:45:28.000000000 +0100
> > @@ -2,9 +2,6 @@
> >  LED handling under Linux
> >  ========================
> >  
> > -If you're reading this and thinking about keyboard leds, these are
> > -handled by the input subsystem and the led class is *not* needed.
> > -
> >  In its simplest form, the LED class just allows control of LEDs from
> >  userspace. LEDs appear in /sys/class/leds/. The maximum brightness of the
> >  LED is defined in max_brightness file. The brightness file will set the brightness
> > diff -ur linux-2.6.33-orig/drivers/char/keyboard.c linux-2.6.33-perso/drivers/char/keyboard.c
> > --- linux-2.6.33-orig/drivers/char/keyboard.c	2010-02-25 01:41:19.000000000 +0100
> > +++ linux-2.6.33-perso/drivers/char/keyboard.c	2010-02-25 01:52:11.000000000 +0100
> > @@ -34,6 +34,7 @@
> >  #include <linux/init.h>
> >  #include <linux/slab.h>
> >  #include <linux/irq.h>
> > +#include <linux/leds.h>
> >  
> >  #include <linux/kbd_kern.h>
> >  #include <linux/kbd_diacr.h>
> > @@ -139,6 +140,9 @@
> >  static char rep;					/* flag telling character repeat */
> >  
> >  static unsigned char ledstate = 0xff;			/* undefined */
> > +#ifdef CONFIG_LEDS_INPUT
> > +static unsigned char lockstate = 0xff;			/* undefined */
> > +#endif
> >  static unsigned char ledioctl;
> >  
> >  static struct ledptr {
> > @@ -971,6 +975,23 @@
> >  	}
> >  }
> >  
> > +#ifdef CONFIG_LEDS_INPUT
> > +/* When input-based leds are enabled, we route keyboard "leds" through triggers
> > + */
> > +DEFINE_LED_TRIGGER(ledtrig_scrolllock);
> > +DEFINE_LED_TRIGGER(ledtrig_numlock);
> > +DEFINE_LED_TRIGGER(ledtrig_capslock);
> > +DEFINE_LED_TRIGGER(ledtrig_kanalock);
> > +DEFINE_LED_TRIGGER(ledtrig_shiftlock);
> > +DEFINE_LED_TRIGGER(ledtrig_altgrlock);
> > +DEFINE_LED_TRIGGER(ledtrig_ctrllock);
> > +DEFINE_LED_TRIGGER(ledtrig_altlock);
> > +DEFINE_LED_TRIGGER(ledtrig_shiftllock);
> > +DEFINE_LED_TRIGGER(ledtrig_shiftrlock);
> > +DEFINE_LED_TRIGGER(ledtrig_ctrlllock);
> > +DEFINE_LED_TRIGGER(ledtrig_ctrlrlock);
> > +#endif
> > +
> >  /*
> >   * The leds display either (i) the status of NumLock, CapsLock, ScrollLock,
> >   * or (ii) whatever pattern of lights people want to show using KDSETLED,
> > @@ -1019,9 +1040,12 @@
> >  	unsigned char leds = *(unsigned char *)data;
> >  
> >  	if (test_bit(EV_LED, handle->dev->evbit)) {
> > -		input_inject_event(handle, EV_LED, LED_SCROLLL, !!(leds & 0x01));
> > -		input_inject_event(handle, EV_LED, LED_NUML,    !!(leds & 0x02));
> > -		input_inject_event(handle, EV_LED, LED_CAPSL,   !!(leds & 0x04));
> > +		input_inject_event(handle, EV_LED, LED_SCROLLL,
> > +				!!(leds & VC_SCROLLOCK));
> > +		input_inject_event(handle, EV_LED, LED_NUML,
> > +				!!(leds & VC_NUMLOCK));
> > +		input_inject_event(handle, EV_LED, LED_CAPSL,
> > +				!!(leds & VC_CAPSLOCK));
> >  		input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
> >  	}
> >  
> > @@ -1040,10 +1064,51 @@
> >  	unsigned char leds = getleds();
> >  
> >  	if (leds != ledstate) {
> > +#ifdef CONFIG_LEDS_INPUT
> > +		led_trigger_event(ledtrig_scrolllock,
> > +				leds & (1 << VC_SCROLLOCK) ? INT_MAX : LED_OFF);
> > +		led_trigger_event(ledtrig_numlock,
> > +				leds & (1 << VC_NUMLOCK)   ? INT_MAX : LED_OFF);
> > +		led_trigger_event(ledtrig_capslock,
> > +				leds & (1 << VC_CAPSLOCK)  ? INT_MAX : LED_OFF);
> > +		led_trigger_event(ledtrig_kanalock,
> > +				leds & (1 << VC_KANALOCK)  ? INT_MAX : LED_OFF);
> > +#else
> >  		input_handler_for_each_handle(&kbd_handler, &leds,
> >  					      kbd_update_leds_helper);
> > +#endif
> >  		ledstate = leds;
> >  	}
> > +
> > +#ifdef CONFIG_LEDS_INPUT
> > +	if (kbd->lockstate != lockstate) {
> > +		led_trigger_event(ledtrig_shiftlock,
> > +			kbd->lockstate & (1<<VC_SHIFTLOCK)
> > +				? INT_MAX : LED_OFF);
> > +		led_trigger_event(ledtrig_altgrlock,
> > +			kbd->lockstate & (1<<VC_ALTGRLOCK)
> > +				? INT_MAX : LED_OFF);
> > +		led_trigger_event(ledtrig_ctrllock,
> > +			kbd->lockstate & (1<<VC_CTRLLOCK)
> > +				? INT_MAX : LED_OFF);
> > +		led_trigger_event(ledtrig_altlock,
> > +			kbd->lockstate & (1<<VC_ALTLOCK)
> > +				? INT_MAX : LED_OFF);
> > +		led_trigger_event(ledtrig_shiftllock,
> > +			kbd->lockstate & (1<<VC_SHIFTLLOCK)
> > +				? INT_MAX : LED_OFF);
> > +		led_trigger_event(ledtrig_shiftrlock,
> > +			kbd->lockstate & (1<<VC_SHIFTRLOCK)
> > +				? INT_MAX : LED_OFF);
> > +		led_trigger_event(ledtrig_ctrlllock,
> > +			kbd->lockstate & (1<<VC_CTRLLLOCK)
> > +				? INT_MAX : LED_OFF);
> > +		led_trigger_event(ledtrig_ctrlrlock,
> > +			kbd->lockstate & (1<<VC_CTRLRLOCK)
> > +				? INT_MAX : LED_OFF);
> > +		lockstate = kbd->lockstate;
> > +	}
> > +#endif
> >  }
> >  
> >  DECLARE_TASKLET_DISABLED(keyboard_tasklet, kbd_bh, 0);
> > @@ -1380,6 +1445,7 @@
> >  	kfree(handle);
> >  }
> >  
> > +#ifndef CONFIG_LEDS_INPUT
> >  /*
> >   * Start keyboard handler on the new keyboard by refreshing LED state to
> >   * match the rest of the system.
> > @@ -1393,6 +1459,7 @@
> >  
> >  	tasklet_enable(&keyboard_tasklet);
> >  }
> > +#endif
> >  
> >  static const struct input_device_id kbd_ids[] = {
> >  	{
> > @@ -1414,7 +1481,9 @@
> >  	.event		= kbd_event,
> >  	.connect	= kbd_connect,
> >  	.disconnect	= kbd_disconnect,
> > +#ifndef CONFIG_LEDS_INPUT
> >  	.start		= kbd_start,
> > +#endif
> >  	.name		= "kbd",
> >  	.id_table	= kbd_ids,
> >  };
> > @@ -1438,6 +1507,21 @@
> >  	if (error)
> >  		return error;
> >  
> > +#ifdef CONFIG_LEDS_INPUT
> > +	led_trigger_register_simple("scrolllock", &ledtrig_scrolllock);
> > +	led_trigger_register_simple("numlock", &ledtrig_numlock);
> > +	led_trigger_register_simple("capslock", &ledtrig_capslock);
> > +	led_trigger_register_simple("kanalock", &ledtrig_kanalock);
> > +	led_trigger_register_simple("shiftlock", &ledtrig_shiftlock);
> > +	led_trigger_register_simple("altgrlock", &ledtrig_altgrlock);
> > +	led_trigger_register_simple("ctrllock", &ledtrig_ctrllock);
> > +	led_trigger_register_simple("altlock", &ledtrig_altlock);
> > +	led_trigger_register_simple("shiftllock", &ledtrig_shiftllock);
> > +	led_trigger_register_simple("shiftrlock", &ledtrig_shiftrlock);
> > +	led_trigger_register_simple("ctrlllock", &ledtrig_ctrlllock);
> > +	led_trigger_register_simple("ctrlrlock", &ledtrig_ctrlrlock);
> > +#endif
> > +
> >  	tasklet_enable(&keyboard_tasklet);
> >  	tasklet_schedule(&keyboard_tasklet);
> >  
> > diff -ur linux-2.6.33-orig/drivers/leds/Kconfig linux-2.6.33-perso/drivers/leds/Kconfig
> > --- linux-2.6.33-orig/drivers/leds/Kconfig	2010-02-25 01:41:27.000000000 +0100
> > +++ linux-2.6.33-perso/drivers/leds/Kconfig	2010-02-25 01:45:28.000000000 +0100
> > @@ -4,9 +4,6 @@
> >  	  Say Y to enable Linux LED support.  This allows control of supported
> >  	  LEDs from both userspace and optionally, by kernel events (triggers).
> >  
> > -	  This is not related to standard keyboard LEDs which are controlled
> > -	  via the input system.
> > -
> >  if NEW_LEDS
> >  
> >  config LEDS_CLASS
> > @@ -17,6 +14,13 @@
> >  
> >  comment "LED drivers"
> >  
> > +config LEDS_INPUT
> > +	tristate "LED Support using input keyboards"
> > +	depends on LEDS_CLASS
> > +	help
> > +	  This option enables support for the LEDs on keyboard managed
> > +	  by the input layer.
> > +
> >  config LEDS_ATMEL_PWM
> >  	tristate "LED Support using Atmel PWM outputs"
> >  	depends on LEDS_CLASS && ATMEL_PWM
> > diff -ur linux-2.6.33-orig/drivers/leds/leds-input.c linux-2.6.33-perso/drivers/leds/leds-input.c
> > --- linux-2.6.33-orig/drivers/leds/leds-input.c	2010-02-21 04:13:41.000000000 +0100
> > +++ linux-2.6.33-perso/drivers/leds/leds-input.c	2010-02-25 01:45:28.000000000 +0100
> > @@ -0,0 +1,195 @@
> > +/*
> > + * LED support for the input layer
> > + *
> > + * Copyright 2010 Samuel Thibault <samuel.thibault@ens-lyon.org>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/init.h>
> > +#include <linux/leds.h>
> > +#include <linux/input.h>
> > +
> > +/* Protects concurrency of led state */
> > +static DEFINE_SPINLOCK(input_leds_lock);
> > +/* Current state */
> > +static unsigned long input_led_leds[BITS_TO_LONGS(LED_CNT)];
> > +/* array of all led classes */
> > +static struct led_classdev input_leds[LED_CNT];
> > +/* Protects concurrency of led registration */
> > +static DEFINE_SPINLOCK(input_led_registered_lock);
> > +/* which led classes are registered */
> > +static unsigned long input_led_registered[BITS_TO_LONGS(LED_CNT)];
> > +/* our handler */
> > +static struct input_handler input_led_handler;
> > +
> > +/* Led state change, update all keyboards */
> > +static void input_led_set(struct led_classdev *cdev,
> > +			  enum led_brightness brightness)
> > +{
> > +	int led = cdev - input_leds;
> > +	unsigned long flags;
> > +	struct input_handle *handle;
> > +
> > +	spin_lock_irqsave(&input_leds_lock, flags);
> > +	list_for_each_entry(handle, &input_led_handler.h_list, h_node) {
> > +		input_inject_event(handle, EV_LED, led, !!brightness);
> > +		input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
> > +	}
> > +	if (brightness)
> > +		set_bit(led, input_led_leds);
> > +	else
> > +		clear_bit(led, input_led_leds);
> > +	spin_unlock_irqrestore(&input_leds_lock, flags);
> > +}
> > +
> > +static struct led_classdev input_leds[LED_CNT] = {
> > +#define DEFINE_INPUT_LED(input_led, nam, deftrig) \
> > +	[input_led] = { \
> > +		.name = "input::"nam, \
> > +		.max_brightness = 1, \
> > +		.brightness_set = input_led_set, \
> > +		.default_trigger = deftrig, \
> > +	}
> > +DEFINE_INPUT_LED(LED_NUML, "numlock", "numlock"),
> > +DEFINE_INPUT_LED(LED_CAPSL, "capslock", "capslock"),
> > +DEFINE_INPUT_LED(LED_SCROLLL, "scrolllock", "scrolllock"),
> > +DEFINE_INPUT_LED(LED_COMPOSE, "compose", NULL),
> > +DEFINE_INPUT_LED(LED_KANA, "kana", "kanalock"),
> > +DEFINE_INPUT_LED(LED_SLEEP, "sleep", NULL),
> > +DEFINE_INPUT_LED(LED_SUSPEND, "suspend", NULL),
> > +DEFINE_INPUT_LED(LED_MUTE, "mute", NULL),
> > +DEFINE_INPUT_LED(LED_MISC, "misc", NULL),
> > +DEFINE_INPUT_LED(LED_MAIL, "mail", NULL),
> > +DEFINE_INPUT_LED(LED_CHARGING, "charging", NULL),
> > +};
> > +
> > +static int input_led_connect(struct input_handler *handler,
> > +			      struct input_dev *dev,
> > +			      const struct input_device_id *id)
> > +{
> > +	struct input_handle *handle;
> > +	int i, error;
> > +	unsigned long flags;
> > +
> > +	if (!test_bit(EV_LED, dev->keybit))
> > +		return -ENODEV;
> > +
> > +	handle = kzalloc(sizeof(struct input_handle), GFP_KERNEL);
> > +	if (!handle)
> > +		return -ENOMEM;
> > +
> > +	handle->dev = dev;
> > +	handle->handler = handler;
> > +	handle->name = "input leds";
> > +
> > +	error = input_register_handle(handle);
> > +	if (error) {
> > +		kfree(handle);
> > +		return error;
> > +	}
> > +
> > +	spin_lock_irqsave(&input_led_registered_lock, flags);
> > +	for (i = 0; i < LED_CNT; i++)
> > +		if (input_leds[i].name
> > +				&& !test_bit(i, input_led_registered)
> > +				&& test_bit(i, dev->ledbit))
> > +			/* This keyboard has led i, try to register it */
> > +			if (!led_classdev_register(NULL, &input_leds[i]))
> > +				set_bit(i, input_led_registered);
> > +	spin_unlock_irqrestore(&input_led_registered_lock, flags);
> > +
> > +	return 0;
> > +}
> > +
> > +static void input_led_disconnect(struct input_handle *handle)
> > +{
> > +	int unregister, i;
> > +	unsigned long flags;
> > +
> > +	input_unregister_handle(handle);
> > +	kfree(handle);
> > +
> > +	spin_lock_irqsave(&input_led_registered_lock, flags);
> > +	for (i = 0; i < LED_CNT; i++) {
> > +		if (!test_bit(i, input_led_registered))
> > +			continue;
> > +
> > +		unregister = 1;
> > +		list_for_each_entry(handle, &input_led_handler.h_list, h_node) {
> > +			if (test_bit(i, handle->dev->ledbit)) {
> > +				unregister = 0;
> > +				break;
> > +			}
> > +		}
> > +		if (!unregister)
> > +			continue;
> > +
> > +		led_classdev_unregister(&input_leds[i]);
> > +		clear_bit(i, input_led_registered);
> > +	}
> > +	spin_unlock_irqrestore(&input_led_registered_lock, flags);
> > +}
> > +
> > +/* New keyboard, update its leds */
> > +static void input_led_start(struct input_handle *handle)
> > +{
> > +	unsigned long flags;
> > +	int i;
> > +
> > +	spin_lock_irqsave(&input_leds_lock, flags);
> > +	for (i = 0; i < LED_CNT; i++)
> > +		if (input_leds[i].name && test_bit(i, handle->dev->ledbit))
> > +			input_inject_event(handle, EV_LED, i,
> > +					test_bit(i, input_led_leds));
> > +	input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
> > +	spin_unlock_irqrestore(&input_leds_lock, flags);
> > +}
> > +
> > +static const struct input_device_id input_led_ids[] = {
> > +	{
> > +		.flags = INPUT_DEVICE_ID_MATCH_EVBIT,
> > +		.evbit = { BIT_MASK(EV_LED) },
> > +	},
> > +
> > +	{ },	/* Terminating entry */
> > +};
> > +
> > +static struct input_handler input_led_handler = {
> > +	.connect	= input_led_connect,
> > +	.disconnect	= input_led_disconnect,
> > +	.start		= input_led_start,
> > +	.name		= "input leds",
> > +	.id_table	= input_led_ids,
> > +};
> > +
> > +static int __init input_led_init(void)
> > +{
> > +	return input_register_handler(&input_led_handler);
> > +}
> > +
> > +static void __exit input_led_exit(void)
> > +{
> > +	int i;
> > +	unsigned long flags;
> > +
> > +	input_unregister_handler(&input_led_handler);
> > +
> > +	spin_lock_irqsave(&input_leds_lock, flags);
> > +	for (i = 0; i < LED_CNT; i++)
> > +		if (test_bit(i, input_led_registered)) {
> > +			led_classdev_unregister(&input_leds[i]);
> > +			clear_bit(i, input_led_registered);
> > +		}
> > +	spin_unlock_irqrestore(&input_leds_lock, flags);
> > +}
> > +
> > +module_init(input_led_init);
> > +module_exit(input_led_exit);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_DESCRIPTION("User LED support for input layer");
> > +MODULE_AUTHOR("Samuel Thibault <samuel.thibault@ens-lyon.org>");
> > diff -ur linux-2.6.33-orig/drivers/leds/Makefile linux-2.6.33-perso/drivers/leds/Makefile
> > --- linux-2.6.33-orig/drivers/leds/Makefile	2010-02-25 01:41:27.000000000 +0100
> > +++ linux-2.6.33-perso/drivers/leds/Makefile	2010-02-25 01:45:28.000000000 +0100
> > @@ -5,6 +5,7 @@
> >  obj-$(CONFIG_LEDS_TRIGGERS)		+= led-triggers.o
> >  
> >  # LED Platform Drivers
> > +obj-$(CONFIG_LEDS_INPUT)		+= leds-input.o
> >  obj-$(CONFIG_LEDS_ATMEL_PWM)		+= leds-atmel-pwm.o
> >  obj-$(CONFIG_LEDS_BD2802)		+= leds-bd2802.o
> >  obj-$(CONFIG_LEDS_LOCOMO)		+= leds-locomo.o
> 

Re: [PATCH] Route kbd leds through the generic leds layer (3rd version)

[Samuel Thibault]

Dmitry Torokhov, le Thu 25 Feb 2010 02:20:56 -0800, a écrit :
> I am aunsure about this patch. It ties all LEDs together

For now, yes.  There could be an additional per-device layer that the
user could select instead, but my patch doesn't prevent that.

> whereas currently it is possible to operate keyboards and their leds
> independently

You mean through the input layer?  Technically, yes, but at least my
patch is an improvement over what exists right now: at first read of
your sentence I thought "of course not! keyboard.c does it for all
devices!", so it's not a regression in that concern.  On the contrary,
it fixes the problem of proper caps lock with led feedback.

Being able to assign only some of the devices to the linux console
would indeed probably be good, but to me it's just a refinement. Users
a priori assume all keyboards get their leds updated, so my patch
makes sense.  And it won't prevent a further patch that, in addition
to input::<led> leds, adds per-device leds, which the user could use
instead of input::<led>.

Samuel

Re: [PATCH] Route kbd leds through the generic leds layer (3rd version)

[Samuel Thibault]

Richard Purdie, le Thu 25 Feb 2010 12:23:20 +0000, a écrit :
> The first would be the triggers themselves which are independent in
> their own right and shouldn't be too controversial.

One issue is that keyboard.c has to know whether to send input events
itself as it does now (e.g. because triggers are not compiled), or to
rely on triggers to do it for him.

> The second part would be the input LEDs themselves. My input susbsystem
> memory is fuzzy, I assume this sets a set of LEDs per input device and
> attaches the appropriate default triggers?

I would rather say that it sets a set of input devices per LED and
attaches them the appropriate default trigger.

> Is there a set of states we can detect (like raw mode) when we could
> just know to disconnect the default triggers?

What for?

Samuel

Re: [PATCH] Route kbd leds through the generic leds layer (3rd version)

[Dmitry Torokhov]

On Thu, Feb 25, 2010 at 10:44:03PM +0100, Samuel Thibault wrote:
> Dmitry Torokhov, le Thu 25 Feb 2010 02:20:56 -0800, a écrit :
> > I am aunsure about this patch. It ties all LEDs together
> 
> For now, yes.  There could be an additional per-device layer that the
> user could select instead, but my patch doesn't prevent that.
> 
> > whereas currently it is possible to operate keyboards and their leds
> > independently
> 
> You mean through the input layer?  Technically, yes, but at least my
> patch is an improvement over what exists right now: at first read of
> your sentence I thought "of course not! keyboard.c does it for all
> devices!", so it's not a regression in that concern.

Right now on .33, if you write LED event to one event device while
legacy keyboard driver is inactive (in raw mode) then that write will
only affect that particular event device, not the rest of them. Your
patch changes that.

Whether LED state should be shared between all input devices or should
be kept separate is a matter of policy and we try to keep policy in
userspace.

>  On the contrary,
> it fixes the problem of proper caps lock with led feedback.
> 

I am unaware of such issue, any pointers?

> Being able to assign only some of the devices to the linux console
> would indeed probably be good, but to me it's just a refinement. Users
> a priori assume all keyboards get their leds updated, so my patch
> makes sense.  And it won't prevent a further patch that, in addition
> to input::<led> leds, adds per-device leds, which the user could use
> instead of input::<led>.

I think that if we want to go LED route we shoudl start with adding led
devices to individual keyboard drivers first and then convert legacy
keyboard to use trigger instead of talking to input devices directly.

Thanks.

-- 
Dmitry

Re: [PATCH] Route kbd leds through the generic leds layer (3rd version)

[Samuel Thibault]

Dmitry Torokhov, le Thu 25 Feb 2010 19:54:48 -0800, a écrit :
> Right now on .33, if you write LED event to one event device while
> legacy keyboard driver is inactive (in raw mode) then that write will
> only affect that particular event device, not the rest of them. Your
> patch changes that.

Errr, I don't think so.  My patch just puts a trigger layer between
keyboard and the loop over devices that keyboard used to do.  When the
legacy keyboard driver is inactive, it doesn't trigger anything any
more, and my patch becomes actually inactive, and other sources of LED
events can send them to various input event devices independently.  I've
just tried it with Xorg (telling it to use only one keyboard) and it
works.

> Whether LED state should be shared between all input devices or should
> be kept separate is a matter of policy and we try to keep policy in
> userspace.

Sure.

> >  On the contrary,
> > it fixes the problem of proper caps lock with led feedback.
> 
> I am unaware of such issue, any pointers?

In the very changelog of my patch :) #7063
And also please see the thread “make CapsLock work as expected even
for non-ASCII” on lkml.

> > Being able to assign only some of the devices to the linux console
> > would indeed probably be good, but to me it's just a refinement. Users
> > a priori assume all keyboards get their leds updated, so my patch
> > makes sense.  And it won't prevent a further patch that, in addition
> > to input::<led> leds, adds per-device leds, which the user could use
> > instead of input::<led>.
> 
> I think that if we want to go LED route we shoudl start with adding led
> devices to individual keyboard drivers first

Mmm, thinking again, I think my patch could be rather easily converted
into creating led instances for each device, with proper trigger
defaults.  Something like input-devicename::numlock? (devicename::numlock
might get conflicts with non-input devices I guess?)

That being said, usermode tools which want to set up the modifiers and
connect the input LEDs to them need an easy and proper way to do so.
Having central input::numlock and such still seems a good thing.

> and then convert legacy
> keyboard to use trigger instead of talking to input devices directly.

This part is already done by my patch.

Samuel

Re: [PATCH] Route kbd leds through the generic leds layer (3rd version)

[Pavel Machek]

On Thu 2010-02-25 22:44:03, Samuel Thibault wrote:
> Dmitry Torokhov, le Thu 25 Feb 2010 02:20:56 -0800, a écrit :
> > I am aunsure about this patch. It ties all LEDs together
> 
> For now, yes.  There could be an additional per-device layer that the
> user could select instead, but my patch doesn't prevent that.

Well, having one LED device for all the keyboard leds, and separate
LED device for each of them indivudually would not be quite nice, and
once we have the "one for all" device, we can't really get rid of it.

> Being able to assign only some of the devices to the linux console
> would indeed probably be good, but to me it's just a refinement. Users
> a priori assume all keyboards get their leds updated, so my patch
> makes sense.  And it won't prevent a further patch that, in addition
> to input::<led> leds, adds per-device leds, which the user could use
> instead of input::<led>.

As I said, having both is ugly... but the triggers could / should be
merged now...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Route kbd leds through the generic leds layer (3rd version)

[Pavel Machek]

Hi!

> > > Being able to assign only some of the devices to the linux console
> > > would indeed probably be good, but to me it's just a refinement. Users
> > > a priori assume all keyboards get their leds updated, so my patch
> > > makes sense.  And it won't prevent a further patch that, in addition
> > > to input::<led> leds, adds per-device leds, which the user could use
> > > instead of input::<led>.
> > 
> > I think that if we want to go LED route we shoudl start with adding led
> > devices to individual keyboard drivers first
> 
> Mmm, thinking again, I think my patch could be rather easily converted
> into creating led instances for each device, with proper trigger
> defaults.  Something like input-devicename::numlock? (devicename::numlock
> might get conflicts with non-input devices I guess?)

Yep, that would be cool.

> That being said, usermode tools which want to set up the modifiers and
> connect the input LEDs to them need an easy and proper way to do so.
> Having central input::numlock and such still seems a good thing.

This is something where I'm not too sure. Opening few files in
sequence is not that hard to the userland so "group of leds"
abstraction does not make too much sense...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Route kbd leds through the generic leds layer (3rd version)

[Samuel Thibault]

Pavel Machek, le Sat 06 Mar 2010 07:54:57 +0100, a écrit :
> > That being said, usermode tools which want to set up the modifiers and
> > connect the input LEDs to them need an easy and proper way to do so.
> > Having central input::numlock and such still seems a good thing.
> 
> This is something where I'm not too sure. Opening few files in
> sequence is not that hard to the userland so "group of leds"
> abstraction does not make too much sense...

But then userland has to monitor keyboard hotplug. At the moment,
neither kbd nor console-setup use a daemon to handle keyboards, and I
doubt their authors will be happy to have to.

Samuel

Re: [PATCH] Route kbd leds through the generic leds layer (3rd version)

[Pavel Machek]

On Sat 2010-03-06 13:30:58, Samuel Thibault wrote:
> Pavel Machek, le Sat 06 Mar 2010 07:54:57 +0100, a écrit :
> > > That being said, usermode tools which want to set up the modifiers and
> > > connect the input LEDs to them need an easy and proper way to do so.
> > > Having central input::numlock and such still seems a good thing.
> > 
> > This is something where I'm not too sure. Opening few files in
> > sequence is not that hard to the userland so "group of leds"
> > abstraction does not make too much sense...
> 
> But then userland has to monitor keyboard hotplug. At the moment,
> neither kbd nor console-setup use a daemon to handle keyboards, and I
> doubt their authors will be happy to have to.

Okay, that makes some sense.

But you mentioned having per-keyboard led would be quite easy, it does
not seem to be controversial, so perhaps that's way to go, first?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH] Route kbd leds through the generic leds layer (3rd version)

[Samuel Thibault]

Pavel Machek, le Sun 07 Mar 2010 09:25:19 +0100, a écrit :
> But you mentioned having per-keyboard led would be quite easy, it does
> not seem to be controversial, so perhaps that's way to go, first?

Thinking more about it, what about the following:

keyboard numlock trigger -> input numlock led -> input numlock trigger -> input keyboard led

Userland applications which want to redirect leds for (a priori) all
keyboards change the trigger used by the input numlock led. Userland
applications which want to redirect leds for a particular keyboard can
change the trigger user by the particular input keyboard led.

Samuel

Re: [PATCH] Route kbd LEDs through the generic LEDs layer

[Pali Rohár]

2014-03-16 11:19 GMT+01:00 Samuel Thibault <samuel.thibault@ens-lyon.org>:
> Pali Rohár, le Sun 16 Mar 2014 11:16:25 +0100, a écrit :
>> Hello, what happened with this patch? Is there any problem with accepting it?
>
> Dmitry finding time to review it, I guess.
>
> Samuel

Dmitry, can you look and review this patch?

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH] Route kbd LEDs through the generic LEDs layer

[Samuel Thibault]

Pali Rohár, le Sun 16 Mar 2014 11:16:25 +0100, a écrit :
> Hello, what happened with this patch? Is there any problem with accepting it?

Dmitry finding time to review it, I guess.

Samuel

Re: [PATCH] Route kbd LEDs through the generic LEDs layer

[Pali Rohár]

2013-07-15 11:12 GMT+02:00 Pavel Machek <pavel@ucw.cz>:
> Hi!
>
>> > > > > This permits to reassign keyboard LEDs to something else than keyboard "leds"
>> > > > > state, by adding keyboard led and modifier triggers connected to a series
>> > > > > of VT input LEDs, themselves connected to VT input triggers, which
>> > > > > per-input device LEDs use by default.  Userland can thus easily change the LED
>> > > > > behavior of (a priori) all input devices, or of particular input devices.
>> > > >
>> > > > Nice! Leds now have proper /sys interface.
>> > > >
>> > > > But... I boot up, switch from X to console, press capslock, and no
>> > > > reaction anywhere.
>> > >
>> > > Is it working without the patch?  Console-setup for instance is known to
>> > > have broken the capslock LED, which is precisely one of the reasons for
>> > > this patch, which will provide console-setup with a way to bring back
>> > > caps lock working properly.
>> >
>> > You are right, it was broken before.
>>
>> Ok. You then may want fix your setup by configuring your caps lock led
>> the way console-setup will be supposed to do in the future:
>>
>> echo ctrlllock > /sys/class/leds/vt::capsl/trigger
>
> And this indeed makes capslock led work on console, again. Nice!
>
> Richard, could we get this applied for 3.12 or something? It makes
> keyboard LEDs sane...
>
> Thanks,
>                                                                         Pavel
>
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
>

Hello, what happened with this patch? Is there any problem with accepting it?

-- 
Pali Rohár
pali.rohar@gmail.com

Re: [PATCH] Route kbd LEDs through the generic LEDs layer

[David Herrmann]

Hi

On Mon, Jul 15, 2013 at 9:08 PM, Samuel Thibault
<samuel.thibault@ens-lyon.org> wrote:
> Hello,
>
> David Herrmann, le Mon 15 Jul 2013 17:03:08 +0200, a écrit :
>> > @@ -13,6 +13,10 @@
>> >         bool "Virtual terminal" if EXPERT
>> >         depends on !S390 && !UML
>> >         select INPUT
>> > +       select NEW_LEDS
>> > +       select LEDS_CLASS
>> > +       select LEDS_TRIGGERS
>> > +       select INPUT_LEDS
>>
>> This looks odd. Any dependency changes in the input-layer will break
>> this. But I guess that's what we get for a non-recursive "select". Hmm
>
> Yes, that's the issue.
>
>> I also think that the macros don't really simplify this. So:
>>   [VC_SHIFTLOCK] = { .name = "shiftlock", .activate =
>> kbd_lockstate_trigger_activate },
>> isn't much longer than:
>>   DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTLOCK,  "shiftlock"),
>
> That makes it more than 80 columns, at least.

Indeed, I missed that.

>> > +       for (i = 0; i < ARRAY_SIZE(ledtrig_ledstate); i++)
>> > +               led_trigger_register(&ledtrig_ledstate[i]);
>> > +       for (i = 0; i < ARRAY_SIZE(ledtrig_lockstate); i++)
>> > +               led_trigger_register(&ledtrig_lockstate[i]);
>> > +
>>
>> This returns "int", are you sure no error-handling is needed?
>
> ATM only registering the same name several times can happen, and
> shouldn't ever happen. But better warn than be sorry, indeed.

Yepp, it also avoids ignoring any future errors that we might
introduce in led_trigger_register().

>> > + * Keyboard LEDs are propagated by default like the following example:
>> > + *
>> > + * VT keyboard numlock trigger
>> > + * -> vt::numl VT LED
>> > + * -> vt-numl VT trigger
>>
>> Nitpick: What's the reason for the syntax difference? "vt-numl" vs. "vt::numl"
>
> vt::numl is a clear LED classification convention. For triggers, I don't
> see a clear convention, but at least I have never seen "::", only "-".
> That makes me realize that for keyboard triggers I haven't introduced a
> prefix, and only used "scrollock", "numlock", etc. Perhaps
> "kbd-scrollock" etc. or (in phase with LEDs), "kbd::scrollock" etc.?

I understand. Due to lack of insight, I cannot really comment on that,
sorry. Just wanted to go sure you're aware of that. So feel free to
pick any of them.

Thanks
David

Re: [PATCH] Route kbd LEDs through the generic LEDs layer

[Samuel Thibault]

Hello,

David Herrmann, le Mon 15 Jul 2013 17:03:08 +0200, a écrit :
> > @@ -13,6 +13,10 @@
> >         bool "Virtual terminal" if EXPERT
> >         depends on !S390 && !UML
> >         select INPUT
> > +       select NEW_LEDS
> > +       select LEDS_CLASS
> > +       select LEDS_TRIGGERS
> > +       select INPUT_LEDS
> 
> This looks odd. Any dependency changes in the input-layer will break
> this. But I guess that's what we get for a non-recursive "select". Hmm

Yes, that's the issue.

> I also think that the macros don't really simplify this. So:
>   [VC_SHIFTLOCK] = { .name = "shiftlock", .activate =
> kbd_lockstate_trigger_activate },
> isn't much longer than:
>   DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTLOCK,  "shiftlock"),

That makes it more than 80 columns, at least.

> > +       for (i = 0; i < ARRAY_SIZE(ledtrig_ledstate); i++)
> > +               led_trigger_register(&ledtrig_ledstate[i]);
> > +       for (i = 0; i < ARRAY_SIZE(ledtrig_lockstate); i++)
> > +               led_trigger_register(&ledtrig_lockstate[i]);
> > +
> 
> This returns "int", are you sure no error-handling is needed?

ATM only registering the same name several times can happen, and
shouldn't ever happen. But better warn than be sorry, indeed.

> > + * Keyboard LEDs are propagated by default like the following example:
> > + *
> > + * VT keyboard numlock trigger
> > + * -> vt::numl VT LED
> > + * -> vt-numl VT trigger
> 
> Nitpick: What's the reason for the syntax difference? "vt-numl" vs. "vt::numl"

vt::numl is a clear LED classification convention. For triggers, I don't
see a clear convention, but at least I have never seen "::", only "-".
That makes me realize that for keyboard triggers I haven't introduced a
prefix, and only used "scrollock", "numlock", etc. Perhaps
"kbd-scrollock" etc. or (in phase with LEDs), "kbd::scrollock" etc.?

Samuel

Re: [PATCH] Route kbd LEDs through the generic LEDs layer

[David Herrmann]

Hi

On Sun, Jul 7, 2013 at 12:10 PM, Samuel Thibault
<samuel.thibault@ens-lyon.org> wrote:
> This permits to reassign keyboard LEDs to something else than keyboard "leds"
> state, by adding keyboard led and modifier triggers connected to a series
> of VT input LEDs, themselves connected to VT input triggers, which
> per-input device LEDs use by default.  Userland can thus easily change the LED
> behavior of (a priori) all input devices, or of particular input devices.
>
> This also permits to fix #7063 from userland by using a modifier to implement
> proper CapsLock behavior and have the keyboard caps lock led show that modifier
> state.
>
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> [ebroder@mokafive.com: Rebased to 3.2-rc1 or so, cleaned up some includes, and fixed some constants]
> Signed-off-by: Evan Broder <evan@ebroder.net>
>
> diff --exclude .svn -ur linux-3.10-orig/Documentation/leds/leds-class.txt linux-3.10-perso/Documentation/leds/leds-class.txt
> --- linux-3.10-orig/Documentation/leds/leds-class.txt   2013-04-29 13:27:16.959258613 +0200
> +++ linux-3.10-perso/Documentation/leds/leds-class.txt  2013-07-01 23:51:39.229318781 +0200
> @@ -2,9 +2,6 @@
>  LED handling under Linux
>  ========================
>
> -If you're reading this and thinking about keyboard leds, these are
> -handled by the input subsystem and the led class is *not* needed.
> -
>  In its simplest form, the LED class just allows control of LEDs from
>  userspace. LEDs appear in /sys/class/leds/. The maximum brightness of the
>  LED is defined in max_brightness file. The brightness file will set the brightness
> diff --exclude .svn -ur linux-3.10-orig/drivers/input/input.c linux-3.10-perso/drivers/input/input.c
> --- linux-3.10-orig/drivers/input/input.c       2013-04-29 13:27:16.959258613 +0200
> +++ linux-3.10-perso/drivers/input/input.c      2013-07-01 23:51:39.233318421 +0200
> @@ -28,6 +28,7 @@
>  #include <linux/mutex.h>
>  #include <linux/rcupdate.h>
>  #include "input-compat.h"
> +#include "leds.h"
>
>  MODULE_AUTHOR("Vojtech Pavlik <vojtech@suse.cz>");
>  MODULE_DESCRIPTION("Input core");
> @@ -708,6 +709,11 @@
>                 handle->open = 0;
>
>         spin_unlock_irq(&dev->event_lock);
> +
> +#ifdef CONFIG_INPUT_LEDS
> +       if (is_event_supported(EV_LED, dev->evbit, EV_MAX))
> +               input_led_disconnect(dev);
> +#endif

Please turn "input_led_disconnect()" into a dummy instead of #ifdef
around function calls. See below.

>  }
>
>  /**
> @@ -2092,6 +2098,11 @@
>
>         list_add_tail(&dev->node, &input_dev_list);
>
> +#ifdef CONFIG_INPUT_LEDS
> +       if (is_event_supported(EV_LED, dev->evbit, EV_MAX))
> +               input_led_connect(dev);
> +#endif
> +

Same as above.

>         list_for_each_entry(handler, &input_handler_list, node)
>                 input_attach_handler(dev, handler);
>
> diff --exclude .svn -ur linux-3.10-orig/drivers/input/Kconfig linux-3.10-perso/drivers/input/Kconfig
> --- linux-3.10-orig/drivers/input/Kconfig       2013-04-29 13:32:29.997497163 +0200
> +++ linux-3.10-perso/drivers/input/Kconfig      2013-07-01 23:51:39.233318421 +0200
> @@ -178,6 +178,15 @@
>
>  source "drivers/input/keyboard/Kconfig"
>
> +config INPUT_LEDS
> +       tristate "LED Support"
> +       depends on LEDS_CLASS
> +       select LEDS_TRIGGERS
> +       default y
> +       help
> +         This option enables support for the LEDs on keyboard managed

-the +keyboard_s_?

> +         by the input layer.
> +
>  source "drivers/input/mouse/Kconfig"
>
>  source "drivers/input/joystick/Kconfig"
> diff --exclude .svn -ur linux-3.10-orig/drivers/input/Makefile linux-3.10-perso/drivers/input/Makefile
> --- linux-3.10-orig/drivers/input/Makefile      2013-04-29 13:27:16.959258613 +0200
> +++ linux-3.10-perso/drivers/input/Makefile     2013-07-01 23:51:39.233318421 +0200
> @@ -18,6 +18,7 @@
>  obj-$(CONFIG_INPUT_EVBUG)      += evbug.o
>
>  obj-$(CONFIG_INPUT_KEYBOARD)   += keyboard/
> +obj-$(CONFIG_INPUT_LEDS)       += leds.o

Nitpick, but could you move it into the section above or below? This
section includes sub-directories only.

>  obj-$(CONFIG_INPUT_MOUSE)      += mouse/
>  obj-$(CONFIG_INPUT_JOYSTICK)   += joystick/
>  obj-$(CONFIG_INPUT_TABLET)     += tablet/
> diff --exclude .svn -ur linux-3.10-orig/drivers/leds/Kconfig linux-3.10-perso/drivers/leds/Kconfig
> --- linux-3.10-orig/drivers/leds/Kconfig        2013-07-01 01:56:00.335874214 +0200
> +++ linux-3.10-perso/drivers/leds/Kconfig       2013-07-01 23:51:39.233318421 +0200
> @@ -11,9 +11,6 @@
>           Say Y to enable Linux LED support.  This allows control of supported
>           LEDs from both userspace and optionally, by kernel events (triggers).
>
> -         This is not related to standard keyboard LEDs which are controlled
> -         via the input system.
> -
>  if NEW_LEDS
>
>  config LEDS_CLASS
> diff --exclude .svn -ur linux-3.10-orig/drivers/tty/Kconfig linux-3.10-perso/drivers/tty/Kconfig
> --- linux-3.10-orig/drivers/tty/Kconfig 2013-04-29 13:32:36.353298705 +0200
> +++ linux-3.10-perso/drivers/tty/Kconfig        2013-07-01 23:51:39.237318056 +0200
> @@ -13,6 +13,10 @@
>         bool "Virtual terminal" if EXPERT
>         depends on !S390 && !UML
>         select INPUT
> +       select NEW_LEDS
> +       select LEDS_CLASS
> +       select LEDS_TRIGGERS
> +       select INPUT_LEDS

This looks odd. Any dependency changes in the input-layer will break
this. But I guess that's what we get for a non-recursive "select". Hmm

>         default y
>         ---help---
>           If you say Y here, you will get support for terminal devices with
> diff --exclude .svn -ur linux-3.10-orig/drivers/tty/vt/keyboard.c linux-3.10-perso/drivers/tty/vt/keyboard.c
> --- linux-3.10-orig/drivers/tty/vt/keyboard.c   2013-04-29 13:32:36.681288463 +0200
> +++ linux-3.10-perso/drivers/tty/vt/keyboard.c  2013-07-01 23:51:39.237318056 +0200
> @@ -33,6 +33,7 @@
>  #include <linux/string.h>
>  #include <linux/init.h>
>  #include <linux/slab.h>
> +#include <linux/leds.h>
>
>  #include <linux/kbd_kern.h>
>  #include <linux/kbd_diacr.h>
> @@ -130,6 +131,7 @@
>  static int shift_state = 0;
>
>  static unsigned char ledstate = 0xff;                  /* undefined */
> +static unsigned char lockstate = 0xff;                 /* undefined */
>  static unsigned char ledioctl;
>
>  static struct ledptr {
> @@ -967,6 +969,32 @@
>         }
>  }
>
> +/* We route VT keyboard "leds" through triggers */
> +static void kbd_ledstate_trigger_activate(struct led_classdev *cdev);
> +static struct led_trigger ledtrig_ledstate[] = {
> +#define DEFINE_LEDSTATE_TRIGGER(kbd_led, nam) \
> +       [kbd_led] = { .name = nam, .activate = kbd_ledstate_trigger_activate, }
> +       DEFINE_LEDSTATE_TRIGGER(VC_SCROLLOCK, "scrollock"),
> +       DEFINE_LEDSTATE_TRIGGER(VC_NUMLOCK,   "numlock"),
> +       DEFINE_LEDSTATE_TRIGGER(VC_CAPSLOCK,  "capslock"),
> +       DEFINE_LEDSTATE_TRIGGER(VC_KANALOCK,  "kanalock"),
> +#undef DEFINE_LEDSTATE_TRIGGER
> +};
> +static void kbd_lockstate_trigger_activate(struct led_classdev *cdev);
> +static struct led_trigger ledtrig_lockstate[] = {
> +#define DEFINE_LOCKSTATE_TRIGGER(kbd_led, nam) \
> +       [kbd_led] = { .name = nam, .activate = kbd_lockstate_trigger_activate, }
> +       DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTLOCK,  "shiftlock"),
> +       DEFINE_LOCKSTATE_TRIGGER(VC_ALTGRLOCK,  "altgrlock"),
> +       DEFINE_LOCKSTATE_TRIGGER(VC_CTRLLOCK,   "ctrllock"),
> +       DEFINE_LOCKSTATE_TRIGGER(VC_ALTLOCK,    "altlock"),
> +       DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTLLOCK, "shiftllock"),
> +       DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTRLOCK, "shiftrlock"),
> +       DEFINE_LOCKSTATE_TRIGGER(VC_CTRLLLOCK,  "ctrlllock"),
> +       DEFINE_LOCKSTATE_TRIGGER(VC_CTRLRLOCK,  "ctrlrlock"),
> +#undef DEFINE_LOCKSTATE_TRIGGER
> +};
> +

We use empty lines between function-declarations and variable
declarations, I think. Would make this a lot easier to read. I also
think that the macros don't really simplify this. So:
  [VC_SHIFTLOCK] = { .name = "shiftlock", .activate =
kbd_lockstate_trigger_activate },
isn't much longer than:
  DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTLOCK,  "shiftlock"),
But I guess that's a matter of taste.

>  /*
>   * The leds display either (i) the status of NumLock, CapsLock, ScrollLock,
>   * or (ii) whatever pattern of lights people want to show using KDSETLED,
> @@ -1002,6 +1030,7 @@
>
>         leds = kbd->ledflagstate;
>
> +       /* TODO: should be replaced by a LED trigger */
>         if (kbd->ledmode == LED_SHOW_MEM) {
>                 for (i = 0; i < 3; i++)
>                         if (ledptrs[i].valid) {
> @@ -1014,18 +1043,24 @@
>         return leds;
>  }
>
> -static int kbd_update_leds_helper(struct input_handle *handle, void *data)
> +/* Called on trigger connection, to set initial state */
> +static void kbd_ledstate_trigger_activate(struct led_classdev *cdev)
>  {
> -       unsigned char leds = *(unsigned char *)data;
> +       struct led_trigger *trigger = cdev->trigger;
> +       int led = trigger - ledtrig_ledstate;
>
> -       if (test_bit(EV_LED, handle->dev->evbit)) {
> -               input_inject_event(handle, EV_LED, LED_SCROLLL, !!(leds & 0x01));
> -               input_inject_event(handle, EV_LED, LED_NUML,    !!(leds & 0x02));
> -               input_inject_event(handle, EV_LED, LED_CAPSL,   !!(leds & 0x04));
> -               input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
> -       }
> +       tasklet_disable(&keyboard_tasklet);
> +       led_trigger_event(trigger, ledstate & (1 << led) ? LED_FULL : LED_OFF);
> +       tasklet_enable(&keyboard_tasklet);
> +}
> +static void kbd_lockstate_trigger_activate(struct led_classdev *cdev)
> +{
> +       struct led_trigger *trigger = cdev->trigger;
> +       int led = trigger - ledtrig_lockstate;
>
> -       return 0;
> +       tasklet_disable(&keyboard_tasklet);
> +       led_trigger_event(trigger, lockstate & (1 << led) ? LED_FULL : LED_OFF);
> +       tasklet_enable(&keyboard_tasklet);
>  }

Again, please add empty lines between functions definitions.

>
>  /**
> @@ -1120,10 +1155,24 @@
>         spin_unlock_irqrestore(&led_lock, flags);
>
>         if (leds != ledstate) {
> -               input_handler_for_each_handle(&kbd_handler, &leds,
> -                                             kbd_update_leds_helper);
> +               int i;

Move "int i;" to the top. You use it again below. gcc is smart enough
to notice that both usages are independent.

> +               for (i = 0; i < ARRAY_SIZE(ledtrig_ledstate); i++)
> +                       if ((leds ^ ledstate) & (1 << i))
> +                               led_trigger_event(&ledtrig_ledstate[i],
> +                                               leds & (1 << i)
> +                                               ? LED_FULL : LED_OFF);
>                 ledstate = leds;
>         }
> +
> +       if (kbd->lockstate != lockstate) {
> +               int i;
> +               for (i = 0; i < ARRAY_SIZE(ledtrig_lockstate); i++)
> +                       if ((kbd->lockstate ^ lockstate) & (1 << i))
> +                               led_trigger_event(&ledtrig_lockstate[i],
> +                                               kbd->lockstate & (1 << i)
> +                                               ? LED_FULL : LED_OFF);
> +               lockstate = kbd->lockstate;
> +       }
>  }
>
>  DECLARE_TASKLET_DISABLED(keyboard_tasklet, kbd_bh, 0);
> @@ -1461,20 +1510,6 @@
>         kfree(handle);
>  }
>
> -/*
> - * Start keyboard handler on the new keyboard by refreshing LED state to
> - * match the rest of the system.
> - */
> -static void kbd_start(struct input_handle *handle)
> -{
> -       tasklet_disable(&keyboard_tasklet);
> -
> -       if (ledstate != 0xff)
> -               kbd_update_leds_helper(handle, &ledstate);
> -
> -       tasklet_enable(&keyboard_tasklet);
> -}
> -
>  static const struct input_device_id kbd_ids[] = {
>         {
>                 .flags = INPUT_DEVICE_ID_MATCH_EVBIT,
> @@ -1496,7 +1531,6 @@
>         .match          = kbd_match,
>         .connect        = kbd_connect,
>         .disconnect     = kbd_disconnect,
> -       .start          = kbd_start,
>         .name           = "kbd",
>         .id_table       = kbd_ids,
>  };
> @@ -1520,6 +1554,11 @@
>         if (error)
>                 return error;
>
> +       for (i = 0; i < ARRAY_SIZE(ledtrig_ledstate); i++)
> +               led_trigger_register(&ledtrig_ledstate[i]);
> +       for (i = 0; i < ARRAY_SIZE(ledtrig_lockstate); i++)
> +               led_trigger_register(&ledtrig_lockstate[i]);
> +

This returns "int", are you sure no error-handling is needed?

>         tasklet_enable(&keyboard_tasklet);
>         tasklet_schedule(&keyboard_tasklet);
>
> diff --exclude .svn -ur linux-3.10-orig/include/linux/input.h linux-3.10-perso/include/linux/input.h
> --- linux-3.10-orig/include/linux/input.h       2013-04-29 13:27:16.959258613 +0200
> +++ linux-3.10-perso/include/linux/input.h      2013-07-01 23:51:39.241317685 +0200
> @@ -164,6 +164,8 @@
>         unsigned long snd[BITS_TO_LONGS(SND_CNT)];
>         unsigned long sw[BITS_TO_LONGS(SW_CNT)];
>
> +       struct led_classdev *leds;
> +
>         int (*open)(struct input_dev *dev);
>         void (*close)(struct input_dev *dev);
>         int (*flush)(struct input_dev *dev, struct file *file);
> --- linux-3.10-orig/drivers/input/leds.h        1970-01-01 01:00:00.000000000 +0100
> +++ linux-3.10-perso/drivers/input/leds.h       2011-11-14 02:24:26.738456456 +0100
> @@ -0,0 +1,14 @@
> +/*
> + * LED support for the input layer
> + *
> + * Copyright 2010-2013 Samuel Thibault <samuel.thibault@ens-lyon.org>
> + *
> + * 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/input.h>
> +
> +extern int input_led_connect(struct input_dev *dev);
> +extern void input_led_disconnect(struct input_dev *dev);

Header-protection is missing. Besides, these 2 lines could be easily
merged into input.h. Any reason not to?
Also remove the "extern" before functions. It does not add any value
and we try to drop it for new declarations.

And you can avoid the #ifdef mentioned above if you use something like this:

#ifdef CONFIG_LEDS

int input_led_connect(struct input_dev *dev);
void input_led_disconnect(struct input_dev *dev);

#else

static inline int input_led_connect(struct input_dev *dev)
{
    return 0;
}

static inline void input_led_disconnect(struct input_dev *dev)
{
}

#endif

> --- linux-3.10-orig/drivers/input/leds.c        1970-01-01 01:00:00.000000000 +0100
> +++ linux-3.10-perso/drivers/input/leds.c       2011-11-14 03:23:07.578469395 +0100
> @@ -0,0 +1,243 @@
> +/*
> + * LED support for the input layer
> + *
> + * Copyright 2010-2013 Samuel Thibault <samuel.thibault@ens-lyon.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/leds.h>
> +#include <linux/input.h>

If you keep "input/leds.h" you should probably include it here. If you
drop it, you're fine.

> +
> +/*
> + * Keyboard LEDs are propagated by default like the following example:
> + *
> + * VT keyboard numlock trigger
> + * -> vt::numl VT LED
> + * -> vt-numl VT trigger

Nitpick: What's the reason for the syntax difference? "vt-numl" vs. "vt::numl"

> + * -> per-device inputx::numl LED

nitpick, but upper-case "X" is probably easier to understand. I had to
read it twice to get what the "x" means.

> + *
> + * Userland can however choose the trigger for the vt::numl LED, or
> + * independently choose the trigger for any inputx::numl LED.
> + */
> +
> +/* VT LED classes and triggers are registered on-demand according to
> + * existing LED devices */

Merge this comment with the comment above?

> +
> +/* Handler for VT LEDs, just triggers the corresponding VT trigger. */
> +static void vt_led_set(struct led_classdev *cdev,
> +                         enum led_brightness brightness);
> +static struct led_classdev vt_leds[LED_CNT] = {
> +#define DEFINE_INPUT_LED(vt_led, nam, deftrig) \
> +       [vt_led] = { \
> +               .name = "vt::"nam, \
> +               .max_brightness = 1, \
> +               .brightness_set = vt_led_set, \
> +               .default_trigger = deftrig, \
> +       }
> +/* Default triggers for the VT LEDs just correspond to the legacy
> + * usage. */
> +       DEFINE_INPUT_LED(LED_NUML, "numl", "numlock"),
> +       DEFINE_INPUT_LED(LED_CAPSL, "capsl", "capslock"),
> +       DEFINE_INPUT_LED(LED_SCROLLL, "scrolll", "scrollock"),
> +       DEFINE_INPUT_LED(LED_COMPOSE, "compose", NULL),
> +       DEFINE_INPUT_LED(LED_KANA, "kana", "kanalock"),
> +       DEFINE_INPUT_LED(LED_SLEEP, "sleep", NULL),
> +       DEFINE_INPUT_LED(LED_SUSPEND, "suspend", NULL),
> +       DEFINE_INPUT_LED(LED_MUTE, "mute", NULL),
> +       DEFINE_INPUT_LED(LED_MISC, "misc", NULL),
> +       DEFINE_INPUT_LED(LED_MAIL, "mail", NULL),
> +       DEFINE_INPUT_LED(LED_CHARGING, "charging", NULL),
> +};
> +static const char *const vt_led_names[LED_CNT] = {
> +       [LED_NUML] = "numl",
> +       [LED_CAPSL] = "capsl",
> +       [LED_SCROLLL] = "scrolll",
> +       [LED_COMPOSE] = "compose",
> +       [LED_KANA] = "kana",
> +       [LED_SLEEP] = "sleep",
> +       [LED_SUSPEND] = "suspend",
> +       [LED_MUTE] = "mute",
> +       [LED_MISC] = "misc",
> +       [LED_MAIL] = "mail",
> +       [LED_CHARGING] = "charging",
> +};
> +/* Handler for hotplug initialization */
> +static void vt_led_trigger_activate(struct led_classdev *cdev);
> +/* VT triggers */
> +static struct led_trigger vt_led_triggers[LED_CNT] = {
> +#define DEFINE_INPUT_LED_TRIGGER(vt_led, nam) \
> +       [vt_led] = { \
> +               .name = "vt-"nam, \
> +               .activate = vt_led_trigger_activate, \
> +       }
> +       DEFINE_INPUT_LED_TRIGGER(LED_NUML, "numl"),
> +       DEFINE_INPUT_LED_TRIGGER(LED_CAPSL, "capsl"),
> +       DEFINE_INPUT_LED_TRIGGER(LED_SCROLLL, "scrolll"),
> +       DEFINE_INPUT_LED_TRIGGER(LED_COMPOSE, "compose"),
> +       DEFINE_INPUT_LED_TRIGGER(LED_KANA, "kana"),
> +       DEFINE_INPUT_LED_TRIGGER(LED_SLEEP, "sleep"),
> +       DEFINE_INPUT_LED_TRIGGER(LED_SUSPEND, "suspend"),
> +       DEFINE_INPUT_LED_TRIGGER(LED_MUTE, "mute"),
> +       DEFINE_INPUT_LED_TRIGGER(LED_MISC, "misc"),
> +       DEFINE_INPUT_LED_TRIGGER(LED_MAIL, "mail"),
> +       DEFINE_INPUT_LED_TRIGGER(LED_CHARGING, "charging"),
> +};
> +/* Lock for registration coherency */
> +static DEFINE_MUTEX(vt_led_registered_lock);
> +/* Which VT LED classes and triggers are registered */
> +static unsigned long vt_led_registered[BITS_TO_LONGS(LED_CNT)];
> +/* Number of input devices having each LED */
> +static int vt_led_references[LED_CNT];

Please add some empty lines here. At least the different static arrays
should be separated by an empty newline.

> +
> +/* VT LED state change, tell the VT trigger.  */
> +static void vt_led_set(struct led_classdev *cdev,
> +                         enum led_brightness brightness)
> +{
> +       int led = cdev - vt_leds;
> +
> +       led_trigger_event(&vt_led_triggers[led], !!brightness);
> +}
> +
> +/* LED state change for some keyboard, notify that keyboard.  */
> +static void perdevice_input_led_set(struct led_classdev *cdev,
> +                         enum led_brightness brightness)
> +{
> +       struct input_dev *dev;
> +       struct led_classdev *leds;
> +       int led;
> +
> +       dev = cdev->dev->platform_data;
> +       if (!dev)
> +               /* Still initializing */
> +               return;
> +       leds = dev->leds;
> +       led = cdev - leds;
> +
> +       input_event(dev, EV_LED, led, !!brightness);
> +       input_event(dev, EV_SYN, SYN_REPORT, 0);
> +}
> +
> +/* Keyboard hotplug, initialize its LED status */
> +static void vt_led_trigger_activate(struct led_classdev *cdev)
> +{
> +       struct led_trigger *trigger = cdev->trigger;
> +       int led = trigger - vt_led_triggers;
> +
> +       if (cdev->brightness_set)
> +               cdev->brightness_set(cdev, vt_leds[led].brightness);
> +}
> +
> +/* Free led stuff from input device, used at abortion and disconnection.  */
> +static void input_led_delete(struct input_dev *dev)
> +{
> +       if (dev) {
> +               struct led_classdev *leds = dev->leds;
> +               if (leds) {
> +                       int i;
> +                       for (i = 0; i < LED_CNT; i++)
> +                               kfree(leds[i].name);
> +                       kfree(leds);
> +                       dev->leds = NULL;
> +               }
> +       }
> +}
> +
> +/* A new input device with potential LEDs to connect.  */
> +int input_led_connect(struct input_dev *dev)
> +{
> +       int i, error = 0;
> +       struct led_classdev *leds;
> +
> +       dev->leds = leds = kzalloc(sizeof(*leds) * LED_CNT, GFP_KERNEL);
> +       if (!dev->leds)
> +               return -ENOMEM;
> +
> +       /* lazily register missing VT LEDs */
> +       mutex_lock(&vt_led_registered_lock);
> +       for (i = 0; i < LED_CNT; i++)
> +               if (vt_leds[i].name && test_bit(i, dev->ledbit)) {
> +                       if (!vt_led_references[i]) {
> +                               led_trigger_register(&vt_led_triggers[i]);
> +                               /* This keyboard is first to have led i,
> +                                * try to register it */
> +                               if (!led_classdev_register(NULL, &vt_leds[i]))
> +                                       vt_led_references[i] = 1;
> +                               else
> +                                       led_trigger_unregister(&vt_led_triggers[i]);
> +                       } else
> +                               vt_led_references[i]++;
> +               }
> +       mutex_unlock(&vt_led_registered_lock);
> +
> +       /* and register this device's LEDs */
> +       for (i = 0; i < LED_CNT; i++)
> +               if (vt_leds[i].name && test_bit(i, dev->ledbit)) {
> +                       leds[i].name = kasprintf(GFP_KERNEL, "%s::%s",
> +                                               dev_name(&dev->dev),
> +                                               vt_led_names[i]);
> +                       if (!leds[i].name) {
> +                               error = -ENOMEM;
> +                               goto err;
> +                       }
> +                       leds[i].max_brightness = 1;
> +                       leds[i].brightness_set = perdevice_input_led_set;
> +                       leds[i].default_trigger = vt_led_triggers[i].name;
> +               }
> +
> +       /* No issue so far, we can register for real.  */
> +       for (i = 0; i < LED_CNT; i++)
> +               if (leds[i].name) {
> +                       led_classdev_register(&dev->dev, &leds[i]);
> +                       leds[i].dev->platform_data = dev;
> +                       perdevice_input_led_set(&leds[i],
> +                                       vt_leds[i].brightness);
> +               }
> +
> +       return 0;
> +
> +err:
> +       input_led_delete(dev);
> +       return error;
> +}
> +
> +/* Disconnected input device. Clean it, and deregister now-useless VT LEDs
> + * and triggers.  */
> +extern void input_led_disconnect(struct input_dev *dev)
> +{
> +       int i;
> +       struct led_classdev *leds = dev->leds;
> +
> +       for (i = 0; i < LED_CNT; i++)
> +               if (leds[i].name)
> +                       led_classdev_unregister(&leds[i]);
> +
> +       input_led_delete(dev);
> +
> +       mutex_lock(&vt_led_registered_lock);
> +       for (i = 0; i < LED_CNT; i++) {
> +               if (!vt_leds[i].name || !test_bit(i, dev->ledbit))
> +                       continue;
> +
> +               vt_led_references[i]--;
> +               if (vt_led_references[i]) {
> +                       /* Still some devices needing it */
> +                       continue;
> +               }
> +
> +               led_classdev_unregister(&vt_leds[i]);
> +               led_trigger_unregister(&vt_led_triggers[i]);
> +               clear_bit(i, vt_led_registered);
> +       }
> +       mutex_unlock(&vt_led_registered_lock);
> +}
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("User LED support for input layer");
> +MODULE_AUTHOR("Samuel Thibault <samuel.thibault@ens-lyon.org>");

I haven't done any locking review, yet, and some of the comments are
probably just a matter of taste. But the patch looks good.

Cheers
David

> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] Route kbd LEDs through the generic LEDs layer

[Peter Korsgaard]

>>>>> "Samuel" == Samuel Thibault <samuel.thibault@ens-lyon.org> writes:

 Samuel> This permits to reassign keyboard LEDs to something else than keyboard "leds"
 Samuel> state, by adding keyboard led and modifier triggers connected to a series
 Samuel> of VT input LEDs, themselves connected to VT input triggers, which
 Samuel> per-input device LEDs use by default.  Userland can thus easily change the LED
 Samuel> behavior of (a priori) all input devices, or of particular input devices.

 Samuel> This also permits to fix #7063 from userland by using a modifier to implement
 Samuel> proper CapsLock behavior and have the keyboard caps lock led show that modifier
 Samuel> state.

 Samuel> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
 Samuel> [ebroder@mokafive.com: Rebased to 3.2-rc1 or so, cleaned up some includes, and fixed some constants]
 Samuel> Signed-off-by: Evan Broder <evan@ebroder.net>

Looks good to me. Samuel, thanks for picking this up again.

Acked-by: Peter Korsgaard <jacmet@sunsite.dk>

-- 
Bye, Peter Korsgaard

Re: [PATCH] Route kbd LEDs through the generic LEDs layer

[Pavel Machek]

Hi!

> > > > > This permits to reassign keyboard LEDs to something else than keyboard "leds"
> > > > > state, by adding keyboard led and modifier triggers connected to a series
> > > > > of VT input LEDs, themselves connected to VT input triggers, which
> > > > > per-input device LEDs use by default.  Userland can thus easily change the LED
> > > > > behavior of (a priori) all input devices, or of particular input devices.
> > > > 
> > > > Nice! Leds now have proper /sys interface.
> > > > 
> > > > But... I boot up, switch from X to console, press capslock, and no
> > > > reaction anywhere.
> > > 
> > > Is it working without the patch?  Console-setup for instance is known to
> > > have broken the capslock LED, which is precisely one of the reasons for
> > > this patch, which will provide console-setup with a way to bring back
> > > caps lock working properly.
> > 
> > You are right, it was broken before.
> 
> Ok. You then may want fix your setup by configuring your caps lock led
> the way console-setup will be supposed to do in the future:
> 
> echo ctrlllock > /sys/class/leds/vt::capsl/trigger

And this indeed makes capslock led work on console, again. Nice!

Richard, could we get this applied for 3.12 or something? It makes
keyboard LEDs sane...

Thanks,
									Pavel

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

Re: [PATCH] Route kbd LEDs through the generic LEDs layer

[Samuel Thibault]

Pavel Machek, le Sat 13 Jul 2013 01:33:01 +0200, a écrit :
> > > > This permits to reassign keyboard LEDs to something else than keyboard "leds"
> > > > state, by adding keyboard led and modifier triggers connected to a series
> > > > of VT input LEDs, themselves connected to VT input triggers, which
> > > > per-input device LEDs use by default.  Userland can thus easily change the LED
> > > > behavior of (a priori) all input devices, or of particular input devices.
> > > 
> > > Nice! Leds now have proper /sys interface.
> > > 
> > > But... I boot up, switch from X to console, press capslock, and no
> > > reaction anywhere.
> > 
> > Is it working without the patch?  Console-setup for instance is known to
> > have broken the capslock LED, which is precisely one of the reasons for
> > this patch, which will provide console-setup with a way to bring back
> > caps lock working properly.
> 
> You are right, it was broken before.

Ok. You then may want fix your setup by configuring your caps lock led
the way console-setup will be supposed to do in the future:

echo ctrlllock > /sys/class/leds/vt::capsl/trigger

Or some other of the locks that console-setup might be using to
implement caps lock.

> > Things work fine with my USB keyboard too, is this perhaps using an odd
> > driver which would not expose LEDs in a standard way?
> 
> No, everything works as well as it did. Feel free to add:
> 
> Tested-by: Pavel Machek <pavel@ucw.cz>

Thanks for testing,
Samuel

Re: [PATCH] Route kbd LEDs through the generic LEDs layer

[Pavel Machek]

Hi!

> > > This permits to reassign keyboard LEDs to something else than keyboard "leds"
> > > state, by adding keyboard led and modifier triggers connected to a series
> > > of VT input LEDs, themselves connected to VT input triggers, which
> > > per-input device LEDs use by default.  Userland can thus easily change the LED
> > > behavior of (a priori) all input devices, or of particular input devices.
> > 
> > Nice! Leds now have proper /sys interface.
> > 
> > But... I boot up, switch from X to console, press capslock, and no
> > reaction anywhere.
> 
> Is it working without the patch?  Console-setup for instance is known to
> have broken the capslock LED, which is precisely one of the reasons for
> this patch, which will provide console-setup with a way to bring back
> caps lock working properly.

You are right, it was broken before.

> At any rate, please provide way more information about your keyboard
> and LED configuration (output of dumpkeys, dmesg, content of
> /sys/class/leds/*/trigger, etc.), as things are just working fine for me
> (just like it has been for the past two years).

Well... I just verified, and it works "as well as before", which it
should.

> > Note that this is notebook with usb keyboard plugged in (and two
> > monitors), but I believe this worked before...
> 
> Things work fine with my USB keyboard too, is this perhaps using an odd
> driver which would not expose LEDs in a standard way?

No, everything works as well as it did. Feel free to add:

Tested-by: Pavel Machek <pavel@ucw.cz>

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

Re: [PATCH] Route kbd LEDs through the generic LEDs layer

[Samuel Thibault]

Pavel Machek, le Fri 12 Jul 2013 13:36:56 +0200, a écrit :
> > This permits to reassign keyboard LEDs to something else than keyboard "leds"
> > state, by adding keyboard led and modifier triggers connected to a series
> > of VT input LEDs, themselves connected to VT input triggers, which
> > per-input device LEDs use by default.  Userland can thus easily change the LED
> > behavior of (a priori) all input devices, or of particular input devices.
> 
> Nice! Leds now have proper /sys interface.
> 
> But... I boot up, switch from X to console, press capslock, and no
> reaction anywhere.

Is it working without the patch?  Console-setup for instance is known to
have broken the capslock LED, which is precisely one of the reasons for
this patch, which will provide console-setup with a way to bring back
caps lock working properly.

At any rate, please provide way more information about your keyboard
and LED configuration (output of dumpkeys, dmesg, content of
/sys/class/leds/*/trigger, etc.), as things are just working fine for me
(just like it has been for the past two years).

> Note that this is notebook with usb keyboard plugged in (and two
> monitors), but I believe this worked before...

Things work fine with my USB keyboard too, is this perhaps using an odd
driver which would not expose LEDs in a standard way?

Samuel

Re: [PATCH] Route kbd LEDs through the generic LEDs layer

[Pavel Machek]

Hi!

> This permits to reassign keyboard LEDs to something else than keyboard "leds"
> state, by adding keyboard led and modifier triggers connected to a series
> of VT input LEDs, themselves connected to VT input triggers, which
> per-input device LEDs use by default.  Userland can thus easily change the LED
> behavior of (a priori) all input devices, or of particular input devices.

Nice! Leds now have proper /sys interface.

But... I boot up, switch from X to console, press capslock, and no
reaction anywhere.

Note that this is notebook with usb keyboard plugged in (and two
monitors), but I believe this worked before...

Thanks,
									Pavel

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

[PATCH] Route kbd LEDs through the generic LEDs layer

[Samuel Thibault]

This permits to reassign keyboard LEDs to something else than keyboard "leds"
state, by adding keyboard led and modifier triggers connected to a series
of VT input LEDs, themselves connected to VT input triggers, which
per-input device LEDs use by default.  Userland can thus easily change the LED
behavior of (a priori) all input devices, or of particular input devices.

This also permits to fix #7063 from userland by using a modifier to implement
proper CapsLock behavior and have the keyboard caps lock led show that modifier
state.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
[ebroder@mokafive.com: Rebased to 3.2-rc1 or so, cleaned up some includes, and fixed some constants]
Signed-off-by: Evan Broder <evan@ebroder.net>

diff --exclude .svn -ur linux-3.10-orig/Documentation/leds/leds-class.txt linux-3.10-perso/Documentation/leds/leds-class.txt
--- linux-3.10-orig/Documentation/leds/leds-class.txt	2013-04-29 13:27:16.959258613 +0200
+++ linux-3.10-perso/Documentation/leds/leds-class.txt	2013-07-01 23:51:39.229318781 +0200
@@ -2,9 +2,6 @@
 LED handling under Linux
 ========================
 
-If you're reading this and thinking about keyboard leds, these are
-handled by the input subsystem and the led class is *not* needed.
-
 In its simplest form, the LED class just allows control of LEDs from
 userspace. LEDs appear in /sys/class/leds/. The maximum brightness of the
 LED is defined in max_brightness file. The brightness file will set the brightness
diff --exclude .svn -ur linux-3.10-orig/drivers/input/input.c linux-3.10-perso/drivers/input/input.c
--- linux-3.10-orig/drivers/input/input.c	2013-04-29 13:27:16.959258613 +0200
+++ linux-3.10-perso/drivers/input/input.c	2013-07-01 23:51:39.233318421 +0200
@@ -28,6 +28,7 @@
 #include <linux/mutex.h>
 #include <linux/rcupdate.h>
 #include "input-compat.h"
+#include "leds.h"
 
 MODULE_AUTHOR("Vojtech Pavlik <vojtech@suse.cz>");
 MODULE_DESCRIPTION("Input core");
@@ -708,6 +709,11 @@
 		handle->open = 0;
 
 	spin_unlock_irq(&dev->event_lock);
+
+#ifdef CONFIG_INPUT_LEDS
+	if (is_event_supported(EV_LED, dev->evbit, EV_MAX))
+		input_led_disconnect(dev);
+#endif
 }
 
 /**
@@ -2092,6 +2098,11 @@
 
 	list_add_tail(&dev->node, &input_dev_list);
 
+#ifdef CONFIG_INPUT_LEDS
+	if (is_event_supported(EV_LED, dev->evbit, EV_MAX))
+		input_led_connect(dev);
+#endif
+
 	list_for_each_entry(handler, &input_handler_list, node)
 		input_attach_handler(dev, handler);
 
diff --exclude .svn -ur linux-3.10-orig/drivers/input/Kconfig linux-3.10-perso/drivers/input/Kconfig
--- linux-3.10-orig/drivers/input/Kconfig	2013-04-29 13:32:29.997497163 +0200
+++ linux-3.10-perso/drivers/input/Kconfig	2013-07-01 23:51:39.233318421 +0200
@@ -178,6 +178,15 @@
 
 source "drivers/input/keyboard/Kconfig"
 
+config INPUT_LEDS
+	tristate "LED Support"
+	depends on LEDS_CLASS
+	select LEDS_TRIGGERS
+	default y
+	help
+	  This option enables support for the LEDs on keyboard managed
+	  by the input layer.
+
 source "drivers/input/mouse/Kconfig"
 
 source "drivers/input/joystick/Kconfig"
diff --exclude .svn -ur linux-3.10-orig/drivers/input/Makefile linux-3.10-perso/drivers/input/Makefile
--- linux-3.10-orig/drivers/input/Makefile	2013-04-29 13:27:16.959258613 +0200
+++ linux-3.10-perso/drivers/input/Makefile	2013-07-01 23:51:39.233318421 +0200
@@ -18,6 +18,7 @@
 obj-$(CONFIG_INPUT_EVBUG)	+= evbug.o
 
 obj-$(CONFIG_INPUT_KEYBOARD)	+= keyboard/
+obj-$(CONFIG_INPUT_LEDS)	+= leds.o
 obj-$(CONFIG_INPUT_MOUSE)	+= mouse/
 obj-$(CONFIG_INPUT_JOYSTICK)	+= joystick/
 obj-$(CONFIG_INPUT_TABLET)	+= tablet/
diff --exclude .svn -ur linux-3.10-orig/drivers/leds/Kconfig linux-3.10-perso/drivers/leds/Kconfig
--- linux-3.10-orig/drivers/leds/Kconfig	2013-07-01 01:56:00.335874214 +0200
+++ linux-3.10-perso/drivers/leds/Kconfig	2013-07-01 23:51:39.233318421 +0200
@@ -11,9 +11,6 @@
 	  Say Y to enable Linux LED support.  This allows control of supported
 	  LEDs from both userspace and optionally, by kernel events (triggers).
 
-	  This is not related to standard keyboard LEDs which are controlled
-	  via the input system.
-
 if NEW_LEDS
 
 config LEDS_CLASS
diff --exclude .svn -ur linux-3.10-orig/drivers/tty/Kconfig linux-3.10-perso/drivers/tty/Kconfig
--- linux-3.10-orig/drivers/tty/Kconfig	2013-04-29 13:32:36.353298705 +0200
+++ linux-3.10-perso/drivers/tty/Kconfig	2013-07-01 23:51:39.237318056 +0200
@@ -13,6 +13,10 @@
 	bool "Virtual terminal" if EXPERT
 	depends on !S390 && !UML
 	select INPUT
+	select NEW_LEDS
+	select LEDS_CLASS
+	select LEDS_TRIGGERS
+	select INPUT_LEDS
 	default y
 	---help---
 	  If you say Y here, you will get support for terminal devices with
diff --exclude .svn -ur linux-3.10-orig/drivers/tty/vt/keyboard.c linux-3.10-perso/drivers/tty/vt/keyboard.c
--- linux-3.10-orig/drivers/tty/vt/keyboard.c	2013-04-29 13:32:36.681288463 +0200
+++ linux-3.10-perso/drivers/tty/vt/keyboard.c	2013-07-01 23:51:39.237318056 +0200
@@ -33,6 +33,7 @@
 #include <linux/string.h>
 #include <linux/init.h>
 #include <linux/slab.h>
+#include <linux/leds.h>
 
 #include <linux/kbd_kern.h>
 #include <linux/kbd_diacr.h>
@@ -130,6 +131,7 @@
 static int shift_state = 0;
 
 static unsigned char ledstate = 0xff;			/* undefined */
+static unsigned char lockstate = 0xff;			/* undefined */
 static unsigned char ledioctl;
 
 static struct ledptr {
@@ -967,6 +969,32 @@
 	}
 }
 
+/* We route VT keyboard "leds" through triggers */
+static void kbd_ledstate_trigger_activate(struct led_classdev *cdev);
+static struct led_trigger ledtrig_ledstate[] = {
+#define DEFINE_LEDSTATE_TRIGGER(kbd_led, nam) \
+	[kbd_led] = { .name = nam, .activate = kbd_ledstate_trigger_activate, }
+	DEFINE_LEDSTATE_TRIGGER(VC_SCROLLOCK, "scrollock"),
+	DEFINE_LEDSTATE_TRIGGER(VC_NUMLOCK,   "numlock"),
+	DEFINE_LEDSTATE_TRIGGER(VC_CAPSLOCK,  "capslock"),
+	DEFINE_LEDSTATE_TRIGGER(VC_KANALOCK,  "kanalock"),
+#undef DEFINE_LEDSTATE_TRIGGER
+};
+static void kbd_lockstate_trigger_activate(struct led_classdev *cdev);
+static struct led_trigger ledtrig_lockstate[] = {
+#define DEFINE_LOCKSTATE_TRIGGER(kbd_led, nam) \
+	[kbd_led] = { .name = nam, .activate = kbd_lockstate_trigger_activate, }
+	DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTLOCK,  "shiftlock"),
+	DEFINE_LOCKSTATE_TRIGGER(VC_ALTGRLOCK,  "altgrlock"),
+	DEFINE_LOCKSTATE_TRIGGER(VC_CTRLLOCK,   "ctrllock"),
+	DEFINE_LOCKSTATE_TRIGGER(VC_ALTLOCK,    "altlock"),
+	DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTLLOCK, "shiftllock"),
+	DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTRLOCK, "shiftrlock"),
+	DEFINE_LOCKSTATE_TRIGGER(VC_CTRLLLOCK,  "ctrlllock"),
+	DEFINE_LOCKSTATE_TRIGGER(VC_CTRLRLOCK,  "ctrlrlock"),
+#undef DEFINE_LOCKSTATE_TRIGGER
+};
+
 /*
  * The leds display either (i) the status of NumLock, CapsLock, ScrollLock,
  * or (ii) whatever pattern of lights people want to show using KDSETLED,
@@ -1002,6 +1030,7 @@
 
 	leds = kbd->ledflagstate;
 
+	/* TODO: should be replaced by a LED trigger */
 	if (kbd->ledmode == LED_SHOW_MEM) {
 		for (i = 0; i < 3; i++)
 			if (ledptrs[i].valid) {
@@ -1014,18 +1043,24 @@
 	return leds;
 }
 
-static int kbd_update_leds_helper(struct input_handle *handle, void *data)
+/* Called on trigger connection, to set initial state */
+static void kbd_ledstate_trigger_activate(struct led_classdev *cdev)
 {
-	unsigned char leds = *(unsigned char *)data;
+	struct led_trigger *trigger = cdev->trigger;
+	int led = trigger - ledtrig_ledstate;
 
-	if (test_bit(EV_LED, handle->dev->evbit)) {
-		input_inject_event(handle, EV_LED, LED_SCROLLL, !!(leds & 0x01));
-		input_inject_event(handle, EV_LED, LED_NUML,    !!(leds & 0x02));
-		input_inject_event(handle, EV_LED, LED_CAPSL,   !!(leds & 0x04));
-		input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
-	}
+	tasklet_disable(&keyboard_tasklet);
+	led_trigger_event(trigger, ledstate & (1 << led) ? LED_FULL : LED_OFF);
+	tasklet_enable(&keyboard_tasklet);
+}
+static void kbd_lockstate_trigger_activate(struct led_classdev *cdev)
+{
+	struct led_trigger *trigger = cdev->trigger;
+	int led = trigger - ledtrig_lockstate;
 
-	return 0;
+	tasklet_disable(&keyboard_tasklet);
+	led_trigger_event(trigger, lockstate & (1 << led) ? LED_FULL : LED_OFF);
+	tasklet_enable(&keyboard_tasklet);
 }
 
 /**
@@ -1120,10 +1155,24 @@
 	spin_unlock_irqrestore(&led_lock, flags);
 
 	if (leds != ledstate) {
-		input_handler_for_each_handle(&kbd_handler, &leds,
-					      kbd_update_leds_helper);
+		int i;
+		for (i = 0; i < ARRAY_SIZE(ledtrig_ledstate); i++)
+			if ((leds ^ ledstate) & (1 << i))
+				led_trigger_event(&ledtrig_ledstate[i],
+						leds & (1 << i)
+						? LED_FULL : LED_OFF);
 		ledstate = leds;
 	}
+
+	if (kbd->lockstate != lockstate) {
+		int i;
+		for (i = 0; i < ARRAY_SIZE(ledtrig_lockstate); i++)
+			if ((kbd->lockstate ^ lockstate) & (1 << i))
+				led_trigger_event(&ledtrig_lockstate[i],
+						kbd->lockstate & (1 << i)
+						? LED_FULL : LED_OFF);
+		lockstate = kbd->lockstate;
+	}
 }
 
 DECLARE_TASKLET_DISABLED(keyboard_tasklet, kbd_bh, 0);
@@ -1461,20 +1510,6 @@
 	kfree(handle);
 }
 
-/*
- * Start keyboard handler on the new keyboard by refreshing LED state to
- * match the rest of the system.
- */
-static void kbd_start(struct input_handle *handle)
-{
-	tasklet_disable(&keyboard_tasklet);
-
-	if (ledstate != 0xff)
-		kbd_update_leds_helper(handle, &ledstate);
-
-	tasklet_enable(&keyboard_tasklet);
-}
-
 static const struct input_device_id kbd_ids[] = {
 	{
 		.flags = INPUT_DEVICE_ID_MATCH_EVBIT,
@@ -1496,7 +1531,6 @@
 	.match		= kbd_match,
 	.connect	= kbd_connect,
 	.disconnect	= kbd_disconnect,
-	.start		= kbd_start,
 	.name		= "kbd",
 	.id_table	= kbd_ids,
 };
@@ -1520,6 +1554,11 @@
 	if (error)
 		return error;
 
+	for (i = 0; i < ARRAY_SIZE(ledtrig_ledstate); i++)
+		led_trigger_register(&ledtrig_ledstate[i]);
+	for (i = 0; i < ARRAY_SIZE(ledtrig_lockstate); i++)
+		led_trigger_register(&ledtrig_lockstate[i]);
+
 	tasklet_enable(&keyboard_tasklet);
 	tasklet_schedule(&keyboard_tasklet);
 
diff --exclude .svn -ur linux-3.10-orig/include/linux/input.h linux-3.10-perso/include/linux/input.h
--- linux-3.10-orig/include/linux/input.h	2013-04-29 13:27:16.959258613 +0200
+++ linux-3.10-perso/include/linux/input.h	2013-07-01 23:51:39.241317685 +0200
@@ -164,6 +164,8 @@
 	unsigned long snd[BITS_TO_LONGS(SND_CNT)];
 	unsigned long sw[BITS_TO_LONGS(SW_CNT)];
 
+	struct led_classdev *leds;
+
 	int (*open)(struct input_dev *dev);
 	void (*close)(struct input_dev *dev);
 	int (*flush)(struct input_dev *dev, struct file *file);
--- linux-3.10-orig/drivers/input/leds.h	1970-01-01 01:00:00.000000000 +0100
+++ linux-3.10-perso/drivers/input/leds.h	2011-11-14 02:24:26.738456456 +0100
@@ -0,0 +1,14 @@
+/*
+ * LED support for the input layer
+ *
+ * Copyright 2010-2013 Samuel Thibault <samuel.thibault@ens-lyon.org>
+ *
+ * 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/input.h>
+
+extern int input_led_connect(struct input_dev *dev);
+extern void input_led_disconnect(struct input_dev *dev);
--- linux-3.10-orig/drivers/input/leds.c	1970-01-01 01:00:00.000000000 +0100
+++ linux-3.10-perso/drivers/input/leds.c	2011-11-14 03:23:07.578469395 +0100
@@ -0,0 +1,243 @@
+/*
+ * LED support for the input layer
+ *
+ * Copyright 2010-2013 Samuel Thibault <samuel.thibault@ens-lyon.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include <linux/input.h>
+
+/*
+ * Keyboard LEDs are propagated by default like the following example:
+ *
+ * VT keyboard numlock trigger
+ * -> vt::numl VT LED
+ * -> vt-numl VT trigger
+ * -> per-device inputx::numl LED
+ *
+ * Userland can however choose the trigger for the vt::numl LED, or
+ * independently choose the trigger for any inputx::numl LED.
+ */
+
+/* VT LED classes and triggers are registered on-demand according to
+ * existing LED devices */
+
+/* Handler for VT LEDs, just triggers the corresponding VT trigger. */
+static void vt_led_set(struct led_classdev *cdev,
+			  enum led_brightness brightness);
+static struct led_classdev vt_leds[LED_CNT] = {
+#define DEFINE_INPUT_LED(vt_led, nam, deftrig) \
+	[vt_led] = { \
+		.name = "vt::"nam, \
+		.max_brightness = 1, \
+		.brightness_set = vt_led_set, \
+		.default_trigger = deftrig, \
+	}
+/* Default triggers for the VT LEDs just correspond to the legacy
+ * usage. */
+	DEFINE_INPUT_LED(LED_NUML, "numl", "numlock"),
+	DEFINE_INPUT_LED(LED_CAPSL, "capsl", "capslock"),
+	DEFINE_INPUT_LED(LED_SCROLLL, "scrolll", "scrollock"),
+	DEFINE_INPUT_LED(LED_COMPOSE, "compose", NULL),
+	DEFINE_INPUT_LED(LED_KANA, "kana", "kanalock"),
+	DEFINE_INPUT_LED(LED_SLEEP, "sleep", NULL),
+	DEFINE_INPUT_LED(LED_SUSPEND, "suspend", NULL),
+	DEFINE_INPUT_LED(LED_MUTE, "mute", NULL),
+	DEFINE_INPUT_LED(LED_MISC, "misc", NULL),
+	DEFINE_INPUT_LED(LED_MAIL, "mail", NULL),
+	DEFINE_INPUT_LED(LED_CHARGING, "charging", NULL),
+};
+static const char *const vt_led_names[LED_CNT] = {
+	[LED_NUML] = "numl",
+	[LED_CAPSL] = "capsl",
+	[LED_SCROLLL] = "scrolll",
+	[LED_COMPOSE] = "compose",
+	[LED_KANA] = "kana",
+	[LED_SLEEP] = "sleep",
+	[LED_SUSPEND] = "suspend",
+	[LED_MUTE] = "mute",
+	[LED_MISC] = "misc",
+	[LED_MAIL] = "mail",
+	[LED_CHARGING] = "charging",
+};
+/* Handler for hotplug initialization */
+static void vt_led_trigger_activate(struct led_classdev *cdev);
+/* VT triggers */
+static struct led_trigger vt_led_triggers[LED_CNT] = {
+#define DEFINE_INPUT_LED_TRIGGER(vt_led, nam) \
+	[vt_led] = { \
+		.name = "vt-"nam, \
+		.activate = vt_led_trigger_activate, \
+	}
+	DEFINE_INPUT_LED_TRIGGER(LED_NUML, "numl"),
+	DEFINE_INPUT_LED_TRIGGER(LED_CAPSL, "capsl"),
+	DEFINE_INPUT_LED_TRIGGER(LED_SCROLLL, "scrolll"),
+	DEFINE_INPUT_LED_TRIGGER(LED_COMPOSE, "compose"),
+	DEFINE_INPUT_LED_TRIGGER(LED_KANA, "kana"),
+	DEFINE_INPUT_LED_TRIGGER(LED_SLEEP, "sleep"),
+	DEFINE_INPUT_LED_TRIGGER(LED_SUSPEND, "suspend"),
+	DEFINE_INPUT_LED_TRIGGER(LED_MUTE, "mute"),
+	DEFINE_INPUT_LED_TRIGGER(LED_MISC, "misc"),
+	DEFINE_INPUT_LED_TRIGGER(LED_MAIL, "mail"),
+	DEFINE_INPUT_LED_TRIGGER(LED_CHARGING, "charging"),
+};
+/* Lock for registration coherency */
+static DEFINE_MUTEX(vt_led_registered_lock);
+/* Which VT LED classes and triggers are registered */
+static unsigned long vt_led_registered[BITS_TO_LONGS(LED_CNT)];
+/* Number of input devices having each LED */
+static int vt_led_references[LED_CNT];
+
+/* VT LED state change, tell the VT trigger.  */
+static void vt_led_set(struct led_classdev *cdev,
+			  enum led_brightness brightness)
+{
+	int led = cdev - vt_leds;
+
+	led_trigger_event(&vt_led_triggers[led], !!brightness);
+}
+
+/* LED state change for some keyboard, notify that keyboard.  */
+static void perdevice_input_led_set(struct led_classdev *cdev,
+			  enum led_brightness brightness)
+{
+	struct input_dev *dev;
+	struct led_classdev *leds;
+	int led;
+
+	dev = cdev->dev->platform_data;
+	if (!dev)
+		/* Still initializing */
+		return;
+	leds = dev->leds;
+	led = cdev - leds;
+
+	input_event(dev, EV_LED, led, !!brightness);
+	input_event(dev, EV_SYN, SYN_REPORT, 0);
+}
+
+/* Keyboard hotplug, initialize its LED status */
+static void vt_led_trigger_activate(struct led_classdev *cdev)
+{
+	struct led_trigger *trigger = cdev->trigger;
+	int led = trigger - vt_led_triggers;
+
+	if (cdev->brightness_set)
+		cdev->brightness_set(cdev, vt_leds[led].brightness);
+}
+
+/* Free led stuff from input device, used at abortion and disconnection.  */
+static void input_led_delete(struct input_dev *dev)
+{
+	if (dev) {
+		struct led_classdev *leds = dev->leds;
+		if (leds) {
+			int i;
+			for (i = 0; i < LED_CNT; i++)
+				kfree(leds[i].name);
+			kfree(leds);
+			dev->leds = NULL;
+		}
+	}
+}
+
+/* A new input device with potential LEDs to connect.  */
+int input_led_connect(struct input_dev *dev)
+{
+	int i, error = 0;
+	struct led_classdev *leds;
+
+	dev->leds = leds = kzalloc(sizeof(*leds) * LED_CNT, GFP_KERNEL);
+	if (!dev->leds)
+		return -ENOMEM;
+
+	/* lazily register missing VT LEDs */
+	mutex_lock(&vt_led_registered_lock);
+	for (i = 0; i < LED_CNT; i++)
+		if (vt_leds[i].name && test_bit(i, dev->ledbit)) {
+			if (!vt_led_references[i]) {
+				led_trigger_register(&vt_led_triggers[i]);
+				/* This keyboard is first to have led i,
+				 * try to register it */
+				if (!led_classdev_register(NULL, &vt_leds[i]))
+					vt_led_references[i] = 1;
+				else
+					led_trigger_unregister(&vt_led_triggers[i]);
+			} else
+				vt_led_references[i]++;
+		}
+	mutex_unlock(&vt_led_registered_lock);
+
+	/* and register this device's LEDs */
+	for (i = 0; i < LED_CNT; i++)
+		if (vt_leds[i].name && test_bit(i, dev->ledbit)) {
+			leds[i].name = kasprintf(GFP_KERNEL, "%s::%s",
+						dev_name(&dev->dev),
+						vt_led_names[i]);
+			if (!leds[i].name) {
+				error = -ENOMEM;
+				goto err;
+			}
+			leds[i].max_brightness = 1;
+			leds[i].brightness_set = perdevice_input_led_set;
+			leds[i].default_trigger = vt_led_triggers[i].name;
+		}
+
+	/* No issue so far, we can register for real.  */
+	for (i = 0; i < LED_CNT; i++)
+		if (leds[i].name) {
+			led_classdev_register(&dev->dev, &leds[i]);
+			leds[i].dev->platform_data = dev;
+			perdevice_input_led_set(&leds[i],
+					vt_leds[i].brightness);
+		}
+
+	return 0;
+
+err:
+	input_led_delete(dev);
+	return error;
+}
+
+/* Disconnected input device. Clean it, and deregister now-useless VT LEDs
+ * and triggers.  */
+extern void input_led_disconnect(struct input_dev *dev)
+{
+	int i;
+	struct led_classdev *leds = dev->leds;
+
+	for (i = 0; i < LED_CNT; i++)
+		if (leds[i].name)
+			led_classdev_unregister(&leds[i]);
+
+	input_led_delete(dev);
+
+	mutex_lock(&vt_led_registered_lock);
+	for (i = 0; i < LED_CNT; i++) {
+		if (!vt_leds[i].name || !test_bit(i, dev->ledbit))
+			continue;
+
+		vt_led_references[i]--;
+		if (vt_led_references[i]) {
+			/* Still some devices needing it */
+			continue;
+		}
+
+		led_classdev_unregister(&vt_leds[i]);
+		led_trigger_unregister(&vt_led_triggers[i]);
+		clear_bit(i, vt_led_registered);
+	}
+	mutex_unlock(&vt_led_registered_lock);
+}
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("User LED support for input layer");
+MODULE_AUTHOR("Samuel Thibault <samuel.thibault@ens-lyon.org>");

[PATCH] Route kbd LEDs through the generic LEDs layer

[Samuel Thibault]

This permits to reassign keyboard LEDs to something else than keyboard "leds"
state, by adding keyboard led and modifier triggers connected to a series
of VT input LEDs, themselves connected to VT input triggers, which
per-input device LEDs use by default.  Userland can thus easily change the LED
behavior of (a priori) all input devices, or of particular input devices.

This also permits to fix #7063 from userland by using a modifier to implement
proper CapsLock behavior and have the keyboard caps lock led show that modifier
state.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
[ebroder@mokafive.com: Rebased to 3.2-rc1 or so, cleaned up some includes, and fixed some constants]
Signed-off-by: Evan Broder <evan@ebroder.net>

---
This is rebased and tested on 3.7.  I've also fixed the Kconfig issue by
just selecting what is needed.

diff -ur linux-3.1-orig/Documentation/leds/leds-class.txt linux-3.1-morris/Documentation/leds/leds-class.txt
--- linux-3.1-orig/Documentation/leds/leds-class.txt	2011-10-24 09:10:05.000000000 +0200
+++ linux-3.1-morris/Documentation/leds/leds-class.txt	2011-11-13 22:01:22.107128241 +0100
@@ -2,9 +2,6 @@
 LED handling under Linux
 ========================
 
-If you're reading this and thinking about keyboard leds, these are
-handled by the input subsystem and the led class is *not* needed.
-
 In its simplest form, the LED class just allows control of LEDs from
 userspace. LEDs appear in /sys/class/leds/. The maximum brightness of the
 LED is defined in max_brightness file. The brightness file will set the brightness
diff -ur linux-3.1-orig/drivers/input/input.c linux-3.1-morris/drivers/input/input.c
--- linux-3.1-orig/drivers/input/input.c	2011-10-24 09:10:05.000000000 +0200
+++ linux-3.1-morris/drivers/input/input.c	2011-11-14 02:07:52.236943816 +0100
@@ -27,6 +27,7 @@
 #include <linux/mutex.h>
 #include <linux/rcupdate.h>
 #include "input-compat.h"
+#include "leds.h"
 
 MODULE_AUTHOR("Vojtech Pavlik <vojtech@suse.cz>");
 MODULE_DESCRIPTION("Input core");
@@ -632,6 +633,11 @@
 		handle->open = 0;
 
 	spin_unlock_irq(&dev->event_lock);
+
+#ifdef CONFIG_INPUT_LEDS
+	if (is_event_supported(EV_LED, dev->evbit, EV_MAX))
+		input_led_disconnect(dev);
+#endif
 }
 
 /**
@@ -1871,6 +1877,11 @@
 
 	list_add_tail(&dev->node, &input_dev_list);
 
+#ifdef CONFIG_INPUT_LEDS
+	if (is_event_supported(EV_LED, dev->evbit, EV_MAX))
+		input_led_connect(dev);
+#endif
+
 	list_for_each_entry(handler, &input_handler_list, node)
 		input_attach_handler(dev, handler);
 
diff -ur linux-3.1-orig/drivers/input/Kconfig linux-3.1-morris/drivers/input/Kconfig
--- linux-3.1-orig/drivers/input/Kconfig	2011-10-24 09:10:05.000000000 +0200
+++ linux-3.1-morris/drivers/input/Kconfig	2011-11-14 02:36:44.253817605 +0100
@@ -165,6 +165,15 @@
 
 source "drivers/input/keyboard/Kconfig"
 
+config INPUT_LEDS
+	tristate "LED Support"
+	depends on LEDS_CLASS
+	select LEDS_TRIGGERS
+	default y
+	help
+	  This option enables support for the LEDs on keyboard managed
+	  by the input layer.
+
 source "drivers/input/mouse/Kconfig"
 
 source "drivers/input/joystick/Kconfig"
diff -ur linux-3.1-orig/drivers/input/Makefile linux-3.1-morris/drivers/input/Makefile
--- linux-3.1-orig/drivers/input/Makefile	2011-10-24 09:10:05.000000000 +0200
+++ linux-3.1-morris/drivers/input/Makefile	2011-11-14 01:24:58.714209193 +0100
@@ -17,6 +17,7 @@
 obj-$(CONFIG_INPUT_EVBUG)	+= evbug.o
 
 obj-$(CONFIG_INPUT_KEYBOARD)	+= keyboard/
+obj-$(CONFIG_INPUT_LEDS)	+= leds.o
 obj-$(CONFIG_INPUT_MOUSE)	+= mouse/
 obj-$(CONFIG_INPUT_JOYSTICK)	+= joystick/
 obj-$(CONFIG_INPUT_TABLET)	+= tablet/
diff -ur linux-3.1-orig/drivers/leds/Kconfig linux-3.1-morris/drivers/leds/Kconfig
--- linux-3.1-orig/drivers/leds/Kconfig	2011-10-24 09:10:05.000000000 +0200
+++ linux-3.1-morris/drivers/leds/Kconfig	2011-11-14 01:21:11.743396542 +0100
@@ -11,9 +11,6 @@
 	  Say Y to enable Linux LED support.  This allows control of supported
 	  LEDs from both userspace and optionally, by kernel events (triggers).
 
-	  This is not related to standard keyboard LEDs which are controlled
-	  via the input system.
-
 if NEW_LEDS
 
 config LEDS_CLASS
diff -ur linux-3.1-orig/drivers/tty/Kconfig linux-3.1-morris/drivers/tty/Kconfig
--- linux-3.1-orig/drivers/tty/Kconfig	2011-10-24 09:10:05.000000000 +0200
+++ linux-3.1-morris/drivers/tty/Kconfig	2011-11-14 02:36:34.838014234 +0100
@@ -2,6 +2,10 @@
 	bool "Virtual terminal" if EXPERT
 	depends on !S390
 	select INPUT
+	select NEW_LEDS
+	select LEDS_CLASS
+	select LEDS_TRIGGERS
+	select INPUT_LEDS
 	default y
 	---help---
 	  If you say Y here, you will get support for terminal devices with
diff -ur linux-3.1-orig/drivers/tty/vt/keyboard.c linux-3.1-morris/drivers/tty/vt/keyboard.c
--- linux-3.1-orig/drivers/tty/vt/keyboard.c	2011-10-24 09:10:05.000000000 +0200
+++ linux-3.1-morris/drivers/tty/vt/keyboard.c	2011-11-14 00:16:40.321709258 +0100
@@ -34,6 +34,7 @@
 #include <linux/string.h>
 #include <linux/init.h>
 #include <linux/slab.h>
+#include <linux/leds.h>
 
 #include <linux/kbd_kern.h>
 #include <linux/kbd_diacr.h>
@@ -137,6 +138,7 @@
 static char rep;					/* flag telling character repeat */
 
 static unsigned char ledstate = 0xff;			/* undefined */
+static unsigned char lockstate = 0xff;			/* undefined */
 static unsigned char ledioctl;
 
 static struct ledptr {
@@ -976,6 +978,32 @@
 	}
 }
 
+/* We route VT keyboard "leds" through triggers */
+static void kbd_ledstate_trigger_activate(struct led_classdev *cdev);
+static struct led_trigger ledtrig_ledstate[] = {
+#define DEFINE_LEDSTATE_TRIGGER(kbd_led, nam) \
+	[kbd_led] = { .name = nam, .activate = kbd_ledstate_trigger_activate, }
+	DEFINE_LEDSTATE_TRIGGER(VC_SCROLLOCK, "scrollock"),
+	DEFINE_LEDSTATE_TRIGGER(VC_NUMLOCK,   "numlock"),
+	DEFINE_LEDSTATE_TRIGGER(VC_CAPSLOCK,  "capslock"),
+	DEFINE_LEDSTATE_TRIGGER(VC_KANALOCK,  "kanalock"),
+#undef DEFINE_LEDSTATE_TRIGGER
+};
+static void kbd_lockstate_trigger_activate(struct led_classdev *cdev);
+static struct led_trigger ledtrig_lockstate[] = {
+#define DEFINE_LOCKSTATE_TRIGGER(kbd_led, nam) \
+	[kbd_led] = { .name = nam, .activate = kbd_lockstate_trigger_activate, }
+	DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTLOCK,  "shiftlock"),
+	DEFINE_LOCKSTATE_TRIGGER(VC_ALTGRLOCK,  "altgrlock"),
+	DEFINE_LOCKSTATE_TRIGGER(VC_CTRLLOCK,   "ctrllock"),
+	DEFINE_LOCKSTATE_TRIGGER(VC_ALTLOCK,    "altlock"),
+	DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTLLOCK, "shiftllock"),
+	DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTRLOCK, "shiftrlock"),
+	DEFINE_LOCKSTATE_TRIGGER(VC_CTRLLLOCK,  "ctrlllock"),
+	DEFINE_LOCKSTATE_TRIGGER(VC_CTRLRLOCK,  "ctrlrlock"),
+#undef DEFINE_LOCKSTATE_TRIGGER
+};
+
 /*
  * The leds display either (i) the status of NumLock, CapsLock, ScrollLock,
  * or (ii) whatever pattern of lights people want to show using KDSETLED,
@@ -1008,6 +1036,7 @@
 
 	leds = kbd->ledflagstate;
 
+	/* TODO: should be replaced by a LED trigger */
 	if (kbd->ledmode == LED_SHOW_MEM) {
 		for (i = 0; i < 3; i++)
 			if (ledptrs[i].valid) {
@@ -1020,18 +1049,24 @@
 	return leds;
 }
 
-static int kbd_update_leds_helper(struct input_handle *handle, void *data)
+/* Called on trigger connection, to set initial state */
+static void kbd_ledstate_trigger_activate(struct led_classdev *cdev)
 {
-	unsigned char leds = *(unsigned char *)data;
+	struct led_trigger *trigger = cdev->trigger;
+	int led = trigger - ledtrig_ledstate;
 
-	if (test_bit(EV_LED, handle->dev->evbit)) {
-		input_inject_event(handle, EV_LED, LED_SCROLLL, !!(leds & 0x01));
-		input_inject_event(handle, EV_LED, LED_NUML,    !!(leds & 0x02));
-		input_inject_event(handle, EV_LED, LED_CAPSL,   !!(leds & 0x04));
-		input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
-	}
+	tasklet_disable(&keyboard_tasklet);
+	led_trigger_event(trigger, ledstate & (1 << led) ? LED_FULL : LED_OFF);
+	tasklet_enable(&keyboard_tasklet);
+}
+static void kbd_lockstate_trigger_activate(struct led_classdev *cdev)
+{
+	struct led_trigger *trigger = cdev->trigger;
+	int led = trigger - ledtrig_lockstate;
 
-	return 0;
+	tasklet_disable(&keyboard_tasklet);
+	led_trigger_event(trigger, lockstate & (1 << led) ? LED_FULL : LED_OFF);
+	tasklet_enable(&keyboard_tasklet);
 }
 
 /*
@@ -1046,10 +1081,24 @@
 	unsigned char leds = getleds();
 
 	if (leds != ledstate) {
-		input_handler_for_each_handle(&kbd_handler, &leds,
-					      kbd_update_leds_helper);
+		int i;
+		for (i = 0; i < ARRAY_SIZE(ledtrig_ledstate); i++)
+			if ((leds ^ ledstate) & (1 << i))
+				led_trigger_event(&ledtrig_ledstate[i],
+						leds & (1 << i)
+						? LED_FULL : LED_OFF);
 		ledstate = leds;
 	}
+
+	if (kbd->lockstate != lockstate) {
+		int i;
+		for (i = 0; i < ARRAY_SIZE(ledtrig_lockstate); i++)
+			if ((kbd->lockstate ^ lockstate) & (1 << i))
+				led_trigger_event(&ledtrig_lockstate[i],
+						kbd->lockstate & (1 << i)
+						? LED_FULL : LED_OFF);
+		lockstate = kbd->lockstate;
+	}
 }
 
 DECLARE_TASKLET_DISABLED(keyboard_tasklet, kbd_bh, 0);
@@ -1387,20 +1436,6 @@
 	kfree(handle);
 }
 
-/*
- * Start keyboard handler on the new keyboard by refreshing LED state to
- * match the rest of the system.
- */
-static void kbd_start(struct input_handle *handle)
-{
-	tasklet_disable(&keyboard_tasklet);
-
-	if (ledstate != 0xff)
-		kbd_update_leds_helper(handle, &ledstate);
-
-	tasklet_enable(&keyboard_tasklet);
-}
-
 static const struct input_device_id kbd_ids[] = {
 	{
                 .flags = INPUT_DEVICE_ID_MATCH_EVBIT,
@@ -1422,7 +1457,6 @@
 	.match		= kbd_match,
 	.connect	= kbd_connect,
 	.disconnect	= kbd_disconnect,
-	.start		= kbd_start,
 	.name		= "kbd",
 	.id_table	= kbd_ids,
 };
@@ -1446,6 +1480,11 @@
 	if (error)
 		return error;
 
+	for (i = 0; i < ARRAY_SIZE(ledtrig_ledstate); i++)
+		led_trigger_register(&ledtrig_ledstate[i]);
+	for (i = 0; i < ARRAY_SIZE(ledtrig_lockstate); i++)
+		led_trigger_register(&ledtrig_lockstate[i]);
+
 	tasklet_enable(&keyboard_tasklet);
 	tasklet_schedule(&keyboard_tasklet);
 
diff -ur linux-3.1-orig/include/linux/input.h linux-3.1-morris/include/linux/input.h
--- linux-3.1-orig/include/linux/input.h	2011-10-24 09:10:05.000000000 +0200
+++ linux-3.1-morris/include/linux/input.h	2011-11-14 01:42:50.930272764 +0100
@@ -1267,6 +1267,8 @@
 	unsigned long snd[BITS_TO_LONGS(SND_CNT)];
 	unsigned long sw[BITS_TO_LONGS(SW_CNT)];
 
+	struct led_classdev *leds;
+
 	int (*open)(struct input_dev *dev);
 	void (*close)(struct input_dev *dev);
 	int (*flush)(struct input_dev *dev, struct file *file);
--- linux-3.1-orig/drivers/input/leds.h	1970-01-01 01:00:00.000000000 +0100
+++ linux-3.1-morris/drivers/input/leds.h	2011-11-14 02:24:26.738456456 +0100
@@ -0,0 +1,14 @@
+/*
+ * LED support for the input layer
+ *
+ * Copyright 2010-2012 Samuel Thibault <samuel.thibault@ens-lyon.org>
+ *
+ * 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/input.h>
+
+extern int input_led_connect(struct input_dev *dev);
+extern void input_led_disconnect(struct input_dev *dev);
--- linux-3.1-orig/drivers/input/leds.c	1970-01-01 01:00:00.000000000 +0100
+++ linux-3.1-morris/drivers/input/leds.c	2011-11-14 03:23:07.578469395 +0100
@@ -0,0 +1,243 @@
+/*
+ * LED support for the input layer
+ *
+ * Copyright 2010-2012 Samuel Thibault <samuel.thibault@ens-lyon.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/leds.h>
+#include <linux/input.h>
+
+/*
+ * Keyboard LEDs are propagated by default like the following example:
+ *
+ * VT keyboard numlock trigger
+ * -> vt::numl VT LED
+ * -> vt-numl VT trigger
+ * -> per-device inputx::numl LED
+ *
+ * Userland can however choose the trigger for the vt::numl LED, or
+ * independently choose the trigger for any inputx::numl LED.
+ */
+
+/* VT LED classes and triggers are registered on-demand according to
+ * existing LED devices */
+
+/* Handler for VT LEDs, just triggers the corresponding VT trigger. */
+static void vt_led_set(struct led_classdev *cdev,
+			  enum led_brightness brightness);
+static struct led_classdev vt_leds[LED_CNT] = {
+#define DEFINE_INPUT_LED(vt_led, nam, deftrig) \
+	[vt_led] = { \
+		.name = "vt::"nam, \
+		.max_brightness = 1, \
+		.brightness_set = vt_led_set, \
+		.default_trigger = deftrig, \
+	}
+/* Default triggers for the VT LEDs just correspond to the legacy
+ * usage. */
+	DEFINE_INPUT_LED(LED_NUML, "numl", "numlock"),
+	DEFINE_INPUT_LED(LED_CAPSL, "capsl", "capslock"),
+	DEFINE_INPUT_LED(LED_SCROLLL, "scrolll", "scrollock"),
+	DEFINE_INPUT_LED(LED_COMPOSE, "compose", NULL),
+	DEFINE_INPUT_LED(LED_KANA, "kana", "kanalock"),
+	DEFINE_INPUT_LED(LED_SLEEP, "sleep", NULL),
+	DEFINE_INPUT_LED(LED_SUSPEND, "suspend", NULL),
+	DEFINE_INPUT_LED(LED_MUTE, "mute", NULL),
+	DEFINE_INPUT_LED(LED_MISC, "misc", NULL),
+	DEFINE_INPUT_LED(LED_MAIL, "mail", NULL),
+	DEFINE_INPUT_LED(LED_CHARGING, "charging", NULL),
+};
+static const char *const vt_led_names[LED_CNT] = {
+	[LED_NUML] = "numl",
+	[LED_CAPSL] = "capsl",
+	[LED_SCROLLL] = "scrolll",
+	[LED_COMPOSE] = "compose",
+	[LED_KANA] = "kana",
+	[LED_SLEEP] = "sleep",
+	[LED_SUSPEND] = "suspend",
+	[LED_MUTE] = "mute",
+	[LED_MISC] = "misc",
+	[LED_MAIL] = "mail",
+	[LED_CHARGING] = "charging",
+};
+/* Handler for hotplug initialization */
+static void vt_led_trigger_activate(struct led_classdev *cdev);
+/* VT triggers */
+static struct led_trigger vt_led_triggers[LED_CNT] = {
+#define DEFINE_INPUT_LED_TRIGGER(vt_led, nam) \
+	[vt_led] = { \
+		.name = "vt-"nam, \
+		.activate = vt_led_trigger_activate, \
+	}
+	DEFINE_INPUT_LED_TRIGGER(LED_NUML, "numl"),
+	DEFINE_INPUT_LED_TRIGGER(LED_CAPSL, "capsl"),
+	DEFINE_INPUT_LED_TRIGGER(LED_SCROLLL, "scrolll"),
+	DEFINE_INPUT_LED_TRIGGER(LED_COMPOSE, "compose"),
+	DEFINE_INPUT_LED_TRIGGER(LED_KANA, "kana"),
+	DEFINE_INPUT_LED_TRIGGER(LED_SLEEP, "sleep"),
+	DEFINE_INPUT_LED_TRIGGER(LED_SUSPEND, "suspend"),
+	DEFINE_INPUT_LED_TRIGGER(LED_MUTE, "mute"),
+	DEFINE_INPUT_LED_TRIGGER(LED_MISC, "misc"),
+	DEFINE_INPUT_LED_TRIGGER(LED_MAIL, "mail"),
+	DEFINE_INPUT_LED_TRIGGER(LED_CHARGING, "charging"),
+};
+/* Lock for registration coherency */
+static DEFINE_MUTEX(vt_led_registered_lock);
+/* Which VT LED classes and triggers are registered */
+static unsigned long vt_led_registered[BITS_TO_LONGS(LED_CNT)];
+/* Number of input devices having each LED */
+static int vt_led_references[LED_CNT];
+
+/* VT LED state change, tell the VT trigger.  */
+static void vt_led_set(struct led_classdev *cdev,
+			  enum led_brightness brightness)
+{
+	int led = cdev - vt_leds;
+
+	led_trigger_event(&vt_led_triggers[led], !!brightness);
+}
+
+/* LED state change for some keyboard, notify that keyboard.  */
+static void perdevice_input_led_set(struct led_classdev *cdev,
+			  enum led_brightness brightness)
+{
+	struct input_dev *dev;
+	struct led_classdev *leds;
+	int led;
+
+	dev = cdev->dev->platform_data;
+	if (!dev)
+		/* Still initializing */
+		return;
+	leds = dev->leds;
+	led = cdev - leds;
+
+	input_event(dev, EV_LED, led, !!brightness);
+	input_event(dev, EV_SYN, SYN_REPORT, 0);
+}
+
+/* Keyboard hotplug, initialize its LED status */
+static void vt_led_trigger_activate(struct led_classdev *cdev)
+{
+	struct led_trigger *trigger = cdev->trigger;
+	int led = trigger - vt_led_triggers;
+
+	if (cdev->brightness_set)
+		cdev->brightness_set(cdev, vt_leds[led].brightness);
+}
+
+/* Free led stuff from input device, used at abortion and disconnection.  */
+static void input_led_delete(struct input_dev *dev)
+{
+	if (dev) {
+		struct led_classdev *leds = dev->leds;
+		if (leds) {
+			int i;
+			for (i = 0; i < LED_CNT; i++)
+				kfree(leds[i].name);
+			kfree(leds);
+			dev->leds = NULL;
+		}
+	}
+}
+
+/* A new input device with potential LEDs to connect.  */
+int input_led_connect(struct input_dev *dev)
+{
+	int i, error = 0;
+	struct led_classdev *leds;
+
+	dev->leds = leds = kzalloc(sizeof(*leds) * LED_CNT, GFP_KERNEL);
+	if (!dev->leds)
+		return -ENOMEM;
+
+	/* lazily register missing VT LEDs */
+	mutex_lock(&vt_led_registered_lock);
+	for (i = 0; i < LED_CNT; i++)
+		if (vt_leds[i].name && test_bit(i, dev->ledbit)) {
+			if (!vt_led_references[i]) {
+				led_trigger_register(&vt_led_triggers[i]);
+				/* This keyboard is first to have led i,
+				 * try to register it */
+				if (!led_classdev_register(NULL, &vt_leds[i]))
+					vt_led_references[i] = 1;
+				else
+					led_trigger_unregister(&vt_led_triggers[i]);
+			} else
+				vt_led_references[i]++;
+		}
+	mutex_unlock(&vt_led_registered_lock);
+
+	/* and register this device's LEDs */
+	for (i = 0; i < LED_CNT; i++)
+		if (vt_leds[i].name && test_bit(i, dev->ledbit)) {
+			leds[i].name = kasprintf(GFP_KERNEL, "%s::%s",
+						dev_name(&dev->dev),
+						vt_led_names[i]);
+			if (!leds[i].name) {
+				error = -ENOMEM;
+				goto err;
+			}
+			leds[i].max_brightness = 1;
+			leds[i].brightness_set = perdevice_input_led_set;
+			leds[i].default_trigger = vt_led_triggers[i].name;
+		}
+
+	/* No issue so far, we can register for real.  */
+	for (i = 0; i < LED_CNT; i++)
+		if (leds[i].name) {
+			led_classdev_register(&dev->dev, &leds[i]);
+			leds[i].dev->platform_data = dev;
+			perdevice_input_led_set(&leds[i],
+					vt_leds[i].brightness);
+		}
+
+	return 0;
+
+err:
+	input_led_delete(dev);
+	return error;
+}
+
+/* Disconnected input device. Clean it, and deregister now-useless VT LEDs
+ * and triggers.  */
+extern void input_led_disconnect(struct input_dev *dev)
+{
+	int i;
+	struct led_classdev *leds = dev->leds;
+
+	for (i = 0; i < LED_CNT; i++)
+		if (leds[i].name)
+			led_classdev_unregister(&leds[i]);
+
+	input_led_delete(dev);
+
+	mutex_lock(&vt_led_registered_lock);
+	for (i = 0; i < LED_CNT; i++) {
+		if (!vt_leds[i].name || !test_bit(i, dev->ledbit))
+			continue;
+
+		vt_led_references[i]--;
+		if (vt_led_references[i]) {
+			/* Still some devices needing it */
+			continue;
+		}
+
+		led_classdev_unregister(&vt_leds[i]);
+		led_trigger_unregister(&vt_led_triggers[i]);
+		clear_bit(i, vt_led_registered);
+	}
+	mutex_unlock(&vt_led_registered_lock);
+}
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("User LED support for input layer");
+MODULE_AUTHOR("Samuel Thibault <samuel.thibault@ens-lyon.org>");