[PATCH] input: change SysRq keycode for systems without SysRq key

[PATCH] input: change SysRq keycode for systems without SysRq key

[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)

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 <xaero@inwind.it>

---

diff -uprN linux.orig/drivers/char/keyboard.c linux/drivers/char/keyboard.c
--- linux.orig/drivers/char/keyboard.c	2005-07-09 21:47:50.000000000 +0200
+++ linux/drivers/char/keyboard.c	2005-07-10 14:42:34.000000000 +0200
@@ -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;
 	}
diff -uprN linux.orig/lib/Kconfig.debug linux/lib/Kconfig.debug
--- linux.orig/lib/Kconfig.debug	2005-07-09 21:47:22.000000000 +0200
+++ linux/lib/Kconfig.debug	2005-07-09 21:50:44.000000000 +0200
@@ -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

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[Linus Torvalds]

On Sun, 15 Jul 2007, federico ferri wrote:
>
> 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

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[federico ferri]

Linus Torvalds ha scritto:
> On Sun, 15 Jul 2007, federico ferri wrote:
>   
>> 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
>
>   
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?  ( :

-- 
Federico Ferri

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[Linus Torvalds]

On Sun, 15 Jul 2007, federico ferri wrote:
>> 
> (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(-)

diff --git a/drivers/char/keyboard.c b/drivers/char/keyboard.c
index 90965b4..e21f52b 100644
--- a/drivers/char/keyboard.c
+++ b/drivers/char/keyboard.c
@@ -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;

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[federico ferri]

Linus Torvalds ha scritto:
> 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?)

-- 
Federico Ferri

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[Linus Torvalds]

On Mon, 16 Jul 2007, federico ferri wrote:

> Linus Torvalds ha scritto:
> > 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. 

> thank you, Linus!

You're welcome.

> (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

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[Dmitry Torokhov]

Hi Linus,

On 7/15/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>
> On Mon, 16 Jul 2007, federico ferri wrote:
>
> > Linus Torvalds ha scritto:
> > > 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.
>
> > thank you, Linus!
>
> You're welcome.
>
> > (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.

-- 
Dmitry

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[federico ferri]

Dmitry Torokhov ha scritto:
>> 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?

-- 
Federico Ferri

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[Dmitry Torokhov]

On 7/17/07, federico ferri <xaero@inwind.it> wrote:
> Dmitry Torokhov ha scritto:
> >> 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?
>

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/

-- 
Dmitry

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[federico ferri]

Dmitry Torokhov ha scritto:
> 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
 >>>> /dev/input/event0:
evdev driver version 1.0.0.
device name: PC Speaker
 >>>> /dev/input/event1:
evdev driver version 1.0.0.
device name: Mitsumi Electric Apple Extended USB Keyboard
 >>>> /dev/input/event2:
evdev driver version 1.0.0.
device name: Mitsumi Electric Apple Extended USB Keyboard
 >>>> /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)

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[Dmitry Torokhov]

On 7/18/07, federico ferri <xaero@inwind.it> wrote:
> Dmitry Torokhov ha scritto:
> > 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
>  >>>> /dev/input/event0:
> evdev driver version 1.0.0.
> device name: PC Speaker
>  >>>> /dev/input/event1:
> evdev driver version 1.0.0.
> device name: Mitsumi Electric Apple Extended USB Keyboard
>  >>>> /dev/input/event2:
> evdev driver version 1.0.0.
> device name: Mitsumi Electric Apple Extended USB Keyboard
>  >>>> /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)
>


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

-- 
Dmitry

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[Dmitry Torokhov]

On 7/19/07, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On 7/18/07, federico ferri <xaero@inwind.it> wrote:
> > Dmitry Torokhov ha scritto:
> > > 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
> >  >>>> /dev/input/event0:
> > evdev driver version 1.0.0.
> > device name: PC Speaker
> >  >>>> /dev/input/event1:
> > evdev driver version 1.0.0.
> > device name: Mitsumi Electric Apple Extended USB Keyboard
> >  >>>> /dev/input/event2:
> > evdev driver version 1.0.0.
> > device name: Mitsumi Electric Apple Extended USB Keyboard
> >  >>>> /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)
> >
>
>
> What is 84? For SUB you map a "usage" to a keycode and regular key
                         ^^^ USB

> usages start with 0x00070000...
>
> --
> Dmitry
>


-- 
Dmitry

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[federico ferri]

Dmitry Torokhov ha scritto:
>> # 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?

-- 
Federico Ferri

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[Dmitry Torokhov]

On 7/19/07, federico ferri <xaero@inwind.it> wrote:
> Dmitry Torokhov ha scritto:
> >> # 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
>

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.

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

USB's "scancode".

> 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.

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

>
> 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

-- 
Dmitry

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[federico ferri]

Dmitry Torokhov ha scritto:
>> 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
-- 
Federico Ferri

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[Jiri Kosina]

On Fri, 20 Jul 2007, federico ferri wrote:

> 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:

**
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 7f81789..c604a1d 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -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.

-- 
Jiri Kosina

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[Vojtech Pavlik]

On Mon, Jul 30, 2007 at 04:09:04PM +0200, Jiri Kosina wrote:
> **
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 7f81789..c604a1d 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -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);

> +	}
> +
>  	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))

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.

> 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.

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

-- 
Vojtech Pavlik
Director SuSE Labs

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[Marvin Raaijmakers]

What's the current state of this patch?

- Marvin Raaijmakers

On Thu, 2007-08-02 at 23:27 +0200, Vojtech Pavlik wrote:
> On Mon, Jul 30, 2007 at 04:09:04PM +0200, Jiri Kosina wrote:
> > **
> > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> > index 7f81789..c604a1d 100644
> > --- a/drivers/hid/hid-input.c
> > +++ b/drivers/hid/hid-input.c
> > @@ -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);
> 
> > +	}
> > +
> >  	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))
> 
> 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.
> 
> > 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.
> 
> Well, the input subsystem knows whether a key state report will be
> forwarded or not. Maybe we could (ab)use the knowledge.

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[Jiri Kosina]

(CCs trimmed a little bit)

On Fri, 17 Aug 2007, Marvin Raaijmakers wrote:

> 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.


diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 8edbd30..7f4731b 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -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))

-- 
Jiri Kosina

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[Marvin Raaijmakers]

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

- Marvin

On Fri, 2007-08-17 at 12:22 +0200, Jiri Kosina wrote:
> (CCs trimmed a little bit)
> 
> On Fri, 17 Aug 2007, Marvin Raaijmakers wrote:
> 
> > 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.
> 
> 
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 8edbd30..7f4731b 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -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))
> 

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[Jiri Kosina]

On Fri, 17 Aug 2007, Marvin Raaijmakers wrote:

> 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.

-- 
Jiri Kosina

Re: [PATCH] input: change SysRq keycode for systems without SysRq key

[Marvin Raaijmakers]

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

On Fri, 2007-08-17 at 16:42 +0200, Jiri Kosina wrote:
> On Fri, 17 Aug 2007, Marvin Raaijmakers wrote:
> 
> > 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.
>