From owner-p4-projects@FreeBSD.ORG Sun Jan 13 15:11:35 2008 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 88B2216A41B; Sun, 13 Jan 2008 15:11:35 +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 4E15A16A419 for ; Sun, 13 Jan 2008 15:11:35 +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 4833613C455 for ; Sun, 13 Jan 2008 15:11:35 +0000 (UTC) (envelope-from hselasky@FreeBSD.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.1/8.14.1) with ESMTP id m0DFBZnh026562 for ; Sun, 13 Jan 2008 15:11:35 GMT (envelope-from hselasky@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.1/8.14.1/Submit) id m0DFBZri026556 for perforce@freebsd.org; Sun, 13 Jan 2008 15:11:35 GMT (envelope-from hselasky@FreeBSD.org) Date: Sun, 13 Jan 2008 15:11:35 GMT Message-Id: <200801131511.m0DFBZri026556@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 133193 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: Sun, 13 Jan 2008 15:11:35 -0000 http://perforce.freebsd.org/chv.cgi?CH=133193 Change 133193 by hselasky@hselasky_laptop001 on 2008/01/13 15:10:39 EHCI bugfix. Fix two "off by one" issues in the isochronous endpoint routines for HIGH and FULL speed alike the ones recently found in the OHCI and UHCI driver. Basically there are two issues: 1) don't call usbd_get_page() when the buffer length is zero. 2) don't call usbd_get_page() with an offset equal to the buffer size. Use buffer size minus one if not case 1. This issue does not affect any non-isochronous endpoints. Affected files ... .. //depot/projects/usb/src/sys/dev/usb/ehci.c#70 edit Differences ... ==== //depot/projects/usb/src/sys/dev/usb/ehci.c#70 (text+ko) ==== @@ -2474,7 +2474,6 @@ nframes = xfer->nframes; buf_offset = 0; - usbd_get_page(xfer->frbuffers + 0, buf_offset, &buf_res); plen = xfer->frlengths; @@ -2524,12 +2523,20 @@ */ sa = usbd_fs_isoc_schedule_alloc(fss, *plen); + if (*plen) { + /* only call "usbd_get_page()" when we have a non-zero length */ + usbd_get_page(xfer->frbuffers + 0, buf_offset, &buf_res); td->sitd_bp[0] = htole32(buf_res.physaddr); - buf_offset += *plen; - usbd_get_page(xfer->frbuffers + 0, buf_offset, &buf_res); - - temp = buf_res.physaddr & (~0xFFF); + /* NOTE: We need to subtract one from the + * offset so that we are on a valid page! + */ + usbd_get_page(xfer->frbuffers + 0, buf_offset-1, &buf_res); + temp = buf_res.physaddr & ~0xFFF; + } else { + td->sitd_bp[0] = 0; + temp = 0; + } if (UE_GET_DIR(xfer->endpoint) == UE_DIR_OUT) { tlen = *plen; @@ -2696,8 +2703,10 @@ uint32_t buf_offset; uint32_t nframes; uint32_t *plen; + uint32_t itd_offset[8+1]; + uint8_t x; + uint8_t td_no; uint8_t page_no; - uint8_t td_no; #ifdef USB_DEBUG uint8_t once = 1; @@ -2752,10 +2761,6 @@ nframes = xfer->nframes; buf_offset = 0; - usbd_get_page(xfer->frbuffers + 0, buf_offset, &buf_res); - - page_addr = buf_res.physaddr & ~0xFFF; - page_no = 0; td_no = 0; plen = xfer->frlengths; @@ -2793,78 +2798,70 @@ #endif *plen = xfer->max_frame_size; } - if (td_no == 0) { + + status = (EHCI_ITD_SET_LEN(*plen) | + EHCI_ITD_ACTIVE | + EHCI_ITD_SET_PG(0)); + td->itd_status[td_no] = htole32(status); + itd_offset[td_no] = buf_offset; + buf_offset += *plen; + plen++; + td_no++; + + if ((td_no == 8) || (nframes == 0)) { + + /* the rest of the transfers are not active, if any */ + for (x = td_no; x != 8; x++) { + td->itd_status[x] = 0; /* not active */ + } + + /* check if there is any data to be transferred */ + if (itd_offset[0] != buf_offset) { + page_no = 0; + itd_offset[td_no] = buf_offset; + + /* get first page offset */ + usbd_get_page(xfer->frbuffers + 0, itd_offset[0], &buf_res); + /* get page address */ + page_addr = buf_res.physaddr & ~0xFFF; - /* update page address */ - td->itd_bp[page_no] &= htole32(0xFFF); - td->itd_bp[page_no] |= htole32(page_addr); + for (x = 0; x != td_no; x++) { + /* set page number and page offset */ + status = (EHCI_ITD_SET_PG(page_no) | + (buf_res.physaddr & 0xFFF)); + td->itd_status[x] |= htole32(status); - if (nframes < 7) { - /* - * clear all status in case some are not - * initialized + /* get next page offset */ + if (itd_offset[x+1] == buf_offset) { + /* + * We subtract one so that we don't go + * off the last page! */ - td->itd_status[0] = 0; - td->itd_status[1] = 0; - td->itd_status[2] = 0; - td->itd_status[3] = 0; - td->itd_status[4] = 0; - td->itd_status[5] = 0; - td->itd_status[6] = 0; - td->itd_status[7] = 0; + usbd_get_page(xfer->frbuffers + 0, buf_offset-1, &buf_res); + } else { + usbd_get_page(xfer->frbuffers + 0, itd_offset[x+1], &buf_res); } - } - /* compute status */ - if (nframes == 0) { - status = - (EHCI_ITD_SET_LEN(*plen) | - EHCI_ITD_ACTIVE | - EHCI_ITD_IOC | - EHCI_ITD_SET_PG(page_no) | - (buf_res.physaddr & 0xFFF)); - } else { - status = - (EHCI_ITD_SET_LEN(*plen) | - EHCI_ITD_ACTIVE | - EHCI_ITD_SET_PG(page_no) | - (buf_res.physaddr & 0xFFF)); - } - buf_offset += *plen; - usbd_get_page(xfer->frbuffers + 0, buf_offset, &buf_res); - - if ((buf_res.physaddr ^ page_addr) & ~0xFFF) { - /* new page needed */ - page_addr = buf_res.physaddr & ~0xFFF; - page_no++; - - if (page_no < 7) { + /* check if we need a new page */ + if ((buf_res.physaddr ^ page_addr) & ~0xFFF) { + /* new page needed */ + page_addr = buf_res.physaddr & ~0xFFF; + if (page_no == 6) { + panic("%s: too many pages\n", __FUNCTION__); + } + page_no++; /* update page address */ td->itd_bp[page_no] &= htole32(0xFFF); td->itd_bp[page_no] |= htole32(page_addr); } - } - if (page_no < 7) { - /* activate the transfer */ - td->itd_status[td_no] = htole32(status); - } else { - /* pretend that the transfer has finished */ - td->itd_status[td_no] = (nframes == 0) ? - htole32(EHCI_ITD_IOC) : 0; -#ifdef USB_DEBUG - if (once) { - once = 0; - printf("%s: isoc limit reached! " - "Max %d bytes per 8 frames. Frame skipped.\n", - __FUNCTION__, (6 << 12)); + } + } + + /* set IOC bit if we are complete */ + if (nframes == 0) { + td->itd_status[7] |= htole32(EHCI_ITD_IOC); } -#endif - } - - plen++; - td_no++; - if ((td_no == 8) || (nframes == 0)) { usbd_pc_cpu_flush(td->page_cache); #ifdef USB_DEBUG if (ehcidebug > 15) { @@ -2876,7 +2873,6 @@ EHCI_APPEND_HS_TD(td, *pp_last); pp_last++; - page_no = 0; td_no = 0; td_last = td; td = td->obj_next;