On Sat, 14 Feb 2004, Vojtech Pavlik wrote:
> > IMHO it also sounds like a good example for what we have discussed
> > recently, i.e. up-rounding the DATA-IN transfer_length to multiple
> > maxpacket size.
>
> The patch looks OK to me, and indeed it could make some devices that
> send more data than we want working.
Ok, thanks for the fast reply.
> > Maybe something like the hack below. It compiles but is completely
> > untested (Well, my mouse is still working ;-). In fact I think there might
> > also be another issue in the hid-core which could easily trigger EOVERFLOW
> > on the int-in endpoint - see the comment in the patch below.
>
> The int-in endpoint isn't active at the time the hid_init_reports()
> function gets executed. The function only runs at init time, and the
> int-in endpoint gets activated on open time.
Ok, I didn't check the path how we would get there. Yes, this rules out we
were racing with int-in completion. However I still don't understand why
we want to change the transfer_length of the hid->urbin several times in
this loop and just leave there with whatever was the result of the last
report there. Please excuse if I'm suggesting something stupid (due to my
lack of hid-knowledge), but wouldn't it be much easier to set the
transfer_length to equal maxpacket exactly once in usb_fill_int_urb?
> So you can remove that comment and resend the patch, and I'll include it
> into my tree and submit to Andrew and Linus later.
Ok, see below. I've followed the approach setting int-in transfer_length
to maxpacket in usb_fill_int_urb.
Again, simple testing with Logitech mouse on OHCI and EHCI (HS/TT hub).
No other hid-devices avail., neither babble-issue nor via-uhci here.
Martin
------------------------
diff -urp linux-2.6.3-rc2/drivers/usb/input/hid-core.c
v2.6.3-rc2-md/drivers/usb/input/hid-core.c
--- linux-2.6.3-rc2/drivers/usb/input/hid-core.c Tue Feb 10 18:14:52 2004
+++ v2.6.3-rc2-md/drivers/usb/input/hid-core.c Sat Feb 14 22:52:33 2004
@@ -1069,22 +1069,37 @@ static int hid_submit_ctrl(struct hid_de
{
struct hid_report *report;
unsigned char dir;
+ int len;
report = hid->ctrl[hid->ctrltail].report;
dir = hid->ctrl[hid->ctrltail].dir;
- if (dir == USB_DIR_OUT)
+ len = ((report->size - 1) >> 3) + 1 + (report->id > 0);
+ if (dir == USB_DIR_OUT) {
hid_output_report(report, hid->ctrlbuf);
-
- hid->urbctrl->transfer_buffer_length = ((report->size - 1) >> 3) + 1 +
(report->id > 0);
- hid->urbctrl->pipe = (dir == USB_DIR_OUT) ? usb_sndctrlpipe(hid->dev, 0) :
usb_rcvctrlpipe(hid->dev, 0);
+ hid->urbctrl->pipe = usb_sndctrlpipe(hid->dev, 0);
+ hid->urbctrl->transfer_buffer_length = len;
+ } else {
+ int maxpacket, padlen;
+
+ hid->urbctrl->pipe = usb_rcvctrlpipe(hid->dev, 0);
+ maxpacket = usb_maxpacket(hid->dev, hid->urbctrl->pipe, 0);
+ if (maxpacket > 0) {
+ padlen = (len + maxpacket - 1) / maxpacket;
+ padlen *= maxpacket;
+ if (padlen > HID_BUFFER_SIZE);
+ padlen = HID_BUFFER_SIZE;
+ } else
+ padlen = 0;
+ hid->urbctrl->transfer_buffer_length = padlen;
+ }
hid->urbctrl->dev = hid->dev;
hid->cr->bRequestType = USB_TYPE_CLASS | USB_RECIP_INTERFACE | dir;
hid->cr->bRequest = (dir == USB_DIR_OUT) ? HID_REQ_SET_REPORT :
HID_REQ_GET_REPORT;
hid->cr->wValue = cpu_to_le16(((report->type + 1) << 8) | report->id);
hid->cr->wIndex = cpu_to_le16(hid->ifnum);
- hid->cr->wLength = cpu_to_le16(hid->urbctrl->transfer_buffer_length);
+ hid->cr->wLength = cpu_to_le16(len);
dbg("submitting ctrl urb");
@@ -1262,7 +1277,6 @@ void hid_init_reports(struct hid_device
struct hid_report_enum *report_enum;
struct hid_report *report;
struct list_head *list;
- int len;
int err, ret;
report_enum = hid->report_enum + HID_INPUT_REPORT;
@@ -1297,9 +1311,6 @@ void hid_init_reports(struct hid_device
list = report_enum->report_list.next;
while (list != &report_enum->report_list) {
report = (struct hid_report *) list;
- len = ((report->size - 1) >> 3) + 1 + report_enum->numbered;
- if (len > hid->urbin->transfer_buffer_length)
- hid->urbin->transfer_buffer_length = len < HID_BUFFER_SIZE ?
len : HID_BUFFER_SIZE;
usb_control_msg(hid->dev, usb_sndctrlpipe(hid->dev, 0),
0x0a, USB_TYPE_CLASS | USB_RECIP_INTERFACE, report->id,
hid->ifnum, NULL, 0, HZ * USB_CTRL_SET_TIMEOUT);
@@ -1518,12 +1529,17 @@ static struct hid_device *usb_hid_config
continue;
if (endpoint->bEndpointAddress & USB_DIR_IN) {
+ int len;
+
if (hid->urbin)
continue;
if (!(hid->urbin = usb_alloc_urb(0, GFP_KERNEL)))
goto fail;
pipe = usb_rcvintpipe(dev, endpoint->bEndpointAddress);
- usb_fill_int_urb(hid->urbin, dev, pipe, hid->inbuf, 0,
+ len = usb_maxpacket(dev, pipe, 0);
+ if (len > HID_BUFFER_SIZE)
+ len = HID_BUFFER_SIZE;
+ usb_fill_int_urb(hid->urbin, dev, pipe, hid->inbuf, len,
hid_irq_in, hid, endpoint->bInterval);
hid->urbin->transfer_dma = hid->inbuf_dma;
hid->urbin->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
diff -urp linux-2.6.3-rc2/drivers/usb/input/hid.h v2.6.3-rc2-md/drivers/usb/input/hid.h
--- linux-2.6.3-rc2/drivers/usb/input/hid.h Mon Jan 26 12:56:15 2004
+++ v2.6.3-rc2-md/drivers/usb/input/hid.h Sat Feb 14 22:36:20 2004
@@ -309,7 +309,7 @@ struct hid_report_enum {
#define HID_REPORT_TYPES 3
-#define HID_BUFFER_SIZE 32
+#define HID_BUFFER_SIZE 64 /* use 64 for compatibility
with all possible packetlen */
#define HID_CONTROL_FIFO_SIZE 256 /* to init devices with >100 reports */
#define HID_OUTPUT_FIFO_SIZE 64
-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id=1356&alloc_id=3438&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
