tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

usbdi(9) improvements



The attached patch series makes several improvements to the usbdi(9)
API (the USB device driver interface) and to the kernel's USB host
controller interface API, and fixes bugs in xhci(4) abort/stall.

I'm not quite ready to commit this because (a) I want to make some
more improvements to the host controller interface API, and (b) I need
help testing and auditing drivers for a change to usbd_abort_pipe.
Review, testing, and comments welcome!


Summary of changes to usbdi(9):

- The following functions which previously returned a usbd_status but
  never failed in practice now return void:

  . usbd_close_pipe
  . usbd_abort_pipe
  . usbd_abort_defaut_pipe

- usbd_transfer no longer races with usbd_abort_pipe to detect when
  the pipe is currently being aborted.

- usbd_abort_pipe is now persistent, not transient: new xfers will
  continue to fail with USBD_CANCELLED even after it returns.

  Previously, usbd_abort_pipe would abort any pending xfers, cause new
  xfers to fail with USDB_CANCELLED _while aborting_, wait for the
  pending xfers to complete, and then allow new xfers to be submitted
  again.

  This causes a race leading to deadlock and/or use-after-free with
  synchronous xfers, such as usbd_sync_transfer or usbd_bulk_transfer.
  Many drivers have logic where one thread (e.g., in read or write on
  a /dev node) is doing I/O and another thread (e.g., in close or
  revoke, or when the device is detached) is trying to abort the I/O:

	/* I/O thread */
	if (sc->sc_dying)
		return EIO;
	// (*)
	usbd_bulk_transfer(...);

	/* abort thread */
	sc->sc_dying = true;
	usbd_abort_pipe(...);
	wait_for_io_thread(...);
	usbd_destroy_xfer(...);

  If the abort logic starts, and finishes, concurrently with the I/O
  thread at the line marked (*), then there is a race: the I/O thread
  doesn't notice sc_dying has been set, and can start to issue a new
  xfer on the pipe; and the abort thread doesn't notice that a new
  xfer may have been issued so it either deadlocks waiting for the I/O
  thread (now waiting indefinitely for the xfer) or proceeds to free
  data structures like the xfer itself which the I/O thread is trying
  to use.

  With this change, usbd_abort_pipe is persistent -- and mandatory
  before usbd_close_pipe, detected by KASSERT.

  This fixes the example above.  I haven't found any other generic way
  to fix this example without changing the API -- it is tempting, for
  instance, to put a lock around both threads, but this deadlocks
  because usbd_bulk_transfer is synchronous.  We could put an
  interlock into the usbd_bulk_transfer which it would release once it
  has the pipe lock, but that's a lot more complexity for little gain.

  CAVEAT: There may be some (badly-written) drivers that rely on
  usbd_abort_pipe to be transient.  For example, it looks like ubt(4)
  might do this.  So it may have to be fixed -- need to close the pipe
  and reopen it later, like usbnet(9) does.

Summary of changes to host controller interface API:

- struct usbd_pipe_methods::upm_transfer is no longer responsible for
  calling usb_insert_transfer to insert the transfer in the pipe's
  queue.  usbd_transfer takes care of that itself.

  This was necessary to fix the usbd_transfer / usbd_abort_pipe race
  mentioned above.

  XXX This part isn't done yet -- I also want to keep the pipe's queue
  locked across .upm_transfer _and_ .upm_start, because right now,
  except for dwc2, every hci driver abuses access to the pipe's queue
  without the lock in .upm_transfer.  This change actually makes dwc2
  slightly worse, but once the lock can cover both .upm_transfer and
  .upm_start, it will be back to what it was before, and all the other
  hci drivers will be better off.

- struct usbd_bus_methods::ubm_abortx is no longer responsible for
  calling usb_transfer_complete -- usbd_xfer_abort now does that.
  Note that .ubm_abortx MUST NOT release the pipe (bus) lock; this was
  a bug in xhci(4) abort.
>From 7b3c6d322dbbc91fcee42bc66776f1827f81d619 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sat, 8 Jan 2022 13:54:12 +0000
Subject: [PATCH 01/13] usb: Factor usb_insert_transfer out of upm_transfer and
 make private.

Almost every upm_transfer function starts with:

	mutex_enter(&sc->sc_lock);
	err = usb_insert_transfer(xfer);
	mutex_exit(&sc->sc_lock);
	if (err)
		return err;

Some of them have debug messages sprinkled in here too, or assert
that err == USBD_NORMAL_COMPLETION (alternative is USBD_IN_PROGRESS,
only for pipes with up_running or up_serialise, presumably not
applicable for these types of pipes).  Some of them also assert
xfer->ux_status == USBD_NOT_STARTED, which is guaranteed on entry and
preserved by usb_insert_transer.

Exceptions:

- arch/mips/adm5120/dev/ahci.c ahci_device_isoc_transfer just returns
  USBD_NORMAL_COMPLETION, but I'm pretty sure this is and always has
  been broken anyway, so won't make anything worse (if anything, might
  make it better...)

- external/bsd/dwc2/dwc2.c dwc2_device_bulk_transfer and
  dwc2_device_isoc_transfer _also_ issue dwc2_device_start(xfer)
  under the lock.  This is probably a better way to do it, but let's
  do it uniformly across all HCIs at once.

- rump/dev/lib/libugenhc/ugenhc.c rumpusb_device_bulk_transfer
  sometimes returns USBD_IN_PROGRESS _without_ queueing the transfer,
  in the !rump_threads case.  Not really sure how this is supposed to
  work...  If it actually breaks anything, we can figure it out.
---
 sys/arch/mips/adm5120/dev/ahci.c    | 38 +----------------
 sys/dev/ic/sl811hs.c                | 17 +-------
 sys/dev/usb/ehci.c                  | 57 +------------------------
 sys/dev/usb/motg.c                  | 46 ++-------------------
 sys/dev/usb/ohci.c                  | 46 ---------------------
 sys/dev/usb/uhci.c                  | 64 ++---------------------------
 sys/dev/usb/usbdi.c                 | 12 +++++-
 sys/dev/usb/usbdivar.h              |  2 -
 sys/dev/usb/usbroothub.c            | 10 -----
 sys/dev/usb/vhci.c                  | 18 --------
 sys/dev/usb/xhci.c                  | 60 +--------------------------
 sys/external/bsd/dwc2/dwc2.c        | 39 +-----------------
 sys/rump/dev/lib/libugenhc/ugenhc.c | 29 -------------
 13 files changed, 24 insertions(+), 414 deletions(-)

diff --git a/sys/arch/mips/adm5120/dev/ahci.c b/sys/arch/mips/adm5120/dev/ahci.c
index 2b98e4ae221e..340650e2a13a 100644
--- a/sys/arch/mips/adm5120/dev/ahci.c
+++ b/sys/arch/mips/adm5120/dev/ahci.c
@@ -729,22 +729,10 @@ ahci_roothub_ctrl(struct usbd_bus *bus, usb_device_request_t *req,
 static usbd_status
 ahci_root_intr_transfer(struct usbd_xfer *xfer)
 {
-	struct ahci_softc *sc = AHCI_XFER2SC(xfer);
-	usbd_status error;
 
 	DPRINTF(D_TRACE, ("SLRItransfer "));
 
-	/* Insert last in queue */
-	mutex_enter(&sc->sc_lock);
-	error = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (error)
-		return error;
-
-	/*
-	 * Pipe isn't running (otherwise error would be USBD_INPROG),
-	 * start first.
-	 */
+	/* Pipe isn't running, start first.  */
 	return ahci_root_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -827,17 +815,9 @@ ahci_root_intr_done(struct usbd_xfer *xfer)
 static usbd_status
 ahci_device_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	struct ahci_softc *sc = AHCI_XFER2SC(xfer);
-	usbd_status error;
 
 	DPRINTF(D_TRACE, ("C"));
 
-	mutex_enter(&sc->sc_lock);
-	error = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (error)
-		return error;
-
 	return ahci_device_ctrl_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -1017,17 +997,9 @@ ahci_device_ctrl_done(struct usbd_xfer *xfer)
 static usbd_status
 ahci_device_intr_transfer(struct usbd_xfer *xfer)
 {
-	struct ahci_softc *sc = AHCI_XFER2SC(xfer);
-	usbd_status error;
 
 	DPRINTF(D_TRACE, ("INTRtrans "));
 
-	mutex_enter(&sc->sc_lock);
-	error = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (error)
-		return error;
-
 	return ahci_device_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -1161,17 +1133,9 @@ ahci_device_isoc_done(struct usbd_xfer *xfer)
 static usbd_status
 ahci_device_bulk_transfer(struct usbd_xfer *xfer)
 {
-	struct ahci_softc *sc = AHCI_XFER2SC(xfer);
-	usbd_status error;
 
 	DPRINTF(D_TRACE, ("B"));
 
-	mutex_enter(&sc->sc_lock);
-	error = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (error)
-		return error;
-
 	return ahci_device_bulk_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
diff --git a/sys/dev/ic/sl811hs.c b/sys/dev/ic/sl811hs.c
index e5773430ccaf..35fea39878d1 100644
--- a/sys/dev/ic/sl811hs.c
+++ b/sys/dev/ic/sl811hs.c
@@ -839,28 +839,13 @@ usbd_status
 slhci_transfer(struct usbd_xfer *xfer)
 {
 	SLHCIHIST_FUNC(); SLHCIHIST_CALLED();
-	struct slhci_softc *sc = SLHCI_XFER2SC(xfer);
 	usbd_status error;
 
 	DLOG(D_TRACE, "transfer type %jd xfer %#jx spipe %#jx ",
 	    SLHCI_XFER_TYPE(xfer), (uintptr_t)xfer, (uintptr_t)xfer->ux_pipe,
 	    0);
 
-	/* Insert last in queue */
-	mutex_enter(&sc->sc_lock);
-	error = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (error) {
-		if (error != USBD_IN_PROGRESS)
-			DLOG(D_ERR, "usb_insert_transfer returns %jd!", error,
-			    0,0,0);
-		return error;
-	}
-
-	/*
-	 * Pipe isn't running (otherwise error would be USBD_INPROG),
-	 * so start it first.
-	 */
+	/* Pipe isn't running, so start it first.  */
 
 	/*
 	 * Start will take the lock.
diff --git a/sys/dev/usb/ehci.c b/sys/dev/usb/ehci.c
index f275b15ad1a8..9a68e7ff87b6 100644
--- a/sys/dev/usb/ehci.c
+++ b/sys/dev/usb/ehci.c
@@ -2748,15 +2748,6 @@ ehci_disown(ehci_softc_t *sc, int index, int lowspeed)
 Static usbd_status
 ehci_root_intr_transfer(struct usbd_xfer *xfer)
 {
-	ehci_softc_t *sc = EHCI_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
 	/* Pipe isn't running, start first */
 	return ehci_root_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
@@ -3606,15 +3597,6 @@ ehci_device_ctrl_fini(struct usbd_xfer *xfer)
 Static usbd_status
 ehci_device_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	ehci_softc_t *sc = EHCI_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
 	/* Pipe isn't running, start first */
 	return ehci_device_ctrl_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
@@ -3886,15 +3868,6 @@ ehci_device_bulk_fini(struct usbd_xfer *xfer)
 Static usbd_status
 ehci_device_bulk_transfer(struct usbd_xfer *xfer)
 {
-	ehci_softc_t *sc = EHCI_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
 	/* Pipe isn't running, start first */
 	return ehci_device_bulk_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
@@ -4099,20 +4072,8 @@ ehci_device_intr_fini(struct usbd_xfer *xfer)
 Static usbd_status
 ehci_device_intr_transfer(struct usbd_xfer *xfer)
 {
-	ehci_softc_t *sc = EHCI_XFER2SC(xfer);
-	usbd_status err;
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
-	/*
-	 * Pipe isn't running (otherwise err would be USBD_INPROG),
-	 * so start it first.
-	 */
+	/* Pipe isn't running, so start it first.  */
 	return ehci_device_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -4351,14 +4312,6 @@ Static usbd_status
 ehci_device_fs_isoc_transfer(struct usbd_xfer *xfer)
 {
 	ehci_softc_t *sc = EHCI_XFER2SC(xfer);
-	usbd_status __diagused err;
-
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-
-	KASSERT(err == USBD_NORMAL_COMPLETION);
-
 	struct ehci_pipe *epipe = EHCI_XFER2EPIPE(xfer);
 	struct usbd_device *dev = xfer->ux_pipe->up_dev;
 	struct ehci_xfer *exfer = EHCI_XFER2EXFER(xfer);
@@ -4724,14 +4677,6 @@ Static usbd_status
 ehci_device_isoc_transfer(struct usbd_xfer *xfer)
 {
 	ehci_softc_t *sc = EHCI_XFER2SC(xfer);
-	usbd_status __diagused err;
-
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-
-	KASSERT(err == USBD_NORMAL_COMPLETION);
-
 	struct ehci_pipe *epipe = EHCI_XFER2EPIPE(xfer);
 	struct ehci_xfer *exfer = EHCI_XFER2EXFER(xfer);
 	ehci_soft_itd_t *itd, *prev;
diff --git a/sys/dev/usb/motg.c b/sys/dev/usb/motg.c
index 63339b4f0d54..54b6afcffb93 100644
--- a/sys/dev/usb/motg.c
+++ b/sys/dev/usb/motg.c
@@ -1009,20 +1009,8 @@ motg_root_intr_abort(struct usbd_xfer *xfer)
 usbd_status
 motg_root_intr_transfer(struct usbd_xfer *xfer)
 {
-	struct motg_softc *sc = MOTG_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
-	/*
-	 * Pipe isn't running (otherwise err would be USBD_INPROG),
-	 * start first
-	 */
+	/* Pipe isn't running, start first */
 	return motg_root_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -1279,21 +1267,8 @@ motg_setup_endpoint_rx(struct usbd_xfer *xfer)
 static usbd_status
 motg_device_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	struct motg_softc *sc = MOTG_XFER2SC(xfer);
-	usbd_status err;
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	KASSERT(xfer->ux_status == USBD_NOT_STARTED);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
-	/*
-	 * Pipe isn't running (otherwise err would be USBD_INPROG),
-	 * so start it first.
-	 */
+	/* Pipe isn't running, so start it first.  */
 	return motg_device_ctrl_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -1731,24 +1706,9 @@ motg_device_ctrl_done(struct usbd_xfer *xfer)
 static usbd_status
 motg_device_data_transfer(struct usbd_xfer *xfer)
 {
-	struct motg_softc *sc = MOTG_XFER2SC(xfer);
-	usbd_status err;
-
 	MOTGHIST_FUNC(); MOTGHIST_CALLED();
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	DPRINTF("xfer %#jx status %jd", (uintptr_t)xfer, xfer->ux_status, 0, 0);
-	err = usb_insert_transfer(xfer);
-	KASSERT(xfer->ux_status == USBD_NOT_STARTED);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
-	/*
-	 * Pipe isn't running (otherwise err would be USBD_INPROG),
-	 * so start it first.
-	 */
+	/* Pipe isn't running, so start it first.  */
 	return motg_device_data_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
diff --git a/sys/dev/usb/ohci.c b/sys/dev/usb/ohci.c
index bd8ab4f33a27..3bbb0a01e239 100644
--- a/sys/dev/usb/ohci.c
+++ b/sys/dev/usb/ohci.c
@@ -2616,15 +2616,6 @@ ohci_roothub_ctrl(struct usbd_bus *bus, usb_device_request_t *req,
 Static usbd_status
 ohci_root_intr_transfer(struct usbd_xfer *xfer)
 {
-	ohci_softc_t *sc = OHCI_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
 	/* Pipe isn't running, start first */
 	return ohci_root_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
@@ -2774,15 +2765,6 @@ ohci_device_ctrl_fini(struct usbd_xfer *xfer)
 Static usbd_status
 ohci_device_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	ohci_softc_t *sc = OHCI_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
 	/* Pipe isn't running, start first */
 	return ohci_device_ctrl_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
@@ -3062,15 +3044,6 @@ ohci_device_bulk_fini(struct usbd_xfer *xfer)
 Static usbd_status
 ohci_device_bulk_transfer(struct usbd_xfer *xfer)
 {
-	ohci_softc_t *sc = OHCI_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
 	/* Pipe isn't running, start first */
 	return ohci_device_bulk_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
@@ -3270,15 +3243,6 @@ ohci_device_intr_fini(struct usbd_xfer *xfer)
 Static usbd_status
 ohci_device_intr_transfer(struct usbd_xfer *xfer)
 {
-	ohci_softc_t *sc = OHCI_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
 	/* Pipe isn't running, start first */
 	return ohci_device_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
@@ -3574,20 +3538,10 @@ ohci_device_isoc_fini(struct usbd_xfer *xfer)
 usbd_status
 ohci_device_isoc_transfer(struct usbd_xfer *xfer)
 {
-	ohci_softc_t *sc = OHCI_XFER2SC(xfer);
-	usbd_status __diagused err;
-
 	OHCIHIST_FUNC(); OHCIHIST_CALLED();
 
 	DPRINTFN(5, "xfer=%#jx", (uintptr_t)xfer, 0, 0, 0);
 
-	/* Put it on our queue, */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-
-	KASSERT(err == USBD_NORMAL_COMPLETION);
-
 	/* insert into schedule, */
 	ohci_device_isoc_enter(xfer);
 
diff --git a/sys/dev/usb/uhci.c b/sys/dev/usb/uhci.c
index acf5e2e32c11..99531c10b753 100644
--- a/sys/dev/usb/uhci.c
+++ b/sys/dev/usb/uhci.c
@@ -2258,20 +2258,8 @@ uhci_device_bulk_fini(struct usbd_xfer *xfer)
 usbd_status
 uhci_device_bulk_transfer(struct usbd_xfer *xfer)
 {
-	uhci_softc_t *sc = UHCI_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
-	/*
-	 * Pipe isn't running (otherwise err would be USBD_INPROG),
-	 * so start it first.
-	 */
+	/* Pipe isn't running, so start it first.  */
 	return uhci_device_bulk_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -2495,20 +2483,8 @@ uhci_device_ctrl_fini(struct usbd_xfer *xfer)
 usbd_status
 uhci_device_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	uhci_softc_t *sc = UHCI_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
-	/*
-	 * Pipe isn't running (otherwise err would be USBD_INPROG),
-	 * so start it first.
-	 */
+	/* Pipe isn't running, so start it first.  */
 	return uhci_device_ctrl_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -2701,20 +2677,8 @@ uhci_device_intr_fini(struct usbd_xfer *xfer)
 usbd_status
 uhci_device_intr_transfer(struct usbd_xfer *xfer)
 {
-	uhci_softc_t *sc = UHCI_XFER2SC(xfer);
-	usbd_status err;
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
-	/*
-	 * Pipe isn't running (otherwise err would be USBD_INPROG),
-	 * so start it first.
-	 */
+	/* Pipe isn't running, so start it first.  */
 	return uhci_device_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -2891,18 +2855,10 @@ usbd_status
 uhci_device_isoc_transfer(struct usbd_xfer *xfer)
 {
 	uhci_softc_t *sc = UHCI_XFER2SC(xfer);
-	usbd_status err __diagused;
 
 	UHCIHIST_FUNC(); UHCIHIST_CALLED();
 	DPRINTFN(5, "xfer=%#jx", (uintptr_t)xfer, 0, 0, 0);
 
-	/* Put it on our queue, */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-
-	KASSERT(err == USBD_NORMAL_COMPLETION);
-
 	/* insert into schedule, */
 
 	struct uhci_pipe *upipe = UHCI_PIPE2UPIPE(xfer->ux_pipe);
@@ -3890,20 +3846,8 @@ uhci_root_intr_abort(struct usbd_xfer *xfer)
 usbd_status
 uhci_root_intr_transfer(struct usbd_xfer *xfer)
 {
-	uhci_softc_t *sc = UHCI_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
-	/*
-	 * Pipe isn't running (otherwise err would be USBD_INPROG),
-	 * start first
-	 */
+	/* Pipe isn't running, start first */
 	return uhci_root_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
diff --git a/sys/dev/usb/usbdi.c b/sys/dev/usb/usbdi.c
index ea5a3430dd03..07406ee96fa8 100644
--- a/sys/dev/usb/usbdi.c
+++ b/sys/dev/usb/usbdi.c
@@ -116,6 +116,7 @@ SDT_PROBE_DEFINE2(usb, device, xfer, done,
 SDT_PROBE_DEFINE1(usb, device, xfer, destroy,  "struct usbd_xfer *"/*xfer*/);
 
 Static usbd_status usbd_ar_pipe(struct usbd_pipe *);
+static usbd_status usb_insert_transfer(struct usbd_xfer *);
 Static void usbd_start_next(struct usbd_pipe *);
 Static usbd_status usbd_open_pipe_ival
 	(struct usbd_interface *, uint8_t, uint8_t, struct usbd_pipe **, int);
@@ -408,7 +409,14 @@ usbd_transfer(struct usbd_xfer *xfer)
 
 	/* xfer is not valid after the transfer method unless synchronous */
 	SDT_PROBE2(usb, device, pipe, transfer__start,  pipe, xfer);
-	err = pipe->up_methods->upm_transfer(xfer);
+	do {
+		usbd_lock_pipe(pipe);
+		err = usb_insert_transfer(xfer);
+		usbd_unlock_pipe(pipe);
+		if (err)
+			break;
+		err = pipe->up_methods->upm_transfer(xfer);
+	} while (0);
 	SDT_PROBE3(usb, device, pipe, transfer__done,  pipe, xfer, err);
 
 	if (err != USBD_IN_PROGRESS && err) {
@@ -1138,7 +1146,7 @@ usb_transfer_complete(struct usbd_xfer *xfer)
 }
 
 /* Called with USB lock held. */
-usbd_status
+static usbd_status
 usb_insert_transfer(struct usbd_xfer *xfer)
 {
 	struct usbd_pipe *pipe = xfer->ux_pipe;
diff --git a/sys/dev/usb/usbdivar.h b/sys/dev/usb/usbdivar.h
index e993fc25b7a1..68acac665e9e 100644
--- a/sys/dev/usb/usbdivar.h
+++ b/sys/dev/usb/usbdivar.h
@@ -64,7 +64,6 @@
  * USB functions known to expect the lock taken include (this list is
  * probably not exhaustive):
  *    usb_transfer_complete()
- *    usb_insert_transfer()
  *    usb_start_next()
  *
  */
@@ -356,7 +355,6 @@ void		usbd_iface_pipeunref(struct usbd_interface *);
 usbd_status	usbd_fill_iface_data(struct usbd_device *, int, int);
 void		usb_free_device(struct usbd_device *);
 
-usbd_status	usb_insert_transfer(struct usbd_xfer *);
 void		usb_transfer_complete(struct usbd_xfer *);
 int		usb_disconnect_port(struct usbd_port *, device_t, int);
 
diff --git a/sys/dev/usb/usbroothub.c b/sys/dev/usb/usbroothub.c
index 79010ba90fd4..f3cb54bed973 100644
--- a/sys/dev/usb/usbroothub.c
+++ b/sys/dev/usb/usbroothub.c
@@ -345,16 +345,6 @@ static const usb_hub_descriptor_t usbroothub_hubd = {
 usbd_status
 roothub_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	struct usbd_pipe *pipe = xfer->ux_pipe;
-	struct usbd_bus *bus = pipe->up_dev->ud_bus;
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(bus->ub_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(bus->ub_lock);
-	if (err)
-		return err;
 
 	/* Pipe isn't running, start first */
 	return roothub_ctrl_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
diff --git a/sys/dev/usb/vhci.c b/sys/dev/usb/vhci.c
index f5c46b9a774e..a73b14e1cb5a 100644
--- a/sys/dev/usb/vhci.c
+++ b/sys/dev/usb/vhci.c
@@ -590,18 +590,9 @@ vhci_roothub_ctrl(struct usbd_bus *bus, usb_device_request_t *req,
 static usbd_status
 vhci_device_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	vhci_softc_t *sc = xfer->ux_bus->ub_hcpriv;
-	usbd_status err;
 
 	DPRINTF("%s: called\n", __func__);
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
 	/* Pipe isn't running, start first */
 	return vhci_device_ctrl_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
@@ -707,18 +698,9 @@ vhci_device_ctrl_done(struct usbd_xfer *xfer)
 static usbd_status
 vhci_root_intr_transfer(struct usbd_xfer *xfer)
 {
-	vhci_softc_t *sc = xfer->ux_bus->ub_hcpriv;
-	usbd_status err;
 
 	DPRINTF("%s: called\n", __func__);
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
 	/* Pipe isn't running, start first */
 	return vhci_root_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
diff --git a/sys/dev/usb/xhci.c b/sys/dev/usb/xhci.c
index cb40eb7832eb..051f656c7dee 100644
--- a/sys/dev/usb/xhci.c
+++ b/sys/dev/usb/xhci.c
@@ -4172,18 +4172,8 @@ xhci_roothub_ctrl(struct usbd_bus *bus, usb_device_request_t *req,
 static usbd_status
 xhci_root_intr_transfer(struct usbd_xfer *xfer)
 {
-	struct xhci_softc * const sc = XHCI_XFER2SC(xfer);
-	usbd_status err;
-
 	XHCIHIST_FUNC(); XHCIHIST_CALLED();
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
 	/* Pipe isn't running, start first */
 	return xhci_root_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
@@ -4276,18 +4266,8 @@ xhci_root_intr_done(struct usbd_xfer *xfer)
 static usbd_status
 xhci_device_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	struct xhci_softc * const sc = XHCI_XFER2SC(xfer);
-	usbd_status err;
-
 	XHCIHIST_FUNC(); XHCIHIST_CALLED();
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
 	/* Pipe isn't running, start first */
 	return xhci_device_ctrl_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
@@ -4409,18 +4389,8 @@ xhci_device_ctrl_close(struct usbd_pipe *pipe)
 static usbd_status
 xhci_device_isoc_transfer(struct usbd_xfer *xfer)
 {
-	struct xhci_softc * const sc = XHCI_XFER2SC(xfer);
-	usbd_status err;
-
 	XHCIHIST_FUNC(); XHCIHIST_CALLED();
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
 	return xhci_device_isoc_enter(xfer);
 }
 
@@ -4573,22 +4543,9 @@ xhci_device_isoc_done(struct usbd_xfer *xfer)
 static usbd_status
 xhci_device_bulk_transfer(struct usbd_xfer *xfer)
 {
-	struct xhci_softc * const sc = XHCI_XFER2SC(xfer);
-	usbd_status err;
-
 	XHCIHIST_FUNC(); XHCIHIST_CALLED();
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
-	/*
-	 * Pipe isn't running (otherwise err would be USBD_INPROG),
-	 * so start it first.
-	 */
+	/* Pipe isn't running, so start it first.  */
 	return xhci_device_bulk_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -4699,22 +4656,9 @@ xhci_device_bulk_close(struct usbd_pipe *pipe)
 static usbd_status
 xhci_device_intr_transfer(struct usbd_xfer *xfer)
 {
-	struct xhci_softc * const sc = XHCI_XFER2SC(xfer);
-	usbd_status err;
-
 	XHCIHIST_FUNC(); XHCIHIST_CALLED();
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
-	/*
-	 * Pipe isn't running (otherwise err would be USBD_INPROG),
-	 * so start it first.
-	 */
+	/* Pipe isn't running, so start it first.  */
 	return xhci_device_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
diff --git a/sys/external/bsd/dwc2/dwc2.c b/sys/external/bsd/dwc2/dwc2.c
index e3e61a072b23..921f5fbed998 100644
--- a/sys/external/bsd/dwc2/dwc2.c
+++ b/sys/external/bsd/dwc2/dwc2.c
@@ -614,18 +614,9 @@ dwc2_roothub_ctrl(struct usbd_bus *bus, usb_device_request_t *req,
 Static usbd_status
 dwc2_root_intr_transfer(struct usbd_xfer *xfer)
 {
-	struct dwc2_softc *sc = DWC2_XFER2SC(xfer);
-	usbd_status err;
 
 	DPRINTF("\n");
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
 	/* Pipe isn't running, start first */
 	return dwc2_root_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
@@ -711,18 +702,9 @@ dwc2_root_intr_done(struct usbd_xfer *xfer)
 Static usbd_status
 dwc2_device_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	struct dwc2_softc *sc = DWC2_XFER2SC(xfer);
-	usbd_status err;
 
 	DPRINTF("\n");
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
 	/* Pipe isn't running, start first */
 	return dwc2_device_ctrl_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
@@ -788,12 +770,8 @@ dwc2_device_bulk_transfer(struct usbd_xfer *xfer)
 
 	DPRINTF("xfer=%p\n", xfer);
 
-	/* Insert last in queue. */
 	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-
-	KASSERT(err == USBD_NORMAL_COMPLETION);
-
+	KASSERT(xfer->ux_status == USBD_NOT_STARTED);
 	xfer->ux_status = USBD_IN_PROGRESS;
 	err = dwc2_device_start(xfer);
 	mutex_exit(&sc->sc_lock);
@@ -833,18 +811,9 @@ dwc2_device_bulk_done(struct usbd_xfer *xfer)
 Static usbd_status
 dwc2_device_intr_transfer(struct usbd_xfer *xfer)
 {
-	struct dwc2_softc *sc = DWC2_XFER2SC(xfer);
-	usbd_status err;
 
 	DPRINTF("xfer=%p\n", xfer);
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
 	/* Pipe isn't running, start first */
 	return dwc2_device_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
@@ -909,12 +878,8 @@ dwc2_device_isoc_transfer(struct usbd_xfer *xfer)
 
 	DPRINTF("xfer=%p\n", xfer);
 
-	/* Insert last in queue. */
 	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-
-	KASSERT(err == USBD_NORMAL_COMPLETION);
-
+	KASSERT(xfer->ux_status == USBD_NOT_STARTED);
 	xfer->ux_status = USBD_IN_PROGRESS;
 	err = dwc2_device_start(xfer);
 	mutex_exit(&sc->sc_lock);
diff --git a/sys/rump/dev/lib/libugenhc/ugenhc.c b/sys/rump/dev/lib/libugenhc/ugenhc.c
index 003f1cc54453..f6f80877940e 100644
--- a/sys/rump/dev/lib/libugenhc/ugenhc.c
+++ b/sys/rump/dev/lib/libugenhc/ugenhc.c
@@ -401,14 +401,6 @@ rumpusb_device_ctrl_start(struct usbd_xfer *xfer)
 static usbd_status
 rumpusb_device_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	struct ugenhc_softc *sc = UGENHC_XFER2SC(xfer);
-	usbd_status err;
-
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
 	return rumpusb_device_ctrl_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
@@ -540,14 +532,6 @@ rumpusb_root_intr_start(struct usbd_xfer *xfer)
 static usbd_status
 rumpusb_root_intr_transfer(struct usbd_xfer *xfer)
 {
-	struct ugenhc_softc *sc = UGENHC_XFER2SC(xfer);
-	usbd_status err;
-
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
 	return rumpusb_root_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
@@ -709,8 +693,6 @@ doxfer_kth(void *arg)
 static usbd_status
 rumpusb_device_bulk_transfer(struct usbd_xfer *xfer)
 {
-	struct ugenhc_softc *sc = UGENHC_XFER2SC(xfer);
-	usbd_status err;
 
 	if (!rump_threads) {
 		/* XXX: lie about supporting async transfers */
@@ -720,20 +702,9 @@ rumpusb_device_bulk_transfer(struct usbd_xfer *xfer)
 			return USBD_IN_PROGRESS;
 		}
 
-		mutex_enter(&sc->sc_lock);
-		err = usb_insert_transfer(xfer);
-		mutex_exit(&sc->sc_lock);
-		if (err)
-			return err;
-
 		return rumpusb_device_bulk_start(
 		    SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 	} else {
-		mutex_enter(&sc->sc_lock);
-		err = usb_insert_transfer(xfer);
-		mutex_exit(&sc->sc_lock);
-		if (err)
-			return err;
 		kthread_create(PRI_NONE, 0, NULL, doxfer_kth, xfer->ux_pipe, NULL,
 		    "rusbhcxf");
 

>From 8384740d815db4da8fb4e00fdd5fc152d72f1557 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sat, 8 Jan 2022 17:47:22 +0000
Subject: [PATCH 02/13] usb: usbd_abort_pipe never fails.  Make it return void.

Prune dead branches as a result of this change.
---
 sys/dev/usb/if_atu.c | 12 ++----------
 sys/dev/usb/ualea.c  |  2 +-
 sys/dev/usb/usbdi.c  | 15 ++++++---------
 sys/dev/usb/usbdi.h  |  4 ++--
 sys/dev/usb/usbnet.c | 15 +++------------
 sys/dev/usb/utoppy.c |  7 ++-----
 6 files changed, 16 insertions(+), 39 deletions(-)

diff --git a/sys/dev/usb/if_atu.c b/sys/dev/usb/if_atu.c
index 85222d1dce28..9d37935701b9 100644
--- a/sys/dev/usb/if_atu.c
+++ b/sys/dev/usb/if_atu.c
@@ -2235,19 +2235,11 @@ atu_stop(struct ifnet *ifp, int disable)
 
 	/* Stop transfers. */
 	if (sc->atu_ep[ATU_ENDPT_RX] != NULL) {
-		err = usbd_abort_pipe(sc->atu_ep[ATU_ENDPT_RX]);
-		if (err) {
-			DPRINTF(("%s: abort rx pipe failed: %s\n",
-			    device_xname(sc->atu_dev), usbd_errstr(err)));
-		}
+		usbd_abort_pipe(sc->atu_ep[ATU_ENDPT_RX]);
 	}
 
 	if (sc->atu_ep[ATU_ENDPT_TX] != NULL) {
-		err = usbd_abort_pipe(sc->atu_ep[ATU_ENDPT_TX]);
-		if (err) {
-			DPRINTF(("%s: abort tx pipe failed: %s\n",
-			    device_xname(sc->atu_dev), usbd_errstr(err)));
-		}
+		usbd_abort_pipe(sc->atu_ep[ATU_ENDPT_TX]);
 	}
 
 	/* Free RX/TX/MGMT list resources. */
diff --git a/sys/dev/usb/ualea.c b/sys/dev/usb/ualea.c
index ede886968af4..333a9e63cab3 100644
--- a/sys/dev/usb/ualea.c
+++ b/sys/dev/usb/ualea.c
@@ -161,7 +161,7 @@ ualea_detach(device_t self, int flags)
 
 	/* Cancel pending xfer.  */
 	if (sc->sc_pipe)
-		(void)usbd_abort_pipe(sc->sc_pipe);
+		usbd_abort_pipe(sc->sc_pipe);
 	KASSERT(!sc->sc_inflight);
 
 	/* All users have drained.  Tear it all down.  */
diff --git a/sys/dev/usb/usbdi.c b/sys/dev/usb/usbdi.c
index 07406ee96fa8..0a812da1fe0a 100644
--- a/sys/dev/usb/usbdi.c
+++ b/sys/dev/usb/usbdi.c
@@ -115,7 +115,7 @@ SDT_PROBE_DEFINE2(usb, device, xfer, done,
     "usbd_status"/*status*/);
 SDT_PROBE_DEFINE1(usb, device, xfer, destroy,  "struct usbd_xfer *"/*xfer*/);
 
-Static usbd_status usbd_ar_pipe(struct usbd_pipe *);
+Static void usbd_ar_pipe(struct usbd_pipe *);
 static usbd_status usb_insert_transfer(struct usbd_xfer *);
 Static void usbd_start_next(struct usbd_pipe *);
 Static usbd_status usbd_open_pipe_ival
@@ -771,23 +771,21 @@ usbd_interface2endpoint_descriptor(struct usbd_interface *iface, uint8_t index)
 
 /* Some drivers may wish to abort requests on the default pipe, *
  * but there is no mechanism for getting a handle on it.        */
-usbd_status
+void
 usbd_abort_default_pipe(struct usbd_device *device)
 {
-	return usbd_abort_pipe(device->ud_pipe0);
+	usbd_abort_pipe(device->ud_pipe0);
 }
 
-usbd_status
+void
 usbd_abort_pipe(struct usbd_pipe *pipe)
 {
-	usbd_status err;
 
 	KASSERT(pipe != NULL);
 
 	usbd_lock_pipe(pipe);
-	err = usbd_ar_pipe(pipe);
+	usbd_ar_pipe(pipe);
 	usbd_unlock_pipe(pipe);
-	return err;
 }
 
 usbd_status
@@ -968,7 +966,7 @@ usbd_get_interface(struct usbd_interface *iface, uint8_t *aiface)
 /*** Internal routines ***/
 
 /* Dequeue all pipe operations, called with bus lock held. */
-Static usbd_status
+Static void
 usbd_ar_pipe(struct usbd_pipe *pipe)
 {
 	struct usbd_xfer *xfer;
@@ -1012,7 +1010,6 @@ usbd_ar_pipe(struct usbd_pipe *pipe)
 	}
 	pipe->up_aborting = 0;
 	SDT_PROBE1(usb, device, pipe, abort__done,  pipe);
-	return USBD_NORMAL_COMPLETION;
 }
 
 /* Called with USB lock held. */
diff --git a/sys/dev/usb/usbdi.h b/sys/dev/usb/usbdi.h
index 71d61eca5394..8052a6487923 100644
--- a/sys/dev/usb/usbdi.h
+++ b/sys/dev/usb/usbdi.h
@@ -118,8 +118,8 @@ void usbd_get_xfer_status(struct usbd_xfer *, void **,
 usb_endpoint_descriptor_t *usbd_interface2endpoint_descriptor
     (struct usbd_interface *, uint8_t);
 
-usbd_status usbd_abort_pipe(struct usbd_pipe *);
-usbd_status usbd_abort_default_pipe(struct usbd_device *);
+void usbd_abort_pipe(struct usbd_pipe *);
+void usbd_abort_default_pipe(struct usbd_device *);
 
 usbd_status usbd_clear_endpoint_stall(struct usbd_pipe *);
 void usbd_clear_endpoint_stall_async(struct usbd_pipe *);
diff --git a/sys/dev/usb/usbnet.c b/sys/dev/usb/usbnet.c
index 1f57e11ea39e..b37290dacc50 100644
--- a/sys/dev/usb/usbnet.c
+++ b/sys/dev/usb/usbnet.c
@@ -817,21 +817,16 @@ usbnet_ep_open_pipes(struct usbnet * const un)
 	return USBD_NORMAL_COMPLETION;
 }
 
-static usbd_status
+static void
 usbnet_ep_stop_pipes(struct usbnet * const un)
 {
 	struct usbnet_private * const unp = un->un_pri;
-	usbd_status err = USBD_NORMAL_COMPLETION;
 
 	for (size_t i = 0; i < __arraycount(unp->unp_ep); i++) {
 		if (unp->unp_ep[i] == NULL)
 			continue;
-		usbd_status err2 = usbd_abort_pipe(unp->unp_ep[i]);
-		if (err == USBD_NORMAL_COMPLETION && err2)
-			err = err2;
+		usbd_abort_pipe(unp->unp_ep[i]);
 	}
-
-	return err;
 }
 
 static int
@@ -1208,17 +1203,13 @@ usbnet_watchdog(struct ifnet *ifp)
 	struct usbnet * const un = ifp->if_softc;
 	struct usbnet_private * const unp = un->un_pri;
 	struct usbnet_cdata * const cd = un_cdata(un);
-	usbd_status err;
 
 	if_statinc(ifp, if_oerrors);
 	device_printf(un->un_dev, "watchdog timeout\n");
 
 	if (cd->uncd_tx_cnt > 0) {
 		DPRINTF("uncd_tx_cnt=%ju non zero, aborting pipe", 0, 0, 0, 0);
-		err = usbd_abort_pipe(unp->unp_ep[USBNET_ENDPT_TX]);
-		if (err)
-			device_printf(un->un_dev, "pipe abort failed: %s\n",
-			    usbd_errstr(err));
+		usbd_abort_pipe(unp->unp_ep[USBNET_ENDPT_TX]);
 		if (cd->uncd_tx_cnt != 0)
 			DPRINTF("uncd_tx_cnt now %ju", cd->uncd_tx_cnt, 0, 0, 0);
 	}
diff --git a/sys/dev/usb/utoppy.c b/sys/dev/usb/utoppy.c
index a40352fedf90..b412542d5c3d 100644
--- a/sys/dev/usb/utoppy.c
+++ b/sys/dev/usb/utoppy.c
@@ -1365,7 +1365,6 @@ static int
 utoppyclose(dev_t dev, int flag, int mode, struct lwp *l)
 {
 	struct utoppy_softc *sc;
-	usbd_status err;
 
 	sc = device_lookup_private(&utoppy_cd, UTOPPYUNIT(dev));
 
@@ -1384,14 +1383,12 @@ utoppyclose(dev_t dev, int flag, int mode, struct lwp *l)
 		(void) utoppy_cancel(sc);
 
 	if (sc->sc_out_pipe != NULL) {
-		if ((err = usbd_abort_pipe(sc->sc_out_pipe)) != 0)
-			printf("usbd_abort_pipe(OUT) returned %d\n", err);
+		usbd_abort_pipe(sc->sc_out_pipe);
 		sc->sc_out_pipe = NULL;
 	}
 
 	if (sc->sc_in_pipe != NULL) {
-		if ((err = usbd_abort_pipe(sc->sc_in_pipe)) != 0)
-			printf("usbd_abort_pipe(IN) returned %d\n", err);
+		usbd_abort_pipe(sc->sc_in_pipe);
 		sc->sc_in_pipe = NULL;
 	}
 

>From 1c4311434cf1caeaeae788c1b6b774404d65e67d Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sat, 8 Jan 2022 17:51:11 +0000
Subject: [PATCH 03/13] usb: usbd_close_pipe never fails.  Make it return void.

Prune dead branches as a result of this change.
---
 sys/dev/usb/if_atu.c  | 13 ++-----------
 sys/dev/usb/if_urtw.c | 13 +++----------
 sys/dev/usb/ualea.c   |  2 +-
 sys/dev/usb/usbdi.c   |  4 +---
 sys/dev/usb/usbdi.h   |  2 +-
 sys/dev/usb/usbnet.c  |  5 +----
 6 files changed, 9 insertions(+), 30 deletions(-)

diff --git a/sys/dev/usb/if_atu.c b/sys/dev/usb/if_atu.c
index 9d37935701b9..6b0d464269a5 100644
--- a/sys/dev/usb/if_atu.c
+++ b/sys/dev/usb/if_atu.c
@@ -2223,7 +2223,6 @@ atu_stop(struct ifnet *ifp, int disable)
 	struct atu_softc	*sc = ifp->if_softc;
 	struct ieee80211com	*ic = &sc->sc_ic;
 	struct atu_cdata	*cd;
-	usbd_status		err;
 	int s;
 
 	s = splnet();
@@ -2249,20 +2248,12 @@ atu_stop(struct ifnet *ifp, int disable)
 
 	/* Close pipes */
 	if (sc->atu_ep[ATU_ENDPT_RX] != NULL) {
-		err = usbd_close_pipe(sc->atu_ep[ATU_ENDPT_RX]);
-		if (err) {
-			DPRINTF(("%s: close rx pipe failed: %s\n",
-			    device_xname(sc->atu_dev), usbd_errstr(err)));
-		}
+		usbd_close_pipe(sc->atu_ep[ATU_ENDPT_RX]);
 		sc->atu_ep[ATU_ENDPT_RX] = NULL;
 	}
 
 	if (sc->atu_ep[ATU_ENDPT_TX] != NULL) {
-		err = usbd_close_pipe(sc->atu_ep[ATU_ENDPT_TX]);
-		if (err) {
-			DPRINTF(("%s: close tx pipe failed: %s\n",
-			    device_xname(sc->atu_dev), usbd_errstr(err)));
-		}
+		usbd_close_pipe(sc->atu_ep[ATU_ENDPT_TX]);
 		sc->atu_ep[ATU_ENDPT_TX] = NULL;
 	}
 
diff --git a/sys/dev/usb/if_urtw.c b/sys/dev/usb/if_urtw.c
index 162141c8072f..f08db3030887 100644
--- a/sys/dev/usb/if_urtw.c
+++ b/sys/dev/usb/if_urtw.c
@@ -829,24 +829,17 @@ urtw_close_pipes(struct urtw_softc *sc)
 	usbd_status error = 0;
 
 	if (sc->sc_rxpipe != NULL) {
-		error = usbd_close_pipe(sc->sc_rxpipe);
-		if (error != 0)
-			goto fail;
+		usbd_close_pipe(sc->sc_rxpipe);
 		sc->sc_rxpipe = NULL;
 	}
 	if (sc->sc_txpipe_low != NULL) {
-		error = usbd_close_pipe(sc->sc_txpipe_low);
-		if (error != 0)
-			goto fail;
+		usbd_close_pipe(sc->sc_txpipe_low);
 		sc->sc_txpipe_low = NULL;
 	}
 	if (sc->sc_txpipe_normal != NULL) {
-		error = usbd_close_pipe(sc->sc_txpipe_normal);
-		if (error != 0)
-			goto fail;
+		usbd_close_pipe(sc->sc_txpipe_normal);
 		sc->sc_txpipe_normal = NULL;
 	}
-fail:
 	return error;
 }
 
diff --git a/sys/dev/usb/ualea.c b/sys/dev/usb/ualea.c
index 333a9e63cab3..d38722351672 100644
--- a/sys/dev/usb/ualea.c
+++ b/sys/dev/usb/ualea.c
@@ -168,7 +168,7 @@ ualea_detach(device_t self, int flags)
 	if (sc->sc_xfer)
 		usbd_destroy_xfer(sc->sc_xfer);
 	if (sc->sc_pipe)
-		(void)usbd_close_pipe(sc->sc_pipe);
+		usbd_close_pipe(sc->sc_pipe);
 	mutex_destroy(&sc->sc_lock);
 
 	return 0;
diff --git a/sys/dev/usb/usbdi.c b/sys/dev/usb/usbdi.c
index 0a812da1fe0a..82361b04d149 100644
--- a/sys/dev/usb/usbdi.c
+++ b/sys/dev/usb/usbdi.c
@@ -323,7 +323,7 @@ usbd_open_pipe_intr(struct usbd_interface *iface, uint8_t address,
 	return err;
 }
 
-usbd_status
+void
 usbd_close_pipe(struct usbd_pipe *pipe)
 {
 	USBHIST_FUNC(); USBHIST_CALLED(usbdebug);
@@ -350,8 +350,6 @@ usbd_close_pipe(struct usbd_pipe *pipe)
 	if (pipe->up_iface)
 		usbd_iface_pipeunref(pipe->up_iface);
 	kmem_free(pipe, pipe->up_dev->ud_bus->ub_pipesize);
-
-	return USBD_NORMAL_COMPLETION;
 }
 
 usbd_status
diff --git a/sys/dev/usb/usbdi.h b/sys/dev/usb/usbdi.h
index 8052a6487923..c9c4274adb65 100644
--- a/sys/dev/usb/usbdi.h
+++ b/sys/dev/usb/usbdi.h
@@ -91,7 +91,7 @@ usbd_status usbd_open_pipe_intr(struct usbd_interface *, uint8_t, uint8_t,
     struct usbd_pipe **, void *, void *, uint32_t, usbd_callback, int);
 usbd_status usbd_open_pipe(struct usbd_interface *, uint8_t, uint8_t,
      struct usbd_pipe **);
-usbd_status usbd_close_pipe(struct usbd_pipe *);
+void usbd_close_pipe(struct usbd_pipe *);
 
 usbd_status usbd_transfer(struct usbd_xfer *);
 
diff --git a/sys/dev/usb/usbnet.c b/sys/dev/usb/usbnet.c
index b37290dacc50..50603bed02d8 100644
--- a/sys/dev/usb/usbnet.c
+++ b/sys/dev/usb/usbnet.c
@@ -779,10 +779,7 @@ usbnet_ep_close_pipes(struct usbnet * const un)
 	for (size_t i = 0; i < __arraycount(unp->unp_ep); i++) {
 		if (unp->unp_ep[i] == NULL)
 			continue;
-		usbd_status err = usbd_close_pipe(unp->unp_ep[i]);
-		if (err)
-			aprint_error_dev(un->un_dev, "close pipe %zu: %s\n", i,
-			    usbd_errstr(err));
+		usbd_close_pipe(unp->unp_ep[i]);
 		unp->unp_ep[i] = NULL;
 	}
 }

>From fd4b344f1b17f8400a5f86e438dd3b2a48d5d2c9 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sat, 8 Jan 2022 17:53:01 +0000
Subject: [PATCH 04/13] usb: usbd_free_xfer never fails.  Make it return void.

---
 sys/dev/usb/usbdi.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/sys/dev/usb/usbdi.c b/sys/dev/usb/usbdi.c
index 82361b04d149..6bdf8f2daa26 100644
--- a/sys/dev/usb/usbdi.c
+++ b/sys/dev/usb/usbdi.c
@@ -123,7 +123,7 @@ Static usbd_status usbd_open_pipe_ival
 static void *usbd_alloc_buffer(struct usbd_xfer *, uint32_t);
 static void usbd_free_buffer(struct usbd_xfer *);
 static struct usbd_xfer *usbd_alloc_xfer(struct usbd_device *, unsigned int);
-static usbd_status usbd_free_xfer(struct usbd_xfer *);
+static void usbd_free_xfer(struct usbd_xfer *);
 static void usbd_request_async_cb(struct usbd_xfer *, void *, usbd_status);
 static void usbd_xfer_timeout(void *);
 static void usbd_xfer_timeout_task(void *);
@@ -590,7 +590,7 @@ out:
 	return xfer;
 }
 
-static usbd_status
+static void
 usbd_free_xfer(struct usbd_xfer *xfer)
 {
 	USBHIST_FUNC();
@@ -610,7 +610,6 @@ usbd_free_xfer(struct usbd_xfer *xfer)
 
 	cv_destroy(&xfer->ux_cv);
 	xfer->ux_bus->ub_methods->ubm_freex(xfer->ux_bus, xfer);
-	return USBD_NORMAL_COMPLETION;
 }
 
 int

>From 7e4e60d8970049f367fcb6187cee0e9a638a56cc Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sat, 8 Jan 2022 19:35:23 +0000
Subject: [PATCH 05/13] usb: Factor usb_transfer_complete out of ubm_abortx
 method.

---
 sys/dev/usb/ehci.c           |  4 ----
 sys/dev/usb/motg.c           |  1 -
 sys/dev/usb/ohci.c           |  5 -----
 sys/dev/usb/uhci.c           |  5 -----
 sys/dev/usb/usbdi.c          | 12 ++++++++----
 sys/dev/usb/xhci.c           |  5 -----
 sys/external/bsd/dwc2/dwc2.c |  7 +------
 7 files changed, 9 insertions(+), 30 deletions(-)

diff --git a/sys/dev/usb/ehci.c b/sys/dev/usb/ehci.c
index 9a68e7ff87b6..16c061de9b46 100644
--- a/sys/dev/usb/ehci.c
+++ b/sys/dev/usb/ehci.c
@@ -3388,14 +3388,10 @@ ehci_abortx(struct usbd_xfer *xfer)
 		    BUS_DMASYNC_PREREAD);
 	}
 
-	/*
-	 * Final step: Notify completion to waiting xfers.
-	 */
 dying:
 #ifdef DIAGNOSTIC
 	exfer->ex_isdone = true;
 #endif
-	usb_transfer_complete(xfer);
 	DPRINTFN(14, "end", 0, 0, 0, 0);
 
 	KASSERT(mutex_owned(&sc->sc_lock));
diff --git a/sys/dev/usb/motg.c b/sys/dev/usb/motg.c
index 54b6afcffb93..36f5e01c4b64 100644
--- a/sys/dev/usb/motg.c
+++ b/sys/dev/usb/motg.c
@@ -2222,6 +2222,5 @@ motg_abortx(struct usbd_xfer *xfer)
 		}
 	}
 dying:
-	usb_transfer_complete(xfer);
 	KASSERT(mutex_owned(&sc->sc_lock));
 }
diff --git a/sys/dev/usb/ohci.c b/sys/dev/usb/ohci.c
index 3bbb0a01e239..4e0f827c100f 100644
--- a/sys/dev/usb/ohci.c
+++ b/sys/dev/usb/ohci.c
@@ -2409,12 +2409,7 @@ ohci_abortx(struct usbd_xfer *xfer)
 	usb_syncmem(&sed->dma, sed->offs + offsetof(ohci_ed_t, ed_flags),
 	    sizeof(sed->ed.ed_flags),
 	    BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
-
-	/*
-	 * Final step: Notify completion to waiting xfers.
-	 */
 dying:
-	usb_transfer_complete(xfer);
 	DPRINTFN(14, "end", 0, 0, 0, 0);
 
 	KASSERT(mutex_owned(&sc->sc_lock));
diff --git a/sys/dev/usb/uhci.c b/sys/dev/usb/uhci.c
index 99531c10b753..1e9718b112c0 100644
--- a/sys/dev/usb/uhci.c
+++ b/sys/dev/usb/uhci.c
@@ -2402,15 +2402,10 @@ uhci_abortx(struct usbd_xfer *xfer)
 	 */
 	/* Hardware finishes in 1ms */
 	usb_delay_ms_locked(upipe->pipe.up_dev->ud_bus, 2, &sc->sc_lock);
-
-	/*
-	 * HC Step 3: Notify completion to waiting xfers.
-	 */
 dying:
 #ifdef DIAGNOSTIC
 	ux->ux_isdone = true;
 #endif
-	usb_transfer_complete(xfer);
 	DPRINTFN(14, "end", 0, 0, 0, 0);
 
 	KASSERT(mutex_owned(&sc->sc_lock));
diff --git a/sys/dev/usb/usbdi.c b/sys/dev/usb/usbdi.c
index 6bdf8f2daa26..2c1b46406ace 100644
--- a/sys/dev/usb/usbdi.c
+++ b/sys/dev/usb/usbdi.c
@@ -1535,11 +1535,13 @@ usbd_xfer_abort(struct usbd_xfer *xfer)
 	usbd_xfer_cancel_timeout_async(xfer);
 
 	/*
-	 * We beat everyone else.  Claim the status as cancelled and do
-	 * the bus-specific dance to abort the hardware.
+	 * We beat everyone else.  Claim the status as cancelled, do
+	 * the bus-specific dance to abort the hardware, and complete
+	 * the xfer.
 	 */
 	xfer->ux_status = USBD_CANCELLED;
 	bus->ub_methods->ubm_abortx(xfer);
+	usb_transfer_complete(xfer);
 }
 
 /*
@@ -1619,11 +1621,13 @@ usbd_xfer_timeout_task(void *cookie)
 		goto out;
 
 	/*
-	 * We beat everyone else.  Claim the status as timed out and do
-	 * the bus-specific dance to abort the hardware.
+	 * We beat everyone else.  Claim the status as timed out, do
+	 * the bus-specific dance to abort the hardware, and complete
+	 * the xfer.
 	 */
 	xfer->ux_status = USBD_TIMEOUT;
 	bus->ub_methods->ubm_abortx(xfer);
+	usb_transfer_complete(xfer);
 
 out:	/* All done -- release the lock.  */
 	mutex_exit(bus->ub_lock);
diff --git a/sys/dev/usb/xhci.c b/sys/dev/usb/xhci.c
index 051f656c7dee..0e80751c1f9b 100644
--- a/sys/dev/usb/xhci.c
+++ b/sys/dev/usb/xhci.c
@@ -2201,12 +2201,7 @@ xhci_abortx(struct usbd_xfer *xfer)
 	 * HC Step 2: Remove any vestiges of the xfer from the ring.
 	 */
 	xhci_set_dequeue_locked(xfer->ux_pipe);
-
-	/*
-	 * Final Step: Notify completion to waiting xfers.
-	 */
 dying:
-	usb_transfer_complete(xfer);
 	DPRINTFN(14, "end", 0, 0, 0, 0);
 
 	KASSERT(mutex_owned(&sc->sc_lock));
diff --git a/sys/external/bsd/dwc2/dwc2.c b/sys/external/bsd/dwc2/dwc2.c
index 921f5fbed998..19ffae0e8ab4 100644
--- a/sys/external/bsd/dwc2/dwc2.c
+++ b/sys/external/bsd/dwc2/dwc2.c
@@ -515,7 +515,7 @@ dwc2_abortx(struct usbd_xfer *xfer)
 	}
 
 	/*
-	 * HC Step 1: Handle the hardware.
+	 * Handle the hardware.
 	 */
 	err = dwc2_hcd_urb_dequeue(hsotg, dxfer->urb);
 	if (err) {
@@ -524,11 +524,6 @@ dwc2_abortx(struct usbd_xfer *xfer)
 
 dying:
 	mutex_spin_exit(&hsotg->lock);
-
-	/*
-	 * Final Step: Notify completion to waiting xfers.
-	 */
-	usb_transfer_complete(xfer);
 	KASSERT(mutex_owned(&sc->sc_lock));
 }
 

>From a2a3938ffd38e9593ba7cc794908b198e7e28ab3 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sat, 22 Jan 2022 23:59:42 +0000
Subject: [PATCH 06/13] usbdi(9): Add some missing header include guards.

---
 sys/dev/usb/usb_mem.h  | 5 +++++
 sys/dev/usb/usbdivar.h | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/sys/dev/usb/usb_mem.h b/sys/dev/usb/usb_mem.h
index 5473c1b3b810..d806e58760b2 100644
--- a/sys/dev/usb/usb_mem.h
+++ b/sys/dev/usb/usb_mem.h
@@ -31,6 +31,9 @@
  * POSSIBILITY OF SUCH DAMAGE.
  */
 
+#ifndef	_DEV_USB_USB_MEM_H_
+#define	_DEV_USB_USB_MEM_H_
+
 typedef struct usb_dma_block {
 	bus_dma_tag_t tag;
 	bus_dmamap_t map;
@@ -60,3 +63,5 @@ bus_addr_t	usb_dmaaddr(usb_dma_t *, unsigned int);
 #define DMAADDR(dma, o)	usb_dmaaddr((dma), (o))
 #define KERNADDR(dma, o) \
 	((void *)((char *)(dma)->udma_block->kaddr + (dma)->udma_offs + (o)))
+
+#endif	/* _DEV_USB_USB_MEM_H_ */
diff --git a/sys/dev/usb/usbdivar.h b/sys/dev/usb/usbdivar.h
index 68acac665e9e..3e2c6ce0b59d 100644
--- a/sys/dev/usb/usbdivar.h
+++ b/sys/dev/usb/usbdivar.h
@@ -30,6 +30,9 @@
  * POSSIBILITY OF SUCH DAMAGE.
  */
 
+#ifndef	_DEV_USB_USBDIVAR_H_
+#define	_DEV_USB_USBDIVAR_H_
+
 /*
  * Discussion about locking in the USB code:
  *
@@ -395,3 +398,5 @@ usb_addr2dindex(int addr)
 
 #define usbd_lock_pipe(p)	mutex_enter((p)->up_dev->ud_bus->ub_lock)
 #define usbd_unlock_pipe(p)	mutex_exit((p)->up_dev->ud_bus->ub_lock)
+
+#endif	/* _DEV_USB_USBDIVAR_H_ */

>From a12a66b492998c7813a524199732ffd162ea7f14 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sun, 23 Jan 2022 11:04:15 +0000
Subject: [PATCH 07/13] xhci(4): Add missing includes to xhcivar.h.

---
 sys/dev/usb/xhcivar.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/sys/dev/usb/xhcivar.h b/sys/dev/usb/xhcivar.h
index 9893037be2e1..b253da58557f 100644
--- a/sys/dev/usb/xhcivar.h
+++ b/sys/dev/usb/xhcivar.h
@@ -29,8 +29,17 @@
 #ifndef _DEV_USB_XHCIVAR_H_
 #define _DEV_USB_XHCIVAR_H_
 
+#include <sys/types.h>
+
+#include <sys/condvar.h>
+#include <sys/device.h>
+#include <sys/mutex.h>
+#include <sys/pmf.h>
 #include <sys/pool.h>
 
+#include <dev/usb/usbdi.h>
+#include <dev/usb/usbdivar.h>
+
 #define XHCI_MAX_DCI	31
 
 struct xhci_soft_trb {

>From 9eb34e165356aa28dcf18f4098adeaf9274cebef Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sun, 23 Jan 2022 11:05:39 +0000
Subject: [PATCH 08/13] usb(9): Add missing includes in usb_mem.h.

---
 sys/dev/usb/usb_mem.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/sys/dev/usb/usb_mem.h b/sys/dev/usb/usb_mem.h
index d806e58760b2..b93d694cfc3c 100644
--- a/sys/dev/usb/usb_mem.h
+++ b/sys/dev/usb/usb_mem.h
@@ -34,6 +34,13 @@
 #ifndef	_DEV_USB_USB_MEM_H_
 #define	_DEV_USB_USB_MEM_H_
 
+#include <sys/types.h>
+
+#include <sys/bus.h>
+#include <sys/queue.h>
+
+#include <dev/usb/usbdivar.h>
+
 typedef struct usb_dma_block {
 	bus_dma_tag_t tag;
 	bus_dmamap_t map;

>From 9df5c2ba5bd545a5e8f286ee94ba23588e45bdf6 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sun, 23 Jan 2022 12:40:22 +0000
Subject: [PATCH 09/13] xhci(4): Fix handling of endpoint reset/stop.

Use the same asynchronous task resetting a stalled/halted endpoint
and stopping a running endpoint -- either way we need to put the
endpoint back into a known state and, if there are transfers waiting
to run, start them up again.

- xhci_abortx must not drop the pipe (bus) lock -- usbdi(9) requires
  this.  So arrange to stop the endpoint and empty the queue
  asynchronously.

- If the xhci softint claims an xfer for completion with
  usbd_xfer_trycomplete, it must call usb_transfer_complete without
  ever releasing the pipe (bus) lock -- it can't claim the xfer and
  then defer the usb_transfer_complete to a task.  So arrange to
  reset the endpoint asynchronously, hold up new transfers until the
  endpoint has been reset, and then do usb_transfer_complete
  immediately.
---
 sys/dev/usb/xhci.c | 301 ++++++++++++++++++++++-----------------------
 1 file changed, 148 insertions(+), 153 deletions(-)

diff --git a/sys/dev/usb/xhci.c b/sys/dev/usb/xhci.c
index 0e80751c1f9b..ad3650760a69 100644
--- a/sys/dev/usb/xhci.c
+++ b/sys/dev/usb/xhci.c
@@ -154,15 +154,18 @@ static usbd_status xhci_new_device(device_t, struct usbd_bus *, int, int, int,
 static int xhci_roothub_ctrl(struct usbd_bus *, usb_device_request_t *,
     void *, int);
 
+static void xhci_pipe_async_task(void *);
+static void xhci_pipe_restart(struct usbd_pipe *);
+
 static usbd_status xhci_configure_endpoint(struct usbd_pipe *);
 //static usbd_status xhci_unconfigure_endpoint(struct usbd_pipe *);
-static usbd_status xhci_reset_endpoint(struct usbd_pipe *);
+static void xhci_reset_endpoint(struct usbd_pipe *);
 static usbd_status xhci_stop_endpoint_cmd(struct xhci_softc *,
     struct xhci_slot *, u_int, uint32_t);
 static usbd_status xhci_stop_endpoint(struct usbd_pipe *);
 
 static void xhci_host_dequeue(struct xhci_ring * const);
-static usbd_status xhci_set_dequeue(struct usbd_pipe *);
+static void xhci_set_dequeue(struct usbd_pipe *);
 
 static usbd_status xhci_do_command(struct xhci_softc * const,
     struct xhci_soft_trb * const, int);
@@ -1857,14 +1860,13 @@ xhci_unconfigure_endpoint(struct usbd_pipe *pipe)
 #endif
 
 /* 4.6.8, 6.4.3.7 */
-static usbd_status
-xhci_reset_endpoint_locked(struct usbd_pipe *pipe)
+static void
+xhci_reset_endpoint(struct usbd_pipe *pipe)
 {
 	struct xhci_softc * const sc = XHCI_PIPE2SC(pipe);
 	struct xhci_slot * const xs = pipe->up_dev->ud_hcpriv;
 	const u_int dci = xhci_ep_get_dci(pipe->up_endpoint->ue_edesc);
 	struct xhci_soft_trb trb;
-	usbd_status err;
 
 	XHCIHIST_FUNC();
 	XHCIHIST_CALLARGS("slot %ju dci %ju", xs->xs_idx, dci, 0, 0);
@@ -1877,21 +1879,10 @@ xhci_reset_endpoint_locked(struct usbd_pipe *pipe)
 	    XHCI_TRB_3_EP_SET(dci) |
 	    XHCI_TRB_3_TYPE_SET(XHCI_TRB_TYPE_RESET_EP);
 
-	err = xhci_do_command_locked(sc, &trb, USBD_DEFAULT_TIMEOUT);
-
-	return err;
-}
-
-static usbd_status
-xhci_reset_endpoint(struct usbd_pipe *pipe)
-{
-	struct xhci_softc * const sc = XHCI_PIPE2SC(pipe);
-
-	mutex_enter(&sc->sc_lock);
-	usbd_status ret = xhci_reset_endpoint_locked(pipe);
-	mutex_exit(&sc->sc_lock);
-
-	return ret;
+	if (xhci_do_command_locked(sc, &trb, USBD_DEFAULT_TIMEOUT)) {
+		device_printf(sc->sc_dev, "%s: endpoint 0x%x: timed out\n",
+		    __func__, pipe->up_endpoint->ue_edesc->bEndpointAddress);
+	}
 }
 
 /*
@@ -1946,15 +1937,14 @@ xhci_stop_endpoint(struct usbd_pipe *pipe)
  * EPSTATE of endpoint must be ERROR or STOPPED, otherwise CONTEXT_STATE
  * error will be generated.
  */
-static usbd_status
-xhci_set_dequeue_locked(struct usbd_pipe *pipe)
+static void
+xhci_set_dequeue(struct usbd_pipe *pipe)
 {
 	struct xhci_softc * const sc = XHCI_PIPE2SC(pipe);
 	struct xhci_slot * const xs = pipe->up_dev->ud_hcpriv;
 	const u_int dci = xhci_ep_get_dci(pipe->up_endpoint->ue_edesc);
 	struct xhci_ring * const xr = xs->xs_xr[dci];
 	struct xhci_soft_trb trb;
-	usbd_status err;
 
 	XHCIHIST_FUNC();
 	XHCIHIST_CALLARGS("slot %ju dci %ju", xs->xs_idx, dci, 0, 0);
@@ -1971,21 +1961,10 @@ xhci_set_dequeue_locked(struct usbd_pipe *pipe)
 	    XHCI_TRB_3_EP_SET(dci) |
 	    XHCI_TRB_3_TYPE_SET(XHCI_TRB_TYPE_SET_TR_DEQUEUE);
 
-	err = xhci_do_command_locked(sc, &trb, USBD_DEFAULT_TIMEOUT);
-
-	return err;
-}
-
-static usbd_status
-xhci_set_dequeue(struct usbd_pipe *pipe)
-{
-	struct xhci_softc * const sc = XHCI_PIPE2SC(pipe);
-
-	mutex_enter(&sc->sc_lock);
-	usbd_status ret = xhci_set_dequeue_locked(pipe);
-	mutex_exit(&sc->sc_lock);
-
-	return ret;
+	if (xhci_do_command_locked(sc, &trb, USBD_DEFAULT_TIMEOUT)) {
+		device_printf(sc->sc_dev, "%s: endpoint 0x%x: timed out\n",
+		    __func__, pipe->up_endpoint->ue_edesc->bEndpointAddress);
+	}
 }
 
 /*
@@ -2035,6 +2014,9 @@ xhci_open(struct usbd_pipe *pipe)
 		return USBD_NORMAL_COMPLETION;
 	}
 
+	usb_init_task(&xpipe->xp_async_task, xhci_pipe_async_task, xpipe,
+	    USB_TASKQ_MPSAFE);
+
 	switch (xfertype) {
 	case UE_CONTROL:
 		pipe->up_methods = &xhci_device_ctrl_methods;
@@ -2080,6 +2062,8 @@ xhci_open(struct usbd_pipe *pipe)
 static void
 xhci_close_pipe(struct usbd_pipe *pipe)
 {
+	struct xhci_pipe * const xp =
+	    container_of(pipe, struct xhci_pipe, xp_pipe);
 	struct xhci_softc * const sc = XHCI_PIPE2SC(pipe);
 	struct xhci_slot * const xs = pipe->up_dev->ud_hcpriv;
 	usb_endpoint_descriptor_t * const ed = pipe->up_endpoint->ue_edesc;
@@ -2089,6 +2073,9 @@ xhci_close_pipe(struct usbd_pipe *pipe)
 
 	XHCIHIST_FUNC();
 
+	usb_rem_task_wait(pipe->up_dev, &xp->xp_async_task, USB_TASKQ_HC,
+	    &sc->sc_lock);
+
 	if (sc->sc_dying)
 		return;
 
@@ -2148,63 +2135,25 @@ xhci_close_pipe(struct usbd_pipe *pipe)
 
 /*
  * Abort transfer.
- * Should be called with sc_lock held.
+ * Should be called with sc_lock held.  Must not drop sc_lock.
  */
 static void
 xhci_abortx(struct usbd_xfer *xfer)
 {
 	XHCIHIST_FUNC();
 	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_CALLARGS("xfer %#jx pipe %#jx",
 	    (uintptr_t)xfer, (uintptr_t)xfer->ux_pipe, 0, 0);
 
 	KASSERT(mutex_owned(&sc->sc_lock));
-	ASSERT_SLEEPABLE();
-
 	KASSERTMSG((xfer->ux_status == USBD_CANCELLED ||
 		xfer->ux_status == USBD_TIMEOUT),
 	    "bad abort status: %d", xfer->ux_status);
 
-	/*
-	 * If we're dying, skip the hardware action and just notify the
-	 * 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;
-	}
-
-	/*
-	 * HC Step 1: Stop execution of TD on the ring.
-	 */
-	switch (xhci_get_epstate(sc, xs, dci)) {
-	case XHCI_EPSTATE_HALTED:
-		(void)xhci_reset_endpoint_locked(xfer->ux_pipe);
-		break;
-	case XHCI_EPSTATE_STOPPED:
-		break;
-	default:
-		(void)xhci_stop_endpoint(xfer->ux_pipe);
-		break;
-	}
-#ifdef DIAGNOSTIC
-	uint32_t epst = xhci_get_epstate(sc, xs, dci);
-	if (epst != XHCI_EPSTATE_STOPPED)
-		DPRINTFN(4, "dci %ju not stopped %ju", dci, epst, 0, 0);
-#endif
+	xhci_pipe_restart(xfer->ux_pipe);
 
-	/*
-	 * HC Step 2: Remove any vestiges of the xfer from the ring.
-	 */
-	xhci_set_dequeue_locked(xfer->ux_pipe);
-dying:
 	DPRINTFN(14, "end", 0, 0, 0, 0);
-
-	KASSERT(mutex_owned(&sc->sc_lock));
 }
 
 static void
@@ -2221,69 +2170,102 @@ xhci_host_dequeue(struct xhci_ring * const xr)
 }
 
 /*
- * Recover STALLed endpoint.
+ * Recover STALLed endpoint, or stop endpoint to abort a pipe.
  * xHCI 1.1 sect 4.10.2.1
  * Issue RESET_EP to recover halt condition and SET_TR_DEQUEUE to remove
  * all transfers on transfer ring.
  * These are done in thread context asynchronously.
  */
 static void
-xhci_clear_endpoint_stall_async_task(void *cookie)
+xhci_pipe_async_task(void *cookie)
 {
-	struct usbd_xfer * const xfer = cookie;
-	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);
+	struct xhci_pipe * const xp = cookie;
+	struct usbd_pipe * const pipe = &xp->xp_pipe;
+	struct xhci_softc * const sc = XHCI_PIPE2SC(pipe);
+	struct xhci_slot * const xs = pipe->up_dev->ud_hcpriv;
+	const u_int dci = xhci_ep_get_dci(pipe->up_endpoint->ue_edesc);
 	struct xhci_ring * const tr = xs->xs_xr[dci];
+	bool restart = false;
 
 	XHCIHIST_FUNC();
-	XHCIHIST_CALLARGS("xfer %#jx slot %ju dci %ju", (uintptr_t)xfer, xs->xs_idx,
-	    dci, 0);
+	XHCIHIST_CALLARGS("pipe %#jx slot %ju dci %ju",
+	    (uintptr_t)pipe, xs->xs_idx, dci, 0);
+
+	mutex_enter(&sc->sc_lock);
 
 	/*
-	 * XXXMRG: Stall task can run after slot is disabled when yanked.
-	 * This hack notices that the xs has been memset() in
-	 * xhci_disable_slot() and returns.  Both xhci_reset_endpoint()
-	 * and xhci_set_dequeue() rely upon a valid ring setup for correct
-	 * operation, and the latter will fault, as would
-	 * usb_transfer_complete() if it got that far.
+	 * - If the endpoint is halted, indicating a stall, reset it.
+	 * - If the endpoint is stopped, we're already good.
+	 * - Otherwise, someone wanted to abort the pipe, so stop the
+	 *   endpoint.
+	 *
+	 * In any case, clear the ring.
 	 */
-	if (xs->xs_idx == 0) {
-		DPRINTFN(4, "ends xs_idx is 0", 0, 0, 0, 0);
-		return;
+	switch (xhci_get_epstate(sc, xs, dci)) {
+	case XHCI_EPSTATE_HALTED:
+		xhci_reset_endpoint(pipe);
+		break;
+	case XHCI_EPSTATE_STOPPED:
+		break;
+	default:
+		xhci_stop_endpoint(pipe);
+		break;
 	}
+	switch (xhci_get_epstate(sc, xs, dci)) {
+	case XHCI_EPSTATE_STOPPED:
+		break;
+	case XHCI_EPSTATE_ERROR:
+		device_printf(sc->sc_dev, "endpoint 0x%x error\n",
+		    pipe->up_endpoint->ue_edesc->bEndpointAddress);
+		break;
+	default:
+		device_printf(sc->sc_dev, "endpoint 0x%x failed to stop\n",
+		    pipe->up_endpoint->ue_edesc->bEndpointAddress);
+	}
+	xhci_set_dequeue(pipe);
 
-	KASSERT(tr != NULL);
-
-	xhci_reset_endpoint(xfer->ux_pipe);
-	xhci_set_dequeue(xfer->ux_pipe);
+	/*
+	 * If we halted our own queue because it stalled, mark it no
+	 * longer halted and arrange to start it up again.
+	 */
+	if (tr->is_halted) {
+		tr->is_halted = false;
+		if (!SIMPLEQ_EMPTY(&pipe->up_queue))
+			restart = true;
+	}
 
-	mutex_enter(&sc->sc_lock);
-	tr->is_halted = false;
-	usb_transfer_complete(xfer);
 	mutex_exit(&sc->sc_lock);
+
+	/*
+	 * If the endpoint was stalled, start issuing queued transfers
+	 * again.
+	 */
+	if (restart) {
+		/*
+		 * XXX Shouldn't touch the queue unlocked -- upm_start
+		 * should be called with the lock held instead.  The
+		 * pipe could be aborted at this point, and the xfer
+		 * freed.
+		 */
+		struct usbd_xfer *xfer = SIMPLEQ_FIRST(&pipe->up_queue);
+		(*pipe->up_methods->upm_start)(xfer);
+	}
+
 	DPRINTFN(4, "ends", 0, 0, 0, 0);
 }
 
-static usbd_status
-xhci_clear_endpoint_stall_async(struct usbd_xfer *xfer)
+static void
+xhci_pipe_restart(struct usbd_pipe *pipe)
 {
-	struct xhci_softc * const sc = XHCI_XFER2SC(xfer);
-	struct xhci_pipe * const xp = (struct xhci_pipe *)xfer->ux_pipe;
+	struct xhci_pipe * const xp =
+	    container_of(pipe, struct xhci_pipe, xp_pipe);
 
 	XHCIHIST_FUNC();
-	XHCIHIST_CALLARGS("xfer %#jx", (uintptr_t)xfer, 0, 0, 0);
+	XHCIHIST_CALLARGS("pipe %#jx", (uintptr_t)pipe, 0, 0, 0);
 
-	if (sc->sc_dying) {
-		return USBD_IOERROR;
-	}
+	usb_add_task(pipe->up_dev, &xp->xp_async_task, USB_TASKQ_HC);
 
-	usb_init_task(&xp->xp_async_task,
-	    xhci_clear_endpoint_stall_async_task, xfer, USB_TASKQ_MPSAFE);
-	usb_add_task(xfer->ux_pipe->up_dev, &xp->xp_async_task, USB_TASKQ_HC);
 	DPRINTFN(4, "ends", 0, 0, 0, 0);
-
-	return USBD_NORMAL_COMPLETION;
 }
 
 /* Process roothub port status/change events and notify to uhub_intr. */
@@ -2461,32 +2443,9 @@ xhci_event_transfer(struct xhci_softc * const sc,
 	case XHCI_TRB_ERROR_BABBLE:
 		DPRINTFN(1, "ERR %ju slot %ju dci %ju", trbcode, slot, dci, 0);
 		xr->is_halted = true;
-		/*
-		 * Try to claim this xfer for completion.  If it has already
-		 * completed or aborted, drop it on the floor.
-		 */
-		if (!usbd_xfer_trycomplete(xfer))
-			return;
-
-		/*
-		 * Stalled endpoints can be recoverd by issuing
-		 * command TRB TYPE_RESET_EP on xHCI instead of
-		 * issuing request CLEAR_FEATURE UF_ENDPOINT_HALT
-		 * on the endpoint. However, this function may be
-		 * called from softint context (e.g. from umass),
-		 * in that case driver gets KASSERT in cv_timedwait
-		 * in xhci_do_command.
-		 * To avoid this, this runs reset_endpoint and
-		 * usb_transfer_complete in usb task thread
-		 * asynchronously (and then umass issues clear
-		 * UF_ENDPOINT_HALT).
-		 */
-
-		/* Override the status.  */
-		xfer->ux_status = USBD_STALLED;
-
-		xhci_clear_endpoint_stall_async(xfer);
-		return;
+		xhci_pipe_restart(xfer->ux_pipe);
+		err = USBD_STALLED;
+		break;
 	default:
 		DPRINTFN(1, "ERR %ju slot %ju dci %ju", trbcode, slot, dci, 0);
 		err = USBD_IOERROR;
@@ -4296,6 +4255,11 @@ xhci_device_ctrl_start(struct usbd_xfer *xfer)
 
 	KASSERT((xfer->ux_rqflags & URQ_REQUEST) != 0);
 
+	if (!polling)
+		mutex_enter(&sc->sc_lock);
+	if (tr->is_halted)
+		goto out;
+
 	i = 0;
 
 	/* setup phase */
@@ -4338,11 +4302,18 @@ xhci_device_ctrl_start(struct usbd_xfer *xfer)
 	if (!polling)
 		mutex_exit(&tr->xr_lock);
 
-	if (!polling)
-		mutex_enter(&sc->sc_lock);
-	xfer->ux_status = USBD_IN_PROGRESS;
 	xhci_db_write_4(sc, XHCI_DOORBELL(xs->xs_idx), dci);
-	usbd_xfer_schedule_timeout(xfer);
+
+out:	if (xfer->ux_status == USBD_NOT_STARTED) {
+		usbd_xfer_schedule_timeout(xfer);
+		xfer->ux_status = USBD_IN_PROGRESS;
+	} else {
+		/*
+		 * We must be coming from xhci_pipe_restart -- timeout
+		 * already set up, nothing to do.
+		 */
+	}
+	KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
 	if (!polling)
 		mutex_exit(&sc->sc_lock);
 
@@ -4569,6 +4540,11 @@ xhci_device_bulk_start(struct usbd_xfer *xfer)
 
 	KASSERT((xfer->ux_rqflags & URQ_REQUEST) == 0);
 
+	if (!polling)
+		mutex_enter(&sc->sc_lock);
+	if (tr->is_halted)
+		goto out;
+
 	parameter = DMAADDR(dma, 0);
 	const bool isread = usbd_xfer_isread(xfer);
 	if (len)
@@ -4601,11 +4577,18 @@ xhci_device_bulk_start(struct usbd_xfer *xfer)
 	if (!polling)
 		mutex_exit(&tr->xr_lock);
 
-	if (!polling)
-		mutex_enter(&sc->sc_lock);
-	xfer->ux_status = USBD_IN_PROGRESS;
 	xhci_db_write_4(sc, XHCI_DOORBELL(xs->xs_idx), dci);
-	usbd_xfer_schedule_timeout(xfer);
+
+out:	if (xfer->ux_status == USBD_NOT_STARTED) {
+		xfer->ux_status = USBD_IN_PROGRESS;
+		usbd_xfer_schedule_timeout(xfer);
+	} else {
+		/*
+		 * We must be coming from xhci_pipe_restart -- timeout
+		 * already set up, nothing to do.
+		 */
+	}
+	KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
 	if (!polling)
 		mutex_exit(&sc->sc_lock);
 
@@ -4680,6 +4663,11 @@ xhci_device_intr_start(struct usbd_xfer *xfer)
 	if (sc->sc_dying)
 		return USBD_IOERROR;
 
+	if (!polling)
+		mutex_enter(&sc->sc_lock);
+	if (tr->is_halted)
+		goto out;
+
 	KASSERT((xfer->ux_rqflags & URQ_REQUEST) == 0);
 
 	const bool isread = usbd_xfer_isread(xfer);
@@ -4702,11 +4690,18 @@ xhci_device_intr_start(struct usbd_xfer *xfer)
 	if (!polling)
 		mutex_exit(&tr->xr_lock);
 
-	if (!polling)
-		mutex_enter(&sc->sc_lock);
-	xfer->ux_status = USBD_IN_PROGRESS;
 	xhci_db_write_4(sc, XHCI_DOORBELL(xs->xs_idx), dci);
-	usbd_xfer_schedule_timeout(xfer);
+
+out:	if (xfer->ux_status == USBD_NOT_STARTED) {
+		xfer->ux_status = USBD_IN_PROGRESS;
+		usbd_xfer_schedule_timeout(xfer);
+	} else {
+		/*
+		 * We must be coming from xhci_pipe_restart -- timeout
+		 * already set up, nothing to do.
+		 */
+	}
+	KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
 	if (!polling)
 		mutex_exit(&sc->sc_lock);
 

>From 59ae5426c1999540d4f5f376a63ea69372e01dd5 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sun, 23 Jan 2022 15:05:41 +0000
Subject: [PATCH 10/13] usbdi(9): Update tables of bus/pipe method locking
 rules.

No functional change.
---
 sys/dev/usb/usbdivar.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/sys/dev/usb/usbdivar.h b/sys/dev/usb/usbdivar.h
index 3e2c6ce0b59d..3f92e323afb2 100644
--- a/sys/dev/usb/usbdivar.h
+++ b/sys/dev/usb/usbdivar.h
@@ -44,16 +44,19 @@
  *	BUS METHOD		LOCK	NOTES
  *	----------------------- -------	-------------------------
  *	ubm_open		-	might want to take lock?
- *	ubm_softint		x
+ *	ubm_softint		x	may release/reacquire lock
  *	ubm_dopoll		-	might want to take lock?
  *	ubm_allocx		-
  *	ubm_freex		-
+ *	ubm_abortx		x	must not release/reacquire lock
  *	ubm_getlock 		-	Called at attach time
  *	ubm_newdev		-	Will take lock
-	ubm_rhctrl
+ *	ubm_rhctrl
  *
  *	PIPE METHOD		LOCK	NOTES
  *	----------------------- -------	-------------------------
+ *	upm_init		-
+ *	upm_fini		-
  *	upm_transfer		-
  *	upm_start		-	might want to take lock?
  *	upm_abort		x

>From 8cacbe20b93d0e0cab72ca6a04d0a3f034c02b6b Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sun, 23 Jan 2022 16:30:13 +0000
Subject: [PATCH 11/13] usbdi(9): Make usbd_abort_pipe persistent.

Also make it mandatory before usbd_close_pipe.

This avoids races such as

	/* read */
	if (sc->sc_dying)
		return ENXIO;
	/* (*) */
	usbd_bulk_transfer(...);

	/* detach or or close */
	sc->sc_dying = true;
	usbd_abort_pipe(...);

The detach or close logic might happen at the same time as (*), with
no way to stop the bulk transfer before it starts.
---
 sys/dev/usb/usbdi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sys/dev/usb/usbdi.c b/sys/dev/usb/usbdi.c
index 2c1b46406ace..ef8785e4cca6 100644
--- a/sys/dev/usb/usbdi.c
+++ b/sys/dev/usb/usbdi.c
@@ -332,6 +332,7 @@ usbd_close_pipe(struct usbd_pipe *pipe)
 
 	usbd_lock_pipe(pipe);
 	SDT_PROBE1(usb, device, pipe, close,  pipe);
+	KASSERTMSG(pipe->up_aborting, "pipe %p not aborted first", pipe);
 	if (!SIMPLEQ_EMPTY(&pipe->up_queue)) {
 		printf("WARNING: pipe closed with active xfers on addr %d\n",
 		    pipe->up_dev->ud_addr);
@@ -1005,7 +1006,6 @@ usbd_ar_pipe(struct usbd_pipe *pipe)
 			/* XXX only for non-0 usbd_clear_endpoint_stall(pipe); */
 		}
 	}
-	pipe->up_aborting = 0;
 	SDT_PROBE1(usb, device, pipe, abort__done,  pipe);
 }
 

>From 3478e430fe77405a586d72a8fb3b34c7e0ebcf25 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sun, 23 Jan 2022 16:40:16 +0000
Subject: [PATCH 12/13] usbdi(9): Inline usb_insert_transfer.

This makes it clearer which part happens irrespective of error
(putting it on the queue -- unconditional, not rolled back by
usb_insert_transfer) and what the possible `errors' mean (neither of
which is an error, per se).
---
 sys/dev/usb/usbdi.c | 43 ++++++++++---------------------------------
 1 file changed, 10 insertions(+), 33 deletions(-)

diff --git a/sys/dev/usb/usbdi.c b/sys/dev/usb/usbdi.c
index ef8785e4cca6..5f9ea204ac0e 100644
--- a/sys/dev/usb/usbdi.c
+++ b/sys/dev/usb/usbdi.c
@@ -116,7 +116,6 @@ SDT_PROBE_DEFINE2(usb, device, xfer, done,
 SDT_PROBE_DEFINE1(usb, device, xfer, destroy,  "struct usbd_xfer *"/*xfer*/);
 
 Static void usbd_ar_pipe(struct usbd_pipe *);
-static usbd_status usb_insert_transfer(struct usbd_xfer *);
 Static void usbd_start_next(struct usbd_pipe *);
 Static usbd_status usbd_open_pipe_ival
 	(struct usbd_interface *, uint8_t, uint8_t, struct usbd_pipe **, int);
@@ -410,7 +409,16 @@ usbd_transfer(struct usbd_xfer *xfer)
 	SDT_PROBE2(usb, device, pipe, transfer__start,  pipe, xfer);
 	do {
 		usbd_lock_pipe(pipe);
-		err = usb_insert_transfer(xfer);
+#ifdef DIAGNOSTIC
+		xfer->ux_state = XFER_ONQU;
+#endif
+		SIMPLEQ_INSERT_TAIL(&pipe->up_queue, xfer, ux_next);
+		if (pipe->up_running && pipe->up_serialise) {
+			err = USBD_IN_PROGRESS;
+		} else {
+			pipe->up_running = 1;
+			err = USBD_NORMAL_COMPLETION;
+		}
 		usbd_unlock_pipe(pipe);
 		if (err)
 			break;
@@ -1139,37 +1147,6 @@ usb_transfer_complete(struct usbd_xfer *xfer)
 		usbd_start_next(pipe);
 }
 
-/* Called with USB lock held. */
-static usbd_status
-usb_insert_transfer(struct usbd_xfer *xfer)
-{
-	struct usbd_pipe *pipe = xfer->ux_pipe;
-	usbd_status err;
-
-	USBHIST_FUNC(); USBHIST_CALLARGS(usbdebug,
-	    "xfer = %#jx pipe = %#jx running = %jd timeout = %jd",
-	    (uintptr_t)xfer, (uintptr_t)pipe,
-	    pipe->up_running, xfer->ux_timeout);
-
-	KASSERT(mutex_owned(pipe->up_dev->ud_bus->ub_lock));
-	KASSERTMSG(xfer->ux_state == XFER_BUSY, "xfer %p state is %x", xfer,
-	    xfer->ux_state);
-
-#ifdef DIAGNOSTIC
-	xfer->ux_state = XFER_ONQU;
-#endif
-	SIMPLEQ_INSERT_TAIL(&pipe->up_queue, xfer, ux_next);
-	if (pipe->up_running && pipe->up_serialise)
-		err = USBD_IN_PROGRESS;
-	else {
-		pipe->up_running = 1;
-		err = USBD_NORMAL_COMPLETION;
-	}
-	USBHIST_LOG(usbdebug, "<- done xfer %#jx, err %jd", (uintptr_t)xfer,
-	    err, 0, 0);
-	return err;
-}
-
 /* Called with USB lock held. */
 void
 usbd_start_next(struct usbd_pipe *pipe)

>From 4695bb74527149254728ffa4eaec2d8a87a18531 Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
Date: Sun, 23 Jan 2022 18:50:12 +0000
Subject: [PATCH 13/13] usbdi(9): In usbd_transfer, test whether aborting under
 the lock.

Otherwise this test is racy and can cause the bad state of a pipe
with a transfer that will never be completed in a pipe that's about
to close under the expectation that the pipe is empty.
---
 sys/dev/usb/usbdi.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/sys/dev/usb/usbdi.c b/sys/dev/usb/usbdi.c
index 5f9ea204ac0e..e70bc288591d 100644
--- a/sys/dev/usb/usbdi.c
+++ b/sys/dev/usb/usbdi.c
@@ -371,13 +371,6 @@ usbd_transfer(struct usbd_xfer *xfer)
 #endif
 	xfer->ux_done = 0;
 
-	if (pipe->up_aborting) {
-		USBHIST_LOG(usbdebug, "<- done xfer %#jx, aborting",
-		    (uintptr_t)xfer, 0, 0, 0);
-		SDT_PROBE2(usb, device, xfer, done,  xfer, USBD_CANCELLED);
-		return USBD_CANCELLED;
-	}
-
 	KASSERT(xfer->ux_length == 0 || xfer->ux_buf != NULL);
 
 	size = xfer->ux_length;
@@ -405,10 +398,23 @@ usbd_transfer(struct usbd_xfer *xfer)
 		}
 	}
 
+	usbd_lock_pipe(pipe);
+	if (pipe->up_aborting) {
+		/*
+		 * XXX For synchronous transfers this is fine.  What to
+		 * do for asynchronous transfers?  The callback is
+		 * never run, not even with status USBD_CANCELLED.
+		 */
+		usbd_unlock_pipe(pipe);
+		USBHIST_LOG(usbdebug, "<- done xfer %#jx, aborting",
+		    (uintptr_t)xfer, 0, 0, 0);
+		SDT_PROBE2(usb, device, xfer, done,  xfer, USBD_CANCELLED);
+		return USBD_CANCELLED;
+	}
+
 	/* xfer is not valid after the transfer method unless synchronous */
 	SDT_PROBE2(usb, device, pipe, transfer__start,  pipe, xfer);
 	do {
-		usbd_lock_pipe(pipe);
 #ifdef DIAGNOSTIC
 		xfer->ux_state = XFER_ONQU;
 #endif
diff --git a/sys/arch/mips/adm5120/dev/ahci.c b/sys/arch/mips/adm5120/dev/ahci.c
index 2b98e4ae221e..340650e2a13a 100644
--- a/sys/arch/mips/adm5120/dev/ahci.c
+++ b/sys/arch/mips/adm5120/dev/ahci.c
@@ -729,22 +729,10 @@ ahci_roothub_ctrl(struct usbd_bus *bus, usb_device_request_t *req,
 static usbd_status
 ahci_root_intr_transfer(struct usbd_xfer *xfer)
 {
-	struct ahci_softc *sc = AHCI_XFER2SC(xfer);
-	usbd_status error;
 
 	DPRINTF(D_TRACE, ("SLRItransfer "));
 
-	/* Insert last in queue */
-	mutex_enter(&sc->sc_lock);
-	error = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (error)
-		return error;
-
-	/*
-	 * Pipe isn't running (otherwise error would be USBD_INPROG),
-	 * start first.
-	 */
+	/* Pipe isn't running, start first.  */
 	return ahci_root_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -827,17 +815,9 @@ ahci_root_intr_done(struct usbd_xfer *xfer)
 static usbd_status
 ahci_device_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	struct ahci_softc *sc = AHCI_XFER2SC(xfer);
-	usbd_status error;
 
 	DPRINTF(D_TRACE, ("C"));
 
-	mutex_enter(&sc->sc_lock);
-	error = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (error)
-		return error;
-
 	return ahci_device_ctrl_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -1017,17 +997,9 @@ ahci_device_ctrl_done(struct usbd_xfer *xfer)
 static usbd_status
 ahci_device_intr_transfer(struct usbd_xfer *xfer)
 {
-	struct ahci_softc *sc = AHCI_XFER2SC(xfer);
-	usbd_status error;
 
 	DPRINTF(D_TRACE, ("INTRtrans "));
 
-	mutex_enter(&sc->sc_lock);
-	error = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (error)
-		return error;
-
 	return ahci_device_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -1161,17 +1133,9 @@ ahci_device_isoc_done(struct usbd_xfer *xfer)
 static usbd_status
 ahci_device_bulk_transfer(struct usbd_xfer *xfer)
 {
-	struct ahci_softc *sc = AHCI_XFER2SC(xfer);
-	usbd_status error;
 
 	DPRINTF(D_TRACE, ("B"));
 
-	mutex_enter(&sc->sc_lock);
-	error = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (error)
-		return error;
-
 	return ahci_device_bulk_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
diff --git a/sys/dev/ic/sl811hs.c b/sys/dev/ic/sl811hs.c
index e5773430ccaf..35fea39878d1 100644
--- a/sys/dev/ic/sl811hs.c
+++ b/sys/dev/ic/sl811hs.c
@@ -839,28 +839,13 @@ usbd_status
 slhci_transfer(struct usbd_xfer *xfer)
 {
 	SLHCIHIST_FUNC(); SLHCIHIST_CALLED();
-	struct slhci_softc *sc = SLHCI_XFER2SC(xfer);
 	usbd_status error;
 
 	DLOG(D_TRACE, "transfer type %jd xfer %#jx spipe %#jx ",
 	    SLHCI_XFER_TYPE(xfer), (uintptr_t)xfer, (uintptr_t)xfer->ux_pipe,
 	    0);
 
-	/* Insert last in queue */
-	mutex_enter(&sc->sc_lock);
-	error = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (error) {
-		if (error != USBD_IN_PROGRESS)
-			DLOG(D_ERR, "usb_insert_transfer returns %jd!", error,
-			    0,0,0);
-		return error;
-	}
-
-	/*
-	 * Pipe isn't running (otherwise error would be USBD_INPROG),
-	 * so start it first.
-	 */
+	/* Pipe isn't running, so start it first.  */
 
 	/*
 	 * Start will take the lock.
diff --git a/sys/dev/usb/ehci.c b/sys/dev/usb/ehci.c
index f275b15ad1a8..16c061de9b46 100644
--- a/sys/dev/usb/ehci.c
+++ b/sys/dev/usb/ehci.c
@@ -2748,15 +2748,6 @@ ehci_disown(ehci_softc_t *sc, int index, int lowspeed)
 Static usbd_status
 ehci_root_intr_transfer(struct usbd_xfer *xfer)
 {
-	ehci_softc_t *sc = EHCI_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
 	/* Pipe isn't running, start first */
 	return ehci_root_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
@@ -3397,14 +3388,10 @@ ehci_abortx(struct usbd_xfer *xfer)
 		    BUS_DMASYNC_PREREAD);
 	}
 
-	/*
-	 * Final step: Notify completion to waiting xfers.
-	 */
 dying:
 #ifdef DIAGNOSTIC
 	exfer->ex_isdone = true;
 #endif
-	usb_transfer_complete(xfer);
 	DPRINTFN(14, "end", 0, 0, 0, 0);
 
 	KASSERT(mutex_owned(&sc->sc_lock));
@@ -3606,15 +3593,6 @@ ehci_device_ctrl_fini(struct usbd_xfer *xfer)
 Static usbd_status
 ehci_device_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	ehci_softc_t *sc = EHCI_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
 	/* Pipe isn't running, start first */
 	return ehci_device_ctrl_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
@@ -3886,15 +3864,6 @@ ehci_device_bulk_fini(struct usbd_xfer *xfer)
 Static usbd_status
 ehci_device_bulk_transfer(struct usbd_xfer *xfer)
 {
-	ehci_softc_t *sc = EHCI_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
 	/* Pipe isn't running, start first */
 	return ehci_device_bulk_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
@@ -4099,20 +4068,8 @@ ehci_device_intr_fini(struct usbd_xfer *xfer)
 Static usbd_status
 ehci_device_intr_transfer(struct usbd_xfer *xfer)
 {
-	ehci_softc_t *sc = EHCI_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
-	/*
-	 * Pipe isn't running (otherwise err would be USBD_INPROG),
-	 * so start it first.
-	 */
+	/* Pipe isn't running, so start it first.  */
 	return ehci_device_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -4351,14 +4308,6 @@ Static usbd_status
 ehci_device_fs_isoc_transfer(struct usbd_xfer *xfer)
 {
 	ehci_softc_t *sc = EHCI_XFER2SC(xfer);
-	usbd_status __diagused err;
-
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-
-	KASSERT(err == USBD_NORMAL_COMPLETION);
-
 	struct ehci_pipe *epipe = EHCI_XFER2EPIPE(xfer);
 	struct usbd_device *dev = xfer->ux_pipe->up_dev;
 	struct ehci_xfer *exfer = EHCI_XFER2EXFER(xfer);
@@ -4724,14 +4673,6 @@ Static usbd_status
 ehci_device_isoc_transfer(struct usbd_xfer *xfer)
 {
 	ehci_softc_t *sc = EHCI_XFER2SC(xfer);
-	usbd_status __diagused err;
-
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-
-	KASSERT(err == USBD_NORMAL_COMPLETION);
-
 	struct ehci_pipe *epipe = EHCI_XFER2EPIPE(xfer);
 	struct ehci_xfer *exfer = EHCI_XFER2EXFER(xfer);
 	ehci_soft_itd_t *itd, *prev;
diff --git a/sys/dev/usb/if_atu.c b/sys/dev/usb/if_atu.c
index 85222d1dce28..6b0d464269a5 100644
--- a/sys/dev/usb/if_atu.c
+++ b/sys/dev/usb/if_atu.c
@@ -2223,7 +2223,6 @@ atu_stop(struct ifnet *ifp, int disable)
 	struct atu_softc	*sc = ifp->if_softc;
 	struct ieee80211com	*ic = &sc->sc_ic;
 	struct atu_cdata	*cd;
-	usbd_status		err;
 	int s;
 
 	s = splnet();
@@ -2235,19 +2234,11 @@ atu_stop(struct ifnet *ifp, int disable)
 
 	/* Stop transfers. */
 	if (sc->atu_ep[ATU_ENDPT_RX] != NULL) {
-		err = usbd_abort_pipe(sc->atu_ep[ATU_ENDPT_RX]);
-		if (err) {
-			DPRINTF(("%s: abort rx pipe failed: %s\n",
-			    device_xname(sc->atu_dev), usbd_errstr(err)));
-		}
+		usbd_abort_pipe(sc->atu_ep[ATU_ENDPT_RX]);
 	}
 
 	if (sc->atu_ep[ATU_ENDPT_TX] != NULL) {
-		err = usbd_abort_pipe(sc->atu_ep[ATU_ENDPT_TX]);
-		if (err) {
-			DPRINTF(("%s: abort tx pipe failed: %s\n",
-			    device_xname(sc->atu_dev), usbd_errstr(err)));
-		}
+		usbd_abort_pipe(sc->atu_ep[ATU_ENDPT_TX]);
 	}
 
 	/* Free RX/TX/MGMT list resources. */
@@ -2257,20 +2248,12 @@ atu_stop(struct ifnet *ifp, int disable)
 
 	/* Close pipes */
 	if (sc->atu_ep[ATU_ENDPT_RX] != NULL) {
-		err = usbd_close_pipe(sc->atu_ep[ATU_ENDPT_RX]);
-		if (err) {
-			DPRINTF(("%s: close rx pipe failed: %s\n",
-			    device_xname(sc->atu_dev), usbd_errstr(err)));
-		}
+		usbd_close_pipe(sc->atu_ep[ATU_ENDPT_RX]);
 		sc->atu_ep[ATU_ENDPT_RX] = NULL;
 	}
 
 	if (sc->atu_ep[ATU_ENDPT_TX] != NULL) {
-		err = usbd_close_pipe(sc->atu_ep[ATU_ENDPT_TX]);
-		if (err) {
-			DPRINTF(("%s: close tx pipe failed: %s\n",
-			    device_xname(sc->atu_dev), usbd_errstr(err)));
-		}
+		usbd_close_pipe(sc->atu_ep[ATU_ENDPT_TX]);
 		sc->atu_ep[ATU_ENDPT_TX] = NULL;
 	}
 
diff --git a/sys/dev/usb/if_urtw.c b/sys/dev/usb/if_urtw.c
index 162141c8072f..f08db3030887 100644
--- a/sys/dev/usb/if_urtw.c
+++ b/sys/dev/usb/if_urtw.c
@@ -829,24 +829,17 @@ urtw_close_pipes(struct urtw_softc *sc)
 	usbd_status error = 0;
 
 	if (sc->sc_rxpipe != NULL) {
-		error = usbd_close_pipe(sc->sc_rxpipe);
-		if (error != 0)
-			goto fail;
+		usbd_close_pipe(sc->sc_rxpipe);
 		sc->sc_rxpipe = NULL;
 	}
 	if (sc->sc_txpipe_low != NULL) {
-		error = usbd_close_pipe(sc->sc_txpipe_low);
-		if (error != 0)
-			goto fail;
+		usbd_close_pipe(sc->sc_txpipe_low);
 		sc->sc_txpipe_low = NULL;
 	}
 	if (sc->sc_txpipe_normal != NULL) {
-		error = usbd_close_pipe(sc->sc_txpipe_normal);
-		if (error != 0)
-			goto fail;
+		usbd_close_pipe(sc->sc_txpipe_normal);
 		sc->sc_txpipe_normal = NULL;
 	}
-fail:
 	return error;
 }
 
diff --git a/sys/dev/usb/motg.c b/sys/dev/usb/motg.c
index 63339b4f0d54..36f5e01c4b64 100644
--- a/sys/dev/usb/motg.c
+++ b/sys/dev/usb/motg.c
@@ -1009,20 +1009,8 @@ motg_root_intr_abort(struct usbd_xfer *xfer)
 usbd_status
 motg_root_intr_transfer(struct usbd_xfer *xfer)
 {
-	struct motg_softc *sc = MOTG_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
-	/*
-	 * Pipe isn't running (otherwise err would be USBD_INPROG),
-	 * start first
-	 */
+	/* Pipe isn't running, start first */
 	return motg_root_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -1279,21 +1267,8 @@ motg_setup_endpoint_rx(struct usbd_xfer *xfer)
 static usbd_status
 motg_device_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	struct motg_softc *sc = MOTG_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	KASSERT(xfer->ux_status == USBD_NOT_STARTED);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
-	/*
-	 * Pipe isn't running (otherwise err would be USBD_INPROG),
-	 * so start it first.
-	 */
+	/* Pipe isn't running, so start it first.  */
 	return motg_device_ctrl_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -1731,24 +1706,9 @@ motg_device_ctrl_done(struct usbd_xfer *xfer)
 static usbd_status
 motg_device_data_transfer(struct usbd_xfer *xfer)
 {
-	struct motg_softc *sc = MOTG_XFER2SC(xfer);
-	usbd_status err;
-
 	MOTGHIST_FUNC(); MOTGHIST_CALLED();
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	DPRINTF("xfer %#jx status %jd", (uintptr_t)xfer, xfer->ux_status, 0, 0);
-	err = usb_insert_transfer(xfer);
-	KASSERT(xfer->ux_status == USBD_NOT_STARTED);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
-	/*
-	 * Pipe isn't running (otherwise err would be USBD_INPROG),
-	 * so start it first.
-	 */
+	/* Pipe isn't running, so start it first.  */
 	return motg_device_data_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -2262,6 +2222,5 @@ motg_abortx(struct usbd_xfer *xfer)
 		}
 	}
 dying:
-	usb_transfer_complete(xfer);
 	KASSERT(mutex_owned(&sc->sc_lock));
 }
diff --git a/sys/dev/usb/ohci.c b/sys/dev/usb/ohci.c
index bd8ab4f33a27..4e0f827c100f 100644
--- a/sys/dev/usb/ohci.c
+++ b/sys/dev/usb/ohci.c
@@ -2409,12 +2409,7 @@ ohci_abortx(struct usbd_xfer *xfer)
 	usb_syncmem(&sed->dma, sed->offs + offsetof(ohci_ed_t, ed_flags),
 	    sizeof(sed->ed.ed_flags),
 	    BUS_DMASYNC_PREWRITE | BUS_DMASYNC_PREREAD);
-
-	/*
-	 * Final step: Notify completion to waiting xfers.
-	 */
 dying:
-	usb_transfer_complete(xfer);
 	DPRINTFN(14, "end", 0, 0, 0, 0);
 
 	KASSERT(mutex_owned(&sc->sc_lock));
@@ -2616,15 +2611,6 @@ ohci_roothub_ctrl(struct usbd_bus *bus, usb_device_request_t *req,
 Static usbd_status
 ohci_root_intr_transfer(struct usbd_xfer *xfer)
 {
-	ohci_softc_t *sc = OHCI_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
 	/* Pipe isn't running, start first */
 	return ohci_root_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
@@ -2774,15 +2760,6 @@ ohci_device_ctrl_fini(struct usbd_xfer *xfer)
 Static usbd_status
 ohci_device_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	ohci_softc_t *sc = OHCI_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
 	/* Pipe isn't running, start first */
 	return ohci_device_ctrl_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
@@ -3062,15 +3039,6 @@ ohci_device_bulk_fini(struct usbd_xfer *xfer)
 Static usbd_status
 ohci_device_bulk_transfer(struct usbd_xfer *xfer)
 {
-	ohci_softc_t *sc = OHCI_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
 	/* Pipe isn't running, start first */
 	return ohci_device_bulk_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
@@ -3270,15 +3238,6 @@ ohci_device_intr_fini(struct usbd_xfer *xfer)
 Static usbd_status
 ohci_device_intr_transfer(struct usbd_xfer *xfer)
 {
-	ohci_softc_t *sc = OHCI_XFER2SC(xfer);
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
 	/* Pipe isn't running, start first */
 	return ohci_device_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
@@ -3574,20 +3533,10 @@ ohci_device_isoc_fini(struct usbd_xfer *xfer)
 usbd_status
 ohci_device_isoc_transfer(struct usbd_xfer *xfer)
 {
-	ohci_softc_t *sc = OHCI_XFER2SC(xfer);
-	usbd_status __diagused err;
-
 	OHCIHIST_FUNC(); OHCIHIST_CALLED();
 
 	DPRINTFN(5, "xfer=%#jx", (uintptr_t)xfer, 0, 0, 0);
 
-	/* Put it on our queue, */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-
-	KASSERT(err == USBD_NORMAL_COMPLETION);
-
 	/* insert into schedule, */
 	ohci_device_isoc_enter(xfer);
 
diff --git a/sys/dev/usb/ualea.c b/sys/dev/usb/ualea.c
index ede886968af4..d38722351672 100644
--- a/sys/dev/usb/ualea.c
+++ b/sys/dev/usb/ualea.c
@@ -161,14 +161,14 @@ ualea_detach(device_t self, int flags)
 
 	/* Cancel pending xfer.  */
 	if (sc->sc_pipe)
-		(void)usbd_abort_pipe(sc->sc_pipe);
+		usbd_abort_pipe(sc->sc_pipe);
 	KASSERT(!sc->sc_inflight);
 
 	/* All users have drained.  Tear it all down.  */
 	if (sc->sc_xfer)
 		usbd_destroy_xfer(sc->sc_xfer);
 	if (sc->sc_pipe)
-		(void)usbd_close_pipe(sc->sc_pipe);
+		usbd_close_pipe(sc->sc_pipe);
 	mutex_destroy(&sc->sc_lock);
 
 	return 0;
diff --git a/sys/dev/usb/uhci.c b/sys/dev/usb/uhci.c
index acf5e2e32c11..1e9718b112c0 100644
--- a/sys/dev/usb/uhci.c
+++ b/sys/dev/usb/uhci.c
@@ -2258,20 +2258,8 @@ uhci_device_bulk_fini(struct usbd_xfer *xfer)
 usbd_status
 uhci_device_bulk_transfer(struct usbd_xfer *xfer)
 {
-	uhci_softc_t *sc = UHCI_XFER2SC(xfer);
-	usbd_status err;
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
-	/*
-	 * Pipe isn't running (otherwise err would be USBD_INPROG),
-	 * so start it first.
-	 */
+	/* Pipe isn't running, so start it first.  */
 	return uhci_device_bulk_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -2414,15 +2402,10 @@ uhci_abortx(struct usbd_xfer *xfer)
 	 */
 	/* Hardware finishes in 1ms */
 	usb_delay_ms_locked(upipe->pipe.up_dev->ud_bus, 2, &sc->sc_lock);
-
-	/*
-	 * HC Step 3: Notify completion to waiting xfers.
-	 */
 dying:
 #ifdef DIAGNOSTIC
 	ux->ux_isdone = true;
 #endif
-	usb_transfer_complete(xfer);
 	DPRINTFN(14, "end", 0, 0, 0, 0);
 
 	KASSERT(mutex_owned(&sc->sc_lock));
@@ -2495,20 +2478,8 @@ uhci_device_ctrl_fini(struct usbd_xfer *xfer)
 usbd_status
 uhci_device_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	uhci_softc_t *sc = UHCI_XFER2SC(xfer);
-	usbd_status err;
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
-	/*
-	 * Pipe isn't running (otherwise err would be USBD_INPROG),
-	 * so start it first.
-	 */
+	/* Pipe isn't running, so start it first.  */
 	return uhci_device_ctrl_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -2701,20 +2672,8 @@ uhci_device_intr_fini(struct usbd_xfer *xfer)
 usbd_status
 uhci_device_intr_transfer(struct usbd_xfer *xfer)
 {
-	uhci_softc_t *sc = UHCI_XFER2SC(xfer);
-	usbd_status err;
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
-	/*
-	 * Pipe isn't running (otherwise err would be USBD_INPROG),
-	 * so start it first.
-	 */
+	/* Pipe isn't running, so start it first.  */
 	return uhci_device_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -2891,18 +2850,10 @@ usbd_status
 uhci_device_isoc_transfer(struct usbd_xfer *xfer)
 {
 	uhci_softc_t *sc = UHCI_XFER2SC(xfer);
-	usbd_status err __diagused;
 
 	UHCIHIST_FUNC(); UHCIHIST_CALLED();
 	DPRINTFN(5, "xfer=%#jx", (uintptr_t)xfer, 0, 0, 0);
 
-	/* Put it on our queue, */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-
-	KASSERT(err == USBD_NORMAL_COMPLETION);
-
 	/* insert into schedule, */
 
 	struct uhci_pipe *upipe = UHCI_PIPE2UPIPE(xfer->ux_pipe);
@@ -3890,20 +3841,8 @@ uhci_root_intr_abort(struct usbd_xfer *xfer)
 usbd_status
 uhci_root_intr_transfer(struct usbd_xfer *xfer)
 {
-	uhci_softc_t *sc = UHCI_XFER2SC(xfer);
-	usbd_status err;
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
-	/*
-	 * Pipe isn't running (otherwise err would be USBD_INPROG),
-	 * start first
-	 */
+	/* Pipe isn't running, start first */
 	return uhci_root_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
diff --git a/sys/dev/usb/usb_mem.h b/sys/dev/usb/usb_mem.h
index 5473c1b3b810..b93d694cfc3c 100644
--- a/sys/dev/usb/usb_mem.h
+++ b/sys/dev/usb/usb_mem.h
@@ -31,6 +31,16 @@
  * POSSIBILITY OF SUCH DAMAGE.
  */
 
+#ifndef	_DEV_USB_USB_MEM_H_
+#define	_DEV_USB_USB_MEM_H_
+
+#include <sys/types.h>
+
+#include <sys/bus.h>
+#include <sys/queue.h>
+
+#include <dev/usb/usbdivar.h>
+
 typedef struct usb_dma_block {
 	bus_dma_tag_t tag;
 	bus_dmamap_t map;
@@ -60,3 +70,5 @@ bus_addr_t	usb_dmaaddr(usb_dma_t *, unsigned int);
 #define DMAADDR(dma, o)	usb_dmaaddr((dma), (o))
 #define KERNADDR(dma, o) \
 	((void *)((char *)(dma)->udma_block->kaddr + (dma)->udma_offs + (o)))
+
+#endif	/* _DEV_USB_USB_MEM_H_ */
diff --git a/sys/dev/usb/usbdi.c b/sys/dev/usb/usbdi.c
index ea5a3430dd03..e70bc288591d 100644
--- a/sys/dev/usb/usbdi.c
+++ b/sys/dev/usb/usbdi.c
@@ -115,14 +115,14 @@ SDT_PROBE_DEFINE2(usb, device, xfer, done,
     "usbd_status"/*status*/);
 SDT_PROBE_DEFINE1(usb, device, xfer, destroy,  "struct usbd_xfer *"/*xfer*/);
 
-Static usbd_status usbd_ar_pipe(struct usbd_pipe *);
+Static void usbd_ar_pipe(struct usbd_pipe *);
 Static void usbd_start_next(struct usbd_pipe *);
 Static usbd_status usbd_open_pipe_ival
 	(struct usbd_interface *, uint8_t, uint8_t, struct usbd_pipe **, int);
 static void *usbd_alloc_buffer(struct usbd_xfer *, uint32_t);
 static void usbd_free_buffer(struct usbd_xfer *);
 static struct usbd_xfer *usbd_alloc_xfer(struct usbd_device *, unsigned int);
-static usbd_status usbd_free_xfer(struct usbd_xfer *);
+static void usbd_free_xfer(struct usbd_xfer *);
 static void usbd_request_async_cb(struct usbd_xfer *, void *, usbd_status);
 static void usbd_xfer_timeout(void *);
 static void usbd_xfer_timeout_task(void *);
@@ -322,7 +322,7 @@ usbd_open_pipe_intr(struct usbd_interface *iface, uint8_t address,
 	return err;
 }
 
-usbd_status
+void
 usbd_close_pipe(struct usbd_pipe *pipe)
 {
 	USBHIST_FUNC(); USBHIST_CALLED(usbdebug);
@@ -331,6 +331,7 @@ usbd_close_pipe(struct usbd_pipe *pipe)
 
 	usbd_lock_pipe(pipe);
 	SDT_PROBE1(usb, device, pipe, close,  pipe);
+	KASSERTMSG(pipe->up_aborting, "pipe %p not aborted first", pipe);
 	if (!SIMPLEQ_EMPTY(&pipe->up_queue)) {
 		printf("WARNING: pipe closed with active xfers on addr %d\n",
 		    pipe->up_dev->ud_addr);
@@ -349,8 +350,6 @@ usbd_close_pipe(struct usbd_pipe *pipe)
 	if (pipe->up_iface)
 		usbd_iface_pipeunref(pipe->up_iface);
 	kmem_free(pipe, pipe->up_dev->ud_bus->ub_pipesize);
-
-	return USBD_NORMAL_COMPLETION;
 }
 
 usbd_status
@@ -372,13 +371,6 @@ usbd_transfer(struct usbd_xfer *xfer)
 #endif
 	xfer->ux_done = 0;
 
-	if (pipe->up_aborting) {
-		USBHIST_LOG(usbdebug, "<- done xfer %#jx, aborting",
-		    (uintptr_t)xfer, 0, 0, 0);
-		SDT_PROBE2(usb, device, xfer, done,  xfer, USBD_CANCELLED);
-		return USBD_CANCELLED;
-	}
-
 	KASSERT(xfer->ux_length == 0 || xfer->ux_buf != NULL);
 
 	size = xfer->ux_length;
@@ -406,9 +398,38 @@ usbd_transfer(struct usbd_xfer *xfer)
 		}
 	}
 
+	usbd_lock_pipe(pipe);
+	if (pipe->up_aborting) {
+		/*
+		 * XXX For synchronous transfers this is fine.  What to
+		 * do for asynchronous transfers?  The callback is
+		 * never run, not even with status USBD_CANCELLED.
+		 */
+		usbd_unlock_pipe(pipe);
+		USBHIST_LOG(usbdebug, "<- done xfer %#jx, aborting",
+		    (uintptr_t)xfer, 0, 0, 0);
+		SDT_PROBE2(usb, device, xfer, done,  xfer, USBD_CANCELLED);
+		return USBD_CANCELLED;
+	}
+
 	/* xfer is not valid after the transfer method unless synchronous */
 	SDT_PROBE2(usb, device, pipe, transfer__start,  pipe, xfer);
-	err = pipe->up_methods->upm_transfer(xfer);
+	do {
+#ifdef DIAGNOSTIC
+		xfer->ux_state = XFER_ONQU;
+#endif
+		SIMPLEQ_INSERT_TAIL(&pipe->up_queue, xfer, ux_next);
+		if (pipe->up_running && pipe->up_serialise) {
+			err = USBD_IN_PROGRESS;
+		} else {
+			pipe->up_running = 1;
+			err = USBD_NORMAL_COMPLETION;
+		}
+		usbd_unlock_pipe(pipe);
+		if (err)
+			break;
+		err = pipe->up_methods->upm_transfer(xfer);
+	} while (0);
 	SDT_PROBE3(usb, device, pipe, transfer__done,  pipe, xfer, err);
 
 	if (err != USBD_IN_PROGRESS && err) {
@@ -584,7 +605,7 @@ out:
 	return xfer;
 }
 
-static usbd_status
+static void
 usbd_free_xfer(struct usbd_xfer *xfer)
 {
 	USBHIST_FUNC();
@@ -604,7 +625,6 @@ usbd_free_xfer(struct usbd_xfer *xfer)
 
 	cv_destroy(&xfer->ux_cv);
 	xfer->ux_bus->ub_methods->ubm_freex(xfer->ux_bus, xfer);
-	return USBD_NORMAL_COMPLETION;
 }
 
 int
@@ -763,23 +783,21 @@ usbd_interface2endpoint_descriptor(struct usbd_interface *iface, uint8_t index)
 
 /* Some drivers may wish to abort requests on the default pipe, *
  * but there is no mechanism for getting a handle on it.        */
-usbd_status
+void
 usbd_abort_default_pipe(struct usbd_device *device)
 {
-	return usbd_abort_pipe(device->ud_pipe0);
+	usbd_abort_pipe(device->ud_pipe0);
 }
 
-usbd_status
+void
 usbd_abort_pipe(struct usbd_pipe *pipe)
 {
-	usbd_status err;
 
 	KASSERT(pipe != NULL);
 
 	usbd_lock_pipe(pipe);
-	err = usbd_ar_pipe(pipe);
+	usbd_ar_pipe(pipe);
 	usbd_unlock_pipe(pipe);
-	return err;
 }
 
 usbd_status
@@ -960,7 +978,7 @@ usbd_get_interface(struct usbd_interface *iface, uint8_t *aiface)
 /*** Internal routines ***/
 
 /* Dequeue all pipe operations, called with bus lock held. */
-Static usbd_status
+Static void
 usbd_ar_pipe(struct usbd_pipe *pipe)
 {
 	struct usbd_xfer *xfer;
@@ -1002,9 +1020,7 @@ usbd_ar_pipe(struct usbd_pipe *pipe)
 			/* XXX only for non-0 usbd_clear_endpoint_stall(pipe); */
 		}
 	}
-	pipe->up_aborting = 0;
 	SDT_PROBE1(usb, device, pipe, abort__done,  pipe);
-	return USBD_NORMAL_COMPLETION;
 }
 
 /* Called with USB lock held. */
@@ -1137,37 +1153,6 @@ usb_transfer_complete(struct usbd_xfer *xfer)
 		usbd_start_next(pipe);
 }
 
-/* Called with USB lock held. */
-usbd_status
-usb_insert_transfer(struct usbd_xfer *xfer)
-{
-	struct usbd_pipe *pipe = xfer->ux_pipe;
-	usbd_status err;
-
-	USBHIST_FUNC(); USBHIST_CALLARGS(usbdebug,
-	    "xfer = %#jx pipe = %#jx running = %jd timeout = %jd",
-	    (uintptr_t)xfer, (uintptr_t)pipe,
-	    pipe->up_running, xfer->ux_timeout);
-
-	KASSERT(mutex_owned(pipe->up_dev->ud_bus->ub_lock));
-	KASSERTMSG(xfer->ux_state == XFER_BUSY, "xfer %p state is %x", xfer,
-	    xfer->ux_state);
-
-#ifdef DIAGNOSTIC
-	xfer->ux_state = XFER_ONQU;
-#endif
-	SIMPLEQ_INSERT_TAIL(&pipe->up_queue, xfer, ux_next);
-	if (pipe->up_running && pipe->up_serialise)
-		err = USBD_IN_PROGRESS;
-	else {
-		pipe->up_running = 1;
-		err = USBD_NORMAL_COMPLETION;
-	}
-	USBHIST_LOG(usbdebug, "<- done xfer %#jx, err %jd", (uintptr_t)xfer,
-	    err, 0, 0);
-	return err;
-}
-
 /* Called with USB lock held. */
 void
 usbd_start_next(struct usbd_pipe *pipe)
@@ -1533,11 +1518,13 @@ usbd_xfer_abort(struct usbd_xfer *xfer)
 	usbd_xfer_cancel_timeout_async(xfer);
 
 	/*
-	 * We beat everyone else.  Claim the status as cancelled and do
-	 * the bus-specific dance to abort the hardware.
+	 * We beat everyone else.  Claim the status as cancelled, do
+	 * the bus-specific dance to abort the hardware, and complete
+	 * the xfer.
 	 */
 	xfer->ux_status = USBD_CANCELLED;
 	bus->ub_methods->ubm_abortx(xfer);
+	usb_transfer_complete(xfer);
 }
 
 /*
@@ -1617,11 +1604,13 @@ usbd_xfer_timeout_task(void *cookie)
 		goto out;
 
 	/*
-	 * We beat everyone else.  Claim the status as timed out and do
-	 * the bus-specific dance to abort the hardware.
+	 * We beat everyone else.  Claim the status as timed out, do
+	 * the bus-specific dance to abort the hardware, and complete
+	 * the xfer.
 	 */
 	xfer->ux_status = USBD_TIMEOUT;
 	bus->ub_methods->ubm_abortx(xfer);
+	usb_transfer_complete(xfer);
 
 out:	/* All done -- release the lock.  */
 	mutex_exit(bus->ub_lock);
diff --git a/sys/dev/usb/usbdi.h b/sys/dev/usb/usbdi.h
index 71d61eca5394..c9c4274adb65 100644
--- a/sys/dev/usb/usbdi.h
+++ b/sys/dev/usb/usbdi.h
@@ -91,7 +91,7 @@ usbd_status usbd_open_pipe_intr(struct usbd_interface *, uint8_t, uint8_t,
     struct usbd_pipe **, void *, void *, uint32_t, usbd_callback, int);
 usbd_status usbd_open_pipe(struct usbd_interface *, uint8_t, uint8_t,
      struct usbd_pipe **);
-usbd_status usbd_close_pipe(struct usbd_pipe *);
+void usbd_close_pipe(struct usbd_pipe *);
 
 usbd_status usbd_transfer(struct usbd_xfer *);
 
@@ -118,8 +118,8 @@ void usbd_get_xfer_status(struct usbd_xfer *, void **,
 usb_endpoint_descriptor_t *usbd_interface2endpoint_descriptor
     (struct usbd_interface *, uint8_t);
 
-usbd_status usbd_abort_pipe(struct usbd_pipe *);
-usbd_status usbd_abort_default_pipe(struct usbd_device *);
+void usbd_abort_pipe(struct usbd_pipe *);
+void usbd_abort_default_pipe(struct usbd_device *);
 
 usbd_status usbd_clear_endpoint_stall(struct usbd_pipe *);
 void usbd_clear_endpoint_stall_async(struct usbd_pipe *);
diff --git a/sys/dev/usb/usbdivar.h b/sys/dev/usb/usbdivar.h
index e993fc25b7a1..3f92e323afb2 100644
--- a/sys/dev/usb/usbdivar.h
+++ b/sys/dev/usb/usbdivar.h
@@ -30,6 +30,9 @@
  * POSSIBILITY OF SUCH DAMAGE.
  */
 
+#ifndef	_DEV_USB_USBDIVAR_H_
+#define	_DEV_USB_USBDIVAR_H_
+
 /*
  * Discussion about locking in the USB code:
  *
@@ -41,16 +44,19 @@
  *	BUS METHOD		LOCK	NOTES
  *	----------------------- -------	-------------------------
  *	ubm_open		-	might want to take lock?
- *	ubm_softint		x
+ *	ubm_softint		x	may release/reacquire lock
  *	ubm_dopoll		-	might want to take lock?
  *	ubm_allocx		-
  *	ubm_freex		-
+ *	ubm_abortx		x	must not release/reacquire lock
  *	ubm_getlock 		-	Called at attach time
  *	ubm_newdev		-	Will take lock
-	ubm_rhctrl
+ *	ubm_rhctrl
  *
  *	PIPE METHOD		LOCK	NOTES
  *	----------------------- -------	-------------------------
+ *	upm_init		-
+ *	upm_fini		-
  *	upm_transfer		-
  *	upm_start		-	might want to take lock?
  *	upm_abort		x
@@ -64,7 +70,6 @@
  * USB functions known to expect the lock taken include (this list is
  * probably not exhaustive):
  *    usb_transfer_complete()
- *    usb_insert_transfer()
  *    usb_start_next()
  *
  */
@@ -356,7 +361,6 @@ void		usbd_iface_pipeunref(struct usbd_interface *);
 usbd_status	usbd_fill_iface_data(struct usbd_device *, int, int);
 void		usb_free_device(struct usbd_device *);
 
-usbd_status	usb_insert_transfer(struct usbd_xfer *);
 void		usb_transfer_complete(struct usbd_xfer *);
 int		usb_disconnect_port(struct usbd_port *, device_t, int);
 
@@ -397,3 +401,5 @@ usb_addr2dindex(int addr)
 
 #define usbd_lock_pipe(p)	mutex_enter((p)->up_dev->ud_bus->ub_lock)
 #define usbd_unlock_pipe(p)	mutex_exit((p)->up_dev->ud_bus->ub_lock)
+
+#endif	/* _DEV_USB_USBDIVAR_H_ */
diff --git a/sys/dev/usb/usbnet.c b/sys/dev/usb/usbnet.c
index 1f57e11ea39e..50603bed02d8 100644
--- a/sys/dev/usb/usbnet.c
+++ b/sys/dev/usb/usbnet.c
@@ -779,10 +779,7 @@ usbnet_ep_close_pipes(struct usbnet * const un)
 	for (size_t i = 0; i < __arraycount(unp->unp_ep); i++) {
 		if (unp->unp_ep[i] == NULL)
 			continue;
-		usbd_status err = usbd_close_pipe(unp->unp_ep[i]);
-		if (err)
-			aprint_error_dev(un->un_dev, "close pipe %zu: %s\n", i,
-			    usbd_errstr(err));
+		usbd_close_pipe(unp->unp_ep[i]);
 		unp->unp_ep[i] = NULL;
 	}
 }
@@ -817,21 +814,16 @@ usbnet_ep_open_pipes(struct usbnet * const un)
 	return USBD_NORMAL_COMPLETION;
 }
 
-static usbd_status
+static void
 usbnet_ep_stop_pipes(struct usbnet * const un)
 {
 	struct usbnet_private * const unp = un->un_pri;
-	usbd_status err = USBD_NORMAL_COMPLETION;
 
 	for (size_t i = 0; i < __arraycount(unp->unp_ep); i++) {
 		if (unp->unp_ep[i] == NULL)
 			continue;
-		usbd_status err2 = usbd_abort_pipe(unp->unp_ep[i]);
-		if (err == USBD_NORMAL_COMPLETION && err2)
-			err = err2;
+		usbd_abort_pipe(unp->unp_ep[i]);
 	}
-
-	return err;
 }
 
 static int
@@ -1208,17 +1200,13 @@ usbnet_watchdog(struct ifnet *ifp)
 	struct usbnet * const un = ifp->if_softc;
 	struct usbnet_private * const unp = un->un_pri;
 	struct usbnet_cdata * const cd = un_cdata(un);
-	usbd_status err;
 
 	if_statinc(ifp, if_oerrors);
 	device_printf(un->un_dev, "watchdog timeout\n");
 
 	if (cd->uncd_tx_cnt > 0) {
 		DPRINTF("uncd_tx_cnt=%ju non zero, aborting pipe", 0, 0, 0, 0);
-		err = usbd_abort_pipe(unp->unp_ep[USBNET_ENDPT_TX]);
-		if (err)
-			device_printf(un->un_dev, "pipe abort failed: %s\n",
-			    usbd_errstr(err));
+		usbd_abort_pipe(unp->unp_ep[USBNET_ENDPT_TX]);
 		if (cd->uncd_tx_cnt != 0)
 			DPRINTF("uncd_tx_cnt now %ju", cd->uncd_tx_cnt, 0, 0, 0);
 	}
diff --git a/sys/dev/usb/usbroothub.c b/sys/dev/usb/usbroothub.c
index 79010ba90fd4..f3cb54bed973 100644
--- a/sys/dev/usb/usbroothub.c
+++ b/sys/dev/usb/usbroothub.c
@@ -345,16 +345,6 @@ static const usb_hub_descriptor_t usbroothub_hubd = {
 usbd_status
 roothub_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	struct usbd_pipe *pipe = xfer->ux_pipe;
-	struct usbd_bus *bus = pipe->up_dev->ud_bus;
-	usbd_status err;
-
-	/* Insert last in queue. */
-	mutex_enter(bus->ub_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(bus->ub_lock);
-	if (err)
-		return err;
 
 	/* Pipe isn't running, start first */
 	return roothub_ctrl_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
diff --git a/sys/dev/usb/utoppy.c b/sys/dev/usb/utoppy.c
index a40352fedf90..b412542d5c3d 100644
--- a/sys/dev/usb/utoppy.c
+++ b/sys/dev/usb/utoppy.c
@@ -1365,7 +1365,6 @@ static int
 utoppyclose(dev_t dev, int flag, int mode, struct lwp *l)
 {
 	struct utoppy_softc *sc;
-	usbd_status err;
 
 	sc = device_lookup_private(&utoppy_cd, UTOPPYUNIT(dev));
 
@@ -1384,14 +1383,12 @@ utoppyclose(dev_t dev, int flag, int mode, struct lwp *l)
 		(void) utoppy_cancel(sc);
 
 	if (sc->sc_out_pipe != NULL) {
-		if ((err = usbd_abort_pipe(sc->sc_out_pipe)) != 0)
-			printf("usbd_abort_pipe(OUT) returned %d\n", err);
+		usbd_abort_pipe(sc->sc_out_pipe);
 		sc->sc_out_pipe = NULL;
 	}
 
 	if (sc->sc_in_pipe != NULL) {
-		if ((err = usbd_abort_pipe(sc->sc_in_pipe)) != 0)
-			printf("usbd_abort_pipe(IN) returned %d\n", err);
+		usbd_abort_pipe(sc->sc_in_pipe);
 		sc->sc_in_pipe = NULL;
 	}
 
diff --git a/sys/dev/usb/vhci.c b/sys/dev/usb/vhci.c
index f5c46b9a774e..a73b14e1cb5a 100644
--- a/sys/dev/usb/vhci.c
+++ b/sys/dev/usb/vhci.c
@@ -590,18 +590,9 @@ vhci_roothub_ctrl(struct usbd_bus *bus, usb_device_request_t *req,
 static usbd_status
 vhci_device_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	vhci_softc_t *sc = xfer->ux_bus->ub_hcpriv;
-	usbd_status err;
 
 	DPRINTF("%s: called\n", __func__);
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
 	/* Pipe isn't running, start first */
 	return vhci_device_ctrl_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
@@ -707,18 +698,9 @@ vhci_device_ctrl_done(struct usbd_xfer *xfer)
 static usbd_status
 vhci_root_intr_transfer(struct usbd_xfer *xfer)
 {
-	vhci_softc_t *sc = xfer->ux_bus->ub_hcpriv;
-	usbd_status err;
 
 	DPRINTF("%s: called\n", __func__);
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
 	/* Pipe isn't running, start first */
 	return vhci_root_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
diff --git a/sys/dev/usb/xhci.c b/sys/dev/usb/xhci.c
index cb40eb7832eb..ad3650760a69 100644
--- a/sys/dev/usb/xhci.c
+++ b/sys/dev/usb/xhci.c
@@ -154,15 +154,18 @@ static usbd_status xhci_new_device(device_t, struct usbd_bus *, int, int, int,
 static int xhci_roothub_ctrl(struct usbd_bus *, usb_device_request_t *,
     void *, int);
 
+static void xhci_pipe_async_task(void *);
+static void xhci_pipe_restart(struct usbd_pipe *);
+
 static usbd_status xhci_configure_endpoint(struct usbd_pipe *);
 //static usbd_status xhci_unconfigure_endpoint(struct usbd_pipe *);
-static usbd_status xhci_reset_endpoint(struct usbd_pipe *);
+static void xhci_reset_endpoint(struct usbd_pipe *);
 static usbd_status xhci_stop_endpoint_cmd(struct xhci_softc *,
     struct xhci_slot *, u_int, uint32_t);
 static usbd_status xhci_stop_endpoint(struct usbd_pipe *);
 
 static void xhci_host_dequeue(struct xhci_ring * const);
-static usbd_status xhci_set_dequeue(struct usbd_pipe *);
+static void xhci_set_dequeue(struct usbd_pipe *);
 
 static usbd_status xhci_do_command(struct xhci_softc * const,
     struct xhci_soft_trb * const, int);
@@ -1857,14 +1860,13 @@ xhci_unconfigure_endpoint(struct usbd_pipe *pipe)
 #endif
 
 /* 4.6.8, 6.4.3.7 */
-static usbd_status
-xhci_reset_endpoint_locked(struct usbd_pipe *pipe)
+static void
+xhci_reset_endpoint(struct usbd_pipe *pipe)
 {
 	struct xhci_softc * const sc = XHCI_PIPE2SC(pipe);
 	struct xhci_slot * const xs = pipe->up_dev->ud_hcpriv;
 	const u_int dci = xhci_ep_get_dci(pipe->up_endpoint->ue_edesc);
 	struct xhci_soft_trb trb;
-	usbd_status err;
 
 	XHCIHIST_FUNC();
 	XHCIHIST_CALLARGS("slot %ju dci %ju", xs->xs_idx, dci, 0, 0);
@@ -1877,21 +1879,10 @@ xhci_reset_endpoint_locked(struct usbd_pipe *pipe)
 	    XHCI_TRB_3_EP_SET(dci) |
 	    XHCI_TRB_3_TYPE_SET(XHCI_TRB_TYPE_RESET_EP);
 
-	err = xhci_do_command_locked(sc, &trb, USBD_DEFAULT_TIMEOUT);
-
-	return err;
-}
-
-static usbd_status
-xhci_reset_endpoint(struct usbd_pipe *pipe)
-{
-	struct xhci_softc * const sc = XHCI_PIPE2SC(pipe);
-
-	mutex_enter(&sc->sc_lock);
-	usbd_status ret = xhci_reset_endpoint_locked(pipe);
-	mutex_exit(&sc->sc_lock);
-
-	return ret;
+	if (xhci_do_command_locked(sc, &trb, USBD_DEFAULT_TIMEOUT)) {
+		device_printf(sc->sc_dev, "%s: endpoint 0x%x: timed out\n",
+		    __func__, pipe->up_endpoint->ue_edesc->bEndpointAddress);
+	}
 }
 
 /*
@@ -1946,15 +1937,14 @@ xhci_stop_endpoint(struct usbd_pipe *pipe)
  * EPSTATE of endpoint must be ERROR or STOPPED, otherwise CONTEXT_STATE
  * error will be generated.
  */
-static usbd_status
-xhci_set_dequeue_locked(struct usbd_pipe *pipe)
+static void
+xhci_set_dequeue(struct usbd_pipe *pipe)
 {
 	struct xhci_softc * const sc = XHCI_PIPE2SC(pipe);
 	struct xhci_slot * const xs = pipe->up_dev->ud_hcpriv;
 	const u_int dci = xhci_ep_get_dci(pipe->up_endpoint->ue_edesc);
 	struct xhci_ring * const xr = xs->xs_xr[dci];
 	struct xhci_soft_trb trb;
-	usbd_status err;
 
 	XHCIHIST_FUNC();
 	XHCIHIST_CALLARGS("slot %ju dci %ju", xs->xs_idx, dci, 0, 0);
@@ -1971,21 +1961,10 @@ xhci_set_dequeue_locked(struct usbd_pipe *pipe)
 	    XHCI_TRB_3_EP_SET(dci) |
 	    XHCI_TRB_3_TYPE_SET(XHCI_TRB_TYPE_SET_TR_DEQUEUE);
 
-	err = xhci_do_command_locked(sc, &trb, USBD_DEFAULT_TIMEOUT);
-
-	return err;
-}
-
-static usbd_status
-xhci_set_dequeue(struct usbd_pipe *pipe)
-{
-	struct xhci_softc * const sc = XHCI_PIPE2SC(pipe);
-
-	mutex_enter(&sc->sc_lock);
-	usbd_status ret = xhci_set_dequeue_locked(pipe);
-	mutex_exit(&sc->sc_lock);
-
-	return ret;
+	if (xhci_do_command_locked(sc, &trb, USBD_DEFAULT_TIMEOUT)) {
+		device_printf(sc->sc_dev, "%s: endpoint 0x%x: timed out\n",
+		    __func__, pipe->up_endpoint->ue_edesc->bEndpointAddress);
+	}
 }
 
 /*
@@ -2035,6 +2014,9 @@ xhci_open(struct usbd_pipe *pipe)
 		return USBD_NORMAL_COMPLETION;
 	}
 
+	usb_init_task(&xpipe->xp_async_task, xhci_pipe_async_task, xpipe,
+	    USB_TASKQ_MPSAFE);
+
 	switch (xfertype) {
 	case UE_CONTROL:
 		pipe->up_methods = &xhci_device_ctrl_methods;
@@ -2080,6 +2062,8 @@ xhci_open(struct usbd_pipe *pipe)
 static void
 xhci_close_pipe(struct usbd_pipe *pipe)
 {
+	struct xhci_pipe * const xp =
+	    container_of(pipe, struct xhci_pipe, xp_pipe);
 	struct xhci_softc * const sc = XHCI_PIPE2SC(pipe);
 	struct xhci_slot * const xs = pipe->up_dev->ud_hcpriv;
 	usb_endpoint_descriptor_t * const ed = pipe->up_endpoint->ue_edesc;
@@ -2089,6 +2073,9 @@ xhci_close_pipe(struct usbd_pipe *pipe)
 
 	XHCIHIST_FUNC();
 
+	usb_rem_task_wait(pipe->up_dev, &xp->xp_async_task, USB_TASKQ_HC,
+	    &sc->sc_lock);
+
 	if (sc->sc_dying)
 		return;
 
@@ -2148,68 +2135,25 @@ xhci_close_pipe(struct usbd_pipe *pipe)
 
 /*
  * Abort transfer.
- * Should be called with sc_lock held.
+ * Should be called with sc_lock held.  Must not drop sc_lock.
  */
 static void
 xhci_abortx(struct usbd_xfer *xfer)
 {
 	XHCIHIST_FUNC();
 	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_CALLARGS("xfer %#jx pipe %#jx",
 	    (uintptr_t)xfer, (uintptr_t)xfer->ux_pipe, 0, 0);
 
 	KASSERT(mutex_owned(&sc->sc_lock));
-	ASSERT_SLEEPABLE();
-
 	KASSERTMSG((xfer->ux_status == USBD_CANCELLED ||
 		xfer->ux_status == USBD_TIMEOUT),
 	    "bad abort status: %d", xfer->ux_status);
 
-	/*
-	 * If we're dying, skip the hardware action and just notify the
-	 * 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;
-	}
+	xhci_pipe_restart(xfer->ux_pipe);
 
-	/*
-	 * HC Step 1: Stop execution of TD on the ring.
-	 */
-	switch (xhci_get_epstate(sc, xs, dci)) {
-	case XHCI_EPSTATE_HALTED:
-		(void)xhci_reset_endpoint_locked(xfer->ux_pipe);
-		break;
-	case XHCI_EPSTATE_STOPPED:
-		break;
-	default:
-		(void)xhci_stop_endpoint(xfer->ux_pipe);
-		break;
-	}
-#ifdef DIAGNOSTIC
-	uint32_t epst = xhci_get_epstate(sc, xs, dci);
-	if (epst != XHCI_EPSTATE_STOPPED)
-		DPRINTFN(4, "dci %ju not stopped %ju", dci, epst, 0, 0);
-#endif
-
-	/*
-	 * HC Step 2: Remove any vestiges of the xfer from the ring.
-	 */
-	xhci_set_dequeue_locked(xfer->ux_pipe);
-
-	/*
-	 * Final Step: Notify completion to waiting xfers.
-	 */
-dying:
-	usb_transfer_complete(xfer);
 	DPRINTFN(14, "end", 0, 0, 0, 0);
-
-	KASSERT(mutex_owned(&sc->sc_lock));
 }
 
 static void
@@ -2226,69 +2170,102 @@ xhci_host_dequeue(struct xhci_ring * const xr)
 }
 
 /*
- * Recover STALLed endpoint.
+ * Recover STALLed endpoint, or stop endpoint to abort a pipe.
  * xHCI 1.1 sect 4.10.2.1
  * Issue RESET_EP to recover halt condition and SET_TR_DEQUEUE to remove
  * all transfers on transfer ring.
  * These are done in thread context asynchronously.
  */
 static void
-xhci_clear_endpoint_stall_async_task(void *cookie)
+xhci_pipe_async_task(void *cookie)
 {
-	struct usbd_xfer * const xfer = cookie;
-	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);
+	struct xhci_pipe * const xp = cookie;
+	struct usbd_pipe * const pipe = &xp->xp_pipe;
+	struct xhci_softc * const sc = XHCI_PIPE2SC(pipe);
+	struct xhci_slot * const xs = pipe->up_dev->ud_hcpriv;
+	const u_int dci = xhci_ep_get_dci(pipe->up_endpoint->ue_edesc);
 	struct xhci_ring * const tr = xs->xs_xr[dci];
+	bool restart = false;
 
 	XHCIHIST_FUNC();
-	XHCIHIST_CALLARGS("xfer %#jx slot %ju dci %ju", (uintptr_t)xfer, xs->xs_idx,
-	    dci, 0);
+	XHCIHIST_CALLARGS("pipe %#jx slot %ju dci %ju",
+	    (uintptr_t)pipe, xs->xs_idx, dci, 0);
+
+	mutex_enter(&sc->sc_lock);
 
 	/*
-	 * XXXMRG: Stall task can run after slot is disabled when yanked.
-	 * This hack notices that the xs has been memset() in
-	 * xhci_disable_slot() and returns.  Both xhci_reset_endpoint()
-	 * and xhci_set_dequeue() rely upon a valid ring setup for correct
-	 * operation, and the latter will fault, as would
-	 * usb_transfer_complete() if it got that far.
+	 * - If the endpoint is halted, indicating a stall, reset it.
+	 * - If the endpoint is stopped, we're already good.
+	 * - Otherwise, someone wanted to abort the pipe, so stop the
+	 *   endpoint.
+	 *
+	 * In any case, clear the ring.
 	 */
-	if (xs->xs_idx == 0) {
-		DPRINTFN(4, "ends xs_idx is 0", 0, 0, 0, 0);
-		return;
+	switch (xhci_get_epstate(sc, xs, dci)) {
+	case XHCI_EPSTATE_HALTED:
+		xhci_reset_endpoint(pipe);
+		break;
+	case XHCI_EPSTATE_STOPPED:
+		break;
+	default:
+		xhci_stop_endpoint(pipe);
+		break;
 	}
+	switch (xhci_get_epstate(sc, xs, dci)) {
+	case XHCI_EPSTATE_STOPPED:
+		break;
+	case XHCI_EPSTATE_ERROR:
+		device_printf(sc->sc_dev, "endpoint 0x%x error\n",
+		    pipe->up_endpoint->ue_edesc->bEndpointAddress);
+		break;
+	default:
+		device_printf(sc->sc_dev, "endpoint 0x%x failed to stop\n",
+		    pipe->up_endpoint->ue_edesc->bEndpointAddress);
+	}
+	xhci_set_dequeue(pipe);
 
-	KASSERT(tr != NULL);
-
-	xhci_reset_endpoint(xfer->ux_pipe);
-	xhci_set_dequeue(xfer->ux_pipe);
+	/*
+	 * If we halted our own queue because it stalled, mark it no
+	 * longer halted and arrange to start it up again.
+	 */
+	if (tr->is_halted) {
+		tr->is_halted = false;
+		if (!SIMPLEQ_EMPTY(&pipe->up_queue))
+			restart = true;
+	}
 
-	mutex_enter(&sc->sc_lock);
-	tr->is_halted = false;
-	usb_transfer_complete(xfer);
 	mutex_exit(&sc->sc_lock);
+
+	/*
+	 * If the endpoint was stalled, start issuing queued transfers
+	 * again.
+	 */
+	if (restart) {
+		/*
+		 * XXX Shouldn't touch the queue unlocked -- upm_start
+		 * should be called with the lock held instead.  The
+		 * pipe could be aborted at this point, and the xfer
+		 * freed.
+		 */
+		struct usbd_xfer *xfer = SIMPLEQ_FIRST(&pipe->up_queue);
+		(*pipe->up_methods->upm_start)(xfer);
+	}
+
 	DPRINTFN(4, "ends", 0, 0, 0, 0);
 }
 
-static usbd_status
-xhci_clear_endpoint_stall_async(struct usbd_xfer *xfer)
+static void
+xhci_pipe_restart(struct usbd_pipe *pipe)
 {
-	struct xhci_softc * const sc = XHCI_XFER2SC(xfer);
-	struct xhci_pipe * const xp = (struct xhci_pipe *)xfer->ux_pipe;
+	struct xhci_pipe * const xp =
+	    container_of(pipe, struct xhci_pipe, xp_pipe);
 
 	XHCIHIST_FUNC();
-	XHCIHIST_CALLARGS("xfer %#jx", (uintptr_t)xfer, 0, 0, 0);
+	XHCIHIST_CALLARGS("pipe %#jx", (uintptr_t)pipe, 0, 0, 0);
 
-	if (sc->sc_dying) {
-		return USBD_IOERROR;
-	}
+	usb_add_task(pipe->up_dev, &xp->xp_async_task, USB_TASKQ_HC);
 
-	usb_init_task(&xp->xp_async_task,
-	    xhci_clear_endpoint_stall_async_task, xfer, USB_TASKQ_MPSAFE);
-	usb_add_task(xfer->ux_pipe->up_dev, &xp->xp_async_task, USB_TASKQ_HC);
 	DPRINTFN(4, "ends", 0, 0, 0, 0);
-
-	return USBD_NORMAL_COMPLETION;
 }
 
 /* Process roothub port status/change events and notify to uhub_intr. */
@@ -2466,32 +2443,9 @@ xhci_event_transfer(struct xhci_softc * const sc,
 	case XHCI_TRB_ERROR_BABBLE:
 		DPRINTFN(1, "ERR %ju slot %ju dci %ju", trbcode, slot, dci, 0);
 		xr->is_halted = true;
-		/*
-		 * Try to claim this xfer for completion.  If it has already
-		 * completed or aborted, drop it on the floor.
-		 */
-		if (!usbd_xfer_trycomplete(xfer))
-			return;
-
-		/*
-		 * Stalled endpoints can be recoverd by issuing
-		 * command TRB TYPE_RESET_EP on xHCI instead of
-		 * issuing request CLEAR_FEATURE UF_ENDPOINT_HALT
-		 * on the endpoint. However, this function may be
-		 * called from softint context (e.g. from umass),
-		 * in that case driver gets KASSERT in cv_timedwait
-		 * in xhci_do_command.
-		 * To avoid this, this runs reset_endpoint and
-		 * usb_transfer_complete in usb task thread
-		 * asynchronously (and then umass issues clear
-		 * UF_ENDPOINT_HALT).
-		 */
-
-		/* Override the status.  */
-		xfer->ux_status = USBD_STALLED;
-
-		xhci_clear_endpoint_stall_async(xfer);
-		return;
+		xhci_pipe_restart(xfer->ux_pipe);
+		err = USBD_STALLED;
+		break;
 	default:
 		DPRINTFN(1, "ERR %ju slot %ju dci %ju", trbcode, slot, dci, 0);
 		err = USBD_IOERROR;
@@ -4172,18 +4126,8 @@ xhci_roothub_ctrl(struct usbd_bus *bus, usb_device_request_t *req,
 static usbd_status
 xhci_root_intr_transfer(struct usbd_xfer *xfer)
 {
-	struct xhci_softc * const sc = XHCI_XFER2SC(xfer);
-	usbd_status err;
-
 	XHCIHIST_FUNC(); XHCIHIST_CALLED();
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
 	/* Pipe isn't running, start first */
 	return xhci_root_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
@@ -4276,18 +4220,8 @@ xhci_root_intr_done(struct usbd_xfer *xfer)
 static usbd_status
 xhci_device_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	struct xhci_softc * const sc = XHCI_XFER2SC(xfer);
-	usbd_status err;
-
 	XHCIHIST_FUNC(); XHCIHIST_CALLED();
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
 	/* Pipe isn't running, start first */
 	return xhci_device_ctrl_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
@@ -4321,6 +4255,11 @@ xhci_device_ctrl_start(struct usbd_xfer *xfer)
 
 	KASSERT((xfer->ux_rqflags & URQ_REQUEST) != 0);
 
+	if (!polling)
+		mutex_enter(&sc->sc_lock);
+	if (tr->is_halted)
+		goto out;
+
 	i = 0;
 
 	/* setup phase */
@@ -4363,11 +4302,18 @@ xhci_device_ctrl_start(struct usbd_xfer *xfer)
 	if (!polling)
 		mutex_exit(&tr->xr_lock);
 
-	if (!polling)
-		mutex_enter(&sc->sc_lock);
-	xfer->ux_status = USBD_IN_PROGRESS;
 	xhci_db_write_4(sc, XHCI_DOORBELL(xs->xs_idx), dci);
-	usbd_xfer_schedule_timeout(xfer);
+
+out:	if (xfer->ux_status == USBD_NOT_STARTED) {
+		usbd_xfer_schedule_timeout(xfer);
+		xfer->ux_status = USBD_IN_PROGRESS;
+	} else {
+		/*
+		 * We must be coming from xhci_pipe_restart -- timeout
+		 * already set up, nothing to do.
+		 */
+	}
+	KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
 	if (!polling)
 		mutex_exit(&sc->sc_lock);
 
@@ -4409,18 +4355,8 @@ xhci_device_ctrl_close(struct usbd_pipe *pipe)
 static usbd_status
 xhci_device_isoc_transfer(struct usbd_xfer *xfer)
 {
-	struct xhci_softc * const sc = XHCI_XFER2SC(xfer);
-	usbd_status err;
-
 	XHCIHIST_FUNC(); XHCIHIST_CALLED();
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
 	return xhci_device_isoc_enter(xfer);
 }
 
@@ -4573,22 +4509,9 @@ xhci_device_isoc_done(struct usbd_xfer *xfer)
 static usbd_status
 xhci_device_bulk_transfer(struct usbd_xfer *xfer)
 {
-	struct xhci_softc * const sc = XHCI_XFER2SC(xfer);
-	usbd_status err;
-
 	XHCIHIST_FUNC(); XHCIHIST_CALLED();
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
-	/*
-	 * Pipe isn't running (otherwise err would be USBD_INPROG),
-	 * so start it first.
-	 */
+	/* Pipe isn't running, so start it first.  */
 	return xhci_device_bulk_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -4617,6 +4540,11 @@ xhci_device_bulk_start(struct usbd_xfer *xfer)
 
 	KASSERT((xfer->ux_rqflags & URQ_REQUEST) == 0);
 
+	if (!polling)
+		mutex_enter(&sc->sc_lock);
+	if (tr->is_halted)
+		goto out;
+
 	parameter = DMAADDR(dma, 0);
 	const bool isread = usbd_xfer_isread(xfer);
 	if (len)
@@ -4649,11 +4577,18 @@ xhci_device_bulk_start(struct usbd_xfer *xfer)
 	if (!polling)
 		mutex_exit(&tr->xr_lock);
 
-	if (!polling)
-		mutex_enter(&sc->sc_lock);
-	xfer->ux_status = USBD_IN_PROGRESS;
 	xhci_db_write_4(sc, XHCI_DOORBELL(xs->xs_idx), dci);
-	usbd_xfer_schedule_timeout(xfer);
+
+out:	if (xfer->ux_status == USBD_NOT_STARTED) {
+		xfer->ux_status = USBD_IN_PROGRESS;
+		usbd_xfer_schedule_timeout(xfer);
+	} else {
+		/*
+		 * We must be coming from xhci_pipe_restart -- timeout
+		 * already set up, nothing to do.
+		 */
+	}
+	KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
 	if (!polling)
 		mutex_exit(&sc->sc_lock);
 
@@ -4699,22 +4634,9 @@ xhci_device_bulk_close(struct usbd_pipe *pipe)
 static usbd_status
 xhci_device_intr_transfer(struct usbd_xfer *xfer)
 {
-	struct xhci_softc * const sc = XHCI_XFER2SC(xfer);
-	usbd_status err;
-
 	XHCIHIST_FUNC(); XHCIHIST_CALLED();
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
-	/*
-	 * Pipe isn't running (otherwise err would be USBD_INPROG),
-	 * so start it first.
-	 */
+	/* Pipe isn't running, so start it first.  */
 	return xhci_device_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
 
@@ -4741,6 +4663,11 @@ xhci_device_intr_start(struct usbd_xfer *xfer)
 	if (sc->sc_dying)
 		return USBD_IOERROR;
 
+	if (!polling)
+		mutex_enter(&sc->sc_lock);
+	if (tr->is_halted)
+		goto out;
+
 	KASSERT((xfer->ux_rqflags & URQ_REQUEST) == 0);
 
 	const bool isread = usbd_xfer_isread(xfer);
@@ -4763,11 +4690,18 @@ xhci_device_intr_start(struct usbd_xfer *xfer)
 	if (!polling)
 		mutex_exit(&tr->xr_lock);
 
-	if (!polling)
-		mutex_enter(&sc->sc_lock);
-	xfer->ux_status = USBD_IN_PROGRESS;
 	xhci_db_write_4(sc, XHCI_DOORBELL(xs->xs_idx), dci);
-	usbd_xfer_schedule_timeout(xfer);
+
+out:	if (xfer->ux_status == USBD_NOT_STARTED) {
+		xfer->ux_status = USBD_IN_PROGRESS;
+		usbd_xfer_schedule_timeout(xfer);
+	} else {
+		/*
+		 * We must be coming from xhci_pipe_restart -- timeout
+		 * already set up, nothing to do.
+		 */
+	}
+	KASSERT(xfer->ux_status == USBD_IN_PROGRESS);
 	if (!polling)
 		mutex_exit(&sc->sc_lock);
 
diff --git a/sys/dev/usb/xhcivar.h b/sys/dev/usb/xhcivar.h
index 9893037be2e1..b253da58557f 100644
--- a/sys/dev/usb/xhcivar.h
+++ b/sys/dev/usb/xhcivar.h
@@ -29,8 +29,17 @@
 #ifndef _DEV_USB_XHCIVAR_H_
 #define _DEV_USB_XHCIVAR_H_
 
+#include <sys/types.h>
+
+#include <sys/condvar.h>
+#include <sys/device.h>
+#include <sys/mutex.h>
+#include <sys/pmf.h>
 #include <sys/pool.h>
 
+#include <dev/usb/usbdi.h>
+#include <dev/usb/usbdivar.h>
+
 #define XHCI_MAX_DCI	31
 
 struct xhci_soft_trb {
diff --git a/sys/external/bsd/dwc2/dwc2.c b/sys/external/bsd/dwc2/dwc2.c
index e3e61a072b23..19ffae0e8ab4 100644
--- a/sys/external/bsd/dwc2/dwc2.c
+++ b/sys/external/bsd/dwc2/dwc2.c
@@ -515,7 +515,7 @@ dwc2_abortx(struct usbd_xfer *xfer)
 	}
 
 	/*
-	 * HC Step 1: Handle the hardware.
+	 * Handle the hardware.
 	 */
 	err = dwc2_hcd_urb_dequeue(hsotg, dxfer->urb);
 	if (err) {
@@ -524,11 +524,6 @@ dwc2_abortx(struct usbd_xfer *xfer)
 
 dying:
 	mutex_spin_exit(&hsotg->lock);
-
-	/*
-	 * Final Step: Notify completion to waiting xfers.
-	 */
-	usb_transfer_complete(xfer);
 	KASSERT(mutex_owned(&sc->sc_lock));
 }
 
@@ -614,18 +609,9 @@ dwc2_roothub_ctrl(struct usbd_bus *bus, usb_device_request_t *req,
 Static usbd_status
 dwc2_root_intr_transfer(struct usbd_xfer *xfer)
 {
-	struct dwc2_softc *sc = DWC2_XFER2SC(xfer);
-	usbd_status err;
 
 	DPRINTF("\n");
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
 	/* Pipe isn't running, start first */
 	return dwc2_root_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
@@ -711,18 +697,9 @@ dwc2_root_intr_done(struct usbd_xfer *xfer)
 Static usbd_status
 dwc2_device_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	struct dwc2_softc *sc = DWC2_XFER2SC(xfer);
-	usbd_status err;
 
 	DPRINTF("\n");
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
 	/* Pipe isn't running, start first */
 	return dwc2_device_ctrl_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
@@ -788,12 +765,8 @@ dwc2_device_bulk_transfer(struct usbd_xfer *xfer)
 
 	DPRINTF("xfer=%p\n", xfer);
 
-	/* Insert last in queue. */
 	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-
-	KASSERT(err == USBD_NORMAL_COMPLETION);
-
+	KASSERT(xfer->ux_status == USBD_NOT_STARTED);
 	xfer->ux_status = USBD_IN_PROGRESS;
 	err = dwc2_device_start(xfer);
 	mutex_exit(&sc->sc_lock);
@@ -833,18 +806,9 @@ dwc2_device_bulk_done(struct usbd_xfer *xfer)
 Static usbd_status
 dwc2_device_intr_transfer(struct usbd_xfer *xfer)
 {
-	struct dwc2_softc *sc = DWC2_XFER2SC(xfer);
-	usbd_status err;
 
 	DPRINTF("xfer=%p\n", xfer);
 
-	/* Insert last in queue. */
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
-
 	/* Pipe isn't running, start first */
 	return dwc2_device_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
@@ -909,12 +873,8 @@ dwc2_device_isoc_transfer(struct usbd_xfer *xfer)
 
 	DPRINTF("xfer=%p\n", xfer);
 
-	/* Insert last in queue. */
 	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-
-	KASSERT(err == USBD_NORMAL_COMPLETION);
-
+	KASSERT(xfer->ux_status == USBD_NOT_STARTED);
 	xfer->ux_status = USBD_IN_PROGRESS;
 	err = dwc2_device_start(xfer);
 	mutex_exit(&sc->sc_lock);
diff --git a/sys/rump/dev/lib/libugenhc/ugenhc.c b/sys/rump/dev/lib/libugenhc/ugenhc.c
index 003f1cc54453..f6f80877940e 100644
--- a/sys/rump/dev/lib/libugenhc/ugenhc.c
+++ b/sys/rump/dev/lib/libugenhc/ugenhc.c
@@ -401,14 +401,6 @@ rumpusb_device_ctrl_start(struct usbd_xfer *xfer)
 static usbd_status
 rumpusb_device_ctrl_transfer(struct usbd_xfer *xfer)
 {
-	struct ugenhc_softc *sc = UGENHC_XFER2SC(xfer);
-	usbd_status err;
-
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
 	return rumpusb_device_ctrl_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
@@ -540,14 +532,6 @@ rumpusb_root_intr_start(struct usbd_xfer *xfer)
 static usbd_status
 rumpusb_root_intr_transfer(struct usbd_xfer *xfer)
 {
-	struct ugenhc_softc *sc = UGENHC_XFER2SC(xfer);
-	usbd_status err;
-
-	mutex_enter(&sc->sc_lock);
-	err = usb_insert_transfer(xfer);
-	mutex_exit(&sc->sc_lock);
-	if (err)
-		return err;
 
 	return rumpusb_root_intr_start(SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 }
@@ -709,8 +693,6 @@ doxfer_kth(void *arg)
 static usbd_status
 rumpusb_device_bulk_transfer(struct usbd_xfer *xfer)
 {
-	struct ugenhc_softc *sc = UGENHC_XFER2SC(xfer);
-	usbd_status err;
 
 	if (!rump_threads) {
 		/* XXX: lie about supporting async transfers */
@@ -720,20 +702,9 @@ rumpusb_device_bulk_transfer(struct usbd_xfer *xfer)
 			return USBD_IN_PROGRESS;
 		}
 
-		mutex_enter(&sc->sc_lock);
-		err = usb_insert_transfer(xfer);
-		mutex_exit(&sc->sc_lock);
-		if (err)
-			return err;
-
 		return rumpusb_device_bulk_start(
 		    SIMPLEQ_FIRST(&xfer->ux_pipe->up_queue));
 	} else {
-		mutex_enter(&sc->sc_lock);
-		err = usb_insert_transfer(xfer);
-		mutex_exit(&sc->sc_lock);
-		if (err)
-			return err;
 		kthread_create(PRI_NONE, 0, NULL, doxfer_kth, xfer->ux_pipe, NULL,
 		    "rusbhcxf");
 


Home | Main Index | Thread Index | Old Index