[patch 0/2] [PATCH] input: correctly handle keys without hardware release event

[patch 0/2] [PATCH] input: correctly handle keys without hardware release event

[Giel de Nijs]

Hi,

This patch adds a soft release key mask to input_dev, to enable keyboard
drivers to determine which keys never generate a hardware release event and
hence add a release event after every press event of such keys. The mask is
controlled by ioctls.

The Fn+F? key combinations of Dell Latitude series laptops (and possibly other
Dells or other brands) only generate a key press event and never a key release
event, which is most probable a hardware flaw (or feature?). Due to this flaw,
combinations like Fn+F1 for hibernate and Fn-F3 for showing battery status
cannot be used. Ubuntu has probably fixed this by patching the X input layer
and HAL, but other distributions (like Debian) cannot use these keys. This
patch adds a generic method to signal if keys with certain scancodes never
generate release events, so the keyboard driver can add those events right
after a key press event.

The ioctls used to read and write to this bitmask might be used in a program
like setkeycodes, which is normally used to map certain scancodes to keycodes.
With a command line option, this program could also set the soft release bit
for a certain scancode if desired. Patches for setkeycodes and getkeycodes
against the Debian console-tools can be found at
http://giel.operation0.org/keyboard-soft-release

This patch also uses the infrastructure for generating release events for
KEY_HANGEUL and KEY_HANJA, something which was already done in atkbd.c.

See also this thread: http://thread.gmane.org/gmane.linux.kernel/401378

Greetings,
Giel
-- 

[patch 1/2] input: add soft release key mask to keyboard driver

[Giel de Nijs]

[-- Attachment #1: add-keyboard-soft-release-mask.patch --]
[-- Type: text/plain, Size: 2484 bytes --]

Add keysrlmask to input_dev and add soft release handling in atkbd.c

Signed-off-by: Giel de Nijs <giel@caffeinetrip.com>
---

Index: linux-2.6.21.1/drivers/input/keyboard/atkbd.c
===================================================================
--- linux-2.6.21.1.orig/drivers/input/keyboard/atkbd.c	2007-05-10 18:34:48.000000000 +0200
+++ linux-2.6.21.1/drivers/input/keyboard/atkbd.c	2007-05-11 19:21:03.000000000 +0200
@@ -201,6 +201,7 @@
 
 	unsigned short id;
 	unsigned char keycode[512];
+	unsigned long keysrlmask[NBITS(512)];
 	unsigned char set;
 	unsigned char translated;
 	unsigned char extra;
@@ -412,14 +413,6 @@
 			       "Some program might be trying access hardware directly.\n",
 			       data == ATKBD_RET_ACK ? "ACK" : "NAK", serio->phys);
 			goto out;
-		case ATKBD_RET_HANGEUL:
-		case ATKBD_RET_HANJA:
-			/*
-			 * These keys do not report release and thus need to be
-			 * flagged properly
-			 */
-			add_release_event = 1;
-			break;
 		case ATKBD_RET_ERR:
 			atkbd->err_count++;
 #ifdef ATKBD_DEBUG
@@ -438,6 +431,9 @@
 	if (keycode != ATKBD_KEY_NULL)
 		input_event(dev, EV_MSC, MSC_SCAN, code);
 
+	if (test_bit(code, atkbd->keysrlmask))
+		add_release_event = 1;
+
 	switch (keycode) {
 		case ATKBD_KEY_NULL:
 			break;
@@ -832,6 +828,7 @@
 	int i, j;
 
 	memset(atkbd->keycode, 0, sizeof(atkbd->keycode));
+	bitmap_zero(atkbd->keysrlmask, 512);
 
 	if (atkbd->translated) {
 		for (i = 0; i < 128; i++) {
@@ -854,6 +851,11 @@
 
 	atkbd->keycode[atkbd_compat_scancode(atkbd, ATKBD_RET_HANGEUL)] = KEY_HANGUEL;
 	atkbd->keycode[atkbd_compat_scancode(atkbd, ATKBD_RET_HANJA)] = KEY_HANJA;
+
+	set_bit(atkbd_compat_scancode(atkbd, ATKBD_RET_HANGEUL),
+		atkbd->keysrlmask);
+	set_bit(atkbd_compat_scancode(atkbd, ATKBD_RET_HANJA),
+		atkbd->keysrlmask);
 }
 
 /*
@@ -911,6 +913,7 @@
 	}
 
 	input_dev->keycode = atkbd->keycode;
+	input_dev->keysrlmask = atkbd->keysrlmask;
 	input_dev->keycodesize = sizeof(unsigned char);
 	input_dev->keycodemax = ARRAY_SIZE(atkbd_set2_keycode);
 
Index: linux-2.6.21.1/include/linux/input.h
===================================================================
--- linux-2.6.21.1.orig/include/linux/input.h	2007-05-10 18:32:39.000000000 +0200
+++ linux-2.6.21.1/include/linux/input.h	2007-05-10 18:56:50.000000000 +0200
@@ -962,6 +962,7 @@
 	unsigned int keycodemax;
 	unsigned int keycodesize;
 	void *keycode;
+	unsigned long *keysrlmask; /* soft-release scancodes key mask */
 
 	struct ff_device *ff;
 

-- 

[patch 2/2] input: add ioctls to console for soft release mask read/write

[Giel de Nijs]

[-- Attachment #1: add-soft-release-ioctl-to-console.patch --]
[-- Type: text/plain, Size: 5354 bytes --]

Add KDGETKEYSRL and KDSETKEYSRL to the console to control the soft release
key mask in input_dev.

Signed-off-by: Giel de Nijs <giel@caffeinetrip.com>
---

Index: linux-2.6.21.1/drivers/char/vt_ioctl.c
===================================================================
--- linux-2.6.21.1.orig/drivers/char/vt_ioctl.c	2007-05-10 19:19:32.000000000 +0200
+++ linux-2.6.21.1/drivers/char/vt_ioctl.c	2007-05-11 19:21:52.000000000 +0200
@@ -181,6 +181,29 @@
 	return kc;
 }
 
+static int
+do_kbkeysrl_ioctl(int cmd, struct kbkeysrl __user *user_kbks, int perm)
+{
+	struct kbkeysrl tmp;
+	int srl = 0;
+
+	if (copy_from_user(&tmp, user_kbks, sizeof(struct kbkeysrl)))
+		return -EFAULT;
+	switch (cmd) {
+	case KDGETKEYSRL:
+		srl = getkeysrl(tmp.scancode);
+		if (srl >= 0)
+			srl = put_user(srl, &user_kbks->srl);
+		break;
+	case KDSETKEYSRL:
+		if (!perm)
+			return -EPERM;
+		srl = setkeysrl(tmp.scancode, tmp.srl);
+		break;
+	}
+	return srl;
+}
+
 static inline int
 do_kdgkb_ioctl(int cmd, struct kbsentry __user *user_kdgkb, int perm)
 {
@@ -571,6 +594,12 @@
 			perm=0;
 		return do_kbkeycode_ioctl(cmd, up, perm);
 
+	case KDGETKEYSRL:
+	case KDSETKEYSRL:
+		if(!capable(CAP_SYS_TTY_CONFIG))
+			perm=0;
+		return do_kbkeysrl_ioctl(cmd, up, perm);
+
 	case KDGKBENT:
 	case KDSKBENT:
 		return do_kdsk_ioctl(cmd, up, perm, kbd);
Index: linux-2.6.21.1/include/linux/kbd_kern.h
===================================================================
--- linux-2.6.21.1.orig/include/linux/kbd_kern.h	2007-05-10 19:20:18.000000000 +0200
+++ linux-2.6.21.1/include/linux/kbd_kern.h	2007-05-10 19:25:59.000000000 +0200
@@ -143,6 +143,8 @@
 
 int getkeycode(unsigned int scancode);
 int setkeycode(unsigned int scancode, unsigned int keycode);
+int getkeysrl(unsigned int scancode);
+int setkeysrl(unsigned int scancode, int srl);
 void compute_shiftstate(void);
 
 /* defkeymap.c */
Index: linux-2.6.21.1/include/linux/kd.h
===================================================================
--- linux-2.6.21.1.orig/include/linux/kd.h	2007-05-10 19:20:27.000000000 +0200
+++ linux-2.6.21.1/include/linux/kd.h	2007-05-10 19:38:52.000000000 +0200
@@ -144,6 +144,13 @@
 
 #define KDFONTOP	0x4B72	/* font operations */
 
+struct kbkeysrl {
+	unsigned int scancode;
+	int srl;
+};
+#define KDGETKEYSRL	0x4B82  /* read kernel soft release key table entry */
+#define KDSETKEYSRL	0x4B83  /* write kernel soft release key table entry */
+
 struct console_font_op {
 	unsigned int op;	/* operation code KD_FONT_OP_* */
 	unsigned int flags;	/* KD_FONT_FLAG_* */
Index: linux-2.6.21.1/drivers/char/keyboard.c
===================================================================
--- linux-2.6.21.1.orig/drivers/char/keyboard.c	2007-05-10 19:19:39.000000000 +0200
+++ linux-2.6.21.1/drivers/char/keyboard.c	2007-05-10 19:39:46.000000000 +0200
@@ -162,7 +162,7 @@
  * Translation of scancodes to keycodes. We set them on only the first attached
  * keyboard - for per-keyboard setting, /dev/input/event is more useful.
  */
-int getkeycode(unsigned int scancode)
+struct input_dev *find_first_keyboard(void)
 {
 	struct list_head *node;
 	struct input_dev *dev = NULL;
@@ -175,6 +175,13 @@
 		}
 	}
 
+	return dev;
+}
+
+int getkeycode(unsigned int scancode)
+{
+	struct input_dev *dev = find_first_keyboard();
+
 	if (!dev)
 		return -ENODEV;
 
@@ -186,18 +193,9 @@
 
 int setkeycode(unsigned int scancode, unsigned int keycode)
 {
-	struct list_head *node;
-	struct input_dev *dev = NULL;
+	struct input_dev *dev = find_first_keyboard();
 	unsigned int i, oldkey;
 
-	list_for_each(node, &kbd_handler.h_list) {
-		struct input_handle *handle = to_handle_h(node);
-		if (handle->dev->keycodesize) {
-			dev = handle->dev;
-			break;
-		}
-	}
-
 	if (!dev)
 		return -ENODEV;
 
@@ -221,6 +219,48 @@
 }
 
 /*
+ * Read/write the soft release bit for scancodes of a keyboard. The soft release
+ * bit is used to mark keys that don't generate a hardware release event.
+ * Here, we only handle the first attached keyboard.
+ */
+int getkeysrl(unsigned int scancode)
+{
+	struct input_dev *dev = find_first_keyboard();
+
+	if (!dev)
+		return -ENODEV;
+
+	if (scancode >= dev->keycodemax)
+		return -EINVAL;
+
+	if (!dev->keysrlmask) /* driver doesn't support soft release */
+		return -EINVAL;
+
+	return !! test_bit(scancode, dev->keysrlmask);
+}
+
+int setkeysrl(unsigned int scancode, int srl)
+{
+	struct input_dev *dev = find_first_keyboard();
+
+	if (!dev)
+		return -ENODEV;
+
+	if (scancode >= dev->keycodemax)
+		return -EINVAL;
+
+	if (!dev->keysrlmask) /* driver doesn't support soft release */
+		return -EINVAL;
+
+	if (srl)
+		set_bit(scancode, dev->keysrlmask);
+	else
+		clear_bit(scancode, dev->keysrlmask);
+
+	return 0;
+}
+
+/*
  * Making beeps and bells.
  */
 static void kd_nosound(unsigned long ignored)
Index: linux-2.6.21.1/include/linux/compat_ioctl.h
===================================================================
--- linux-2.6.21.1.orig/include/linux/compat_ioctl.h	2007-05-10 20:28:43.000000000 +0200
+++ linux-2.6.21.1/include/linux/compat_ioctl.h	2007-05-10 20:30:08.000000000 +0200
@@ -172,6 +172,8 @@
 ULONG_IOCTL(KDSIGACCEPT)
 COMPATIBLE_IOCTL(KDGETKEYCODE)
 COMPATIBLE_IOCTL(KDSETKEYCODE)
+COMPATIBLE_IOCTL(KDGETKEYSRL)
+COMPATIBLE_IOCTL(KDSETKEYSRL)
 ULONG_IOCTL(KIOCSOUND)
 ULONG_IOCTL(KDMKTONE)
 COMPATIBLE_IOCTL(KDGKBTYPE)

-- 

Re: [patch 0/2] [PATCH] input: correctly handle keys without hardware release event

[Dmitry Torokhov]

Hi Giel,

On Friday 11 May 2007 13:23, Giel de Nijs wrote:
> Hi,
> 
> This patch adds a soft release key mask to input_dev, to enable keyboard
> drivers to determine which keys never generate a hardware release event and
> hence add a release event after every press event of such keys. The mask is
> controlled by ioctls.

I don't think we want to add all the infrastructure for the benefit of single
driver. Can we add a quirk to atkbd and activate it based on DMI?

-- 
Dmitry

Re: [patch 0/2] [PATCH] input: correctly handle keys without hardware release event

[Giel de Nijs]

On 5/14/07, Dmitry Torokhov <dtor@insightbb.com> wrote:

> > This patch adds a soft release key mask to input_dev, to enable keyboard
> > drivers to determine which keys never generate a hardware release event and
> > hence add a release event after every press event of such keys. The mask is
> > controlled by ioctls.
>
> I don't think we want to add all the infrastructure for the benefit of single
> driver. Can we add a quirk to atkbd and activate it based on DMI?

As the atkbd.c driver already had a work-around for exactly the same problem
with some (I think) Korean keyboards (KEY_HANGEUL and KEY_HANJA), adding
this infrastructure (well, one pointer and two ioctls) seemed a logical step.
If you prefer a hack just in atkbd.c and activate it on Dell laptops, I see
what I can do. The problem with that is that I don't know the scancodes of
all keys on all models (and future models) that exhibit this behaviour. I'll
implement what I know, though (it'll take some time because I'll be
travelling quite a lot very soon).

Greetings,
Giel