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);
