Module Name: src
Committed By: mrg
Date: Thu Aug 9 06:26:47 UTC 2018
Modified Files:
src/sys/dev/usb: 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: dwc2.c
Log Message:
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
--
#ifdef DIAGNOSTIC -> KASSERT and add another KASSERT
--
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.
To generate a diff of this commit:
cvs rdiff -u -r1.259 -r1.260 src/sys/dev/usb/ehci.c
cvs rdiff -u -r1.44 -r1.45 src/sys/dev/usb/ehcivar.h
cvs rdiff -u -r1.21 -r1.22 src/sys/dev/usb/motg.c
cvs rdiff -u -r1.281 -r1.282 src/sys/dev/usb/ohci.c
cvs rdiff -u -r1.59 -r1.60 src/sys/dev/usb/ohcivar.h
cvs rdiff -u -r1.280 -r1.281 src/sys/dev/usb/uhci.c
cvs rdiff -u -r1.54 -r1.55 src/sys/dev/usb/uhcivar.h
cvs rdiff -u -r1.176 -r1.177 src/sys/dev/usb/usbdi.c
cvs rdiff -u -r1.116 -r1.117 src/sys/dev/usb/usbdivar.h
cvs rdiff -u -r1.95 -r1.96 src/sys/dev/usb/xhci.c
cvs rdiff -u -r1.9 -r1.10 src/sys/dev/usb/xhcivar.h
cvs rdiff -u -r1.51 -r1.52 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.259 src/sys/dev/usb/ehci.c:1.260
--- src/sys/dev/usb/ehci.c:1.259 Wed Jun 6 01:49:09 2018
+++ src/sys/dev/usb/ehci.c Thu Aug 9 06:26:47 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: ehci.c,v 1.259 2018/06/06 01:49:09 maya Exp $ */
+/* $NetBSD: ehci.c,v 1.260 2018/08/09 06:26:47 mrg 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.259 2018/06/06 01:49:09 maya Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ehci.c,v 1.260 2018/08/09 06:26:47 mrg 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);
@@ -757,6 +756,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);
@@ -859,11 +859,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
@@ -945,7 +940,6 @@ ehci_check_qh_intr(ehci_softc_t *sc, str
}
done:
DPRINTFN(10, "ex=%#jx done", (uintptr_t)ex, 0, 0, 0);
- callout_stop(&ex->ex_xfer.ux_callout);
ehci_idone(ex, cq);
}
@@ -992,7 +986,6 @@ ehci_check_itd_intr(ehci_softc_t *sc, st
return;
done:
DPRINTF("ex %#jx done", (uintptr_t)ex, 0, 0, 0);
- callout_stop(&ex->ex_xfer.ux_callout);
ehci_idone(ex, cq);
}
@@ -1030,7 +1023,6 @@ ehci_check_sitd_intr(ehci_softc_t *sc, s
return;
DPRINTFN(10, "ex=%#jx done", (uintptr_t)ex, 0, 0, 0);
- callout_stop(&(ex->ex_xfer.ux_callout));
ehci_idone(ex, cq);
}
@@ -1038,25 +1030,39 @@ 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);
ehci_soft_qtd_t *sqtd, *fsqtd, *lsqtd;
uint32_t status = 0, nstatus = 0;
int actlen = 0;
+ bool polling = sc->sc_bus.ub_usepolling;
- EHCIHIST_FUNC(); EHCIHIST_CALLED();
-
- KASSERT(sc->sc_bus.ub_usepolling || mutex_owned(&sc->sc_lock));
+ KASSERT(polling || mutex_owned(&sc->sc_lock));
DPRINTF("ex=%#jx", (uintptr_t)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=%#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 EHCI_DEBUG
if (ex->ex_isdone) {
@@ -1340,7 +1346,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 */
@@ -1519,6 +1524,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;
@@ -2172,6 +2181,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%08jx sts = 0x%08jx",
@@ -3103,20 +3113,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);
@@ -3125,48 +3139,58 @@ 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=%#jx pipe=%#jx", (uintptr_t)xfer, (uintptr_t)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,
@@ -3202,17 +3226,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
@@ -3253,17 +3273,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));
}
@@ -3271,14 +3288,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);
@@ -3287,33 +3306,36 @@ ehci_abort_isoc_xfer(struct usbd_xfer *x
(uintptr_t)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) {
@@ -3354,53 +3376,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 %#jx", (uintptr_t)exfer, 0, 0, 0);
+ DPRINTF("xfer %#jx", (uintptr_t)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
@@ -4468,7 +4473,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;
@@ -4862,7 +4866,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.44 src/sys/dev/usb/ehcivar.h:1.45
--- src/sys/dev/usb/ehcivar.h:1.44 Mon Apr 9 16:21:11 2018
+++ src/sys/dev/usb/ehcivar.h Thu Aug 9 06:26:47 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: ehcivar.h,v 1.44 2018/04/09 16:21:11 jakllsch Exp $ */
+/* $NetBSD: ehcivar.h,v 1.45 2018/08/09 06:26:47 mrg 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,
@@ -208,8 +207,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.21 src/sys/dev/usb/motg.c:1.22
--- src/sys/dev/usb/motg.c:1.21 Mon Apr 9 16:21:11 2018
+++ src/sys/dev/usb/motg.c Thu Aug 9 06:26:47 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: motg.c,v 1.21 2018/04/09 16:21:11 jakllsch Exp $ */
+/* $NetBSD: motg.c,v 1.22 2018/08/09 06:26:47 mrg 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.21 2018/04/09 16:21:11 jakllsch Exp $");
+__KERNEL_RCSID(0, "$NetBSD: motg.c,v 1.22 2018/08/09 06:26:47 mrg Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -2153,22 +2153,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;
@@ -2196,10 +2220,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.281 src/sys/dev/usb/ohci.c:1.282
--- src/sys/dev/usb/ohci.c:1.281 Wed Jun 6 01:49:09 2018
+++ src/sys/dev/usb/ohci.c Thu Aug 9 06:26:47 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: ohci.c,v 1.281 2018/06/06 01:49:09 maya Exp $ */
+/* $NetBSD: ohci.c,v 1.282 2018/08/09 06:26:47 mrg 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.281 2018/06/06 01:49:09 maya Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ohci.c,v 1.282 2018/08/09 06:26:47 mrg Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -382,8 +382,6 @@ ohci_detach(struct ohci_softc *sc, int f
callout_halt(&sc->sc_tmo_rhsc, NULL);
callout_destroy(&sc->sc_tmo_rhsc);
- cv_destroy(&sc->sc_softwake_cv);
-
mutex_destroy(&sc->sc_lock);
mutex_destroy(&sc->sc_intr_lock);
@@ -788,7 +786,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);
@@ -1070,6 +1067,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
@@ -1388,8 +1389,9 @@ ohci_softintr(void *v)
int len, cc;
int i, j, actlen, iframes, uedir;
ohci_physaddr_t done;
+ bool polling = sc->sc_bus.ub_usepolling;
- KASSERT(sc->sc_bus.ub_usepolling || mutex_owned(&sc->sc_lock));
+ KASSERT(polling || mutex_owned(&sc->sc_lock));
OHCIHIST_FUNC(); OHCIHIST_CALLED();
@@ -1459,14 +1461,25 @@ ohci_softintr(void *v)
*/
continue;
}
- if (xfer->ux_status == USBD_CANCELLED ||
- xfer->ux_status == USBD_TIMEOUT) {
- DPRINTF("cancel/timeout %#jx", (uintptr_t)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)
@@ -1537,13 +1550,26 @@ ohci_softintr(void *v)
0);
if (xfer == NULL)
continue;
- if (xfer->ux_status == USBD_CANCELLED ||
- xfer->ux_status == USBD_TIMEOUT) {
- DPRINTF("cancel/timeout %#jx",
- (uintptr_t)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;
@@ -1597,12 +1623,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(polling || mutex_owned(&sc->sc_lock));
}
void
@@ -1889,25 +1911,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=%#jx", (uintptr_t)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=%#jx", (uintptr_t)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
@@ -2197,68 +2211,96 @@ 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=%#jx pipe=%#jx sed=%#jx", (uintptr_t)xfer,
(uintptr_t)opipe, (uintptr_t)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=%#jx", (uintptr_t)sed, 0, 0, 0);
usb_syncmem(&sed->dma, sed->offs + offsetof(ohci_ed_t, ed_flags),
sizeof(sed->ed.ed_flags),
@@ -2269,18 +2311,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
@@ -2320,7 +2358,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),
@@ -2331,15 +2369,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));
}
@@ -2839,6 +2874,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 +3083,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;
@@ -3232,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.59 src/sys/dev/usb/ohcivar.h:1.60
--- src/sys/dev/usb/ohcivar.h:1.59 Mon Apr 9 16:21:11 2018
+++ src/sys/dev/usb/ohcivar.h Thu Aug 9 06:26:47 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: ohcivar.h,v 1.59 2018/04/09 16:21:11 jakllsch Exp $ */
+/* $NetBSD: ohcivar.h,v 1.60 2018/08/09 06:26:47 mrg 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;
@@ -142,7 +139,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.280 src/sys/dev/usb/uhci.c:1.281
--- src/sys/dev/usb/uhci.c:1.280 Mon Apr 9 16:21:11 2018
+++ src/sys/dev/usb/uhci.c Thu Aug 9 06:26:47 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: uhci.c,v 1.280 2018/04/09 16:21:11 jakllsch Exp $ */
+/* $NetBSD: uhci.c,v 1.281 2018/08/09 06:26:47 mrg 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.280 2018/04/09 16:21:11 jakllsch Exp $");
+__KERNEL_RCSID(0, "$NetBSD: uhci.c,v 1.281 2018/08/09 06:26:47 mrg 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=%#jx done", (uintptr_t)ux, 0, 0, 0);
-
- callout_stop(&xfer->ux_callout);
uhci_idone(ux, cqp);
return;
}
@@ -1555,18 +1549,39 @@ 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);
uhci_soft_td_t *std;
uint32_t status = 0, nstatus;
+ bool polling = sc->sc_bus.ub_usepolling;
int actlen;
- KASSERT(sc->sc_bus.ub_usepolling || mutex_owned(&sc->sc_lock));
+ KASSERT(polling || mutex_owned(&sc->sc_lock));
- UHCIHIST_FUNC(); UHCIHIST_CALLED();
DPRINTFN(12, "ux=%#jx", (uintptr_t)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) {
@@ -1691,7 +1706,7 @@ uhci_idone(struct uhci_xfer *ux, ux_comp
if (cqp)
TAILQ_INSERT_TAIL(cqp, ux, ux_list);
- KASSERT(sc->sc_bus.ub_usepolling || mutex_owned(&sc->sc_lock));
+ KASSERT(polling || mutex_owned(&sc->sc_lock));
DPRINTFN(12, "ux=%#jx done", (uintptr_t)ux, 0, 0, 0);
}
@@ -1701,26 +1716,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 %#jx", (uintptr_t)uxfer, 0, 0, 0);
-
- if (sc->sc_dying) {
- mutex_enter(&sc->sc_lock);
- uhci_abort_xfer(xfer, USBD_TIMEOUT);
- mutex_exit(&sc->sc_lock);
- return;
- }
+ DPRINTF("xfer %#jx", (uintptr_t)xfer, 0, 0, 0);
- /* 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
@@ -2329,65 +2335,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=%#jx, status=%jd", (uintptr_t)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=%#jx", (uintptr_t)ux, 0, 0, 0);
@@ -2404,30 +2426,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.54 src/sys/dev/usb/uhcivar.h:1.55
--- src/sys/dev/usb/uhcivar.h:1.54 Mon Apr 9 16:21:11 2018
+++ src/sys/dev/usb/uhcivar.h Thu Aug 9 06:26:47 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: uhcivar.h,v 1.54 2018/04/09 16:21:11 jakllsch Exp $ */
+/* $NetBSD: uhcivar.h,v 1.55 2018/08/09 06:26:47 mrg 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.176 src/sys/dev/usb/usbdi.c:1.177
--- src/sys/dev/usb/usbdi.c:1.176 Tue Jul 31 16:44:30 2018
+++ src/sys/dev/usb/usbdi.c Thu Aug 9 06:26:47 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: usbdi.c,v 1.176 2018/07/31 16:44:30 khorben Exp $ */
+/* $NetBSD: usbdi.c,v 1.177 2018/08/09 06:26:47 mrg 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.176 2018/07/31 16:44:30 khorben Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usbdi.c,v 1.177 2018/08/09 06:26:47 mrg Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -283,6 +283,7 @@ usbd_transfer(struct usbd_xfer *xfer)
USBHIST_LOG(usbdebug,
"xfer = %#jx, flags = %#jx, pipe = %#jx, running = %jd",
(uintptr_t)xfer, xfer->ux_flags, (uintptr_t)pipe, pipe->up_running);
+ KASSERT(xfer->ux_status == USBD_NOT_STARTED);
#ifdef USB_DEBUG
if (usbdebug > 5)
@@ -348,7 +349,7 @@ usbd_transfer(struct usbd_xfer *xfer)
}
if (err != USBD_IN_PROGRESS) {
- USBHIST_LOG(usbdebug, "<- done xfer %#jx, err %jd "
+ USBHIST_LOG(usbdebug, "<- done xfer %#jx, sync (err %jd)"
"(complete/error)", (uintptr_t)xfer, err, 0, 0);
return err;
}
@@ -477,7 +478,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 %#jx", (uintptr_t)xfer, 0, 0, 0);
@@ -500,7 +500,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;
}
@@ -912,7 +911,8 @@ usb_transfer_complete(struct usbd_xfer *
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.116 src/sys/dev/usb/usbdivar.h:1.117
--- src/sys/dev/usb/usbdivar.h:1.116 Fri Jun 29 17:48:24 2018
+++ src/sys/dev/usb/usbdivar.h Thu Aug 9 06:26:47 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: usbdivar.h,v 1.116 2018/06/29 17:48:24 msaitoh Exp $ */
+/* $NetBSD: usbdivar.h,v 1.117 2018/08/09 06:26:47 mrg Exp $ */
/*
* Copyright (c) 1998, 2012 The NetBSD Foundation, Inc.
@@ -285,11 +285,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.95 src/sys/dev/usb/xhci.c:1.96
--- src/sys/dev/usb/xhci.c:1.95 Wed Jul 18 10:44:17 2018
+++ src/sys/dev/usb/xhci.c Thu Aug 9 06:26:47 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: xhci.c,v 1.95 2018/07/18 10:44:17 msaitoh Exp $ */
+/* $NetBSD: xhci.c,v 1.96 2018/08/09 06:26:47 mrg Exp $ */
/*
* Copyright (c) 2013 Jonathan A. Kollasch
@@ -34,7 +34,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: xhci.c,v 1.95 2018/07/18 10:44:17 msaitoh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: xhci.c,v 1.96 2018/08/09 06:26:47 mrg Exp $");
#ifdef _KERNEL_OPT
#include "opt_usb.h"
@@ -1696,55 +1696,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 %#jx pipe %#jx status %jd",
(uintptr_t)xfer, (uintptr_t)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 %#jx dying %ju", (uintptr_t)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 %#jx waiting for abort to finish",
- (uintptr_t)xfer, 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;
- }
- xfer->ux_hcflags |= UXFER_ABORTING;
+
+ /* 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:
@@ -1763,19 +1772,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));
@@ -2006,7 +2011,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 %#jx",
(uintptr_t)xfer, 0, 0, 0);
return;
@@ -2017,7 +2023,6 @@ xhci_event_transfer(struct xhci_softc *
case XHCI_TRB_ERROR_BABBLE:
DPRINTFN(1, "ERR %ju slot %ju dci %ju", 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
@@ -2031,8 +2036,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:
@@ -2040,15 +2056,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);
}
}
@@ -2213,6 +2246,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
@@ -3800,6 +3835,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);
@@ -3917,6 +3953,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);
@@ -4026,6 +4063,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;
if (!polling)
mutex_enter(&tr->xr_lock);
@@ -4093,30 +4131,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.9 src/sys/dev/usb/xhcivar.h:1.10
--- src/sys/dev/usb/xhcivar.h:1.9 Mon Apr 9 16:21:11 2018
+++ src/sys/dev/usb/xhcivar.h Thu Aug 9 06:26:47 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: xhcivar.h,v 1.9 2018/04/09 16:21:11 jakllsch Exp $ */
+/* $NetBSD: xhcivar.h,v 1.10 2018/08/09 06:26:47 mrg 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;
pool_cache_t sc_xferpool;
Index: src/sys/external/bsd/dwc2/dwc2.c
diff -u src/sys/external/bsd/dwc2/dwc2.c:1.51 src/sys/external/bsd/dwc2/dwc2.c:1.52
--- src/sys/external/bsd/dwc2/dwc2.c:1.51 Tue Aug 7 16:35:08 2018
+++ src/sys/external/bsd/dwc2/dwc2.c Thu Aug 9 06:26:47 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: dwc2.c,v 1.51 2018/08/07 16:35:08 skrll Exp $ */
+/* $NetBSD: dwc2.c,v 1.52 2018/08/09 06:26:47 mrg Exp $ */
/*-
* Copyright (c) 2013 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: dwc2.c,v 1.51 2018/08/07 16:35:08 skrll Exp $");
+__KERNEL_RCSID(0, "$NetBSD: dwc2.c,v 1.52 2018/08/09 06:26:47 mrg 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
@@ -1112,7 +1125,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);
@@ -1414,6 +1427,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;
@@ -1500,8 +1532,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);
