Date: Sat, 21 Feb 2004 14:35:27 +0000 From: Brian Candler <B.Candler@pobox.com> To: FreeBSD-gnats-submit@FreeBSD.org Subject: kern/63171: USB keyboard rollover error [ukbd.c patch] Message-ID: <E1AuYE3-00006b-3x@vaio.linnet.org> Resent-Message-ID: <200402211440.i1LEeETR099257@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 63171 >Category: kern >Synopsis: USB keyboard rollover error [ukbd.c patch] >Confidential: no >Severity: serious >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Sat Feb 21 06:40:14 PST 2004 >Closed-Date: >Last-Modified: >Originator: Brian Candler >Release: FreeBSD 4.8-RELEASE i386 >Organization: >Environment: System: 4.8-RELEASE FreeBSD 4.8-RELEASE #5: Sat Feb 21 13:49:32 GMT 2004 root@vaio.linnet.org:/usr/src/sys/compile/VAIO i386 Sony Vaio PCG-C1F USB Keyboard (Packard Bell PB-KB400) >Description: N-key rollover fails to work properly with a USB keyboard, giving duplicate characters. Thisi is a prorbleme when youo a rer typing veryr quiuckcly! A second, unrelated issue is that there is a wrong pointer type being passed to usbd_set_polling, which manifests itself as a compiler warning but could cause memory corruption. The attached patch corrects both problems. >How-To-Repeat: Plug in a USB keyboard. Activate it using #!/bin/sh kbdcontrol -k /dev/kbd1 </dev/console kbdcontrol -l /usr/share/syscons/keymaps/uk.cp850.kbd # optional kbdcontrol -r slow # makes it easier to demonstrate the problem Perform the following operations: Press "a", press "s", release "a", press "d" The sequence "asds" (instead of "asd") is printed. If you keep "s" held and keep releasing and pressing "d", you will get "ds" printed each time you press "d". If you enable USB debugging in the kernel, and set # sysctl -w hw.usb.ukbd.debug=1 then you get Feb 20 15:21:21 vaio /kernel: 0x428 (1064) released Feb 20 15:21:21 vaio /kernel: Feb 20 15:21:26 vaio /kernel: 0x4 (4) pressed <<< a Feb 20 15:21:26 vaio /kernel: 4 Feb 20 15:21:26 vaio /kernel: 0x16 (22) pressed <<< s Feb 20 15:21:26 vaio /kernel: 4 22 Feb 20 15:21:26 vaio /kernel: 0x404 (1028) released Feb 20 15:21:26 vaio /kernel: 0x416 (1046) released Feb 20 15:21:26 vaio /kernel: 22 Feb 20 15:21:26 vaio /kernel: 0x7 (7) pressed <<< d Feb 20 15:21:26 vaio /kernel: 0x16 (22) pressed <<< s Feb 20 15:21:26 vaio /kernel: 7 22 Feb 20 15:21:26 vaio /kernel: 0x416 (1046) released Feb 20 15:21:26 vaio /kernel: 7 Feb 20 15:21:27 vaio /kernel: 0x407 (1031) released Feb 20 15:21:27 vaio /kernel: This is even after updating my ukbd.c to the latest in CVS (rev 1.46). >Fix: The keyboard driver keeps a list of keys held (ks_ndata and ks_odata) and looks for differences between them to identify key presses and releases. However it makes the erroneous assumption that the first zero entry seen indicates the end of the list. In fact, when you press the key sequence above, you get (1) _ _ _ _ _ _ Initial empty list (2) a _ _ _ _ _ "a" pressed (3) a s _ _ _ _ "s" pressed (4) _ s _ _ _ _ "a" released (5) d s _ _ _ _ "d" pressed At least this is true with my USB keyboard; I haven't checked the HID spec, but given that my keyboard works correctly under Windows98 with no rollover problems I think it is entitled to behave in this way. The fix is very straightforward, changing 'break' to 'continue' in a few places. The pointer error looks like an oversight, and the last part of the patch makes what seems to be the obvious correction, although I am not intimate with the USB data structures so this ought to be checked by someone who is. (usbd_set_polling requires a usbd_interface_handle, not a usbd_device_handle, as its first parameter). Regards, Brian Candler. --- sys/dev/usb/ukbd.c-1.46 Sat Feb 21 12:17:38 2004 +++ sys/dev/usb/ukbd.c Sat Feb 21 13:47:56 2004 @@ -43,7 +43,7 @@ __FBSDID("$FreeBSD: /repoman/r/ncvs/src/sys/dev/usb/ukbd.c,v 1.46 2004/01/03 15:01:04 sanpei Exp $"); /* - * HID spec: http://www.usb.org/developers/data/devclass/hid1_1.pdf + * HID spec: http://www.usb.org/developers/devclass_docs/HID1_11.pdf */ #include "opt_kbd.h" @@ -746,10 +746,10 @@ for (i = 0; i < NKEYCODE; i++) { key = state->ks_odata.keycode[i]; if (key == 0) - break; + continue; for (j = 0; j < NKEYCODE; j++) { if (ud->keycode[j] == 0) - break; + continue; if (key == ud->keycode[j]) goto rfound; } @@ -762,11 +762,11 @@ for (i = 0; i < NKEYCODE; i++) { key = ud->keycode[i]; if (key == 0) - break; + continue; state->ks_ntime[i] = now + kbd->kb_delay1; for (j = 0; j < NKEYCODE; j++) { if (state->ks_odata.keycode[j] == 0) - break; + continue; if (key == state->ks_odata.keycode[j]) { state->ks_ntime[i] = state->ks_otime[j]; if (state->ks_otime[j] > now) @@ -1327,21 +1327,19 @@ ukbd_poll(keyboard_t *kbd, int on) { ukbd_state_t *state; - usbd_device_handle dev; int s; state = (ukbd_state_t *)kbd->kb_data; - usbd_interface2device_handle(state->ks_iface, &dev); s = splusb(); if (on) { if (state->ks_polling == 0) - usbd_set_polling(dev, on); + usbd_set_polling(state->ks_iface, on); ++state->ks_polling; } else { --state->ks_polling; if (state->ks_polling == 0) - usbd_set_polling(dev, on); + usbd_set_polling(state->ks_iface, on); } splx(s); return 0; >Release-Note: >Audit-Trail: >Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?E1AuYE3-00006b-3x>