ChangeSet 1.1673.8.21, 2004/03/25 17:18:01-08:00, stern@rowland.harvard.edu

[PATCH] USB: UHCI: Get rid of excessive spinlocks

This patch introduces a major simplification into the UHCI driver by
replacing its multiple spinlocks with a single one.  The protected area of
code is slightly larger and there's more possibilities for contention on
an SMP system, but I think that shouldn't be a problem.  Stephen Hemminger
has been kind enough to test this on his SMP computer and he hasn't
encountered any difficulties.


 drivers/usb/host/uhci-debug.c |   20 +----
 drivers/usb/host/uhci-hcd.c   |  152 ++++++------------------------------------
 drivers/usb/host/uhci-hcd.h   |   42 +++--------
 3 files changed, 42 insertions(+), 172 deletions(-)


diff -Nru a/drivers/usb/host/uhci-debug.c b/drivers/usb/host/uhci-debug.c
--- a/drivers/usb/host/uhci-debug.c	Wed Apr 14 14:38:36 2004
+++ b/drivers/usb/host/uhci-debug.c	Wed Apr 14 14:38:36 2004
@@ -27,7 +27,7 @@
 		p = strchr(buf, '\n');
 		if (p)
 			*p = 0;
-		printk("%s\n", buf);
+		printk(KERN_DEBUG "%s\n", buf);
 		buf = p;
 		if (buf)
 			buf++;
@@ -328,21 +328,17 @@
 	//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);
 	}
 
@@ -352,12 +348,10 @@
 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 {
@@ -373,10 +367,8 @@
 			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 {
@@ -392,10 +384,8 @@
 			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 {
@@ -411,7 +401,6 @@
 			tmp = tmp->next;
 		}
 	}
-	spin_unlock_irqrestore(&uhci->complete_list_lock, flags);
 
 	return out - buf;
 }
@@ -425,7 +414,7 @@
 	struct uhci_td *td;
 	struct list_head *tmp, *head;
 
-	spin_lock_irqsave(&uhci->frame_list_lock, flags);
+	spin_lock_irqsave(&uhci->schedule_lock, flags);
 
 	out += sprintf(out, "HC status\n");
 	out += uhci_show_status(uhci, out, len - (out - buf));
@@ -508,11 +497,11 @@
 		}
 	}
 
-	spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
-
 	if (debug > 2)
 		out += uhci_show_lists(uhci, out, len - (out - buf));
 
+	spin_unlock_irqrestore(&uhci->schedule_lock, flags);
+
 	return out - buf;
 }
 
@@ -623,4 +612,3 @@
 	.release =	uhci_proc_release,
 };
 #endif
-
diff -Nru a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
--- a/drivers/usb/host/uhci-hcd.c	Wed Apr 14 14:38:36 2004
+++ b/drivers/usb/host/uhci-hcd.c	Wed Apr 14 14:38:36 2004
@@ -117,26 +117,18 @@
  */
 static inline void uhci_set_next_interrupt(struct uhci_hcd *uhci)
 {
-	unsigned long flags;
-
-	spin_lock_irqsave(&uhci->frame_list_lock, flags);
 	uhci->term_td->status |= cpu_to_le32(TD_CTRL_IOC); 
-	spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
 }
 
 static inline void uhci_clear_next_interrupt(struct uhci_hcd *uhci)
 {
-	spin_lock(&uhci->frame_list_lock);
 	uhci->term_td->status &= ~cpu_to_le32(TD_CTRL_IOC);
-	spin_unlock(&uhci->frame_list_lock);
 }
 
 static inline void uhci_moveto_complete(struct uhci_hcd *uhci, 
 					struct urb_priv *urbp)
 {
-	spin_lock(&uhci->complete_list_lock);
 	list_move_tail(&urbp->urb_list, &uhci->complete_list);
-	spin_unlock(&uhci->complete_list_lock);
 }
 
 static struct uhci_td *uhci_alloc_td(struct uhci_hcd *uhci, struct usb_device *dev)
@@ -178,12 +170,8 @@
  */
 static void uhci_insert_td_frame_list(struct uhci_hcd *uhci, struct uhci_td *td, unsigned framenum)
 {
-	unsigned long flags;
-
 	framenum %= UHCI_NUMFRAMES;
 
-	spin_lock_irqsave(&uhci->frame_list_lock, flags);
-
 	td->frame = framenum;
 
 	/* Is there a TD already mapped there? */
@@ -204,18 +192,13 @@
 		uhci->fl->frame[framenum] = cpu_to_le32(td->dma_handle);
 		uhci->fl->frame_cpu[framenum] = td;
 	}
-
-	spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
 }
 
 static void uhci_remove_td(struct uhci_hcd *uhci, struct uhci_td *td)
 {
-	unsigned long flags;
-
 	/* If it's not inserted, don't remove it */
-	spin_lock_irqsave(&uhci->frame_list_lock, flags);
 	if (td->frame == -1 && list_empty(&td->fl_list))
-		goto out;
+		return;
 
 	if (td->frame != -1 && uhci->fl->frame_cpu[td->frame] == td) {
 		if (list_empty(&td->fl_list)) {
@@ -240,9 +223,6 @@
 
 	list_del_init(&td->fl_list);
 	td->frame = -1;
-
-out:
-	spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
 }
 
 /*
@@ -339,12 +319,11 @@
 
 /*
  * Append this urb's qh after the last qh in skelqh->list
- * MUST be called with uhci->frame_list_lock acquired
  *
  * Note that urb_priv.queue_list doesn't have a separate queue head;
  * it's a ring with every element "live".
  */
-static void _uhci_insert_qh(struct uhci_hcd *uhci, struct uhci_qh *skelqh, struct urb *urb)
+static void uhci_insert_qh(struct uhci_hcd *uhci, struct uhci_qh *skelqh, struct urb *urb)
 {
 	struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
 	struct list_head *tmp;
@@ -396,22 +375,12 @@
 	list_add_tail(&urbp->qh->list, &skelqh->list);
 }
 
-static void uhci_insert_qh(struct uhci_hcd *uhci, struct uhci_qh *skelqh, struct urb *urb)
-{
-	unsigned long flags;
-
-	spin_lock_irqsave(&uhci->frame_list_lock, flags);
-	_uhci_insert_qh(uhci, skelqh, urb);
-	spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
-}
-
 /*
  * Start removal of QH from schedule; it finishes next frame.
  * TDs should be unlinked before this is called.
  */
 static void uhci_remove_qh(struct uhci_hcd *uhci, struct uhci_qh *qh)
 {
-	unsigned long flags;
 	struct uhci_qh *pqh;
 
 	if (!qh)
@@ -420,7 +389,6 @@
 	/*
 	 * Only go through the hoops if it's actually linked in
 	 */
-	spin_lock_irqsave(&uhci->frame_list_lock, flags);
 	if (!list_empty(&qh->list)) {
 		pqh = list_entry(qh->list.prev, struct uhci_qh, list);
 
@@ -456,20 +424,15 @@
 		}
 		list_del_init(&qh->list);
 	}
-	spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
 
 	qh->urbp = NULL;
 
-	spin_lock_irqsave(&uhci->qh_remove_list_lock, flags);
-
 	/* Check to see if the remove list is empty. Set the IOC bit */
 	/* to force an interrupt so we can remove the QH */
 	if (list_empty(&uhci->qh_remove_list))
 		uhci_set_next_interrupt(uhci);
 
 	list_add(&qh->remove_list, &uhci->qh_remove_list);
-
-	spin_unlock_irqrestore(&uhci->qh_remove_list_lock, flags);
 }
 
 static int uhci_fixup_toggle(struct urb *urb, unsigned int toggle)
@@ -503,13 +466,10 @@
 	struct urb_priv *eurbp, *urbp, *furbp, *lurbp;
 	struct list_head *tmp;
 	struct uhci_td *lltd;
-	unsigned long flags;
 
 	eurbp = eurb->hcpriv;
 	urbp = urb->hcpriv;
 
-	spin_lock_irqsave(&uhci->frame_list_lock, flags);
-
 	/* Find the first URB in the queue */
 	if (eurbp->queued) {
 		struct list_head *head = &eurbp->queue_list;
@@ -549,8 +509,6 @@
 	list_add_tail(&urbp->queue_list, &furbp->queue_list);
 
 	urbp->queued = 1;
-
-	spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
 }
 
 static void uhci_delete_queued_urb(struct uhci_hcd *uhci, struct urb *urb)
@@ -560,14 +518,11 @@
 	struct urb_priv *purbp;
 	struct uhci_td *pltd;
 	unsigned int toggle;
-	unsigned long flags;
 
 	urbp = urb->hcpriv;
 
-	spin_lock_irqsave(&uhci->frame_list_lock, flags);
-
 	if (list_empty(&urbp->queue_list))
-		goto out;
+		return;
 
 	nurbp = list_entry(urbp->queue_list.next, struct urb_priv, queue_list);
 
@@ -625,9 +580,6 @@
 	}
 
 	list_del_init(&urbp->queue_list);
-
-out:
-	spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
 }
 
 static struct urb_priv *uhci_alloc_urb_priv(struct uhci_hcd *uhci, struct urb *urb)
@@ -655,9 +607,6 @@
 	return urbp;
 }
 
-/*
- * MUST be called with urb->lock acquired
- */
 static void uhci_add_td_to_urb(struct urb *urb, struct uhci_td *td)
 {
 	struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
@@ -667,9 +616,6 @@
 	list_add_tail(&td->list, &urbp->td_list);
 }
 
-/*
- * MUST be called with urb->lock acquired
- */
 static void uhci_remove_td_from_urb(struct uhci_td *td)
 {
 	if (list_empty(&td->list))
@@ -680,14 +626,10 @@
 	td->urb = NULL;
 }
 
-/*
- * MUST be called with urb->lock acquired
- */
 static void uhci_destroy_urb_priv(struct uhci_hcd *uhci, struct urb *urb)
 {
 	struct list_head *head, *tmp;
 	struct urb_priv *urbp;
-	unsigned long flags;
 
 	urbp = (struct urb_priv *)urb->hcpriv;
 	if (!urbp)
@@ -697,8 +639,6 @@
 		dev_warn(uhci_dev(uhci), "urb %p still on uhci->urb_list "
 				"or uhci->remove_list!\n", urb);
 
-	spin_lock_irqsave(&uhci->td_remove_list_lock, flags);
-
 	/* Check to see if the remove list is empty. Set the IOC bit */
 	/* to force an interrupt so we can remove the TD's*/
 	if (list_empty(&uhci->td_remove_list))
@@ -716,42 +656,30 @@
 		list_add(&td->remove_list, &uhci->td_remove_list);
 	}
 
-	spin_unlock_irqrestore(&uhci->td_remove_list_lock, flags);
-
 	urb->hcpriv = NULL;
 	kmem_cache_free(uhci_up_cachep, urbp);
 }
 
 static void uhci_inc_fsbr(struct uhci_hcd *uhci, struct urb *urb)
 {
-	unsigned long flags;
 	struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
 
-	spin_lock_irqsave(&uhci->frame_list_lock, flags);
-
 	if ((!(urb->transfer_flags & URB_NO_FSBR)) && !urbp->fsbr) {
 		urbp->fsbr = 1;
 		if (!uhci->fsbr++ && !uhci->fsbrtimeout)
 			uhci->skel_term_qh->link = cpu_to_le32(uhci->skel_hs_control_qh->dma_handle) | UHCI_PTR_QH;
 	}
-
-	spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
 }
 
 static void uhci_dec_fsbr(struct uhci_hcd *uhci, struct urb *urb)
 {
-	unsigned long flags;
 	struct urb_priv *urbp = (struct urb_priv *)urb->hcpriv;
 
-	spin_lock_irqsave(&uhci->frame_list_lock, flags);
-
 	if ((!(urb->transfer_flags & URB_NO_FSBR)) && urbp->fsbr) {
 		urbp->fsbr = 0;
 		if (!--uhci->fsbr)
 			uhci->fsbrtimeout = jiffies + FSBR_DELAY;
 	}
-
-	spin_unlock_irqrestore(&uhci->frame_list_lock, flags);
 }
 
 /*
@@ -1365,9 +1293,6 @@
 	return ret;
 }
 
-/*
- * MUST be called with uhci->urb_list_lock acquired
- */
 static struct urb *uhci_find_urb_ep(struct uhci_hcd *uhci, struct urb *urb)
 {
 	struct list_head *tmp, *head;
@@ -1405,7 +1330,7 @@
 	struct urb *eurb;
 	int bustime;
 
-	spin_lock_irqsave(&uhci->urb_list_lock, flags);
+	spin_lock_irqsave(&uhci->schedule_lock, flags);
 
 	if (urb->status != -EINPROGRESS)	/* URB already unlinked! */
 		goto out;
@@ -1462,14 +1387,12 @@
 		ret = 0;
 
 out:
-	spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
+	spin_unlock_irqrestore(&uhci->schedule_lock, flags);
 	return ret;
 }
 
 /*
  * Return the result of a transfer
- *
- * MUST be called with urb_list_lock acquired
  */
 static void uhci_transfer_result(struct uhci_hcd *uhci, struct urb *urb)
 {
@@ -1505,7 +1428,6 @@
 	case PIPE_BULK:
 	case PIPE_ISOCHRONOUS:
 		/* Release bandwidth for Interrupt or Isoc. transfers */
-		/* Spinlock needed ? */
 		if (urb->bandwidth)
 			usb_release_bandwidth(urb->dev, urb, 1);
 		uhci_unlink_generic(uhci, urb);
@@ -1513,15 +1435,12 @@
 	case PIPE_INTERRUPT:
 		/* Release bandwidth for Interrupt or Isoc. transfers */
 		/* Make sure we don't release if we have a queued URB */
-		spin_lock(&uhci->frame_list_lock);
-		/* Spinlock needed ? */
 		if (list_empty(&urbp->queue_list) && urb->bandwidth)
 			usb_release_bandwidth(urb->dev, urb, 0);
 		else
 			/* bandwidth was passed on to queued URB, */
 			/* so don't let usb_unlink_urb() release it */
 			urb->bandwidth = 0;
-		spin_unlock(&uhci->frame_list_lock);
 		uhci_unlink_generic(uhci, urb);
 		break;
 	default:
@@ -1537,9 +1456,6 @@
 	spin_unlock(&urb->lock);
 }
 
-/*
- * MUST be called with urb->lock acquired
- */
 static void uhci_unlink_generic(struct uhci_hcd *uhci, struct urb *urb)
 {
 	struct list_head *head, *tmp;
@@ -1595,7 +1511,7 @@
 	unsigned long flags;
 	struct urb_priv *urbp;
 
-	spin_lock_irqsave(&uhci->urb_list_lock, flags);
+	spin_lock_irqsave(&uhci->schedule_lock, flags);
 	urbp = urb->hcpriv;
 	if (!urbp)			/* URB was never linked! */
 		goto done;
@@ -1603,16 +1519,13 @@
 
 	uhci_unlink_generic(uhci, urb);
 
-	spin_lock(&uhci->urb_remove_list_lock);
-
 	/* If we're the first, set the next interrupt bit */
 	if (list_empty(&uhci->urb_remove_list))
 		uhci_set_next_interrupt(uhci);
 	list_add_tail(&urbp->urb_list, &uhci->urb_remove_list);
 
-	spin_unlock(&uhci->urb_remove_list_lock);
 done:
-	spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
+	spin_unlock_irqrestore(&uhci->schedule_lock, flags);
 	return 0;
 }
 
@@ -1674,7 +1587,7 @@
 
 	INIT_LIST_HEAD(&list);
 
-	spin_lock_irqsave(&uhci->urb_list_lock, flags);
+	spin_lock_irqsave(&uhci->schedule_lock, flags);
 	head = &uhci->urb_list;
 	tmp = head->next;
 	while (tmp != head) {
@@ -1690,12 +1603,15 @@
 			uhci_fsbr_timeout(uhci, u);
 
 		/* Check if the URB timed out */
-		if (u->timeout && time_after_eq(jiffies, up->inserttime + u->timeout))
+		if (u->timeout && u->status == -EINPROGRESS &&
+			time_after_eq(jiffies, up->inserttime + u->timeout)) {
+			u->status = -ETIMEDOUT;
 			list_move_tail(&up->urb_list, &list);
+		}
 
 		spin_unlock(&u->lock);
 	}
-	spin_unlock_irqrestore(&uhci->urb_list_lock, flags);
+	spin_unlock_irqrestore(&uhci->schedule_lock, flags);
 
 	head = &list;
 	tmp = head->next;
@@ -1737,7 +1653,6 @@
 {
 	struct list_head *tmp, *head;
 
-	spin_lock(&uhci->qh_remove_list_lock);
 	head = &uhci->qh_remove_list;
 	tmp = head->next;
 	while (tmp != head) {
@@ -1749,14 +1664,12 @@
 
 		uhci_free_qh(uhci, qh);
 	}
-	spin_unlock(&uhci->qh_remove_list_lock);
 }
 
 static void uhci_free_pending_tds(struct uhci_hcd *uhci)
 {
 	struct list_head *tmp, *head;
 
-	spin_lock(&uhci->td_remove_list_lock);
 	head = &uhci->td_remove_list;
 	tmp = head->next;
 	while (tmp != head) {
@@ -1768,18 +1681,17 @@
 
 		uhci_free_td(uhci, td);
 	}
-	spin_unlock(&uhci->td_remove_list_lock);
 }
 
 static void uhci_finish_urb(struct usb_hcd *hcd, struct urb *urb, struct pt_regs *regs)
 {
 	struct uhci_hcd *uhci = hcd_to_uhci(hcd);
 
-	spin_lock(&urb->lock);
 	uhci_destroy_urb_priv(uhci, urb);
-	spin_unlock(&urb->lock);
 
+	spin_unlock(&uhci->schedule_lock);
 	usb_hcd_giveback_urb(hcd, urb, regs);
+	spin_lock(&uhci->schedule_lock);
 }
 
 static void uhci_finish_completion(struct usb_hcd *hcd, struct pt_regs *regs)
@@ -1787,7 +1699,6 @@
 	struct uhci_hcd *uhci = hcd_to_uhci(hcd);
 	struct list_head *tmp, *head;
 
-	spin_lock(&uhci->complete_list_lock);
 	head = &uhci->complete_list;
 	tmp = head->next;
 	while (tmp != head) {
@@ -1795,26 +1706,18 @@
 		struct urb *urb = urbp->urb;
 
 		list_del_init(&urbp->urb_list);
-		spin_unlock(&uhci->complete_list_lock);
-
 		uhci_finish_urb(hcd, urb, regs);
 
-		spin_lock(&uhci->complete_list_lock);
 		head = &uhci->complete_list;
 		tmp = head->next;
 	}
-	spin_unlock(&uhci->complete_list_lock);
 }
 
 static void uhci_remove_pending_urbps(struct uhci_hcd *uhci)
 {
-	spin_lock(&uhci->urb_remove_list_lock);
-	spin_lock(&uhci->complete_list_lock);
 
 	/* Splice the urb_remove_list onto the end of the complete_list */
 	list_splice_init(&uhci->urb_remove_list, uhci->complete_list.prev);
-	spin_unlock(&uhci->complete_list_lock);
-	spin_unlock(&uhci->urb_remove_list_lock);
 }
 
 static irqreturn_t uhci_irq(struct usb_hcd *hcd, struct pt_regs *regs)
@@ -1851,16 +1754,15 @@
 	if (status & USBSTS_RD)
 		uhci->resume_detect = 1;
 
-	uhci_free_pending_qhs(uhci);
+	spin_lock(&uhci->schedule_lock);
 
+	uhci_free_pending_qhs(uhci);
 	uhci_free_pending_tds(uhci);
-
 	uhci_remove_pending_urbps(uhci);
 
 	uhci_clear_next_interrupt(uhci);
 
 	/* Walk the list of pending URB's to see which ones completed */
-	spin_lock(&uhci->urb_list_lock);
 	head = &uhci->urb_list;
 	tmp = head->next;
 	while (tmp != head) {
@@ -1872,9 +1774,10 @@
 		/* Checks the status and does all of the magic necessary */
 		uhci_transfer_result(uhci, urb);
 	}
-	spin_unlock(&uhci->urb_list_lock);
-
 	uhci_finish_completion(hcd, regs);
+
+	spin_unlock(&uhci->schedule_lock);
+
 	return IRQ_HANDLED;
 }
 
@@ -2164,23 +2067,17 @@
 	uhci->fsbr = 0;
 	uhci->fsbrtimeout = 0;
 
-	spin_lock_init(&uhci->qh_remove_list_lock);
+	spin_lock_init(&uhci->schedule_lock);
 	INIT_LIST_HEAD(&uhci->qh_remove_list);
 
-	spin_lock_init(&uhci->td_remove_list_lock);
 	INIT_LIST_HEAD(&uhci->td_remove_list);
 
-	spin_lock_init(&uhci->urb_remove_list_lock);
 	INIT_LIST_HEAD(&uhci->urb_remove_list);
 
-	spin_lock_init(&uhci->urb_list_lock);
 	INIT_LIST_HEAD(&uhci->urb_list);
 
-	spin_lock_init(&uhci->complete_list_lock);
 	INIT_LIST_HEAD(&uhci->complete_list);
 
-	spin_lock_init(&uhci->frame_list_lock);
-
 	uhci->fl = dma_alloc_coherent(uhci_dev(uhci), sizeof(*uhci->fl),
 			&dma_handle, 0);
 	if (!uhci->fl) {
@@ -2372,7 +2269,6 @@
 static void uhci_stop(struct usb_hcd *hcd)
 {
 	struct uhci_hcd *uhci = hcd_to_uhci(hcd);
-	unsigned long flags;
 
 	del_timer_sync(&uhci->stall_timer);
 
@@ -2380,16 +2276,18 @@
 	 * At this point, we're guaranteed that no new connects can be made
 	 * to this bus since there are no more parents
 	 */
-	local_irq_save(flags);
+	spin_lock_irq(&uhci->schedule_lock);
 	uhci_free_pending_qhs(uhci);
 	uhci_free_pending_tds(uhci);
 	uhci_remove_pending_urbps(uhci);
+	spin_unlock_irq(&uhci->schedule_lock);
 
 	reset_hc(uhci);
 
+	spin_lock_irq(&uhci->schedule_lock);
 	uhci_free_pending_qhs(uhci);
 	uhci_free_pending_tds(uhci);
-	local_irq_restore(flags);
+	spin_unlock_irq(&uhci->schedule_lock);
 	
 	release_uhci(uhci);
 }
diff -Nru a/drivers/usb/host/uhci-hcd.h b/drivers/usb/host/uhci-hcd.h
--- a/drivers/usb/host/uhci-hcd.h	Wed Apr 14 14:38:36 2004
+++ b/drivers/usb/host/uhci-hcd.h	Wed Apr 14 14:38:36 2004
@@ -342,8 +342,8 @@
 	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;
-	struct uhci_frame_list *fl;		/* P: uhci->frame_list_lock */
+	spinlock_t schedule_lock;
+	struct uhci_frame_list *fl;		/* P: uhci->schedule_lock */
 	int fsbr;				/* Full-speed bandwidth reclamation */
 	unsigned long fsbrtimeout;		/* FSBR delay */
 
@@ -353,24 +353,19 @@
 	unsigned int saved_framenumber;		/* Save during PM suspend */
 
 	/* Main list of URB's currently controlled by this HC */
-	spinlock_t urb_list_lock;
-	struct list_head urb_list;		/* P: uhci->urb_list_lock */
+	struct list_head urb_list;		/* P: uhci->schedule_lock */
 
 	/* List of QH's that are done, but waiting to be unlinked (race) */
-	spinlock_t qh_remove_list_lock;
-	struct list_head qh_remove_list;	/* P: uhci->qh_remove_list_lock */
+	struct list_head qh_remove_list;	/* P: uhci->schedule_lock */
 
 	/* List of TD's that are done, but waiting to be freed (race) */
-	spinlock_t td_remove_list_lock;
-	struct list_head td_remove_list;	/* P: uhci->td_remove_list_lock */
+	struct list_head td_remove_list;	/* P: uhci->schedule_lock */
 
 	/* List of asynchronously unlinked URB's */
-	spinlock_t urb_remove_list_lock;
-	struct list_head urb_remove_list;	/* P: uhci->urb_remove_list_lock */
+	struct list_head urb_remove_list;	/* P: uhci->schedule_lock */
 
 	/* List of URB's awaiting completion callback */
-	spinlock_t complete_list_lock;
-	struct list_head complete_list;		/* P: uhci->complete_list_lock */
+	struct list_head complete_list;		/* P: uhci->schedule_lock */
 
 	int rh_numports;
 
@@ -401,26 +396,15 @@
 /*
  * Locking in uhci.c
  *
- * spinlocks are used extensively to protect the many lists and data
- * structures we have. It's not that pretty, but it's necessary. We
- * need to be done with all of the locks (except complete_list_lock) when
- * we call urb->complete. I've tried to make it simple enough so I don't
- * have to spend hours racking my brain trying to figure out if the
- * locking is safe.
+ * Almost everything relating to the hardware schedule and processing
+ * of URBs is protected by uhci->schedule_lock.  urb->status is protected
+ * by urb->lock; that's the one exception.
  *
- * Here's the safe locking order to prevent deadlocks:
+ * To prevent deadlocks, never lock uhci->schedule_lock while holding
+ * urb->lock.  The safe order of locking is:
  *
- * #1 uhci->urb_list_lock
+ * #1 uhci->schedule_lock
  * #2 urb->lock
- * #3 uhci->urb_remove_list_lock, uhci->frame_list_lock, 
- *   uhci->qh_remove_list_lock
- * #4 uhci->complete_list_lock
- *
- * If you're going to grab 2 or more locks at once, ALWAYS grab the lock
- * at the lowest level FIRST and NEVER grab locks at the same level at the
- * same time.
- * 
- * So, if you need uhci->urb_list_lock, grab it before you grab urb->lock
  */
 
 #endif