On Tue, Jul 03, 2012 at 03:23:16PM -0700, Mike Larkin wrote:
> Many low-cost USB keyboards have a limit of either 3 or 6 simultaneous
> keypresses before they wedge and stop supplying any more keypress events
> (at least until you release one of the pressed keys).
>
> Some newer (usually called "gaming") keyboards use a different way of
> reporting keypress events in order to support an arbitrary number of
> simultaneous keypresses (likely under the assumption that in certain
> games, you might need more than 3 or 6 keys pressed at the same time).
>
> Our USB HID keyboard code had some bad assumptions in it - namely, that
> every key reported in this fashion was a 'modifier' key like shift,
> alt, or control, and that there would only be 8 such keys maximum.
>
> The diff below reworks the HID keyboard code a bit to remove these
> assumptions and support these newer style USB keyboards. It also
> changes the term 'modifier' to 'variable' which is a more proper
> description of these keys (as per the HID spec), in various
> comments and printfs.
>
> If you have a USB keyboard (whether or not it is a "gaming" variety),
> please test this diff to ensure that it does not break you. I've tested
> on i386 and amd64 on a variety of keyboards and haven't seen any
> problems, so it's time to turn it loose to a wider audience.
>
> -ml
This keyboard is still working fine:
uhidev2 at uhub2 port 1 configuration 1 interface 0 "Microsoft Natural\M-.
Ergonomic Keyboard 4000" rev 2.00/1.73 addr 2
uhidev2: iclass 3/1
ukbd1 at uhidev2: 8 variable keys, 6 key codes
wskbd2 at ukbd1 mux 1
wskbd2: connecting to wsdisplay0
uhidev3 at uhub2 port 1 configuration 1 interface 1 "Microsoft Natural\M-.
Ergonomic Keyboard 4000" rev 2.00/1.73 addr 2
uhidev3: iclass 3/0, 1 report id
uhid0 at uhidev3 reportid 1: input=7, output=0, feature=0
Although it's still suffering from the fact I need to detach/attach the cables
upon every boot/suspend otherwise it just sits there playing dead. But that's
unrelated to this diff.
> Index: hidkbd.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/hidkbd.c,v
> retrieving revision 1.5
> diff -a -u -r1.5 hidkbd.c
> --- hidkbd.c 9 Nov 2011 14:22:38 -0000 1.5
> +++ hidkbd.c 29 Jun 2012 06:45:59 -0000
> @@ -41,6 +41,7 @@
> #include <sys/kernel.h>
> #include <sys/device.h>
> #include <sys/ioctl.h>
> +#include <sys/malloc.h>
>
> #include <dev/usb/usb.h>
> #include <dev/usb/usbhid.h>
> @@ -164,6 +165,8 @@
> {
> const char *parserr;
>
> + kbd->sc_var = NULL;
> +
> parserr = hidkbd_parse_desc(kbd, id, desc, dlen);
> if (parserr != NULL) {
> printf(": %s\n", parserr);
> @@ -171,8 +174,8 @@
> }
>
> #ifdef DIAGNOSTIC
> - printf(": %d modifier keys, %d key codes",
> - kbd->sc_nmod, kbd->sc_nkeycode);
> + printf(": %d variable keys, %d key codes",
> + kbd->sc_nvar, kbd->sc_nkeycode);
> #endif
>
> kbd->sc_device = self;
> @@ -245,6 +248,9 @@
> if (kbd->sc_wskbddev != NULL)
> rv = config_detach(kbd->sc_wskbddev, flags);
>
> + if (kbd->sc_var != NULL)
> + free(kbd->sc_var, M_DEVBUF);
> +
> return (rv);
> }
>
> @@ -263,11 +269,9 @@
> }
> #endif
>
> - /* extract key modifiers */
> - ud->modifiers = 0;
> - for (i = 0; i < kbd->sc_nmod; i++)
> - if (hid_get_data(data, &kbd->sc_modloc[i]))
> - ud->modifiers |= kbd->sc_mods[i].mask;
> + /* extract variable keys */
> + for (i = 0; i < kbd->sc_nvar; i++)
> + ud->var[i] = (u_int8_t)hid_get_data(data, &kbd->sc_var[i].loc);
>
> /* extract keycodes */
> memcpy(ud->keycode, data + kbd->sc_keycodeloc.pos / 8,
> @@ -311,7 +315,6 @@
> void
> hidkbd_decode(struct hidkbd *kbd, struct hidkbd_data *ud)
> {
> - uint32_t mod, omod;
> u_int16_t ibuf[MAXKEYS]; /* chars events */
> int s;
> int nkeys, i, j;
> @@ -347,15 +350,15 @@
> return; /* ignore */
> }
> nkeys = 0;
> - mod = ud->modifiers;
> - omod = kbd->sc_odata.modifiers;
> - if (mod != omod)
> - for (i = 0; i < kbd->sc_nmod; i++)
> - if (( mod & kbd->sc_mods[i].mask) !=
> - (omod & kbd->sc_mods[i].mask))
> - ADDKEY(kbd->sc_mods[i].key |
> - (mod & kbd->sc_mods[i].mask
> - ? PRESS : RELEASE));
> +
> + for (i = 0; i < kbd->sc_nvar; i++)
> + if ((kbd->sc_odata.var[i] & kbd->sc_var[i].mask) !=
> + (ud->var[i] & kbd->sc_var[i].mask)) {
> + ADDKEY(kbd->sc_var[i].key |
> + ((ud->var[i] & kbd->sc_var[i].mask) ?
> + PRESS : RELEASE));
> + }
> +
> if (memcmp(ud->keycode, kbd->sc_odata.keycode, kbd->sc_nkeycode) != 0) {
> /* Check for released keys. */
> for (i = 0; i < kbd->sc_nkeycode; i++) {
> @@ -547,10 +550,36 @@
> {
> struct hid_data *d;
> struct hid_item h;
> - int imod;
> + int i, ivar = 0;
>
> - imod = 0;
> kbd->sc_nkeycode = 0;
> +
> + d = hid_start_parse(desc, dlen, hid_input);
> + while (hid_get_item(d, &h)) {
> + if (h.kind != hid_input || (h.flags & HIO_CONST) ||
> + HID_GET_USAGE_PAGE(h.usage) != HUP_KEYBOARD ||
> + h.report_ID != id)
> + continue;
> + if (h.flags & HIO_VARIABLE)
> + ivar++;
> + }
> + hid_end_parse(d);
> +
> + if (ivar > MAXVARS) {
> + DPRINTF((": too many variable keys\n"));
> + ivar = MAXVARS;
> + }
> +
> + kbd->sc_nvar = ivar;
> + kbd->sc_var = (struct hidkbd_variable *)malloc(
> + sizeof(struct hidkbd_variable) * ivar, M_DEVBUF,
> + M_NOWAIT);
> +
> + if (!kbd->sc_var)
> + return NULL;
> +
> + i = 0;
> +
> d = hid_start_parse(desc, dlen, hid_input);
> while (hid_get_item(d, &h)) {
> if (h.kind != hid_input || (h.flags & HIO_CONST) ||
> @@ -562,20 +591,18 @@
> "cnt=%d", imod,
> h.usage, h.flags, h.loc.pos, h.loc.size, h.loc.count));
> if (h.flags & HIO_VARIABLE) {
> - /* modifier reports should be one bit each */
> + /* variable reports should be one bit each */
> if (h.loc.size != 1) {
> - DPRINTF((": bad modifier size\n"));
> + DPRINTF((": bad variable size\n"));
> continue;
> }
> - /* single item */
> - if (imod < MAXMOD) {
> - kbd->sc_modloc[imod] = h.loc;
> - kbd->sc_mods[imod].mask = 1 << imod;
> - kbd->sc_mods[imod].key = HID_GET_USAGE(h.usage);
> - imod++;
> - } else {
> - /* ignore extra modifiers */
> - DPRINTF((": too many modifier keys\n"));
> +
> + /* variable report */
> + if (ivar < MAXVARS) {
> + kbd->sc_var[i].loc = h.loc;
> + kbd->sc_var[i].mask = 1 << (i % 8);
> + kbd->sc_var[i].key = HID_GET_USAGE(h.usage);
> + i++;
> }
> } else {
> /* keys array should be in bytes, on a byte boundary */
> @@ -600,11 +627,10 @@
> }
> DPRINTF(("\n"));
> }
> - kbd->sc_nmod = imod;
> hid_end_parse(d);
>
> /* don't attach if no keys... */
> - if (kbd->sc_nkeycode == 0)
> + if (kbd->sc_nkeycode == 0 && ivar == 0)
> return "no usable key codes array";
>
> hid_locate(desc, dlen, HID_USAGE2(HUP_LEDS, HUD_LED_NUM_LOCK),
> Index: hidkbdsc.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/hidkbdsc.h,v
> retrieving revision 1.2
> diff -a -u -r1.2 hidkbdsc.h
> --- hidkbdsc.h 9 Nov 2011 14:22:38 -0000 1.2
> +++ hidkbdsc.h 29 Jun 2012 06:39:37 -0000
> @@ -32,13 +32,19 @@
> */
>
> #define MAXKEYCODE 6
> -#define MAXMOD 8 /* max 32 */
> +#define MAXVARS 128
>
> -#define MAXKEYS (MAXMOD+2*MAXKEYCODE)
> +#define MAXKEYS (MAXVARS+2*MAXKEYCODE)
> +
> +struct hidkbd_variable {
> + struct hid_location loc;
> + u_int8_t mask;
> + u_int8_t key;
> +};
>
> struct hidkbd_data {
> - u_int32_t modifiers;
> u_int8_t keycode[MAXKEYCODE];
> + u_int8_t var[MAXVARS];
> };
>
> struct hidkbd {
> @@ -47,12 +53,8 @@
> struct hidkbd_data sc_odata;
>
> /* input reports */
> - struct hid_location sc_modloc[MAXMOD];
> - u_int sc_nmod;
> - struct {
> - u_int32_t mask;
> - u_int8_t key;
> - } sc_mods[MAXMOD];
> + u_int sc_nvar;
> + struct hidkbd_variable *sc_var;
>
> struct hid_location sc_keycodeloc;
> u_int sc_nkeycode;
>
--
Cheers,
Jasper
"Stay Hungry. Stay Foolish"
