* Re: [patch 20/35] leds: route kbd LEDs through the generic LEDs layer [not found] ` <20110112182702.GA9168@core.coreip.homeip.net> @ 2011-01-15 19:09 ` Samuel Thibault 2011-11-14 4:06 ` Samuel Thibault 1 sibling, 0 replies; 39+ messages in thread From: Samuel Thibault @ 2011-01-15 19:09 UTC (permalink / raw) To: Dmitry Torokhov Cc: linux-kernel, Pavel Machek, jslaby, rpurdie, arnaud.patard Dmitry Torokhov, le Wed 12 Jan 2011 10:27:02 -0800, a écrit : > > index 43d3395..0e2c9ba 100644 > > --- a/drivers/char/Kconfig > > +++ b/drivers/char/Kconfig > > @@ -8,6 +8,7 @@ config VT > > bool "Virtual terminal" if EMBEDDED > > depends on !S390 > > select INPUT > > + select LEDS_INPUT > > This will not work as LEDS_INPUT is itself depends and selects other > symbols and "select" statement does not propagate dependencies. Sure, I didn't expect it to be building fine as it was completely untested, it was rather just to show some code :) > Anyway, I was looking at the patch this way and that and I convinced > myself that while input should be integrated with LED subsystem doing it > via an input handler is not the right way. Input handler is, by it's > nature, an optional operation or an interface that exists to transform > the input core data stream into some other form. Here OTOH we have a > piece of hardware that we want to manage. I think the proper conversion > would involve attaching led_classdev structures directly to input_dev > objects and controlling them at the core level. Mmm, you mean, something like the patch below, which embeds the led devices inside the input structure? (don't even try to build with the patch below, again it's just to show code which is usually clearer than phrases :) ; it can probably be made #if CONFIG_LEDS BTW) Where would you prefer the vt-global virtual leds to go to? In keyboard.c or in a new file in drivers/leds/ for instances? Same question for the keyboard state triggers, which I had put into keyboard.c but Arnaud had put in a new file, but thus needing to pass through an input handler again. Samuel diff -urp linux-2.6.37-orig/drivers/input/input.c linux-2.6.37-perso/drivers/input/input.c --- linux-2.6.37-orig/drivers/input/input.c 2011-01-05 03:16:31.000000000 +0100 +++ linux-2.6.37-perso/drivers/input/input.c 2011-01-15 20:01:36.000000000 +0100 @@ -404,6 +404,19 @@ void input_inject_event(struct input_han } EXPORT_SYMBOL(input_inject_event); +static void input_led_set(struct led_classdev *cdev, + enum led_brightness brightness) +{ + dev = cdev->dev; + input_dev = struct_up(dev); + + lock_stuff(); + input_handle_event(input_dev, NULL, EV_LED, led, !!brightness); + input_handle_event(input_dev, NULL, EV_SYN, SYN_REPORT, 0); + unlock_stuff(); +} + + /** * input_alloc_absinfo - allocates array of input_absinfo structs * @dev: the input device emitting absolute events @@ -1907,6 +1920,18 @@ int input_register_device(struct input_d dev->name ? dev->name : "Unspecified device", path ? path : "N/A"); kfree(path); + if (evbit[EV_LED]) { + for (i = 0; i < LED_CNT; i++) { + if (test_bit(i, dev->ledbits)) { + initialize_stuff(&dev->leds[i]); + dev->leds[i].brightness_set = input_led_set; + dev->leds[i].default_trigger = vt_led_triggers[i].name; + led_classdev_register(&dev->dev, &dev->leds[i]); + lazily_register_vt_led(i); + } + } + } + error = mutex_lock_interruptible(&input_mutex); if (error) { device_del(&dev->dev); @@ -1952,6 +1977,15 @@ void input_unregister_device(struct inpu mutex_unlock(&input_mutex); + if (evbit[EV_LED]) { + for (i = 0; i < LED_CNT; i++) { + if (test_bit(i, dev->ledbits)) { + led_classdev_unregister(&dev->leds[i]); + lazily_unregister_vt_led(i); + } + } + } + device_unregister(&dev->dev); } EXPORT_SYMBOL(input_unregister_device); diff -urp linux-2.6.37-orig/drivers/tty/vt/keyboard.c linux-2.6.37-perso/drivers/tty/vt/keyboard.c --- linux-2.6.37-orig/drivers/tty/vt/keyboard.c 2011-01-05 03:17:37.000000000 +0100 +++ linux-2.6.37-perso/drivers/tty/vt/keyboard.c 2011-01-15 19:37:47.000000000 +0100 @@ -36,6 +36,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,7 @@ static unsigned int diacr; 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 { @@ -977,6 +979,33 @@ static void k_brl(struct vc_data *vc, un } } +/* VT keyboard lock states are exposed through triggers used by default by the + * central VT leds */ +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, @@ -1009,6 +1038,7 @@ static inline unsigned char getleds(void leds = kbd->ledflagstate; + /* TODO: reimplement in input LED layer */ if (kbd->ledmode == LED_SHOW_MEM) { for (i = 0; i < 3; i++) if (ledptrs[i].valid) { @@ -1021,18 +1051,24 @@ static inline unsigned char getleds(void 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) ? INT_MAX : 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) ? INT_MAX : LED_OFF); + tasklet_enable(&keyboard_tasklet); } /* @@ -1047,10 +1083,24 @@ static void kbd_bh(unsigned long dummy) 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) + ? INT_MAX : 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) + ? INT_MAX : LED_OFF); + lockstate = kbd->lockstate; + } } DECLARE_TASKLET_DISABLED(keyboard_tasklet, kbd_bh, 0); @@ -1388,20 +1438,6 @@ static void kbd_disconnect(struct input_ 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, @@ -1423,7 +1459,6 @@ static struct input_handler kbd_handler .match = kbd_match, .connect = kbd_connect, .disconnect = kbd_disconnect, - .start = kbd_start, .name = "kbd", .id_table = kbd_ids, }; @@ -1450,5 +1485,10 @@ int __init kbd_init(void) tasklet_enable(&keyboard_tasklet); tasklet_schedule(&keyboard_tasklet); + 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]); + return 0; } ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [patch 20/35] leds: route kbd LEDs through the generic LEDs layer [not found] ` <20110112182702.GA9168@core.coreip.homeip.net> 2011-01-15 19:09 ` [patch 20/35] leds: route kbd LEDs through the generic LEDs layer Samuel Thibault @ 2011-11-14 4:06 ` Samuel Thibault 2012-02-06 14:19 ` Pavel Machek ` (2 more replies) 1 sibling, 3 replies; 39+ messages in thread From: Samuel Thibault @ 2011-11-14 4:06 UTC (permalink / raw) To: Dmitry Torokhov Cc: Pavel Machek, akpm, jslaby, rpurdie, linux-kernel, Evan Broder, Arnaud Patard Dmitry Torokhov, le Wed 12 Jan 2011 10:27:02 -0800, a écrit : > > --- a/drivers/char/Kconfig > > +++ b/drivers/char/Kconfig > > @@ -8,6 +8,7 @@ config VT > > bool "Virtual terminal" if EMBEDDED > > depends on !S390 > > select INPUT > > + select LEDS_INPUT > > This will not work as LEDS_INPUT is itself depends and selects other > symbols and "select" statement does not propagate dependencies. > > Anyway, I was looking at the patch this way and that and I convinced > myself that while input should be integrated with LED subsystem doing it > via an input handler is not the right way. Input handler is, by it's > nature, an optional operation or an interface that exists to transform > the input core data stream into some other form. Here OTOH we have a > piece of hardware that we want to manage. I think the proper conversion > would involve attaching led_classdev structures directly to input_dev > objects and controlling them at the core level. Here is (at last!) an updated & tested patch. It actually makes things simpler, which is a good sign :) I however don't know how to fix the issue about VT selecting INPUT_LEDS. Samuel From: Samuel Thibault <samuel.thibault@ens-lyon.org> Subject: [PATCH] Route keyboard LEDs through the generic LEDs layer. 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 -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,7 @@ bool "Virtual terminal" if EXPERT depends on !S390 select INPUT + 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/init.h> #include <linux/slab.h> #include <linux/irq.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-2011 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-2011 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>"); ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [patch 20/35] leds: route kbd LEDs through the generic LEDs layer 2011-11-14 4:06 ` Samuel Thibault @ 2012-02-06 14:19 ` Pavel Machek 2012-11-28 22:06 ` Samuel Thibault 2012-12-21 0:34 ` [PATCH] Route " Samuel Thibault 2012-12-21 0:34 ` Samuel Thibault 2 siblings, 1 reply; 39+ messages in thread From: Pavel Machek @ 2012-02-06 14:19 UTC (permalink / raw) To: Samuel Thibault, Dmitry Torokhov, akpm, jslaby, rpurdie, linux-kernel, Evan Broder, Arnaud Patard On Mon 2011-11-14 05:06:13, Samuel Thibault wrote: > Dmitry Torokhov, le Wed 12 Jan 2011 10:27:02 -0800, a écrit : > > > --- a/drivers/char/Kconfig > > > +++ b/drivers/char/Kconfig > > > @@ -8,6 +8,7 @@ config VT > > > bool "Virtual terminal" if EMBEDDED > > > depends on !S390 > > > select INPUT > > > + select LEDS_INPUT > > > > This will not work as LEDS_INPUT is itself depends and selects other > > symbols and "select" statement does not propagate dependencies. > > > > Anyway, I was looking at the patch this way and that and I convinced > > myself that while input should be integrated with LED subsystem doing it > > via an input handler is not the right way. Input handler is, by it's > > nature, an optional operation or an interface that exists to transform > > the input core data stream into some other form. Here OTOH we have a > > piece of hardware that we want to manage. I think the proper conversion > > would involve attaching led_classdev structures directly to input_dev > > objects and controlling them at the core level. > > Here is (at last!) an updated & tested patch. It actually makes things > simpler, which is a good sign :) > > I however don't know how to fix the issue about VT selecting > INPUT_LEDS. Any news about this one? Having keyboard leds in the common framework seems very welcome to me. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [patch 20/35] leds: route kbd LEDs through the generic LEDs layer 2012-02-06 14:19 ` Pavel Machek @ 2012-11-28 22:06 ` Samuel Thibault 0 siblings, 0 replies; 39+ messages in thread From: Samuel Thibault @ 2012-11-28 22:06 UTC (permalink / raw) To: Pavel Machek Cc: Dmitry Torokhov, akpm, jslaby, rpurdie, linux-kernel, Evan Broder, Arnaud Patard Pavel Machek, le Mon 06 Feb 2012 15:19:23 +0100, a écrit : > On Mon 2011-11-14 05:06:13, Samuel Thibault wrote: > > Dmitry Torokhov, le Wed 12 Jan 2011 10:27:02 -0800, a écrit : > > > > --- a/drivers/char/Kconfig > > > > +++ b/drivers/char/Kconfig > > > > @@ -8,6 +8,7 @@ config VT > > > > bool "Virtual terminal" if EMBEDDED > > > > depends on !S390 > > > > select INPUT > > > > + select LEDS_INPUT > > > > > > This will not work as LEDS_INPUT is itself depends and selects other > > > symbols and "select" statement does not propagate dependencies. > > > > > > Anyway, I was looking at the patch this way and that and I convinced > > > myself that while input should be integrated with LED subsystem doing it > > > via an input handler is not the right way. Input handler is, by it's > > > nature, an optional operation or an interface that exists to transform > > > the input core data stream into some other form. Here OTOH we have a > > > piece of hardware that we want to manage. I think the proper conversion > > > would involve attaching led_classdev structures directly to input_dev > > > objects and controlling them at the core level. > > > > Here is (at last!) an updated & tested patch. It actually makes things > > simpler, which is a good sign :) > > > > I however don't know how to fix the issue about VT selecting > > INPUT_LEDS. > > Any news about this one? Having keyboard leds in the common framework > seems very welcome to me. Do you perhaps have an idea about the issue of VT selecting INPUT_LEDS? Samuel ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route kbd LEDs through the generic LEDs layer 2011-11-14 4:06 ` Samuel Thibault 2012-02-06 14:19 ` Pavel Machek @ 2012-12-21 0:34 ` Samuel Thibault 2012-12-21 0:34 ` Samuel Thibault 2 siblings, 0 replies; 39+ messages in thread From: Samuel Thibault @ 2012-12-21 0:34 UTC (permalink / raw) To: Dmitry Torokhov Cc: Pavel Machek, akpm, jslaby, rpurdie, linux-kernel, Evan Broder, Arnaud Patard, Peter Korsgaard 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>"); ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route kbd LEDs through the generic LEDs layer 2011-11-14 4:06 ` Samuel Thibault @ 2012-12-21 0:34 ` Samuel Thibault 2012-12-21 0:34 ` [PATCH] Route " Samuel Thibault 2012-12-21 0:34 ` Samuel Thibault 2 siblings, 0 replies; 39+ messages in thread From: Samuel Thibault @ 2012-12-21 0:34 UTC (permalink / raw) To: Dmitry Torokhov Cc: Pavel Machek, akpm, jslaby, rpurdie, linux-kernel, Evan Broder, Arnaud Patard, Peter Korsgaard, Arnaud Patard, Sascha Hauer, Matt Sealey, Rob Clark, Niels de Vos, linux-arm-kernel, Steev Klimaszewski 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>"); ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route kbd LEDs through the generic LEDs layer @ 2012-12-21 0:34 ` Samuel Thibault 0 siblings, 0 replies; 39+ messages in thread From: Samuel Thibault @ 2012-12-21 0:34 UTC (permalink / raw) To: linux-arm-kernel 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 at 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>"); ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route kbd LEDs through the generic LEDs layer 2012-12-21 0:34 ` Samuel Thibault @ 2013-07-07 10:10 ` Samuel Thibault -1 siblings, 0 replies; 39+ messages in thread From: Samuel Thibault @ 2013-07-07 10:10 UTC (permalink / raw) To: Dmitry Torokhov, Pavel Machek, akpm, jslaby, rpurdie, linux-kernel, Evan Broder, Arnaud Patard, Peter Korsgaard, Sascha Hauer, Matt Sealey, Rob Clark, Niels de Vos, linux-arm-kernel, Steev Klimaszewski 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>"); ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route kbd LEDs through the generic LEDs layer @ 2013-07-07 10:10 ` Samuel Thibault 0 siblings, 0 replies; 39+ messages in thread From: Samuel Thibault @ 2013-07-07 10:10 UTC (permalink / raw) To: linux-arm-kernel 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 at 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>"); ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route kbd LEDs through the generic LEDs layer 2013-07-07 10:10 ` Samuel Thibault @ 2013-07-12 11:36 ` Pavel Machek -1 siblings, 0 replies; 39+ messages in thread From: Pavel Machek @ 2013-07-12 11:36 UTC (permalink / raw) To: Samuel Thibault, Dmitry Torokhov, akpm, jslaby, rpurdie, linux-kernel, Evan Broder, Arnaud Patard, Peter Korsgaard, Sascha Hauer, Matt Sealey, Rob Clark, Niels de Vos, linux-arm-kernel, Steev Klimaszewski 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route kbd LEDs through the generic LEDs layer @ 2013-07-12 11:36 ` Pavel Machek 0 siblings, 0 replies; 39+ messages in thread From: Pavel Machek @ 2013-07-12 11:36 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route kbd LEDs through the generic LEDs layer 2013-07-12 11:36 ` Pavel Machek @ 2013-07-12 12:42 ` Samuel Thibault -1 siblings, 0 replies; 39+ messages in thread From: Samuel Thibault @ 2013-07-12 12:42 UTC (permalink / raw) To: Pavel Machek Cc: Dmitry Torokhov, akpm, jslaby, rpurdie, linux-kernel, Evan Broder, Arnaud Patard, Peter Korsgaard, Sascha Hauer, Matt Sealey, Rob Clark, Niels de Vos, linux-arm-kernel, Steev Klimaszewski 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route kbd LEDs through the generic LEDs layer @ 2013-07-12 12:42 ` Samuel Thibault 0 siblings, 0 replies; 39+ messages in thread From: Samuel Thibault @ 2013-07-12 12:42 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route kbd LEDs through the generic LEDs layer 2013-07-12 12:42 ` Samuel Thibault @ 2013-07-12 23:33 ` Pavel Machek -1 siblings, 0 replies; 39+ messages in thread From: Pavel Machek @ 2013-07-12 23:33 UTC (permalink / raw) To: Samuel Thibault, Dmitry Torokhov, akpm, jslaby, rpurdie, linux-kernel, Evan Broder, Arnaud Patard, Peter Korsgaard, Sascha Hauer, Matt Sealey, Rob Clark, Niels de Vos, linux-arm-kernel, Steev Klimaszewski 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route kbd LEDs through the generic LEDs layer @ 2013-07-12 23:33 ` Pavel Machek 0 siblings, 0 replies; 39+ messages in thread From: Pavel Machek @ 2013-07-12 23:33 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route kbd LEDs through the generic LEDs layer 2013-07-12 23:33 ` Pavel Machek @ 2013-07-13 9:35 ` Samuel Thibault -1 siblings, 0 replies; 39+ messages in thread From: Samuel Thibault @ 2013-07-13 9:35 UTC (permalink / raw) To: Pavel Machek Cc: Dmitry Torokhov, akpm, jslaby, rpurdie, linux-kernel, Evan Broder, Arnaud Patard, Peter Korsgaard, Sascha Hauer, Matt Sealey, Rob Clark, Niels de Vos, linux-arm-kernel, Steev Klimaszewski 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route kbd LEDs through the generic LEDs layer @ 2013-07-13 9:35 ` Samuel Thibault 0 siblings, 0 replies; 39+ messages in thread From: Samuel Thibault @ 2013-07-13 9:35 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route kbd LEDs through the generic LEDs layer 2013-07-13 9:35 ` Samuel Thibault @ 2013-07-15 9:12 ` Pavel Machek -1 siblings, 0 replies; 39+ messages in thread From: Pavel Machek @ 2013-07-15 9:12 UTC (permalink / raw) To: Samuel Thibault, Dmitry Torokhov, akpm, jslaby, rpurdie, linux-kernel, Evan Broder, Arnaud Patard, Peter Korsgaard, Sascha Hauer, Matt Sealey, Rob Clark, Niels de Vos, linux-arm-kernel, Steev Klimaszewski 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route kbd LEDs through the generic LEDs layer @ 2013-07-15 9:12 ` Pavel Machek 0 siblings, 0 replies; 39+ messages in thread From: Pavel Machek @ 2013-07-15 9:12 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route kbd LEDs through the generic LEDs layer 2013-07-15 9:12 ` Pavel Machek (?) @ 2014-03-16 10:16 ` Pali Rohár 2014-03-16 10:19 ` Samuel Thibault -1 siblings, 1 reply; 39+ messages in thread From: Pali Rohár @ 2014-03-16 10:16 UTC (permalink / raw) To: Pavel Machek, Samuel Thibault Cc: Dmitry Torokhov, Andrew Morton, jslaby, Richard Purdie, LKML, Evan Broder, Arnaud Patard 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route kbd LEDs through the generic LEDs layer 2014-03-16 10:16 ` Pali Rohár @ 2014-03-16 10:19 ` Samuel Thibault 2014-03-27 1:08 ` Pali Rohár 0 siblings, 1 reply; 39+ messages in thread From: Samuel Thibault @ 2014-03-16 10:19 UTC (permalink / raw) To: Pali Rohár Cc: Pavel Machek, Dmitry Torokhov, Andrew Morton, jslaby, Richard Purdie, LKML, Evan Broder, Arnaud Patard 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route kbd LEDs through the generic LEDs layer 2014-03-16 10:19 ` Samuel Thibault @ 2014-03-27 1:08 ` Pali Rohár 2014-03-28 7:01 ` Pavel Machek 0 siblings, 1 reply; 39+ messages in thread From: Pali Rohár @ 2014-03-27 1:08 UTC (permalink / raw) To: Samuel Thibault, Pali Rohár, Pavel Machek, Dmitry Torokhov, Andrew Morton, jslaby, Richard Purdie, LKML, Evan Broder, Arnaud Patard 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* 8 months to review a patch (was Re: [PATCH] Route kbd LEDs through the generic LEDs layer) 2014-03-27 1:08 ` Pali Rohár @ 2014-03-28 7:01 ` Pavel Machek 0 siblings, 0 replies; 39+ messages in thread From: Pavel Machek @ 2014-03-28 7:01 UTC (permalink / raw) To: Pali Rohár, dtor, linux-input, jkosina Cc: Samuel Thibault, Dmitry Torokhov, Andrew Morton, jslaby, Richard Purdie, LKML, Evan Broder, Arnaud Patard, Greg KH, Linus Torvalds On Thu 2014-03-27 02:08:17, Pali Rohár wrote: > 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. > Dmitry, can you look and review this patch? Dmitry, what happened to you, are you there? This patch is like 8 months old, and not too scary, either. Can you at least pinpoint some typos in it, or run in through the checkpatch? If Dmitry does not respond, what are the next steps. Greg, could you just take the patch? Linus? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 39+ messages in thread
* 8 months to review a patch (was Re: [PATCH] Route kbd LEDs through the generic LEDs layer) @ 2014-03-28 7:01 ` Pavel Machek 0 siblings, 0 replies; 39+ messages in thread From: Pavel Machek @ 2014-03-28 7:01 UTC (permalink / raw) To: Pali Rohár, dtor, linux-input, jkosina Cc: Samuel Thibault, Dmitry Torokhov, Andrew Morton, jslaby, Richard Purdie, LKML, Evan Broder, Arnaud Patard, Greg KH, Linus Torvalds On Thu 2014-03-27 02:08:17, Pali Rohár wrote: > 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. > Dmitry, can you look and review this patch? Dmitry, what happened to you, are you there? This patch is like 8 months old, and not too scary, either. Can you at least pinpoint some typos in it, or run in through the checkpatch? If Dmitry does not respond, what are the next steps. Greg, could you just take the patch? Linus? 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-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: 8 months to review a patch (was Re: [PATCH] Route kbd LEDs through the generic LEDs layer) 2014-03-28 7:01 ` Pavel Machek @ 2014-03-28 7:17 ` Greg KH -1 siblings, 0 replies; 39+ messages in thread From: Greg KH @ 2014-03-28 7:17 UTC (permalink / raw) To: Pavel Machek Cc: Pali Rohár, dtor, linux-input, jkosina, Samuel Thibault, Dmitry Torokhov, Andrew Morton, jslaby, Richard Purdie, LKML, Evan Broder, Arnaud Patard, Linus Torvalds On Fri, Mar 28, 2014 at 08:01:36AM +0100, Pavel Machek wrote: > On Thu 2014-03-27 02:08:17, Pali Rohár wrote: > > 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. > > > Dmitry, can you look and review this patch? > > Dmitry, what happened to you, are you there? This patch is like 8 > months old, and not too scary, either. Can you at least pinpoint some > typos in it, or run in through the checkpatch? > > If Dmitry does not respond, what are the next steps. Greg, could you > just take the patch? Linus? No, work with Dmitry please. ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: 8 months to review a patch (was Re: [PATCH] Route kbd LEDs through the generic LEDs layer) @ 2014-03-28 7:17 ` Greg KH 0 siblings, 0 replies; 39+ messages in thread From: Greg KH @ 2014-03-28 7:17 UTC (permalink / raw) To: Pavel Machek Cc: Pali Rohár, dtor, linux-input, jkosina, Samuel Thibault, Dmitry Torokhov, Andrew Morton, jslaby, Richard Purdie, LKML, Evan Broder, Arnaud Patard, Linus Torvalds On Fri, Mar 28, 2014 at 08:01:36AM +0100, Pavel Machek wrote: > On Thu 2014-03-27 02:08:17, Pali Rohár wrote: > > 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. > > > Dmitry, can you look and review this patch? > > Dmitry, what happened to you, are you there? This patch is like 8 > months old, and not too scary, either. Can you at least pinpoint some > typos in it, or run in through the checkpatch? > > If Dmitry does not respond, what are the next steps. Greg, could you > just take the patch? Linus? No, work with Dmitry please. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: 8 months to review a patch (was Re: [PATCH] Route kbd LEDs through the generic LEDs layer) 2014-03-28 7:17 ` Greg KH @ 2014-04-06 9:43 ` Pali Rohár -1 siblings, 0 replies; 39+ messages in thread From: Pali Rohár @ 2014-04-06 9:43 UTC (permalink / raw) To: Greg KH Cc: Pavel Machek, dtor, linux-input, Jiri Kosina, Samuel Thibault, Dmitry Torokhov, Andrew Morton, jslaby, Richard Purdie, LKML, Evan Broder, Arnaud Patard, Linus Torvalds 2014-03-28 8:17 GMT+01:00 Greg KH <greg@kroah.com>: > On Fri, Mar 28, 2014 at 08:01:36AM +0100, Pavel Machek wrote: >> On Thu 2014-03-27 02:08:17, Pali Rohár wrote: >> > 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. >> >> > Dmitry, can you look and review this patch? >> >> Dmitry, what happened to you, are you there? This patch is like 8 >> months old, and not too scary, either. Can you at least pinpoint some >> typos in it, or run in through the checkpatch? >> >> If Dmitry does not respond, what are the next steps. Greg, could you >> just take the patch? Linus? > > No, work with Dmitry please. > Greg, problem is that Dmitry does not respond and has not reviewed this patch for more that 8 months. From my point of view this looks like Dmitry does not have time for review anymore... What to do if maintainer is busy for a long time? Really could not you look at this patch? https://lkml.org/lkml/2014/3/31/141 -- Pali Rohár pali.rohar@gmail.com ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: 8 months to review a patch (was Re: [PATCH] Route kbd LEDs through the generic LEDs layer) @ 2014-04-06 9:43 ` Pali Rohár 0 siblings, 0 replies; 39+ messages in thread From: Pali Rohár @ 2014-04-06 9:43 UTC (permalink / raw) To: Greg KH Cc: Pavel Machek, dtor, linux-input, Jiri Kosina, Samuel Thibault, Dmitry Torokhov, Andrew Morton, jslaby, Richard Purdie, LKML, Evan Broder, Arnaud Patard, Linus Torvalds 2014-03-28 8:17 GMT+01:00 Greg KH <greg@kroah.com>: > On Fri, Mar 28, 2014 at 08:01:36AM +0100, Pavel Machek wrote: >> On Thu 2014-03-27 02:08:17, Pali Rohár wrote: >> > 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. >> >> > Dmitry, can you look and review this patch? >> >> Dmitry, what happened to you, are you there? This patch is like 8 >> months old, and not too scary, either. Can you at least pinpoint some >> typos in it, or run in through the checkpatch? >> >> If Dmitry does not respond, what are the next steps. Greg, could you >> just take the patch? Linus? > > No, work with Dmitry please. > Greg, problem is that Dmitry does not respond and has not reviewed this patch for more that 8 months. From my point of view this looks like Dmitry does not have time for review anymore... What to do if maintainer is busy for a long time? Really could not you look at this patch? https://lkml.org/lkml/2014/3/31/141 -- Pali Rohár pali.rohar@gmail.com -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: 8 months to review a patch (was Re: [PATCH] Route kbd LEDs through the generic LEDs layer) 2014-04-06 9:43 ` Pali Rohár (?) @ 2014-04-06 9:55 ` Sebastian Reichel -1 siblings, 0 replies; 39+ messages in thread From: Sebastian Reichel @ 2014-04-06 9:55 UTC (permalink / raw) To: Pali Rohár Cc: Greg KH, Pavel Machek, dtor, linux-input, Jiri Kosina, Samuel Thibault, Dmitry Torokhov, Andrew Morton, jslaby, Richard Purdie, LKML, Evan Broder, Arnaud Patard, Linus Torvalds [-- Attachment #1: Type: text/plain, Size: 922 bytes --] Hi Pali, On Sun, Apr 06, 2014 at 11:43:13AM +0200, Pali Rohár wrote: > Greg, problem is that Dmitry does not respond and has not reviewed > this patch for more that 8 months. From my point of view this looks > like Dmitry does not have time for review anymore... > > What to do if maintainer is busy for a long time? > Really could not you look at this patch? > > https://lkml.org/lkml/2014/3/31/141 I don't think that he doesn't have time for review anymore, since he is active in his git tree [0] and has sent a pull request to torvalds recently [1]. Maybe this thread is blocked by gmail? Or he didn't notice, that this patchset involves input.git, since the subject is mainly about LEDs? Have you tried contacting him directly, possibly without referencing the thread? [0] https://git.kernel.org/cgit/linux/kernel/git/dtor/input.git/ [1] https://lkml.org/lkml/2014/4/3/154 -- Sebastian [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: 8 months to review a patch (was Re: [PATCH] Route kbd LEDs through the generic LEDs layer) 2014-03-28 7:01 ` Pavel Machek @ 2014-03-28 8:08 ` Samuel Thibault -1 siblings, 0 replies; 39+ messages in thread From: Samuel Thibault @ 2014-03-28 8:08 UTC (permalink / raw) To: Pavel Machek Cc: Pali Rohár, dtor, linux-input, jkosina, Dmitry Torokhov, Andrew Morton, jslaby, Richard Purdie, LKML, Evan Broder, Arnaud Patard, Greg KH, Linus Torvalds Pavel Machek, le Fri 28 Mar 2014 08:01:36 +0100, a écrit : > On Thu 2014-03-27 02:08:17, Pali Rohár wrote: > > 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. > > > Dmitry, can you look and review this patch? > > Dmitry, what happened to you, are you there? This patch is like 8 > months old, Well, the patch was written in Feb 2010 actually, it got some reviews overs years, the version submitted in July 2013 is the same as the one submitted in Dec 2013. Samuel ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: 8 months to review a patch (was Re: [PATCH] Route kbd LEDs through the generic LEDs layer) @ 2014-03-28 8:08 ` Samuel Thibault 0 siblings, 0 replies; 39+ messages in thread From: Samuel Thibault @ 2014-03-28 8:08 UTC (permalink / raw) To: Pavel Machek Cc: Pali Rohár, dtor, linux-input, jkosina, Dmitry Torokhov, Andrew Morton, jslaby, Richard Purdie, LKML, Evan Broder, Arnaud Patard, Greg KH, Linus Torvalds Pavel Machek, le Fri 28 Mar 2014 08:01:36 +0100, a écrit : > On Thu 2014-03-27 02:08:17, Pali Rohár wrote: > > 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. > > > Dmitry, can you look and review this patch? > > Dmitry, what happened to you, are you there? This patch is like 8 > months old, Well, the patch was written in Feb 2010 actually, it got some reviews overs years, the version submitted in July 2013 is the same as the one submitted in Dec 2013. Samuel -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route kbd LEDs through the generic LEDs layer 2013-07-07 10:10 ` Samuel Thibault @ 2013-07-15 9:27 ` Peter Korsgaard -1 siblings, 0 replies; 39+ messages in thread From: Peter Korsgaard @ 2013-07-15 9:27 UTC (permalink / raw) To: Samuel Thibault Cc: Dmitry Torokhov, Pavel Machek, akpm, jslaby, rpurdie, linux-kernel, Evan Broder, Arnaud Patard, Sascha Hauer, Matt Sealey, Rob Clark, Niels de Vos, linux-arm-kernel, Steev Klimaszewski >>>>> "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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route kbd LEDs through the generic LEDs layer @ 2013-07-15 9:27 ` Peter Korsgaard 0 siblings, 0 replies; 39+ messages in thread From: Peter Korsgaard @ 2013-07-15 9:27 UTC (permalink / raw) To: linux-arm-kernel >>>>> "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 at 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route kbd LEDs through the generic LEDs layer 2013-07-07 10:10 ` Samuel Thibault @ 2013-07-15 15:03 ` David Herrmann -1 siblings, 0 replies; 39+ messages in thread From: David Herrmann @ 2013-07-15 15:03 UTC (permalink / raw) To: Samuel Thibault, Dmitry Torokhov, Pavel Machek, Andrew Morton, jslaby, rpurdie, linux-kernel, Evan Broder, Arnaud Patard, Peter Korsgaard, Sascha Hauer, Matt Sealey, Rob Clark, Niels de Vos, linux-arm-kernel, Steev Klimaszewski 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/ ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route kbd LEDs through the generic LEDs layer @ 2013-07-15 15:03 ` David Herrmann 0 siblings, 0 replies; 39+ messages in thread From: David Herrmann @ 2013-07-15 15:03 UTC (permalink / raw) To: linux-arm-kernel 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 at 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 at vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route kbd LEDs through the generic LEDs layer 2013-07-15 15:03 ` David Herrmann @ 2013-07-15 19:08 ` Samuel Thibault -1 siblings, 0 replies; 39+ messages in thread From: Samuel Thibault @ 2013-07-15 19:08 UTC (permalink / raw) To: David Herrmann Cc: Dmitry Torokhov, Pavel Machek, Andrew Morton, jslaby, rpurdie, linux-kernel, Evan Broder, Arnaud Patard, Peter Korsgaard, Sascha Hauer, Matt Sealey, Rob Clark, Niels de Vos, linux-arm-kernel, Steev Klimaszewski 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route kbd LEDs through the generic LEDs layer @ 2013-07-15 19:08 ` Samuel Thibault 0 siblings, 0 replies; 39+ messages in thread From: Samuel Thibault @ 2013-07-15 19:08 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route kbd LEDs through the generic LEDs layer 2013-07-15 19:08 ` Samuel Thibault @ 2013-07-17 15:14 ` David Herrmann -1 siblings, 0 replies; 39+ messages in thread From: David Herrmann @ 2013-07-17 15:14 UTC (permalink / raw) To: Samuel Thibault, David Herrmann, Dmitry Torokhov, Pavel Machek, Andrew Morton, jslaby, rpurdie, linux-kernel, Evan Broder, Arnaud Patard, Peter Korsgaard, Sascha Hauer, Matt Sealey, Rob Clark, Niels de Vos, linux-arm-kernel, Steev Klimaszewski 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route kbd LEDs through the generic LEDs layer @ 2013-07-17 15:14 ` David Herrmann 0 siblings, 0 replies; 39+ messages in thread From: David Herrmann @ 2013-07-17 15:14 UTC (permalink / raw) To: linux-arm-kernel 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2014-04-06 9:56 UTC | newest]
Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <201011112205.oABM5KVJ005298@imap1.linux-foundation.org>
[not found] ` <201011111440.07882.dmitry.torokhov@gmail.com>
[not found] ` <20110102090935.GV32469@atrey.karlin.mff.cuni.cz>
[not found] ` <20110102103210.GA25662@core.coreip.homeip.net>
[not found] ` <20110102225741.GX5480@const.famille.thibault.fr>
[not found] ` <20110112182702.GA9168@core.coreip.homeip.net>
2011-01-15 19:09 ` [patch 20/35] leds: route kbd LEDs through the generic LEDs layer Samuel Thibault
2011-11-14 4:06 ` Samuel Thibault
2012-02-06 14:19 ` Pavel Machek
2012-11-28 22:06 ` Samuel Thibault
2012-12-21 0:34 ` [PATCH] Route " Samuel Thibault
2012-12-21 0:34 ` Samuel Thibault
2012-12-21 0:34 ` Samuel Thibault
2013-07-07 10:10 ` Samuel Thibault
2013-07-07 10:10 ` Samuel Thibault
2013-07-12 11:36 ` Pavel Machek
2013-07-12 11:36 ` Pavel Machek
2013-07-12 12:42 ` Samuel Thibault
2013-07-12 12:42 ` Samuel Thibault
2013-07-12 23:33 ` Pavel Machek
2013-07-12 23:33 ` Pavel Machek
2013-07-13 9:35 ` Samuel Thibault
2013-07-13 9:35 ` Samuel Thibault
2013-07-15 9:12 ` Pavel Machek
2013-07-15 9:12 ` Pavel Machek
2014-03-16 10:16 ` Pali Rohár
2014-03-16 10:19 ` Samuel Thibault
2014-03-27 1:08 ` Pali Rohár
2014-03-28 7:01 ` 8 months to review a patch (was Re: [PATCH] Route kbd LEDs through the generic LEDs layer) Pavel Machek
2014-03-28 7:01 ` Pavel Machek
2014-03-28 7:17 ` Greg KH
2014-03-28 7:17 ` Greg KH
2014-04-06 9:43 ` Pali Rohár
2014-04-06 9:43 ` Pali Rohár
2014-04-06 9:55 ` Sebastian Reichel
2014-03-28 8:08 ` Samuel Thibault
2014-03-28 8:08 ` Samuel Thibault
2013-07-15 9:27 ` [PATCH] Route kbd LEDs through the generic LEDs layer Peter Korsgaard
2013-07-15 9:27 ` Peter Korsgaard
2013-07-15 15:03 ` David Herrmann
2013-07-15 15:03 ` David Herrmann
2013-07-15 19:08 ` Samuel Thibault
2013-07-15 19:08 ` Samuel Thibault
2013-07-17 15:14 ` David Herrmann
2013-07-17 15:14 ` David Herrmann
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.