Bug 181425 - USB keyboard with full N-key rollover not working
Summary: USB keyboard with full N-key rollover not working
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: usb (show other bugs)
Version: 10.0-CURRENT
Hardware: Any Any
: Normal Affects Only Me
Assignee: freebsd-usb (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-20 12:10 UTC by Andrey Zholos
Modified: 2020-07-30 12:12 UTC (History)
1 user (show)

See Also:


Attachments
file.txt (517 bytes, text/plain)
2013-08-20 12:10 UTC, Andrey Zholos
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey Zholos 2013-08-20 12:10:00 UTC
The keyboard I am using is a Max Keyboard Nighthawk X9. It features
full N-key rollover, which is supposed to allow pressing all keys at
once, so it reports all keys as a bitmap, rather than the usual 6-byte
array of keycodes. This confuses the ukbd driver.

The relevant part of dmesg with hw.usb.ukbd.debug=1:

    ugen0.4: <EST> at usbus0
    ukbd0: <EST Gaming keyboard, class 0/0, rev 2.00/0.97, addr 3> on usbus0
    ukbd_attach: Parsing HID descriptor of 65 bytes
    ukbd_parse_hid: Found left control
    ukbd_parse_hid: Found right control
    ukbd_parse_hid: Found left shift
    ukbd_parse_hid: Found right shift
    ukbd_parse_hid: Found left alt
    ukbd_parse_hid: Found right alt
    ukbd_parse_hid: Found left GUI
    ukbd_parse_hid: Found right GUI
    ukbd_parse_hid: Found keyboard events
    ukbd_parse_hid: Found keyboard numlock
    ukbd_parse_hid: Found keyboard capslock
    ukbd_parse_hid: Found keyboard scrolllock
    ukbd_set_leds: leds=0x00
    ukbd_set_leds: leds=0x00
    kbd2 at ukbd0
    ukbd_set_leds_callback: len=1, id=0
    ukbd1: <EST Gaming keyboard, class 0/0, rev 2.00/0.97, addr 3> on usbus0
    ukbd_intr_callback: actlen=8 bytes
    ukbd_intr_callback: modifiers = 0x0000
    ukbd_attach: Parsing HID descriptor of 37 bytes
    ukbd_parse_hid: Found left control
    ukbd_parse_hid: Found right control
    ukbd_parse_hid: Found left shift
    ukbd_parse_hid: Found right shift
    ukbd_parse_hid: Found left alt
    ukbd_parse_hid: Found right alt
    ukbd_parse_hid: Found left GUI
    ukbd_parse_hid: Found right GUI
    ukbd_parse_hid: Found keyboard events
    ukbd_set_leds: leds=0x00
    ukbd_set_leds: leds=0x00
    kbd3 at ukbd1

Most keys don't work and show this:

    ukbd_intr_callback: actlen=15 bytes
    ukbd_intr_callback: modifiers = 0x0000
    ukbd_intr_callback: actlen=15 bytes
    ukbd_intr_callback: modifiers = 0x0000

Modifier keys (e.g. Ctrl) appear to work:

    ukbd_intr_callback: actlen=15 bytes
    ukbd_intr_callback: modifiers = 0x0001
    ukbd_put_key: 0xe0 (224) pressed
    ukbd_intr_callback: actlen=15 bytes
    ukbd_intr_callback: modifiers = 0x0000
    ukbd_put_key: 0x4e0 (1248) released

Multimedia keys don't do anything at all.

Special profile keys (e.g. Fn+PF1) also appear to work:

    ukbd_intr_callback: actlen=8 bytes
    ukbd_intr_callback: modifiers = 0x0000
    ukbd_intr_callback: [0] = 0xf1
    ukbd_put_key: 0xf1 (241) pressed
    ukbd_intr_callback: actlen=8 bytes
    ukbd_intr_callback: modifiers = 0x0000
    ukbd_put_key: 0x4f1 (1265) released
    ukbd_intr_callback: actlen=8 bytes
    ukbd_intr_callback: modifiers = 0x0000

There are three HID descriptors:

    # uhidd -D /dev/ugen0.4
    ugen0.4[0]-> Report descriptor dump:
            USAGE PAGE Generic Desktop(0x1)
            USAGE Keyboard(0x6)[Generic Desktop(0x1)]
            COLLECTION Application(1)
              USAGE PAGE Keyboard(0x7)
              USAGE MINIMUM Keyboard LeftControl(224)
              USAGE MAXIMUM Keyboard Right GUI(231)
              LOGICAL MINIMUM 0
              LOGICAL MAXIMUM 1
              REPORT SIZE 1
              REPORT COUNT 8
              INPUT ( Data Variable Absolute ) (2)
              REPORT SIZE 8
              REPORT COUNT 1
              INPUT ( Const Array Absolute ) (1)
              USAGE PAGE Keyboard(0x7)
              USAGE MINIMUM Reserved (no event indicated)(0)
              USAGE MAXIMUM Unknown Usage(255)
              LOGICAL MINIMUM 0
              LOGICAL MAXIMUM 255
              REPORT SIZE 8
              REPORT COUNT 6
              INPUT ( Data Array Absolute ) (0)
              USAGE PAGE LEDs(0x8)
              USAGE MINIMUM Num Lock(1)
              USAGE MAXIMUM Scroll Lock(3)
              LOGICAL MAXIMUM 1
              REPORT SIZE 1
              REPORT COUNT 3
              OUTPUT ( Data Variable Absolute ) (2)
              REPORT COUNT 5
              OUTPUT ( Const Array Absolute ) (1)
            END COLLECTION
    ugen0.4[0]-> Kernel driver is active
    ugen0.4[0]-> Abort attach since kernel driver is active
    ugen0.4[0]-> Please try running uhidd with option '-u' to detach the kernel drivers
    ugen0.4[1]-> Report descriptor dump:
            USAGE PAGE Consumer(0xc)
            USAGE Consumer Control(0x1)[Consumer(0xc)]
            COLLECTION Application(1)
              USAGE PAGE Consumer(0xc)
              USAGE MINIMUM Unassigned(0)
              USAGE MAXIMUM Unknown Usage(4095)
              LOGICAL MINIMUM 0
              LOGICAL MAXIMUM 4095
              REPORT SIZE 16
              REPORT COUNT 2
              INPUT ( Data Array Absolute ) (0)
            END COLLECTION
    ugen0.4[1]-> Kernel driver is active
    ugen0.4[1]-> Abort attach since kernel driver is active
    ugen0.4[1]-> Please try running uhidd with option '-u' to detach the kernel drivers
    ugen0.4[2]-> Report descriptor dump:
            USAGE PAGE Generic Desktop(0x1)
            USAGE Keyboard(0x6)[Generic Desktop(0x1)]
            COLLECTION Application(1)
              USAGE PAGE Keyboard(0x7)
              USAGE MINIMUM Keyboard LeftControl(224)
              USAGE MAXIMUM Keyboard Right GUI(231)
              LOGICAL MINIMUM 0
              LOGICAL MAXIMUM 1
              REPORT SIZE 1
              REPORT COUNT 8
              INPUT ( Data Variable Absolute ) (2)
              USAGE MINIMUM Reserved (no event indicated)(0)
              USAGE MAXIMUM Keyboard F20(111)
              LOGICAL MINIMUM 0
              LOGICAL MAXIMUM 1
              REPORT SIZE 1
              REPORT COUNT 112
              INPUT ( Data Variable Absolute ) (2)
            END COLLECTION
    ugen0.4[2]-> Kernel driver is active
    ugen0.4[2]-> Abort attach since kernel driver is active
    ugen0.4[2]-> Please try running uhidd with option '-u' to detach the kernel drivers

Note the 112-count bitmap in descriptor number 2. This is what is in
the 15-byte report used for most keys. There is a 6-byte array in
descriptor number 0, but that is only used to report the special
profile keys. Descriptor number 1 is used for multimedia keys.

Fix: The keyboard works fine in boot protocol. Running this

    # usbconfig -d 0.4 add_quirk UQ_KBD_BOOTPROTO

and unplugging and plugging the keyboard back in fixes it, as does
adding the quirk for vendor 0x0665 product 0x6000 in usb_quirk.c.

However, I think it's better to detect this type of HID descriptor,
rather than to enumerate all the products that have it, so I propose
the attached patch.

Without UKBD_FLAG_EVENTS the keyboard gets forced into boot protocol in
ukbd_attach(). I have tested this and it works. Ordinary USB keyboard
should continue working in normal protocol as before, but I haven't
tested this.

I suspect this patch also obviates the need for the Corsair Vengeance
K60 keyboard quirk in usb_quirk.c, because that keyboard has 20-key
rollover and so probably the same sort of descriptor, but I don't have
one to test.

Ideally I would like to get this keyboard to work in normal protocol
with full rollover for perfectionist reasons, but I guess that would
require too many changes to ukbd.c and introduce overhead for the
majority of keyboards.


Patch attached with submission follows:
How-To-Repeat: Plug in a Max Keyboard Nighthawk X9 keyboard. It works in BIOS and in
the FreeBSD boot loader menu, but stops working after boot.
Comment 1 HPS 2013-08-20 12:56:13 UTC
Hi,

Have you looked into the USB HID specification from USB.org regarding 
this? I think USB keyboard stuff is explicitly defined. You are right 
that we don't support more than a few simultaneously pressed keys, and 
that would reguire a bit more changes in ukbd driver.

Instead of applying a quirk, maybe ukbd.c could switch to boot-proto 
automatically when HIO_VARIABLE is set? It is simply a so-called USB 
control request.

I'm not sure if it is better to use uhidd or ukbd for the purpose you 
want to use the keyboard.

Thank you for your investigation!

--HPS

>
> Ideally I would like to get this keyboard to work in normal protocol
> with full rollover for perfectionist reasons, but I guess that would
> require too many changes to ukbd.c and introduce overhead for the
> majority of keyboards.
>
>
> >
> --- sys/dev/usb/input/ukbd.c	(revision 254515)
> +++ sys/dev/usb/input/ukbd.c	(working copy)
> @@ -1130,8 +1130,12 @@
>   	    HID_USAGE2(HUP_KEYBOARD, 0x00),
>   	    hid_input, 0, &sc->sc_loc_events, &flags,
>   	    &sc->sc_id_events)) {
> -		sc->sc_flags |= UKBD_FLAG_EVENTS;
> -		DPRINTFN(1, "Found keyboard events\n");
> +		if (flags & HIO_VARIABLE)
> +			DPRINTFN(1, "Ignoring key bitmap\n");
> +		else {
> +			sc->sc_flags |= UKBD_FLAG_EVENTS;
> +			DPRINTFN(1, "Found keyboard events\n");
> +		}
>   	}
>
>   	/* figure out leds on keyboard */
>
Comment 2 Andrey Zholos 2013-08-20 14:12:25 UTC
> Have you looked into the USB HID specification from USB.org regarding 
> this? I think USB keyboard stuff is explicitly defined. You are right 
> that we don't support more than a few simultaneously pressed keys, and 
> that would reguire a bit more changes in ukbd driver.

Yes, I looked, it was interesting. I was kind of disappointed
afterwards when I realized that the drivers (not just the kernel but
uhidd and Linux) supported just a specific subset of the possible
descriptors.

> Instead of applying a quirk, maybe ukbd.c could switch to boot-proto 
> automatically when HIO_VARIABLE is set? It is simply a so-called USB 
> control request.

Exactly! That's the point of the patch.

The descriptor parser currently assumes that if a keyboard usage range
starts at 0, then it must be an array of bytes. It should also make
sure that flags doesn't have HIO_VARIABLE (check added by this patch),
and maybe that sc_loc_events.size == 8.

The actual switch to boot protocol happens at ukbd.c:1241 - not because
of a quirk, but be because now, with this additional check,
!(sc->sc_flags & UKBD_FLAG_EVENTS).

> I'm not sure if it is better to use uhidd or ukbd for the purpose you 
> want to use the keyboard.

Probably uhidd is the best place for full rollover support. It's needed
anyway to support the multimedia keys. But currently it makes the same
assumptions as ukbd about the events buffer and so doesn't work.

Of course, ukbd has to work with the keyboard at least in boot protocol,
otherwise you need to use a different keyboard to install uhidd.
Comment 3 dfilter service freebsd_committer 2013-08-20 17:21:17 UTC
Author: hselasky
Date: Tue Aug 20 16:21:05 2013
New Revision: 254572
URL: http://svnweb.freebsd.org/changeset/base/254572

Log:
  Force keyboards which don't have the required
  HID fields to use the USB BOOT protocol for now.
  
  PR:		usb/181425
  Submitted by:	Andrey Zholos <aaz@q-fu.com>
  MFC after:	4 weeks

Modified:
  head/sys/dev/usb/input/ukbd.c

Modified: head/sys/dev/usb/input/ukbd.c
==============================================================================
--- head/sys/dev/usb/input/ukbd.c	Tue Aug 20 14:59:31 2013	(r254571)
+++ head/sys/dev/usb/input/ukbd.c	Tue Aug 20 16:21:05 2013	(r254572)
@@ -1130,8 +1130,12 @@ ukbd_parse_hid(struct ukbd_softc *sc, co
 	    HID_USAGE2(HUP_KEYBOARD, 0x00),
 	    hid_input, 0, &sc->sc_loc_events, &flags,
 	    &sc->sc_id_events)) {
-		sc->sc_flags |= UKBD_FLAG_EVENTS;
-		DPRINTFN(1, "Found keyboard events\n");
+		if (flags & HIO_VARIABLE) {
+			DPRINTFN(1, "Ignoring keyboard event control\n");
+		} else {
+			sc->sc_flags |= UKBD_FLAG_EVENTS;
+			DPRINTFN(1, "Found keyboard event array\n");
+		}
 	}
 
 	/* figure out leds on keyboard */
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 4 dfilter service freebsd_committer 2013-09-17 13:53:16 UTC
Author: hselasky
Date: Tue Sep 17 12:53:09 2013
New Revision: 255631
URL: http://svnweb.freebsd.org/changeset/base/255631

Log:
  MFC r254572:
  Force keyboards which don't have the required
  HID fields to use the USB BOOT protocol for now.
  
  PR:		usb/181425

Modified:
  stable/9/sys/dev/usb/input/ukbd.c
Directory Properties:
  stable/9/sys/   (props changed)
  stable/9/sys/dev/   (props changed)

Modified: stable/9/sys/dev/usb/input/ukbd.c
==============================================================================
--- stable/9/sys/dev/usb/input/ukbd.c	Tue Sep 17 12:50:57 2013	(r255630)
+++ stable/9/sys/dev/usb/input/ukbd.c	Tue Sep 17 12:53:09 2013	(r255631)
@@ -1126,8 +1126,12 @@ ukbd_parse_hid(struct ukbd_softc *sc, co
 	    HID_USAGE2(HUP_KEYBOARD, 0x00),
 	    hid_input, 0, &sc->sc_loc_events, &flags,
 	    &sc->sc_id_events)) {
-		sc->sc_flags |= UKBD_FLAG_EVENTS;
-		DPRINTFN(1, "Found keyboard events\n");
+		if (flags & HIO_VARIABLE) {
+			DPRINTFN(1, "Ignoring keyboard event control\n");
+		} else {
+			sc->sc_flags |= UKBD_FLAG_EVENTS;
+			DPRINTFN(1, "Found keyboard event array\n");
+		}
 	}
 
 	/* figure out leds on keyboard */
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 5 dfilter service freebsd_committer 2013-09-17 13:58:29 UTC
Author: hselasky
Date: Tue Sep 17 12:58:17 2013
New Revision: 255633
URL: http://svnweb.freebsd.org/changeset/base/255633

Log:
  MFC r254572:
  Force keyboards which don't have the required
  HID fields to use the USB BOOT protocol for now.
  
  PR:		usb/181425

Modified:
  stable/8/sys/dev/usb/input/ukbd.c
Directory Properties:
  stable/8/sys/   (props changed)
  stable/8/sys/dev/   (props changed)
  stable/8/sys/dev/usb/   (props changed)

Modified: stable/8/sys/dev/usb/input/ukbd.c
==============================================================================
--- stable/8/sys/dev/usb/input/ukbd.c	Tue Sep 17 12:56:37 2013	(r255632)
+++ stable/8/sys/dev/usb/input/ukbd.c	Tue Sep 17 12:58:17 2013	(r255633)
@@ -1118,8 +1118,12 @@ ukbd_parse_hid(struct ukbd_softc *sc, co
 	    HID_USAGE2(HUP_KEYBOARD, 0x00),
 	    hid_input, 0, &sc->sc_loc_events, &flags,
 	    &sc->sc_id_events)) {
-		sc->sc_flags |= UKBD_FLAG_EVENTS;
-		DPRINTFN(1, "Found keyboard events\n");
+		if (flags & HIO_VARIABLE) {
+			DPRINTFN(1, "Ignoring keyboard event control\n");
+		} else {
+			sc->sc_flags |= UKBD_FLAG_EVENTS;
+			DPRINTFN(1, "Found keyboard event array\n");
+		}
 	}
 
 	/* figure out leds on keyboard */
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 6 Eitan Adler freebsd_committer freebsd_triage 2017-12-31 08:01:21 UTC
For bugs matching the following criteria:

Status: In Progress Changed: (is less than) 2014-06-01

Reset to default assignee and clear in-progress tags.

Mail being skipped
Comment 7 Oleksandr Tymoshenko freebsd_committer freebsd_triage 2019-01-21 09:18:09 UTC
There is a commit referencing this PR, but it's still not closed and has been inactive for some time. Closing the PR as fixed but feel free to re-open it if the issue hasn't been completely resolved.

Thanks