From 10d6a2ddbb724c01ef1314f818671049f0708816 Mon Sep 17 00:00:00 2001 From: wpaul Date: Fri, 28 Jan 2000 02:15:31 +0000 Subject: [PATCH] Fix a bug in the uhci driver that breaks large bulk IN transfers. The uhci_check_intr() routine needs to be more careful about deciding when the end of a transfer has been detected. This allows me to remove the nasty workaround code from if_aue and if_cue. Receive performance is now much better for these adapters (500KB/sec vs. 350KB/sec). Also removed unused KUE_CUTOFF define from if_kuereg.h. Submitted by: Lennart Augustsson Reviewed by: n_hibma --- sys/dev/usb/if_aue.c | 40 +++------------------------------------- sys/dev/usb/if_auereg.h | 2 -- sys/dev/usb/if_cue.c | 31 +++---------------------------- sys/dev/usb/if_cuereg.h | 1 - sys/dev/usb/if_kuereg.h | 1 - sys/dev/usb/uhci.c | 21 ++++++++++++++++----- 6 files changed, 22 insertions(+), 74 deletions(-) diff --git a/sys/dev/usb/if_aue.c b/sys/dev/usb/if_aue.c index f321a63443ba..4a9303efb9ad 100644 --- a/sys/dev/usb/if_aue.c +++ b/sys/dev/usb/if_aue.c @@ -813,7 +813,6 @@ static int aue_rx_list_init(sc) c = &cd->aue_rx_chain[i]; c->aue_sc = sc; c->aue_idx = i; - c->aue_accum = 0; if (aue_newbuf(sc, c, NULL) == ENOBUFS) return(ENOBUFS); if (c->aue_xfer == NULL) { @@ -913,7 +912,7 @@ static void aue_rxstart(ifp) /* Setup new transfer. */ usbd_setup_xfer(c->aue_xfer, sc->aue_ep[AUE_ENDPT_RX], - c, mtod(c->aue_mbuf, char *), AUE_CUTOFF, USBD_SHORT_XFER_OK, + c, mtod(c->aue_mbuf, char *), AUE_BUFSZ, USBD_SHORT_XFER_OK, USBD_NO_TIMEOUT, aue_rxeof); usbd_transfer(c->aue_xfer); @@ -923,15 +922,6 @@ static void aue_rxstart(ifp) /* * A frame has been uploaded: pass the resulting mbuf chain up to * the higher level protocols. - * - * Grrr. Receiving transfers larger than about 1152 bytes sometimes - * doesn't work. We get an incomplete frame. In order to avoid - * this, we queue up RX transfers that are shorter than a full sized - * frame. If the received frame is larger than our transfer size, - * we snag the rest of the data using a second transfer. Does this - * hurt performance? Yes. But after fighting with this stupid thing - * for three days, I'm willing to settle. I'd rather have reliable - * receive performance that fast but spotty performance. */ static void aue_rxeof(xfer, priv, status) usbd_xfer_handle xfer; @@ -964,15 +954,6 @@ static void aue_rxeof(xfer, priv, status) usbd_get_xfer_status(xfer, NULL, NULL, &total_len, NULL); - /* - * See if we've already accumulated some data from - * a previous transfer. - */ - if (c->aue_accum) { - total_len += c->aue_accum; - c->aue_accum = 0; - } - if (total_len <= 4 + ETHER_CRC_LEN) { ifp->if_ierrors++; goto done; @@ -984,21 +965,6 @@ static void aue_rxeof(xfer, priv, status) /* Turn off all the non-error bits in the rx status word. */ r.aue_rxstat &= AUE_RXSTAT_MASK; - /* - * Check to see if this is just the first chunk of a - * split transfer. We really need a more reliable way - * to detect this. - */ - if (total_len == AUE_CUTOFF && r.aue_pktlen != (AUE_CUTOFF - 4)) { - c->aue_accum = AUE_CUTOFF; - usbd_setup_xfer(xfer, sc->aue_ep[AUE_ENDPT_RX], - c, mtod(c->aue_mbuf, char *) + AUE_CUTOFF, - AUE_CUTOFF, USBD_SHORT_XFER_OK, - USBD_NO_TIMEOUT, aue_rxeof); - usbd_transfer(xfer); - return; - } - if (r.aue_rxstat) { ifp->if_ierrors++; goto done; @@ -1019,7 +985,7 @@ static void aue_rxeof(xfer, priv, status) /* Setup new transfer. */ usbd_setup_xfer(xfer, sc->aue_ep[AUE_ENDPT_RX], - c, mtod(c->aue_mbuf, char *), AUE_CUTOFF, USBD_SHORT_XFER_OK, + c, mtod(c->aue_mbuf, char *), AUE_BUFSZ, USBD_SHORT_XFER_OK, USBD_NO_TIMEOUT, aue_rxeof); usbd_transfer(xfer); @@ -1298,7 +1264,7 @@ static void aue_init(xsc) for (i = 0; i < AUE_RX_LIST_CNT; i++) { c = &sc->aue_cdata.aue_rx_chain[i]; usbd_setup_xfer(c->aue_xfer, sc->aue_ep[AUE_ENDPT_RX], - c, mtod(c->aue_mbuf, char *), AUE_CUTOFF, + c, mtod(c->aue_mbuf, char *), AUE_BUFSZ, USBD_SHORT_XFER_OK, USBD_NO_TIMEOUT, aue_rxeof); usbd_transfer(c->aue_xfer); } diff --git a/sys/dev/usb/if_auereg.h b/sys/dev/usb/if_auereg.h index 20caa52e7466..9e4d5467798a 100644 --- a/sys/dev/usb/if_auereg.h +++ b/sys/dev/usb/if_auereg.h @@ -217,7 +217,6 @@ struct aue_chain { usbd_xfer_handle aue_xfer; char *aue_buf; struct mbuf *aue_mbuf; - int aue_accum; int aue_idx; }; @@ -252,6 +251,5 @@ struct aue_softc { #define AUE_TIMEOUT 1000 #define ETHER_ALIGN 2 #define AUE_BUFSZ 1536 -#define AUE_CUTOFF 1088 #define AUE_MIN_FRAMELEN 60 #define AUE_INTR_INTERVAL 100 /* ms */ diff --git a/sys/dev/usb/if_cue.c b/sys/dev/usb/if_cue.c index 7cbf68ffbd6c..55d28e60e04b 100644 --- a/sys/dev/usb/if_cue.c +++ b/sys/dev/usb/if_cue.c @@ -645,7 +645,6 @@ static int cue_rx_list_init(sc) c = &cd->cue_rx_chain[i]; c->cue_sc = sc; c->cue_idx = i; - c->cue_accum = 0; if (cue_newbuf(sc, c, NULL) == ENOBUFS) return(ENOBUFS); if (c->cue_xfer == NULL) { @@ -700,7 +699,7 @@ static void cue_rxstart(ifp) /* Setup new transfer. */ usbd_setup_xfer(c->cue_xfer, sc->cue_ep[CUE_ENDPT_RX], - c, mtod(c->cue_mbuf, char *), CUE_CUTOFF, USBD_SHORT_XFER_OK, + c, mtod(c->cue_mbuf, char *), CUE_BUFSZ, USBD_SHORT_XFER_OK, USBD_NO_TIMEOUT, cue_rxeof); usbd_transfer(c->cue_xfer); @@ -742,33 +741,9 @@ static void cue_rxeof(xfer, priv, status) usbd_get_xfer_status(xfer, NULL, NULL, &total_len, NULL); - /* - * See if we've already accumulated some data from - * a previous transfer. - */ - if (c->cue_accum) { - total_len += c->cue_accum; - c->cue_accum = 0; - } - m = c->cue_mbuf; len = *mtod(m, u_int16_t *); - /* - * Check to see if this is just the first chunk of a - * split transfer. We really need a more reliable way - * to detect this. - */ - if (len != total_len && total_len == CUE_CUTOFF) { - c->cue_accum = CUE_CUTOFF; - usbd_setup_xfer(xfer, sc->cue_ep[CUE_ENDPT_RX], - c, mtod(c->cue_mbuf, char *) + CUE_CUTOFF, - CUE_CUTOFF, USBD_SHORT_XFER_OK, - USBD_NO_TIMEOUT, cue_rxeof); - usbd_transfer(xfer); - return; - } - /* No errors; receive the packet. */ total_len = len; @@ -789,7 +764,7 @@ static void cue_rxeof(xfer, priv, status) done: /* Setup new transfer. */ usbd_setup_xfer(c->cue_xfer, sc->cue_ep[CUE_ENDPT_RX], - c, mtod(c->cue_mbuf, char *), CUE_CUTOFF, USBD_SHORT_XFER_OK, + c, mtod(c->cue_mbuf, char *), CUE_BUFSZ, USBD_SHORT_XFER_OK, USBD_NO_TIMEOUT, cue_rxeof); usbd_transfer(c->cue_xfer); @@ -1046,7 +1021,7 @@ static void cue_init(xsc) for (i = 0; i < CUE_RX_LIST_CNT; i++) { c = &sc->cue_cdata.cue_rx_chain[i]; usbd_setup_xfer(c->cue_xfer, sc->cue_ep[CUE_ENDPT_RX], - c, mtod(c->cue_mbuf, char *), CUE_CUTOFF, + c, mtod(c->cue_mbuf, char *), CUE_BUFSZ, USBD_SHORT_XFER_OK, USBD_NO_TIMEOUT, cue_rxeof); usbd_transfer(c->cue_xfer); } diff --git a/sys/dev/usb/if_cuereg.h b/sys/dev/usb/if_cuereg.h index 19e6a43eb117..ca90383ccf49 100644 --- a/sys/dev/usb/if_cuereg.h +++ b/sys/dev/usb/if_cuereg.h @@ -118,7 +118,6 @@ #define CUE_TIMEOUT 1000 #define ETHER_ALIGN 2 #define CUE_BUFSZ 1536 -#define CUE_CUTOFF 1088 #define CUE_MIN_FRAMELEN 60 #define CUE_RX_FRAMES 1 #define CUE_TX_FRAMES 1 diff --git a/sys/dev/usb/if_kuereg.h b/sys/dev/usb/if_kuereg.h index 97b629a74bc6..5a315b172d62 100644 --- a/sys/dev/usb/if_kuereg.h +++ b/sys/dev/usb/if_kuereg.h @@ -112,7 +112,6 @@ struct kue_ether_desc { #define KUE_TIMEOUT 1000 #define ETHER_ALIGN 2 #define KUE_BUFSZ 1536 -#define KUE_CUTOFF 1088 #define KUE_MIN_FRAMELEN 60 #define KUE_RX_LIST_CNT 1 diff --git a/sys/dev/usb/uhci.c b/sys/dev/usb/uhci.c index 8d754b82835e..efc86a231af3 100644 --- a/sys/dev/usb/uhci.c +++ b/sys/dev/usb/uhci.c @@ -1062,16 +1062,27 @@ uhci_check_intr(sc, ii) DPRINTFN(15, ("uhci_check_intr: active ii=%p\n", ii)); for (std = ii->stdstart; std != lstd; std = std->link.std) { status = LE(std->td.td_status); - if ((status & UHCI_TD_STALLED) || - (status & (UHCI_TD_SPD | UHCI_TD_ACTIVE)) == - UHCI_TD_SPD) + /* If there's an active TD the xfer isn't done. */ + if (status & UHCI_TD_ACTIVE) + break; + /* Any kind of error makes the xfer done. */ + if (status & UHCI_TD_STALLED) + goto done; + /* + * We want short packets, + * and it is short: it's done + */ + if ((status & UHCI_TD_SPD) && + UHCI_TD_GET_ACTLEN(status) < + UHCI_TD_GET_MAXLEN(LE(std->td.td_token))) goto done; } DPRINTFN(15, ("uhci_check_intr: ii=%p std=%p still active\n", - ii, ii->stdstart)); + ii, ii->stdstart)); return; } - done: +done: + usb_untimeout(uhci_timeout, ii, ii->timeout_handle); uhci_idone(ii); }