From owner-p4-projects@FreeBSD.ORG Thu Oct 1 15:30:13 2009 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id F376E1065679; Thu, 1 Oct 2009 15:30:12 +0000 (UTC) Delivered-To: perforce@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B8C581065670 for ; Thu, 1 Oct 2009 15:30:12 +0000 (UTC) (envelope-from hselasky@FreeBSD.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id A767F8FC0A for ; Thu, 1 Oct 2009 15:30:12 +0000 (UTC) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.3/8.14.3) with ESMTP id n91FUCRO025739 for ; Thu, 1 Oct 2009 15:30:12 GMT (envelope-from hselasky@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id n91FUCc0025737 for perforce@freebsd.org; Thu, 1 Oct 2009 15:30:12 GMT (envelope-from hselasky@FreeBSD.org) Date: Thu, 1 Oct 2009 15:30:12 GMT Message-Id: <200910011530.n91FUCc0025737@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to hselasky@FreeBSD.org using -f From: Hans Petter Selasky To: Perforce Change Reviews Cc: Subject: PERFORCE change 169085 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Oct 2009 15:30:13 -0000 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);