Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. =================================================================== RCS file: /ftp/cvs/cvsroot/src/sys/dev/usb/ehci.c,v rcsdiff: /ftp/cvs/cvsroot/src/sys/dev/usb/ehci.c,v: warning: Unknown phrases like `commitid ...;' are present. retrieving revision 1.268 retrieving revision 1.268.2.1 diff -u -p -r1.268 -r1.268.2.1 --- src/sys/dev/usb/ehci.c 2019/12/31 18:11:18 1.268 +++ src/sys/dev/usb/ehci.c 2020/02/29 20:19:16 1.268.2.1 @@ -1,4 +1,4 @@ -/* $NetBSD: ehci.c,v 1.268 2019/12/31 18:11:18 skrll Exp $ */ +/* $NetBSD: ehci.c,v 1.268.2.1 2020/02/29 20:19:16 ad Exp $ */ /* * Copyright (c) 2004-2012 The NetBSD Foundation, Inc. @@ -53,7 +53,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: ehci.c,v 1.268 2019/12/31 18:11:18 skrll Exp $"); +__KERNEL_RCSID(0, "$NetBSD: ehci.c,v 1.268.2.1 2020/02/29 20:19:16 ad Exp $"); #include "ohci.h" #include "uhci.h" @@ -166,8 +166,6 @@ Static void ehci_check_itd_intr(ehci_so Static void ehci_check_sitd_intr(ehci_softc_t *, struct ehci_xfer *, ex_completeq_t *); Static void ehci_idone(struct ehci_xfer *, ex_completeq_t *); -Static void ehci_timeout(void *); -Static void ehci_timeout_task(void *); Static void ehci_intrlist_timeout(void *); Static void ehci_doorbell(void *); Static void ehci_pcd(void *); @@ -177,6 +175,7 @@ Static struct usbd_xfer * Static void ehci_freex(struct usbd_bus *, struct usbd_xfer *); Static void ehci_get_lock(struct usbd_bus *, kmutex_t **); +Static bool ehci_dying(struct usbd_bus *); Static int ehci_roothub_ctrl(struct usbd_bus *, usb_device_request_t *, void *, int); @@ -278,7 +277,7 @@ Static void ehci_set_qh_qtd(ehci_soft_q Static void ehci_sync_hc(ehci_softc_t *); Static void ehci_close_pipe(struct usbd_pipe *, ehci_soft_qh_t *); -Static void ehci_abort_xfer(struct usbd_xfer *, usbd_status); +Static void ehci_abortx(struct usbd_xfer *); #ifdef EHCI_DEBUG Static ehci_softc_t *theehci; @@ -319,6 +318,8 @@ Static const struct usbd_bus_methods ehc .ubm_dopoll = ehci_poll, .ubm_allocx = ehci_allocx, .ubm_freex = ehci_freex, + .ubm_abortx = ehci_abortx, + .ubm_dying = ehci_dying, .ubm_getlock = ehci_get_lock, .ubm_rhctrl = ehci_roothub_ctrl, }; @@ -784,6 +785,7 @@ ehci_pcd(void *addr) /* Just ignore the change. */ goto done; } + KASSERT(xfer->ux_status == USBD_IN_PROGRESS); p = xfer->ux_buf; m = uimin(sc->sc_noport, xfer->ux_length * 8 - 1); @@ -1046,24 +1048,11 @@ ehci_idone(struct ehci_xfer *ex, ex_comp DPRINTF("ex=%#jx", (uintptr_t)ex, 0, 0, 0); /* - * If software has completed it, either by cancellation - * or timeout, drop it on the floor. + * Try to claim this xfer for completion. If it has already + * completed or aborted, drop it on the floor. */ - if (xfer->ux_status != USBD_IN_PROGRESS) { - KASSERT(xfer->ux_status == USBD_CANCELLED || - xfer->ux_status == USBD_TIMEOUT); - DPRINTF("aborted xfer=%#jx", (uintptr_t)xfer, 0, 0, 0); + if (!usbd_xfer_trycomplete(xfer)) return; - } - - /* - * Cancel the timeout and the task, which have not yet - * run. If they have already fired, at worst they are - * waiting for the lock. They will see that the xfer - * is no longer in progress and give up. - */ - callout_stop(&xfer->ux_callout); - usb_rem_task(xfer->ux_pipe->up_dev, &xfer->ux_aborttask); #ifdef DIAGNOSTIC #ifdef EHCI_DEBUG @@ -1538,9 +1527,6 @@ ehci_allocx(struct usbd_bus *bus, unsign if (xfer != NULL) { memset(xfer, 0, sizeof(struct ehci_xfer)); - /* Initialise this always so we can call remove on it. */ - usb_init_task(&xfer->ux_aborttask, ehci_timeout_task, xfer, - USB_TASKQ_MPSAFE); #ifdef DIAGNOSTIC struct ehci_xfer *ex = EHCI_XFER2EXFER(xfer); ex->ex_isdone = true; @@ -1568,6 +1554,14 @@ ehci_freex(struct usbd_bus *bus, struct pool_cache_put(sc->sc_xferpool, xfer); } +Static bool +ehci_dying(struct usbd_bus *bus) +{ + struct ehci_softc *sc = EHCI_BUS2SC(bus); + + return sc->sc_dying; +} + Static void ehci_get_lock(struct usbd_bus *bus, kmutex_t **lock) { @@ -2729,7 +2723,9 @@ ehci_root_intr_start(struct usbd_xfer *x if (!polling) mutex_enter(&sc->sc_lock); + KASSERT(sc->sc_intrxfer == NULL); sc->sc_intrxfer = xfer; + xfer->ux_status = USBD_IN_PROGRESS; if (!polling) mutex_exit(&sc->sc_lock); @@ -2745,8 +2741,16 @@ ehci_root_intr_abort(struct usbd_xfer *x KASSERT(mutex_owned(&sc->sc_lock)); KASSERT(xfer->ux_pipe->up_intrxfer == xfer); - sc->sc_intrxfer = NULL; + /* If xfer has already completed, nothing to do here. */ + if (sc->sc_intrxfer == NULL) + return; + /* + * Otherwise, sc->sc_intrxfer had better be this transfer. + * Cancel it. + */ + KASSERT(sc->sc_intrxfer == xfer); + KASSERT(xfer->ux_status == USBD_IN_PROGRESS); xfer->ux_status = USBD_CANCELLED; usb_transfer_complete(xfer); } @@ -2755,18 +2759,30 @@ ehci_root_intr_abort(struct usbd_xfer *x Static void ehci_root_intr_close(struct usbd_pipe *pipe) { - ehci_softc_t *sc = EHCI_PIPE2SC(pipe); + ehci_softc_t *sc __diagused = EHCI_PIPE2SC(pipe); EHCIHIST_FUNC(); EHCIHIST_CALLED(); KASSERT(mutex_owned(&sc->sc_lock)); - sc->sc_intrxfer = NULL; + /* + * Caller must guarantee the xfer has completed first, by + * closing the pipe only after normal completion or an abort. + */ + KASSERT(sc->sc_intrxfer == NULL); } Static void ehci_root_intr_done(struct usbd_xfer *xfer) { + struct ehci_softc *sc = EHCI_XFER2SC(xfer); + + KASSERT(mutex_owned(&sc->sc_lock)); + + /* Claim the xfer so it doesn't get completed again. */ + KASSERT(sc->sc_intrxfer == xfer); + KASSERT(xfer->ux_status != USBD_IN_PROGRESS); + sc->sc_intrxfer = NULL; } /************************/ @@ -3200,22 +3216,12 @@ ehci_close_pipe(struct usbd_pipe *pipe, } /* - * Cancel or timeout a device request. We have two cases to deal with - * - * 1) A driver wants to stop scheduled or inflight transfers - * 2) A transfer has timed out - * - * have (partially) happened since the hardware runs concurrently. - * - * Transfer state is protected by the bus lock and we set the transfer status - * as soon as either of the above happens (with bus lock held). - * - * Then we arrange for the hardware to tells us that it is not still + * Arrrange for the hardware to tells us that it is not still * processing the TDs by setting the QH halted bit and wait for the ehci * door bell */ Static void -ehci_abort_xfer(struct usbd_xfer *xfer, usbd_status status) +ehci_abortx(struct usbd_xfer *xfer) { EHCIHIST_FUNC(); EHCIHIST_CALLED(); struct ehci_pipe *epipe = EHCI_XFER2EPIPE(xfer); @@ -3227,45 +3233,14 @@ ehci_abort_xfer(struct usbd_xfer *xfer, uint32_t qhstatus; int hit; - KASSERTMSG((status == USBD_CANCELLED || status == USBD_TIMEOUT), - "invalid status for abort: %d", (int)status); - DPRINTF("xfer=%#jx pipe=%#jx", (uintptr_t)xfer, (uintptr_t)epipe, 0, 0); KASSERT(mutex_owned(&sc->sc_lock)); ASSERT_SLEEPABLE(); - if (status == USBD_CANCELLED) { - /* - * We are synchronously aborting. Try to stop the - * callout and task, but if we can't, wait for them to - * complete. - */ - callout_halt(&xfer->ux_callout, &sc->sc_lock); - usb_rem_task_wait(xfer->ux_pipe->up_dev, &xfer->ux_aborttask, - USB_TASKQ_HC, &sc->sc_lock); - } else { - /* Otherwise, we are timing out. */ - KASSERT(status == USBD_TIMEOUT); - } - - /* - * The xfer cannot have been cancelled already. It is the - * responsibility of the caller of usbd_abort_pipe not to try - * to abort a pipe multiple times, whether concurrently or - * sequentially. - */ - KASSERT(xfer->ux_status != USBD_CANCELLED); - - /* Only the timeout, which runs only once, can time it out. */ - KASSERT(xfer->ux_status != USBD_TIMEOUT); - - /* If anyone else beat us, we're done. */ - if (xfer->ux_status != USBD_IN_PROGRESS) - return; - - /* We beat everyone else. Claim the status. */ - xfer->ux_status = status; + KASSERTMSG((xfer->ux_status == USBD_CANCELLED || + xfer->ux_status == USBD_TIMEOUT), + "bad abort status: %d", xfer->ux_status); /* * If we're dying, skip the hardware action and just notify the @@ -3473,43 +3448,6 @@ dying: KASSERT(mutex_owned(&sc->sc_lock)); } -Static void -ehci_timeout(void *addr) -{ - EHCIHIST_FUNC(); EHCIHIST_CALLED(); - struct usbd_xfer *xfer = addr; - ehci_softc_t *sc = EHCI_XFER2SC(xfer); - struct usbd_device *dev = xfer->ux_pipe->up_dev; - - DPRINTF("xfer %#jx", (uintptr_t)xfer, 0, 0, 0); -#ifdef EHCI_DEBUG - if (ehcidebug >= 2) { - struct usbd_pipe *pipe = xfer->ux_pipe; - usbd_dump_pipe(pipe); - } -#endif - - mutex_enter(&sc->sc_lock); - if (!sc->sc_dying && xfer->ux_status == USBD_IN_PROGRESS) - usb_add_task(dev, &xfer->ux_aborttask, USB_TASKQ_HC); - mutex_exit(&sc->sc_lock); -} - -Static void -ehci_timeout_task(void *addr) -{ - struct usbd_xfer *xfer = addr; - ehci_softc_t *sc = EHCI_XFER2SC(xfer); - - EHCIHIST_FUNC(); EHCIHIST_CALLED(); - - DPRINTF("xfer=%#jx", (uintptr_t)xfer, 0, 0, 0); - - mutex_enter(&sc->sc_lock); - ehci_abort_xfer(xfer, USBD_TIMEOUT); - mutex_exit(&sc->sc_lock); -} - /************************/ Static int @@ -3751,10 +3689,7 @@ ehci_device_ctrl_start(struct usbd_xfer /* Insert qTD in QH list - also does usb_syncmem(sqh) */ ehci_set_qh_qtd(sqh, setup); - if (xfer->ux_timeout && !sc->sc_bus.ub_usepolling) { - callout_reset(&xfer->ux_callout, mstohz(xfer->ux_timeout), - ehci_timeout, xfer); - } + usbd_xfer_schedule_timeout(xfer); ehci_add_intr_list(sc, exfer); xfer->ux_status = USBD_IN_PROGRESS; if (!polling) @@ -3805,7 +3740,7 @@ ehci_device_ctrl_abort(struct usbd_xfer EHCIHIST_FUNC(); EHCIHIST_CALLED(); DPRINTF("xfer=%#jx", (uintptr_t)xfer, 0, 0, 0); - ehci_abort_xfer(xfer, USBD_CANCELLED); + usbd_xfer_abort(xfer); } /* Close a device control pipe. */ @@ -3813,7 +3748,7 @@ Static void ehci_device_ctrl_close(struct usbd_pipe *pipe) { ehci_softc_t *sc = EHCI_PIPE2SC(pipe); - /*struct ehci_pipe *epipe = EHCI_PIPE2EPIPE(pipe);*/ + struct ehci_pipe * const epipe = EHCI_PIPE2EPIPE(pipe); EHCIHIST_FUNC(); EHCIHIST_CALLED(); @@ -3822,6 +3757,8 @@ ehci_device_ctrl_close(struct usbd_pipe DPRINTF("pipe=%#jx", (uintptr_t)pipe, 0, 0, 0); ehci_close_pipe(pipe, sc->sc_async_head); + + usb_freemem(&sc->sc_bus, &epipe->ctrl.reqdma); } /* @@ -3946,15 +3883,13 @@ ehci_device_bulk_start(struct usbd_xfer DPRINTFN(5, "--- dump end ---", 0, 0, 0, 0); #endif - usb_syncmem(&xfer->ux_dmabuf, 0, xfer->ux_length, - isread ? BUS_DMASYNC_PREREAD : BUS_DMASYNC_PREWRITE); + if (xfer->ux_length) + usb_syncmem(&xfer->ux_dmabuf, 0, xfer->ux_length, + isread ? BUS_DMASYNC_PREREAD : BUS_DMASYNC_PREWRITE); /* also does usb_syncmem(sqh) */ ehci_set_qh_qtd(sqh, exfer->ex_sqtdstart); - if (xfer->ux_timeout && !sc->sc_bus.ub_usepolling) { - callout_reset(&xfer->ux_callout, mstohz(xfer->ux_timeout), - ehci_timeout, xfer); - } + usbd_xfer_schedule_timeout(xfer); ehci_add_intr_list(sc, exfer); xfer->ux_status = USBD_IN_PROGRESS; if (!polling) @@ -3985,7 +3920,7 @@ ehci_device_bulk_abort(struct usbd_xfer EHCIHIST_FUNC(); EHCIHIST_CALLED(); DPRINTF("xfer %#jx", (uintptr_t)xfer, 0, 0, 0); - ehci_abort_xfer(xfer, USBD_CANCELLED); + usbd_xfer_abort(xfer); } /* @@ -4020,8 +3955,9 @@ ehci_device_bulk_done(struct usbd_xfer * KASSERT(sc->sc_bus.ub_usepolling || mutex_owned(&sc->sc_lock)); - usb_syncmem(&xfer->ux_dmabuf, 0, xfer->ux_length, - rd ? BUS_DMASYNC_POSTREAD : BUS_DMASYNC_POSTWRITE); + if (xfer->ux_length) + usb_syncmem(&xfer->ux_dmabuf, 0, xfer->ux_length, + rd ? BUS_DMASYNC_POSTREAD : BUS_DMASYNC_POSTWRITE); DPRINTF("length=%jd", xfer->ux_actlen, 0, 0, 0); } @@ -4163,15 +4099,13 @@ ehci_device_intr_start(struct usbd_xfer DPRINTFN(5, "--- dump end ---", 0, 0, 0, 0); #endif - usb_syncmem(&xfer->ux_dmabuf, 0, xfer->ux_length, - isread ? BUS_DMASYNC_PREREAD : BUS_DMASYNC_PREWRITE); + if (xfer->ux_length) + usb_syncmem(&xfer->ux_dmabuf, 0, xfer->ux_length, + isread ? BUS_DMASYNC_PREREAD : BUS_DMASYNC_PREWRITE); /* also does usb_syncmem(sqh) */ ehci_set_qh_qtd(sqh, exfer->ex_sqtdstart); - if (xfer->ux_timeout && !sc->sc_bus.ub_usepolling) { - callout_reset(&xfer->ux_callout, mstohz(xfer->ux_timeout), - ehci_timeout, xfer); - } + usbd_xfer_schedule_timeout(xfer); ehci_add_intr_list(sc, exfer); xfer->ux_status = USBD_IN_PROGRESS; if (!polling) @@ -4205,7 +4139,7 @@ ehci_device_intr_abort(struct usbd_xfer * async doorbell. That's dependent on the async list, wheras * intr xfers are periodic, should not use this? */ - ehci_abort_xfer(xfer, USBD_CANCELLED); + usbd_xfer_abort(xfer); } Static void @@ -4226,7 +4160,6 @@ ehci_device_intr_done(struct usbd_xfer * { ehci_softc_t *sc __diagused = EHCI_XFER2SC(xfer); struct ehci_pipe *epipe = EHCI_XFER2EPIPE(xfer); - int isread, endpt; EHCIHIST_FUNC(); EHCIHIST_CALLED(); @@ -4234,10 +4167,14 @@ ehci_device_intr_done(struct usbd_xfer * KASSERT(sc->sc_bus.ub_usepolling || mutex_owned(&sc->sc_lock)); - endpt = epipe->pipe.up_endpoint->ue_edesc->bEndpointAddress; - isread = UE_GET_DIR(endpt) == UE_DIR_IN; - usb_syncmem(&xfer->ux_dmabuf, 0, xfer->ux_length, - isread ? BUS_DMASYNC_POSTREAD : BUS_DMASYNC_POSTWRITE); + if (xfer->ux_length) { + int isread, endpt; + + endpt = epipe->pipe.up_endpoint->ue_edesc->bEndpointAddress; + isread = UE_GET_DIR(endpt) == UE_DIR_IN; + usb_syncmem(&xfer->ux_dmabuf, 0, xfer->ux_length, + isread ? BUS_DMASYNC_POSTREAD : BUS_DMASYNC_POSTWRITE); + } } /************************/ @@ -4493,8 +4430,9 @@ ehci_device_fs_isoc_transfer(struct usbd sizeof(sitd->sitd.sitd_trans), BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD); - usb_syncmem(&exfer->ex_xfer.ux_dmabuf, 0, total_length, - BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE); + if (total_length) + usb_syncmem(&exfer->ex_xfer.ux_dmabuf, 0, total_length, + BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE); /* * Part 2: Transfer descriptors have now been set up, now they must @@ -4601,8 +4539,9 @@ ehci_device_fs_isoc_done(struct usbd_xfe epipe->isoc.cur_xfers--; ehci_remove_sitd_chain(sc, exfer->ex_itdstart); - usb_syncmem(&xfer->ux_dmabuf, 0, xfer->ux_length, - BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD); + if (xfer->ux_length) + usb_syncmem(&xfer->ux_dmabuf, 0, xfer->ux_length, + BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD); } /* -------------------------------------------------------------------------- */ @@ -4879,8 +4818,9 @@ ehci_device_isoc_transfer(struct usbd_xf prev = itd; } /* End of frame */ - usb_syncmem(&exfer->ex_xfer.ux_dmabuf, 0, total_length, - BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE); + if (total_length) + usb_syncmem(&exfer->ex_xfer.ux_dmabuf, 0, total_length, + BUS_DMASYNC_PREREAD | BUS_DMASYNC_PREWRITE); /* * Part 2: Transfer descriptors have now been set up, now they must @@ -4991,6 +4931,7 @@ ehci_device_isoc_done(struct usbd_xfer * epipe->isoc.cur_xfers--; ehci_remove_itd_chain(sc, exfer->ex_sitdstart); - usb_syncmem(&xfer->ux_dmabuf, 0, xfer->ux_length, - BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD); + if (xfer->ux_length) + usb_syncmem(&xfer->ux_dmabuf, 0, xfer->ux_length, + BUS_DMASYNC_POSTWRITE | BUS_DMASYNC_POSTREAD); }