[PATCH] 2.5 uhci control and interrupt queuing

This is 95% Dan's patch, but I made some small changes. The changes I've
made relative to Dan's patch is:

Drop concept of skeleton TD. After Dan's patch, this was reduced to one
entry in the form of skel_term_td. That wasn't even a skeleton TD in the
first place. This simplifies the code a little bit.
Minor formatting tweaks.
Pass on USB bandwidth changes for now.
Pass on Interrupt auto-resubmit changes for now.
Use complete_list_lock, not complete_list as the lock.
Use frame_list_lock, not fame_list_lock.
Reorganize skeleton QH's to match the order in the schedule. This cleaned
up some debugging code and made the list more logical.
Update some obsolete documentation.
Pass on code to check for race conditions and fix up. It was racy as well.

Dan, do you compile with SMP? Those two locking typos would have
generated a warning and/or error.

I've done some light testing and haven't found any problems yet. I
haven't tested the control or interrupt queuing for lack of anything to
test with yet.

Anyway, please apply.

   Merge in Dan's patch to add interrupt and control queuing support. Summary of changes:
   Add queuing support for Interrupt and Control message in addition to Bulk.
   This resulted in some merging of code.
   Fix a queuing bug when moving a child into the parent position.
   Update documentation.
   Update debugging code.

drivers/usb/host/uhci-debug.c

@@ -34,17 +34,6 @@ static void inline lprintk(char *buf)
 	}
 }
 
-static int inline uhci_is_skeleton_td(struct uhci_hcd *uhci, struct uhci_td *td)
-{
-	int i;
-
-	for (i = 0; i < UHCI_NUM_SKELTD; i++)
-		if (td == uhci->skeltd[i])
-			return 1;
-
-	return 0;
-}
-
 static int inline uhci_is_skeleton_qh(struct uhci_hcd *uhci, struct uhci_qh *qh)
 {
 	int i;
@@ -285,13 +274,14 @@ static int uhci_show_qh(struct uhci_qh *qh, char *buf, int len, int space)
 	return out - buf;
 }
 
-static const char *td_names[] = {"skel_int1_td", "skel_int2_td",
-				 "skel_int4_td", "skel_int8_td",
-				 "skel_int16_td", "skel_int32_td",
-				 "skel_int64_td", "skel_int128_td",
-				 "skel_int256_td", "skel_term_td" };
-static const char *qh_names[] = { "skel_ls_control_qh", "skel_hs_control_qh",
-				  "skel_bulk_qh", "skel_term_qh" };
+static const char *qh_names[] = {
+  "skel_int128_qh", "skel_int64_qh",
+  "skel_int32_qh", "skel_int16_qh",
+  "skel_int8_qh", "skel_int4_qh",
+  "skel_int2_qh", "skel_int1_qh",
+  "skel_ls_control_qh", "skel_hs_control_qh",
+  "skel_bulk_qh", "skel_term_qh"
+};
 
 #define show_frame_num()	\
 	if (!shown) {		\
@@ -299,26 +289,141 @@ static const char *qh_names[] = { "skel_ls_control_qh", "skel_hs_control_qh",
 	  out += sprintf(out, "- Frame %d\n", i); \
 	}
 
-#define show_td_name()		\
-	if (!shown) {		\
-	  shown = 1;		\
-	  out += sprintf(out, "- %s\n", td_names[i]); \
-	}
-
 #define show_qh_name()		\
 	if (!shown) {		\
 	  shown = 1;		\
 	  out += sprintf(out, "- %s\n", qh_names[i]); \
 	}
 
+static int uhci_show_urbp(struct uhci_hcd *uhci, struct urb_priv *urbp, char *buf, int len)
+{
+	struct list_head *tmp;
+	char *out = buf;
+	int count = 0;
+
+	if (len < 200)
+		return 0;
+
+	out += sprintf(out, "urb_priv [%p] ", urbp);
+	out += sprintf(out, "urb [%p] ", urbp->urb);
+	out += sprintf(out, "qh [%p] ", urbp->qh);
+	out += sprintf(out, "Dev=%d ", usb_pipedevice(urbp->urb->pipe));
+	out += sprintf(out, "EP=%x(%s) ", usb_pipeendpoint(urbp->urb->pipe), (usb_pipein(urbp->urb->pipe) ? "IN" : "OUT"));
+
+	switch (usb_pipetype(urbp->urb->pipe)) {
+	case PIPE_ISOCHRONOUS: out += sprintf(out, "ISO "); break;
+	case PIPE_INTERRUPT: out += sprintf(out, "INT "); break;
+	case PIPE_BULK: out += sprintf(out, "BLK "); break;
+	case PIPE_CONTROL: out += sprintf(out, "CTL "); break;
+	}
+
+	out += sprintf(out, "%s", (urbp->fsbr ? "FSBR " : ""));
+	out += sprintf(out, "%s", (urbp->fsbr_timeout ? "FSBR_TO " : ""));
+
+	if (urbp->status != -EINPROGRESS)
+		out += sprintf(out, "Status=%d ", urbp->status);
+	//out += sprintf(out, "Inserttime=%lx ",urbp->inserttime);
+	//out += sprintf(out, "FSBRtime=%lx ",urbp->fsbrtime);
+
+	spin_lock(&urbp->urb->lock);
+	count = 0;
+	list_for_each(tmp, &urbp->td_list)
+		count++;
+	spin_unlock(&urbp->urb->lock);
+	out += sprintf(out, "TDs=%d ",count);
+
+	if (urbp->queued)
+		out += sprintf(out, "queued\n");
+	else {
+		spin_lock(&uhci->frame_list_lock);
+		count = 0;
+		list_for_each(tmp, &urbp->queue_list)
+			count++;
+		spin_unlock(&uhci->frame_list_lock);
+		out += sprintf(out, "queued URBs=%d\n", count);
+	}
+
+	return out - buf;
+}
+
+static int uhci_show_lists(struct uhci_hcd *uhci, char *buf, int len)
+{
+	char *out = buf;
+	unsigned long flags;
+	struct list_head *head, *tmp;
+	int count;
+
+	out += sprintf(out, "Main list URBs:");
+	spin_lock_irqsave(&uhci->urb_list_lock, flags);
+	if (list_empty(&uhci->urb_list))
+		out += sprintf(out, " Empty\n");
+	else {
+		out += sprintf(out, "\n");
+		count = 0;
+		head = &uhci->urb_list;
+		tmp = head->next;
+		while (tmp != head) {
+			struct urb_priv *urbp = list_entry(tmp, struct urb_priv, urb_list);
+
+			out += sprintf(out, "  %d: ", ++count);
+			out += uhci_show_urbp(uhci, urbp, out, len - (out - buf));
+			tmp = tmp->next;
+		}
+	}
+	spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
+
+	out += sprintf(out, "Remove list URBs:");
+	spin_lock_irqsave(&uhci->urb_remove_list_lock, flags);
+	if (list_empty(&uhci->urb_remove_list))
+		out += sprintf(out, " Empty\n");
+	else {
+		out += sprintf(out, "\n");
+		count = 0;
+		head = &uhci->urb_remove_list;
+		tmp = head->next;
+		while (tmp != head) {
+			struct urb_priv *urbp = list_entry(tmp, struct urb_priv, urb_list);
+
+			out += sprintf(out, "  %d: ", ++count);
+			out += uhci_show_urbp(uhci, urbp, out, len - (out - buf));
+			tmp = tmp->next;
+		}
+	}
+	spin_unlock_irqrestore(&uhci->urb_remove_list_lock, flags);
+
+	out += sprintf(out, "Complete list URBs:");
+	spin_lock_irqsave(&uhci->complete_list_lock, flags);
+	if (list_empty(&uhci->complete_list))
+		out += sprintf(out, " Empty\n");
+	else {
+		out += sprintf(out, "\n");
+		count = 0;
+		head = &uhci->complete_list;
+		tmp = head->next;
+		while (tmp != head) {
+			struct urb_priv *urbp = list_entry(tmp, struct urb_priv, complete_list);
+
+			out += sprintf(out, "  %d: ", ++count);
+			out += uhci_show_urbp(uhci, urbp, out, len - (out - buf));
+			tmp = tmp->next;
+		}
+	}
+	spin_unlock_irqrestore(&uhci->complete_list_lock, flags);
+
+	return out - buf;
+}
+
 static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len)
 {
+	unsigned long flags;
 	char *out = buf;
 	int i;
 	struct uhci_qh *qh;
 	struct uhci_td *td;
 	struct list_head *tmp, *head;
 
+	spin_lock_irqsave(&uhci->frame_list_lock, flags);
+
 	out += sprintf(out, "HC status\n");
 	out += uhci_show_status(uhci, out, len - (out - buf));
 
@@ -333,8 +438,6 @@ static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len)
 			show_frame_num();
 			out += sprintf(out, "    frame list does not match td->dma_handle!\n");
 		}
-		if (uhci_is_skeleton_td(uhci, td))
-			continue;
 		show_frame_num();
 
 		head = &td->fl_list;
@@ -346,67 +449,6 @@ static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len)
 		} while (tmp != head);
 	}
 
-	out += sprintf(out, "Skeleton TD's\n");
-	for (i = UHCI_NUM_SKELTD - 1; i >= 0; i--) {
-		int shown = 0;
-
-		td = uhci->skeltd[i];
-
-		if (debug > 1) {
-			show_td_name();
-			out += uhci_show_td(td, out, len - (out - buf), 4);
-		}
-
-		if (list_empty(&td->fl_list)) {
-			/* TD 0 is the int1 TD and links to control_ls_qh */
-			if (!i) {
-				if (td->link !=
-				    (cpu_to_le32(uhci->skel_ls_control_qh->dma_handle) | UHCI_PTR_QH)) {
-					show_td_name();
-					out += sprintf(out, "    skeleton TD not linked to ls_control QH!\n");
-				}
-			} else if (i < 9) {
-				if (td->link != cpu_to_le32(uhci->skeltd[i - 1]->dma_handle)) {
-					show_td_name();
-					out += sprintf(out, "    skeleton TD not linked to next skeleton TD!\n");
-				}
-			} else {
-				show_td_name();
-
-				if (td->link != cpu_to_le32(td->dma_handle))
-					out += sprintf(out, "    skel_term_td does not link to self\n");
-
-				/* Don't show it twice */
-				if (debug <= 1)
-					out += uhci_show_td(td, out, len - (out - buf), 4);
-			}
-
-			continue;
-		}
-
-		show_td_name();
-
-		head = &td->fl_list;
-		tmp = head->next;
-
-		while (tmp != head) {
-			td = list_entry(tmp, struct uhci_td, fl_list);
-
-			tmp = tmp->next;
-
-			out += uhci_show_td(td, out, len - (out - buf), 4);
-		}
-
-		if (!i) {
-			if (td->link !=
-			    (cpu_to_le32(uhci->skel_ls_control_qh->dma_handle) | UHCI_PTR_QH))
-				out += sprintf(out, "    last TD not linked to ls_control QH!\n");
-		} else if (i < 9) {
-			if (td->link != cpu_to_le32(uhci->skeltd[i - 1]->dma_handle))
-				out += sprintf(out, "    last TD not linked to next skeleton!\n");
-		}
-	}
-
 	out += sprintf(out, "Skeleton QH's\n");
 
 	for (i = 0; i < UHCI_NUM_SKELQH; ++i) {
@@ -419,21 +461,19 @@ static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len)
 			out += uhci_show_qh(qh, out, len - (out - buf), 4);
 		}
 
-		/* QH 3 is the Terminating QH, it's different */
-		if (i == 3) {
-			if (qh->link != UHCI_PTR_TERM) {
-				show_qh_name();
+		/* Last QH is the Terminating QH, it's different */
+		if (i == UHCI_NUM_SKELQH - 1) {
+			if (qh->link != UHCI_PTR_TERM)
 				out += sprintf(out, "    bandwidth reclamation on!\n");
-			}
 
-			if (qh->element != cpu_to_le32(uhci->skel_term_td->dma_handle)) {
-				show_qh_name();
-				out += sprintf(out, "    skel_term_qh element is not set to skel_term_td\n");
-			}
+			if (qh->element != cpu_to_le32(uhci->term_td->dma_handle))
+				out += sprintf(out, "    skel_term_qh element is not set to term_td!\n");
+
+			continue;
 		}
 
 		if (list_empty(&qh->list)) {
-			if (i < 3) {
+			if (i < UHCI_NUM_SKELQH - 1) {
 				if (qh->link !=
 				    (cpu_to_le32(uhci->skelqh[i + 1]->dma_handle) | UHCI_PTR_QH)) {
 					show_qh_name();
@@ -457,18 +497,23 @@ static int uhci_sprint_schedule(struct uhci_hcd *uhci, char *buf, int len)
 			out += uhci_show_qh(qh, out, len - (out - buf), 4);
 		}
 
-		if (i < 3) {
+		if (i < UHCI_NUM_SKELQH - 1) {
 			if (qh->link !=
 			    (cpu_to_le32(uhci->skelqh[i + 1]->dma_handle) | UHCI_PTR_QH))
 				out += sprintf(out, "    last QH not linked to next skeleton!\n");
 		}
 	}
 
+	spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
+
+	if (debug > 2)
+		out += uhci_show_lists(uhci, out, len - (out - buf));
+
 	return out - buf;
 }
 
 #ifdef CONFIG_PROC_FS
-#define MAX_OUTPUT	(PAGE_SIZE * 8)
+#define MAX_OUTPUT	(PAGE_SIZE * 16)
 
 static struct proc_dir_entry *uhci_proc_root = NULL;
 
@@ -483,7 +528,6 @@ static int uhci_proc_open(struct inode *inode, struct file *file)
 	const struct proc_dir_entry *dp = PDE(inode);
 	struct uhci_hcd *uhci = dp->data;
 	struct uhci_proc *up;
-	unsigned long flags;
 	int ret = -ENOMEM;
 
 	lock_kernel();
@@ -497,9 +541,7 @@ static int uhci_proc_open(struct inode *inode, struct file *file)
 		goto out;
 	}
 
-	spin_lock_irqsave(&uhci->frame_list_lock, flags);
 	up->size = uhci_sprint_schedule(uhci, up->data, MAX_OUTPUT);
-	spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
 
 	file->private_data = up;
 

drivers/usb/host/uhci-hcd.h

@@ -81,7 +81,8 @@ struct uhci_frame_list {
 
 struct urb_priv;
 
-/* One role of a QH is to hold a queue of TDs for some endpoint.  Each QH is
+/*
+ * One role of a QH is to hold a queue of TDs for some endpoint.  Each QH is
  * used with one URB, and qh->element (updated by the HC) is either:
  *   - the next unprocessed TD for the URB, or
  *   - UHCI_PTR_TERM (when there's no more traffic for this endpoint), or
@@ -194,85 +195,63 @@ struct uhci_td {
 } __attribute__((aligned(16)));
 
 /*
- * There are various standard queues. We set up several different
- * queues for each of the three basic queue types: interrupt,
- * control, and bulk.
- *
- *  - There are various different interrupt latencies: ranging from
- *    every other USB frame (2 ms apart) to every 256 USB frames (ie
- *    256 ms apart). Make your choice according to how obnoxious you
- *    want to be on the wire, vs how critical latency is for you.
- *  - The control list is done every frame.
- *  - There are 4 bulk lists, so that up to four devices can have a
- *    bulk list of their own and when run concurrently all four lists
- *    will be be serviced.
- *
- * This is a bit misleading, there are various interrupt latencies, but they
- * vary a bit, interrupt2 isn't exactly 2ms, it can vary up to 4ms since the
- * other queues can "override" it. interrupt4 can vary up to 8ms, etc. Minor
- * problem
- *
- * In the case of the root hub, these QH's are just head's of qh's. Don't
- * be scared, it kinda makes sense. Look at this wonderful picture care of
- * Linus:
+ * The UHCI driver places Interrupt, Control and Bulk into QH's both
+ * to group together TD's for one transfer, and also to faciliate queuing
+ * of URB's. To make it easy to insert entries into the schedule, we have
+ * a skeleton of QH's for each predefined Interrupt latency, low speed
+ * control, high speed control and terminating QH (see explanation for
+ * the terminating QH below).
  *
- *  generic-  ->  dev1-  ->  generic-  ->  dev1-  ->  control-  ->  bulk- -> ...
- *   iso-QH      iso-QH       irq-QH      irq-QH        QH           QH
- *      |           |            |           |           |            |
- *     End     dev1-iso-TD1     End     dev1-irq-TD1    ...          ... 
- *                  |
- *             dev1-iso-TD2
- *                  |
- *                ....
+ * When we want to add a new QH, we add it to the end of the list for the
+ * skeleton QH.
  *
- * This may vary a bit (the UHCI docs don't explicitly say you can put iso
- * transfers in QH's and all of their pictures don't have that either) but
- * other than that, that is what we're doing now
+ * For instance, the queue can look like this:
  *
- * And now we don't put Iso transfers in QH's, so we don't waste one on it
- * --jerdfelt
+ * skel int1 QH
+ * dev 1 interrupt QH
+ * dev 5 interrupt QH
+ * skel int2 QH
+ * skel int4 QH
+ * ...
+ * skel int128 QH
+ * skel low speed control QH
+ * dev 5 control QH
+ * skel high speed control QH
+ * skel bulk QH
+ * dev 1 bulk QH
+ * dev 2 bulk QH
+ * skel terminating QH
  *
- * To keep with Linus' nomenclature, this is called the QH skeleton. These
- * labels (below) are only signficant to the root hub's QH's
+ * The terminating QH is used for 2 reasons:
+ * - To place a terminating TD which is used to workaround a PIIX bug
+ *   (see Intel errata for explanation)
+ * - To loop back to the high speed control queue for full speed bandwidth
+ *   reclamation
  *
- *
- * NOTE:  That ASCII art doesn't match the current (August 2002) code, in
- * more ways than just not using QHs for ISO.
- *
- * NOTE:  Another way to look at the UHCI schedules is to compare them to what
- * other host controller interfaces use.  EHCI, OHCI, and UHCI all have tables
- * of transfers that the controller scans, frame by frame, and which hold the
- * scheduled periodic transfers.  The key differences are that UHCI
- *
- *   (a) puts control and bulk transfers into that same table; the others
- *       have separate data structures for non-periodic transfers.
- *   (b) lets QHs be linked from TDs, not just other QHs, since they don't
- *       hold endpoint data.  this driver chooses to use one QH per URB.
- *   (c) needs more TDs, since it uses one per packet.  the data toggle
- *       is stored in those TDs, along with all other endpoint state.
+ * Isochronous transfers are stored before the start of the skeleton
+ * schedule and don't use QH's. While the UHCI spec doesn't forbid the
+ * use of QH's for Isochronous, it doesn't use them either. Since we don't
+ * need to use them either, we follow the spec diagrams in hope that it'll
+ * be more compatible with future UHCI implementations.
  */
 
-#define UHCI_NUM_SKELTD		10
-#define skel_int1_td		skeltd[0]
-#define skel_int2_td		skeltd[1]
-#define skel_int4_td		skeltd[2]
-#define skel_int8_td		skeltd[3]
-#define skel_int16_td		skeltd[4]
-#define skel_int32_td		skeltd[5]
-#define skel_int64_td		skeltd[6]
-#define skel_int128_td		skeltd[7]
-#define skel_int256_td		skeltd[8]
-#define skel_term_td		skeltd[9]	/* To work around PIIX UHCI bug */
-
-#define UHCI_NUM_SKELQH		4
-#define skel_ls_control_qh	skelqh[0]
-#define skel_hs_control_qh	skelqh[1]
-#define skel_bulk_qh		skelqh[2]
-#define skel_term_qh		skelqh[3]
+#define UHCI_NUM_SKELQH		12
+#define skel_int128_qh		skelqh[0]
+#define skel_int64_qh		skelqh[1]
+#define skel_int32_qh		skelqh[2]
+#define skel_int16_qh		skelqh[3]
+#define skel_int8_qh		skelqh[4]
+#define skel_int4_qh		skelqh[5]
+#define skel_int2_qh		skelqh[6]
+#define skel_int1_qh		skelqh[7]
+#define skel_ls_control_qh	skelqh[8]
+#define skel_hs_control_qh	skelqh[9]
+#define skel_bulk_qh		skelqh[10]
+#define skel_term_qh		skelqh[11]
 
 /*
- * Search tree for determining where <interval> fits in the
- * skelqh[] skeleton.
+ * Search tree for determining where <interval> fits in the skelqh[]
+ * skeleton.
  *
  * An interrupt request should be placed into the slowest skelqh[]
  * which meets the interval/period/frequency requirement.
@@ -280,32 +259,27 @@ struct uhci_td {
  *
  * For a given <interval>, this function returns the appropriate/matching
  * skelqh[] index value.
- *
- * NOTE: For UHCI, we don't really need int256_qh since the maximum interval
- * is 255 ms.  However, we do need an int1_qh since 1 is a valid interval
- * and we should meet that frequency when requested to do so.
- * This will require some change(s) to the UHCI skeleton.
  */
 static inline int __interval_to_skel(int interval)
 {
 	if (interval < 16) {
 		if (interval < 4) {
 			if (interval < 2)
-				return 0;	/* int1 for 0-1 ms */
-			return 1;		/* int2 for 2-3 ms */
+				return 7;	/* int1 for 0-1 ms */
+			return 6;		/* int2 for 2-3 ms */
 		}
 		if (interval < 8)
-			return 2;		/* int4 for 4-7 ms */
-		return 3;			/* int8 for 8-15 ms */
+			return 5;		/* int4 for 4-7 ms */
+		return 4;			/* int8 for 8-15 ms */
 	}
 	if (interval < 64) {
 		if (interval < 32)
-			return 4;		/* int16 for 16-31 ms */
-		return 5;			/* int32 for 32-63 ms */
+			return 3;		/* int16 for 16-31 ms */
+		return 2;			/* int32 for 32-63 ms */
 	}
 	if (interval < 128)
-		return 6;			/* int64 for 64-127 ms */
-	return 7;				/* int128 for 128-255 ms (Max.) */
+		return 1;			/* int64 for 64-127 ms */
+	return 0;				/* int128 for 128-255 ms (Max.) */
 }
 
 #define hcd_to_uhci(hcd_ptr) container_of(hcd_ptr, struct uhci_hcd, hcd)
@@ -332,7 +306,7 @@ struct uhci_hcd {
 
 	struct usb_bus *bus;
 
-	struct uhci_td *skeltd[UHCI_NUM_SKELTD];	/* Skeleton TD's */
+	struct uhci_td *term_td;	/* Terminating TD, see UHCI bug */
 	struct uhci_qh *skelqh[UHCI_NUM_SKELQH];	/* Skeleton QH's */
 
 	spinlock_t frame_list_lock;