On Mon, Aug 16, 2004 at 11:22:13AM -0400, Alan Stern wrote: > On Mon, 16 Aug 2004, Norbert Preining wrote: > > > Hi ALan! > > > > On Fre, 13 Aug 2004, Alan Stern wrote: > > > In the meantime, you can try out this patch and see if it helps at all. > > > > Yup, fixed the problem. Thanks. > > > > One more question: > > > > > @@ -1654,7 +1654,8 @@ > > > usb_fill_int_urb(hid->urbout, dev, pipe, hid->outbuf, 0, > > > hid_irq_out, hid, interval); > > > hid->urbout->transfer_dma = hid->outbuf_dma; > > > - hid->urbout->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; > > > + hid->urbout->transfer_flags |= (URB_ASYNC_UNLINK | > > > + URB_NO_TRANSFER_DMA_MAP); > > > } > > > } > > > > > > I have NO idea about usb programming, but there is a similar line a bit > > above for > > hid->urbin->transfer_flags > > and I wanted to ask wether there should be a URB_ASYNC_UNLINK there, > > too? > > I don't know. Someone who is more familiar with the HID driver (like > Vojtech) will have to answer. All I did was change the parts that were > obviously wrong, but there could easily be other things that are > non-obviously wrong. Ok, the final patch goes here ...
-- Vojtech Pavlik SuSE Labs, SuSE CR
[EMAIL PROTECTED], 2004-08-19 17:02:03+02:00, [EMAIL PROTECTED] input: Make sure the HID request queue survives report transfer failures gracefully. Signed-off-by: Vojtech Pavlik <[EMAIL PROTECTED]> Problem-spotted-by: Alan Stern <[EMAIL PROTECTED]> hid-core.c | 95 ++++++++++++++++++++++++++++++++++--------------------------- 1 files changed, 54 insertions(+), 41 deletions(-) diff -Nru a/drivers/usb/input/hid-core.c b/drivers/usb/input/hid-core.c --- a/drivers/usb/input/hid-core.c 2004-08-19 17:02:53 +02:00 +++ b/drivers/usb/input/hid-core.c 2004-08-19 17:02:53 +02:00 @@ -219,17 +219,13 @@ dbg("logical range invalid %d %d", parser->global.logical_minimum, parser->global.logical_maximum); return -1; } - usages = parser->local.usage_index; + + if (!(usages = max_t(int, parser->local.usage_index, parser->global.report_count))) + return 0; /* Ignore padding fields */ offset = report->size; report->size += parser->global.report_size * parser->global.report_count; - if (usages < parser->global.report_count) - usages = parser->global.report_count; - - if (usages == 0) - return 0; /* ignore padding fields */ - if ((field = hid_register_field(report, usages, parser->global.report_count)) == NULL) return 0; @@ -923,20 +919,20 @@ int status; switch (urb->status) { - case 0: /* success */ - hid_input_report(HID_INPUT_REPORT, urb, regs); - break; - case -ECONNRESET: /* unlink */ - case -ENOENT: - case -ESHUTDOWN: - return; - default: /* error */ - dbg("nonzero status in input irq %d", urb->status); + case 0: /* success */ + hid_input_report(HID_INPUT_REPORT, urb, regs); + break; + case -ECONNRESET: /* unlink */ + case -ENOENT: + case -ESHUTDOWN: + return; + default: /* error */ + warn("input irq status %d received", urb->status); } - status = usb_submit_urb (urb, SLAB_ATOMIC); + status = usb_submit_urb(urb, SLAB_ATOMIC); if (status) - err ("can't resubmit intr, %s-%s/input%d, status %d", + err("can't resubmit intr, %s-%s/input%d, status %d", hid->dev->bus->bus_name, hid->dev->devpath, hid->ifnum, status); } @@ -1137,23 +1133,31 @@ struct hid_device *hid = urb->context; unsigned long flags; - if (urb->status) - warn("output irq status %d received", urb->status); + switch (urb->status) { + case 0: /* success */ + case -ECONNRESET: /* unlink */ + case -ENOENT: + case -ESHUTDOWN: + break; + default: /* error */ + warn("output irq status %d received", urb->status); + } spin_lock_irqsave(&hid->outlock, flags); hid->outtail = (hid->outtail + 1) & (HID_OUTPUT_FIFO_SIZE - 1); if (hid->outhead != hid->outtail) { - hid_submit_out(hid); + if (hid_submit_out(hid)) { + clear_bit(HID_OUT_RUNNING, &hid->iofl);; + wake_up(&hid->wait); + } spin_unlock_irqrestore(&hid->outlock, flags); return; } clear_bit(HID_OUT_RUNNING, &hid->iofl); - spin_unlock_irqrestore(&hid->outlock, flags); - wake_up(&hid->wait); } @@ -1166,26 +1170,34 @@ struct hid_device *hid = urb->context; unsigned long flags; - if (urb->status) - warn("ctrl urb status %d received", urb->status); - spin_lock_irqsave(&hid->ctrllock, flags); - if (hid->ctrl[hid->ctrltail].dir == USB_DIR_IN) - hid_input_report(hid->ctrl[hid->ctrltail].report->type, urb, regs); + switch (urb->status) { + case 0: /* success */ + if (hid->ctrl[hid->ctrltail].dir == USB_DIR_IN) + hid_input_report(hid->ctrl[hid->ctrltail].report->type, urb, regs); + case -ECONNRESET: /* unlink */ + case -ENOENT: + case -ESHUTDOWN: + case -EPIPE: /* report not available */ + break; + default: /* error */ + warn("ctrl urb status %d received", urb->status); + } hid->ctrltail = (hid->ctrltail + 1) & (HID_CONTROL_FIFO_SIZE - 1); if (hid->ctrlhead != hid->ctrltail) { - hid_submit_ctrl(hid); + if (hid_submit_ctrl(hid)) { + clear_bit(HID_CTRL_RUNNING, &hid->iofl); + wake_up(&hid->wait); + } spin_unlock_irqrestore(&hid->ctrllock, flags); return; } clear_bit(HID_CTRL_RUNNING, &hid->iofl); - spin_unlock_irqrestore(&hid->ctrllock, flags); - wake_up(&hid->wait); } @@ -1211,7 +1223,8 @@ hid->outhead = head; if (!test_and_set_bit(HID_OUT_RUNNING, &hid->iofl)) - hid_submit_out(hid); + if (hid_submit_out(hid)) + clear_bit(HID_OUT_RUNNING, &hid->iofl); spin_unlock_irqrestore(&hid->outlock, flags); return; @@ -1230,7 +1243,8 @@ hid->ctrlhead = head; if (!test_and_set_bit(HID_CTRL_RUNNING, &hid->iofl)) - hid_submit_ctrl(hid); + if (hid_submit_ctrl(hid)) + clear_bit(HID_CTRL_RUNNING, &hid->iofl); spin_unlock_irqrestore(&hid->ctrllock, flags); } @@ -1282,7 +1296,7 @@ void hid_close(struct hid_device *hid) { if (!--hid->open) - usb_unlink_urb(hid->urbin); + usb_kill_urb(hid->urbin); } /* @@ -1643,7 +1657,7 @@ 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; + hid->urbin->transfer_flags |=(URB_NO_TRANSFER_DMA_MAP | URB_ASYNC_UNLINK); } else { if (hid->urbout) continue; @@ -1653,7 +1667,7 @@ usb_fill_int_urb(hid->urbout, dev, pipe, hid->outbuf, 0, hid_irq_out, hid, 1); hid->urbout->transfer_dma = hid->outbuf_dma; - hid->urbout->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; + hid->urbout->transfer_flags |= (URB_NO_TRANSFER_DMA_MAP | URB_ASYNC_UNLINK); } } @@ -1703,8 +1717,7 @@ hid->ctrlbuf, 1, hid_ctrl, hid); hid->urbctrl->setup_dma = hid->cr_dma; hid->urbctrl->transfer_dma = hid->ctrlbuf_dma; - hid->urbctrl->transfer_flags |= (URB_NO_TRANSFER_DMA_MAP - | URB_NO_SETUP_DMA_MAP); + hid->urbctrl->transfer_flags |= (URB_NO_TRANSFER_DMA_MAP | URB_NO_SETUP_DMA_MAP | URB_ASYNC_UNLINK); return hid; @@ -1730,9 +1743,9 @@ return; usb_set_intfdata(intf, NULL); - usb_unlink_urb(hid->urbin); - usb_unlink_urb(hid->urbout); - usb_unlink_urb(hid->urbctrl); + usb_kill_urb(hid->urbin); + usb_kill_urb(hid->urbout); + usb_kill_urb(hid->urbctrl); if (hid->claimed & HID_CLAIMED_INPUT) hidinput_disconnect(hid);