aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Brownell <david-b@pacbell.net>2008-04-12 08:32:05 -0700
committerGreg Kroah-Hartman <gregkh@suse.de>2008-04-24 21:16:53 -0700
commit4f6676274fb6303a8e8100d086ea8c2c00c0d8e3 (patch)
treec59746300f9a343e13bf102c063c372e34f809d4
parentc6dbf554bc8a79c9caab3dbf891a33c19068f646 (diff)
USB: ehci: qh_completions cleanup and bugfix
Simplify processing of completed qtds, and correct handling of short reads, by removing two state variables: - "qtd_status" wasn't needed. The current URB's status is either OK (-EINPROGRESS) or some fault status. Once a fault appears, the queue halts and any later QTDs are immediately removed, so no temporary status is needed. (Or for typical short reads, it's not treated as a fault, so no queue halt is needed.) - "do_status" was erroneous. Because of how the queue is set up, short control reads can (and should!) be treated like full size reads, and cleaned up the usual way. The status stage will be executed transparently, and usbcore handles the choice of whether to report this status as unexected. The "do_status" problem caused a rather perplexing timing-dependent problem with usbtest case 10. Sometimes it would make the controller skip a dozen transactions while (wrongly) trying to clean up after a short transfer. Fortunately, removing a dcache contention issue made this become trivial to reproduce (on one test rig), so enough clues finally presented themselves ... I think this has been around for a very long time, but was worsened by recent urb->status changes. Signed-off-by: David Brownell <dbrownell@users.sourceforge.net> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
-rw-r--r--drivers/usb/host/ehci-q.c34
1 files changed, 16 insertions, 18 deletions
diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index 64942ec584c..315c7c14aaa 100644
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -242,7 +242,8 @@ __acquires(ehci->lock)
if (unlikely(urb->unlinked)) {
COUNT(ehci->stats.unlink);
} else {
- if (likely(status == -EINPROGRESS))
+ /* report non-error and short read status as zero */
+ if (status == -EINPROGRESS || status == -EREMOTEIO)
status = 0;
COUNT(ehci->stats.complete);
}
@@ -283,7 +284,6 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
int last_status = -EINPROGRESS;
int stopped;
unsigned count = 0;
- int do_status = 0;
u8 state;
u32 halt = HALT_BIT(ehci);
@@ -309,7 +309,6 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
struct ehci_qtd *qtd;
struct urb *urb;
u32 token = 0;
- int qtd_status;
qtd = list_entry (entry, struct ehci_qtd, qtd_list);
urb = qtd->urb;
@@ -377,13 +376,6 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
else if (last_status == -EINPROGRESS && !urb->unlinked)
continue;
- /* issue status after short control reads */
- if (unlikely (do_status != 0)
- && QTD_PID (token) == 0 /* OUT */) {
- do_status = 0;
- continue;
- }
-
/* qh unlinked; token in overlay may be most current */
if (state == QH_STATE_IDLE
&& cpu_to_hc32(ehci, qtd->qtd_dma)
@@ -401,15 +393,21 @@ halt:
}
}
- /* remove it from the queue */
- qtd_status = qtd_copy_status(ehci, urb, qtd->length, token);
- if (unlikely(qtd_status == -EREMOTEIO)) {
- do_status = (!urb->unlinked &&
- usb_pipecontrol(urb->pipe));
- qtd_status = 0;
+ /* unless we already know the urb's status, collect qtd status
+ * and update count of bytes transferred. in common short read
+ * cases with only one data qtd (including control transfers),
+ * queue processing won't halt. but with two or more qtds (for
+ * example, with a 32 KB transfer), when the first qtd gets a
+ * short read the second must be removed by hand.
+ */
+ if (last_status == -EINPROGRESS) {
+ last_status = qtd_copy_status(ehci, urb,
+ qtd->length, token);
+ if (last_status == -EREMOTEIO
+ && (qtd->hw_alt_next
+ & EHCI_LIST_END(ehci)))
+ last_status = -EINPROGRESS;
}
- if (likely(last_status == -EINPROGRESS))
- last_status = qtd_status;
/* if we're removing something not at the queue head,
* patch the hardware queue pointer.