1) Change printf's into DPRINTF.

2) rename variables to be more conclusive.
3) fix a problem in uhci_ii_done. Avoid collecting all the status's of the
TD's, we only need to one from the last inactive one.
4) Change the errorcount from 2 to 3 (see UHCI spec.).

1 files changed, 95 insertions, 84 deletions

diff --git a/sys/dev/usb/uhci.c b/sys/dev/usb/uhci.c
index 62cb106..713eb52 100644
--- a/sys/dev/usb/uhci.c
+++ b/sys/dev/usb/uhci.c
@@ -45,8 +45,7 @@
  * Data sheets: ftp://download.intel.com/design/intarch/datashts/29055002.pdf
  *              ftp://download.intel.com/design/intarch/datashts/29056201.pdf
  * UHCI spec: http://www.intel.com/design/usb/uhci11d.pdf
- * USB spec: http://www.usb.org/cgi-usb/mailmerge.cgi/home/usb/docs/developers/
-cgiform.tpl
+ * USB spec: http://www.usb.org/cgi-usb/mailmerge.cgi/home/usb/docs/developers/cgiform.tpl
  */
 
 #include <sys/param.h>
@@ -399,7 +398,7 @@ static void
 uhci_dumpregs(sc)
 	uhci_softc_t *sc;
 {
-	printf("%s: regs: cmd=%04x, sts=%04x, intr=%04x, frnum=%04x, "
+	DPRINTF(("%s: regs: cmd=%04x, sts=%04x, intr=%04x, frnum=%04x, "
 	       "flbase=%08x, sof=%02x, portsc1=%04x, portsc2=%04x, ",
 	       USBDEVNAME(sc->sc_bus.bdev),
 	       UREAD2(sc, UHCI_CMD),
@@ -409,7 +408,7 @@ uhci_dumpregs(sc)
 	       UREAD4(sc, UHCI_FLBASEADDR),
 	       UREAD1(sc, UHCI_SOF),
 	       UREAD2(sc, UHCI_PORTSC1),
-	       UREAD2(sc, UHCI_PORTSC2));
+	       UREAD2(sc, UHCI_PORTSC2)));
 }
 
 int uhci_longtd = 1;
@@ -418,14 +417,14 @@ void
 uhci_dump_td(p)
 	uhci_soft_td_t *p;
 {
-	printf("TD(%p) at %08lx link=0x%08lx st=0x%08lx tok=0x%08lx buf=0x%08lx\n",
+	DPRINTF(("TD(%p) at %08lx link=0x%08lx st=0x%08lx tok=0x%08lx buf=0x%08lx\n",
 	       p, (long)p->physaddr,
 	       (long)p->td->td_link,
 	       (long)p->td->td_status,
 	       (long)p->td->td_token,
-	       (long)p->td->td_buffer);
+	       (long)p->td->td_buffer));
 	if (uhci_longtd)
-		printf("  %b %b,errcnt=%d,actlen=%d pid=%02x,addr=%d,endpt=%d,"
+		DPRINTF((" %b %b,errcnt=%d,actlen=%d pid=%02x,addr=%d,endpt=%d,"
 		       "D=%d,maxlen=%d\n",
 		       (int)p->td->td_link,
 		       "\20\1T\2Q\3VF",
@@ -438,15 +437,15 @@ uhci_dump_td(p)
 		       UHCI_TD_GET_DEVADDR(p->td->td_token),
 		       UHCI_TD_GET_ENDPT(p->td->td_token),
 		       UHCI_TD_GET_DT(p->td->td_token),
-		       UHCI_TD_GET_MAXLEN(p->td->td_token));
+		       UHCI_TD_GET_MAXLEN(p->td->td_token)));
 }
 
 void
 uhci_dump_qh(p)
 	uhci_soft_qh_t *p;
 {
-	printf("QH(%p) at %08x: hlink=%08x elink=%08x\n", p, (int)p->physaddr,
-	       p->qh->qh_hlink, p->qh->qh_elink);
+	DPRINTF(("QH(%p) at %08x: hlink=%08x elink=%08x\n", p, (int)p->physaddr,
+	       p->qh->qh_hlink, p->qh->qh_elink));
 }
 
 
@@ -457,8 +456,8 @@ uhci_dump()
 	uhci_softc_t *sc = uhci;
 
 	uhci_dumpregs(sc);
-	printf("intrs=%d\n", sc->sc_intrs);
-	printf("framelist[i].link = %08x\n", sc->sc_framelist[0].link);
+	DPRINTF("intrs=%d\n", sc->sc_intrs);
+	DPRINTF("framelist[i].link = %08x\n", sc->sc_framelist[0].link);
 	uhci_dump_qh(sc->sc_ctl_start->qh->hlink);
 }
 #endif
@@ -596,7 +595,7 @@ uhci_remove_ctrl(sc, sqh)
 	for (pqh = sc->sc_ctl_start; pqh->qh->hlink != sqh; pqh=pqh->qh->hlink)
 #if defined(DIAGNOSTIC) || defined(UHCI_DEBUG)		
 		if (pqh->qh->qh_hlink & UHCI_PTR_T) {
-			printf("uhci_remove_ctrl: QH not found\n");
+			DPRINTF(("uhci_remove_ctrl: QH not found\n"));
 			return;
 		}
 #else
@@ -639,7 +638,7 @@ uhci_remove_bulk(sc, sqh)
 	     pqh = pqh->qh->hlink)
 #if defined(DIAGNOSTIC) || defined(UHCI_DEBUG)		
 		if (pqh->qh->qh_hlink & UHCI_PTR_T) {
-			printf("uhci_remove_bulk: QH not found\n");
+			DPRINTF(("uhci_remove_bulk: QH not found\n"));
 			return;
 		}
 #else
@@ -662,7 +661,7 @@ uhci_intr(p)
 	sc->sc_intrs++;
 #if defined(UHCI_DEBUG)
 	if (uhcidebug > 9) {
-		printf("uhci_intr %p\n", sc);
+		DPRINTF(("uhci_intr %p\n", sc));
 		uhci_dumpregs(sc);
 	}
 #endif
@@ -678,21 +677,21 @@ uhci_intr(p)
 	}
 	if (status & UHCI_STS_RD) {
 		UWRITE2(sc, UHCI_STS, UHCI_STS_RD); /* acknowledge */
-		printf("%s: resume detect\n", USBDEVNAME(sc->sc_bus.bdev));
+		DPRINTF(("%s: resume detect\n", USBDEVNAME(sc->sc_bus.bdev)));
 		ret = 1;
 	}
 	if (status & UHCI_STS_HSE) {
 		UWRITE2(sc, UHCI_STS, UHCI_STS_HSE); /* acknowledge */
-		printf("%s: Host System Error\n", USBDEVNAME(sc->sc_bus.bdev));
+		DPRINTF(("%s: Host System Error\n", USBDEVNAME(sc->sc_bus.bdev)));
 		ret = 1;
 	}
 	if (status & UHCI_STS_HCPE) {
 		UWRITE2(sc, UHCI_STS, UHCI_STS_HCPE); /* acknowledge */
-		printf("%s: Host System Error\n", USBDEVNAME(sc->sc_bus.bdev));
+		DPRINTF(("%s: Host System Error\n", USBDEVNAME(sc->sc_bus.bdev)));
 		ret = 1;
 	}
 	if (status & UHCI_STS_HCH)
-		printf("%s: controller halted\n", USBDEVNAME(sc->sc_bus.bdev));
+		DPRINTF(("%s: controller halted\n", USBDEVNAME(sc->sc_bus.bdev)));
 	if (!ret)
 		return 0;
 
@@ -721,7 +720,7 @@ uhci_check_intr(sc, ii)
 	uhci_intr_info_t *ii;
 {
 	struct uhci_pipe *upipe;
-	uhci_soft_td_t *std, *lstd;
+	uhci_soft_td_t *std;
 	u_int32_t status;
 
 	DPRINTFN(15, ("uhci_check_intr: ii=%p\n", ii));
@@ -730,28 +729,27 @@ uhci_check_intr(sc, ii)
 		printf("uhci_check_intr: no ii? %p\n", ii);
 		return;
 	}
-#endif
-	if (!ii->stdstart)
-		return;
-	lstd = ii->stdend;
-#ifdef DIAGNOSTIC
-	if (!lstd) {
-		printf("uhci_check_intr: std==0\n");
+	if (!ii->stdend) {
+		printf("uhci_check_intr: ii->stdend==0\n");
 		return;
 	}
 #endif
-	/* If the last TD is still active the whole transfer probably is. */
-	if (lstd->td->td_status & UHCI_TD_ACTIVE) {
-		DPRINTFN(15, ("uhci_check_intr: active ii=%p\n", ii));
-		for (std = ii->stdstart; std != lstd; std = std->td->link.std){
+
+	if (!ii->stdstart)
+		return;
+
+	/* If the last TD is still active we need to check whether there
+	 * is a an error somewhere in the middle, or whether there was a
+	 * short packet (SPD and not ACTIVE).
+	 */
+	if (ii->stdend->td->td_status & UHCI_TD_ACTIVE) {
+		for (std = ii->stdstart; std != ii->stdend; std = std->td->link.std){
 			status = std->td->td_status;
+			DPRINTF(("status=0x%04x\n", status));
 			if ((status & UHCI_TD_STALLED) ||
-			     (status & (UHCI_TD_SPD | UHCI_TD_ACTIVE)) == 
-			     UHCI_TD_SPD)
+			     (status & (UHCI_TD_SPD | UHCI_TD_ACTIVE)) == UHCI_TD_SPD)
 				goto done;
 		}
-		DPRINTFN(15, ("uhci_check_intr: ii=%p std=%p still active\n",
-			      ii, ii->stdstart));
 		return;
 	}
  done:
@@ -764,21 +762,22 @@ uhci_check_intr(sc, ii)
 void
 uhci_ii_done(ii, timo)
 	uhci_intr_info_t *ii;
-	int timo;
+	int timo;		/* timeout that triggered function call? */
 {
 	usbd_request_handle reqh = ii->reqh;
 	uhci_soft_td_t *std;
-	u_int32_t tst;
-	int len, status, attr;
+	int actlen = 0;		/* accumulated actual length for queue */
+	int err = 0;		/* error status of last inactive transfer */
 
 	DPRINTFN(10, ("uhci_ii_done: ii=%p ready %d\n", ii, timo));
 
 #ifdef DIAGNOSTIC
 	{
+		/* avoid finishing a transfer more than once */
 		int s = splhigh();
 		if (ii->isdone) {
-			printf("uhci_ii_done: is done!\n");
 			splx(s);
+			printf("uhci_ii_done: is done!\n");
 			return;
 		}
 		ii->isdone = 1;
@@ -786,54 +785,56 @@ uhci_ii_done(ii, timo)
 	}
 #endif
 
-	/* The transfer is done, compute length and status. */
-	/* XXX Should stop at first inactive to get toggle right. */
+	/* The transfer is done; compute actual length and status */
 	/* XXX Is this correct for control xfers? */
-	for (len = status = 0, std = ii->stdstart; 
-	     std != 0; 
-	     std = std->td->link.std) {
-		tst = std->td->td_status;
-		status |= tst;
-#ifdef UHCI_DEBUG
-		if ((tst & UHCI_TD_ERROR) && uhcidebug) {
-			printf("uhci_ii_done: intr error TD:\n");
-			uhci_dump_td(std);
-		}
-#endif
+	for (std = ii->stdstart; std; std = std->td->link.std) {
+		if (std->td->td_status & UHCI_TD_ACTIVE)
+			break;
+
+		/* error status of last TD for error handling below */
+		err = std->td->td_status & UHCI_TD_ERROR;
+
 		if (UHCI_TD_GET_PID(std->td->td_token) != UHCI_TD_PID_SETUP)
-			len += UHCI_TD_GET_ACTLEN(tst);
+			actlen += UHCI_TD_GET_ACTLEN(std->td->td_status);
 	}
-	status &= UHCI_TD_ERROR;
-	DPRINTFN(10, ("uhci_ii_done: len=%d, status=0x%x\n", len, status));
-	if (status != 0) {
-		DPRINTFN(-1+(status & UHCI_TD_STALLED),
+
+	DPRINTFN(10, ("uhci_ii_done: actlen=%d, err=0x%x\n", actlen, err));
+
+	if (err != 0) {
+		DPRINTFN(-1+((err & ~UHCI_TD_STALLED) != 0),
 			 ("uhci_ii_done: error, addr=%d, endpt=0x%02x, "
-			  "status 0x%b\n",
+			  "err=0x%b\n",
 			  reqh->pipe->device->address,
 			  reqh->pipe->endpoint->edesc->bEndpointAddress,
-			  (int)status, 
+			  (int)err, 
 			  "\20\22BITSTUFF\23CRCTO\24NAK\25BABBLE\26DBUFFER\27"
 			  "STALLED\30ACTIVE"));
-		if (status & UHCI_TD_STALLED)
-			reqh->status = USBD_STALLED;
-		else
+
+		if (err & ~UHCI_TD_STALLED) {
+			/* more then STALLED, like +BABBLE or +CRC/TIMEOUT */
 			reqh->status = USBD_IOERROR; /* more info XXX */
-		reqh->actlen = 0;
+		} else {
+			reqh->status = USBD_STALLED;
+		}
 	} else {
 		reqh->status = USBD_NORMAL_COMPLETION;
-		reqh->actlen = len;
 	}
+
+	reqh->actlen = actlen;
+
 	if (timo) {
 		/* We got a timeout.  Make sure transaction is not active. */
-		reqh->status = USBD_TIMEOUT;
 		for (std = ii->stdstart; std != 0; std = std->td->link.std)
 			std->td->td_status &= ~UHCI_TD_ACTIVE;
 		/* XXX should we wait 1 ms */
+		reqh->status = USBD_TIMEOUT;
 	}
 	DPRINTFN(5, ("uhci_ii_done: calling handler ii=%p\n", ii));
 
-	attr = reqh->pipe->endpoint->edesc->bmAttributes;
-	switch (attr & UE_XFERTYPE) {
+	/* select the proper type termination of the transfer
+	 * based on the transfer type for the queue
+	 */
+	switch (reqh->pipe->endpoint->edesc->bmAttributes & UE_XFERTYPE) {
 	case UE_CONTROL:
 		uhci_ctrl_done(ii);
 		usb_start_next(reqh->pipe);
@@ -866,6 +867,9 @@ uhci_timeout(addr)
 	int s;
 
 	DPRINTF(("uhci_timeout: ii=%p\n", ii));
+#ifdef USB_DEBUG
+	uhci_dump_tds(ii->stdstart);
+#endif
 	s = splusb();
 	uhci_ii_done(ii, 1);
 	splx(s);
@@ -1121,7 +1125,7 @@ uhci_alloc_std_chain(upipe, sc, len, rd, spd, dma, sp, ep)
 	int addr = upipe->pipe.device->address;
 	int endpt = upipe->pipe.endpoint->edesc->bEndpointAddress;
 
-	DPRINTFN(15, ("uhci_alloc_std_chain: addr=%d endpt=%d len=%d ls=%d "
+	DPRINTFN(15, ("uhci_alloc_std_chain: addr=%d endpt=%d len=%d lowspeed=%d "
 		      "spd=%d\n", addr, endpt, len, 
 		      upipe->pipe.device->lowspeed, spd));
 	if (len == 0) {
@@ -1142,9 +1146,9 @@ uhci_alloc_std_chain(upipe, sc, len, rd, spd, dma, sp, ep)
 	lastp = 0;
 	lastlink = UHCI_PTR_T;
 	ntd--;
-	status = UHCI_TD_SET_ERRCNT(2) | UHCI_TD_ACTIVE;
+	status = UHCI_TD_SET_ERRCNT(3) | UHCI_TD_ACTIVE;
 	if (upipe->pipe.device->lowspeed)
-		status |= UHCI_TD_LS;
+		status |= UHCI_TD_LOWSPEED;
 	if (spd)
 		status |= UHCI_TD_SPD;
 	for (i = ntd; i >= 0; i--) {
@@ -1224,6 +1228,8 @@ uhci_device_bulk_start(reqh)
 	upipe->u.bulk.isread = isread;
 	upipe->u.bulk.length = len;
 
+	/* XXX FreeBSD can do without dmap, all mem is dma-able */
+	
 	r = usb_allocmem(sc->sc_dmatag, len, 0, dmap);
 	if (r != USBD_NORMAL_COMPLETION)
 		goto ret1;
@@ -1269,12 +1275,17 @@ uhci_device_bulk_start(reqh)
 	}
 	splx(s);
 
+#if 0
 #ifdef UHCI_DEBUG
-	if (uhcidebug > 10) {
-		printf("uhci_device_bulk_transfer: xfer(2)\n");
+	/* this fails, as the transfer is finished before this one
+	 * gets anywhere near the end -> traverse of non-existing list
+	 */ 
+	if (uhcidebug > 100) {
+		DPRINTF(("uhci_device_bulk_transfer: xfer(2)\n"));
 		uhci_dump_tds(xfer);
 	}
 #endif
+#endif
 
 	return (USBD_IN_PROGRESS);
 
@@ -1399,7 +1410,7 @@ uhci_device_intr_start(reqh)
 
 #ifdef UHCI_DEBUG
 	if (uhcidebug > 10) {
-		printf("uhci_device_intr_transfer: xfer(1)\n");
+		DPRINTF(("uhci_device_intr_transfer: xfer(1)\n"));
 		uhci_dump_tds(xfer);
 		uhci_dump_qh(upipe->u.intr.qhs[0]);
 	}
@@ -1425,7 +1436,7 @@ uhci_device_intr_start(reqh)
 
 #ifdef UHCI_DEBUG
 	if (uhcidebug > 10) {
-		printf("uhci_device_intr_transfer: xfer(2)\n");
+		DPRINTF(("uhci_device_intr_transfer: xfer(2)\n"));
 		uhci_dump_tds(xfer);
 		uhci_dump_qh(upipe->u.intr.qhs[0]);
 	}
@@ -1531,7 +1542,7 @@ uhci_device_request(reqh)
 	uhci_soft_qh_t *sqh;
 	usb_dma_t *dmap;
 	int len;
-	u_int32_t ls;
+	u_int32_t lowspeed;
 	usbd_status r;
 	int isread;
 	int s;
@@ -1542,7 +1553,7 @@ uhci_device_request(reqh)
 		    UGETW(req->wIndex), UGETW(req->wLength),
 		    addr, endpt));
 
-	ls = dev->lowspeed ? UHCI_TD_LS : 0;
+	lowspeed = dev->lowspeed ? UHCI_TD_LOWSPEED : 0;
 	isread = req->bmRequestType & UT_READ;
 	len = UGETW(req->wLength);
 
@@ -1576,13 +1587,13 @@ uhci_device_request(reqh)
 
 	setup->td->link.std = next;
 	setup->td->td_link = next->physaddr;
-	setup->td->td_status = UHCI_TD_SET_ERRCNT(2) | ls | UHCI_TD_ACTIVE;
+	setup->td->td_status = UHCI_TD_SET_ERRCNT(3) | lowspeed | UHCI_TD_ACTIVE;
 	setup->td->td_token = UHCI_TD_SETUP(sizeof *req, endpt, addr);
 	setup->td->td_buffer = DMAADDR(&upipe->u.ctl.reqdma);
 
 	stat->td->link.std = 0;
 	stat->td->td_link = UHCI_PTR_T;
-	stat->td->td_status = UHCI_TD_SET_ERRCNT(2) | ls | 
+	stat->td->td_status = UHCI_TD_SET_ERRCNT(3) | lowspeed | 
 		UHCI_TD_ACTIVE | UHCI_TD_IOC;
 	stat->td->td_token = 
 		isread ? UHCI_TD_OUT(0, endpt, addr, 1) :
@@ -1591,9 +1602,9 @@ uhci_device_request(reqh)
 
 #ifdef UHCI_DEBUG
 	if (uhcidebug > 20) {
-		printf("uhci_device_request: setup\n");
+		DPRINTF(("uhci_device_request: setup\n"));
 		uhci_dump_td(setup);
-		printf("uhci_device_request: stat\n");
+		DPRINTF(("uhci_device_request: stat\n"));
 		uhci_dump_td(stat);
 	}
 #endif
@@ -1623,7 +1634,7 @@ uhci_device_request(reqh)
 		uhci_soft_qh_t *sxqh;
 		int maxqh = 0;
 		uhci_physaddr_t link;
-		printf("uhci_enter_ctl_q: follow from [0]\n");
+		DPRINTF(("uhci_enter_ctl_q: follow from [0]\n"));
 		for (std = sc->sc_vframes[0].htd, link = 0;
 		     (link & UHCI_PTR_Q) == 0;
 		     std = std->td->link.std) {
@@ -1637,7 +1648,7 @@ uhci_device_request(reqh)
 			uhci_dump_qh(xqh);
 			uhci_dump_qh(sxqh);
 		}
-		printf("Enqueued QH:\n");
+		DPRINTF(("Enqueued QH:\n"))
 		uhci_dump_qh(sqh);
 		uhci_dump_tds(sqh->qh->elink);
 	}
@@ -1865,7 +1876,7 @@ uhci_intr_done(ii)
 
 #ifdef UHCI_DEBUG
 		if (uhcidebug > 10) {
-			printf("uhci_device_intr_done: xfer(1)\n");
+			DPRINTF(("uhci_device_intr_done: xfer(1)\n"));
 			uhci_dump_tds(xfer);
 			uhci_dump_qh(upipe->u.intr.qhs[0]);
 		}
@@ -1981,7 +1992,7 @@ uhci_remove_intr(sc, n, sqh)
 	for (pqh = vf->hqh; pqh->qh->hlink != sqh; pqh = pqh->qh->hlink)
 #if defined(DIAGNOSTIC) || defined(UHCI_DEBUG)		
 		if (pqh->qh->qh_hlink & UHCI_PTR_T) {
-			printf("uhci_remove_intr: QH not found\n");
+			DPRINTF(("uhci_remove_intr: QH not found\n"));
 			return;
 		}
 #else