* [PATCH] Route keyboard LEDs through the generic LEDs layer.
@ 2014-03-31 12:23 ` Samuel Thibault
0 siblings, 0 replies; 39+ messages in thread
From: Samuel Thibault @ 2014-03-31 12:23 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Pavel Machek, David Herrmann, akpm, jslaby, Bryan Wu, rpurdie,
linux-kernel, Evan Broder, Arnaud Patard, Peter Korsgaard,
Sascha Hauer, Matt Sealey, Rob Clark, Niels de Vos,
linux-arm-kernel, Steev Klimaszewski, blogic, Pali Rohár
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.
[ebroder@mokafive.com: Rebased to 3.2-rc1 or so, cleaned up some includes, and fixed some constants]
[blogic@openwrt.org: CONFIG_INPUT_LEDS stubs should be static inline]
[akpm@linux-foundation.org: remove unneeded `extern', fix comment layout]
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Signed-off-by: Evan Broder <evan@ebroder.net>
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Tested-by: Pavel Machek <pavel@ucw.cz>
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
Signed-off-by: John Crispin <blogic@openwrt.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
Changed in this version:
- fixes symbol dependencies between input.c and leds.c (notably
input_led_connect/disconnect) by stuffing them together in input.ko.
- documents the new leds field of struct input_dev.
--- a/Documentation/leds/leds-class.txt
+++ b/Documentation/leds/leds-class.txt
@@ -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
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -708,6 +708,9 @@ static void input_disconnect_device(stru
handle->open = 0;
spin_unlock_irq(&dev->event_lock);
+
+ if (is_event_supported(EV_LED, dev->evbit, EV_MAX))
+ input_led_disconnect(dev);
}
/**
@@ -2134,6 +2137,9 @@ int input_register_device(struct input_d
list_add_tail(&dev->node, &input_dev_list);
+ if (is_event_supported(EV_LED, dev->evbit, EV_MAX))
+ input_led_connect(dev);
+
list_for_each_entry(handler, &input_handler_list, node)
input_attach_handler(dev, handler);
--- a/drivers/input/Kconfig
+++ b/drivers/input/Kconfig
@@ -178,6 +178,15 @@ comment "Input Device Drivers"
source "drivers/input/keyboard/Kconfig"
+config INPUT_LEDS
+ bool "LED Support"
+ depends on LEDS_CLASS = INPUT || LEDS_CLASS = y
+ select LEDS_TRIGGERS
+ default y
+ help
+ This option enables support for LEDs on keyboards managed
+ by the input layer.
+
source "drivers/input/mouse/Kconfig"
source "drivers/input/joystick/Kconfig"
--- a/drivers/input/Makefile
+++ b/drivers/input/Makefile
@@ -6,6 +6,9 @@
obj-$(CONFIG_INPUT) += input-core.o
input-core-y := input.o input-compat.o input-mt.o ff-core.o
+ifeq ($(CONFIG_INPUT_LEDS),y)
+input-core-y += leds.o
+endif
obj-$(CONFIG_INPUT_FF_MEMLESS) += ff-memless.o
obj-$(CONFIG_INPUT_POLLDEV) += input-polldev.o
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -11,9 +11,6 @@ menuconfig NEW_LEDS
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
--- a/drivers/tty/Kconfig
+++ b/drivers/tty/Kconfig
@@ -13,6 +13,10 @@ config VT
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
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -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 char rep; /* flag telling cha
static int shift_state = 0;
static unsigned char ledstate = 0xff; /* undefined */
+static unsigned char lockstate = 0xff; /* undefined */
static unsigned char ledioctl;
/*
@@ -961,6 +963,41 @@ static void k_brl(struct vc_data *vc, un
}
}
+/* 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, "kbd-scrollock"),
+ DEFINE_LEDSTATE_TRIGGER(VC_NUMLOCK, "kbd-numlock"),
+ DEFINE_LEDSTATE_TRIGGER(VC_CAPSLOCK, "kbd-capslock"),
+ DEFINE_LEDSTATE_TRIGGER(VC_KANALOCK, "kbd-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, "kbd-shiftlock"),
+ DEFINE_LOCKSTATE_TRIGGER(VC_ALTGRLOCK, "kbd-altgrlock"),
+ DEFINE_LOCKSTATE_TRIGGER(VC_CTRLLOCK, "kbd-ctrllock"),
+ DEFINE_LOCKSTATE_TRIGGER(VC_ALTLOCK, "kbd-altlock"),
+ DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTLLOCK, "kbd-shiftllock"),
+ DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTRLOCK, "kbd-shiftrlock"),
+ DEFINE_LOCKSTATE_TRIGGER(VC_CTRLLLOCK, "kbd-ctrlllock"),
+ DEFINE_LOCKSTATE_TRIGGER(VC_CTRLRLOCK, "kbd-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,
@@ -995,18 +1032,25 @@ static inline unsigned char getleds(void
return kbd->ledflagstate;
}
-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);
+}
- return 0;
+static void kbd_lockstate_trigger_activate(struct led_classdev *cdev)
+{
+ struct led_trigger *trigger = cdev->trigger;
+ int led = trigger - ledtrig_lockstate;
+
+ tasklet_disable(&keyboard_tasklet);
+ led_trigger_event(trigger, lockstate & (1 << led) ? LED_FULL : LED_OFF);
+ tasklet_enable(&keyboard_tasklet);
}
/**
@@ -1095,16 +1139,29 @@ static void kbd_bh(unsigned long dummy)
{
unsigned char leds;
unsigned long flags;
-
+ int i;
+
spin_lock_irqsave(&led_lock, flags);
leds = getleds();
spin_unlock_irqrestore(&led_lock, flags);
if (leds != ledstate) {
- input_handler_for_each_handle(&kbd_handler, &leds,
- kbd_update_leds_helper);
+ 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) {
+ 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);
@@ -1442,20 +1499,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,
@@ -1477,7 +1520,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,
};
@@ -1501,6 +1543,20 @@ int __init kbd_init(void)
if (error)
return error;
+ for (i = 0; i < ARRAY_SIZE(ledtrig_ledstate); i++) {
+ error = led_trigger_register(&ledtrig_ledstate[i]);
+ if (error)
+ pr_err("error %d while registering trigger %s\n",
+ error, ledtrig_ledstate[i].name);
+ }
+
+ for (i = 0; i < ARRAY_SIZE(ledtrig_lockstate); i++) {
+ error = led_trigger_register(&ledtrig_lockstate[i]);
+ if (error)
+ pr_err("error %d while registering trigger %s\n",
+ error, ledtrig_lockstate[i].name);
+ }
+
tasklet_enable(&keyboard_tasklet);
tasklet_schedule(&keyboard_tasklet);
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -79,6 +79,7 @@ struct input_value {
* @led: reflects current state of device's LEDs
* @snd: reflects current state of sound effects
* @sw: reflects current state of device's switches
+ * @leds: leds objects for the device's LEDs
* @open: this method is called when the very first user calls
* input_open_device(). The driver must prepare the device
* to start generating events (start polling thread,
@@ -164,6 +165,8 @@ struct input_dev {
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);
@@ -531,4 +534,22 @@ int input_ff_erase(struct input_dev *dev
int input_ff_create_memless(struct input_dev *dev, void *data,
int (*play_effect)(struct input_dev *, void *, struct ff_effect *));
+#ifdef CONFIG_INPUT_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
+
#endif
--- /dev/null
+++ b/drivers/input/leds.c
@@ -0,0 +1,249 @@
+/*
+ * LED support for the input layer
+ *
+ * Copyright 2010-2014 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", "kbd-numlock"),
+ DEFINE_INPUT_LED(LED_CAPSL, "capsl", "kbd-capslock"),
+ DEFINE_INPUT_LED(LED_SCROLLL, "scrolll", "kbd-scrollock"),
+ DEFINE_INPUT_LED(LED_COMPOSE, "compose", NULL),
+ DEFINE_INPUT_LED(LED_KANA, "kana", "kbd-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 = kcalloc(LED_CNT, sizeof(*leds), 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.
+ */
+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 keyboard LEDs through the generic LEDs layer.
@ 2014-03-31 12:23 ` Samuel Thibault
0 siblings, 0 replies; 39+ messages in thread
From: Samuel Thibault @ 2014-03-31 12:23 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.
[ebroder at mokafive.com: Rebased to 3.2-rc1 or so, cleaned up some includes, and fixed some constants]
[blogic at openwrt.org: CONFIG_INPUT_LEDS stubs should be static inline]
[akpm at linux-foundation.org: remove unneeded `extern', fix comment layout]
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Signed-off-by: Evan Broder <evan@ebroder.net>
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Tested-by: Pavel Machek <pavel@ucw.cz>
Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
Signed-off-by: John Crispin <blogic@openwrt.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
Changed in this version:
- fixes symbol dependencies between input.c and leds.c (notably
input_led_connect/disconnect) by stuffing them together in input.ko.
- documents the new leds field of struct input_dev.
--- a/Documentation/leds/leds-class.txt
+++ b/Documentation/leds/leds-class.txt
@@ -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
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -708,6 +708,9 @@ static void input_disconnect_device(stru
handle->open = 0;
spin_unlock_irq(&dev->event_lock);
+
+ if (is_event_supported(EV_LED, dev->evbit, EV_MAX))
+ input_led_disconnect(dev);
}
/**
@@ -2134,6 +2137,9 @@ int input_register_device(struct input_d
list_add_tail(&dev->node, &input_dev_list);
+ if (is_event_supported(EV_LED, dev->evbit, EV_MAX))
+ input_led_connect(dev);
+
list_for_each_entry(handler, &input_handler_list, node)
input_attach_handler(dev, handler);
--- a/drivers/input/Kconfig
+++ b/drivers/input/Kconfig
@@ -178,6 +178,15 @@ comment "Input Device Drivers"
source "drivers/input/keyboard/Kconfig"
+config INPUT_LEDS
+ bool "LED Support"
+ depends on LEDS_CLASS = INPUT || LEDS_CLASS = y
+ select LEDS_TRIGGERS
+ default y
+ help
+ This option enables support for LEDs on keyboards managed
+ by the input layer.
+
source "drivers/input/mouse/Kconfig"
source "drivers/input/joystick/Kconfig"
--- a/drivers/input/Makefile
+++ b/drivers/input/Makefile
@@ -6,6 +6,9 @@
obj-$(CONFIG_INPUT) += input-core.o
input-core-y := input.o input-compat.o input-mt.o ff-core.o
+ifeq ($(CONFIG_INPUT_LEDS),y)
+input-core-y += leds.o
+endif
obj-$(CONFIG_INPUT_FF_MEMLESS) += ff-memless.o
obj-$(CONFIG_INPUT_POLLDEV) += input-polldev.o
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -11,9 +11,6 @@ menuconfig NEW_LEDS
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
--- a/drivers/tty/Kconfig
+++ b/drivers/tty/Kconfig
@@ -13,6 +13,10 @@ config VT
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
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -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 char rep; /* flag telling cha
static int shift_state = 0;
static unsigned char ledstate = 0xff; /* undefined */
+static unsigned char lockstate = 0xff; /* undefined */
static unsigned char ledioctl;
/*
@@ -961,6 +963,41 @@ static void k_brl(struct vc_data *vc, un
}
}
+/* 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, "kbd-scrollock"),
+ DEFINE_LEDSTATE_TRIGGER(VC_NUMLOCK, "kbd-numlock"),
+ DEFINE_LEDSTATE_TRIGGER(VC_CAPSLOCK, "kbd-capslock"),
+ DEFINE_LEDSTATE_TRIGGER(VC_KANALOCK, "kbd-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, "kbd-shiftlock"),
+ DEFINE_LOCKSTATE_TRIGGER(VC_ALTGRLOCK, "kbd-altgrlock"),
+ DEFINE_LOCKSTATE_TRIGGER(VC_CTRLLOCK, "kbd-ctrllock"),
+ DEFINE_LOCKSTATE_TRIGGER(VC_ALTLOCK, "kbd-altlock"),
+ DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTLLOCK, "kbd-shiftllock"),
+ DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTRLOCK, "kbd-shiftrlock"),
+ DEFINE_LOCKSTATE_TRIGGER(VC_CTRLLLOCK, "kbd-ctrlllock"),
+ DEFINE_LOCKSTATE_TRIGGER(VC_CTRLRLOCK, "kbd-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,
@@ -995,18 +1032,25 @@ static inline unsigned char getleds(void
return kbd->ledflagstate;
}
-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);
+}
- return 0;
+static void kbd_lockstate_trigger_activate(struct led_classdev *cdev)
+{
+ struct led_trigger *trigger = cdev->trigger;
+ int led = trigger - ledtrig_lockstate;
+
+ tasklet_disable(&keyboard_tasklet);
+ led_trigger_event(trigger, lockstate & (1 << led) ? LED_FULL : LED_OFF);
+ tasklet_enable(&keyboard_tasklet);
}
/**
@@ -1095,16 +1139,29 @@ static void kbd_bh(unsigned long dummy)
{
unsigned char leds;
unsigned long flags;
-
+ int i;
+
spin_lock_irqsave(&led_lock, flags);
leds = getleds();
spin_unlock_irqrestore(&led_lock, flags);
if (leds != ledstate) {
- input_handler_for_each_handle(&kbd_handler, &leds,
- kbd_update_leds_helper);
+ 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) {
+ 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);
@@ -1442,20 +1499,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,
@@ -1477,7 +1520,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,
};
@@ -1501,6 +1543,20 @@ int __init kbd_init(void)
if (error)
return error;
+ for (i = 0; i < ARRAY_SIZE(ledtrig_ledstate); i++) {
+ error = led_trigger_register(&ledtrig_ledstate[i]);
+ if (error)
+ pr_err("error %d while registering trigger %s\n",
+ error, ledtrig_ledstate[i].name);
+ }
+
+ for (i = 0; i < ARRAY_SIZE(ledtrig_lockstate); i++) {
+ error = led_trigger_register(&ledtrig_lockstate[i]);
+ if (error)
+ pr_err("error %d while registering trigger %s\n",
+ error, ledtrig_lockstate[i].name);
+ }
+
tasklet_enable(&keyboard_tasklet);
tasklet_schedule(&keyboard_tasklet);
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -79,6 +79,7 @@ struct input_value {
* @led: reflects current state of device's LEDs
* @snd: reflects current state of sound effects
* @sw: reflects current state of device's switches
+ * @leds: leds objects for the device's LEDs
* @open: this method is called when the very first user calls
* input_open_device(). The driver must prepare the device
* to start generating events (start polling thread,
@@ -164,6 +165,8 @@ struct input_dev {
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);
@@ -531,4 +534,22 @@ int input_ff_erase(struct input_dev *dev
int input_ff_create_memless(struct input_dev *dev, void *data,
int (*play_effect)(struct input_dev *, void *, struct ff_effect *));
+#ifdef CONFIG_INPUT_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
+
#endif
--- /dev/null
+++ b/drivers/input/leds.c
@@ -0,0 +1,249 @@
+/*
+ * LED support for the input layer
+ *
+ * Copyright 2010-2014 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", "kbd-numlock"),
+ DEFINE_INPUT_LED(LED_CAPSL, "capsl", "kbd-capslock"),
+ DEFINE_INPUT_LED(LED_SCROLLL, "scrolll", "kbd-scrollock"),
+ DEFINE_INPUT_LED(LED_COMPOSE, "compose", NULL),
+ DEFINE_INPUT_LED(LED_KANA, "kana", "kbd-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 = kcalloc(LED_CNT, sizeof(*leds), 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.
+ */
+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 keyboard LEDs through the generic LEDs layer.
2014-03-31 12:23 ` Samuel Thibault
(?)
@ 2014-04-01 7:02 ` Pali Rohár
-1 siblings, 0 replies; 39+ messages in thread
From: Pali Rohár @ 2014-04-01 7:02 UTC (permalink / raw)
To: Samuel Thibault, Dmitry Torokhov, Pavel Machek, David Herrmann,
Andrew Morton, jslaby, Bryan Wu, Richard Purdie, LKML,
Evan Broder, Arnaud Patard, Peter Korsgaard, Sascha Hauer,
Matt Sealey, Rob Clark, Niels de Vos, linux-arm-kernel,
Steev Klimaszewski, blogic, Pali Rohár, linux-input
2014-03-31 14:23 GMT+02:00 Samuel Thibault <samuel.thibault@ens-lyon.org>:
> 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.
>
> [ebroder@mokafive.com: Rebased to 3.2-rc1 or so, cleaned up some includes, and fixed some constants]
> [blogic@openwrt.org: CONFIG_INPUT_LEDS stubs should be static inline]
> [akpm@linux-foundation.org: remove unneeded `extern', fix comment layout]
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> Signed-off-by: Evan Broder <evan@ebroder.net>
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
> Tested-by: Pavel Machek <pavel@ucw.cz>
> Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
> Signed-off-by: John Crispin <blogic@openwrt.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> Changed in this version:
> - fixes symbol dependencies between input.c and leds.c (notably
> input_led_connect/disconnect) by stuffing them together in input.ko.
> - documents the new leds field of struct input_dev.
>
> --- a/Documentation/leds/leds-class.txt
> +++ b/Documentation/leds/leds-class.txt
> @@ -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
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -708,6 +708,9 @@ static void input_disconnect_device(stru
> handle->open = 0;
>
> spin_unlock_irq(&dev->event_lock);
> +
> + if (is_event_supported(EV_LED, dev->evbit, EV_MAX))
> + input_led_disconnect(dev);
> }
>
> /**
> @@ -2134,6 +2137,9 @@ int input_register_device(struct input_d
>
> list_add_tail(&dev->node, &input_dev_list);
>
> + if (is_event_supported(EV_LED, dev->evbit, EV_MAX))
> + input_led_connect(dev);
> +
> list_for_each_entry(handler, &input_handler_list, node)
> input_attach_handler(dev, handler);
>
> --- a/drivers/input/Kconfig
> +++ b/drivers/input/Kconfig
> @@ -178,6 +178,15 @@ comment "Input Device Drivers"
>
> source "drivers/input/keyboard/Kconfig"
>
> +config INPUT_LEDS
> + bool "LED Support"
> + depends on LEDS_CLASS = INPUT || LEDS_CLASS = y
> + select LEDS_TRIGGERS
> + default y
> + help
> + This option enables support for LEDs on keyboards managed
> + by the input layer.
> +
> source "drivers/input/mouse/Kconfig"
>
> source "drivers/input/joystick/Kconfig"
> --- a/drivers/input/Makefile
> +++ b/drivers/input/Makefile
> @@ -6,6 +6,9 @@
>
> obj-$(CONFIG_INPUT) += input-core.o
> input-core-y := input.o input-compat.o input-mt.o ff-core.o
> +ifeq ($(CONFIG_INPUT_LEDS),y)
> +input-core-y += leds.o
> +endif
>
> obj-$(CONFIG_INPUT_FF_MEMLESS) += ff-memless.o
> obj-$(CONFIG_INPUT_POLLDEV) += input-polldev.o
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -11,9 +11,6 @@ menuconfig NEW_LEDS
> 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
> --- a/drivers/tty/Kconfig
> +++ b/drivers/tty/Kconfig
> @@ -13,6 +13,10 @@ config VT
> 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
> --- a/drivers/tty/vt/keyboard.c
> +++ b/drivers/tty/vt/keyboard.c
> @@ -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 char rep; /* flag telling cha
> static int shift_state = 0;
>
> static unsigned char ledstate = 0xff; /* undefined */
> +static unsigned char lockstate = 0xff; /* undefined */
> static unsigned char ledioctl;
>
> /*
> @@ -961,6 +963,41 @@ static void k_brl(struct vc_data *vc, un
> }
> }
>
> +/* 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, "kbd-scrollock"),
> + DEFINE_LEDSTATE_TRIGGER(VC_NUMLOCK, "kbd-numlock"),
> + DEFINE_LEDSTATE_TRIGGER(VC_CAPSLOCK, "kbd-capslock"),
> + DEFINE_LEDSTATE_TRIGGER(VC_KANALOCK, "kbd-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, "kbd-shiftlock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_ALTGRLOCK, "kbd-altgrlock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_CTRLLOCK, "kbd-ctrllock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_ALTLOCK, "kbd-altlock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTLLOCK, "kbd-shiftllock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTRLOCK, "kbd-shiftrlock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_CTRLLLOCK, "kbd-ctrlllock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_CTRLRLOCK, "kbd-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,
> @@ -995,18 +1032,25 @@ static inline unsigned char getleds(void
> return kbd->ledflagstate;
> }
>
> -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);
> +}
>
> - return 0;
> +static void kbd_lockstate_trigger_activate(struct led_classdev *cdev)
> +{
> + struct led_trigger *trigger = cdev->trigger;
> + int led = trigger - ledtrig_lockstate;
> +
> + tasklet_disable(&keyboard_tasklet);
> + led_trigger_event(trigger, lockstate & (1 << led) ? LED_FULL : LED_OFF);
> + tasklet_enable(&keyboard_tasklet);
> }
>
> /**
> @@ -1095,16 +1139,29 @@ static void kbd_bh(unsigned long dummy)
> {
> unsigned char leds;
> unsigned long flags;
> -
> + int i;
> +
> spin_lock_irqsave(&led_lock, flags);
> leds = getleds();
> spin_unlock_irqrestore(&led_lock, flags);
>
> if (leds != ledstate) {
> - input_handler_for_each_handle(&kbd_handler, &leds,
> - kbd_update_leds_helper);
> + 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) {
> + 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);
> @@ -1442,20 +1499,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,
> @@ -1477,7 +1520,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,
> };
> @@ -1501,6 +1543,20 @@ int __init kbd_init(void)
> if (error)
> return error;
>
> + for (i = 0; i < ARRAY_SIZE(ledtrig_ledstate); i++) {
> + error = led_trigger_register(&ledtrig_ledstate[i]);
> + if (error)
> + pr_err("error %d while registering trigger %s\n",
> + error, ledtrig_ledstate[i].name);
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(ledtrig_lockstate); i++) {
> + error = led_trigger_register(&ledtrig_lockstate[i]);
> + if (error)
> + pr_err("error %d while registering trigger %s\n",
> + error, ledtrig_lockstate[i].name);
> + }
> +
> tasklet_enable(&keyboard_tasklet);
> tasklet_schedule(&keyboard_tasklet);
>
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -79,6 +79,7 @@ struct input_value {
> * @led: reflects current state of device's LEDs
> * @snd: reflects current state of sound effects
> * @sw: reflects current state of device's switches
> + * @leds: leds objects for the device's LEDs
> * @open: this method is called when the very first user calls
> * input_open_device(). The driver must prepare the device
> * to start generating events (start polling thread,
> @@ -164,6 +165,8 @@ struct input_dev {
> 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);
> @@ -531,4 +534,22 @@ int input_ff_erase(struct input_dev *dev
> int input_ff_create_memless(struct input_dev *dev, void *data,
> int (*play_effect)(struct input_dev *, void *, struct ff_effect *));
>
> +#ifdef CONFIG_INPUT_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
> +
> #endif
> --- /dev/null
> +++ b/drivers/input/leds.c
> @@ -0,0 +1,249 @@
> +/*
> + * LED support for the input layer
> + *
> + * Copyright 2010-2014 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", "kbd-numlock"),
> + DEFINE_INPUT_LED(LED_CAPSL, "capsl", "kbd-capslock"),
> + DEFINE_INPUT_LED(LED_SCROLLL, "scrolll", "kbd-scrollock"),
> + DEFINE_INPUT_LED(LED_COMPOSE, "compose", NULL),
> + DEFINE_INPUT_LED(LED_KANA, "kana", "kbd-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 = kcalloc(LED_CNT, sizeof(*leds), 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.
> + */
> +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>");
>
Dmitry, can you review this patch and include it into 3.15?
--
Pali Rohár
pali.rohar@gmail.com
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route keyboard LEDs through the generic LEDs layer.
@ 2014-04-01 7:02 ` Pali Rohár
0 siblings, 0 replies; 39+ messages in thread
From: Pali Rohár @ 2014-04-01 7:02 UTC (permalink / raw)
To: Samuel Thibault, Dmitry Torokhov, Pavel Machek, David Herrmann,
Andrew Morton, jslaby, Bryan Wu, Richard Purdie, LKML,
Evan Broder, Arnaud Patard, Peter Korsgaard, Sascha Hauer,
Matt Sealey, Rob Clark, Niels de Vos, linux-arm-kernel,
Steev Klimaszewski, blogic, Pali Rohár, linux-input
2014-03-31 14:23 GMT+02:00 Samuel Thibault <samuel.thibault@ens-lyon.org>:
> 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.
>
> [ebroder@mokafive.com: Rebased to 3.2-rc1 or so, cleaned up some includes, and fixed some constants]
> [blogic@openwrt.org: CONFIG_INPUT_LEDS stubs should be static inline]
> [akpm@linux-foundation.org: remove unneeded `extern', fix comment layout]
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> Signed-off-by: Evan Broder <evan@ebroder.net>
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
> Tested-by: Pavel Machek <pavel@ucw.cz>
> Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
> Signed-off-by: John Crispin <blogic@openwrt.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> Changed in this version:
> - fixes symbol dependencies between input.c and leds.c (notably
> input_led_connect/disconnect) by stuffing them together in input.ko.
> - documents the new leds field of struct input_dev.
>
> --- a/Documentation/leds/leds-class.txt
> +++ b/Documentation/leds/leds-class.txt
> @@ -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
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -708,6 +708,9 @@ static void input_disconnect_device(stru
> handle->open = 0;
>
> spin_unlock_irq(&dev->event_lock);
> +
> + if (is_event_supported(EV_LED, dev->evbit, EV_MAX))
> + input_led_disconnect(dev);
> }
>
> /**
> @@ -2134,6 +2137,9 @@ int input_register_device(struct input_d
>
> list_add_tail(&dev->node, &input_dev_list);
>
> + if (is_event_supported(EV_LED, dev->evbit, EV_MAX))
> + input_led_connect(dev);
> +
> list_for_each_entry(handler, &input_handler_list, node)
> input_attach_handler(dev, handler);
>
> --- a/drivers/input/Kconfig
> +++ b/drivers/input/Kconfig
> @@ -178,6 +178,15 @@ comment "Input Device Drivers"
>
> source "drivers/input/keyboard/Kconfig"
>
> +config INPUT_LEDS
> + bool "LED Support"
> + depends on LEDS_CLASS = INPUT || LEDS_CLASS = y
> + select LEDS_TRIGGERS
> + default y
> + help
> + This option enables support for LEDs on keyboards managed
> + by the input layer.
> +
> source "drivers/input/mouse/Kconfig"
>
> source "drivers/input/joystick/Kconfig"
> --- a/drivers/input/Makefile
> +++ b/drivers/input/Makefile
> @@ -6,6 +6,9 @@
>
> obj-$(CONFIG_INPUT) += input-core.o
> input-core-y := input.o input-compat.o input-mt.o ff-core.o
> +ifeq ($(CONFIG_INPUT_LEDS),y)
> +input-core-y += leds.o
> +endif
>
> obj-$(CONFIG_INPUT_FF_MEMLESS) += ff-memless.o
> obj-$(CONFIG_INPUT_POLLDEV) += input-polldev.o
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -11,9 +11,6 @@ menuconfig NEW_LEDS
> 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
> --- a/drivers/tty/Kconfig
> +++ b/drivers/tty/Kconfig
> @@ -13,6 +13,10 @@ config VT
> 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
> --- a/drivers/tty/vt/keyboard.c
> +++ b/drivers/tty/vt/keyboard.c
> @@ -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 char rep; /* flag telling cha
> static int shift_state = 0;
>
> static unsigned char ledstate = 0xff; /* undefined */
> +static unsigned char lockstate = 0xff; /* undefined */
> static unsigned char ledioctl;
>
> /*
> @@ -961,6 +963,41 @@ static void k_brl(struct vc_data *vc, un
> }
> }
>
> +/* 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, "kbd-scrollock"),
> + DEFINE_LEDSTATE_TRIGGER(VC_NUMLOCK, "kbd-numlock"),
> + DEFINE_LEDSTATE_TRIGGER(VC_CAPSLOCK, "kbd-capslock"),
> + DEFINE_LEDSTATE_TRIGGER(VC_KANALOCK, "kbd-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, "kbd-shiftlock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_ALTGRLOCK, "kbd-altgrlock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_CTRLLOCK, "kbd-ctrllock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_ALTLOCK, "kbd-altlock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTLLOCK, "kbd-shiftllock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTRLOCK, "kbd-shiftrlock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_CTRLLLOCK, "kbd-ctrlllock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_CTRLRLOCK, "kbd-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,
> @@ -995,18 +1032,25 @@ static inline unsigned char getleds(void
> return kbd->ledflagstate;
> }
>
> -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);
> +}
>
> - return 0;
> +static void kbd_lockstate_trigger_activate(struct led_classdev *cdev)
> +{
> + struct led_trigger *trigger = cdev->trigger;
> + int led = trigger - ledtrig_lockstate;
> +
> + tasklet_disable(&keyboard_tasklet);
> + led_trigger_event(trigger, lockstate & (1 << led) ? LED_FULL : LED_OFF);
> + tasklet_enable(&keyboard_tasklet);
> }
>
> /**
> @@ -1095,16 +1139,29 @@ static void kbd_bh(unsigned long dummy)
> {
> unsigned char leds;
> unsigned long flags;
> -
> + int i;
> +
> spin_lock_irqsave(&led_lock, flags);
> leds = getleds();
> spin_unlock_irqrestore(&led_lock, flags);
>
> if (leds != ledstate) {
> - input_handler_for_each_handle(&kbd_handler, &leds,
> - kbd_update_leds_helper);
> + 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) {
> + 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);
> @@ -1442,20 +1499,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,
> @@ -1477,7 +1520,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,
> };
> @@ -1501,6 +1543,20 @@ int __init kbd_init(void)
> if (error)
> return error;
>
> + for (i = 0; i < ARRAY_SIZE(ledtrig_ledstate); i++) {
> + error = led_trigger_register(&ledtrig_ledstate[i]);
> + if (error)
> + pr_err("error %d while registering trigger %s\n",
> + error, ledtrig_ledstate[i].name);
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(ledtrig_lockstate); i++) {
> + error = led_trigger_register(&ledtrig_lockstate[i]);
> + if (error)
> + pr_err("error %d while registering trigger %s\n",
> + error, ledtrig_lockstate[i].name);
> + }
> +
> tasklet_enable(&keyboard_tasklet);
> tasklet_schedule(&keyboard_tasklet);
>
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -79,6 +79,7 @@ struct input_value {
> * @led: reflects current state of device's LEDs
> * @snd: reflects current state of sound effects
> * @sw: reflects current state of device's switches
> + * @leds: leds objects for the device's LEDs
> * @open: this method is called when the very first user calls
> * input_open_device(). The driver must prepare the device
> * to start generating events (start polling thread,
> @@ -164,6 +165,8 @@ struct input_dev {
> 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);
> @@ -531,4 +534,22 @@ int input_ff_erase(struct input_dev *dev
> int input_ff_create_memless(struct input_dev *dev, void *data,
> int (*play_effect)(struct input_dev *, void *, struct ff_effect *));
>
> +#ifdef CONFIG_INPUT_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
> +
> #endif
> --- /dev/null
> +++ b/drivers/input/leds.c
> @@ -0,0 +1,249 @@
> +/*
> + * LED support for the input layer
> + *
> + * Copyright 2010-2014 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", "kbd-numlock"),
> + DEFINE_INPUT_LED(LED_CAPSL, "capsl", "kbd-capslock"),
> + DEFINE_INPUT_LED(LED_SCROLLL, "scrolll", "kbd-scrollock"),
> + DEFINE_INPUT_LED(LED_COMPOSE, "compose", NULL),
> + DEFINE_INPUT_LED(LED_KANA, "kana", "kbd-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 = kcalloc(LED_CNT, sizeof(*leds), 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.
> + */
> +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>");
>
Dmitry, can you review this patch and include it into 3.15?
--
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
* [PATCH] Route keyboard LEDs through the generic LEDs layer.
@ 2014-04-01 7:02 ` Pali Rohár
0 siblings, 0 replies; 39+ messages in thread
From: Pali Rohár @ 2014-04-01 7:02 UTC (permalink / raw)
To: linux-arm-kernel
2014-03-31 14:23 GMT+02:00 Samuel Thibault <samuel.thibault@ens-lyon.org>:
> 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.
>
> [ebroder at mokafive.com: Rebased to 3.2-rc1 or so, cleaned up some includes, and fixed some constants]
> [blogic at openwrt.org: CONFIG_INPUT_LEDS stubs should be static inline]
> [akpm at linux-foundation.org: remove unneeded `extern', fix comment layout]
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> Signed-off-by: Evan Broder <evan@ebroder.net>
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
> Tested-by: Pavel Machek <pavel@ucw.cz>
> Acked-by: Peter Korsgaard <jacmet@sunsite.dk>
> Signed-off-by: John Crispin <blogic@openwrt.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> Changed in this version:
> - fixes symbol dependencies between input.c and leds.c (notably
> input_led_connect/disconnect) by stuffing them together in input.ko.
> - documents the new leds field of struct input_dev.
>
> --- a/Documentation/leds/leds-class.txt
> +++ b/Documentation/leds/leds-class.txt
> @@ -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
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -708,6 +708,9 @@ static void input_disconnect_device(stru
> handle->open = 0;
>
> spin_unlock_irq(&dev->event_lock);
> +
> + if (is_event_supported(EV_LED, dev->evbit, EV_MAX))
> + input_led_disconnect(dev);
> }
>
> /**
> @@ -2134,6 +2137,9 @@ int input_register_device(struct input_d
>
> list_add_tail(&dev->node, &input_dev_list);
>
> + if (is_event_supported(EV_LED, dev->evbit, EV_MAX))
> + input_led_connect(dev);
> +
> list_for_each_entry(handler, &input_handler_list, node)
> input_attach_handler(dev, handler);
>
> --- a/drivers/input/Kconfig
> +++ b/drivers/input/Kconfig
> @@ -178,6 +178,15 @@ comment "Input Device Drivers"
>
> source "drivers/input/keyboard/Kconfig"
>
> +config INPUT_LEDS
> + bool "LED Support"
> + depends on LEDS_CLASS = INPUT || LEDS_CLASS = y
> + select LEDS_TRIGGERS
> + default y
> + help
> + This option enables support for LEDs on keyboards managed
> + by the input layer.
> +
> source "drivers/input/mouse/Kconfig"
>
> source "drivers/input/joystick/Kconfig"
> --- a/drivers/input/Makefile
> +++ b/drivers/input/Makefile
> @@ -6,6 +6,9 @@
>
> obj-$(CONFIG_INPUT) += input-core.o
> input-core-y := input.o input-compat.o input-mt.o ff-core.o
> +ifeq ($(CONFIG_INPUT_LEDS),y)
> +input-core-y += leds.o
> +endif
>
> obj-$(CONFIG_INPUT_FF_MEMLESS) += ff-memless.o
> obj-$(CONFIG_INPUT_POLLDEV) += input-polldev.o
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -11,9 +11,6 @@ menuconfig NEW_LEDS
> 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
> --- a/drivers/tty/Kconfig
> +++ b/drivers/tty/Kconfig
> @@ -13,6 +13,10 @@ config VT
> 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
> --- a/drivers/tty/vt/keyboard.c
> +++ b/drivers/tty/vt/keyboard.c
> @@ -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 char rep; /* flag telling cha
> static int shift_state = 0;
>
> static unsigned char ledstate = 0xff; /* undefined */
> +static unsigned char lockstate = 0xff; /* undefined */
> static unsigned char ledioctl;
>
> /*
> @@ -961,6 +963,41 @@ static void k_brl(struct vc_data *vc, un
> }
> }
>
> +/* 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, "kbd-scrollock"),
> + DEFINE_LEDSTATE_TRIGGER(VC_NUMLOCK, "kbd-numlock"),
> + DEFINE_LEDSTATE_TRIGGER(VC_CAPSLOCK, "kbd-capslock"),
> + DEFINE_LEDSTATE_TRIGGER(VC_KANALOCK, "kbd-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, "kbd-shiftlock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_ALTGRLOCK, "kbd-altgrlock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_CTRLLOCK, "kbd-ctrllock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_ALTLOCK, "kbd-altlock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTLLOCK, "kbd-shiftllock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_SHIFTRLOCK, "kbd-shiftrlock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_CTRLLLOCK, "kbd-ctrlllock"),
> + DEFINE_LOCKSTATE_TRIGGER(VC_CTRLRLOCK, "kbd-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,
> @@ -995,18 +1032,25 @@ static inline unsigned char getleds(void
> return kbd->ledflagstate;
> }
>
> -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);
> +}
>
> - return 0;
> +static void kbd_lockstate_trigger_activate(struct led_classdev *cdev)
> +{
> + struct led_trigger *trigger = cdev->trigger;
> + int led = trigger - ledtrig_lockstate;
> +
> + tasklet_disable(&keyboard_tasklet);
> + led_trigger_event(trigger, lockstate & (1 << led) ? LED_FULL : LED_OFF);
> + tasklet_enable(&keyboard_tasklet);
> }
>
> /**
> @@ -1095,16 +1139,29 @@ static void kbd_bh(unsigned long dummy)
> {
> unsigned char leds;
> unsigned long flags;
> -
> + int i;
> +
> spin_lock_irqsave(&led_lock, flags);
> leds = getleds();
> spin_unlock_irqrestore(&led_lock, flags);
>
> if (leds != ledstate) {
> - input_handler_for_each_handle(&kbd_handler, &leds,
> - kbd_update_leds_helper);
> + 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) {
> + 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);
> @@ -1442,20 +1499,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,
> @@ -1477,7 +1520,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,
> };
> @@ -1501,6 +1543,20 @@ int __init kbd_init(void)
> if (error)
> return error;
>
> + for (i = 0; i < ARRAY_SIZE(ledtrig_ledstate); i++) {
> + error = led_trigger_register(&ledtrig_ledstate[i]);
> + if (error)
> + pr_err("error %d while registering trigger %s\n",
> + error, ledtrig_ledstate[i].name);
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(ledtrig_lockstate); i++) {
> + error = led_trigger_register(&ledtrig_lockstate[i]);
> + if (error)
> + pr_err("error %d while registering trigger %s\n",
> + error, ledtrig_lockstate[i].name);
> + }
> +
> tasklet_enable(&keyboard_tasklet);
> tasklet_schedule(&keyboard_tasklet);
>
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -79,6 +79,7 @@ struct input_value {
> * @led: reflects current state of device's LEDs
> * @snd: reflects current state of sound effects
> * @sw: reflects current state of device's switches
> + * @leds: leds objects for the device's LEDs
> * @open: this method is called when the very first user calls
> * input_open_device(). The driver must prepare the device
> * to start generating events (start polling thread,
> @@ -164,6 +165,8 @@ struct input_dev {
> 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);
> @@ -531,4 +534,22 @@ int input_ff_erase(struct input_dev *dev
> int input_ff_create_memless(struct input_dev *dev, void *data,
> int (*play_effect)(struct input_dev *, void *, struct ff_effect *));
>
> +#ifdef CONFIG_INPUT_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
> +
> #endif
> --- /dev/null
> +++ b/drivers/input/leds.c
> @@ -0,0 +1,249 @@
> +/*
> + * LED support for the input layer
> + *
> + * Copyright 2010-2014 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", "kbd-numlock"),
> + DEFINE_INPUT_LED(LED_CAPSL, "capsl", "kbd-capslock"),
> + DEFINE_INPUT_LED(LED_SCROLLL, "scrolll", "kbd-scrollock"),
> + DEFINE_INPUT_LED(LED_COMPOSE, "compose", NULL),
> + DEFINE_INPUT_LED(LED_KANA, "kana", "kbd-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 = kcalloc(LED_CNT, sizeof(*leds), 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.
> + */
> +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>");
>
Dmitry, can you review this patch and include it into 3.15?
--
Pali Roh?r
pali.rohar at gmail.com
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route keyboard LEDs through the generic LEDs layer.
2014-03-31 12:23 ` Samuel Thibault
@ 2014-04-07 2:10 ` Dmitry Torokhov
-1 siblings, 0 replies; 39+ messages in thread
From: Dmitry Torokhov @ 2014-04-07 2:10 UTC (permalink / raw)
To: Samuel Thibault, Pavel Machek, David Herrmann, akpm, jslaby,
Bryan Wu, rpurdie, linux-kernel, Evan Broder, Arnaud Patard,
Peter Korsgaard, Sascha Hauer, Matt Sealey, Rob Clark,
Niels de Vos, linux-arm-kernel, Steev Klimaszewski, blogic,
Pali Rohár
Hi Samuel,
On Mon, Mar 31, 2014 at 02:23:23PM +0200, Samuel Thibault 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.
>
I still have the same concern that I believe I already mentioned a while
ago: how do we reconcile the LED control via triggers with LED control
done through event devices? Currently, as far as I can see, they will be
clashing with each other. I.e. if I remap my capslock led to be the new
shiftlock and then userspace writes EV_LED/LED_CAPSL it would light up
my new "shift lock", right?
Also I wonder if we really need to have the multiplexing (VT-level) leds
in addition to per-device ones.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route keyboard LEDs through the generic LEDs layer.
@ 2014-04-07 2:10 ` Dmitry Torokhov
0 siblings, 0 replies; 39+ messages in thread
From: Dmitry Torokhov @ 2014-04-07 2:10 UTC (permalink / raw)
To: linux-arm-kernel
Hi Samuel,
On Mon, Mar 31, 2014 at 02:23:23PM +0200, Samuel Thibault 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.
>
I still have the same concern that I believe I already mentioned a while
ago: how do we reconcile the LED control via triggers with LED control
done through event devices? Currently, as far as I can see, they will be
clashing with each other. I.e. if I remap my capslock led to be the new
shiftlock and then userspace writes EV_LED/LED_CAPSL it would light up
my new "shift lock", right?
Also I wonder if we really need to have the multiplexing (VT-level) leds
in addition to per-device ones.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route keyboard LEDs through the generic LEDs layer.
2014-04-07 2:10 ` Dmitry Torokhov
@ 2014-04-07 7:54 ` Samuel Thibault
-1 siblings, 0 replies; 39+ messages in thread
From: Samuel Thibault @ 2014-04-07 7:54 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Pavel Machek, David Herrmann, akpm, jslaby, Bryan Wu, rpurdie,
linux-kernel, Evan Broder, Arnaud Patard, Peter Korsgaard,
Sascha Hauer, Matt Sealey, Rob Clark, Niels de Vos,
linux-arm-kernel, Steev Klimaszewski, blogic, Pali Rohár
Dmitry Torokhov, le Sun 06 Apr 2014 19:10:15 -0700, a écrit :
> On Mon, Mar 31, 2014 at 02:23:23PM +0200, Samuel Thibault 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.
> >
>
> I still have the same concern that I believe I already mentioned a while
> ago: how do we reconcile the LED control via triggers with LED control
> done through event devices?
I don't remember that raised during the discussion.
> Currently, as far as I can see, they will be
> clashing with each other. I.e. if I remap my capslock led to be the new
> shiftlock and then userspace writes EV_LED/LED_CAPSL it would light up
> my new "shift lock", right?
Well, yes, sure, if you shoot in your foot it will hurt :) (although
here the damage is really small, it is just LED lighting or not).
I don't see why people would both tinker with such triggers and run
userland programs writing to evdev. Of course it typically happens with
X, but that's not a problem in the usual case: kbd triggers are not
doing anything while on X, so X can play with evdev with no problem.
If the user puts e.g. a heartbeat on some LED and then runs X, the X
events will mix with the heartbeat. The solution is for the user to
just disable the corresponding LED in the X keyboard configuration.
Do we really want to impose some overriding between VT-generated and
other application-generated events? AIUI, we don't do this between
applications which would open the same evdev, so I don't see why we
should care more about the VT.
> Also I wonder if we really need to have the multiplexing (VT-level) leds
> in addition to per-device ones.
We do: we want to be able to easily remap all (current and future)
keyboards' LEDs easily.
My goal, at the beginning, was to be able to fix the console-setup
bug. That was that console-setup wants to use, for the capslock key,
something else than the capslock mechanism hard-wired in the kernel,
so as to get way more fine-grain control over how caps is handled.
But then the capslock LED would not lit any more. By remapping the
VT-level capslock led to the foo-lock used by console-setup, one gets
back capslock LEDs of all keyboards working back as expected by the
user. I also use it for getting a group LED, just like I have on X,
to know which keyboard layout group I am in. People using non-latin
layouts really need that.
Samuel
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route keyboard LEDs through the generic LEDs layer.
@ 2014-04-07 7:54 ` Samuel Thibault
0 siblings, 0 replies; 39+ messages in thread
From: Samuel Thibault @ 2014-04-07 7:54 UTC (permalink / raw)
To: linux-arm-kernel
Dmitry Torokhov, le Sun 06 Apr 2014 19:10:15 -0700, a ?crit :
> On Mon, Mar 31, 2014 at 02:23:23PM +0200, Samuel Thibault 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.
> >
>
> I still have the same concern that I believe I already mentioned a while
> ago: how do we reconcile the LED control via triggers with LED control
> done through event devices?
I don't remember that raised during the discussion.
> Currently, as far as I can see, they will be
> clashing with each other. I.e. if I remap my capslock led to be the new
> shiftlock and then userspace writes EV_LED/LED_CAPSL it would light up
> my new "shift lock", right?
Well, yes, sure, if you shoot in your foot it will hurt :) (although
here the damage is really small, it is just LED lighting or not).
I don't see why people would both tinker with such triggers and run
userland programs writing to evdev. Of course it typically happens with
X, but that's not a problem in the usual case: kbd triggers are not
doing anything while on X, so X can play with evdev with no problem.
If the user puts e.g. a heartbeat on some LED and then runs X, the X
events will mix with the heartbeat. The solution is for the user to
just disable the corresponding LED in the X keyboard configuration.
Do we really want to impose some overriding between VT-generated and
other application-generated events? AIUI, we don't do this between
applications which would open the same evdev, so I don't see why we
should care more about the VT.
> Also I wonder if we really need to have the multiplexing (VT-level) leds
> in addition to per-device ones.
We do: we want to be able to easily remap all (current and future)
keyboards' LEDs easily.
My goal, at the beginning, was to be able to fix the console-setup
bug. That was that console-setup wants to use, for the capslock key,
something else than the capslock mechanism hard-wired in the kernel,
so as to get way more fine-grain control over how caps is handled.
But then the capslock LED would not lit any more. By remapping the
VT-level capslock led to the foo-lock used by console-setup, one gets
back capslock LEDs of all keyboards working back as expected by the
user. I also use it for getting a group LED, just like I have on X,
to know which keyboard layout group I am in. People using non-latin
layouts really need that.
Samuel
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route keyboard LEDs through the generic LEDs layer.
2014-04-07 7:54 ` Samuel Thibault
@ 2014-04-08 8:39 ` Dmitry Torokhov
-1 siblings, 0 replies; 39+ messages in thread
From: Dmitry Torokhov @ 2014-04-08 8:39 UTC (permalink / raw)
To: Samuel Thibault, Pavel Machek, David Herrmann, akpm, jslaby,
Bryan Wu, rpurdie, linux-kernel, Evan Broder, Arnaud Patard,
Peter Korsgaard, Sascha Hauer, Matt Sealey, Rob Clark,
Niels de Vos, linux-arm-kernel, Steev Klimaszewski, blogic,
Pali Rohár
On Mon, Apr 07, 2014 at 09:54:23AM +0200, Samuel Thibault wrote:
> Dmitry Torokhov, le Sun 06 Apr 2014 19:10:15 -0700, a écrit :
> > On Mon, Mar 31, 2014 at 02:23:23PM +0200, Samuel Thibault 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.
> > >
> >
> > I still have the same concern that I believe I already mentioned a while
> > ago: how do we reconcile the LED control via triggers with LED control
> > done through event devices?
>
> I don't remember that raised during the discussion.
Hmm, I might be mistaken, I thought I did mention that.
>
> > Currently, as far as I can see, they will be
> > clashing with each other. I.e. if I remap my capslock led to be the new
> > shiftlock and then userspace writes EV_LED/LED_CAPSL it would light up
> > my new "shift lock", right?
>
> Well, yes, sure, if you shoot in your foot it will hurt :) (although
> here the damage is really small, it is just LED lighting or not).
This is not about amount of damage but the overall correctness of the
implementation. I'd rather not have a solution with known holes, if
possible.
>
> I don't see why people would both tinker with such triggers and run
> userland programs writing to evdev. Of course it typically happens with
> X, but that's not a problem in the usual case: kbd triggers are not
> doing anything while on X, so X can play with evdev with no problem.
> If the user puts e.g. a heartbeat on some LED and then runs X, the X
> events will mix with the heartbeat. The solution is for the user to
> just disable the corresponding LED in the X keyboard configuration.
> Do we really want to impose some overriding between VT-generated and
> other application-generated events? AIUI, we don't do this between
> applications which would open the same evdev, so I don't see why we
> should care more about the VT.
It is not about the VT, I am talking about pure input core. If I
repurpose CapsLock LED for my WiFi indicator I do not want to go into
other programs and teach them that they should stay away from trying to
control this LED. IOW if we switch to LED-subsystem-backed LEDs then we
should go all the way and implement this cleanly.
>
> > Also I wonder if we really need to have the multiplexing (VT-level) leds
> > in addition to per-device ones.
>
> We do: we want to be able to easily remap all (current and future)
> keyboards' LEDs easily.
>
> My goal, at the beginning, was to be able to fix the console-setup
> bug. That was that console-setup wants to use, for the capslock key,
> something else than the capslock mechanism hard-wired in the kernel,
> so as to get way more fine-grain control over how caps is handled.
> But then the capslock LED would not lit any more. By remapping the
> VT-level capslock led to the foo-lock used by console-setup, one gets
> back capslock LEDs of all keyboards working back as expected by the
> user. I also use it for getting a group LED, just like I have on X,
> to know which keyboard layout group I am in. People using non-latin
> layouts really need that.
I understand the desire, however I still wonder if we should be
extending legacy VT given that David wants to move it all to userspace.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route keyboard LEDs through the generic LEDs layer.
@ 2014-04-08 8:39 ` Dmitry Torokhov
0 siblings, 0 replies; 39+ messages in thread
From: Dmitry Torokhov @ 2014-04-08 8:39 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Apr 07, 2014 at 09:54:23AM +0200, Samuel Thibault wrote:
> Dmitry Torokhov, le Sun 06 Apr 2014 19:10:15 -0700, a ?crit :
> > On Mon, Mar 31, 2014 at 02:23:23PM +0200, Samuel Thibault 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.
> > >
> >
> > I still have the same concern that I believe I already mentioned a while
> > ago: how do we reconcile the LED control via triggers with LED control
> > done through event devices?
>
> I don't remember that raised during the discussion.
Hmm, I might be mistaken, I thought I did mention that.
>
> > Currently, as far as I can see, they will be
> > clashing with each other. I.e. if I remap my capslock led to be the new
> > shiftlock and then userspace writes EV_LED/LED_CAPSL it would light up
> > my new "shift lock", right?
>
> Well, yes, sure, if you shoot in your foot it will hurt :) (although
> here the damage is really small, it is just LED lighting or not).
This is not about amount of damage but the overall correctness of the
implementation. I'd rather not have a solution with known holes, if
possible.
>
> I don't see why people would both tinker with such triggers and run
> userland programs writing to evdev. Of course it typically happens with
> X, but that's not a problem in the usual case: kbd triggers are not
> doing anything while on X, so X can play with evdev with no problem.
> If the user puts e.g. a heartbeat on some LED and then runs X, the X
> events will mix with the heartbeat. The solution is for the user to
> just disable the corresponding LED in the X keyboard configuration.
> Do we really want to impose some overriding between VT-generated and
> other application-generated events? AIUI, we don't do this between
> applications which would open the same evdev, so I don't see why we
> should care more about the VT.
It is not about the VT, I am talking about pure input core. If I
repurpose CapsLock LED for my WiFi indicator I do not want to go into
other programs and teach them that they should stay away from trying to
control this LED. IOW if we switch to LED-subsystem-backed LEDs then we
should go all the way and implement this cleanly.
>
> > Also I wonder if we really need to have the multiplexing (VT-level) leds
> > in addition to per-device ones.
>
> We do: we want to be able to easily remap all (current and future)
> keyboards' LEDs easily.
>
> My goal, at the beginning, was to be able to fix the console-setup
> bug. That was that console-setup wants to use, for the capslock key,
> something else than the capslock mechanism hard-wired in the kernel,
> so as to get way more fine-grain control over how caps is handled.
> But then the capslock LED would not lit any more. By remapping the
> VT-level capslock led to the foo-lock used by console-setup, one gets
> back capslock LEDs of all keyboards working back as expected by the
> user. I also use it for getting a group LED, just like I have on X,
> to know which keyboard layout group I am in. People using non-latin
> layouts really need that.
I understand the desire, however I still wonder if we should be
extending legacy VT given that David wants to move it all to userspace.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route keyboard LEDs through the generic LEDs layer.
2014-04-08 8:39 ` Dmitry Torokhov
@ 2014-04-08 23:33 ` Samuel Thibault
-1 siblings, 0 replies; 39+ messages in thread
From: Samuel Thibault @ 2014-04-08 23:33 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Pavel Machek, David Herrmann, akpm, jslaby, Bryan Wu, rpurdie,
linux-kernel, Evan Broder, Arnaud Patard, Peter Korsgaard,
Sascha Hauer, Matt Sealey, Rob Clark, Niels de Vos,
linux-arm-kernel, Steev Klimaszewski, blogic, Pali Rohár
Hello,
Dmitry Torokhov, le Tue 08 Apr 2014 01:39:40 -0700, a écrit :
> It is not about the VT, I am talking about pure input core. If I
> repurpose CapsLock LED for my WiFi indicator I do not want to go into
> other programs and teach them that they should stay away from trying to
> control this LED.
Err, but even without talking about repurposing Capslock LED for WiFi...
How is managed the conflict between the normal capslock behavior and
other programs' action on the underlying input device? I don't think
this patch does not introduce the problem.
Now you may answer "well, if it could fix the problem along the way it'd
be good". I'd like to answer "well, this is not my shit" :)
But still, the problem is interesting, and I don't know how to deal
properly with it. More precisely, I don't think we *want* to deal with
it. Currently, what happens is that there is no priority between what
the VT keyboard events produce and what applications produce, so things
happily mix with strange results as soon as a user tries to combine
both. My concern is:
How to decide which one to prioritize?
Is it just because console-setup happens to repurpose the capslock LED
key that applications should suddenly not have capslock LED control
at all? That's contradictory with the use case you have given. That
leads me into believing that we should not try to push a hard rule, and
just let the user manage what accesses it. We could indeed make the VT
always take priority, but then that would probably break some existing
applications. Then, is "repurposing a keyboard LED" enough of an action
to be able to decide that applications modifying that LED should be just
ignored? I doubt it.
> I understand the desire, however I still wonder if we should be
> extending legacy VT given that David wants to move it all to userspace.
I believe I have heard about moving the console implementation to
userland for more that a decide. Will that really happen any time?
In the meanwhile, we have broken capslock keys on e.g. all Debian and
Ubuntu systems for a couple of years already (because they have adopted
console-setup), and some ARM systems don't have keyboard LEDs at all
without a patched kernel.
Samuel
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route keyboard LEDs through the generic LEDs layer.
@ 2014-04-08 23:33 ` Samuel Thibault
0 siblings, 0 replies; 39+ messages in thread
From: Samuel Thibault @ 2014-04-08 23:33 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
Dmitry Torokhov, le Tue 08 Apr 2014 01:39:40 -0700, a ?crit :
> It is not about the VT, I am talking about pure input core. If I
> repurpose CapsLock LED for my WiFi indicator I do not want to go into
> other programs and teach them that they should stay away from trying to
> control this LED.
Err, but even without talking about repurposing Capslock LED for WiFi...
How is managed the conflict between the normal capslock behavior and
other programs' action on the underlying input device? I don't think
this patch does not introduce the problem.
Now you may answer "well, if it could fix the problem along the way it'd
be good". I'd like to answer "well, this is not my shit" :)
But still, the problem is interesting, and I don't know how to deal
properly with it. More precisely, I don't think we *want* to deal with
it. Currently, what happens is that there is no priority between what
the VT keyboard events produce and what applications produce, so things
happily mix with strange results as soon as a user tries to combine
both. My concern is:
How to decide which one to prioritize?
Is it just because console-setup happens to repurpose the capslock LED
key that applications should suddenly not have capslock LED control
at all? That's contradictory with the use case you have given. That
leads me into believing that we should not try to push a hard rule, and
just let the user manage what accesses it. We could indeed make the VT
always take priority, but then that would probably break some existing
applications. Then, is "repurposing a keyboard LED" enough of an action
to be able to decide that applications modifying that LED should be just
ignored? I doubt it.
> I understand the desire, however I still wonder if we should be
> extending legacy VT given that David wants to move it all to userspace.
I believe I have heard about moving the console implementation to
userland for more that a decide. Will that really happen any time?
In the meanwhile, we have broken capslock keys on e.g. all Debian and
Ubuntu systems for a couple of years already (because they have adopted
console-setup), and some ARM systems don't have keyboard LEDs at all
without a patched kernel.
Samuel
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route keyboard LEDs through the generic LEDs layer.
2014-04-08 23:33 ` Samuel Thibault
@ 2014-04-11 6:12 ` Samuel Thibault
-1 siblings, 0 replies; 39+ messages in thread
From: Samuel Thibault @ 2014-04-11 6:12 UTC (permalink / raw)
To: Dmitry Torokhov, Pavel Machek, David Herrmann, akpm, jslaby,
Bryan Wu, rpurdie, linux-kernel, Evan Broder, Arnaud Patard,
Peter Korsgaard, Sascha Hauer, Matt Sealey, Rob Clark,
Niels de Vos, linux-arm-kernel, Steev Klimaszewski, blogic,
Pali Rohár
Hello,
I'm sorry this went out with a few mistakes.
Samuel Thibault, le Wed 09 Apr 2014 01:33:06 +0200, a écrit :
> Dmitry Torokhov, le Tue 08 Apr 2014 01:39:40 -0700, a écrit :
> > It is not about the VT, I am talking about pure input core. If I
> > repurpose CapsLock LED for my WiFi indicator I do not want to go into
> > other programs and teach them that they should stay away from trying to
> > control this LED.
>
> Err, but even without talking about repurposing Capslock LED for WiFi...
> How is managed the conflict between the normal capslock behavior and
> other programs' action on the underlying input device? I don't think
> this patch does not introduce the problem.
I of course meant "I don't think this patch introduces the problem".
> How to decide which one to prioritize?
>
> Is it just because console-setup happens to repurpose the capslock LED
> key that applications should suddenly not have capslock LED control
> at all? That's contradictory with the use case you have given.
Oops, not the use case you have given, but a typical use-case of wanting
to use a program which does nice things with the capslock LED, and then
having to teach console-setup not to repurpose the capslock LED.
> That
> leads me into believing that we should not try to push a hard rule, and
> just let the user manage what accesses it. We could indeed make the VT
> always take priority, but then that would probably break some existing
> applications.
Such as the example above, with the capslock LED.
Samuel
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route keyboard LEDs through the generic LEDs layer.
@ 2014-04-11 6:12 ` Samuel Thibault
0 siblings, 0 replies; 39+ messages in thread
From: Samuel Thibault @ 2014-04-11 6:12 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
I'm sorry this went out with a few mistakes.
Samuel Thibault, le Wed 09 Apr 2014 01:33:06 +0200, a ?crit :
> Dmitry Torokhov, le Tue 08 Apr 2014 01:39:40 -0700, a ?crit :
> > It is not about the VT, I am talking about pure input core. If I
> > repurpose CapsLock LED for my WiFi indicator I do not want to go into
> > other programs and teach them that they should stay away from trying to
> > control this LED.
>
> Err, but even without talking about repurposing Capslock LED for WiFi...
> How is managed the conflict between the normal capslock behavior and
> other programs' action on the underlying input device? I don't think
> this patch does not introduce the problem.
I of course meant "I don't think this patch introduces the problem".
> How to decide which one to prioritize?
>
> Is it just because console-setup happens to repurpose the capslock LED
> key that applications should suddenly not have capslock LED control
> at all? That's contradictory with the use case you have given.
Oops, not the use case you have given, but a typical use-case of wanting
to use a program which does nice things with the capslock LED, and then
having to teach console-setup not to repurpose the capslock LED.
> That
> leads me into believing that we should not try to push a hard rule, and
> just let the user manage what accesses it. We could indeed make the VT
> always take priority, but then that would probably break some existing
> applications.
Such as the example above, with the capslock LED.
Samuel
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route keyboard LEDs through the generic LEDs layer.
2014-04-08 8:39 ` Dmitry Torokhov
@ 2014-04-12 10:09 ` Pavel Machek
-1 siblings, 0 replies; 39+ messages in thread
From: Pavel Machek @ 2014-04-12 10:09 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Samuel Thibault, David Herrmann, akpm, jslaby, Bryan Wu, rpurdie,
linux-kernel, Evan Broder, Arnaud Patard, Peter Korsgaard,
Sascha Hauer, Matt Sealey, Rob Clark, Niels de Vos,
linux-arm-kernel, Steev Klimaszewski, blogic, Pali Rohár
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.
> > > >
> > >
> > > I still have the same concern that I believe I already mentioned a while
> > > ago: how do we reconcile the LED control via triggers with LED control
> > > done through event devices?
I think we should deprecate LED controls using event devices. We have
nice API for LEDs, and it is not in input.
> > > Currently, as far as I can see, they will be
> > > clashing with each other. I.e. if I remap my capslock led to be the new
> > > shiftlock and then userspace writes EV_LED/LED_CAPSL it would light up
> > > my new "shift lock", right?
> >
> > Well, yes, sure, if you shoot in your foot it will hurt :) (although
> > here the damage is really small, it is just LED lighting or not).
>
> This is not about amount of damage but the overall correctness of the
> implementation. I'd rather not have a solution with known holes, if
> possible.
I'd say that applications using direct EV_LED interface should just
stop doing it. Yes, you can probably use led API and still toggle the
led using gpio api behind leds back (not tested, perhaps there are
interlocks that prevent that)... and it is same situation with
EV_LED. We should just teach applications not to do that.
Would solution where EV_LED would be ignored when there's non-default
trigger selected work for you?
> > My goal, at the beginning, was to be able to fix the console-setup
> > bug. That was that console-setup wants to use, for the capslock key,
> > something else than the capslock mechanism hard-wired in the kernel,
> > so as to get way more fine-grain control over how caps is handled.
> > But then the capslock LED would not lit any more. By remapping the
> > VT-level capslock led to the foo-lock used by console-setup, one gets
> > back capslock LEDs of all keyboards working back as expected by the
> > user. I also use it for getting a group LED, just like I have on X,
> > to know which keyboard layout group I am in. People using non-latin
> > layouts really need that.
>
> I understand the desire, however I still wonder if we should be
> extending legacy VT given that David wants to move it all to
> userspace.
It looks like legacy VT is going to stay here for long long time...
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 keyboard LEDs through the generic LEDs layer.
@ 2014-04-12 10:09 ` Pavel Machek
0 siblings, 0 replies; 39+ messages in thread
From: Pavel Machek @ 2014-04-12 10:09 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.
> > > >
> > >
> > > I still have the same concern that I believe I already mentioned a while
> > > ago: how do we reconcile the LED control via triggers with LED control
> > > done through event devices?
I think we should deprecate LED controls using event devices. We have
nice API for LEDs, and it is not in input.
> > > Currently, as far as I can see, they will be
> > > clashing with each other. I.e. if I remap my capslock led to be the new
> > > shiftlock and then userspace writes EV_LED/LED_CAPSL it would light up
> > > my new "shift lock", right?
> >
> > Well, yes, sure, if you shoot in your foot it will hurt :) (although
> > here the damage is really small, it is just LED lighting or not).
>
> This is not about amount of damage but the overall correctness of the
> implementation. I'd rather not have a solution with known holes, if
> possible.
I'd say that applications using direct EV_LED interface should just
stop doing it. Yes, you can probably use led API and still toggle the
led using gpio api behind leds back (not tested, perhaps there are
interlocks that prevent that)... and it is same situation with
EV_LED. We should just teach applications not to do that.
Would solution where EV_LED would be ignored when there's non-default
trigger selected work for you?
> > My goal, at the beginning, was to be able to fix the console-setup
> > bug. That was that console-setup wants to use, for the capslock key,
> > something else than the capslock mechanism hard-wired in the kernel,
> > so as to get way more fine-grain control over how caps is handled.
> > But then the capslock LED would not lit any more. By remapping the
> > VT-level capslock led to the foo-lock used by console-setup, one gets
> > back capslock LEDs of all keyboards working back as expected by the
> > user. I also use it for getting a group LED, just like I have on X,
> > to know which keyboard layout group I am in. People using non-latin
> > layouts really need that.
>
> I understand the desire, however I still wonder if we should be
> extending legacy VT given that David wants to move it all to
> userspace.
It looks like legacy VT is going to stay here for long long time...
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 keyboard LEDs through the generic LEDs layer.
2014-04-11 6:12 ` Samuel Thibault
@ 2014-04-13 1:25 ` Dmitry Torokhov
-1 siblings, 0 replies; 39+ messages in thread
From: Dmitry Torokhov @ 2014-04-13 1:25 UTC (permalink / raw)
To: Samuel Thibault, Pavel Machek, David Herrmann, akpm, jslaby,
Bryan Wu, rpurdie, linux-kernel, Evan Broder, Arnaud Patard,
Peter Korsgaard, Sascha Hauer, Matt Sealey, Rob Clark,
Niels de Vos, linux-arm-kernel, Steev Klimaszewski, blogic,
Pali Rohár
On Fri, Apr 11, 2014 at 08:12:02AM +0200, Samuel Thibault wrote:
> Hello,
>
> I'm sorry this went out with a few mistakes.
>
> Samuel Thibault, le Wed 09 Apr 2014 01:33:06 +0200, a écrit :
> > Dmitry Torokhov, le Tue 08 Apr 2014 01:39:40 -0700, a écrit :
> > > It is not about the VT, I am talking about pure input core. If I
> > > repurpose CapsLock LED for my WiFi indicator I do not want to go into
> > > other programs and teach them that they should stay away from trying to
> > > control this LED.
> >
> > Err, but even without talking about repurposing Capslock LED for WiFi...
> > How is managed the conflict between the normal capslock behavior and
> > other programs' action on the underlying input device? I don't think
> > this patch does not introduce the problem.
>
> I of course meant "I don't think this patch introduces the problem".
The difference in my eyes is that with old interface both players knew
that they would be affecting (and potentially interfering) with the
state of CapsLock LED. With your proposed changes users of old
interfaces have no idea if they are actually toggling CapsLock LED or if
they are affecting something that is no longer a CapsLock LED.
>
> > How to decide which one to prioritize?
> >
> > Is it just because console-setup happens to repurpose the capslock LED
> > key that applications should suddenly not have capslock LED control
> > at all? That's contradictory with the use case you have given.
>
> Oops, not the use case you have given, but a typical use-case of wanting
> to use a program which does nice things with the capslock LED, and then
> having to teach console-setup not to repurpose the capslock LED.
I believe that applications should be able to control sate of CapsLock
and other LEDs and that the affected LED should not be the physical LED
but rather LED that is currently tied to CapsLock trigger (if any). This
way everything works as is by default and if I decide to have my
physical CapsLock LED to be repurposed for Wifi or HDD activity or
whatever I do not need to teach unrelated applications to stop touching
it.
>
> > That
> > leads me into believing that we should not try to push a hard rule, and
> > just let the user manage what accesses it. We could indeed make the VT
> > always take priority, but then that would probably break some existing
> > applications.
>
> Such as the example above, with the capslock LED.
I am not saying that VT shoudl have priority, I am saying we need to
come up with implementation that does not result in inconsistent
behavior.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route keyboard LEDs through the generic LEDs layer.
@ 2014-04-13 1:25 ` Dmitry Torokhov
0 siblings, 0 replies; 39+ messages in thread
From: Dmitry Torokhov @ 2014-04-13 1:25 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Apr 11, 2014 at 08:12:02AM +0200, Samuel Thibault wrote:
> Hello,
>
> I'm sorry this went out with a few mistakes.
>
> Samuel Thibault, le Wed 09 Apr 2014 01:33:06 +0200, a ?crit :
> > Dmitry Torokhov, le Tue 08 Apr 2014 01:39:40 -0700, a ?crit :
> > > It is not about the VT, I am talking about pure input core. If I
> > > repurpose CapsLock LED for my WiFi indicator I do not want to go into
> > > other programs and teach them that they should stay away from trying to
> > > control this LED.
> >
> > Err, but even without talking about repurposing Capslock LED for WiFi...
> > How is managed the conflict between the normal capslock behavior and
> > other programs' action on the underlying input device? I don't think
> > this patch does not introduce the problem.
>
> I of course meant "I don't think this patch introduces the problem".
The difference in my eyes is that with old interface both players knew
that they would be affecting (and potentially interfering) with the
state of CapsLock LED. With your proposed changes users of old
interfaces have no idea if they are actually toggling CapsLock LED or if
they are affecting something that is no longer a CapsLock LED.
>
> > How to decide which one to prioritize?
> >
> > Is it just because console-setup happens to repurpose the capslock LED
> > key that applications should suddenly not have capslock LED control
> > at all? That's contradictory with the use case you have given.
>
> Oops, not the use case you have given, but a typical use-case of wanting
> to use a program which does nice things with the capslock LED, and then
> having to teach console-setup not to repurpose the capslock LED.
I believe that applications should be able to control sate of CapsLock
and other LEDs and that the affected LED should not be the physical LED
but rather LED that is currently tied to CapsLock trigger (if any). This
way everything works as is by default and if I decide to have my
physical CapsLock LED to be repurposed for Wifi or HDD activity or
whatever I do not need to teach unrelated applications to stop touching
it.
>
> > That
> > leads me into believing that we should not try to push a hard rule, and
> > just let the user manage what accesses it. We could indeed make the VT
> > always take priority, but then that would probably break some existing
> > applications.
>
> Such as the example above, with the capslock LED.
I am not saying that VT shoudl have priority, I am saying we need to
come up with implementation that does not result in inconsistent
behavior.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route keyboard LEDs through the generic LEDs layer.
2014-04-12 10:09 ` Pavel Machek
@ 2014-04-13 1:30 ` Dmitry Torokhov
-1 siblings, 0 replies; 39+ messages in thread
From: Dmitry Torokhov @ 2014-04-13 1:30 UTC (permalink / raw)
To: Pavel Machek
Cc: Samuel Thibault, David Herrmann, akpm, jslaby, Bryan Wu, rpurdie,
linux-kernel, Evan Broder, Arnaud Patard, Peter Korsgaard,
Sascha Hauer, Matt Sealey, Rob Clark, Niels de Vos,
linux-arm-kernel, Steev Klimaszewski, blogic, Pali Rohár
On Sat, Apr 12, 2014 at 12:09:34PM +0200, Pavel Machek wrote:
> 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.
> > > > >
> > > >
> > > > I still have the same concern that I believe I already mentioned a while
> > > > ago: how do we reconcile the LED control via triggers with LED control
> > > > done through event devices?
>
> I think we should deprecate LED controls using event devices. We have
> nice API for LEDs, and it is not in input.
>
>
>
>
> > > > Currently, as far as I can see, they will be
> > > > clashing with each other. I.e. if I remap my capslock led to be the new
> > > > shiftlock and then userspace writes EV_LED/LED_CAPSL it would light up
> > > > my new "shift lock", right?
> > >
> > > Well, yes, sure, if you shoot in your foot it will hurt :) (although
> > > here the damage is really small, it is just LED lighting or not).
> >
> > This is not about amount of damage but the overall correctness of the
> > implementation. I'd rather not have a solution with known holes, if
> > possible.
>
> I'd say that applications using direct EV_LED interface should just
> stop doing it. Yes, you can probably use led API and still toggle the
> led using gpio api behind leds back (not tested, perhaps there are
> interlocks that prevent that)... and it is same situation with
> EV_LED. We should just teach applications not to do that.
>
> Would solution where EV_LED would be ignored when there's non-default
> trigger selected work for you?
Not ignored but rather routed to the LED that is currently selected for
given function. This way if I re-purposed CapsLock LED for Wifi and do
not provide a replacement EV_LED will be effectively dropped, but if I
switch CapsLock with NumLock I want the event to affect the appropriate
LED.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route keyboard LEDs through the generic LEDs layer.
@ 2014-04-13 1:30 ` Dmitry Torokhov
0 siblings, 0 replies; 39+ messages in thread
From: Dmitry Torokhov @ 2014-04-13 1:30 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Apr 12, 2014 at 12:09:34PM +0200, Pavel Machek wrote:
> 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.
> > > > >
> > > >
> > > > I still have the same concern that I believe I already mentioned a while
> > > > ago: how do we reconcile the LED control via triggers with LED control
> > > > done through event devices?
>
> I think we should deprecate LED controls using event devices. We have
> nice API for LEDs, and it is not in input.
>
>
>
>
> > > > Currently, as far as I can see, they will be
> > > > clashing with each other. I.e. if I remap my capslock led to be the new
> > > > shiftlock and then userspace writes EV_LED/LED_CAPSL it would light up
> > > > my new "shift lock", right?
> > >
> > > Well, yes, sure, if you shoot in your foot it will hurt :) (although
> > > here the damage is really small, it is just LED lighting or not).
> >
> > This is not about amount of damage but the overall correctness of the
> > implementation. I'd rather not have a solution with known holes, if
> > possible.
>
> I'd say that applications using direct EV_LED interface should just
> stop doing it. Yes, you can probably use led API and still toggle the
> led using gpio api behind leds back (not tested, perhaps there are
> interlocks that prevent that)... and it is same situation with
> EV_LED. We should just teach applications not to do that.
>
> Would solution where EV_LED would be ignored when there's non-default
> trigger selected work for you?
Not ignored but rather routed to the LED that is currently selected for
given function. This way if I re-purposed CapsLock LED for Wifi and do
not provide a replacement EV_LED will be effectively dropped, but if I
switch CapsLock with NumLock I want the event to affect the appropriate
LED.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route keyboard LEDs through the generic LEDs layer.
2014-04-13 1:25 ` Dmitry Torokhov
@ 2014-04-16 23:38 ` Samuel Thibault
-1 siblings, 0 replies; 39+ messages in thread
From: Samuel Thibault @ 2014-04-16 23:38 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Pavel Machek, David Herrmann, akpm, jslaby, Bryan Wu, rpurdie,
linux-kernel, Evan Broder, Arnaud Patard, Peter Korsgaard,
Sascha Hauer, Matt Sealey, Rob Clark, Niels de Vos,
linux-arm-kernel, Steev Klimaszewski, blogic, Pali Rohár
Hello,
Dmitry Torokhov, le Sat 12 Apr 2014 18:25:57 -0700, a écrit :
> On Fri, Apr 11, 2014 at 08:12:02AM +0200, Samuel Thibault wrote:
> > I'm sorry this went out with a few mistakes.
> >
> > Samuel Thibault, le Wed 09 Apr 2014 01:33:06 +0200, a écrit :
> > > Dmitry Torokhov, le Tue 08 Apr 2014 01:39:40 -0700, a écrit :
> > > > It is not about the VT, I am talking about pure input core. If I
> > > > repurpose CapsLock LED for my WiFi indicator I do not want to go into
> > > > other programs and teach them that they should stay away from trying to
> > > > control this LED.
> > >
> > > Err, but even without talking about repurposing Capslock LED for WiFi...
> > > How is managed the conflict between the normal capslock behavior and
> > > other programs' action on the underlying input device? I don't think
> > > this patch does not introduce the problem.
> >
> > I of course meant "I don't think this patch introduces the problem".
>
> The difference in my eyes is that with old interface both players knew
> that they would be affecting (and potentially interfering) with the
> state of CapsLock LED. With your proposed changes users of old
> interfaces have no idea if they are actually toggling CapsLock LED or if
> they are affecting something that is no longer a CapsLock LED.
Mmm, I thought you were talking about evdev programs (which would always
get to manipulate the hardware capslock leds anyway)?
Are you here talking about KDSETLEDS? This is a very odd ioctl
actually. Its name suggests that its purpose is to just light LEDs,
but AFAIK it is essentially used by users to change the status of the
keyboard state, and the LED change is just a side effect for them. So
if e.g. the numlock LED is repurposed to wifi, should KDSETLEDS really
just switch on the LED (thus defeating the wifi purpose), while the user
probably used setleds +num only to get its numlock enabled by default
(and doesn't care about seeing that on the keyboard, on the contrary, he
preferred to see the wifi state).
> > > How to decide which one to prioritize?
> > >
> > > Is it just because console-setup happens to repurpose the capslock LED
> > > key that applications should suddenly not have capslock LED control
> > > at all? That's contradictory with the use case you have given.
> >
> > Oops, not the use case you have given, but a typical use-case of wanting
> > to use a program which does nice things with the capslock LED, and then
> > having to teach console-setup not to repurpose the capslock LED.
>
> I believe that applications should be able to control sate of CapsLock
> and other LEDs and that the affected LED should not be the physical LED
> but rather LED that is currently tied to CapsLock trigger (if any).
Which kind of applications are you talking about? What I have heard
from users was that they either wanted to have an effect on the physical
LED, or on the keyboard state, but not really "on the LED which shows
the keyboard state", which would just be a side consequence of what they
really want to achieve.
> This
> way everything works as is by default and if I decide to have my
> physical CapsLock LED to be repurposed for Wifi or HDD activity or
> whatever I do not need to teach unrelated applications to stop touching
> it.
Again, which applications are you thinking about precisely? If a
user has an application which wants to really show something on the
capslock LED, then the user's intent was really the physical LED, and he
hurted himself in the foot. Otherwise, what was the intention of the
application? I don't understand why an application would want to light
"the LED which shows the capslock state", and not simply the keyboard
capslock status (which thus may or may not be reflected on some physical
LED, depending whether some LED is plugged on the keyboard capslock
status).
> > > That
> > > leads me into believing that we should not try to push a hard rule, and
> > > just let the user manage what accesses it. We could indeed make the VT
> > > always take priority, but then that would probably break some existing
> > > applications.
> >
> > Such as the example above, with the capslock LED.
>
> I am not saying that VT shoudl have priority, I am saying we need to
> come up with implementation that does not result in inconsistent
> behavior.
Ok, but for now I don't see in which exact case we'd have an
inconsistency.
Dmitry Torokhov, le Sat 12 Apr 2014 18:30:49 -0700, a écrit :
> > I'd say that applications using direct EV_LED interface should just
> > stop doing it. Yes, you can probably use led API and still toggle the
> > led using gpio api behind leds back (not tested, perhaps there are
> > interlocks that prevent that)... and it is same situation with
> > EV_LED. We should just teach applications not to do that.
> >
> > Would solution where EV_LED would be ignored when there's non-default
> > trigger selected work for you?
>
> Not ignored but rather routed to the LED that is currently selected for
> given function. This way if I re-purposed CapsLock LED for Wifi and do
> not provide a replacement EV_LED will be effectively dropped, but if I
> switch CapsLock with NumLock I want the event to affect the appropriate
> LED.
Err, but are we really talking about the same layer? I thought
EV_LED was meant to drive hardware, not to drive any kind of software
abstraction on top of it. Adding an indirection to what EV_LED actually
means at least surely needs quite some heavy patching in the input
layer, I'm not even sure I really want to dive into that.
Adding an indirection there also looks to me like confusing the picture
even more. The way I see it is (using a monospace font):
kbd state -> kbd led -> kbd trigger -> vt led -> vt trigger -> input led
^^ ^^ ^^ ^^
KDSETLED XX YY EV_LED
XX is vt::capsl/trigger, and YY is input*::capsl/trigger. Applications
can either modify the keyboard state through KDSETLED, which may or may
not be shown by some LEDs depending on XX and YY, or modify the hardware
leds through EV_LED, which thus doesn't care about XX and YY.
The only "case" I see "broken" is applications which would be using
KDSETLED to just flash some keyboard LEDs (and happen to also tamper
with keyboard state...). With a default configuration they will still
work. If the LEDs are repurposed, the application effect won't show up
(unless the scroll/caps/num still appear in the LED reassignation, and
then the application effect will show up, with the repurpose exchanges).
In the end, I really don't see which use case you are thinking about
precisely.
Samuel
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route keyboard LEDs through the generic LEDs layer.
@ 2014-04-16 23:38 ` Samuel Thibault
0 siblings, 0 replies; 39+ messages in thread
From: Samuel Thibault @ 2014-04-16 23:38 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
Dmitry Torokhov, le Sat 12 Apr 2014 18:25:57 -0700, a ?crit :
> On Fri, Apr 11, 2014 at 08:12:02AM +0200, Samuel Thibault wrote:
> > I'm sorry this went out with a few mistakes.
> >
> > Samuel Thibault, le Wed 09 Apr 2014 01:33:06 +0200, a ?crit :
> > > Dmitry Torokhov, le Tue 08 Apr 2014 01:39:40 -0700, a ?crit :
> > > > It is not about the VT, I am talking about pure input core. If I
> > > > repurpose CapsLock LED for my WiFi indicator I do not want to go into
> > > > other programs and teach them that they should stay away from trying to
> > > > control this LED.
> > >
> > > Err, but even without talking about repurposing Capslock LED for WiFi...
> > > How is managed the conflict between the normal capslock behavior and
> > > other programs' action on the underlying input device? I don't think
> > > this patch does not introduce the problem.
> >
> > I of course meant "I don't think this patch introduces the problem".
>
> The difference in my eyes is that with old interface both players knew
> that they would be affecting (and potentially interfering) with the
> state of CapsLock LED. With your proposed changes users of old
> interfaces have no idea if they are actually toggling CapsLock LED or if
> they are affecting something that is no longer a CapsLock LED.
Mmm, I thought you were talking about evdev programs (which would always
get to manipulate the hardware capslock leds anyway)?
Are you here talking about KDSETLEDS? This is a very odd ioctl
actually. Its name suggests that its purpose is to just light LEDs,
but AFAIK it is essentially used by users to change the status of the
keyboard state, and the LED change is just a side effect for them. So
if e.g. the numlock LED is repurposed to wifi, should KDSETLEDS really
just switch on the LED (thus defeating the wifi purpose), while the user
probably used setleds +num only to get its numlock enabled by default
(and doesn't care about seeing that on the keyboard, on the contrary, he
preferred to see the wifi state).
> > > How to decide which one to prioritize?
> > >
> > > Is it just because console-setup happens to repurpose the capslock LED
> > > key that applications should suddenly not have capslock LED control
> > > at all? That's contradictory with the use case you have given.
> >
> > Oops, not the use case you have given, but a typical use-case of wanting
> > to use a program which does nice things with the capslock LED, and then
> > having to teach console-setup not to repurpose the capslock LED.
>
> I believe that applications should be able to control sate of CapsLock
> and other LEDs and that the affected LED should not be the physical LED
> but rather LED that is currently tied to CapsLock trigger (if any).
Which kind of applications are you talking about? What I have heard
from users was that they either wanted to have an effect on the physical
LED, or on the keyboard state, but not really "on the LED which shows
the keyboard state", which would just be a side consequence of what they
really want to achieve.
> This
> way everything works as is by default and if I decide to have my
> physical CapsLock LED to be repurposed for Wifi or HDD activity or
> whatever I do not need to teach unrelated applications to stop touching
> it.
Again, which applications are you thinking about precisely? If a
user has an application which wants to really show something on the
capslock LED, then the user's intent was really the physical LED, and he
hurted himself in the foot. Otherwise, what was the intention of the
application? I don't understand why an application would want to light
"the LED which shows the capslock state", and not simply the keyboard
capslock status (which thus may or may not be reflected on some physical
LED, depending whether some LED is plugged on the keyboard capslock
status).
> > > That
> > > leads me into believing that we should not try to push a hard rule, and
> > > just let the user manage what accesses it. We could indeed make the VT
> > > always take priority, but then that would probably break some existing
> > > applications.
> >
> > Such as the example above, with the capslock LED.
>
> I am not saying that VT shoudl have priority, I am saying we need to
> come up with implementation that does not result in inconsistent
> behavior.
Ok, but for now I don't see in which exact case we'd have an
inconsistency.
Dmitry Torokhov, le Sat 12 Apr 2014 18:30:49 -0700, a ?crit :
> > I'd say that applications using direct EV_LED interface should just
> > stop doing it. Yes, you can probably use led API and still toggle the
> > led using gpio api behind leds back (not tested, perhaps there are
> > interlocks that prevent that)... and it is same situation with
> > EV_LED. We should just teach applications not to do that.
> >
> > Would solution where EV_LED would be ignored when there's non-default
> > trigger selected work for you?
>
> Not ignored but rather routed to the LED that is currently selected for
> given function. This way if I re-purposed CapsLock LED for Wifi and do
> not provide a replacement EV_LED will be effectively dropped, but if I
> switch CapsLock with NumLock I want the event to affect the appropriate
> LED.
Err, but are we really talking about the same layer? I thought
EV_LED was meant to drive hardware, not to drive any kind of software
abstraction on top of it. Adding an indirection to what EV_LED actually
means at least surely needs quite some heavy patching in the input
layer, I'm not even sure I really want to dive into that.
Adding an indirection there also looks to me like confusing the picture
even more. The way I see it is (using a monospace font):
kbd state -> kbd led -> kbd trigger -> vt led -> vt trigger -> input led
^^ ^^ ^^ ^^
KDSETLED XX YY EV_LED
XX is vt::capsl/trigger, and YY is input*::capsl/trigger. Applications
can either modify the keyboard state through KDSETLED, which may or may
not be shown by some LEDs depending on XX and YY, or modify the hardware
leds through EV_LED, which thus doesn't care about XX and YY.
The only "case" I see "broken" is applications which would be using
KDSETLED to just flash some keyboard LEDs (and happen to also tamper
with keyboard state...). With a default configuration they will still
work. If the LEDs are repurposed, the application effect won't show up
(unless the scroll/caps/num still appear in the LED reassignation, and
then the application effect will show up, with the repurpose exchanges).
In the end, I really don't see which use case you are thinking about
precisely.
Samuel
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route keyboard LEDs through the generic LEDs layer.
2014-03-31 12:23 ` Samuel Thibault
@ 2014-10-01 18:42 ` Andrew Morton
-1 siblings, 0 replies; 39+ messages in thread
From: Andrew Morton @ 2014-10-01 18:42 UTC (permalink / raw)
To: Samuel Thibault
Cc: Dmitry Torokhov, Pavel Machek, David Herrmann, jslaby, Bryan Wu,
rpurdie, linux-kernel, Evan Broder, Arnaud Patard,
Peter Korsgaard, Sascha Hauer, Matt Sealey, Rob Clark,
Niels de Vos, linux-arm-kernel, Steev Klimaszewski, blogic,
Pali Rohár
On Mon, 31 Mar 2014 14:23:23 +0200 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.
When this patch is combined with current linux-next I'm getting the
below lockdep splat. Config: http://ozlabs.org/~akpm/config-akpm2.txt
[ 4.304279]
[ 4.304280] =============================================
[ 4.304281] [ INFO: possible recursive locking detected ]
[ 4.304283] 3.17.0-rc7-mm1 #5 Not tainted
[ 4.304284] ---------------------------------------------
[ 4.304285] kworker/6:1/398 is trying to acquire lock:
[ 4.304294] (&trig->leddev_list_lock){.+.?..}, at: [<ffffffff81385474>] led_trigger_event+0x22/0x6b
[ 4.304294]
[ 4.304294] but task is already holding lock:
[ 4.304298] (&trig->leddev_list_lock){.+.?..}, at: [<ffffffff81385474>] led_trigger_event+0x22/0x6b
[ 4.304299]
[ 4.304299] other info that might help us debug this:
[ 4.304300] Possible unsafe locking scenario:
[ 4.304300]
[ 4.304301] CPU0
[ 4.304301] ----
[ 4.304303] lock(&trig->leddev_list_lock);
[ 4.304305] lock(&trig->leddev_list_lock);
[ 4.304306]
[ 4.304306] *** DEADLOCK ***
[ 4.304306]
[ 4.304307] May be due to missing lock nesting notation
[ 4.304307]
[ 4.304308] 11 locks held by kworker/6:1/398:
[ 4.304315] #0: ("events_long"){.+.+.+}, at: [<ffffffff810542db>] process_one_work+0x1af/0x39d
[ 4.304319] #1: (serio_event_work){+.+.+.}, at: [<ffffffff810542db>] process_one_work+0x1af/0x39d
[ 4.304326] #2: (serio_mutex){+.+.+.}, at: [<ffffffff81350c83>] serio_handle_event+0x19/0x1f1
[ 4.304332] #3: (&dev->mutex){......}, at: [<ffffffff812b6fb9>] __driver_attach+0x39/0x80
[ 4.304336] #4: (&dev->mutex){......}, at: [<ffffffff812b6fc7>] __driver_attach+0x47/0x80
[ 4.304341] #5: (&serio->drv_mutex){+.+.+.}, at: [<ffffffff813505cd>] serio_connect_driver+0x24/0x48
[ 4.304346] #6: (input_mutex){+.+.+.}, at: [<ffffffff813554d2>] input_register_device+0x2e9/0x3c0
[ 4.304351] #7: (vt_led_registered_lock){+.+.+.}, at: [<ffffffff81357a0d>] input_led_connect+0x50/0x1ff
[ 4.304355] #8: (triggers_list_lock){++++.+}, at: [<ffffffff813858dc>] led_trigger_set_default+0x2b/0x88
[ 4.304359] #9: (&led_cdev->trigger_lock){+.+.+.}, at: [<ffffffff813858e4>] led_trigger_set_default+0x33/0x88
[ 4.304363] #10: (&trig->leddev_list_lock){.+.?..}, at: [<ffffffff81385474>] led_trigger_event+0x22/0x6b
[ 4.304364]
[ 4.304364] stack backtrace:
[ 4.304367] CPU: 6 PID: 398 Comm: kworker/6:1 Not tainted 3.17.0-rc7-mm1 #5
[ 4.304368] Hardware name: , BIOS Bridgeport CRB BIOS 73 external 2006-08-05
[ 4.304371] Workqueue: events_long serio_handle_event
[ 4.304374] ffffffff82190450 ffff880255b577f8 ffffffff81463e05 ffffffff82a886e0
[ 4.304377] ffff880255890850 ffff880255b578b8 ffffffff81076fef ffff880255b57838
[ 4.304379] ffffffff82190450 ffff880200000000 ffff880255891258 ffff880355b57848
[ 4.304380] Call Trace:
[ 4.304384] [<ffffffff81463e05>] dump_stack+0x49/0x5c
[ 4.304387] [<ffffffff81076fef>] validate_chain+0x741/0xf93
[ 4.304390] [<ffffffff81074ef4>] ? check_usage_backwards+0x9a/0xd3
[ 4.304394] [<ffffffff81063689>] ? sched_clock_local+0x1c/0x82
[ 4.304396] [<ffffffff810781bb>] __lock_acquire+0x97a/0xa29
[ 4.304398] [<ffffffff81074248>] ? mark_lock+0x475/0x5a4
[ 4.304400] [<ffffffff81078314>] lock_acquire+0xaa/0xc4
[ 4.304402] [<ffffffff81385474>] ? led_trigger_event+0x22/0x6b
[ 4.304405] [<ffffffff81468da2>] _raw_read_lock+0x34/0x69
[ 4.304408] [<ffffffff81385474>] ? led_trigger_event+0x22/0x6b
[ 4.304410] [<ffffffff81385474>] led_trigger_event+0x22/0x6b
[ 4.304412] [<ffffffff81357831>] vt_led_set+0x32/0x34
[ 4.304415] [<ffffffff81384e23>] led_set_brightness+0x49/0x4b
[ 4.304417] [<ffffffff81385490>] led_trigger_event+0x3e/0x6b
[ 4.304421] [<ffffffff812924ff>] kbd_ledstate_trigger_activate+0x49/0x52
[ 4.304423] [<ffffffff813855f8>] led_trigger_set+0xfd/0x13b
[ 4.304426] [<ffffffff81467227>] ? down_write+0x8c/0xa4
[ 4.304428] [<ffffffff813858e4>] ? led_trigger_set_default+0x33/0x88
[ 4.304430] [<ffffffff81385909>] led_trigger_set_default+0x58/0x88
[ 4.304433] [<ffffffff813852b2>] led_classdev_register+0x124/0x12f
[ 4.304435] [<ffffffff81357a70>] input_led_connect+0xb3/0x1ff
[ 4.304437] [<ffffffff8135550a>] input_register_device+0x321/0x3c0
[ 4.304441] [<ffffffff8135cd67>] atkbd_connect+0x235/0x27e
[ 4.304443] [<ffffffff813505d8>] serio_connect_driver+0x2f/0x48
[ 4.304447] [<ffffffff8117efe5>] ? sysfs_create_link+0x2a/0x2c
[ 4.304449] [<ffffffff8135060c>] serio_driver_probe+0x1b/0x1d
[ 4.304451] [<ffffffff812b6e6e>] driver_probe_device+0xac/0x1be
[ 4.304453] [<ffffffff812b6fdc>] __driver_attach+0x5c/0x80
[ 4.304455] [<ffffffff812b6f80>] ? driver_probe_device+0x1be/0x1be
[ 4.304457] [<ffffffff812b56e6>] bus_for_each_dev+0x56/0x94
[ 4.304460] [<ffffffff812b6be2>] driver_attach+0x19/0x1b
[ 4.304462] [<ffffffff81350dcd>] serio_handle_event+0x163/0x1f1
[ 4.304464] [<ffffffff81054342>] process_one_work+0x216/0x39d
[ 4.304466] [<ffffffff810542db>] ? process_one_work+0x1af/0x39d
[ 4.304468] [<ffffffff8105482f>] worker_thread+0x366/0x442
[ 4.304470] [<ffffffff810747e0>] ? trace_hardirqs_on+0xd/0xf
[ 4.304472] [<ffffffff810544c9>] ? process_one_work+0x39d/0x39d
[ 4.304475] [<ffffffff81058390>] kthread+0xe1/0xe9
[ 4.304478] [<ffffffff810582af>] ? __init_kthread_worker+0x56/0x56
[ 4.304481] [<ffffffff8146922c>] ret_from_fork+0x7c/0xb0
[ 4.304483] [<ffffffff810582af>] ? __init_kthread_worker+0x56/0x56
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route keyboard LEDs through the generic LEDs layer.
@ 2014-10-01 18:42 ` Andrew Morton
0 siblings, 0 replies; 39+ messages in thread
From: Andrew Morton @ 2014-10-01 18:42 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 31 Mar 2014 14:23:23 +0200 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.
When this patch is combined with current linux-next I'm getting the
below lockdep splat. Config: http://ozlabs.org/~akpm/config-akpm2.txt
[ 4.304279]
[ 4.304280] =============================================
[ 4.304281] [ INFO: possible recursive locking detected ]
[ 4.304283] 3.17.0-rc7-mm1 #5 Not tainted
[ 4.304284] ---------------------------------------------
[ 4.304285] kworker/6:1/398 is trying to acquire lock:
[ 4.304294] (&trig->leddev_list_lock){.+.?..}, at: [<ffffffff81385474>] led_trigger_event+0x22/0x6b
[ 4.304294]
[ 4.304294] but task is already holding lock:
[ 4.304298] (&trig->leddev_list_lock){.+.?..}, at: [<ffffffff81385474>] led_trigger_event+0x22/0x6b
[ 4.304299]
[ 4.304299] other info that might help us debug this:
[ 4.304300] Possible unsafe locking scenario:
[ 4.304300]
[ 4.304301] CPU0
[ 4.304301] ----
[ 4.304303] lock(&trig->leddev_list_lock);
[ 4.304305] lock(&trig->leddev_list_lock);
[ 4.304306]
[ 4.304306] *** DEADLOCK ***
[ 4.304306]
[ 4.304307] May be due to missing lock nesting notation
[ 4.304307]
[ 4.304308] 11 locks held by kworker/6:1/398:
[ 4.304315] #0: ("events_long"){.+.+.+}, at: [<ffffffff810542db>] process_one_work+0x1af/0x39d
[ 4.304319] #1: (serio_event_work){+.+.+.}, at: [<ffffffff810542db>] process_one_work+0x1af/0x39d
[ 4.304326] #2: (serio_mutex){+.+.+.}, at: [<ffffffff81350c83>] serio_handle_event+0x19/0x1f1
[ 4.304332] #3: (&dev->mutex){......}, at: [<ffffffff812b6fb9>] __driver_attach+0x39/0x80
[ 4.304336] #4: (&dev->mutex){......}, at: [<ffffffff812b6fc7>] __driver_attach+0x47/0x80
[ 4.304341] #5: (&serio->drv_mutex){+.+.+.}, at: [<ffffffff813505cd>] serio_connect_driver+0x24/0x48
[ 4.304346] #6: (input_mutex){+.+.+.}, at: [<ffffffff813554d2>] input_register_device+0x2e9/0x3c0
[ 4.304351] #7: (vt_led_registered_lock){+.+.+.}, at: [<ffffffff81357a0d>] input_led_connect+0x50/0x1ff
[ 4.304355] #8: (triggers_list_lock){++++.+}, at: [<ffffffff813858dc>] led_trigger_set_default+0x2b/0x88
[ 4.304359] #9: (&led_cdev->trigger_lock){+.+.+.}, at: [<ffffffff813858e4>] led_trigger_set_default+0x33/0x88
[ 4.304363] #10: (&trig->leddev_list_lock){.+.?..}, at: [<ffffffff81385474>] led_trigger_event+0x22/0x6b
[ 4.304364]
[ 4.304364] stack backtrace:
[ 4.304367] CPU: 6 PID: 398 Comm: kworker/6:1 Not tainted 3.17.0-rc7-mm1 #5
[ 4.304368] Hardware name: , BIOS Bridgeport CRB BIOS 73 external 2006-08-05
[ 4.304371] Workqueue: events_long serio_handle_event
[ 4.304374] ffffffff82190450 ffff880255b577f8 ffffffff81463e05 ffffffff82a886e0
[ 4.304377] ffff880255890850 ffff880255b578b8 ffffffff81076fef ffff880255b57838
[ 4.304379] ffffffff82190450 ffff880200000000 ffff880255891258 ffff880355b57848
[ 4.304380] Call Trace:
[ 4.304384] [<ffffffff81463e05>] dump_stack+0x49/0x5c
[ 4.304387] [<ffffffff81076fef>] validate_chain+0x741/0xf93
[ 4.304390] [<ffffffff81074ef4>] ? check_usage_backwards+0x9a/0xd3
[ 4.304394] [<ffffffff81063689>] ? sched_clock_local+0x1c/0x82
[ 4.304396] [<ffffffff810781bb>] __lock_acquire+0x97a/0xa29
[ 4.304398] [<ffffffff81074248>] ? mark_lock+0x475/0x5a4
[ 4.304400] [<ffffffff81078314>] lock_acquire+0xaa/0xc4
[ 4.304402] [<ffffffff81385474>] ? led_trigger_event+0x22/0x6b
[ 4.304405] [<ffffffff81468da2>] _raw_read_lock+0x34/0x69
[ 4.304408] [<ffffffff81385474>] ? led_trigger_event+0x22/0x6b
[ 4.304410] [<ffffffff81385474>] led_trigger_event+0x22/0x6b
[ 4.304412] [<ffffffff81357831>] vt_led_set+0x32/0x34
[ 4.304415] [<ffffffff81384e23>] led_set_brightness+0x49/0x4b
[ 4.304417] [<ffffffff81385490>] led_trigger_event+0x3e/0x6b
[ 4.304421] [<ffffffff812924ff>] kbd_ledstate_trigger_activate+0x49/0x52
[ 4.304423] [<ffffffff813855f8>] led_trigger_set+0xfd/0x13b
[ 4.304426] [<ffffffff81467227>] ? down_write+0x8c/0xa4
[ 4.304428] [<ffffffff813858e4>] ? led_trigger_set_default+0x33/0x88
[ 4.304430] [<ffffffff81385909>] led_trigger_set_default+0x58/0x88
[ 4.304433] [<ffffffff813852b2>] led_classdev_register+0x124/0x12f
[ 4.304435] [<ffffffff81357a70>] input_led_connect+0xb3/0x1ff
[ 4.304437] [<ffffffff8135550a>] input_register_device+0x321/0x3c0
[ 4.304441] [<ffffffff8135cd67>] atkbd_connect+0x235/0x27e
[ 4.304443] [<ffffffff813505d8>] serio_connect_driver+0x2f/0x48
[ 4.304447] [<ffffffff8117efe5>] ? sysfs_create_link+0x2a/0x2c
[ 4.304449] [<ffffffff8135060c>] serio_driver_probe+0x1b/0x1d
[ 4.304451] [<ffffffff812b6e6e>] driver_probe_device+0xac/0x1be
[ 4.304453] [<ffffffff812b6fdc>] __driver_attach+0x5c/0x80
[ 4.304455] [<ffffffff812b6f80>] ? driver_probe_device+0x1be/0x1be
[ 4.304457] [<ffffffff812b56e6>] bus_for_each_dev+0x56/0x94
[ 4.304460] [<ffffffff812b6be2>] driver_attach+0x19/0x1b
[ 4.304462] [<ffffffff81350dcd>] serio_handle_event+0x163/0x1f1
[ 4.304464] [<ffffffff81054342>] process_one_work+0x216/0x39d
[ 4.304466] [<ffffffff810542db>] ? process_one_work+0x1af/0x39d
[ 4.304468] [<ffffffff8105482f>] worker_thread+0x366/0x442
[ 4.304470] [<ffffffff810747e0>] ? trace_hardirqs_on+0xd/0xf
[ 4.304472] [<ffffffff810544c9>] ? process_one_work+0x39d/0x39d
[ 4.304475] [<ffffffff81058390>] kthread+0xe1/0xe9
[ 4.304478] [<ffffffff810582af>] ? __init_kthread_worker+0x56/0x56
[ 4.304481] [<ffffffff8146922c>] ret_from_fork+0x7c/0xb0
[ 4.304483] [<ffffffff810582af>] ? __init_kthread_worker+0x56/0x56
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route keyboard LEDs through the generic LEDs layer.
2014-10-01 18:42 ` Andrew Morton
@ 2014-10-01 21:29 ` Samuel Thibault
-1 siblings, 0 replies; 39+ messages in thread
From: Samuel Thibault @ 2014-10-01 21:29 UTC (permalink / raw)
To: Andrew Morton
Cc: Dmitry Torokhov, Pavel Machek, David Herrmann, jslaby, Bryan Wu,
rpurdie, linux-kernel, Evan Broder, Arnaud Patard,
Peter Korsgaard, Sascha Hauer, Matt Sealey, Rob Clark,
Niels de Vos, linux-arm-kernel, Steev Klimaszewski, blogic,
Pali Rohár
Andrew Morton, le Wed 01 Oct 2014 11:42:57 -0700, a écrit :
> On Mon, 31 Mar 2014 14:23:23 +0200 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.
>
> When this patch is combined with current linux-next I'm getting the
> below lockdep splat. Config: http://ozlabs.org/~akpm/config-akpm2.txt
Yes, this was reported and I sent a fix some time ago (Tue, 26 Aug 2014
11:17:25 +0200), here is the patch again:
Subject: Defer input led work to workqueue
When the kbd changes its led state (e.g. caps lock), this triggers
(led_trigger_event) the kbd-capsl trigger, which is by default
used by the vt::capsl LED, which triggers (led_trigger_event) the
vt-capsl trigger. These two nested led_trigger_event calls take a
trig->leddev_list_lock lock and thus lockdep complains.
Actually the user can make the vt::capsl LED use its own vt-capsl
trigger and thus build a loop. This produces an immediate oops.
This changeset defers the second led_trigger_event call into a
workqueue, which avoids the nested locking altogether. This does
not prevent the user from shooting himself in the foot by creating a
vt::capsl <-> vt-capsl loop, but the only consequence is the workqueue
threads eating some CPU until the user breaks the loop, which is not too
bad.
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
--- a/drivers/input/leds.c
+++ b/drivers/input/leds.c
@@ -100,13 +100,24 @@ static unsigned long vt_led_registered[B
/* Number of input devices having each LED */
static int vt_led_references[LED_CNT];
+static int vt_led_state[LED_CNT];
+static struct work_struct vt_led_work[LED_CNT];
+
+static void vt_led_cb(struct work_struct *work)
+{
+ int led = work - vt_led_work;
+
+ led_trigger_event(&vt_led_triggers[led], vt_led_state[led]);
+}
+
/* 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);
+ vt_led_state[led] = !!brightness;
+ schedule_work(&vt_led_work[led]);
}
/* LED state change for some keyboard, notify that keyboard. */
@@ -244,6 +255,22 @@ void input_led_disconnect(struct input_d
mutex_unlock(&vt_led_registered_lock);
}
+static int __init input_led_init(void)
+{
+ unsigned i;
+
+ for (i = 0; i < LED_CNT; i++)
+ INIT_WORK(&vt_led_work[i], vt_led_cb);
+
+ return 0;
+}
+
+static void __exit input_led_exit(void)
+{
+}
+
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("User LED support for input layer");
MODULE_AUTHOR("Samuel Thibault <samuel.thibault@ens-lyon.org>");
+module_init(input_led_init);
+module_exit(input_led_exit);
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route keyboard LEDs through the generic LEDs layer.
@ 2014-10-01 21:29 ` Samuel Thibault
0 siblings, 0 replies; 39+ messages in thread
From: Samuel Thibault @ 2014-10-01 21:29 UTC (permalink / raw)
To: linux-arm-kernel
Andrew Morton, le Wed 01 Oct 2014 11:42:57 -0700, a ?crit :
> On Mon, 31 Mar 2014 14:23:23 +0200 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.
>
> When this patch is combined with current linux-next I'm getting the
> below lockdep splat. Config: http://ozlabs.org/~akpm/config-akpm2.txt
Yes, this was reported and I sent a fix some time ago (Tue, 26 Aug 2014
11:17:25 +0200), here is the patch again:
Subject: Defer input led work to workqueue
When the kbd changes its led state (e.g. caps lock), this triggers
(led_trigger_event) the kbd-capsl trigger, which is by default
used by the vt::capsl LED, which triggers (led_trigger_event) the
vt-capsl trigger. These two nested led_trigger_event calls take a
trig->leddev_list_lock lock and thus lockdep complains.
Actually the user can make the vt::capsl LED use its own vt-capsl
trigger and thus build a loop. This produces an immediate oops.
This changeset defers the second led_trigger_event call into a
workqueue, which avoids the nested locking altogether. This does
not prevent the user from shooting himself in the foot by creating a
vt::capsl <-> vt-capsl loop, but the only consequence is the workqueue
threads eating some CPU until the user breaks the loop, which is not too
bad.
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
--- a/drivers/input/leds.c
+++ b/drivers/input/leds.c
@@ -100,13 +100,24 @@ static unsigned long vt_led_registered[B
/* Number of input devices having each LED */
static int vt_led_references[LED_CNT];
+static int vt_led_state[LED_CNT];
+static struct work_struct vt_led_work[LED_CNT];
+
+static void vt_led_cb(struct work_struct *work)
+{
+ int led = work - vt_led_work;
+
+ led_trigger_event(&vt_led_triggers[led], vt_led_state[led]);
+}
+
/* 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);
+ vt_led_state[led] = !!brightness;
+ schedule_work(&vt_led_work[led]);
}
/* LED state change for some keyboard, notify that keyboard. */
@@ -244,6 +255,22 @@ void input_led_disconnect(struct input_d
mutex_unlock(&vt_led_registered_lock);
}
+static int __init input_led_init(void)
+{
+ unsigned i;
+
+ for (i = 0; i < LED_CNT; i++)
+ INIT_WORK(&vt_led_work[i], vt_led_cb);
+
+ return 0;
+}
+
+static void __exit input_led_exit(void)
+{
+}
+
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("User LED support for input layer");
MODULE_AUTHOR("Samuel Thibault <samuel.thibault@ens-lyon.org>");
+module_init(input_led_init);
+module_exit(input_led_exit);
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route keyboard LEDs through the generic LEDs layer.
2014-10-01 21:29 ` Samuel Thibault
@ 2014-10-12 14:52 ` Pali Rohár
-1 siblings, 0 replies; 39+ messages in thread
From: Pali Rohár @ 2014-10-12 14:52 UTC (permalink / raw)
To: Samuel Thibault
Cc: Andrew Morton, Dmitry Torokhov, Pavel Machek, David Herrmann,
jslaby, Bryan Wu, rpurdie, linux-kernel, Evan Broder,
Arnaud Patard, Peter Korsgaard, Sascha Hauer, Matt Sealey,
Rob Clark, Niels de Vos, linux-arm-kernel, Steev Klimaszewski,
blogic
[-- Attachment #1: Type: Text/Plain, Size: 160 bytes --]
Hello,
I would like to ask if something was changed and if this patch
(in any way) is going to mainline kernel.
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route keyboard LEDs through the generic LEDs layer.
@ 2014-10-12 14:52 ` Pali Rohár
0 siblings, 0 replies; 39+ messages in thread
From: Pali Rohár @ 2014-10-12 14:52 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
I would like to ask if something was changed and if this patch
(in any way) is going to mainline kernel.
--
Pali Roh?r
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141012/aaa01df7/attachment.sig>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route keyboard LEDs through the generic LEDs layer.
2014-10-12 14:52 ` Pali Rohár
@ 2014-10-15 23:15 ` Samuel Thibault
-1 siblings, 0 replies; 39+ messages in thread
From: Samuel Thibault @ 2014-10-15 23:15 UTC (permalink / raw)
To: Pali Rohár
Cc: Andrew Morton, Dmitry Torokhov, Pavel Machek, David Herrmann,
jslaby, Bryan Wu, rpurdie, linux-kernel, Evan Broder,
Arnaud Patard, Peter Korsgaard, Sascha Hauer, Matt Sealey,
Rob Clark, Niels de Vos, linux-arm-kernel, Steev Klimaszewski,
blogic
Pali Rohár, le Sun 12 Oct 2014 16:52:08 +0200, a écrit :
> I would like to ask if something was changed and if this patch
> (in any way) is going to mainline kernel.
Well, the latest news I got from Dmity was on 12 Apr 2014, to which I
answered, and never got more news since then. I'm indeed starting to
wonder whether it'll ever get applied at all, after the several years it
has been waiting. It's no use trying to contribute to free software if
review/discussion etc. doesn't happen.
Samuel
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route keyboard LEDs through the generic LEDs layer.
@ 2014-10-15 23:15 ` Samuel Thibault
0 siblings, 0 replies; 39+ messages in thread
From: Samuel Thibault @ 2014-10-15 23:15 UTC (permalink / raw)
To: linux-arm-kernel
Pali Roh?r, le Sun 12 Oct 2014 16:52:08 +0200, a ?crit :
> I would like to ask if something was changed and if this patch
> (in any way) is going to mainline kernel.
Well, the latest news I got from Dmity was on 12 Apr 2014, to which I
answered, and never got more news since then. I'm indeed starting to
wonder whether it'll ever get applied at all, after the several years it
has been waiting. It's no use trying to contribute to free software if
review/discussion etc. doesn't happen.
Samuel
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route keyboard LEDs through the generic LEDs layer.
2014-10-15 23:15 ` Samuel Thibault
@ 2014-10-15 23:29 ` Pali Rohár
-1 siblings, 0 replies; 39+ messages in thread
From: Pali Rohár @ 2014-10-15 23:29 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Samuel Thibault, Andrew Morton, Pavel Machek, David Herrmann,
jslaby, Bryan Wu, rpurdie, linux-kernel, Evan Broder,
Arnaud Patard, Peter Korsgaard, Sascha Hauer, Matt Sealey,
Rob Clark, Niels de Vos, linux-arm-kernel, Steev Klimaszewski,
blogic
[-- Attachment #1: Type: Text/Plain, Size: 801 bytes --]
On Thursday 16 October 2014 01:15:45 Samuel Thibault wrote:
> Pali Rohár, le Sun 12 Oct 2014 16:52:08 +0200, a écrit :
> > I would like to ask if something was changed and if this
> > patch (in any way) is going to mainline kernel.
>
> Well, the latest news I got from Dmity was on 12 Apr 2014, to
> which I answered, and never got more news since then. I'm
> indeed starting to wonder whether it'll ever get applied at
> all, after the several years it has been waiting. It's no
> use trying to contribute to free software if
> review/discussion etc. doesn't happen.
>
> Samuel
Dmitry, ping!
https://lkml.org/lkml/2014/4/16/725
There are more people who would like to see this patch in
mainline kernel and we are waiting for you.
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route keyboard LEDs through the generic LEDs layer.
@ 2014-10-15 23:29 ` Pali Rohár
0 siblings, 0 replies; 39+ messages in thread
From: Pali Rohár @ 2014-10-15 23:29 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 16 October 2014 01:15:45 Samuel Thibault wrote:
> Pali Roh?r, le Sun 12 Oct 2014 16:52:08 +0200, a ?crit :
> > I would like to ask if something was changed and if this
> > patch (in any way) is going to mainline kernel.
>
> Well, the latest news I got from Dmity was on 12 Apr 2014, to
> which I answered, and never got more news since then. I'm
> indeed starting to wonder whether it'll ever get applied at
> all, after the several years it has been waiting. It's no
> use trying to contribute to free software if
> review/discussion etc. doesn't happen.
>
> Samuel
Dmitry, ping!
https://lkml.org/lkml/2014/4/16/725
There are more people who would like to see this patch in
mainline kernel and we are waiting for you.
--
Pali Roh?r
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141016/f35a2bdd/attachment.sig>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route keyboard LEDs through the generic LEDs layer.
2014-10-15 23:29 ` Pali Rohár
@ 2014-12-09 17:12 ` Pali Rohár
-1 siblings, 0 replies; 39+ messages in thread
From: Pali Rohár @ 2014-12-09 17:12 UTC (permalink / raw)
To: Dmitry Torokhov, Samuel Thibault, Andrew Morton, Pavel Machek
Cc: David Herrmann, jslaby, Bryan Wu, rpurdie, linux-kernel,
Evan Broder, Arnaud Patard, Peter Korsgaard, Sascha Hauer,
Matt Sealey, Rob Clark, Niels de Vos, linux-arm-kernel,
Steev Klimaszewski, blogic
[-- Attachment #1: Type: Text/Plain, Size: 1015 bytes --]
On Thursday 16 October 2014 01:29:21 Pali Rohár wrote:
> On Thursday 16 October 2014 01:15:45 Samuel Thibault wrote:
> > Pali Rohár, le Sun 12 Oct 2014 16:52:08 +0200, a écrit :
> > > I would like to ask if something was changed and if this
> > > patch (in any way) is going to mainline kernel.
> >
> > Well, the latest news I got from Dmity was on 12 Apr 2014,
> > to which I answered, and never got more news since then.
> > I'm indeed starting to wonder whether it'll ever get
> > applied at all, after the several years it has been
> > waiting. It's no use trying to contribute to free software
> > if
> > review/discussion etc. doesn't happen.
> >
> > Samuel
>
> Dmitry, ping!
> https://lkml.org/lkml/2014/4/16/725
>
> There are more people who would like to see this patch in
> mainline kernel and we are waiting for you.
Dmitry: ping!
I would like you to remind you this patch which is waiting for
your review since April 2014!
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route keyboard LEDs through the generic LEDs layer.
@ 2014-12-09 17:12 ` Pali Rohár
0 siblings, 0 replies; 39+ messages in thread
From: Pali Rohár @ 2014-12-09 17:12 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 16 October 2014 01:29:21 Pali Roh?r wrote:
> On Thursday 16 October 2014 01:15:45 Samuel Thibault wrote:
> > Pali Roh?r, le Sun 12 Oct 2014 16:52:08 +0200, a ?crit :
> > > I would like to ask if something was changed and if this
> > > patch (in any way) is going to mainline kernel.
> >
> > Well, the latest news I got from Dmity was on 12 Apr 2014,
> > to which I answered, and never got more news since then.
> > I'm indeed starting to wonder whether it'll ever get
> > applied at all, after the several years it has been
> > waiting. It's no use trying to contribute to free software
> > if
> > review/discussion etc. doesn't happen.
> >
> > Samuel
>
> Dmitry, ping!
> https://lkml.org/lkml/2014/4/16/725
>
> There are more people who would like to see this patch in
> mainline kernel and we are waiting for you.
Dmitry: ping!
I would like you to remind you this patch which is waiting for
your review since April 2014!
--
Pali Roh?r
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141209/7af7ed31/attachment.sig>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route keyboard LEDs through the generic LEDs layer.
2014-12-09 17:12 ` Pali Rohár
@ 2014-12-09 20:22 ` Peter Korsgaard
-1 siblings, 0 replies; 39+ messages in thread
From: Peter Korsgaard @ 2014-12-09 20:22 UTC (permalink / raw)
To: Pali Rohár
Cc: Dmitry Torokhov, Samuel Thibault, Andrew Morton, Pavel Machek,
David Herrmann, jslaby, Bryan Wu, rpurdie, linux-kernel,
Evan Broder, Arnaud Patard, Peter Korsgaard, Sascha Hauer,
Matt Sealey, Rob Clark, Niels de Vos, linux-arm-kernel,
Steev Klimaszewski, blogic
>>>>> "Pali" == Pali Rohár <pali.rohar@gmail.com> writes:
Hi,
>> There are more people who would like to see this patch in
>> mainline kernel and we are waiting for you.
> Dmitry: ping!
> I would like you to remind you this patch which is waiting for
> your review since April 2014!
Hah, I'm pretty sure it dates back to 2010 atleast.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route keyboard LEDs through the generic LEDs layer.
@ 2014-12-09 20:22 ` Peter Korsgaard
0 siblings, 0 replies; 39+ messages in thread
From: Peter Korsgaard @ 2014-12-09 20:22 UTC (permalink / raw)
To: linux-arm-kernel
>>>>> "Pali" == Pali Roh?r <pali.rohar@gmail.com> writes:
Hi,
>> There are more people who would like to see this patch in
>> mainline kernel and we are waiting for you.
> Dmitry: ping!
> I would like you to remind you this patch which is waiting for
> your review since April 2014!
Hah, I'm pretty sure it dates back to 2010 atleast.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] Route keyboard LEDs through the generic LEDs layer.
2014-12-09 20:22 ` Peter Korsgaard
@ 2014-12-10 1:03 ` Samuel Thibault
-1 siblings, 0 replies; 39+ messages in thread
From: Samuel Thibault @ 2014-12-10 1:03 UTC (permalink / raw)
To: Peter Korsgaard
Cc: Pali Rohár, Dmitry Torokhov, Andrew Morton, Pavel Machek,
David Herrmann, jslaby, Bryan Wu, rpurdie, linux-kernel,
Evan Broder, Arnaud Patard, Peter Korsgaard, Sascha Hauer,
Matt Sealey, Rob Clark, Niels de Vos, linux-arm-kernel,
Steev Klimaszewski, blogic
Peter Korsgaard, le Tue 09 Dec 2014 21:22:03 +0100, a écrit :
> > I would like you to remind you this patch which is waiting for
> > your review since April 2014!
>
> Hah, I'm pretty sure it dates back to 2010 atleast.
Yes, the first version was posted on February 2010. There has been some
reviews, and thus a few versions, I've just posted a 4th version which
has been sitting in -mm for some time now.
Samuel
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] Route keyboard LEDs through the generic LEDs layer.
@ 2014-12-10 1:03 ` Samuel Thibault
0 siblings, 0 replies; 39+ messages in thread
From: Samuel Thibault @ 2014-12-10 1:03 UTC (permalink / raw)
To: linux-arm-kernel
Peter Korsgaard, le Tue 09 Dec 2014 21:22:03 +0100, a ?crit :
> > I would like you to remind you this patch which is waiting for
> > your review since April 2014!
>
> Hah, I'm pretty sure it dates back to 2010 atleast.
Yes, the first version was posted on February 2010. There has been some
reviews, and thus a few versions, I've just posted a 4th version which
has been sitting in -mm for some time now.
Samuel
^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2014-12-10 1:04 UTC | newest]
Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-31 12:23 [PATCH] Route keyboard LEDs through the generic LEDs layer Samuel Thibault
2014-03-31 12:23 ` Samuel Thibault
2014-04-01 7:02 ` Pali Rohár
2014-04-01 7:02 ` Pali Rohár
2014-04-01 7:02 ` Pali Rohár
2014-04-07 2:10 ` Dmitry Torokhov
2014-04-07 2:10 ` Dmitry Torokhov
2014-04-07 7:54 ` Samuel Thibault
2014-04-07 7:54 ` Samuel Thibault
2014-04-08 8:39 ` Dmitry Torokhov
2014-04-08 8:39 ` Dmitry Torokhov
2014-04-08 23:33 ` Samuel Thibault
2014-04-08 23:33 ` Samuel Thibault
2014-04-11 6:12 ` Samuel Thibault
2014-04-11 6:12 ` Samuel Thibault
2014-04-13 1:25 ` Dmitry Torokhov
2014-04-13 1:25 ` Dmitry Torokhov
2014-04-16 23:38 ` Samuel Thibault
2014-04-16 23:38 ` Samuel Thibault
2014-04-12 10:09 ` Pavel Machek
2014-04-12 10:09 ` Pavel Machek
2014-04-13 1:30 ` Dmitry Torokhov
2014-04-13 1:30 ` Dmitry Torokhov
2014-10-01 18:42 ` Andrew Morton
2014-10-01 18:42 ` Andrew Morton
2014-10-01 21:29 ` Samuel Thibault
2014-10-01 21:29 ` Samuel Thibault
2014-10-12 14:52 ` Pali Rohár
2014-10-12 14:52 ` Pali Rohár
2014-10-15 23:15 ` Samuel Thibault
2014-10-15 23:15 ` Samuel Thibault
2014-10-15 23:29 ` Pali Rohár
2014-10-15 23:29 ` Pali Rohár
2014-12-09 17:12 ` Pali Rohár
2014-12-09 17:12 ` Pali Rohár
2014-12-09 20:22 ` Peter Korsgaard
2014-12-09 20:22 ` Peter Korsgaard
2014-12-10 1:03 ` Samuel Thibault
2014-12-10 1:03 ` Samuel Thibault
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.