New patch to review for coreboot: 3f035bf libpayload: Implement correct interrupt-queue linking for UHCI

[Nico Huber]

Nico Huber ([e-mail]) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/1897

-gerrit

commit 3f035bfc1a2cbd74b9630a586c7819187a2260eb
Author: Nico Huber 
Date:   Tue Nov 20 17:27:46 2012 +0100

libpayload: Implement correct interrupt-queue linking for UHCI

The linking of interrupt queues into UHCI controller's framelist (in
    uhci_create_intr_queue()) was incomplete. The implementation of
    uhci_destroy_intr_queue() was even worse, looking like it wanted to
    clean up more than uhci_create_intr_queue() did.

This patch follows the simple approach that we used for OHCI and EHCI:
    Each slot in the framelist holds only one interrupt queue. Therefore, we
    have to look for free slots each time we want to link an interrupt queue
    into the framelist. In return, we have a much simpler structured
    framelist.

With this, USB devices using interrupt transfers (e.g. keyboards) can be
    detached cleanly from UHCI controllers. Also, more than one of such
    devices can be attached without further risk.

Change-Id: I07b81a3b6f2cb3ff69515c973b3ae6321ad969aa
    Signed-off-by: Nico Huber 
---
 payloads/libpayload/drivers/usb/uhci.c | 49 +++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/payloads/libpayload/drivers/usb/uhci.c b/payloads/libpayload/drivers/usb/uhci.c
index 2eea375..bfa53f6 100644
--- a/payloads/libpayload/drivers/usb/uhci.c
+++ b/payloads/libpayload/drivers/usb/uhci.c
@@ -517,11 +517,27 @@ uhci_create_intr_queue (endpoint_t *ep, int reqsize, int reqcount, int reqtiming
    	data += reqsize;
    }
    tds[reqcount - 1].ptr = 0 | TD_TERMINATE;
-	for (i = reqtiming; i < 1024; i += reqtiming) {
-		/* FIXME: wrap in another qh, one for each occurance of the qh in the framelist */
-		qh->headlinkptr = UHCI_INST (ep->dev->controller)->framelistptr[i] & ~FLISTP_TERMINATE;
-		UHCI_INST (ep->dev->controller)->framelistptr[i] = virt_to_phys(qh) | FLISTP_QH;
+
+	/* insert QH into framelist */
+	uhci_t *const uhcic = UHCI_INST(ep->dev->controller);
+	const u32 def_ptr = virt_to_phys(uhcic->qh_prei) | FLISTP_QH;
+	int nothing_placed = 1;
+	qh->headlinkptr = def_ptr;
+	for (i = 0; i < 1024; i += reqtiming) {
+		/* advance to the next free position */
+		while ((i < 1024) && (uhcic->framelistptr[i] != def_ptr)) ++i;
+		if (i < 1024) {
+			uhcic->framelistptr[i] = virt_to_phys(qh) | FLISTP_QH;
+			nothing_placed = 0;
+		}
    }
+	if (nothing_placed) {
+		printf("Error: Failed to place UHCI interrupt queue "
+			      "head into framelist: no space left\n");
+		uhci_destroy_intr_queue(ep, q);
+		return NULL;
+	}
+
    return q;
 }

@@ -529,23 +545,18 @@ uhci_create_intr_queue (endpoint_t *ep, int reqsize, int reqcount, int reqtiming
 static void
 uhci_destroy_intr_queue (endpoint_t *ep, void *q_)
 {
-	intr_q *q = (intr_q*)q_;
-	u32 val = virt_to_phys (q->qh);
-	u32 end = virt_to_phys (UHCI_INST (ep->dev->controller)->qh_intr);
+	intr_q *const q = (intr_q*)q_;
+
+	/* remove QH from framelist */
+	uhci_t *const uhcic = UHCI_INST(ep->dev->controller);
+	const u32 qh_ptr = virt_to_phys(q->qh) | FLISTP_QH;
+	const u32 def_ptr = virt_to_phys(uhcic->qh_prei) | FLISTP_QH;
    int i;
-	for (i=0; i<1024; i++) {
-		u32 oldptr = 0;
-		u32 ptr = UHCI_INST (ep->dev->controller)->framelistptr[i];
-		while (ptr != end) {
-			if (((qh_t*)phys_to_virt(ptr))->elementlinkptr == val) {
-				((qh_t*)phys_to_virt(oldptr))->headlinkptr = ((qh_t*)phys_to_virt(ptr))->headlinkptr;
-				free(phys_to_virt(ptr));
-				break;
-			}
-			oldptr = ptr;
-			ptr = ((qh_t*)phys_to_virt(ptr))->headlinkptr;
-		}
+	for (i = 0; i < 1024; ++i) {
+		if (uhcic->framelistptr[i] == qh_ptr)
+			uhcic->framelistptr[i] = def_ptr;
    }
+
    free(q->data);
    free(q->tds);
    free(q->qh);