[PATCH 087/142] USB: reorganize urb->status use in ehci-hcd

Greg Kroah-Hartman <[email protected]>
Newsgroups gmane.linux.usb.devel
Message-ID <[email protected]>
From: Alan Stern <[email protected]>

This patch (as974) reorganizes the way ehci-hcd sets urb->status.  It
now keeps the information in a local variable until the last moment.

The patch also simplifies the handling of -EREMOTEIO, since the only
use of that code is to set the do_status flag.

Signed-off-by: Alan Stern <[email protected]>
CC: David Brownell <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
 drivers/usb/host/ehci-q.c     |   62 +++++++++++++++++++++-------------------
 drivers/usb/host/ehci-sched.c |    4 +-
 2 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/drivers/usb/host/ehci-q.c b/drivers/usb/host/ehci-q.c
index a8f5408..794d27e 100644
--- a/drivers/usb/host/ehci-q.c
+++ b/drivers/usb/host/ehci-q.c
@@ -139,63 +139,65 @@ qh_refresh (struct ehci_hcd *ehci, struct ehci_qh *qh)
 
 /*-------------------------------------------------------------------------*/
 
-static void qtd_copy_status (
+static int qtd_copy_status (
 	struct ehci_hcd *ehci,
 	struct urb *urb,
 	size_t length,
 	u32 token
 )
 {
+	int	status = -EINPROGRESS;
+
 	/* count IN/OUT bytes, not SETUP (even short packets) */
 	if (likely (QTD_PID (token) != 2))
 		urb->actual_length += length - QTD_LENGTH (token);
 
 	/* don't modify error codes */
 	if (unlikely(urb->unlinked))
-		return;
+		return status;
 
 	/* force cleanup after short read; not always an error */
 	if (unlikely (IS_SHORT_READ (token)))
-		urb->status = -EREMOTEIO;
+		status = -EREMOTEIO;
 
 	/* serious "can't proceed" faults reported by the hardware */
 	if (token & QTD_STS_HALT) {
 		if (token & QTD_STS_BABBLE) {
 			/* FIXME "must" disable babbling device's port too */
-			urb->status = -EOVERFLOW;
+			status = -EOVERFLOW;
 		} else if (token & QTD_STS_MMF) {
 			/* fs/ls interrupt xfer missed the complete-split */
-			urb->status = -EPROTO;
+			status = -EPROTO;
 		} else if (token & QTD_STS_DBE) {
-			urb->status = (QTD_PID (token) == 1) /* IN ? */
+			status = (QTD_PID (token) == 1) /* IN ? */
 				? -ENOSR  /* hc couldn't read data */
 				: -ECOMM; /* hc couldn't write data */
 		} else if (token & QTD_STS_XACT) {
 			/* timeout, bad crc, wrong PID, etc; retried */
 			if (QTD_CERR (token))
-				urb->status = -EPIPE;
+				status = -EPIPE;
 			else {
 				ehci_dbg (ehci, "devpath %s ep%d%s 3strikes\n",
 					urb->dev->devpath,
 					usb_pipeendpoint (urb->pipe),
 					usb_pipein (urb->pipe) ? "in" : "out");
-				urb->status = -EPROTO;
+				status = -EPROTO;
 			}
 		/* CERR nonzero + no errors + halt --> stall */
 		} else if (QTD_CERR (token))
-			urb->status = -EPIPE;
+			status = -EPIPE;
 		else	/* unknown */
-			urb->status = -EPROTO;
+			status = -EPROTO;
 
 		ehci_vdbg (ehci,
 			"dev%d ep%d%s qtd token %08x --> status %d\n",
 			usb_pipedevice (urb->pipe),
 			usb_pipeendpoint (urb->pipe),
 			usb_pipein (urb->pipe) ? "in" : "out",
-			token, urb->status);
+			token, status);
 
 		/* if async CSPLIT failed, try cleaning out the TT buffer */
-		if (urb->status != -EPIPE
+		if (status != -EPIPE
 				&& urb->dev->tt && !usb_pipeint (urb->pipe)
 				&& ((token & QTD_STS_MMF) != 0
 					|| QTD_CERR(token) == 0)
@@ -212,10 +214,12 @@ static void qtd_copy_status (
 			usb_hub_tt_clear_buffer (urb->dev, urb->pipe);
 		}
 	}
+
+	return status;
 }
 
 static void
-ehci_urb_done (struct ehci_hcd *ehci, struct urb *urb)
+ehci_urb_done(struct ehci_hcd *ehci, struct urb *urb, int status)
 __releases(ehci->lock)
 __acquires(ehci->lock)
 {
@@ -231,17 +235,13 @@ __acquires(ehci->lock)
 		qh_put (qh);
 	}
 
-	spin_lock (&urb->lock);
 	if (unlikely(urb->unlinked)) {
 		COUNT(ehci->stats.unlink);
 	} else {
-		if (likely(urb->status == -EINPROGRESS ||
-				(urb->status == -EREMOTEIO &&
-				 !(urb->transfer_flags & URB_SHORT_NOT_OK))))
-			urb->status = 0;
+		if (likely(status == -EINPROGRESS))
+			status = 0;
 		COUNT(ehci->stats.complete);
 	}
-	spin_unlock (&urb->lock);
 
 #ifdef EHCI_URB_TRACE
 	ehci_dbg (ehci,
@@ -249,13 +249,14 @@ __acquires(ehci->lock)
 		__FUNCTION__, urb->dev->devpath, urb,
 		usb_pipeendpoint (urb->pipe),
 		usb_pipein (urb->pipe) ? "in" : "out",
-		urb->status,
+		status,
 		urb->actual_length, urb->transfer_buffer_length);
 #endif
 
 	/* complete() can reenter this HCD */
 	usb_hcd_unlink_urb_from_ep(ehci_to_hcd(ehci), urb);
 	spin_unlock (&ehci->lock);
+	urb->status = status;
 	usb_hcd_giveback_urb (ehci_to_hcd(ehci), urb);
 	spin_lock (&ehci->lock);
 }
@@ -276,6 +277,7 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
 {
 	struct ehci_qtd		*last = NULL, *end = qh->dummy;
 	struct list_head	*entry, *tmp;
+	int			last_status = -EINPROGRESS;
 	int			stopped;
 	unsigned		count = 0;
 	int			do_status = 0;
@@ -304,6 +306,7 @@ 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;
@@ -311,11 +314,12 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
 		/* clean up any state from previous QTD ...*/
 		if (last) {
 			if (likely (last->urb != urb)) {
-				ehci_urb_done (ehci, last->urb);
+				ehci_urb_done(ehci, last->urb, last_status);
 				count++;
 			}
 			ehci_qtd_free (ehci, last);
 			last = NULL;
+			last_status = -EINPROGRESS;
 		}
 
 		/* ignore urbs submitted during completions we reported */
@@ -351,13 +355,13 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh)
 			stopped = 1;
 
 			if (unlikely (!HC_IS_RUNNING (ehci_to_hcd(ehci)->state)))
-				urb->status = -ESHUTDOWN;
+				last_status = -ESHUTDOWN;
 
 			/* ignore active urbs unless some previous qtd
 			 * for the urb faulted (including short read) or
 			 * its urb was canceled.  we may patch qh or qtds.
 			 */
-			if (likely(urb->status == -EINPROGRESS &&
+			if (likely(last_status == -EINPROGRESS &&
 					!urb->unlinked))
 				continue;
 
@@ -386,14 +390,14 @@ halt:
 		}
 
 		/* remove it from the queue */
-		spin_lock (&urb->lock);
-		qtd_copy_status (ehci, urb, qtd->length, token);
-		if (unlikely(urb->status == -EREMOTEIO)) {
+		qtd_status = qtd_copy_status(ehci, urb, qtd->length, token);
+		if (unlikely(qtd_status == -EREMOTEIO)) {
 			do_status = (!urb->unlinked &&
 					usb_pipecontrol(urb->pipe));
-			urb->status = 0;
+			qtd_status = 0;
 		}
-		spin_unlock (&urb->lock);
+		if (likely(last_status == -EINPROGRESS))
+			last_status = qtd_status;
 
 		if (stopped && qtd->qtd_list.prev != &qh->qtd_list) {
 			last = list_entry (qtd->qtd_list.prev,
@@ -406,7 +410,7 @@ halt:
 
 	/* last urb's completion might still need calling */
 	if (likely (last != NULL)) {
-		ehci_urb_done (ehci, last->urb);
+		ehci_urb_done(ehci, last->urb, last_status);
 		count++;
 		ehci_qtd_free (ehci, last);
 	}
diff --git a/drivers/usb/host/ehci-sched.c b/drivers/usb/host/ehci-sched.c
index 8b267b3..80d99bc 100644
--- a/drivers/usb/host/ehci-sched.c
+++ b/drivers/usb/host/ehci-sched.c
@@ -1627,7 +1627,7 @@ itd_complete (
 
 	/* give urb back to the driver ... can be out-of-order */
 	dev = urb->dev;
-	ehci_urb_done (ehci, urb);
+	ehci_urb_done(ehci, urb, 0);
 	urb = NULL;
 
 	/* defer stopping schedule; completion can submit */
@@ -2000,7 +2000,7 @@ sitd_complete (
 
 	/* give urb back to the driver */
 	dev = urb->dev;
-	ehci_urb_done (ehci, urb);
+	ehci_urb_done(ehci, urb, 0);
 	urb = NULL;
 
 	/* defer stopping schedule; completion can submit */
-- 
1.5.3.4


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.