Skip to contentSkip to conversation

input: change SysRq keycode for systems without SysRq key

PATCH In review federico ferri 6 participants 22 messages Jul 15, 2007 – Aug 17, 2007

Subsystems (3)
Library code HID Char and misc drivers
lore
Share & tools
lore

federico ferri

author
#0 lore raw

PATCH 1/1 input: change SysRq keycode for systems without SysRq key

this patch adds an entry in Kconfig after 'config MAGIC_SYSRQ' which asks for a different keycode (replacing also KEY_SYSRQ with CONFIG_MAGIC_SYSRQ_KEYCODE in char/keyboard.c)

it was tested on some USB keyboards (Microsoft and Apple USB keyboards, and it works)

I sumbitted this patch two years ago, but it was ignored.

I think this patch is very important, even to a non-developer (SysRq-k is very helpful when testing unstable software/drivers)

[for 2.6]

Signed-off-by Federico Ferri <[email protected]>

---

drivers/char/keyboard.c +0 -0
diff -uprN linux.orig/drivers/char/keyboard.c linux/drivers/char/keyboard.c
@@ -1081,7 +1087,7 @@ static void kbd_keycode(unsigned int key
 				printk(KERN_WARNING "keyboard.c: can't emulate rawmode for keycode

%d\n", keycode);

#ifdef CONFIG_MAGIC_SYSRQ /* Handle the SysRq Hack */
- if (keycode == KEY_SYSRQ && (sysrq_down || (down == 1 && sysrq_alt))) {
+ if (keycode == CONFIG_MAGIC_SYSRQ_KEYCODE && (sysrq_down || (down == 1
&& sysrq_alt))) {
sysrq_down = down;
return;
}

lib/Kconfig.debug +12 -0
diff -uprN linux.orig/lib/Kconfig.debug linux/lib/Kconfig.debug
@@ -28,6 +28,16 @@ config MAGIC_SYSRQ
 	  send a BREAK and then within 5 seconds a command keypress. The
 	  keys are documented in <file:Documentation/sysrq.txt>. Don't say Y
 	  unless you really know what this hack does.
+	
+config MAGIC_SYSRQ_KEYCODE
+	int
+	prompt "Change SysRq key-code" if MAGIC_SYSRQ
+	default 99
+	depends on MAGIC_SYSRQ
+	help
+	  If your keyboard doesn't have a SysRq key (also labeled PrintScr),
+	  you can specify another keycode which should act as SysRq.
+	  Default is 99 (KEY_SYSRQ).
+	  You can find this number using programs like evtest, or (maybe)
+	  showkey.

config LOG_BUF_SHIFT
int "Kernel log buffer size (16 => 64KB, 17 => 128KB)" if DEBUG_KERNEL

#1 lore raw
Quoted 4 lines · federico ferri

this patch adds an entry in Kconfig after 'config MAGIC_SYSRQ' which asks for a different keycode (replacing also KEY_SYSRQ with CONFIG_MAGIC_SYSRQ_KEYCODE in char/keyboard.c)

Quite frankly, I think that if we want to make something like this configurable, it should be dynamic, not compiled-in. So that the same kernel image can be used with different keyboards.

Linus

author
#2 lore raw

Linus Torvalds ha scritto:

Quoted 7 lines · federico ferri
Quoted 5 lines

this patch adds an entry in Kconfig after 'config MAGIC_SYSRQ' which asks for a different keycode (replacing also KEY_SYSRQ with CONFIG_MAGIC_SYSRQ_KEYCODE in char/keyboard.c)

Quoted 7 lines

Quite frankly, I think that if we want to make something like this configurable, it should be dynamic, not compiled-in. So that the same kernel image can be used with different keyboards.

Linus

That's right; but let me explain: I started this as a hack, just because I both wanted to use this Apple USB keyboard and wanted to be able to hit SysRq.

I have no experience at all in programming kernel. Even that small patch it was reviewed and corrected by some people on kernel and input mailing lists.

(I can imagine it could be done via a /proc/sysrq-keycode virtual file, but that's just one possibility)

So, this should turn into a feature request...
would you accept feature requests? ( :

Signature · 1 line
Federico Ferri
#3 lore raw
Quoted 6 lines · federico ferri
Quoted 1 line

(I can imagine it could be done via a /proc/sysrq-keycode virtual file, but that's just one possibility)

So, this should turn into a feature request...
would you accept feature requests? ( :

Well, this is totally untested, and I won't guarantee that this works at all, but this is how to generally do these kinds of things..

Linus

---
 drivers/char/keyboard.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)
drivers/char/keyboard.c +5 -1
index 90965b4..e21f52b 100644
@@ -157,6 +157,10 @@ static int sysrq_alt_use;
 #endif
 static int sysrq_alt;
 
+static int sysrq_key = KEY_SYSRQ;
+module_param_named(sysrq_key, sysrq_key, uint, 0644);
+MODULE_PARM_DESC(sysrq_key, "The keycode to be used for sysrq");
+
 /*
  * Translation of scancodes to keycodes. We set them on only the first
  * keyboard in the list that accepts the scancode and keycode.

@@ -1139,7 +1143,7 @@ static void kbd_keycode(unsigned int keycode, int down, int hw_raw)
 				printk(KERN_WARNING "keyboard.c: can't emulate rawmode for keycode %d\n", keycode);
 
 #ifdef CONFIG_MAGIC_SYSRQ	       /* Handle the SysRq Hack */
-	if (keycode == KEY_SYSRQ && (sysrq_down || (down == 1 && sysrq_alt))) {
+	if (keycode == sysrq_key && (sysrq_down || (down == 1 && sysrq_alt))) {
 		if (!sysrq_down) {
 			sysrq_down = down;
 			sysrq_alt_use = sysrq_alt;
author
#4 lore raw

Linus Torvalds ha scritto:

Quoted 3 lines

Well, this is totally untested, and I won't guarantee that this works at all, but this is how to generally do these kinds of things..

YAY! it works great.
tried with:

# echo 183 > /sys/module/keyboard/parameters/sysrq_key
but also keyboard.sys_rq=183 on the command line should work; I'll
discover that on next reboot.

tested only on my machine, but that's a really trivial change (and the previous patch was tested on more machines)

thank you, Linus!

(now that this has done in the proper way, is this patch going to be merged in the tree?)

Signature · 1 line
Federico Ferri
#5 lore raw
Quoted 9 lines · federico ferri

Linus Torvalds ha scritto:

Quoted 2 lines

Well, this is totally untested, and I won't guarantee that this works at all, but this is how to generally do these kinds of things..

YAY! it works great.
tried with:

# echo 183 > /sys/module/keyboard/parameters/sysrq_key
but also keyboard.sys_rq=183 on the command line should work; I'll
discover that on next reboot.

Yes, please do verify.

Quoted 1 line

thank you, Linus!

You're welcome.

Quoted 2 lines

(now that this has done in the proper way, is this patch going to be merged in the tree?)

I'm certainly ok with it, but I guess it's more up to Dmitry. If he ack's it, I can just commit it.

Dmitry? Any reason not to do this?

Linus

#6 lore raw

Hi Linus,

Quoted 20 lines · Linus Torvalds
Quoted 9 lines · federico ferri

Linus Torvalds ha scritto:

Quoted 2 lines

Well, this is totally untested, and I won't guarantee that this works at all, but this is how to generally do these kinds of things..

YAY! it works great.
tried with:

# echo 183 > /sys/module/keyboard/parameters/sysrq_key
but also keyboard.sys_rq=183 on the command line should work; I'll
discover that on next reboot.

Yes, please do verify.

Quoted 1 line

thank you, Linus!

You're welcome.

Quoted 8 lines
Quoted 2 lines

(now that this has done in the proper way, is this patch going to be merged in the tree?)

I'm certainly ok with it, but I guess it's more up to Dmitry. If he ack's it, I can just commit it.

Dmitry? Any reason not to do this?

Recent kernels have the ability to remap keymap for USB keyboards via EVIOCSKEYCODE ioctl (we allowed 0adjusting keymaps on PS/2 keyboards for a long time). So instead of having the new parameter redefining SysRq keycode Frederico can remap one of the keys on his keyboard to generate KEY_SYSRQ. This way SysRq should still work if he plugs in another USB keyboard that has SysRq key or a PS/2 keyboard.

Signature · 1 line
Dmitry
author
#7 lore raw

Dmitry Torokhov ha scritto:

Quoted 9 lines
Quoted 2 lines

Dmitry? Any reason not to do this?

Recent kernels have the ability to remap keymap for USB keyboards via EVIOCSKEYCODE ioctl (we allowed 0adjusting keymaps on PS/2 keyboards for a long time). So instead of having the new parameter redefining SysRq keycode Frederico can remap one of the keys on his keyboard to generate KEY_SYSRQ. This way SysRq should still work if he plugs in another USB keyboard that has SysRq key or a PS/2 keyboard.

would setkeycodes still just work for PS2 keyboards?
I upgraded to newer kbd-1.13 package (kernel 2.6.21), and while trying to change sysrq keycode still I get the old error:

KDSETKEYCODE: No such device
failed to set scancode 38b7 to keycode 84

which I reported on gentoo a long time ago: http://bugs.gentoo.org/show_bug.cgi?id=97388

(it doesn't use EVIOCSKEYCODE)
maybe I'm not picking up the right program?

Signature · 1 line
Federico Ferri
#8 lore raw
Quoted 15 lines · federico ferri

Dmitry Torokhov ha scritto:

Quoted 9 lines
Quoted 2 lines

Dmitry? Any reason not to do this?

Recent kernels have the ability to remap keymap for USB keyboards via EVIOCSKEYCODE ioctl (we allowed 0adjusting keymaps on PS/2 keyboards for a long time). So instead of having the new parameter redefining SysRq keycode Frederico can remap one of the keys on his keyboard to generate KEY_SYSRQ. This way SysRq should still work if he plugs in another USB keyboard that has SysRq key or a PS/2 keyboard.

would setkeycodes still just work for PS2 keyboards?
I upgraded to newer kbd-1.13 package (kernel 2.6.21), and while trying to change sysrq keycode still I get the old error:

Quoted 9 lines

KDSETKEYCODE: No such device
failed to set scancode 38b7 to keycode 84

which I reported on gentoo a long time ago:
http://bugs.gentoo.org/show_bug.cgi?id=97388

(it doesn't use EVIOCSKEYCODE)
maybe I'm not picking up the right program?

EVIOCSKEYCODE support for HID (USB) went in in 2.6.22-rc1.

There is keyfuzz utility you can use to play with keymaps for a particular device:

http://0pointer.de/lennart/projects/keyfuzz/

Signature · 1 line
Dmitry
author
#9 lore raw

Dmitry Torokhov ha scritto:

Quoted 6 lines

EVIOCSKEYCODE support for HID (USB) went in in 2.6.22-rc1.

There is keyfuzz utility you can use to play with keymaps for a particular device:

http://0pointer.de/lennart/projects/keyfuzz/

hi Dmitry,
I tried keyfuzz, but I'm not able to change sysrq key (I get an invalid argument error). I paste some log of my tries:

# uname -r
2.6.22
# for i in /dev/input/event*; do echo -e ">>>> $i:"; keyfuzz -i -d$i; done
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

/dev/input/event0:

evdev driver version 1.0.0.
device name: PC Speaker

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

/dev/input/event1:

evdev driver version 1.0.0.
device name: Mitsumi Electric Apple Extended USB Keyboard

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

/dev/input/event2:

evdev driver version 1.0.0.
device name: Mitsumi Electric Apple Extended USB Keyboard

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

/dev/input/event3:

evdev driver version 1.0.0.
device name: Microsoft Basic Optical Mouse

# echo "84 183" | keyfuzz -s -d/dev/input/event1
EVIOCGKEYCODE: Invalid argument
# echo "84 183" | keyfuzz -s -d/dev/input/event2
EVIOCGKEYCODE: Invalid argument
# echo "0x05d 0x0b7" | keyfuzz -s -d/dev/input/event2
EVIOCGKEYCODE: Invalid argument

(strace gives me no useful info)

#10 lore raw
Quoted 32 lines · federico ferri

Dmitry Torokhov ha scritto:

Quoted 6 lines

EVIOCSKEYCODE support for HID (USB) went in in 2.6.22-rc1.

There is keyfuzz utility you can use to play with keymaps for a particular device:

http://0pointer.de/lennart/projects/keyfuzz/

hi Dmitry,
I tried keyfuzz, but I'm not able to change sysrq key (I get an invalid argument error).
I paste some log of my tries:

# uname -r
2.6.22
# for i in /dev/input/event*; do echo -e ">>>> $i:"; keyfuzz -i -d$i; done
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

/dev/input/event0:

evdev driver version 1.0.0.
device name: PC Speaker

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

/dev/input/event1:

evdev driver version 1.0.0.
device name: Mitsumi Electric Apple Extended USB Keyboard

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

/dev/input/event2:

evdev driver version 1.0.0.
device name: Mitsumi Electric Apple Extended USB Keyboard

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

/dev/input/event3:

evdev driver version 1.0.0.
device name: Microsoft Basic Optical Mouse

# echo "84 183" | keyfuzz -s -d/dev/input/event1
EVIOCGKEYCODE: Invalid argument
Quoted 8 lines
# echo "84 183" | keyfuzz -s -d/dev/input/event2
EVIOCGKEYCODE: Invalid argument
# echo "0x05d 0x0b7" | keyfuzz -s -d/dev/input/event2
EVIOCGKEYCODE: Invalid argument

(strace gives me no useful info)

What is 84? For SUB you map a "usage" to a keycode and regular key usages start with 0x00070000...

Signature · 1 line
Dmitry
#11 lore raw
Quoted 36 lines · Dmitry Torokhov
Quoted 35 lines · federico ferri

Dmitry Torokhov ha scritto:

Quoted 6 lines

EVIOCSKEYCODE support for HID (USB) went in in 2.6.22-rc1.

There is keyfuzz utility you can use to play with keymaps for a particular device:

http://0pointer.de/lennart/projects/keyfuzz/

hi Dmitry,
I tried keyfuzz, but I'm not able to change sysrq key (I get an invalid argument error).
I paste some log of my tries:

# uname -r
2.6.22
# for i in /dev/input/event*; do echo -e ">>>> $i:"; keyfuzz -i -d$i; done
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

/dev/input/event0:

evdev driver version 1.0.0.
device name: PC Speaker

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

/dev/input/event1:

evdev driver version 1.0.0.
device name: Mitsumi Electric Apple Extended USB Keyboard

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

/dev/input/event2:

evdev driver version 1.0.0.
device name: Mitsumi Electric Apple Extended USB Keyboard

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

/dev/input/event3:

evdev driver version 1.0.0.
device name: Microsoft Basic Optical Mouse

# echo "84 183" | keyfuzz -s -d/dev/input/event1
EVIOCGKEYCODE: Invalid argument
# echo "84 183" | keyfuzz -s -d/dev/input/event2
EVIOCGKEYCODE: Invalid argument
Quoted 8 lines
Quoted 5 lines
# echo "0x05d 0x0b7" | keyfuzz -s -d/dev/input/event2
EVIOCGKEYCODE: Invalid argument

(strace gives me no useful info)

What is 84? For SUB you map a "usage" to a keycode and regular key

                        ^^^ USB
Quoted 5 lines

usages start with 0x00070000...

--
Dmitry

Signature · 1 line
Dmitry
author
#12 lore raw
In reply to Dmitry Torokhov

Dmitry Torokhov ha scritto:

Quoted 10 lines
Quoted 8 lines
# echo "84 183" | keyfuzz -s -d/dev/input/event1
EVIOCGKEYCODE: Invalid argument
# echo "84 183" | keyfuzz -s -d/dev/input/event2
EVIOCGKEYCODE: Invalid argument
# echo "0x05d 0x0b7" | keyfuzz -s -d/dev/input/event2
EVIOCGKEYCODE: Invalid argument

What is 84? For SUB you map a "usage" to a keycode and regular key usages start with 0x00070000...

84 == 0x5d (KEY_SYSRQ)
183 is the code generated from my keyboard when I press F13

I don't get what "usage" means here; my keyboard has no fancy/multimedia keys (except 4 buttons_ volumeup, volume-down, mute, eject, but those belong to another event device)

the keyfuzz manual page says:
The scancode/keycode translation tables as read from STDIN [...]. All other lines have to contain a scancode and a keycode number separated by white space. The numbers may be specified either in decimal or in hexadecimal notation. [...]

I typed my commands with the above in mind.
anyway, if I try to load the bundled translation tables (r.g. the default one) to my event device, I get the same error.

do you see some solution? perhaps can you supply an example of re-mapping F13 to SysRq?

Signature · 1 line
Federico Ferri
#13 lore raw
Quoted 7 lines · federico ferri

Dmitry Torokhov ha scritto:

Quoted 6 lines
Quoted 6 lines
# echo "84 183" | keyfuzz -s -d/dev/input/event1
EVIOCGKEYCODE: Invalid argument
# echo "84 183" | keyfuzz -s -d/dev/input/event2
EVIOCGKEYCODE: Invalid argument
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
# echo "0x05d 0x0b7" | keyfuzz -s -d/dev/input/event2
EVIOCGKEYCODE: Invalid argument

What is 84? For SUB you map a "usage" to a keycode and regular key usages start with 0x00070000...

84 == 0x5d (KEY_SYSRQ)
183 is the code generated from my keyboard when I press F13

Ahh, I see. But when you press F13 hardware does not generate 183. USB generates some big number (below), PS/2 generates sequence like 0x5d (not sure) and keyboard connected to a serial port generates something else. Keyfuzz needs scancode in the format that is driver-specific.

Quoted 1 line

I don't get what "usage" means here;

USB's "scancode".

Quoted 10 lines

my keyboard has no fancy/multimedia
keys (except 4 buttons_ volumeup, volume-down, mute, eject, but those belong to another event device)

the keyfuzz manual page says:
The scancode/keycode translation tables as read from STDIN [...]. All other lines have to contain a scancode and a keycode number separated by white space. The numbers may be specified either in decimal or in hexadecimal notation. [...]

Quoted 3 lines

I typed my commands with the above in mind.
anyway, if I try to load the bundled translation tables (r.g. the default one) to my event device, I get the same error.

That is undersandable because your keyboard is difefrent and does not use the same "scancodes" as AT keyboard.

Quoted 4 lines

do you see some solution? perhaps can you supply an example of re-mapping F13 to SysRq?

KEY_F13 is normally assigned to usage 0x00070068. Since you want your F13 to be used as SysRq you need map that usage to KEY_SYSRQ. Please try doing:

echo "458856 84" | keyfuzz -s -d/dev/input/eventX

Signature · 1 line
Dmitry
author
#14 lore raw

Dmitry Torokhov ha scritto:

Quoted 9 lines
Quoted 3 lines

do you see some solution? perhaps can you supply an example of re-mapping F13 to SysRq?

KEY_F13 is normally assigned to usage 0x00070068. Since you want your F13 to be used as SysRq you need map that usage to KEY_SYSRQ. Please try doing:

echo "458856 84" | keyfuzz -s -d/dev/input/eventX

oh, great! finally I got to remap F13 to SysRq (which was keycode 99).
would be nice to have some informations in the manpage about how to find/dump those raw scancodes, cause with showkey/evtest I only see translated keycodes. [I added keyfuzz's author to CC ;)]

thank you Dmitry

Signature · 1 line
Federico Ferri
#15 lore raw
Quoted 4 lines · federico ferri

oh, great! finally I got to remap F13 to SysRq (which was keycode 99). would be nice to have some informations in the manpage about how to find/dump those raw scancodes, cause with showkey/evtest I only see translated keycodes. [I added keyfuzz's author to CC ;)]

Hi Federico,

JFYI - actually Marvin (added to CC) requested that the hid driver (i.e. USB keyboards, etc.) would report also HID usage codes through evdev (exactly to suit the use cases similar to your situation).

Quoting below an excerpt from the mail I sent him some time ago on this matter:

**

drivers/hid/hid-input.c +8 -0
index 7f81789..c604a1d 100644
@@ -818,6 +818,11 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 			field->dpad = usage->code;
 	}
 
+	if (usage->type == EV_KEY) {
+		set_bit(EV_MSC, input->evbit);
+		set_bit(usage->code, input->mscbit);
+	}
+
 	hid_resolv_event(usage->type, usage->code);
 #ifdef CONFIG_HID_DEBUG
 	printk("\n");

@@ -908,6 +913,9 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
 	if ((usage->type == EV_KEY) && (usage->code == 0)) /* Key 0 is "unassigned", not KEY_UNKNOWN */
 		return;
 
+	if (usage->type == EV_KEY)
+		input_event(input, EV_MSC, MSC_SCAN, usage->hid);
+
 	input_event(input, usage->type, usage->code, value);
 
 	if ((field->flags & HID_MAIN_ITEM_RELATIVE) && (usage->type == EV_KEY))

There is an slight issue with this patch though. Imagine you have such a keyboard that sends in one field the status of Caps/Num/Shift/etc and in another field the pressed key(s) itself, i.e. something like this:

INPUT[INPUT]

    Field(0)
      Usage(8)
        Keyboard.00e0
        Keyboard.00e1
        Keyboard.00e2
        Keyboard.00e3
        Keyboard.00e4
        Keyboard.00e5
        Keyboard.00e6
        Keyboard.00e7
      Logical Minimum(0)
      Logical Maximum(1)
      Report Size(1)
      Report Count(8)
      Report Offset(0)
      Flags( Variable Absolute )

...

and then the next field is used to transfer the actual keys. Therefore, with this patch, hid-input sees a report containing usages e0-e7 set to zero (when no caps/num/shift/... was pressed). This results in evtest seeing this (with the patch below applied) for a single keypress:

Event: time 1182930853.026146, type 4 (Misc), code 4 (ScanCode), value 700e0
Event: time 1182930853.026159, type 4 (Misc), code 4 (ScanCode), value 700e1
Event: time 1182930853.026168, type 4 (Misc), code 4 (ScanCode), value 700e2
Event: time 1182930853.026178, type 4 (Misc), code 4 (ScanCode), value 700e3
Event: time 1182930853.026187, type 4 (Misc), code 4 (ScanCode), value 700e4
Event: time 1182930853.026197, type 4 (Misc), code 4 (ScanCode), value 700e5
Event: time 1182930853.026206, type 4 (Misc), code 4 (ScanCode), value 700e6
Event: time 1182930853.026216, type 4 (Misc), code 4 (ScanCode), value 700e7
Event: time 1182930853.026227, type 4 (Misc), code 4 (ScanCode), value 70004
Event: time 1182930853.026233, type 1 (Key), code 30 (A), value 1
Event: time 1182930853.026237, -------------- Report Sync ------------
Event: time 1182930853.114137, type 4 (Misc), code 4 (ScanCode), value 700e0
Event: time 1182930853.114150, type 4 (Misc), code 4 (ScanCode), value 700e1
Event: time 1182930853.114160, type 4 (Misc), code 4 (ScanCode), value 700e2
Event: time 1182930853.114169, type 4 (Misc), code 4 (ScanCode), value 700e3
Event: time 1182930853.114179, type 4 (Misc), code 4 (ScanCode), value 700e4
Event: time 1182930853.114188, type 4 (Misc), code 4 (ScanCode), value 700e5
Event: time 1182930853.114198, type 4 (Misc), code 4 (ScanCode), value 700e6
Event: time 1182930853.114207, type 4 (Misc), code 4 (ScanCode), value 700e7
Event: time 1182930853.114218, type 4 (Misc), code 4 (ScanCode), value 70004
Event: time 1182930853.114223, type 1 (Key), code 30 (A), value 0
Event: time 1182930853.114227, -------------- Report Sync ------------

I don't think this is what you want. We could alternatively for example remember all the previous states of all the KEY controls and report scancodes only of those which got changed, this would solve this issue. **

However, this might look like an overhead.

Signature · 1 line
Jiri Kosina
#16 lore raw

drivers/hid/hid-input.c static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel · line 823

Quoted 12 lines · Jiri Kosina

**

drivers/hid/hid-input.c +3 -0
index 7f81789..c604a1d 100644
@@ -818,6 +818,11 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 			field->dpad = usage->code;
 	}
 
+	if (usage->type == EV_KEY) {
+		set_bit(EV_MSC, input->evbit);
+		set_bit(usage->code, input->mscbit);

Should be set_bit(MSC_SCAN, input->mscbit);

Quoted 6 lines
drivers/hid/hid-input.c +2 -0
+	}
+
 	hid_resolv_event(usage->type, usage->code);
 #ifdef CONFIG_HID_DEBUG
 	printk("\n");

drivers/hid/hid-input.c void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct · line 921

Quoted 9 lines
drivers/hid/hid-input.c +3 -0
@@ -908,6 +913,9 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
 	if ((usage->type == EV_KEY) && (usage->code == 0)) /* Key 0 is "unassigned", not KEY_UNKNOWN */
 		return;
 
+	if (usage->type == EV_KEY)
+		input_event(input, EV_MSC, MSC_SCAN, usage->hid);
+
 	input_event(input, usage->type, usage->code, value);
 
 	if ((field->flags & HID_MAIN_ITEM_RELATIVE) && (usage->type == EV_KEY))

If you look back through the history of hid-input.c, you'll see that very much this usage reporting was already in there and was removed later because of excessive number of events generated.

It was partly because I didn't limit the usage reporting to keys, for the sake of genericity.

Quoted 51 lines

There is an slight issue with this patch though. Imagine you have such a keyboard that sends in one field the status of Caps/Num/Shift/etc and in another field the pressed key(s) itself, i.e. something like this:

INPUT[INPUT]

    Field(0)
      Usage(8)
        Keyboard.00e0
        Keyboard.00e1
        Keyboard.00e2
        Keyboard.00e3
        Keyboard.00e4
        Keyboard.00e5
        Keyboard.00e6
        Keyboard.00e7
      Logical Minimum(0)
      Logical Maximum(1)
      Report Size(1)
      Report Count(8)
      Report Offset(0)
      Flags( Variable Absolute )

...

and then the next field is used to transfer the actual keys. Therefore, with this patch, hid-input sees a report containing usages e0-e7 set to zero (when no caps/num/shift/... was pressed). This results in evtest seeing this (with the patch below applied) for a single keypress:

Event: time 1182930853.026146, type 4 (Misc), code 4 (ScanCode), value 700e0
Event: time 1182930853.026159, type 4 (Misc), code 4 (ScanCode), value 700e1
Event: time 1182930853.026168, type 4 (Misc), code 4 (ScanCode), value 700e2
Event: time 1182930853.026178, type 4 (Misc), code 4 (ScanCode), value 700e3
Event: time 1182930853.026187, type 4 (Misc), code 4 (ScanCode), value 700e4
Event: time 1182930853.026197, type 4 (Misc), code 4 (ScanCode), value 700e5
Event: time 1182930853.026206, type 4 (Misc), code 4 (ScanCode), value 700e6
Event: time 1182930853.026216, type 4 (Misc), code 4 (ScanCode), value 700e7
Event: time 1182930853.026227, type 4 (Misc), code 4 (ScanCode), value 70004
Event: time 1182930853.026233, type 1 (Key), code 30 (A), value 1
Event: time 1182930853.026237, -------------- Report Sync ------------
Event: time 1182930853.114137, type 4 (Misc), code 4 (ScanCode), value 700e0
Event: time 1182930853.114150, type 4 (Misc), code 4 (ScanCode), value 700e1
Event: time 1182930853.114160, type 4 (Misc), code 4 (ScanCode), value 700e2
Event: time 1182930853.114169, type 4 (Misc), code 4 (ScanCode), value 700e3
Event: time 1182930853.114179, type 4 (Misc), code 4 (ScanCode), value 700e4
Event: time 1182930853.114188, type 4 (Misc), code 4 (ScanCode), value 700e5
Event: time 1182930853.114198, type 4 (Misc), code 4 (ScanCode), value 700e6
Event: time 1182930853.114207, type 4 (Misc), code 4 (ScanCode), value 700e7
Event: time 1182930853.114218, type 4 (Misc), code 4 (ScanCode), value 70004
Event: time 1182930853.114223, type 1 (Key), code 30 (A), value 0
Event: time 1182930853.114227, -------------- Report Sync ------------

Quoted 6 lines

I don't think this is what you want. We could alternatively for example remember all the previous states of all the KEY controls and report scancodes only of those which got changed, this would solve this issue. **

However, this might look like an overhead.

Well, the input subsystem knows whether a key state report will be forwarded or not. Maybe we could (ab)use the knowledge.

Signature · 2 lines
Vojtech Pavlik
Director SuSE Labs
#17 lore raw

What's the current state of this patch?

- Marvin Raaijmakers

Quoted 99 lines · Vojtech Pavlik

drivers/hid/hid-input.c static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel · line 823

Quoted 12 lines · Jiri Kosina

**

drivers/hid/hid-input.c +3 -0
index 7f81789..c604a1d 100644
@@ -818,6 +818,11 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 			field->dpad = usage->code;
 	}
 
+	if (usage->type == EV_KEY) {
+		set_bit(EV_MSC, input->evbit);
+		set_bit(usage->code, input->mscbit);

Should be set_bit(MSC_SCAN, input->mscbit);

Quoted 6 lines
drivers/hid/hid-input.c +2 -0
+	}
+
 	hid_resolv_event(usage->type, usage->code);
 #ifdef CONFIG_HID_DEBUG
 	printk("\n");

drivers/hid/hid-input.c void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct · line 921

Quoted 9 lines
drivers/hid/hid-input.c +3 -0
@@ -908,6 +913,9 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
 	if ((usage->type == EV_KEY) && (usage->code == 0)) /* Key 0 is "unassigned", not KEY_UNKNOWN */
 		return;
 
+	if (usage->type == EV_KEY)
+		input_event(input, EV_MSC, MSC_SCAN, usage->hid);
+
 	input_event(input, usage->type, usage->code, value);
 
 	if ((field->flags & HID_MAIN_ITEM_RELATIVE) && (usage->type == EV_KEY))

If you look back through the history of hid-input.c, you'll see that very much this usage reporting was already in there and was removed later because of excessive number of events generated.

It was partly because I didn't limit the usage reporting to keys, for the sake of genericity.

Quoted 51 lines

There is an slight issue with this patch though. Imagine you have such a keyboard that sends in one field the status of Caps/Num/Shift/etc and in another field the pressed key(s) itself, i.e. something like this:

INPUT[INPUT]

    Field(0)
      Usage(8)
        Keyboard.00e0
        Keyboard.00e1
        Keyboard.00e2
        Keyboard.00e3
        Keyboard.00e4
        Keyboard.00e5
        Keyboard.00e6
        Keyboard.00e7
      Logical Minimum(0)
      Logical Maximum(1)
      Report Size(1)
      Report Count(8)
      Report Offset(0)
      Flags( Variable Absolute )

...

and then the next field is used to transfer the actual keys. Therefore, with this patch, hid-input sees a report containing usages e0-e7 set to zero (when no caps/num/shift/... was pressed). This results in evtest seeing this (with the patch below applied) for a single keypress:

Event: time 1182930853.026146, type 4 (Misc), code 4 (ScanCode), value 700e0
Event: time 1182930853.026159, type 4 (Misc), code 4 (ScanCode), value 700e1
Event: time 1182930853.026168, type 4 (Misc), code 4 (ScanCode), value 700e2
Event: time 1182930853.026178, type 4 (Misc), code 4 (ScanCode), value 700e3
Event: time 1182930853.026187, type 4 (Misc), code 4 (ScanCode), value 700e4
Event: time 1182930853.026197, type 4 (Misc), code 4 (ScanCode), value 700e5
Event: time 1182930853.026206, type 4 (Misc), code 4 (ScanCode), value 700e6
Event: time 1182930853.026216, type 4 (Misc), code 4 (ScanCode), value 700e7
Event: time 1182930853.026227, type 4 (Misc), code 4 (ScanCode), value 70004
Event: time 1182930853.026233, type 1 (Key), code 30 (A), value 1
Event: time 1182930853.026237, -------------- Report Sync ------------
Event: time 1182930853.114137, type 4 (Misc), code 4 (ScanCode), value 700e0
Event: time 1182930853.114150, type 4 (Misc), code 4 (ScanCode), value 700e1
Event: time 1182930853.114160, type 4 (Misc), code 4 (ScanCode), value 700e2
Event: time 1182930853.114169, type 4 (Misc), code 4 (ScanCode), value 700e3
Event: time 1182930853.114179, type 4 (Misc), code 4 (ScanCode), value 700e4
Event: time 1182930853.114188, type 4 (Misc), code 4 (ScanCode), value 700e5
Event: time 1182930853.114198, type 4 (Misc), code 4 (ScanCode), value 700e6
Event: time 1182930853.114207, type 4 (Misc), code 4 (ScanCode), value 700e7
Event: time 1182930853.114218, type 4 (Misc), code 4 (ScanCode), value 70004
Event: time 1182930853.114223, type 1 (Key), code 30 (A), value 0
Event: time 1182930853.114227, -------------- Report Sync ------------

Quoted 6 lines

I don't think this is what you want. We could alternatively for example remember all the previous states of all the KEY controls and report scancodes only of those which got changed, this would solve this issue. **

However, this might look like an overhead.

Well, the input subsystem knows whether a key state report will be forwarded or not. Maybe we could (ab)use the knowledge.

#18 lore raw

(CCs trimmed a little bit)

Quoted 1 line · Marvin Raaijmakers

What's the current state of this patch?

Hi Marvin,

does the patch below establish the functionality you are looking for? Please note that we do the reporting only for such usages that have the type of EV_KEY.

drivers/hid/hid-input.c +8 -0
index 8edbd30..7f4731b 100644
@@ -888,6 +888,11 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 		set_bit(KEY_VOLUMEDOWN, input->keybit);
 	}
 
+	if (usage->type == EV_KEY) {
+		set_bit(EV_MSC, input->evbit);
+		set_bit(MSC_SCAN, input->mscbit);
+	}
+
 	hid_resolv_event(usage->type, usage->code);
 
 	dbg_hid_line("\n");

@@ -991,6 +996,9 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
 		return;
 	}
 
+	if (usage->type == EV_KEY && !!test_bit(usage->code, input->key) != value)
+		input_event(input, EV_MSC, MSC_SCAN, usage->hid);
+
 	input_event(input, usage->type, usage->code, value);
 
 	if ((field->flags & HID_MAIN_ITEM_RELATIVE) && (usage->type == EV_KEY))
Signature · 1 line
Jiri Kosina
#19 lore raw

This code doesn't filter out the "status events" right? So that still needs to be done(?).

- Marvin

Quoted 40 lines · Jiri Kosina

(CCs trimmed a little bit)

Quoted 1 line · Marvin Raaijmakers

What's the current state of this patch?

Hi Marvin,

does the patch below establish the functionality you are looking for? Please note that we do the reporting only for such usages that have the type of EV_KEY.

drivers/hid/hid-input.c +8 -0
index 8edbd30..7f4731b 100644
@@ -888,6 +888,11 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 		set_bit(KEY_VOLUMEDOWN, input->keybit);
 	}
 
+	if (usage->type == EV_KEY) {
+		set_bit(EV_MSC, input->evbit);
+		set_bit(MSC_SCAN, input->mscbit);
+	}
+
 	hid_resolv_event(usage->type, usage->code);
 
 	dbg_hid_line("\n");

@@ -991,6 +996,9 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
 		return;
 	}
 
+	if (usage->type == EV_KEY && !!test_bit(usage->code, input->key) != value)
+		input_event(input, EV_MSC, MSC_SCAN, usage->hid);
+
 	input_event(input, usage->type, usage->code, value);
 
 	if ((field->flags & HID_MAIN_ITEM_RELATIVE) && (usage->type == EV_KEY))
#20 lore raw
Quoted 2 lines · Marvin Raaijmakers

This code doesn't filter out the "status events" right? So that still needs to be done(?).

Hi Marvin,

I am not sure what you mean by that -- this patch is supposed to issue input_event(EV_MSC, MSC_SCAN) if and only if it would mean the change in key state.

Signature · 1 line
Jiri Kosina
#21 lore raw

Ah this is the part that checks for a change in key state:
!!test_bit(usage->code, input->key) != value

Well, in that case: yes, this is the functionality I was looking for.

- Marvin

Quoted 11 lines · Jiri Kosina
Quoted 2 lines · Marvin Raaijmakers

This code doesn't filter out the "status events" right? So that still needs to be done(?).

Hi Marvin,

I am not sure what you mean by that -- this patch is supposed to issue input_event(EV_MSC, MSC_SCAN) if and only if it would mean the change in key state.