aboutsummaryrefslogtreecommitdiffstats
path: root/drivers/usb/input
diff options
context:
space:
mode:
authorChristian Krause <chkr@plauener.de>2005-10-17 17:30:48 -0400
committerLinus Torvalds <torvalds@g5.osdl.org>2005-10-17 17:45:49 -0400
commit13b58ee51802a45d2b8853ffe0003d9fa768195c (patch)
treef08cf0e2a4afbd007fbc11864cd3c4fa0df6e9d8 /drivers/usb/input
parente9b765decfb49ddc105d303d491e1bee9769436f (diff)
[PATCH] USB: fix bug in handling of highspeed usb HID devices
During the development of an USB device I found a bug in the handling of Highspeed HID devices in the kernel. What happened? Highspeed HID devices are correctly recognized and enumerated by the kernel. But even if usbhid kernel module is loaded, no HID reports are received by the kernel. The output of the hardware USB analyzer told me that the host doesn't even poll for interrupt IN transfers (even the "interrupt in" USB transfer are polled by the host). After some debugging in hid-core.c I've found the reason. In case of a highspeed device, the endpoint interval is re-calculated in driver/usb/input/hid-core.c: line 1669: /* handle potential highspeed HID correctly */ interval = endpoint->bInterval; if (dev->speed == USB_SPEED_HIGH) interval = 1 << (interval - 1); Basically this calculation is correct (refer to USB 2.0 spec, 9.6.6). This new calculated value of "interval" is used as input for usb_fill_int_urb: line 1685: usb_fill_int_urb(hid->urbin, dev, pipe, hid->inbuf, 0, hid_irq_in, hid, interval); Unfortunately the same calculation as above is done a second time in usb_fill_int_urb in the file include/linux/usb.h: line 933: if (dev->speed == USB_SPEED_HIGH) urb->interval = 1 << (interval - 1); else urb->interval = interval; This means, that if the endpoint descriptor (of a high speed device) specifies e.g. bInterval = 7, the urb->interval gets the value: hid-core.c: interval = 1 << (7-1) = 0x40 = 64 urb->interval = 1 << (interval -1) = 1 << (63) = integer overflow Because of this the value of urb->interval is sometimes negative and is rejected in core/urb.c: line 353: /* too small? */ if (urb->interval <= 0) return -EINVAL; The conclusion is, that the recalculaton of the interval (which is necessary for highspeed) should not be made twice, because this is simply wrong. ;-) Re-calculation in usb_fill_int_urb makes more sense, because it is the most general approach. So it would make sense to remove it from hid-core.c. Because in hid-core.c the interval variable is only used for calling usb_fill_int_urb, it is no problem to remove the highspeed re-calculation in this file. Signed-off-by: Christian Krause <chkr@plauener.de> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Diffstat (limited to 'drivers/usb/input')
-rw-r--r--drivers/usb/input/hid-core.c3
1 files changed, 0 insertions, 3 deletions
diff --git a/drivers/usb/input/hid-core.c b/drivers/usb/input/hid-core.c
index a99865c689c5..41f92b924761 100644
--- a/drivers/usb/input/hid-core.c
+++ b/drivers/usb/input/hid-core.c
@@ -1702,10 +1702,7 @@ static struct hid_device *usb_hid_configure(struct usb_interface *intf)
1702 if ((endpoint->bmAttributes & 3) != 3) /* Not an interrupt endpoint */ 1702 if ((endpoint->bmAttributes & 3) != 3) /* Not an interrupt endpoint */
1703 continue; 1703 continue;
1704 1704
1705 /* handle potential highspeed HID correctly */
1706 interval = endpoint->bInterval; 1705 interval = endpoint->bInterval;
1707 if (dev->speed == USB_SPEED_HIGH)
1708 interval = 1 << (interval - 1);
1709 1706
1710 /* Change the polling interval of mice. */ 1707 /* Change the polling interval of mice. */
1711 if (hid->collection->usage == HID_GD_MOUSE && hid_mousepoll_interval > 0) 1708 if (hid->collection->usage == HID_GD_MOUSE && hid_mousepoll_interval > 0)