diff mbox

[2/4] tty/vt/keyboard: Fix Caps Lock LED on major distributions

Message ID 20170127171318.2596-3-benjamin.tissoires@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Benjamin Tissoires Jan. 27, 2017, 5:13 p.m. UTC
The way the kernel handles caps lock on TTYs is not compatible with
all keymaps[1].
The solution that was found was to replace the native Caps_Lock by an
other modifier - ControlL_Lock - from user space. ckbcomp generates
a keymap which can emulate the Caps Lock behavior without relying on
the kernel to translate keys into upper case.

On Fedora and RHEL, the generic keymaps are generated by ckbcomp, and
when systemd-localed loads the locale, it calls "loadkeys -u us-intl",
which uses these generated keymaps.

The problem with these custom keymaps is that the Caps Lock LED is now
not updated properly. In the INPUT_LEDS version, the trigger that can
now handle the Caps Lock LED is "kbd-ctrlllock", but the default is
"kbd-capslock".
We can detect such custom keymaps when they are set by the console ioctl,
and in that case, we can sync both triggers "kbd-capslock" and
"kbd-ctrlllock" to restore a proper LED behavior.

Both version of LEDs (with or without INPUT_LEDS) are fixed.

[1] https://bugzilla.kernel.org/show_bug.cgi?id=7746#c24

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/tty/vt/keyboard.c | 43 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 40 insertions(+), 3 deletions(-)

Comments

Samuel Thibault Jan. 27, 2017, 5:25 p.m. UTC | #1
Benjamin Tissoires, on Fri 27 Jan 2017 18:13:16 +0100, wrote:
> +static bool caps_as_controlllock;

Really, we don't want these.  Userspace could invent using another
modifier, so it's really not the way forward.  Please have a look at
what I suggested.

Samuel
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Torokhov Jan. 27, 2017, 5:29 p.m. UTC | #2
On January 27, 2017 9:25:14 AM PST, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
>Benjamin Tissoires, on Fri 27 Jan 2017 18:13:16 +0100, wrote:
>> +static bool caps_as_controlllock;
>
>Really, we don't want these.  Userspace could invent using another
>modifier, so it's really not the way forward.  Please have a look at
>what I suggested.

Completely agree.


Thanks.
diff mbox

Patch

diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index 4a3907e..ca1d614 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -65,6 +65,8 @@  static inline int kbd_defleds(void)
 
 #define KBD_DEFLOCK 0
 
+#define KBD_LOCKSTATE_OFFSET 8
+
 /*
  * Handler Tables.
  */
@@ -132,6 +134,8 @@  static int shift_state = 0;
 
 static unsigned int ledstate = -1U;			/* undefined */
 static unsigned char ledioctl;
+static bool caps_as_controlllock;
+static bool task_caps_as_controlllock;
 
 /*
  * Notifier list for console keyboard events
@@ -979,7 +983,7 @@  static void kbd_led_trigger_activate(struct led_classdev *cdev)
 	}
 
 #define KBD_LOCKSTATE_TRIGGER(_led_bit, _name)		\
-	KBD_LED_TRIGGER((_led_bit) + 8, _name)
+	KBD_LED_TRIGGER((_led_bit) + KBD_LOCKSTATE_OFFSET, _name)
 
 static struct kbd_led_trigger kbd_led_triggers[] = {
 	KBD_LED_TRIGGER(VC_SCROLLOCK, "kbd-scrolllock"),
@@ -1004,6 +1008,18 @@  static void kbd_propagate_led_state(unsigned int old_state,
 	unsigned int changed = old_state ^ new_state;
 	int i;
 
+	/*
+	 * special case where user space uses its own caps lock implementation
+	 * through ControlL_Lock.
+	 */
+	if (task_caps_as_controlllock) {
+		trigger = &kbd_led_triggers[VC_CAPSLOCK];
+		trigger->mask |= BIT(VC_CTRLLLOCK) << KBD_LOCKSTATE_OFFSET;
+	} else {
+		trigger = &kbd_led_triggers[VC_CAPSLOCK];
+		trigger->mask &= ~(BIT(VC_CTRLLLOCK) << KBD_LOCKSTATE_OFFSET);
+	}
+
 	for (i = 0; i < ARRAY_SIZE(kbd_led_triggers); i++) {
 		trigger = &kbd_led_triggers[i];
 
@@ -1042,11 +1058,19 @@  static void kbd_init_leds(void)
 static int kbd_update_leds_helper(struct input_handle *handle, void *data)
 {
 	unsigned int leds = *(unsigned int *)data;
+	unsigned int capsl_mask = BIT(VC_CAPSLOCK);
+
+	/*
+	 * special case where user space uses its own caps lock implementation
+	 * through ControlL_Lock.
+	 */
+	if (task_caps_as_controlllock)
+		capsl_mask |= BIT(VC_CTRLLLOCK) << KBD_LOCKSTATE_OFFSET;
 
 	if (test_bit(EV_LED, handle->dev->evbit)) {
 		input_inject_event(handle, EV_LED, LED_SCROLLL, !!(leds & BIT(VC_SCROLLOCK)));
 		input_inject_event(handle, EV_LED, LED_NUML,    !!(leds & BIT(VC_NUMLOCK)));
-		input_inject_event(handle, EV_LED, LED_CAPSL,   !!(leds & BIT(VC_CAPSLOCK)));
+		input_inject_event(handle, EV_LED, LED_CAPSL,   !!(leds & capsl_mask));
 		input_inject_event(handle, EV_SYN, SYN_REPORT, 0);
 	}
 
@@ -1188,7 +1212,8 @@  static void kbd_bh(unsigned long dummy)
 
 	spin_lock_irqsave(&led_lock, flags);
 	leds = getleds();
-	leds |= (unsigned int)kbd->lockstate << 8;
+	leds |= (unsigned int)kbd->lockstate << KBD_LOCKSTATE_OFFSET;
+	task_caps_as_controlllock = caps_as_controlllock;
 	spin_unlock_irqrestore(&led_lock, flags);
 
 	if (leds != ledstate) {
@@ -1939,6 +1964,18 @@  int vt_do_kdsk_ioctl(int cmd, struct kbentry __user *user_kbe, int perm,
 			spin_unlock_irqrestore(&kbd_event_lock, flags);
 			return -EPERM;
 		}
+
+		/*
+		 * See https://bugzilla.kernel.org/show_bug.cgi?id=7746#c24
+		 *
+		 * The kernel can't properly deal with all keymaps/capslock
+		 * interactions, so userspace (ckbcomp) may use ControlL_Lock
+		 * as a replacement for Caps_Lock.
+		 * It works but breaks the LED, so mark it there that we need
+		 * to forward proper Caps Lock LED on K_CTRLLLOCK.
+		 */
+		if (i == KEY_CAPSLOCK)
+			caps_as_controlllock = (v == K_CTRLLLOCK);
 		key_map[i] = U(v);
 		if (!s && (KTYP(ov) == KT_SHIFT || KTYP(v) == KT_SHIFT))
 			do_compute_shiftstate();