On Wednesday 09 February 2005 11:59 am, Brian Murphy wrote:
> I would very much like to see this patch or equivalent in the next
> official kernel:
I'd call it three separate patches:
> --- linux-2.6.11-rc3.orig/drivers/usb/core/devio.c 2005-02-05
> 10:41:56.000000000 +0100
> +++ linux-2.6.11-rc3/drivers/usb/core/devio.c 2005-02-07
> 01:35:11.000000000 +0100
> @@ -661,8 +661,9 @@
> }
> kfree(tbuf);
> if (i < 0) {
> - dev_warn(&dev->dev, "usbfs: USBDEVFS_BULK failed "
> - "ep 0x%x len %u ret %d\n", bulk.ep, bulk.len, i);
> + if (i != -ETIMEDOUT)
> + dev_warn(&dev->dev, "usbfs: USBDEVFS_BULK failed "
> + "ep 0x%x len %u ret %d\n",
> bulk.ep, bulk.len, i);
> return i;
> }
> return len2;
Makes sense to me. Timeouts are common not-exactly-errors.
In fact, it'd make sense to me to have that be dev_dbg() not
dev_warn() ... the appropriate place to have a policy about
whether that fault needs to generate any message at all is
in userspace. (And that diagnostic is pretty useless anyway,
it doesn't even identify which process -- of hundreds! -- was
making the request.)
> --- linux-2.6.11-rc3.orig/drivers/usb/core/message.c 2005-02-05
> 10:41:57.000000000 +0100
> +++ linux-2.6.11-rc3/drivers/usb/core/message.c 2005-02-07
> 01:40:22.000000000 +0100
> @@ -65,12 +65,10 @@
> status = urb->status;
> /* note: HCDs return ETIMEDOUT for other reasons too */
> if (status == -ECONNRESET) {
> - dev_warn(&urb->dev->dev,
> - "%s timed out on ep%d%s\n",
> - current->comm,
> - usb_pipeendpoint(urb->pipe),
> - usb_pipein(urb->pipe) ? "in" : "out");
> - status = -ETIMEDOUT;
> + if (urb->actual_length > 0)
> + status = 0;
> + else
> + status = -ETIMEDOUT;
> }
> if (timeout > 0)
> del_timer_sync(&timer);
Well, I like the second part -- returning non-error when any data
was received -- better than the first. I can't see any reason to
discard data that was received, and have said as much before.
Re the first, although normally I'd think a diagnostic should be up
at a higher level, virtually no drivers have them. And it's sometimes
been very useful to have a decent diagnostic in that path, especially
for control transfers ... which are used all over the place, and
are rarely associated with useful diagnostics. Though again, maybe
that should be a dev_dbg() in any case.
> Another comment: it seems like the data loss is worse for ohci than for
> ehci but both lose data in this way.
That is, it seems like the timeouts in your driver cause more trouble
at lower data rates than high. Presumably you've looked at using
longer timeouts? Bulk and control traffic are both on a "bandwidth
as available" basis, so a busy USB bus is going to generate timeouts
more often.
- Dave
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
