* Alan Stern | 2012-08-29 13:31:54 [-0400]:
>On Wed, 29 Aug 2012, Sebastian Andrzej Siewior wrote:
>> --- a/drivers/net/usb/usbnet.c
>> +++ b/drivers/net/usb/usbnet.c
>> @@ -194,6 +194,7 @@ static void intr_complete (struct urb *urb)
>> /* software-driven interface shutdown */
>> case -ENOENT: /* urb killed */
>> case -ESHUTDOWN: /* hardware gone */
>> + case -EPIPE:
>> netif_dbg(dev, ifdown, dev->net,
>> "intr shutdown, code %d\n", status);
>> return;
>
>No, that's not right. For one thing, the error is -EPROTO, not -EPIPE.
Ehm yes.
>But aside from that, usbnet has a comment a little farther down:
>
> /* NOTE: not throttling like RX/TX, since this endpoint
> * already polls infrequently
> */
>
>Corresponding to this, there's a comment in dummy-hcd:
>
> case PIPE_INTERRUPT:
> /* FIXME is it urb->interval since the last xfer?
> * this almost certainly polls too fast.
> */
>
>So the problem appears to be that dummy-hcd doesn't implement the right
>interval for periodic transfers. I'd accept a patch to fix that.
That is also missing. However, we are pulled down, that means
find_endpoint() in dummy returns always NULL due to !active(). So we
never get here to delay the interrupt packet. If we remove that active
check in find address then we have the same problem once
dummy_disable() disables the endpoint on UDC side. *Now* we can't find
a match between URB and endpoint.
As a result we _loop_ in timer interrupt and won't leave. Once we
deliver a packet we start the URB list from the beginning. If the
bandwith runs <= 0 and we have BULK URBs only left then we might end.
That means we have either to account bandwith for the failures and
consider the bandwith also for INT URBs or something like this:
--- a/drivers/usb/gadget/dummy_hcd.c
+++ b/drivers/usb/gadget/dummy_hcd.c
@@ -1668,6 +1668,7 @@ static void dummy_timer(unsigned long _dum_hcd)
unsigned long flags;
int limit, total;
int i;
+ int errors_allowed = 4;
/* simplistic model for one frame's bandwidth */
switch (dum->gadget.speed) {
@@ -1708,6 +1709,8 @@ static void dummy_timer(unsigned long _dum_hcd)
}
restart:
+ if (!errors_allowed)
+ goto leave_now;
list_for_each_entry_safe(urbp, tmp, &dum_hcd->urbp_list, urbp_list) {
struct urb *urb;
struct dummy_request *req;
@@ -1740,6 +1743,7 @@ restart:
dev_dbg(dummy_dev(dum_hcd),
"no ep configured for urb %p\n",
urb);
+ errors_allowed--;
status = -EPROTO;
goto return_urb;
}
@@ -1755,6 +1759,7 @@ restart:
/* NOTE: must not be iso! */
dev_dbg(dummy_dev(dum_hcd), "ep %s halted, urb %p\n",
ep->ep.name, urb);
+ errors_allowed--;
status = -EPIPE;
goto return_urb;
}
@@ -1864,6 +1869,7 @@ return_urb:
goto restart;
}
+leave_now:
if (list_empty(&dum_hcd->urbp_list)) {
usb_put_dev(dum_hcd->udev);
Or we loop and loop and loopâ?Š
>Alan Stern
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
