Date: Thu, 1 Oct 2009 15:30:12 GMT From: Hans Petter Selasky <hselasky@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 169085 for review Message-ID: <200910011530.n91FUCc0025737@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=169085 Change 169085 by hselasky@hselasky_laptop001 on 2009/10/01 15:29:25 USB controller: (EHCI Hardware BUG workaround) The EHCI HW can use the qtd_next field instead of qtd_altnext when a short packet is received. This contradicts what is stated in the EHCI datasheet. Also the total-bytes field in the status field of the following TD gets corrupted upon reception of a short packet! We work this around in software by not queueing more than one job/TD at a time of up to 16Kbytes! The bug has been seen on multiple INTEL based EHCI chips. Other vendors have not been tested yet. - Applications using /dev/usb/X.Y.Z, where Z is non-zero are affected, but not applications using LibUSB v0.1, v1.2 and v2.0. - Mass Storage is affected. Affected files ... .. //depot/projects/usb/src/sys/dev/usb/controller/ehci.c#34 edit Differences ... ==== //depot/projects/usb/src/sys/dev/usb/controller/ehci.c#34 (text+ko) ==== @@ -131,6 +131,7 @@ uint8_t auto_data_toggle; uint8_t setup_alt_next; uint8_t last_frame; + uint8_t can_use_next; }; void @@ -1207,11 +1208,6 @@ xfer->td_transfer_cache = td; - /* update data toggle */ - - xfer->endpoint->toggle_next = - (status & EHCI_QTD_TOGGLE_MASK) ? 1 : 0; - #if USB_DEBUG if (status & EHCI_QTD_STATERRS) { DPRINTFN(11, "error, addr=%d, endpt=0x%02x, frame=0x%02x" @@ -1235,6 +1231,9 @@ static void ehci_non_isoc_done(struct usb_xfer *xfer) { + ehci_softc_t *sc = EHCI_BUS2SC(xfer->xroot->bus); + ehci_qh_t *qh; + uint32_t status; usb_error_t err = 0; DPRINTFN(13, "xfer=%p endpoint=%p transfer done\n", @@ -1248,6 +1247,17 @@ } #endif + /* extract data toggle directly from the QH's overlay area */ + + qh = xfer->qh_start[xfer->flags_int.curr_dma_set]; + + usb_pc_cpu_invalidate(qh->page_cache); + + status = hc32toh(sc, qh->qh_qtd.qtd_status); + + xfer->endpoint->toggle_next = + (status & EHCI_QTD_TOGGLE_MASK) ? 1 : 0; + /* reset scanner */ xfer->td_transfer_cache = xfer->td_transfer_first; @@ -1348,6 +1358,7 @@ } } else { ehci_qtd_t *td; + ehci_qh_t *qh; /* non-isochronous transfer */ @@ -1357,16 +1368,35 @@ */ td = xfer->td_transfer_cache; + qh = xfer->qh_start[xfer->flags_int.curr_dma_set]; + + usb_pc_cpu_invalidate(qh->page_cache); + + status = hc32toh(sc, qh->qh_qtd.qtd_status); + if (status & EHCI_QTD_ACTIVE) { + /* transfer is pending */ + goto done; + } + while (1) { usb_pc_cpu_invalidate(td->page_cache); status = hc32toh(sc, td->qtd_status); /* - * if there is an active TD the transfer isn't done + * Check if there is an active TD which + * indicates that the transfer isn't done. */ if (status & EHCI_QTD_ACTIVE) { /* update cache */ - xfer->td_transfer_cache = td; + if (xfer->td_transfer_cache != td) { + xfer->td_transfer_cache = td; + if (qh->qh_qtd.qtd_next & + htohc32(sc, EHCI_LINK_TERMINATE)) { + /* XXX - manually advance to next frame */ + qh->qh_qtd.qtd_next = td->qtd_self; + usb_pc_cpu_flush(td->page_cache); + } + } goto done; } /* @@ -1545,7 +1575,6 @@ ehci_qtd_t *td; ehci_qtd_t *td_next; ehci_qtd_t *td_alt_next; - uint32_t qtd_altnext; uint32_t buf_offset; uint32_t average; uint32_t len_old; @@ -1554,7 +1583,6 @@ uint8_t precompute; terminate = htohc32(temp->sc, EHCI_LINK_TERMINATE); - qtd_altnext = terminate; td_alt_next = NULL; buf_offset = 0; shortpkt_old = temp->shortpkt; @@ -1612,7 +1640,8 @@ td->qtd_status = temp->qtd_status | - htohc32(temp->sc, EHCI_QTD_SET_BYTES(average)); + htohc32(temp->sc, EHCI_QTD_IOC | + EHCI_QTD_SET_BYTES(average)); if (average == 0) { @@ -1687,11 +1716,23 @@ td->qtd_buffer_hi[x] = 0; } - if (td_next) { - /* link the current TD with the next one */ - td->qtd_next = td_next->qtd_self; + if (temp->can_use_next) { + if (td_next) { + /* link the current TD with the next one */ + td->qtd_next = td_next->qtd_self; + } + } else { + /* + * BUG WARNING: The EHCI HW can use the + * qtd_next field instead of qtd_altnext when + * a short packet is received! We work this + * around in software by not queueing more + * than one job/TD at a time! + */ + td->qtd_next = terminate; } - td->qtd_altnext = qtd_altnext; + + td->qtd_altnext = terminate; td->alt_next = td_alt_next; usb_pc_cpu_flush(td->page_cache); @@ -1703,15 +1744,9 @@ /* setup alt next pointer, if any */ if (temp->last_frame) { td_alt_next = NULL; - qtd_altnext = terminate; } else { /* we use this field internally */ td_alt_next = td_next; - if (temp->setup_alt_next) { - qtd_altnext = td_next->qtd_self; - } else { - qtd_altnext = terminate; - } } /* restore */ @@ -1756,6 +1791,8 @@ temp.qtd_status = 0; temp.last_frame = 0; temp.setup_alt_next = xfer->flags_int.short_frames_ok; + temp.can_use_next = (xfer->flags_int.control_xfr || + (UE_GET_DIR(xfer->endpointno) == UE_DIR_OUT)); if (xfer->flags_int.control_xfr) { if (xfer->endpoint->toggle_next) { @@ -1889,7 +1926,6 @@ /* the last TD terminates the transfer: */ td->qtd_next = htohc32(temp.sc, EHCI_LINK_TERMINATE); td->qtd_altnext = htohc32(temp.sc, EHCI_LINK_TERMINATE); - td->qtd_status |= htohc32(temp.sc, EHCI_QTD_IOC); usb_pc_cpu_flush(td->page_cache);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200910011530.n91FUCc0025737>