isp116x-hcd: prepare for urb->status

This patch (as931b), adapted from a patch by Olav Kongas, makes a small
set of conservative changes to the isp116x-hcd driver in preparation
for the removal of urb->status.

finish_request() is moved up in the source and is called
as soon as the URB is known to have completed, rather than
after all the active endpoints have been scanned.

The status of a completed URB is kept in a local variable
and copied to urb->status only when the URB is about to be
given back.

-EREMOTEIO error status for control transfers is set after
the status stage rather than when the short packet arrives.

Some unnecessary uses of urb->lock are removed.

Signed-off-by: Alan Stern <[e-mail]>
Cc: Olav Kongas <[e-mail]>
Signed-off-by: Greg Kroah-Hartman <[e-mail]>

drivers/usb/host/isp116x-hcd.c

···
 				   struct urb, urb_list);
 		ptd = &ep->ptd;
 		len = ep->length;
-		spin_lock(&urb->lock);
 		ep->data = (unsigned char *)urb->transfer_buffer
 		    + urb->actual_length;
 
···
 		    | PTD_EP(ep->epnum);
 		ptd->len = PTD_LEN(len) | PTD_DIR(dir);
 		ptd->faddr = PTD_FA(usb_pipedevice(urb->pipe));
-		spin_unlock(&urb->lock);
 		if (!ep->active) {
 			ptd->mps |= PTD_LAST_MSK;
 			isp116x->atl_last_dir = dir;
 		}
 		isp116x->atl_bufshrt = sizeof(struct ptd) + isp116x->atl_buflen;
 		isp116x->atl_buflen = isp116x->atl_bufshrt + ALIGN(len, 4);
-	}
-}
-
-/*
-  Analyze transfer results, handle partial transfers and errors
-*/
-static void postproc_atl_queue(struct isp116x *isp116x)
-{
-	struct isp116x_ep *ep;
-	struct urb *urb;
-	struct usb_device *udev;
-	struct ptd *ptd;
-	int short_not_ok;
-	u8 cc;
-
-	for (ep = isp116x->atl_active; ep; ep = ep->active) {
-		BUG_ON(list_empty(&ep->hep->urb_list));
-		urb =
-		    container_of(ep->hep->urb_list.next, struct urb, urb_list);
-		udev = urb->dev;
-		ptd = &ep->ptd;
-		cc = PTD_GET_CC(ptd);
-		short_not_ok = 1;
-		spin_lock(&urb->lock);
-
-		/* Data underrun is special. For allowed underrun
-		   we clear the error and continue as normal. For
-		   forbidden underrun we finish the DATA stage
-		   immediately while for control transfer,
-		   we do a STATUS stage. */
-		if (cc == TD_DATAUNDERRUN) {
-			if (!(urb->transfer_flags & URB_SHORT_NOT_OK)) {
-				DBG("Allowed data underrun\n");
-				cc = TD_CC_NOERROR;
-				short_not_ok = 0;
-			} else {
-				ep->error_count = 1;
-				if (usb_pipecontrol(urb->pipe))
-					ep->nextpid = USB_PID_ACK;
-				else
-					usb_settoggle(udev, ep->epnum,
-						      ep->nextpid ==
-						      USB_PID_OUT,
-						      PTD_GET_TOGGLE(ptd));
-				urb->actual_length += PTD_GET_COUNT(ptd);
-				urb->status = cc_to_error[TD_DATAUNDERRUN];
-				spin_unlock(&urb->lock);
-				continue;
-			}
-		}
-		/* Keep underrun error through the STATUS stage */
-		if (urb->status == cc_to_error[TD_DATAUNDERRUN])
-			cc = TD_DATAUNDERRUN;
-
-		if (cc != TD_CC_NOERROR && cc != TD_NOTACCESSED
-		    && (++ep->error_count >= 3 || cc == TD_CC_STALL
-			|| cc == TD_DATAOVERRUN)) {
-			if (urb->status == -EINPROGRESS)
-				urb->status = cc_to_error[cc];
-			if (ep->nextpid == USB_PID_ACK)
-				ep->nextpid = 0;
-			spin_unlock(&urb->lock);
-			continue;
-		}
-		/* According to usb spec, zero-length Int transfer signals
-		   finishing of the urb. Hey, does this apply only
-		   for IN endpoints? */
-		if (usb_pipeint(urb->pipe) && !PTD_GET_LEN(ptd)) {
-			if (urb->status == -EINPROGRESS)
-				urb->status = 0;
-			spin_unlock(&urb->lock);
-			continue;
-		}
-
-		/* Relax after previously failed, but later succeeded
-		   or correctly NAK'ed retransmission attempt */
-		if (ep->error_count
-		    && (cc == TD_CC_NOERROR || cc == TD_NOTACCESSED))
-			ep->error_count = 0;
-
-		/* Take into account idiosyncracies of the isp116x chip
-		   regarding toggle bit for failed transfers */
-		if (ep->nextpid == USB_PID_OUT)
-			usb_settoggle(udev, ep->epnum, 1, PTD_GET_TOGGLE(ptd)
-				      ^ (ep->error_count > 0));
-		else if (ep->nextpid == USB_PID_IN)
-			usb_settoggle(udev, ep->epnum, 0, PTD_GET_TOGGLE(ptd)
-				      ^ (ep->error_count > 0));
-
-		switch (ep->nextpid) {
-		case USB_PID_IN:
-		case USB_PID_OUT:
-			urb->actual_length += PTD_GET_COUNT(ptd);
-			if (PTD_GET_ACTIVE(ptd)
-			    || (cc != TD_CC_NOERROR && cc < 0x0E))
-				break;
-			if (urb->transfer_buffer_length != urb->actual_length) {
-				if (short_not_ok)
-					break;
-			} else {
-				if (urb->transfer_flags & URB_ZERO_PACKET
-				    && ep->nextpid == USB_PID_OUT
-				    && !(PTD_GET_COUNT(ptd) % ep->maxpacket)) {
-					DBG("Zero packet requested\n");
-					break;
-				}
-			}
-			/* All data for this URB is transferred, let's finish */
-			if (usb_pipecontrol(urb->pipe))
-				ep->nextpid = USB_PID_ACK;
-			else if (urb->status == -EINPROGRESS)
-				urb->status = 0;
-			break;
-		case USB_PID_SETUP:
-			if (PTD_GET_ACTIVE(ptd)
-			    || (cc != TD_CC_NOERROR && cc < 0x0E))
-				break;
-			if (urb->transfer_buffer_length == urb->actual_length)
-				ep->nextpid = USB_PID_ACK;
-			else if (usb_pipeout(urb->pipe)) {
-				usb_settoggle(udev, 0, 1, 1);
-				ep->nextpid = USB_PID_OUT;
-			} else {
-				usb_settoggle(udev, 0, 0, 1);
-				ep->nextpid = USB_PID_IN;
-			}
-			break;
-		case USB_PID_ACK:
-			if (PTD_GET_ACTIVE(ptd)
-			    || (cc != TD_CC_NOERROR && cc < 0x0E))
-				break;
-			if (urb->status == -EINPROGRESS)
-				urb->status = 0;
-			ep->nextpid = 0;
-			break;
-		default:
-			BUG();
-		}
-		spin_unlock(&urb->lock);
 	}
 }
 
···
 	if (!--isp116x->periodic_count) {
 		isp116x->irqenb &= ~HCuPINT_SOF;
 		isp116x->irqenb |= HCuPINT_ATL;
+	}
+}
+
+/*
+  Analyze transfer results, handle partial transfers and errors
+*/
+static void postproc_atl_queue(struct isp116x *isp116x)
+{
+	struct isp116x_ep *ep;
+	struct urb *urb;
+	struct usb_device *udev;
+	struct ptd *ptd;
+	int short_not_ok;
+	int status;
+	u8 cc;
+
+	for (ep = isp116x->atl_active; ep; ep = ep->active) {
+		BUG_ON(list_empty(&ep->hep->urb_list));
+		urb =
+		    container_of(ep->hep->urb_list.next, struct urb, urb_list);
+		udev = urb->dev;
+		ptd = &ep->ptd;
+		cc = PTD_GET_CC(ptd);
+		short_not_ok = 1;
+		status = -EINPROGRESS;
+
+		/* Data underrun is special. For allowed underrun
+		   we clear the error and continue as normal. For
+		   forbidden underrun we finish the DATA stage
+		   immediately while for control transfer,
+		   we do a STATUS stage. */
+		if (cc == TD_DATAUNDERRUN) {
+			if (!(urb->transfer_flags & URB_SHORT_NOT_OK) ||
+					usb_pipecontrol(urb->pipe)) {
+				DBG("Allowed or control data underrun\n");
+				cc = TD_CC_NOERROR;
+				short_not_ok = 0;
+			} else {
+				ep->error_count = 1;
+				usb_settoggle(udev, ep->epnum,
+					      ep->nextpid == USB_PID_OUT,
+					      PTD_GET_TOGGLE(ptd));
+				urb->actual_length += PTD_GET_COUNT(ptd);
+				status = cc_to_error[TD_DATAUNDERRUN];
+				goto done;
+			}
+		}
+
+		if (cc != TD_CC_NOERROR && cc != TD_NOTACCESSED
+		    && (++ep->error_count >= 3 || cc == TD_CC_STALL
+			|| cc == TD_DATAOVERRUN)) {
+			status = cc_to_error[cc];
+			if (ep->nextpid == USB_PID_ACK)
+				ep->nextpid = 0;
+			goto done;
+		}
+		/* According to usb spec, zero-length Int transfer signals
+		   finishing of the urb. Hey, does this apply only
+		   for IN endpoints? */
+		if (usb_pipeint(urb->pipe) && !PTD_GET_LEN(ptd)) {
+			status = 0;
+			goto done;
+		}
+
+		/* Relax after previously failed, but later succeeded
+		   or correctly NAK'ed retransmission attempt */
+		if (ep->error_count
+		    && (cc == TD_CC_NOERROR || cc == TD_NOTACCESSED))
+			ep->error_count = 0;
+
+		/* Take into account idiosyncracies of the isp116x chip
+		   regarding toggle bit for failed transfers */
+		if (ep->nextpid == USB_PID_OUT)
+			usb_settoggle(udev, ep->epnum, 1, PTD_GET_TOGGLE(ptd)
+				      ^ (ep->error_count > 0));
+		else if (ep->nextpid == USB_PID_IN)
+			usb_settoggle(udev, ep->epnum, 0, PTD_GET_TOGGLE(ptd)
+				      ^ (ep->error_count > 0));
+
+		switch (ep->nextpid) {
+		case USB_PID_IN:
+		case USB_PID_OUT:
+			urb->actual_length += PTD_GET_COUNT(ptd);
+			if (PTD_GET_ACTIVE(ptd)
+			    || (cc != TD_CC_NOERROR && cc < 0x0E))
+				break;
+			if (urb->transfer_buffer_length != urb->actual_length) {
+				if (short_not_ok)
+					break;
+			} else {
+				if (urb->transfer_flags & URB_ZERO_PACKET
+				    && ep->nextpid == USB_PID_OUT
+				    && !(PTD_GET_COUNT(ptd) % ep->maxpacket)) {
+					DBG("Zero packet requested\n");
+					break;
+				}
+			}
+			/* All data for this URB is transferred, let's finish */
+			if (usb_pipecontrol(urb->pipe))
+				ep->nextpid = USB_PID_ACK;
+			else
+				status = 0;
+			break;
+		case USB_PID_SETUP:
+			if (PTD_GET_ACTIVE(ptd)
+			    || (cc != TD_CC_NOERROR && cc < 0x0E))
+				break;
+			if (urb->transfer_buffer_length == urb->actual_length)
+				ep->nextpid = USB_PID_ACK;
+			else if (usb_pipeout(urb->pipe)) {
+				usb_settoggle(udev, 0, 1, 1);
+				ep->nextpid = USB_PID_OUT;
+			} else {
+				usb_settoggle(udev, 0, 0, 1);
+				ep->nextpid = USB_PID_IN;
+			}
+			break;
+		case USB_PID_ACK:
+			if (PTD_GET_ACTIVE(ptd)
+			    || (cc != TD_CC_NOERROR && cc < 0x0E))
+				break;
+			if ((urb->transfer_flags & URB_SHORT_NOT_OK) &&
+					urb->actual_length <
+						urb->transfer_buffer_length)
+				status = -EREMOTEIO;
+			else
+				status = 0;
+			ep->nextpid = 0;
+			break;
+		default:
+			BUG();
+		}
+
+ done:
+		if (status != -EINPROGRESS) {
+			spin_lock(&urb->lock);
+			if (urb->status == -EINPROGRESS)
+				urb->status = status;
+			spin_unlock(&urb->lock);
+		}
+		if (urb->status != -EINPROGRESS)
+			finish_request(isp116x, ep, urb);
 	}
 }
 
···
 */
 static void finish_atl_transfers(struct isp116x *isp116x)
 {
-	struct isp116x_ep *ep;
-	struct urb *urb;
-
 	if (!isp116x->atl_active)
 		return;
 	/* Fifo not ready? */
···
 	atomic_inc(&isp116x->atl_finishing);
 	unpack_fifo(isp116x);
 	postproc_atl_queue(isp116x);
-	for (ep = isp116x->atl_active; ep; ep = ep->active) {
-		urb =
-		    container_of(ep->hep->urb_list.next, struct urb, urb_list);
-		/* USB_PID_ACK check here avoids finishing of
-		   control transfers, for which TD_DATAUNDERRUN
-		   occured, while URB_SHORT_NOT_OK was set */
-		if (urb && urb->status != -EINPROGRESS
-		    && ep->nextpid != USB_PID_ACK)
-			finish_request(isp116x, ep, urb);
-	}
 	atomic_dec(&isp116x->atl_finishing);
 }
 
···
 	}
 
 	/* in case of unlink-during-submit */
-	spin_lock(&urb->lock);
 	if (urb->status != -EINPROGRESS) {
-		spin_unlock(&urb->lock);
 		finish_request(isp116x, ep, urb);
 		ret = 0;
 		goto fail;
 	}
 	urb->hcpriv = hep;
-	spin_unlock(&urb->lock);
 	start_atl_transfers(isp116x);
 
       fail: