[PATCH] ehci misc fixes

This removes some bugs:

 - a short read problem with control requests
 - only creates one control qh (memleak fix)
 - adds an omitted hardware handshake
 - reset timeout in octal, say what?
 - a couple BUG()s outlived their value

Plus it deletes unused stub code for split ISO
and updates some internal doc.

drivers/usb/host/ehci-hcd.c

@@ -190,7 +190,7 @@ static int ehci_reset (struct ehci_hcd *ehci)
 	dbg_cmd (ehci, "reset", command);
 	writel (command, &ehci->regs->command);
 	ehci->hcd.state = USB_STATE_HALT;
-	return handshake (&ehci->regs->command, CMD_RESET, 0, 050);
+	return handshake (&ehci->regs->command, CMD_RESET, 0, 250);
 }
 
 /* idle the controller (from running) */
@@ -368,6 +368,7 @@ static int ehci_start (struct usb_hcd *hcd)
 	 *
 	 * NOTE:  layered drivers can't yet tell when we enable that,
 	 * so they can't pass this info along (like NETIF_F_HIGHDMA)
+	 * (or like Scsi_Host.highmem_io) ... usb_bus.flags?
 	 */
 	if (HCC_64BIT_ADDR (hcc_params)) {
 		writel (0, &ehci->regs->segment);
@@ -586,6 +587,10 @@ static void ehci_tasklet (unsigned long param)
 	struct ehci_hcd		*ehci = (struct ehci_hcd *) param;
 	unsigned long		flags;
 
+	// FIXME don't pass flags; on sparc they aren't really flags.
+	// qh_completions can just leave irqs blocked,
+	// then have scan_async() allow IRQs if it's very busy 
+
 	spin_lock_irqsave (&ehci->lock, flags);
 
 	if (ehci->reclaim_ready)

drivers/usb/host/ehci-q.c

@@ -26,8 +26,7 @@
  * Control, bulk, and interrupt traffic all use "qh" lists.  They list "qtd"
  * entries describing USB transactions, max 16-20kB/entry (with 4kB-aligned
  * buffers needed for the larger number).  We use one QH per endpoint, queue
- * multiple (bulk or control) urbs per endpoint.  URBs may need several qtds.
- * A scheduled interrupt qh always (for now) has one qtd, one urb.
+ * multiple urbs (all three types) per endpoint.  URBs may need several qtds.
  *
  * ISO traffic uses "ISO TD" (itd, and sitd) records, and (along with
  * interrupts) needs careful scheduling.  Performance improvements can be
@@ -281,7 +280,12 @@ qh_completions (struct ehci_hcd *ehci, struct ehci_qh *qh, unsigned long flags)
 			|| (qh->qh_state == QH_STATE_IDLE);
 
 		// FIXME Remove the automagic unlink mode.
-		// Drivers can now clean up safely; its' their job.
+		// Drivers can now clean up safely; it's their job.
+		//
+		// FIXME Removing it should fix the short read scenarios
+		// with "huge" urb data (more than one 16+KByte td) with
+		// the short read someplace other than the last data TD.
+		// Except the control case: 'retrigger' status ACKs.
 
 		/* fault: unlink the rest, since this qtd saw an error? */
 		if (unlikely ((token & QTD_STS_HALT) != 0)) {
@@ -391,7 +395,7 @@ qh_urb_transaction (
 	struct ehci_qtd		*qtd, *qtd_prev;
 	dma_addr_t		buf;
 	int			len, maxpacket;
-	int			is_input;
+	int			is_input, status_patch = 0;
 	u32			token;
 
 	/*
@@ -422,6 +426,9 @@ qh_urb_transaction (
 		qtd->urb = urb;
 		qtd_prev->hw_next = QTD_NEXT (qtd->qtd_dma);
 		list_add_tail (&qtd->qtd_list, head);
+
+		if (!(urb->transfer_flags & URB_SHORT_NOT_OK))
+			status_patch = 1;
 	} 
 
 	/*
@@ -499,6 +506,19 @@ qh_urb_transaction (
 		}
 	}
 
+	/* if we're permitting a short control read, we want the hardware to
+	 * just continue after short data and send the status ack.  it can do
+	 * that on the last data packet (typically the only one).  for other
+	 * packets, software fixup is needed (in qh_completions).
+	 */
+	if (status_patch) {
+		struct ehci_qtd		*prev;
+
+		prev = list_entry (qtd->qtd_list.prev,
+				struct ehci_qtd, qtd_list);
+		prev->hw_alt_next = QTD_NEXT (qtd->qtd_dma);
+	}
+
 	/* by default, enable interrupt on urb completion */
 	if (likely (!(urb->transfer_flags & URB_NO_INTERRUPT)))
 		qtd->hw_token |= __constant_cpu_to_le32 (QTD_IOC);
@@ -653,9 +673,8 @@ ehci_qh_make (
 		}
 		break;
 	default:
-#ifdef DEBUG
-		BUG ();
-#endif
+ 		dbg ("bogus dev %p speed %d", urb->dev, urb->dev->speed);
+ 		return 0;
 	}
 
 	/* NOTE:  if (PIPE_INTERRUPT) { scheduler sets s-mask } */
@@ -837,7 +856,7 @@ submit_async (
 	qtd = list_entry (qtd_list->next, struct ehci_qtd, qtd_list);
 	dev = (struct hcd_dev *)urb->dev->hcpriv;
 	epnum = usb_pipeendpoint (urb->pipe);
-	if (usb_pipein (urb->pipe))
+	if (usb_pipein (urb->pipe) && !usb_pipecontrol (urb->pipe))
 		epnum |= 0x10;
 
 	vdbg ("%s: submit_async urb %p len %d ep %d-%s qtd %p [qh %p]",
@@ -923,9 +942,11 @@ static void start_unlink_async (struct ehci_hcd *ehci, struct ehci_qh *qh)
 	if (unlikely (qh == ehci->async && qh->qh_next.qh == qh)) {
 		/* can't get here without STS_ASS set */
 		if (ehci->hcd.state != USB_STATE_HALT) {
-			if (cmd & CMD_PSE)
+			if (cmd & CMD_PSE) {
 				writel (cmd & ~CMD_ASE, &ehci->regs->command);
-			else
+				(void) handshake (&ehci->regs->status,
+						  STS_ASS, 0, 150);
+			} else
 				ehci_ready (ehci);
 		}
 		qh->qh_next.qh = ehci->async = 0;
@@ -944,10 +965,6 @@ static void start_unlink_async (struct ehci_hcd *ehci, struct ehci_qh *qh)
 	prev = ehci->async;
 	while (prev->qh_next.qh != qh && prev->qh_next.qh != ehci->async)
 		prev = prev->qh_next.qh;
-#ifdef DEBUG
-	if (prev->qh_next.qh != qh)
-		BUG ();
-#endif
 
 	if (qh->hw_info1 & __constant_cpu_to_le32 (QH_HEAD)) {
 		ehci->async = prev;

drivers/usb/host/ehci-sched.c

@@ -971,119 +971,10 @@ static int itd_submit (struct ehci_hcd *ehci, struct urb *urb, int mem_flags)
 /*
  * "Split ISO TDs" ... used for USB 1.1 devices going through
  * the TTs in USB 2.0 hubs.
+ *
+ * FIXME not yet implemented
  */
 
-static void
-sitd_free (struct ehci_hcd *ehci, struct ehci_sitd *sitd)
-{
-	pci_pool_free (ehci->sitd_pool, sitd, sitd->sitd_dma);
-}
-
-static struct ehci_sitd *
-sitd_make (
-	struct ehci_hcd	*ehci,
-	struct urb	*urb,
-	unsigned	index,		// urb->iso_frame_desc [index]
-	unsigned	uframe,		// scheduled start
-	dma_addr_t	dma,		// mapped transfer buffer
-	int		mem_flags
-) {
-	struct ehci_sitd	*sitd;
-	unsigned		length;
-
-	sitd = pci_pool_alloc (ehci->sitd_pool, mem_flags, &dma);
-	if (!sitd)
-		return sitd;
-	sitd->urb = urb;
-	length = urb->iso_frame_desc [index].length;
-	dma += urb->iso_frame_desc [index].offset;
-
-#if 0
-	// FIXME:  do the rest!
-#else
-	sitd_free (ehci, sitd);
-	return 0;
-#endif
-
-}
-
-static void
-sitd_link (struct ehci_hcd *ehci, unsigned frame, struct ehci_sitd *sitd)
-{
-	u32		ptr;
-
-	ptr = cpu_to_le32 (sitd->sitd_dma | 2);	// type 2 == sitd
-	if (ehci->pshadow [frame].ptr) {
-		if (!sitd->sitd_next.ptr) {
-			sitd->sitd_next = ehci->pshadow [frame];
-			sitd->hw_next = ehci->periodic [frame];
-		} else if (sitd->sitd_next.ptr != ehci->pshadow [frame].ptr) {
-			dbg ("frame %d sitd link goof", frame);
-			BUG ();
-		}
-	}
-	ehci->pshadow [frame].sitd = sitd;
-	ehci->periodic [frame] = ptr;
-}
-
-static unsigned long
-sitd_complete (
-	struct ehci_hcd *ehci,
-	struct ehci_sitd	*sitd,
-	unsigned long		flags
-) {
-	// FIXME -- implement!
-
-	dbg ("NYI -- sitd_complete");
-	return flags;
-}
-
-/*-------------------------------------------------------------------------*/
-
-static int sitd_submit (struct ehci_hcd *ehci, struct urb *urb, int mem_flags)
-{
-	// struct ehci_sitd	*first_sitd = 0;
-	unsigned		frame_index;
-	dma_addr_t		dma;
-
-	dbg ("NYI -- sitd_submit");
-
-	// FIXME -- implement!
-
-	// FIXME:  setup one big dma mapping
-	dma = 0;
-
-	for (frame_index = 0;
-			frame_index < urb->number_of_packets;
-			frame_index++) {
-		struct ehci_sitd	*sitd;
-		unsigned		uframe;
-
-		// FIXME:  use real arguments, schedule this!
-		uframe = -1;
-
-		sitd = sitd_make (ehci, urb, frame_index,
-				uframe, dma, mem_flags);
-
-		if (sitd) {
-    /*
-			if (first_sitd)
-				list_add_tail (&sitd->sitd_list,
-						&first_sitd->sitd_list);
-			else
-				first_sitd = sitd;
-    */
-		} else {
-			// FIXME:  clean everything up
-		}
-	}
-
-	// if we have a first sitd, then
-		// store them all into the periodic schedule!
-		// urb->hcpriv = first sitd in sitd_list
-
-	return -ENOSYS;
-}
 #endif /* have_split_iso */
 
 /*-------------------------------------------------------------------------*/