Source-Changes-HG archive

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

[src/trunk]: src/sys Factor out HCI-independent xfer completion logic.



details:   https://anonhg.NetBSD.org/src/rev/2ca8db2c11cf
branches:  trunk
changeset: 744767:2ca8db2c11cf
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Wed Feb 12 16:01:00 2020 +0000

description:
Factor out HCI-independent xfer completion logic.

New API for HCI drivers to synchronize hardware completion
interrupts, synchronous aborts, and asynchronous timeouts:

- When submitting an xfer to hardware, call
  usbd_xfer_schedule_timeout(xfer).

- On HCI completion interrupt for xfer completion:

        if (!usbd_xfer_trycomplete(xfer))
                return;         /* timed out or aborted, ignore it */

- In upm_abort methods, call usbd_xfer_abort(xfer).

For HCI drivers that use this API (not needed in drivers that don't,
or for xfers like root intr xfers that don't use it):

- New ubm_abortx method serves role of former *hci_abort_xfer, but
  without any logic for wrangling timeouts/callouts/tasks -- caller
  in usbd_xfer_abort has already handled them.

- New ubm_dying method, returns true if the device is in the process
  of detaching, used by the timeout logic.

Converted and tested:
- ehci
- ohci

Converted and compile-tested:
- ahci (XXX did this ever work?)
- dwc2
- motg (XXX missing usbd_xfer_schedule_timeout in motg_*_start?)
- uhci
- xhci

Not changed:

- slhci (sys/dev/ic/sl811hs.c) -- doesn't use a separate per-xfer
  callout for timeouts (XXX but maybe should?)

- ugenhc (sys/rump/dev/lib/libugenhc/ugenhc.c) -- doesn't manage its
  own transfer timeouts

- vhci -- times transfers out only on detach; could be adapted easily
  if we wanted to use the xfer->ux_callout

diffstat:

 sys/arch/mips/adm5120/dev/ahci.c |   25 +-
 sys/dev/usb/ehci.c               |  393 ++------------------------------------
 sys/dev/usb/motg.c               |   70 +++--
 sys/dev/usb/ohci.c               |  160 +++------------
 sys/dev/usb/uhci.c               |  148 ++-----------
 sys/dev/usb/usbdi.c              |  389 ++++++++++++++++++++++++++++++++++++++-
 sys/dev/usb/usbdi.h              |    7 +-
 sys/dev/usb/usbdivar.h           |    4 +-
 sys/dev/usb/xhci.c               |  167 +++-------------
 sys/external/bsd/dwc2/dwc2.c     |  239 +++++++++--------------
 sys/external/bsd/dwc2/dwc2var.h  |    3 +-
 11 files changed, 673 insertions(+), 932 deletions(-)

diffs (truncated from 2566 to 300 lines):

diff -r bf423600bdcb -r 2ca8db2c11cf sys/arch/mips/adm5120/dev/ahci.c
--- a/sys/arch/mips/adm5120/dev/ahci.c  Wed Feb 12 16:00:34 2020 +0000
+++ b/sys/arch/mips/adm5120/dev/ahci.c  Wed Feb 12 16:01:00 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ahci.c,v 1.17 2019/02/17 04:17:52 rin Exp $    */
+/*     $NetBSD: ahci.c,v 1.18 2020/02/12 16:01:00 riastradh Exp $      */
 
 /*-
  * Copyright (c) 2007 Ruslan Ermilov and Vsevolod Lobko.
@@ -64,7 +64,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ahci.c,v 1.17 2019/02/17 04:17:52 rin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ahci.c,v 1.18 2020/02/12 16:01:00 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -98,6 +98,7 @@
 static struct usbd_xfer *
                        ahci_allocx(struct usbd_bus *, unsigned int);
 static void            ahci_freex(struct usbd_bus *, struct usbd_xfer *);
+static void            ahci_abortx(struct usbd_xfer *);
 
 static void            ahci_get_lock(struct usbd_bus *, kmutex_t **);
 static int             ahci_roothub_ctrl(struct usbd_bus *, usb_device_request_t *,
@@ -136,7 +137,6 @@
 static int             ahci_transaction(struct ahci_softc *,
        struct usbd_pipe *, uint8_t, int, u_char *, uint8_t);
 static void            ahci_noop(struct usbd_pipe *);
-static void            ahci_abort_xfer(struct usbd_xfer *, usbd_status);
 static void            ahci_device_clear_toggle(struct usbd_pipe *);
 
 extern int usbdebug;
@@ -169,6 +169,7 @@
        .ubm_dopoll = ahci_poll,
        .ubm_allocx = ahci_allocx,
        .ubm_freex = ahci_freex,
+       .ubm_abortx = ahci_abortx,
        .ubm_getlock = ahci_get_lock,
        .ubm_rhctrl = ahci_roothub_ctrl,
 };
@@ -919,7 +920,7 @@
 ahci_device_ctrl_abort(struct usbd_xfer *xfer)
 {
        DPRINTF(D_TRACE, ("Cab "));
-       ahci_abort_xfer(xfer, USBD_CANCELLED);
+       usbd_xfer_abort(xfer);
 }
 
 static void
@@ -1031,7 +1032,7 @@
        } else {
                printf("%s: sx == NULL!\n", __func__);
        }
-       ahci_abort_xfer(xfer, USBD_CANCELLED);
+       usbd_xfer_abort(xfer);
 }
 
 static void
@@ -1247,7 +1248,7 @@
 ahci_device_bulk_abort(struct usbd_xfer *xfer)
 {
        DPRINTF(D_TRACE, ("Bab "));
-       ahci_abort_xfer(xfer, USBD_CANCELLED);
+       usbd_xfer_abort(xfer);
 }
 
 static void
@@ -1377,11 +1378,15 @@
 #endif
 }
 
-void
-ahci_abort_xfer(struct usbd_xfer *xfer, usbd_status status)
+static void
+ahci_abortx(struct usbd_xfer *xfer)
 {
-       xfer->ux_status = status;
-       usb_transfer_complete(xfer);
+       /*
+        * XXX This is totally busted; there's no way it can possibly
+        * work!  All transfers are busy-waited, it seems, so there is
+        * no opportunity to abort.
+        */
+       KASSERT(xfer->ux_status != USBD_IN_PROGRESS);
 }
 
 void
diff -r bf423600bdcb -r 2ca8db2c11cf sys/dev/usb/ehci.c
--- a/sys/dev/usb/ehci.c        Wed Feb 12 16:00:34 2020 +0000
+++ b/sys/dev/usb/ehci.c        Wed Feb 12 16:01:00 2020 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: ehci.c,v 1.270 2020/02/12 16:00:34 riastradh Exp $ */
+/*     $NetBSD: ehci.c,v 1.271 2020/02/12 16:01:00 riastradh Exp $ */
 
 /*
  * Copyright (c) 2004-2012 The NetBSD Foundation, Inc.
@@ -53,7 +53,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ehci.c,v 1.270 2020/02/12 16:00:34 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ehci.c,v 1.271 2020/02/12 16:01:00 riastradh Exp $");
 
 #include "ohci.h"
 #include "uhci.h"
@@ -166,11 +166,6 @@
 Static void            ehci_check_sitd_intr(ehci_softc_t *, struct ehci_xfer *,
                            ex_completeq_t *);
 Static void            ehci_idone(struct ehci_xfer *, ex_completeq_t *);
-Static void            ehci_timeout(void *);
-Static void            ehci_timeout_task(void *);
-Static bool            ehci_probe_timeout(struct usbd_xfer *);
-Static void            ehci_schedule_timeout(struct usbd_xfer *);
-Static void            ehci_cancel_timeout_async(struct usbd_xfer *);
 Static void            ehci_intrlist_timeout(void *);
 Static void            ehci_doorbell(void *);
 Static void            ehci_pcd(void *);
@@ -180,6 +175,7 @@
 Static void            ehci_freex(struct usbd_bus *, struct usbd_xfer *);
 
 Static void            ehci_get_lock(struct usbd_bus *, kmutex_t **);
+Static bool            ehci_dying(struct usbd_bus *);
 Static int             ehci_roothub_ctrl(struct usbd_bus *,
                            usb_device_request_t *, void *, int);
 
@@ -281,7 +277,7 @@
 Static void            ehci_sync_hc(ehci_softc_t *);
 
 Static void            ehci_close_pipe(struct usbd_pipe *, ehci_soft_qh_t *);
-Static void            ehci_abort_xfer(struct usbd_xfer *, usbd_status);
+Static void            ehci_abortx(struct usbd_xfer *);
 
 #ifdef EHCI_DEBUG
 Static ehci_softc_t    *theehci;
@@ -322,6 +318,8 @@
        .ubm_dopoll =   ehci_poll,
        .ubm_allocx =   ehci_allocx,
        .ubm_freex =    ehci_freex,
+       .ubm_abortx =   ehci_abortx,
+       .ubm_dying =    ehci_dying,
        .ubm_getlock =  ehci_get_lock,
        .ubm_rhctrl =   ehci_roothub_ctrl,
 };
@@ -1049,22 +1047,11 @@
        DPRINTF("ex=%#jx", (uintptr_t)ex, 0, 0, 0);
 
        /*
-        * If software has completed it, either by cancellation
-        * or timeout, drop it on the floor.
+        * Try to claim this xfer for completion.  If it has already
+        * completed or aborted, drop it on the floor.
         */
-       if (xfer->ux_status != USBD_IN_PROGRESS) {
-               KASSERT(xfer->ux_status == USBD_CANCELLED ||
-                   xfer->ux_status == USBD_TIMEOUT);
-               DPRINTF("aborted xfer=%#jx", (uintptr_t)xfer, 0, 0, 0);
+       if (!usbd_xfer_trycomplete(xfer))
                return;
-       }
-
-       /*
-        * We are completing the xfer.  Cancel the timeout if we can,
-        * but only asynchronously.  See ehci_cancel_timeout_async for
-        * why we need not wait for the callout or task here.
-        */
-       ehci_cancel_timeout_async(xfer);
 
 #ifdef DIAGNOSTIC
 #ifdef EHCI_DEBUG
@@ -1539,9 +1526,6 @@
        if (xfer != NULL) {
                memset(xfer, 0, sizeof(struct ehci_xfer));
 
-               /* Initialise this always so we can call remove on it. */
-               usb_init_task(&xfer->ux_aborttask, ehci_timeout_task, xfer,
-                   USB_TASKQ_MPSAFE);
 #ifdef DIAGNOSTIC
                struct ehci_xfer *ex = EHCI_XFER2EXFER(xfer);
                ex->ex_isdone = true;
@@ -1569,6 +1553,14 @@
        pool_cache_put(sc->sc_xferpool, xfer);
 }
 
+Static bool
+ehci_dying(struct usbd_bus *bus)
+{
+       struct ehci_softc *sc = EHCI_BUS2SC(bus);
+
+       return sc->sc_dying;
+}
+
 Static void
 ehci_get_lock(struct usbd_bus *bus, kmutex_t **lock)
 {
@@ -3201,22 +3193,12 @@
 }
 
 /*
- * Cancel or timeout a device request.  We have two cases to deal with
- *
- * 1) A driver wants to stop scheduled or inflight transfers
- * 2) A transfer has timed out
- *
- * have (partially) happened since the hardware runs concurrently.
- *
- * Transfer state is protected by the bus lock and we set the transfer status
- * as soon as either of the above happens (with bus lock held).
- *
- * Then we arrange for the hardware to tells us that it is not still
+ * Arrrange for the hardware to tells us that it is not still
  * processing the TDs by setting the QH halted bit and wait for the ehci
  * door bell
  */
 Static void
-ehci_abort_xfer(struct usbd_xfer *xfer, usbd_status status)
+ehci_abortx(struct usbd_xfer *xfer)
 {
        EHCIHIST_FUNC(); EHCIHIST_CALLED();
        struct ehci_pipe *epipe = EHCI_XFER2EPIPE(xfer);
@@ -3228,48 +3210,14 @@
        uint32_t qhstatus;
        int hit;
 
-       KASSERTMSG((status == USBD_CANCELLED || status == USBD_TIMEOUT),
-           "invalid status for abort: %d", (int)status);
-
        DPRINTF("xfer=%#jx pipe=%#jx", (uintptr_t)xfer, (uintptr_t)epipe, 0, 0);
 
        KASSERT(mutex_owned(&sc->sc_lock));
        ASSERT_SLEEPABLE();
 
-       /*
-        * Nobody else can set this status: only one caller can time
-        * out, and only one caller can synchronously abort.  So the
-        * status can't be the status we're trying to set this to.
-        */
-       KASSERT(xfer->ux_status != status);
-
-       /*
-        * If host controller or timer interrupt has completed it, too
-        * late to abort.  Forget about it.
-        */
-       if (xfer->ux_status != USBD_IN_PROGRESS)
-               return;
-
-       if (status == USBD_CANCELLED) {
-               /*
-                * We are synchronously aborting.  Cancel the timeout
-                * if we can, but only asynchronously.  See
-                * ehci_cancel_timeout_async for why we need not wait
-                * for the callout or task here.
-                */
-               ehci_cancel_timeout_async(xfer);
-       } else {
-               /*
-                * Otherwise, we are timing out.  No action needed to
-                * cancel the timeout because we _are_ the timeout.
-                * This case should happen only via the timeout task,
-                * invoked via the callout.
-                */
-               KASSERT(status == USBD_TIMEOUT);
-       }
-
-       /* We beat everyone else.  Claim the status.  */
-       xfer->ux_status = status;
+       KASSERTMSG((xfer->ux_status == USBD_CANCELLED ||
+               xfer->ux_status == USBD_TIMEOUT),
+           "bad abort status: %d", xfer->ux_status);
 
        /*
         * If we're dying, skip the hardware action and just notify the
@@ -3477,291 +3425,6 @@
        KASSERT(mutex_owned(&sc->sc_lock));
 }
 
-/*
- * ehci_timeout(xfer)
- *
- *     Called at IPL_SOFTCLOCK when too much time has elapsed waiting
- *     for xfer to complete.  Since we can't abort the xfer at
- *     IPL_SOFTCLOCK, defer to a usb_task to run it in thread context,
- *     unless the xfer has completed or aborted concurrently -- and if
- *     the xfer has also been resubmitted, take care of rescheduling
- *     the callout.
- */
-Static void
-ehci_timeout(void *addr)
-{
-       EHCIHIST_FUNC(); EHCIHIST_CALLED();
-       struct usbd_xfer *xfer = addr;
-       ehci_softc_t *sc = EHCI_XFER2SC(xfer);
-       struct usbd_device *dev = xfer->ux_pipe->up_dev;
-
-       DPRINTF("xfer %#jx", (uintptr_t)xfer, 0, 0, 0);
-#ifdef EHCI_DEBUG
-       if (ehcidebug >= 2) {
-               struct usbd_pipe *pipe = xfer->ux_pipe;
-               usbd_dump_pipe(pipe);



Home | Main Index | Thread Index | Old Index