Module Name: src Committed By: martin Date: Sat Aug 25 14:57:35 UTC 2018
Modified Files: src/sys/dev/usb [netbsd-7]: ehci.c ehcivar.h motg.c ohci.c ohcivar.h uhci.c uhcivar.h usbdi.c usbdivar.h xhci.c xhcivar.h src/sys/external/bsd/dwc2 [netbsd-7]: dwc2.c Log Message: Pull up following revision(s) (requested by mrg in ticket #1632): sys/dev/usb/usbdivar.h: revision 1.117 sys/external/bsd/dwc2/dwc2.c: revision 1.52 sys/dev/usb/xhcivar.h: revision 1.10 sys/dev/usb/motg.c: revision 1.22 sys/dev/usb/ehci.c: revision 1.260 sys/dev/usb/ehci.c: revision 1.261 sys/dev/usb/xhci.c: revision 1.96 sys/dev/usb/ohci.c: revision 1.282 sys/dev/usb/ohci.c: revision 1.283 sys/dev/usb/ehcivar.h: revision 1.45 sys/dev/usb/uhci.c: revision 1.281 sys/dev/usb/uhci.c: revision 1.282 sys/dev/usb/usbdi.c: revision 1.177 sys/dev/usb/ohcivar.h: revision 1.60 sys/dev/usb/uhcivar.h: revision 1.55 (all via patch) pull across abort fixes from nick-nhusb. add more abort fixes, using ideas from Taylor and Nick, and myself. special thanks to both who inspired much of the code here, if not wrote it directly. among other problems, this assert should no longer trigger: panic: kernel diagnostic assertion "xfer->ux_state == XFER_ONQU" failed: file "/current/src/sys/dev/usb/usbdi.c", line 914 using usbhist i was able to track down my instance of it being related to userland close() beginning, dropping the sc_lock, and then the usb softintr completes the transfer normally, and when it is done, the abort path attempts to re-complete the transfer, and the above assert is tripped. changes from nhusb were commited with these logs: -- Move the struct usb_task to struct usbd_xfer for everyone to use. -- Set device transfer status to USBD_IN_PROGRESS if start methods succeeds -- Actually set the transfer status on transfers in ohci_abort_xfer and the controller is dying -- Don't supply the lock to callout_halt when polling as it won't be held -- Improve transfer abort -- Mark device transfers as USBD_IN_PROGRESS appropriately and improve abort handling -- -- Mark device transfers as USBD_IN_PROGRESS appropriately and improve abort handling -- additional changes include: - initialise the usb abort task in the HCI allocx routine, so that it can be safely usb_rem_task()'d. - rework the handling of softintr vs cancellation vs timeout abort based upon a scheme from Taylor: when completing a transfer normally: - if the status is not in progress, it must be cancelled or timed out, and we should not process this xfer. - set the status as normal. - unconditionallly callout_stop() and usb_rem_task(). they're safe and either aren't running, or will run and do nothing. - finally call usb_transfer_complete(). when aborting a transfer: - status should be cancelled or timed out. - if cancelling, callout_halt and usb_rem_task_wait() to make sure the timer is either done or cancelled. - at this point, the ux_status must not be cancelled or timed out, and if it is not in progress we're done. - set the status. - if the controller is dying, just return. - perform HCI-specific tasks to abort this xfer. - finally call usb_transfer_complete(). for the timeout and timeout task: - if the HCI is not dying, and the ux_status is in progress, then trigger the usb abort task. - remove UXFER_ABORTWAIT and UXFER_ABORTING. tested on: - multiple PC systems with several types of devices: ugen/UPS, ucom, umass with disk, ssd and cdrom backends, kbd, ms, using uhci, ehci and xhci. - erlite3: sd@umass on dwc2. - sunblade2000: kbd/ms and umass disk on ohci. untested: - motg, slhci and ahci. motg has some portion of the new scheme applied, but slhci and ahci require more study. future work includes pushing a lot of the common abort handling into usbdi.c and leaving upm_abort() for HC specific tasks, but this change is pullup-able to netbsd-7 and netbsd-8 as it does not change any external API, as well as removing over 100 lines of code while adding over 30 new asserts. XXX: pullup-7, pullup-8. fix DIAGNOSTIC build by not copying ub_usepolling to stack before use Sprinkle __diagused To generate a diff of this commit: cvs rdiff -u -r1.228.2.2 -r1.228.2.3 src/sys/dev/usb/ehci.c cvs rdiff -u -r1.42.12.1 -r1.42.12.2 src/sys/dev/usb/ehcivar.h cvs rdiff -u -r1.6.4.4 -r1.6.4.5 src/sys/dev/usb/motg.c cvs rdiff -u -r1.253.2.4 -r1.253.2.5 src/sys/dev/usb/ohci.c cvs rdiff -u -r1.55.4.1 -r1.55.4.2 src/sys/dev/usb/ohcivar.h cvs rdiff -u -r1.264.2.3 -r1.264.2.4 src/sys/dev/usb/uhci.c cvs rdiff -u -r1.52.12.1 -r1.52.12.2 src/sys/dev/usb/uhcivar.h cvs rdiff -u -r1.161.2.2 -r1.161.2.3 src/sys/dev/usb/usbdi.c cvs rdiff -u -r1.107.4.2 -r1.107.4.3 src/sys/dev/usb/usbdivar.h cvs rdiff -u -r1.23.2.7 -r1.23.2.8 src/sys/dev/usb/xhci.c cvs rdiff -u -r1.4.8.1 -r1.4.8.2 src/sys/dev/usb/xhcivar.h cvs rdiff -u -r1.31.2.3 -r1.31.2.4 src/sys/external/bsd/dwc2/dwc2.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sys/dev/usb/ehci.c diff -u src/sys/dev/usb/ehci.c:1.228.2.2 src/sys/dev/usb/ehci.c:1.228.2.3 --- src/sys/dev/usb/ehci.c:1.228.2.2 Wed Jan 3 20:02:37 2018 +++ src/sys/dev/usb/ehci.c Sat Aug 25 14:57:35 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: ehci.c,v 1.228.2.2 2018/01/03 20:02:37 snj Exp $ */ +/* $NetBSD: ehci.c,v 1.228.2.3 2018/08/25 14:57:35 martin Exp $ */ /* * Copyright (c) 2004-2012 The NetBSD Foundation, Inc. @@ -53,7 +53,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: ehci.c,v 1.228.2.2 2018/01/03 20:02:37 snj Exp $"); +__KERNEL_RCSID(0, "$NetBSD: ehci.c,v 1.228.2.3 2018/08/25 14:57:35 martin Exp $"); #include "ohci.h" #include "uhci.h" @@ -412,8 +412,7 @@ ehci_init(ehci_softc_t *sc) mutex_init(&sc->sc_lock, MUTEX_DEFAULT, IPL_SOFTUSB); mutex_init(&sc->sc_intr_lock, MUTEX_DEFAULT, IPL_USB); - cv_init(&sc->sc_softwake_cv, "ehciab"); - cv_init(&sc->sc_doorbell, "ehcidi"); + cv_init(&sc->sc_doorbell, "ehcidb"); sc->sc_xferpool = pool_cache_init(sizeof(struct ehci_xfer), 0, 0, 0, "ehcixfer", NULL, IPL_USB, NULL, NULL, NULL); @@ -751,6 +750,7 @@ Static void ehci_doorbell(void *addr) { ehci_softc_t *sc = addr; + EHCIHIST_FUNC(); EHCIHIST_CALLED(); mutex_enter(&sc->sc_lock); cv_broadcast(&sc->sc_doorbell); @@ -853,11 +853,6 @@ ehci_softintr(void *v) !TAILQ_EMPTY(&sc->sc_intrhead)) callout_reset(&sc->sc_tmo_intrlist, hz, ehci_intrlist_timeout, sc); - - if (sc->sc_softwake) { - sc->sc_softwake = 0; - cv_broadcast(&sc->sc_softwake_cv); - } } Static void @@ -939,7 +934,6 @@ ehci_check_qh_intr(ehci_softc_t *sc, str } done: DPRINTFN(10, "ex=%p done", ex, 0, 0, 0); - callout_stop(&ex->ex_xfer.ux_callout); ehci_idone(ex, cq); } @@ -985,7 +979,6 @@ ehci_check_itd_intr(ehci_softc_t *sc, st return; done: DPRINTF("ex %p done", ex, 0, 0, 0); - callout_stop(&ex->ex_xfer.ux_callout); ehci_idone(ex, cq); } @@ -1023,7 +1016,6 @@ ehci_check_sitd_intr(ehci_softc_t *sc, s return; DPRINTFN(10, "ex=%p done", ex, 0, 0, 0); - callout_stop(&(ex->ex_xfer.ux_callout)); ehci_idone(ex, cq); } @@ -1031,6 +1023,7 @@ ehci_check_sitd_intr(ehci_softc_t *sc, s Static void ehci_idone(struct ehci_xfer *ex, ex_completeq_t *cq) { + EHCIHIST_FUNC(); EHCIHIST_CALLED(); struct usbd_xfer *xfer = &ex->ex_xfer; struct ehci_pipe *epipe = EHCI_XFER2EPIPE(xfer); struct ehci_softc *sc = EHCI_XFER2SC(xfer); @@ -1038,18 +1031,30 @@ ehci_idone(struct ehci_xfer *ex, ex_comp uint32_t status = 0, nstatus = 0; int actlen = 0; - EHCIHIST_FUNC(); EHCIHIST_CALLED(); - KASSERT(sc->sc_bus.ub_usepolling || mutex_owned(&sc->sc_lock)); DPRINTF("ex=%p", ex, 0, 0, 0); - if (xfer->ux_status == USBD_CANCELLED || - xfer->ux_status == USBD_TIMEOUT) { + /* + * If software has completed it, either by cancellation + * or timeout, 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=%p", xfer, 0, 0, 0); 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 if (ex->ex_isdone) { @@ -1332,7 +1337,6 @@ ehci_detach(struct ehci_softc *sc, int f kmem_free(sc->sc_softitds, sc->sc_flsize * sizeof(ehci_soft_itd_t *)); cv_destroy(&sc->sc_doorbell); - cv_destroy(&sc->sc_softwake_cv); #if 0 /* XXX destroyed in ehci_pci.c as it controls ehci_intr access */ @@ -1511,6 +1515,10 @@ ehci_allocx(struct usbd_bus *bus, unsign xfer = pool_cache_get(sc->sc_xferpool, PR_WAITOK); 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; @@ -2158,6 +2166,7 @@ ehci_sync_hc(ehci_softc_t *sc) DPRINTF("dying", 0, 0, 0, 0); return; } + /* ask for doorbell */ EOWRITE4(sc, EHCI_USBCMD, EOREAD4(sc, EHCI_USBCMD) | EHCI_CMD_IAAD); DPRINTF("cmd = 0x%08x sts = 0x%08x", @@ -3095,20 +3104,24 @@ ehci_close_pipe(struct usbd_pipe *pipe, } /* - * Abort a device request. - * If this routine is called at splusb() it guarantees that the request - * will be removed from the hardware scheduling and that the callback - * for it will be called with USBD_CANCELLED status. - * It's impossible to guarantee that the requested transfer will not - * have happened since the hardware runs concurrently. - * If the transaction has already happened we rely on the ordinary - * interrupt processing to process it. - * XXX This is most probably wrong. - * XXXMRG this doesn't make sense anymore. + * 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 + * 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) { + EHCIHIST_FUNC(); EHCIHIST_CALLED(); struct ehci_pipe *epipe = EHCI_XFER2EPIPE(xfer); struct ehci_xfer *exfer = EHCI_XFER2EXFER(xfer); ehci_softc_t *sc = EHCI_XFER2SC(xfer); @@ -3117,48 +3130,59 @@ ehci_abort_xfer(struct usbd_xfer *xfer, ehci_physaddr_t cur; uint32_t qhstatus; int hit; - int wake; - EHCIHIST_FUNC(); EHCIHIST_CALLED(); + KASSERTMSG((status == USBD_CANCELLED || status == USBD_TIMEOUT), + "invalid status for abort: %d", (int)status); DPRINTF("xfer=%p pipe=%p", xfer, epipe, 0, 0); KASSERT(mutex_owned(&sc->sc_lock)); ASSERT_SLEEPABLE(); - if (sc->sc_dying) { - /* If we're dying, just do the software part. */ - xfer->ux_status = status; /* make software ignore it */ - callout_stop(&xfer->ux_callout); - usb_transfer_complete(xfer); - return; + 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); } /* - * If an abort is already in progress then just wait for it to - * complete and return. + * 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. */ - if (xfer->ux_hcflags & UXFER_ABORTING) { - DPRINTF("already aborting", 0, 0, 0, 0); -#ifdef DIAGNOSTIC - if (status == USBD_TIMEOUT) - printf("ehci_abort_xfer: TIMEOUT while aborting\n"); -#endif - /* Override the status which might be USBD_TIMEOUT. */ - xfer->ux_status = status; - DPRINTF("waiting for abort to finish", 0, 0, 0, 0); - xfer->ux_hcflags |= UXFER_ABORTWAIT; - while (xfer->ux_hcflags & UXFER_ABORTING) - cv_wait(&xfer->ux_hccv, &sc->sc_lock); + + 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; + + /* + * If we're dying, skip the hardware action and just notify the + * software that we're done. + */ + if (sc->sc_dying) { + goto dying; } - xfer->ux_hcflags |= UXFER_ABORTING; /* - * Step 1: Make interrupt routine and hardware ignore xfer. + * HC Step 1: Make interrupt routine and hardware ignore xfer. */ - xfer->ux_status = status; /* make software ignore it */ - callout_stop(&xfer->ux_callout); ehci_del_intr_list(sc, exfer); usb_syncmem(&sqh->dma, @@ -3194,17 +3218,13 @@ ehci_abort_xfer(struct usbd_xfer *xfer, } /* - * Step 2: Wait until we know hardware has finished any possible - * use of the xfer. Also make sure the soft interrupt routine - * has run. + * HC Step 2: Wait until we know hardware has finished any possible + * use of the xfer. */ ehci_sync_hc(sc); - sc->sc_softwake = 1; - usb_schedsoftintr(&sc->sc_bus); - cv_wait(&sc->sc_softwake_cv, &sc->sc_lock); /* - * Step 3: Remove any vestiges of the xfer from the hardware. + * HC Step 3: Remove any vestiges of the xfer from the hardware. * The complication here is that the hardware may have executed * beyond the xfer we're trying to abort. So as we're scanning * the TDs of this xfer we check if the hardware points to @@ -3245,17 +3265,14 @@ ehci_abort_xfer(struct usbd_xfer *xfer, } /* - * Step 4: Execute callback. + * Final step: Notify completion to waiting xfers. */ +dying: #ifdef DIAGNOSTIC exfer->ex_isdone = true; #endif - wake = xfer->ux_hcflags & UXFER_ABORTWAIT; - xfer->ux_hcflags &= ~(UXFER_ABORTING | UXFER_ABORTWAIT); usb_transfer_complete(xfer); - if (wake) { - cv_broadcast(&xfer->ux_hccv); - } + DPRINTFN(14, "end", 0, 0, 0, 0); KASSERT(mutex_owned(&sc->sc_lock)); } @@ -3263,14 +3280,16 @@ ehci_abort_xfer(struct usbd_xfer *xfer, Static void ehci_abort_isoc_xfer(struct usbd_xfer *xfer, usbd_status status) { + EHCIHIST_FUNC(); EHCIHIST_CALLED(); ehci_isoc_trans_t trans_status; struct ehci_xfer *exfer; ehci_softc_t *sc; struct ehci_soft_itd *itd; struct ehci_soft_sitd *sitd; - int i, wake; + int i; - EHCIHIST_FUNC(); EHCIHIST_CALLED(); + KASSERTMSG(status == USBD_CANCELLED, + "invalid status for abort: %d", (int)status); exfer = EHCI_XFER2EXFER(xfer); sc = EHCI_XFER2SC(xfer); @@ -3278,33 +3297,36 @@ ehci_abort_isoc_xfer(struct usbd_xfer *x DPRINTF("xfer %p pipe %p", xfer, xfer->ux_pipe, 0, 0); KASSERT(mutex_owned(&sc->sc_lock)); + ASSERT_SLEEPABLE(); - if (sc->sc_dying) { - xfer->ux_status = status; - callout_stop(&xfer->ux_callout); - usb_transfer_complete(xfer); - return; - } + /* No timeout or task here. */ - if (xfer->ux_hcflags & UXFER_ABORTING) { - DPRINTF("already aborting", 0, 0, 0, 0); + /* + * 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); -#ifdef DIAGNOSTIC - if (status == USBD_TIMEOUT) - printf("ehci_abort_isoc_xfer: TIMEOUT while aborting\n"); -#endif + /* If anyone else beat us, we're done. */ + if (xfer->ux_status != USBD_IN_PROGRESS) + return; - xfer->ux_status = status; - DPRINTF("waiting for abort to finish", 0, 0, 0, 0); - xfer->ux_hcflags |= UXFER_ABORTWAIT; - while (xfer->ux_hcflags & UXFER_ABORTING) - cv_wait(&xfer->ux_hccv, &sc->sc_lock); - goto done; + /* We beat everyone else. Claim the status. */ + xfer->ux_status = status; + + /* + * If we're dying, skip the hardware action and just notify the + * software that we're done. + */ + if (sc->sc_dying) { + goto dying; } - xfer->ux_hcflags |= UXFER_ABORTING; - xfer->ux_status = status; - callout_stop(&xfer->ux_callout); + /* + * HC Step 1: Make interrupt routine and hardware ignore xfer. + */ ehci_del_intr_list(sc, exfer); if (xfer->ux_pipe->up_dev->ud_speed == USB_SPEED_HIGH) { @@ -3345,53 +3367,36 @@ ehci_abort_isoc_xfer(struct usbd_xfer *x } } - sc->sc_softwake = 1; - usb_schedsoftintr(&sc->sc_bus); - cv_wait(&sc->sc_softwake_cv, &sc->sc_lock); - +dying: #ifdef DIAGNOSTIC exfer->ex_isdone = true; #endif - wake = xfer->ux_hcflags & UXFER_ABORTWAIT; - xfer->ux_hcflags &= ~(UXFER_ABORTING | UXFER_ABORTWAIT); usb_transfer_complete(xfer); - if (wake) { - cv_broadcast(&xfer->ux_hccv); - } + DPRINTFN(14, "end", 0, 0, 0, 0); -done: KASSERT(mutex_owned(&sc->sc_lock)); - return; } Static void ehci_timeout(void *addr) { + EHCIHIST_FUNC(); EHCIHIST_CALLED(); struct usbd_xfer *xfer = addr; - struct ehci_xfer *exfer = EHCI_XFER2EXFER(xfer); - struct usbd_pipe *pipe = xfer->ux_pipe; - struct usbd_device *dev = pipe->up_dev; ehci_softc_t *sc = EHCI_XFER2SC(xfer); + struct usbd_device *dev = xfer->ux_pipe->up_dev; - EHCIHIST_FUNC(); EHCIHIST_CALLED(); - - DPRINTF("exfer %p", exfer, 0, 0, 0); + DPRINTF("xfer %p", xfer, 0, 0, 0); #ifdef EHCI_DEBUG - if (ehcidebug >= 2) + if (ehcidebug >= 2) { + struct usbd_pipe *pipe = xfer->ux_pipe; usbd_dump_pipe(pipe); -#endif - - if (sc->sc_dying) { - mutex_enter(&sc->sc_lock); - ehci_abort_xfer(xfer, USBD_TIMEOUT); - mutex_exit(&sc->sc_lock); - return; } +#endif - /* Execute the abort in a process context. */ - usb_init_task(&exfer->ex_aborttask, ehci_timeout_task, xfer, - USB_TASKQ_MPSAFE); - usb_add_task(dev, &exfer->ex_aborttask, USB_TASKQ_HC); + 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 @@ -4458,7 +4463,6 @@ ehci_device_fs_isoc_transfer(struct usbd ehci_add_intr_list(sc, exfer); xfer->ux_status = USBD_IN_PROGRESS; - mutex_exit(&sc->sc_lock); return USBD_IN_PROGRESS; @@ -4852,7 +4856,6 @@ ehci_device_isoc_transfer(struct usbd_xf ehci_add_intr_list(sc, exfer); xfer->ux_status = USBD_IN_PROGRESS; - mutex_exit(&sc->sc_lock); return USBD_IN_PROGRESS; Index: src/sys/dev/usb/ehcivar.h diff -u src/sys/dev/usb/ehcivar.h:1.42.12.1 src/sys/dev/usb/ehcivar.h:1.42.12.2 --- src/sys/dev/usb/ehcivar.h:1.42.12.1 Wed Apr 5 19:54:19 2017 +++ src/sys/dev/usb/ehcivar.h Sat Aug 25 14:57:35 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: ehcivar.h,v 1.42.12.1 2017/04/05 19:54:19 snj Exp $ */ +/* $NetBSD: ehcivar.h,v 1.42.12.2 2018/08/25 14:57:35 martin Exp $ */ /* * Copyright (c) 2001 The NetBSD Foundation, Inc. @@ -91,7 +91,6 @@ typedef struct ehci_soft_itd { struct ehci_xfer { struct usbd_xfer ex_xfer; - struct usb_task ex_aborttask; TAILQ_ENTRY(ehci_xfer) ex_next; /* list of active xfers */ enum { EX_NONE, @@ -211,8 +210,6 @@ typedef struct ehci_softc { uint8_t sc_istthreshold; /* ISOC Scheduling Threshold (uframes) */ struct usbd_xfer *sc_intrxfer; char sc_isreset[EHCI_MAX_PORTS]; - char sc_softwake; - kcondvar_t sc_softwake_cv; uint32_t sc_eintrs; ehci_soft_qh_t *sc_async_head; Index: src/sys/dev/usb/motg.c diff -u src/sys/dev/usb/motg.c:1.6.4.4 src/sys/dev/usb/motg.c:1.6.4.5 --- src/sys/dev/usb/motg.c:1.6.4.4 Wed Jan 3 20:02:37 2018 +++ src/sys/dev/usb/motg.c Sat Aug 25 14:57:35 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: motg.c,v 1.6.4.4 2018/01/03 20:02:37 snj Exp $ */ +/* $NetBSD: motg.c,v 1.6.4.5 2018/08/25 14:57:35 martin Exp $ */ /* * Copyright (c) 1998, 2004, 2011, 2012, 2014 The NetBSD Foundation, Inc. @@ -40,7 +40,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: motg.c,v 1.6.4.4 2018/01/03 20:02:37 snj Exp $"); +__KERNEL_RCSID(0, "$NetBSD: motg.c,v 1.6.4.5 2018/08/25 14:57:35 martin Exp $"); #ifdef _KERNEL_OPT #include "opt_motg.h" @@ -2160,22 +2160,46 @@ motg_device_clear_toggle(struct usbd_pip static void motg_device_xfer_abort(struct usbd_xfer *xfer) { - int wake; + MOTGHIST_FUNC(); MOTGHIST_CALLED(); uint8_t csr; struct motg_softc *sc = MOTG_XFER2SC(xfer); struct motg_pipe *otgpipe = MOTG_PIPE2MPIPE(xfer->ux_pipe); + KASSERT(mutex_owned(&sc->sc_lock)); + ASSERT_SLEEPABLE(); - MOTGHIST_FUNC(); MOTGHIST_CALLED(); + /* + * 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); + + /* + * 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); - if (xfer->ux_hcflags & UXFER_ABORTING) { - DPRINTF("already aborting", 0, 0, 0, 0); - xfer->ux_hcflags |= UXFER_ABORTWAIT; - while (xfer->ux_hcflags & UXFER_ABORTING) - cv_wait(&xfer->ux_hccv, &sc->sc_lock); + /* 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 = USBD_CANCELLED; + + /* + * If we're dying, skip the hardware action and just notify the + * software that we're done. + */ + if (sc->sc_dying) { + goto dying; } - xfer->ux_hcflags |= UXFER_ABORTING; + if (otgpipe->hw_ep->xfer == xfer) { KASSERT(xfer->ux_status == USBD_IN_PROGRESS); otgpipe->hw_ep->xfer = NULL; @@ -2203,10 +2227,7 @@ motg_device_xfer_abort(struct usbd_xfer otgpipe->hw_ep->phase = IDLE; } } - xfer->ux_status = USBD_CANCELLED; /* make software ignore it */ - wake = xfer->ux_hcflags & UXFER_ABORTWAIT; - xfer->ux_hcflags &= ~(UXFER_ABORTING | UXFER_ABORTWAIT); +dying: usb_transfer_complete(xfer); - if (wake) - cv_broadcast(&xfer->ux_hccv); + KASSERT(mutex_owned(&sc->sc_lock)); } Index: src/sys/dev/usb/ohci.c diff -u src/sys/dev/usb/ohci.c:1.253.2.4 src/sys/dev/usb/ohci.c:1.253.2.5 --- src/sys/dev/usb/ohci.c:1.253.2.4 Wed Jan 3 20:02:37 2018 +++ src/sys/dev/usb/ohci.c Sat Aug 25 14:57:35 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: ohci.c,v 1.253.2.4 2018/01/03 20:02:37 snj Exp $ */ +/* $NetBSD: ohci.c,v 1.253.2.5 2018/08/25 14:57:35 martin Exp $ */ /* * Copyright (c) 1998, 2004, 2005, 2012 The NetBSD Foundation, Inc. @@ -41,7 +41,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: ohci.c,v 1.253.2.4 2018/01/03 20:02:37 snj Exp $"); +__KERNEL_RCSID(0, "$NetBSD: ohci.c,v 1.253.2.5 2018/08/25 14:57:35 martin Exp $"); #ifdef _KERNEL_OPT #include "opt_usb.h" @@ -384,8 +384,6 @@ ohci_detach(struct ohci_softc *sc, int f softint_disestablish(sc->sc_rhsc_si); - cv_destroy(&sc->sc_softwake_cv); - mutex_destroy(&sc->sc_lock); mutex_destroy(&sc->sc_intr_lock); @@ -786,7 +784,6 @@ ohci_init(ohci_softc_t *sc) mutex_init(&sc->sc_lock, MUTEX_DEFAULT, IPL_SOFTUSB); mutex_init(&sc->sc_intr_lock, MUTEX_DEFAULT, IPL_USB); - cv_init(&sc->sc_softwake_cv, "ohciab"); sc->sc_rhsc_si = softint_establish(SOFTINT_USB | SOFTINT_MPSAFE, ohci_rhsc_softint, sc); @@ -1068,6 +1065,10 @@ ohci_allocx(struct usbd_bus *bus, unsign xfer = pool_cache_get(sc->sc_xferpool, PR_WAITOK); if (xfer != NULL) { memset(xfer, 0, sizeof(struct ohci_xfer)); + + /* Initialise this always so we can call remove on it. */ + usb_init_task(&xfer->ux_aborttask, ohci_timeout_task, xfer, + USB_TASKQ_MPSAFE); #ifdef DIAGNOSTIC xfer->ux_state = XFER_BUSY; #endif @@ -1453,13 +1454,25 @@ ohci_softintr(void *v) */ continue; } - if (xfer->ux_status == USBD_CANCELLED || - xfer->ux_status == USBD_TIMEOUT) { - DPRINTF("cancel/timeout %p", xfer, 0, 0, 0); - /* Handled by abort routine. */ + + /* + * If software has completed it, either by cancellation + * or timeout, drop it on the floor. + */ + if (xfer->ux_status != USBD_IN_PROGRESS) { + KASSERT(xfer->ux_status == USBD_CANCELLED || + xfer->ux_status == USBD_TIMEOUT); continue; } + + /* + * 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); len = std->len; if (std->td.td_cbp != 0) @@ -1529,12 +1542,26 @@ ohci_softintr(void *v) xfer ? xfer->ux_hcpriv : 0, 0); if (xfer == NULL) continue; - if (xfer->ux_status == USBD_CANCELLED || - xfer->ux_status == USBD_TIMEOUT) { - DPRINTF("cancel/timeout %p", xfer, 0, 0, 0); - /* Handled by abort routine. */ + + /* + * If software has completed it, either by cancellation + * or timeout, drop it on the floor. + */ + if (xfer->ux_status != USBD_IN_PROGRESS) { + KASSERT(xfer->ux_status == USBD_CANCELLED || + xfer->ux_status == USBD_TIMEOUT); continue; } + + /* + * 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); + KASSERT(!sitd->isdone); #ifdef DIAGNOSTIC sitd->isdone = true; @@ -1588,12 +1615,8 @@ ohci_softintr(void *v) } } - if (sc->sc_softwake) { - sc->sc_softwake = 0; - cv_broadcast(&sc->sc_softwake_cv); - } - DPRINTFN(10, "done", 0, 0, 0, 0); + KASSERT(sc->sc_bus.ub_usepolling || mutex_owned(&sc->sc_lock)); } void @@ -1876,25 +1899,17 @@ ohci_hash_find_itd(ohci_softc_t *sc, ohc void ohci_timeout(void *addr) { + OHCIHIST_FUNC(); OHCIHIST_CALLED(); struct usbd_xfer *xfer = addr; - struct ohci_xfer *oxfer = OHCI_XFER2OXFER(xfer); ohci_softc_t *sc = OHCI_XFER2SC(xfer); + struct usbd_device *dev = xfer->ux_pipe->up_dev; - OHCIHIST_FUNC(); OHCIHIST_CALLED(); - DPRINTF("oxfer=%p", oxfer, 0, 0, 0); - - if (sc->sc_dying) { - mutex_enter(&sc->sc_lock); - ohci_abort_xfer(xfer, USBD_TIMEOUT); - mutex_exit(&sc->sc_lock); - return; - } + DPRINTF("xfer=%p", xfer, 0, 0, 0); - /* Execute the abort in a process context. */ - usb_init_task(&oxfer->abort_task, ohci_timeout_task, addr, - USB_TASKQ_MPSAFE); - usb_add_task(xfer->ux_pipe->up_dev, &oxfer->abort_task, - USB_TASKQ_HC); + 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); } void @@ -2184,67 +2199,95 @@ ohci_close_pipe(struct usbd_pipe *pipe, } /* - * Abort a device request. - * If this routine is called at splusb() it guarantees that the request - * will be removed from the hardware scheduling and that the callback - * for it will be called with USBD_CANCELLED status. + * 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 + * * It's impossible to guarantee that the requested transfer will not - * have happened since the hardware runs concurrently. - * If the transaction has already happened we rely on the ordinary - * interrupt processing to process it. - * XXX This is most probably wrong. - * XXXMRG this doesn't make sense anymore. + * 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 + * processing the TDs by setting the sKip bit and requesting a SOF interrupt + * + * Once we see the SOF interrupt we can check the transfer TDs/iTDs to see if + * they've been processed and either + * a) if they're unused recover them for later use, or + * b) if they've been used allocate new TD/iTDs to replace those + * used. The softint handler will free the old ones. */ void ohci_abort_xfer(struct usbd_xfer *xfer, usbd_status status) { + OHCIHIST_FUNC(); OHCIHIST_CALLED(); struct ohci_pipe *opipe = OHCI_PIPE2OPIPE(xfer->ux_pipe); ohci_softc_t *sc = OHCI_XFER2SC(xfer); ohci_soft_ed_t *sed = opipe->sed; ohci_soft_td_t *p, *n; ohci_physaddr_t headp; int hit; - int wake; - OHCIHIST_FUNC(); OHCIHIST_CALLED(); + KASSERTMSG((status == USBD_CANCELLED || status == USBD_TIMEOUT), + "invalid status for abort: %d", (int)status); + DPRINTF("xfer=%p pipe=%p sed=%p", xfer, opipe,sed, 0); KASSERT(mutex_owned(&sc->sc_lock)); ASSERT_SLEEPABLE(); - if (sc->sc_dying) { - /* If we're dying, just do the software part. */ - xfer->ux_status = status; /* make software ignore it */ + 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_transfer_complete(xfer); - return; + 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); } /* - * If an abort is already in progress then just wait for it to - * complete and return. + * 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. */ - if (xfer->ux_hcflags & UXFER_ABORTING) { - DPRINTFN(2, "already aborting", 0, 0, 0, 0); -#ifdef DIAGNOSTIC - if (status == USBD_TIMEOUT) - printf("%s: TIMEOUT while aborting\n", __func__); -#endif - /* Override the status which might be USBD_TIMEOUT. */ - xfer->ux_status = status; - DPRINTFN(2, "waiting for abort to finish", 0, 0, 0, 0); - xfer->ux_hcflags |= UXFER_ABORTWAIT; - while (xfer->ux_hcflags & UXFER_ABORTING) - cv_wait(&xfer->ux_hccv, &sc->sc_lock); - goto done; + 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; + + /* + * If we're dying, skip the hardware action and just notify the + * software that we're done. + */ + if (sc->sc_dying) { + DPRINTFN(4, "xfer %#jx dying %ju", (uintptr_t)xfer, + xfer->ux_status, 0, 0); + goto dying; } - xfer->ux_hcflags |= UXFER_ABORTING; /* - * Step 1: Make interrupt routine and hardware ignore xfer. + * HC Step 1: Unless the endpoint is already halted, we set the endpoint + * descriptor sKip bit and wait for hardware to complete processing. + * + * This includes ensuring that any TDs of the transfer that got onto + * the done list are also removed. We ensure this by waiting for + * both a WDH and SOF interrupt. */ - xfer->ux_status = status; /* make software ignore it */ - callout_stop(&xfer->ux_callout); DPRINTFN(1, "stop ed=%p", sed, 0, 0, 0); usb_syncmem(&sed->dma, sed->offs + offsetof(ohci_ed_t, ed_flags), sizeof(sed->ed.ed_flags), @@ -2255,18 +2298,14 @@ ohci_abort_xfer(struct usbd_xfer *xfer, BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD); /* - * Step 2: Wait until we know hardware has finished any possible - * use of the xfer. Also make sure the soft interrupt routine - * has run. + * HC Step 2: Wait until we know hardware has finished any possible + * use of the xfer. */ /* Hardware finishes in 1ms */ usb_delay_ms_locked(opipe->pipe.up_dev->ud_bus, 20, &sc->sc_lock); - sc->sc_softwake = 1; - usb_schedsoftintr(&sc->sc_bus); - cv_wait(&sc->sc_softwake_cv, &sc->sc_lock); /* - * Step 3: Remove any vestiges of the xfer from the hardware. + * HC Step 3: Remove any vestiges of the xfer from the hardware. * The complication here is that the hardware may have executed * beyond the xfer we're trying to abort. So as we're scanning * the TDs of this xfer we check if the hardware points to @@ -2306,7 +2345,7 @@ ohci_abort_xfer(struct usbd_xfer *xfer, } /* - * Step 4: Turn on hardware again. + * HC Step 4: Turn on hardware again. */ usb_syncmem(&sed->dma, sed->offs + offsetof(ohci_ed_t, ed_flags), sizeof(sed->ed.ed_flags), @@ -2317,15 +2356,12 @@ ohci_abort_xfer(struct usbd_xfer *xfer, BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD); /* - * Step 5: Execute callback. + * Final step: Notify completion to waiting xfers. */ - wake = xfer->ux_hcflags & UXFER_ABORTWAIT; - xfer->ux_hcflags &= ~(UXFER_ABORTING | UXFER_ABORTWAIT); +dying: usb_transfer_complete(xfer); - if (wake) - cv_broadcast(&xfer->ux_hccv); + DPRINTFN(14, "end", 0, 0, 0, 0); -done: KASSERT(mutex_owned(&sc->sc_lock)); } @@ -2840,6 +2876,7 @@ ohci_device_ctrl_start(struct usbd_xfer DPRINTF("done", 0, 0, 0, 0); + xfer->ux_status = USBD_IN_PROGRESS; mutex_exit(&sc->sc_lock); return USBD_IN_PROGRESS; @@ -3047,6 +3084,8 @@ ohci_device_bulk_start(struct usbd_xfer callout_reset(&xfer->ux_callout, mstohz(xfer->ux_timeout), ohci_timeout, xfer); } + + xfer->ux_status = USBD_IN_PROGRESS; mutex_exit(&sc->sc_lock); return USBD_IN_PROGRESS; @@ -3231,6 +3270,7 @@ ohci_device_intr_start(struct usbd_xfer usb_syncmem(&sed->dma, sed->offs, sizeof(sed->ed), BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD); + xfer->ux_status = USBD_IN_PROGRESS; mutex_exit(&sc->sc_lock); return USBD_IN_PROGRESS; Index: src/sys/dev/usb/ohcivar.h diff -u src/sys/dev/usb/ohcivar.h:1.55.4.1 src/sys/dev/usb/ohcivar.h:1.55.4.2 --- src/sys/dev/usb/ohcivar.h:1.55.4.1 Wed Apr 5 19:54:19 2017 +++ src/sys/dev/usb/ohcivar.h Sat Aug 25 14:57:35 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: ohcivar.h,v 1.55.4.1 2017/04/05 19:54:19 snj Exp $ */ +/* $NetBSD: ohcivar.h,v 1.55.4.2 2018/08/25 14:57:35 martin Exp $ */ /* * Copyright (c) 1998 The NetBSD Foundation, Inc. @@ -118,9 +118,6 @@ typedef struct ohci_softc { int sc_flags; #define OHCIF_SUPERIO 0x0001 - char sc_softwake; - kcondvar_t sc_softwake_cv; - ohci_soft_ed_t *sc_freeeds; ohci_soft_td_t *sc_freetds; ohci_soft_itd_t *sc_freeitds; @@ -145,7 +142,6 @@ typedef struct ohci_softc { struct ohci_xfer { struct usbd_xfer xfer; - struct usb_task abort_task; /* ctrl */ ohci_soft_td_t *ox_setup; ohci_soft_td_t *ox_stat; Index: src/sys/dev/usb/uhci.c diff -u src/sys/dev/usb/uhci.c:1.264.2.3 src/sys/dev/usb/uhci.c:1.264.2.4 --- src/sys/dev/usb/uhci.c:1.264.2.3 Wed Jan 3 20:02:37 2018 +++ src/sys/dev/usb/uhci.c Sat Aug 25 14:57:35 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: uhci.c,v 1.264.2.3 2018/01/03 20:02:37 snj Exp $ */ +/* $NetBSD: uhci.c,v 1.264.2.4 2018/08/25 14:57:35 martin Exp $ */ /* * Copyright (c) 1998, 2004, 2011, 2012 The NetBSD Foundation, Inc. @@ -42,7 +42,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: uhci.c,v 1.264.2.3 2018/01/03 20:02:37 snj Exp $"); +__KERNEL_RCSID(0, "$NetBSD: uhci.c,v 1.264.2.4 2018/08/25 14:57:35 martin Exp $"); #ifdef _KERNEL_OPT #include "opt_usb.h" @@ -574,8 +574,6 @@ uhci_init(uhci_softc_t *sc) callout_init(&sc->sc_poll_handle, CALLOUT_MPSAFE); - cv_init(&sc->sc_softwake_cv, "uhciab"); - /* Set up the bus struct. */ sc->sc_bus.ub_methods = &uhci_bus_methods; sc->sc_bus.ub_pipesize = sizeof(struct uhci_pipe); @@ -639,8 +637,6 @@ uhci_detach(struct uhci_softc *sc, int f callout_halt(&sc->sc_poll_handle, NULL); callout_destroy(&sc->sc_poll_handle); - cv_destroy(&sc->sc_softwake_cv); - mutex_destroy(&sc->sc_lock); mutex_destroy(&sc->sc_intr_lock); @@ -661,6 +657,9 @@ uhci_allocx(struct usbd_bus *bus, unsign if (xfer != NULL) { memset(xfer, 0, sizeof(struct uhci_xfer)); + /* Initialise this always so we can call remove on it. */ + usb_init_task(&xfer->ux_aborttask, uhci_timeout_task, xfer, + USB_TASKQ_MPSAFE); #ifdef DIAGNOSTIC struct uhci_xfer *uxfer = UHCI_XFER2UXFER(xfer); uxfer->ux_isdone = true; @@ -1423,10 +1422,7 @@ uhci_softintr(void *v) usb_transfer_complete(&ux->ux_xfer); } - if (sc->sc_softwake) { - sc->sc_softwake = 0; - cv_broadcast(&sc->sc_softwake_cv); - } + KASSERT(sc->sc_bus.ub_usepolling || mutex_owned(&sc->sc_lock)); } /* Check for an interrupt. */ @@ -1482,8 +1478,6 @@ uhci_check_intr(uhci_softc_t *sc, struct if (!(status & UHCI_TD_ACTIVE)) { done: DPRINTFN(12, "ux=%p done", ux, 0, 0, 0); - - callout_stop(&xfer->ux_callout); uhci_idone(ux, cqp); return; } @@ -1554,6 +1548,7 @@ uhci_check_intr(uhci_softc_t *sc, struct void uhci_idone(struct uhci_xfer *ux, ux_completeq_t *cqp) { + UHCIHIST_FUNC(); UHCIHIST_CALLED(); struct usbd_xfer *xfer = &ux->ux_xfer; uhci_softc_t *sc __diagused = UHCI_XFER2SC(xfer); struct uhci_pipe *upipe = UHCI_PIPE2UPIPE(xfer->ux_pipe); @@ -1563,9 +1558,28 @@ uhci_idone(struct uhci_xfer *ux, ux_comp KASSERT(sc->sc_bus.ub_usepolling || mutex_owned(&sc->sc_lock)); - UHCIHIST_FUNC(); UHCIHIST_CALLED(); DPRINTFN(12, "ux=%p", ux, 0, 0, 0); + /* + * If software has completed it, either by cancellation + * or timeout, 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); + 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 UHCI_DEBUG if (ux->ux_isdone) { @@ -1699,26 +1713,17 @@ uhci_idone(struct uhci_xfer *ux, ux_comp void uhci_timeout(void *addr) { + UHCIHIST_FUNC(); UHCIHIST_CALLED(); struct usbd_xfer *xfer = addr; - struct uhci_xfer *uxfer = UHCI_XFER2UXFER(xfer); uhci_softc_t *sc = UHCI_XFER2SC(xfer); + struct usbd_device *dev = xfer->ux_pipe->up_dev; - UHCIHIST_FUNC(); UHCIHIST_CALLED(); - - DPRINTF("uxfer %p", uxfer, 0, 0, 0); + DPRINTF("xfer %p", xfer, 0, 0, 0); - if (sc->sc_dying) { - mutex_enter(&sc->sc_lock); - uhci_abort_xfer(xfer, USBD_TIMEOUT); - mutex_exit(&sc->sc_lock); - return; - } - - /* Execute the abort in a process context. */ - usb_init_task(&uxfer->ux_aborttask, uhci_timeout_task, xfer, - USB_TASKQ_MPSAFE); - usb_add_task(uxfer->ux_xfer.ux_pipe->up_dev, &uxfer->ux_aborttask, - USB_TASKQ_HC); + 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); } void @@ -2325,65 +2330,81 @@ uhci_device_bulk_abort(struct usbd_xfer } /* - * Abort a device request. - * If this routine is called at splusb() it guarantees that the request - * will be removed from the hardware scheduling and that the callback - * for it will be called with USBD_CANCELLED status. + * 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 + * * It's impossible to guarantee that the requested transfer will not - * have happened since the hardware runs concurrently. - * If the transaction has already happened we rely on the ordinary - * interrupt processing to process it. - * XXX This is most probably wrong. - * XXXMRG this doesn't make sense anymore. + * 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). + * + * To allow the hardware time to notice we simply wait. */ void uhci_abort_xfer(struct usbd_xfer *xfer, usbd_status status) { + UHCIHIST_FUNC(); UHCIHIST_CALLED(); struct uhci_xfer *ux = UHCI_XFER2UXFER(xfer); struct uhci_pipe *upipe = UHCI_PIPE2UPIPE(xfer->ux_pipe); uhci_softc_t *sc = UHCI_XFER2SC(xfer); uhci_soft_td_t *std; - int wake; - UHCIHIST_FUNC(); UHCIHIST_CALLED(); + KASSERTMSG((status == USBD_CANCELLED || status == USBD_TIMEOUT), + "invalid status for abort: %d", (int)status); + DPRINTFN(1,"xfer=%p, status=%d", xfer, status, 0, 0); KASSERT(mutex_owned(&sc->sc_lock)); ASSERT_SLEEPABLE(); - if (sc->sc_dying) { - /* If we're dying, just do the software part. */ - xfer->ux_status = status; /* make software ignore it */ - callout_stop(&xfer->ux_callout); - usb_transfer_complete(xfer); - return; + 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); } /* - * If an abort is already in progress then just wait for it to - * complete and return. + * 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. */ - if (xfer->ux_hcflags & UXFER_ABORTING) { - DPRINTFN(2, "already aborting", 0, 0, 0, 0); -#ifdef DIAGNOSTIC - if (status == USBD_TIMEOUT) - printf("%s: TIMEOUT while aborting\n", __func__); -#endif - /* Override the status which might be USBD_TIMEOUT. */ - xfer->ux_status = status; - DPRINTFN(2, "waiting for abort to finish", 0, 0, 0, 0); - xfer->ux_hcflags |= UXFER_ABORTWAIT; - while (xfer->ux_hcflags & UXFER_ABORTING) - cv_wait(&xfer->ux_hccv, &sc->sc_lock); - goto done; + 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; + + /* + * If we're dying, skip the hardware action and just notify the + * software that we're done. + */ + if (sc->sc_dying) { + DPRINTFN(4, "xfer %#jx dying %ju", (uintptr_t)xfer, + xfer->ux_status, 0, 0); + goto dying; } - xfer->ux_hcflags |= UXFER_ABORTING; /* - * Step 1: Make interrupt routine and hardware ignore xfer. + * HC Step 1: Make interrupt routine and hardware ignore xfer. */ - xfer->ux_status = status; /* make software ignore it */ - callout_stop(&xfer->ux_callout); uhci_del_intr_list(sc, ux); DPRINTF("stop ux=%p", ux, 0, 0, 0); @@ -2400,30 +2421,22 @@ uhci_abort_xfer(struct usbd_xfer *xfer, } /* - * Step 2: Wait until we know hardware has finished any possible - * use of the xfer. Also make sure the soft interrupt routine - * has run. + * HC Step 2: Wait until we know hardware has finished any possible + * use of the xfer. */ /* Hardware finishes in 1ms */ usb_delay_ms_locked(upipe->pipe.up_dev->ud_bus, 2, &sc->sc_lock); - sc->sc_softwake = 1; - usb_schedsoftintr(&sc->sc_bus); - DPRINTF("cv_wait", 0, 0, 0, 0); - cv_wait(&sc->sc_softwake_cv, &sc->sc_lock); /* - * Step 3: Execute callback. + * HC Step 3: Notify completion to waiting xfers. */ - DPRINTF("callback", 0, 0, 0, 0); +dying: #ifdef DIAGNOSTIC ux->ux_isdone = true; #endif - wake = xfer->ux_hcflags & UXFER_ABORTWAIT; - xfer->ux_hcflags &= ~(UXFER_ABORTING | UXFER_ABORTWAIT); usb_transfer_complete(xfer); - if (wake) - cv_broadcast(&xfer->ux_hccv); -done: + DPRINTFN(14, "end", 0, 0, 0, 0); + KASSERT(mutex_owned(&sc->sc_lock)); } Index: src/sys/dev/usb/uhcivar.h diff -u src/sys/dev/usb/uhcivar.h:1.52.12.1 src/sys/dev/usb/uhcivar.h:1.52.12.2 --- src/sys/dev/usb/uhcivar.h:1.52.12.1 Wed Apr 5 19:54:20 2017 +++ src/sys/dev/usb/uhcivar.h Sat Aug 25 14:57:35 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: uhcivar.h,v 1.52.12.1 2017/04/05 19:54:20 snj Exp $ */ +/* $NetBSD: uhcivar.h,v 1.52.12.2 2018/08/25 14:57:35 martin Exp $ */ /* * Copyright (c) 1998 The NetBSD Foundation, Inc. @@ -61,7 +61,6 @@ typedef union { struct uhci_xfer { struct usbd_xfer ux_xfer; - struct usb_task ux_aborttask; enum { UX_NONE, UX_CTRL, UX_BULK, UX_INTR, UX_ISOC } ux_type; @@ -152,7 +151,6 @@ typedef struct uhci_softc { kmutex_t sc_lock; kmutex_t sc_intr_lock; - kcondvar_t sc_softwake_cv; uhci_physaddr_t *sc_pframes; usb_dma_t sc_dma; @@ -175,8 +173,6 @@ typedef struct uhci_softc { uint8_t sc_saved_sof; uint16_t sc_saved_frnum; - char sc_softwake; - char sc_isreset; char sc_suspend; char sc_dying; Index: src/sys/dev/usb/usbdi.c diff -u src/sys/dev/usb/usbdi.c:1.161.2.2 src/sys/dev/usb/usbdi.c:1.161.2.3 --- src/sys/dev/usb/usbdi.c:1.161.2.2 Wed Apr 5 19:54:21 2017 +++ src/sys/dev/usb/usbdi.c Sat Aug 25 14:57:35 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: usbdi.c,v 1.161.2.2 2017/04/05 19:54:21 snj Exp $ */ +/* $NetBSD: usbdi.c,v 1.161.2.3 2018/08/25 14:57:35 martin Exp $ */ /* * Copyright (c) 1998, 2012, 2015 The NetBSD Foundation, Inc. @@ -32,7 +32,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: usbdi.c,v 1.161.2.2 2017/04/05 19:54:21 snj Exp $"); +__KERNEL_RCSID(0, "$NetBSD: usbdi.c,v 1.161.2.3 2018/08/25 14:57:35 martin Exp $"); #ifdef _KERNEL_OPT #include "opt_usb.h" @@ -279,6 +279,7 @@ usbd_transfer(struct usbd_xfer *xfer) USBHIST_LOG(usbdebug, "xfer = %p, flags = %#x, pipe = %p, running = %d", xfer, xfer->ux_flags, pipe, pipe->up_running); + KASSERT(xfer->ux_status == USBD_NOT_STARTED); #ifdef USB_DEBUG if (usbdebug > 5) @@ -343,7 +344,7 @@ usbd_transfer(struct usbd_xfer *xfer) } if (err != USBD_IN_PROGRESS) { - USBHIST_LOG(usbdebug, "<- done xfer %p, err %d (complete/error)", xfer, + USBHIST_LOG(usbdebug, "<- done xfer %p, sync err %d (complete/error)", xfer, err, 0, 0); return err; } @@ -475,7 +476,6 @@ usbd_alloc_xfer(struct usbd_device *dev, xfer->ux_bus = dev->ud_bus; callout_init(&xfer->ux_callout, CALLOUT_MPSAFE); cv_init(&xfer->ux_cv, "usbxfer"); - cv_init(&xfer->ux_hccv, "usbhcxfer"); USBHIST_LOG(usbdebug, "returns %p", xfer, 0, 0, 0); @@ -498,7 +498,6 @@ usbd_free_xfer(struct usbd_xfer *xfer) } #endif cv_destroy(&xfer->ux_cv); - cv_destroy(&xfer->ux_hccv); xfer->ux_bus->ub_methods->ubm_freex(xfer->ux_bus, xfer); return USBD_NORMAL_COMPLETION; } @@ -907,7 +906,8 @@ usb_transfer_complete(struct usbd_xfer * pipe, xfer, xfer->ux_status, xfer->ux_actlen); KASSERT(polling || mutex_owned(pipe->up_dev->ud_bus->ub_lock)); - KASSERT(xfer->ux_state == XFER_ONQU); + KASSERTMSG(xfer->ux_state == XFER_ONQU, "xfer %p state is %x", xfer, + xfer->ux_state); KASSERT(pipe != NULL); if (!repeat) { Index: src/sys/dev/usb/usbdivar.h diff -u src/sys/dev/usb/usbdivar.h:1.107.4.2 src/sys/dev/usb/usbdivar.h:1.107.4.3 --- src/sys/dev/usb/usbdivar.h:1.107.4.2 Wed Apr 5 19:54:21 2017 +++ src/sys/dev/usb/usbdivar.h Sat Aug 25 14:57:35 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: usbdivar.h,v 1.107.4.2 2017/04/05 19:54:21 snj Exp $ */ +/* $NetBSD: usbdivar.h,v 1.107.4.3 2018/08/25 14:57:35 martin Exp $ */ /* * Copyright (c) 1998, 2012 The NetBSD Foundation, Inc. @@ -284,11 +284,8 @@ struct usbd_xfer { ux_next; void *ux_hcpriv; /* private use by the HC driver */ - uint8_t ux_hcflags; /* private use by the HC driver */ -#define UXFER_ABORTING 0x01 /* xfer is aborting. */ -#define UXFER_ABORTWAIT 0x02 /* abort completion is being awaited. */ - kcondvar_t ux_hccv; /* private use by the HC driver */ + struct usb_task ux_aborttask; struct callout ux_callout; }; Index: src/sys/dev/usb/xhci.c diff -u src/sys/dev/usb/xhci.c:1.23.2.7 src/sys/dev/usb/xhci.c:1.23.2.8 --- src/sys/dev/usb/xhci.c:1.23.2.7 Wed Jan 3 20:02:37 2018 +++ src/sys/dev/usb/xhci.c Sat Aug 25 14:57:35 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: xhci.c,v 1.23.2.7 2018/01/03 20:02:37 snj Exp $ */ +/* $NetBSD: xhci.c,v 1.23.2.8 2018/08/25 14:57:35 martin Exp $ */ /* * Copyright (c) 2013 Jonathan A. Kollasch @@ -34,7 +34,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: xhci.c,v 1.23.2.7 2018/01/03 20:02:37 snj Exp $"); +__KERNEL_RCSID(0, "$NetBSD: xhci.c,v 1.23.2.8 2018/08/25 14:57:35 martin Exp $"); #ifdef _KERNEL_OPT #include "opt_usb.h" @@ -1660,54 +1660,64 @@ xhci_close_pipe(struct usbd_pipe *pipe) static void xhci_abort_xfer(struct usbd_xfer *xfer, usbd_status status) { + XHCIHIST_FUNC(); XHCIHIST_CALLED(); struct xhci_softc * const sc = XHCI_XFER2SC(xfer); struct xhci_slot * const xs = xfer->ux_pipe->up_dev->ud_hcpriv; const u_int dci = xhci_ep_get_dci(xfer->ux_pipe->up_endpoint->ue_edesc); - XHCIHIST_FUNC(); XHCIHIST_CALLED(); + KASSERTMSG((status == USBD_CANCELLED || status == USBD_TIMEOUT), + "invalid status for abort: %d", (int)status); + DPRINTFN(4, "xfer %p pipe %p status %d", xfer, xfer->ux_pipe, status, 0); KASSERT(mutex_owned(&sc->sc_lock)); + ASSERT_SLEEPABLE(); - if (sc->sc_dying) { - /* If we're dying, just do the software part. */ - DPRINTFN(4, "xfer %p dying %u", xfer, xfer->ux_status, 0, 0); - xfer->ux_status = status; - callout_stop(&xfer->ux_callout); - usb_transfer_complete(xfer); - return; + 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); } /* - * If an abort is already in progress then just wait for it to - * complete and return. + * 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. */ - if (xfer->ux_hcflags & UXFER_ABORTING) { - DPRINTFN(4, "already aborting", 0, 0, 0, 0); -#ifdef DIAGNOSTIC - if (status == USBD_TIMEOUT) - DPRINTFN(4, "TIMEOUT while aborting", 0, 0, 0, 0); -#endif - /* Override the status which might be USBD_TIMEOUT. */ - xfer->ux_status = status; - DPRINTFN(4, "xfer %p waiting for abort to finish", xfer, 0, 0, - 0); - xfer->ux_hcflags |= UXFER_ABORTWAIT; - while (xfer->ux_hcflags & UXFER_ABORTING) - cv_wait(&xfer->ux_hccv, &sc->sc_lock); - return; - } - xfer->ux_hcflags |= UXFER_ABORTING; + 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; /* - * Step 1: Stop xfer timeout timer. + * If we're dying, skip the hardware action and just notify the + * software that we're done. */ - xfer->ux_status = status; - callout_stop(&xfer->ux_callout); + if (sc->sc_dying) { + DPRINTFN(4, "xfer %#jx dying %ju", (uintptr_t)xfer, + xfer->ux_status, 0, 0); + goto dying; + } /* - * Step 2: Stop execution of TD on the ring. + * HC Step 1: Stop execution of TD on the ring. */ switch (xhci_get_epstate(sc, xs, dci)) { case XHCI_EPSTATE_HALTED: @@ -1726,19 +1736,15 @@ xhci_abort_xfer(struct usbd_xfer *xfer, #endif /* - * Step 3: Remove any vestiges of the xfer from the ring. + * HC Step 2: Remove any vestiges of the xfer from the ring. */ xhci_set_dequeue_locked(xfer->ux_pipe); /* - * Step 4: Notify completion to waiting xfers. + * Final Step: Notify completion to waiting xfers. */ - int wake = xfer->ux_hcflags & UXFER_ABORTWAIT; - xfer->ux_hcflags &= ~(UXFER_ABORTING | UXFER_ABORTWAIT); +dying: usb_transfer_complete(xfer); - if (wake) { - cv_broadcast(&xfer->ux_hccv); - } DPRINTFN(14, "end", 0, 0, 0, 0); KASSERT(mutex_owned(&sc->sc_lock)); @@ -1967,7 +1973,8 @@ xhci_event_transfer(struct xhci_softc * * don't complete the transfer being aborted * as abort_xfer does instead. */ - if (xfer->ux_hcflags & UXFER_ABORTING) { + if (xfer->ux_status == USBD_CANCELLED || + xfer->ux_status == USBD_TIMEOUT) { DPRINTFN(14, "ignore aborting xfer %p", xfer, 0, 0, 0); return; } @@ -1977,7 +1984,6 @@ xhci_event_transfer(struct xhci_softc * case XHCI_TRB_ERROR_BABBLE: DPRINTFN(1, "ERR %u slot %u dci %u", trbcode, slot, dci, 0); xr->is_halted = true; - err = USBD_STALLED; /* * Stalled endpoints can be recoverd by issuing * command TRB TYPE_RESET_EP on xHCI instead of @@ -1991,8 +1997,19 @@ xhci_event_transfer(struct xhci_softc * * asynchronously (and then umass issues clear * UF_ENDPOINT_HALT). */ - xfer->ux_status = err; + + /* Override the status. */ + xfer->ux_status = USBD_STALLED; + + /* + * 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); + xhci_clear_endpoint_stall_async(xfer); return; default: @@ -2000,15 +2017,32 @@ xhci_event_transfer(struct xhci_softc * err = USBD_IOERROR; break; } + + /* + * If software has completed it, either by cancellation + * or timeout, drop it on the floor. + */ + if (xfer->ux_status != USBD_IN_PROGRESS) { + KASSERTMSG((xfer->ux_status == USBD_CANCELLED || + xfer->ux_status == USBD_TIMEOUT), + "xfer %p status %x", xfer, xfer->ux_status); + return;; + } + + /* Otherwise, set the status. */ xfer->ux_status = err; - if ((trb_3 & XHCI_TRB_3_ED_BIT) != 0) { - if ((trb_0 & 0x3) == 0x0) { - callout_stop(&xfer->ux_callout); - usb_transfer_complete(xfer); - } - } else { - callout_stop(&xfer->ux_callout); + /* + * 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); + + if ((trb_3 & XHCI_TRB_3_ED_BIT) == 0 || + (trb_0 & 0x3) == 0x0) { usb_transfer_complete(xfer); } } @@ -2171,6 +2205,8 @@ xhci_allocx(struct usbd_bus *bus, unsign xfer = pool_cache_get(sc->sc_xferpool, PR_WAITOK); if (xfer != NULL) { memset(xfer, 0, sizeof(struct xhci_xfer)); + usb_init_task(&xfer->ux_aborttask, xhci_timeout_task, xfer, + USB_TASKQ_MPSAFE); #ifdef DIAGNOSTIC xfer->ux_state = XFER_BUSY; #endif @@ -3772,6 +3808,7 @@ xhci_device_ctrl_start(struct usbd_xfer XHCI_TRB_3_TYPE_SET(XHCI_TRB_TYPE_STATUS_STAGE) | XHCI_TRB_3_IOC_BIT; xhci_trb_put(&xx->xx_trb[i++], parameter, status, control); + xfer->ux_status = USBD_IN_PROGRESS; mutex_enter(&tr->xr_lock); xhci_ring_put(sc, tr, xfer, xx->xx_trb, i); @@ -3888,6 +3925,7 @@ xhci_device_bulk_start(struct usbd_xfer (usbd_xfer_isread(xfer) ? XHCI_TRB_3_ISP_BIT : 0) | XHCI_TRB_3_IOC_BIT; xhci_trb_put(&xx->xx_trb[i++], parameter, status, control); + xfer->ux_status = USBD_IN_PROGRESS; mutex_enter(&tr->xr_lock); xhci_ring_put(sc, tr, xfer, xx->xx_trb, i); @@ -3994,6 +4032,7 @@ xhci_device_intr_start(struct usbd_xfer (usbd_xfer_isread(xfer) ? XHCI_TRB_3_ISP_BIT : 0) | XHCI_TRB_3_IOC_BIT; xhci_trb_put(&xx->xx_trb[i++], parameter, status, control); + xfer->ux_status = USBD_IN_PROGRESS; mutex_enter(&tr->xr_lock); xhci_ring_put(sc, tr, xfer, xx->xx_trb, i); @@ -4058,30 +4097,25 @@ xhci_device_intr_close(struct usbd_pipe static void xhci_timeout(void *addr) { + XHCIHIST_FUNC(); XHCIHIST_CALLED(); struct xhci_xfer * const xx = addr; struct usbd_xfer * const xfer = &xx->xx_xfer; struct xhci_softc * const sc = XHCI_XFER2SC(xfer); + struct usbd_device *dev = xfer->ux_pipe->up_dev; - XHCIHIST_FUNC(); XHCIHIST_CALLED(); - - if (sc->sc_dying) { - return; - } - - usb_init_task(&xx->xx_abort_task, xhci_timeout_task, addr, - USB_TASKQ_MPSAFE); - usb_add_task(xx->xx_xfer.ux_pipe->up_dev, &xx->xx_abort_task, - USB_TASKQ_HC); + 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 xhci_timeout_task(void *addr) { + XHCIHIST_FUNC(); XHCIHIST_CALLED(); struct usbd_xfer * const xfer = addr; struct xhci_softc * const sc = XHCI_XFER2SC(xfer); - XHCIHIST_FUNC(); XHCIHIST_CALLED(); - mutex_enter(&sc->sc_lock); xhci_abort_xfer(xfer, USBD_TIMEOUT); mutex_exit(&sc->sc_lock); Index: src/sys/dev/usb/xhcivar.h diff -u src/sys/dev/usb/xhcivar.h:1.4.8.1 src/sys/dev/usb/xhcivar.h:1.4.8.2 --- src/sys/dev/usb/xhcivar.h:1.4.8.1 Wed Apr 5 19:54:21 2017 +++ src/sys/dev/usb/xhcivar.h Sat Aug 25 14:57:35 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: xhcivar.h,v 1.4.8.1 2017/04/05 19:54:21 snj Exp $ */ +/* $NetBSD: xhcivar.h,v 1.4.8.2 2018/08/25 14:57:35 martin Exp $ */ /* * Copyright (c) 2013 Jonathan A. Kollasch @@ -35,7 +35,6 @@ struct xhci_xfer { struct usbd_xfer xx_xfer; - struct usb_task xx_abort_task; struct xhci_trb xx_trb[XHCI_XFER_NTRB]; }; @@ -85,7 +84,6 @@ struct xhci_softc { kmutex_t sc_lock; kmutex_t sc_intr_lock; - kcondvar_t sc_softwake_cv; char sc_vendor[32]; /* vendor string for root hub */ int sc_id_vendor; /* vendor ID for root hub */ Index: src/sys/external/bsd/dwc2/dwc2.c diff -u src/sys/external/bsd/dwc2/dwc2.c:1.31.2.3 src/sys/external/bsd/dwc2/dwc2.c:1.31.2.4 --- src/sys/external/bsd/dwc2/dwc2.c:1.31.2.3 Wed Jan 3 20:02:37 2018 +++ src/sys/external/bsd/dwc2/dwc2.c Sat Aug 25 14:57:35 2018 @@ -1,4 +1,4 @@ -/* $NetBSD: dwc2.c,v 1.31.2.3 2018/01/03 20:02:37 snj Exp $ */ +/* $NetBSD: dwc2.c,v 1.31.2.4 2018/08/25 14:57:35 martin Exp $ */ /*- * Copyright (c) 2013 The NetBSD Foundation, Inc. @@ -30,7 +30,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: dwc2.c,v 1.31.2.3 2018/01/03 20:02:37 snj Exp $"); +__KERNEL_RCSID(0, "$NetBSD: dwc2.c,v 1.31.2.4 2018/08/25 14:57:35 martin Exp $"); #include "opt_usb.h" @@ -203,17 +203,22 @@ dwc2_allocx(struct usbd_bus *bus, unsign { struct dwc2_softc *sc = DWC2_BUS2SC(bus); struct dwc2_xfer *dxfer; + struct usbd_xfer *xfer; DPRINTFN(10, "\n"); DWC2_EVCNT_INCR(sc->sc_ev_xferpoolget); dxfer = pool_cache_get(sc->sc_xferpool, PR_WAITOK); + xfer = (struct usbd_xfer *)dxfer; if (dxfer != NULL) { memset(dxfer, 0, sizeof(*dxfer)); dxfer->urb = dwc2_hcd_urb_alloc(sc->sc_hsotg, nframes, GFP_KERNEL); + /* Initialise this always so we can call remove on it. */ + usb_init_task(&xfer->ux_aborttask, dwc2_timeout_task, xfer, + USB_TASKQ_MPSAFE); #ifdef DIAGNOSTIC dxfer->xfer.ux_state = XFER_BUSY; #endif @@ -298,8 +303,8 @@ dwc2_softintr(void *v) * sc_complete queue */ /*XXXNH not tested */ - if (dxfer->xfer.ux_hcflags & UXFER_ABORTING) { - cv_broadcast(&dxfer->xfer.ux_hccv); + if (dxfer->xfer.ux_status == USBD_CANCELLED || + dxfer->xfer.ux_status == USBD_TIMEOUT) { continue; } @@ -316,24 +321,15 @@ Static void dwc2_timeout(void *addr) { struct usbd_xfer *xfer = addr; - struct dwc2_xfer *dxfer = DWC2_XFER2DXFER(xfer); -// struct dwc2_pipe *dpipe = DWC2_XFER2DPIPE(xfer); struct dwc2_softc *sc = DWC2_XFER2SC(xfer); + struct usbd_device *dev = xfer->ux_pipe->up_dev; DPRINTF("dxfer=%p\n", dxfer); - if (sc->sc_dying) { - mutex_enter(&sc->sc_lock); - dwc2_abort_xfer(&dxfer->xfer, USBD_TIMEOUT); - mutex_exit(&sc->sc_lock); - return; - } - - /* Execute the abort in a process context. */ - usb_init_task(&dxfer->abort_task, dwc2_timeout_task, addr, - USB_TASKQ_MPSAFE); - usb_add_task(dxfer->xfer.ux_pipe->up_dev, &dxfer->abort_task, - USB_TASKQ_HC); + 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 @@ -449,46 +445,67 @@ dwc2_abort_xfer(struct usbd_xfer *xfer, struct dwc2_softc *sc = DWC2_XFER2SC(xfer); struct dwc2_hsotg *hsotg = sc->sc_hsotg; struct dwc2_xfer *d, *tmp; - bool wake; int err; - DPRINTF("xfer=%p\n", xfer); + KASSERTMSG((status == USBD_CANCELLED || status == USBD_TIMEOUT), + "invalid status for abort: %d", (int)status); + + DPRINTF("xfer %pjx pipe %pjx status %jd", xfer, xfer->ux_pipe, status); KASSERT(mutex_owned(&sc->sc_lock)); - KASSERT(!cpu_intr_p() && !cpu_softintr_p()); + ASSERT_SLEEPABLE(); - if (sc->sc_dying) { - xfer->ux_status = status; - callout_stop(&xfer->ux_callout); - usb_transfer_complete(xfer); - return; + 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); } /* - * If an abort is already in progress then just wait for it to - * complete and return. + * 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. */ - if (xfer->ux_hcflags & UXFER_ABORTING) { - xfer->ux_status = status; - xfer->ux_hcflags |= UXFER_ABORTWAIT; - while (xfer->ux_hcflags & UXFER_ABORTING) - cv_wait(&xfer->ux_hccv, &sc->sc_lock); + 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; + + /* + * If we're dying, skip the hardware action and just notify the + * software that we're done. + */ + if (sc->sc_dying) { + DPRINTFN(4, "xfer %#jx dying %ju", (uintptr_t)xfer, + xfer->ux_status, 0, 0); + goto dying; } /* - * Step 1: Make the stack ignore it and stop the callout. + * HC Step 1: Handle the hardware. */ mutex_spin_enter(&hsotg->lock); - xfer->ux_hcflags |= UXFER_ABORTING; - - xfer->ux_status = status; /* make software ignore it */ - callout_stop(&xfer->ux_callout); - /* XXXNH suboptimal */ TAILQ_FOREACH_SAFE(d, &sc->sc_complete, xnext, tmp) { if (d == dxfer) { TAILQ_REMOVE(&sc->sc_complete, dxfer, xnext); + break; } } @@ -500,15 +517,11 @@ dwc2_abort_xfer(struct usbd_xfer *xfer, mutex_spin_exit(&hsotg->lock); /* - * Step 2: Execute callback. + * Final Step: Notify completion to waiting xfers. */ - wake = xfer->ux_hcflags & UXFER_ABORTWAIT; - xfer->ux_hcflags &= ~(UXFER_ABORTING | UXFER_ABORTWAIT); - +dying: usb_transfer_complete(xfer); - if (wake) { - cv_broadcast(&xfer->ux_hccv); - } + KASSERT(mutex_owned(&sc->sc_lock)); } Static void @@ -1118,7 +1131,7 @@ dwc2_device_start(struct usbd_xfer *xfer return USBD_IN_PROGRESS; fail2: - callout_stop(&xfer->ux_callout); + callout_halt(&xfer->ux_callout, &hsotg->lock); dwc2_urb->priv = NULL; mutex_spin_exit(&hsotg->lock); pool_cache_put(sc->sc_qtdpool, qtd); @@ -1426,6 +1439,25 @@ void dwc2_host_complete(struct dwc2_hsot return; } + /* + * If software has completed it, either by cancellation + * or timeout, drop it on the floor. + */ + if (xfer->ux_status != USBD_IN_PROGRESS) { + KASSERT(xfer->ux_status == USBD_CANCELLED || + xfer->ux_status == USBD_TIMEOUT); + 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); + dxfer = DWC2_XFER2DXFER(xfer); sc = DWC2_XFER2SC(xfer); ed = xfer->ux_pipe->up_endpoint->ue_edesc; @@ -1510,8 +1542,6 @@ void dwc2_host_complete(struct dwc2_hsot } qtd->urb = NULL; - callout_stop(&xfer->ux_callout); - KASSERT(mutex_owned(&hsotg->lock)); TAILQ_INSERT_TAIL(&sc->sc_complete, dxfer, xnext);