Alan,
> >
> > So, how it used to be up until 2.6.24:
> > Short transfer was detected inside qh_completions. EREMOTEIO status was
> > set to urb_status. By means of qtd_copy_status function and urb_status
> > field, the EREMOTEIO status was carried through all of qtd belonging to
> > the URB. As EREMOTEIO is a "error" status, all subsequent qtd of the
> > same URB even with QTD_STS_ACTIVE (!!!) were unlinked.
>
> That's the key point: -EREMOTEIO is an "error" status. But 0 is an
> unusual status also; the "normal" status is -EINPROGRESS.
status is set to -EREMOTEIO at the end of the main while iteration when
we detect short read condition for the current qtd, then it changed to
0.
However, 0 status is almost immediately changed to -EINPROGRESS in the
beginning of the while loop (inside if (last) clause), when we are
analyzing next qtd.
> > And ehci_urb_done
> > was eventually called (calling in turn usb_hcd_giveback_urb).
> >
> > After your patch:
> > Short transfer was detected in qh_completions. EREMOTEIO "error" status
> > was set to last_status. Then it was almost immediately replaced by
> > "normal" EINPROGRESS status.
>
> Yes. However the "if ((token & QTD_STS_HALT) != 0)" test should
> succeed, causing "stopped" to be set to 1.
> > All subsequent qtd of the same URB have
> > QTD_STS_ACTIVE flag set... forever.
>
> That shouldn't matter. Since "stopped" is set and last_status isn't
> -EINPROGRESS, the QTD shouldn't be skipped over; it should be removed
> from the list.
Unfortunately, status is -EINPROGRESS (see above).
> In the case where (last->urb == urb) we are handling one of these
> still-active QTDs following a bad one. In this case we shouldn't set
> last_status back to -EINPROGRESS. So perhaps we need to move that
> statement into the "if" block:
>
> if (last) {
> if (likely (last->urb != urb)) {
> ehci_urb_done(ehci, last->urb, last_status);
> count++;
> last_status = -EINPROGRESS;
> }
> ehci_qtd_free (ehci, last);
> last = NULL;
> }
>
> Does that suffice to fix your problem?
That's exactly what I did in the patch I've sent you. In addition to
that, last_status carried -EREMOTEIO instead of 0. But I guess it's a
matter of preference. Important thing for the fix is to carry an error
status through all USB's qtds.
Here is the amended patch:
[PATCH] USB: ehci: Fixes completion for multi-qtd URB the short read case
Signed-off-by: Misha Zhilin <[EMAIL PROTECTED]>
diff -ur a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
--- a/drivers/usb/host/ehci-q.c 2008-02-11 00:51:11.000000000 -0500
+++ b/drivers/usb/host/ehci-q.c 2008-02-22 13:46:43.000000000 -0500
@@ -315,10 +315,10 @@
if (likely (last->urb != urb)) {
ehci_urb_done(ehci, last->urb, last_status);
count++;
+ last_status = -EINPROGRESS;
}
ehci_qtd_free (ehci, last);
last = NULL;
- last_status = -EINPROGRESS;
}
/* ignore urbs submitted during completions we reported */
-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
