Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/netbsd-8]: src/sys Pull up following revision(s) (requested by mrg in ti...
details: https://anonhg.NetBSD.org/src/rev/70aaf0269de7
branches: netbsd-8
changeset: 435204:70aaf0269de7
user: martin <martin%NetBSD.org@localhost>
date: Sat Aug 25 11:29:52 2018 +0000
description:
Pull up following revision(s) (requested by mrg in ticket #980):
sys/dev/usb/usbdivar.h: revision 1.117
sys/external/bsd/dwc2/dwc2.c: revision 1.52
sys/dev/usb/xhcivar.h: revision 1.10
sys/dev/usb/motg.c: revision 1.22
sys/dev/usb/ehci.c: revision 1.260
sys/dev/usb/ehci.c: revision 1.261
sys/dev/usb/xhci.c: revision 1.96
sys/dev/usb/ohci.c: revision 1.282
sys/dev/usb/ohci.c: revision 1.283
sys/dev/usb/ehcivar.h: revision 1.45
sys/dev/usb/uhci.c: revision 1.281
sys/dev/usb/uhci.c: revision 1.282
sys/dev/usb/usbdi.c: revision 1.177
sys/dev/usb/ohcivar.h: revision 1.60
sys/dev/usb/uhcivar.h: revision 1.55
(all via patch)
pull across abort fixes from nick-nhusb. add more abort fixes, using
ideas from Taylor and Nick, and myself. special thanks to both who
inspired much of the code here, if not wrote it directly.
among other problems, this assert should no longer trigger:
panic: kernel diagnostic assertion "xfer->ux_state == XFER_ONQU" failed: file "/current/src/sys/dev/usb/usbdi.c", line 914
using usbhist i was able to track down my instance of it being related
to userland close() beginning, dropping the sc_lock, and then the usb
softintr completes the transfer normally, and when it is done, the
abort path attempts to re-complete the transfer, and the above assert
is tripped.
changes from nhusb were commited with these logs:
--
Move the struct usb_task to struct usbd_xfer for everyone to use.
--
Set device transfer status to USBD_IN_PROGRESS if start methods succeeds
--
Actually set the transfer status on transfers in ohci_abort_xfer and
the controller is dying
--
Don't supply the lock to callout_halt when polling as it won't be held
--
Improve transfer abort
--
Mark device transfers as USBD_IN_PROGRESS appropriately and improve
abort handling
--
--
Mark device transfers as USBD_IN_PROGRESS appropriately and improve
abort handling
--
additional changes include:
- initialise the usb abort task in the HCI allocx routine, so that it
can be safely usb_rem_task()'d.
- rework the handling of softintr vs cancellation vs timeout abort based
upon a scheme from Taylor:
when completing a transfer normally:
- if the status is not in progress, it must be cancelled or timed out,
and we should not process this xfer.
- set the status as normal.
- unconditionallly callout_stop() and usb_rem_task(). they're safe and
either aren't running, or will run and do nothing.
- finally call usb_transfer_complete().
when aborting a transfer:
- status should be cancelled or timed out.
- if cancelling, callout_halt and usb_rem_task_wait() to make sure the
timer is either done or cancelled.
- at this point, the ux_status must not be cancelled or timed out, and
if it is not in progress we're done.
- set the status.
- if the controller is dying, just return.
- perform HCI-specific tasks to abort this xfer.
- finally call usb_transfer_complete().
for the timeout and timeout task:
- if the HCI is not dying, and the ux_status is in progress, then
trigger the usb abort task.
- remove UXFER_ABORTWAIT and UXFER_ABORTING.
tested on:
- multiple PC systems with several types of devices: ugen/UPS, ucom,
umass with disk, ssd and cdrom backends, kbd, ms, using uhci, ehci
and xhci.
- erlite3: sd@umass on dwc2.
- sunblade2000: kbd/ms and umass disk on ohci.
untested:
- motg, slhci and ahci. motg has some portion of the new scheme
applied, but slhci and ahci require more study.
future work includes pushing a lot of the common abort handling into
usbdi.c and leaving upm_abort() for HC specific tasks, but this change
is pullup-able to netbsd-7 and netbsd-8 as it does not change any
external API, as well as removing over 100 lines of code while adding
over 30 new asserts.
XXX: pullup-7, pullup-8.
fix DIAGNOSTIC build by not copying ub_usepolling to stack before use
Sprinkle __diagused
diffstat:
sys/dev/usb/ehci.c | 258 +++++++++++++++++++++---------------------
sys/dev/usb/ehcivar.h | 5 +-
sys/dev/usb/motg.c | 51 ++++++--
sys/dev/usb/ohci.c | 216 +++++++++++++++++++++--------------
sys/dev/usb/ohcivar.h | 6 +-
sys/dev/usb/uhci.c | 179 ++++++++++++++++-------------
sys/dev/usb/uhcivar.h | 6 +-
sys/dev/usb/usbdi.c | 12 +-
sys/dev/usb/usbdivar.h | 7 +-
sys/dev/usb/xhci.c | 159 ++++++++++++++++----------
sys/dev/usb/xhcivar.h | 4 +-
sys/external/bsd/dwc2/dwc2.c | 128 +++++++++++++--------
12 files changed, 576 insertions(+), 455 deletions(-)
diffs (truncated from 1940 to 300 lines):
diff -r 08cef02eb0d9 -r 70aaf0269de7 sys/dev/usb/ehci.c
--- a/sys/dev/usb/ehci.c Sat Aug 25 11:21:53 2018 +0000
+++ b/sys/dev/usb/ehci.c Sat Aug 25 11:29:52 2018 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ehci.c,v 1.254.8.3 2017/11/23 13:29:32 martin Exp $ */
+/* $NetBSD: ehci.c,v 1.254.8.4 2018/08/25 11:29:52 martin Exp $ */
/*
* Copyright (c) 2004-2012 The NetBSD Foundation, Inc.
@@ -53,7 +53,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ehci.c,v 1.254.8.3 2017/11/23 13:29:32 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ehci.c,v 1.254.8.4 2018/08/25 11:29:52 martin Exp $");
#include "ohci.h"
#include "uhci.h"
@@ -412,8 +412,7 @@
mutex_init(&sc->sc_lock, MUTEX_DEFAULT, IPL_SOFTUSB);
mutex_init(&sc->sc_intr_lock, MUTEX_DEFAULT, IPL_USB);
- cv_init(&sc->sc_softwake_cv, "ehciab");
- cv_init(&sc->sc_doorbell, "ehcidi");
+ cv_init(&sc->sc_doorbell, "ehcidb");
sc->sc_xferpool = pool_cache_init(sizeof(struct ehci_xfer), 0, 0, 0,
"ehcixfer", NULL, IPL_USB, NULL, NULL, NULL);
@@ -757,6 +756,7 @@
ehci_doorbell(void *addr)
{
ehci_softc_t *sc = addr;
+ EHCIHIST_FUNC(); EHCIHIST_CALLED();
mutex_enter(&sc->sc_lock);
cv_broadcast(&sc->sc_doorbell);
@@ -859,11 +859,6 @@
!TAILQ_EMPTY(&sc->sc_intrhead))
callout_reset(&sc->sc_tmo_intrlist,
hz, ehci_intrlist_timeout, sc);
-
- if (sc->sc_softwake) {
- sc->sc_softwake = 0;
- cv_broadcast(&sc->sc_softwake_cv);
- }
}
Static void
@@ -945,7 +940,6 @@
}
done:
DPRINTFN(10, "ex=%#jx done", (uintptr_t)ex, 0, 0, 0);
- callout_stop(&ex->ex_xfer.ux_callout);
ehci_idone(ex, cq);
}
@@ -992,7 +986,6 @@
return;
done:
DPRINTF("ex %#jx done", (uintptr_t)ex, 0, 0, 0);
- callout_stop(&ex->ex_xfer.ux_callout);
ehci_idone(ex, cq);
}
@@ -1030,7 +1023,6 @@
return;
DPRINTFN(10, "ex=%#jx done", (uintptr_t)ex, 0, 0, 0);
- callout_stop(&(ex->ex_xfer.ux_callout));
ehci_idone(ex, cq);
}
@@ -1038,6 +1030,7 @@
Static void
ehci_idone(struct ehci_xfer *ex, ex_completeq_t *cq)
{
+ EHCIHIST_FUNC(); EHCIHIST_CALLED();
struct usbd_xfer *xfer = &ex->ex_xfer;
struct ehci_pipe *epipe = EHCI_XFER2EPIPE(xfer);
struct ehci_softc *sc = EHCI_XFER2SC(xfer);
@@ -1045,18 +1038,30 @@
uint32_t status = 0, nstatus = 0;
int actlen = 0;
- EHCIHIST_FUNC(); EHCIHIST_CALLED();
-
KASSERT(sc->sc_bus.ub_usepolling || mutex_owned(&sc->sc_lock));
DPRINTF("ex=%#jx", (uintptr_t)ex, 0, 0, 0);
- if (xfer->ux_status == USBD_CANCELLED ||
- xfer->ux_status == USBD_TIMEOUT) {
+ /*
+ * If software has completed it, either by cancellation
+ * or timeout, drop it on the floor.
+ */
+ if (xfer->ux_status != USBD_IN_PROGRESS) {
+ KASSERT(xfer->ux_status == USBD_CANCELLED ||
+ xfer->ux_status == USBD_TIMEOUT);
DPRINTF("aborted xfer=%#jx", (uintptr_t)xfer, 0, 0, 0);
return;
}
+ /*
+ * Cancel the timeout and the task, which have not yet
+ * run. If they have already fired, at worst they are
+ * waiting for the lock. They will see that the xfer
+ * is no longer in progress and give up.
+ */
+ callout_stop(&xfer->ux_callout);
+ usb_rem_task(xfer->ux_pipe->up_dev, &xfer->ux_aborttask);
+
#ifdef DIAGNOSTIC
#ifdef EHCI_DEBUG
if (ex->ex_isdone) {
@@ -1340,7 +1345,6 @@
kmem_free(sc->sc_softitds,
sc->sc_flsize * sizeof(ehci_soft_itd_t *));
cv_destroy(&sc->sc_doorbell);
- cv_destroy(&sc->sc_softwake_cv);
#if 0
/* XXX destroyed in ehci_pci.c as it controls ehci_intr access */
@@ -1519,6 +1523,10 @@
xfer = pool_cache_get(sc->sc_xferpool, PR_WAITOK);
if (xfer != NULL) {
memset(xfer, 0, sizeof(struct ehci_xfer));
+
+ /* Initialise this always so we can call remove on it. */
+ usb_init_task(&xfer->ux_aborttask, ehci_timeout_task, xfer,
+ USB_TASKQ_MPSAFE);
#ifdef DIAGNOSTIC
struct ehci_xfer *ex = EHCI_XFER2EXFER(xfer);
ex->ex_isdone = true;
@@ -2172,6 +2180,7 @@
DPRINTF("dying", 0, 0, 0, 0);
return;
}
+
/* ask for doorbell */
EOWRITE4(sc, EHCI_USBCMD, EOREAD4(sc, EHCI_USBCMD) | EHCI_CMD_IAAD);
DPRINTF("cmd = 0x%08jx sts = 0x%08jx",
@@ -3116,20 +3125,24 @@
}
/*
- * Abort a device request.
- * If this routine is called at splusb() it guarantees that the request
- * will be removed from the hardware scheduling and that the callback
- * for it will be called with USBD_CANCELLED status.
- * It's impossible to guarantee that the requested transfer will not
- * have happened since the hardware runs concurrently.
- * If the transaction has already happened we rely on the ordinary
- * interrupt processing to process it.
- * XXX This is most probably wrong.
- * XXXMRG this doesn't make sense anymore.
+ * Cancel or timeout a device request. We have two cases to deal with
+ *
+ * 1) A driver wants to stop scheduled or inflight transfers
+ * 2) A transfer has timed out
+ *
+ * have (partially) happened since the hardware runs concurrently.
+ *
+ * Transfer state is protected by the bus lock and we set the transfer status
+ * as soon as either of the above happens (with bus lock held).
+ *
+ * Then we arrange for the hardware to tells us that it is not still
+ * processing the TDs by setting the QH halted bit and wait for the ehci
+ * door bell
*/
Static void
ehci_abort_xfer(struct usbd_xfer *xfer, usbd_status status)
{
+ EHCIHIST_FUNC(); EHCIHIST_CALLED();
struct ehci_pipe *epipe = EHCI_XFER2EPIPE(xfer);
struct ehci_xfer *exfer = EHCI_XFER2EXFER(xfer);
ehci_softc_t *sc = EHCI_XFER2SC(xfer);
@@ -3138,48 +3151,58 @@
ehci_physaddr_t cur;
uint32_t qhstatus;
int hit;
- int wake;
-
- EHCIHIST_FUNC(); EHCIHIST_CALLED();
+
+ KASSERTMSG((status == USBD_CANCELLED || status == USBD_TIMEOUT),
+ "invalid status for abort: %d", (int)status);
DPRINTF("xfer=%#jx pipe=%#jx", (uintptr_t)xfer, (uintptr_t)epipe, 0, 0);
KASSERT(mutex_owned(&sc->sc_lock));
ASSERT_SLEEPABLE();
- if (sc->sc_dying) {
- /* If we're dying, just do the software part. */
- xfer->ux_status = status; /* make software ignore it */
- callout_stop(&xfer->ux_callout);
- usb_transfer_complete(xfer);
- return;
+ if (status == USBD_CANCELLED) {
+ /*
+ * We are synchronously aborting. Try to stop the
+ * callout and task, but if we can't, wait for them to
+ * complete.
+ */
+ callout_halt(&xfer->ux_callout, &sc->sc_lock);
+ usb_rem_task_wait(xfer->ux_pipe->up_dev, &xfer->ux_aborttask,
+ USB_TASKQ_HC, &sc->sc_lock);
+ } else {
+ /* Otherwise, we are timing out. */
+ KASSERT(status == USBD_TIMEOUT);
}
/*
- * If an abort is already in progress then just wait for it to
- * complete and return.
+ * The xfer cannot have been cancelled already. It is the
+ * responsibility of the caller of usbd_abort_pipe not to try
+ * to abort a pipe multiple times, whether concurrently or
+ * sequentially.
*/
- if (xfer->ux_hcflags & UXFER_ABORTING) {
- DPRINTF("already aborting", 0, 0, 0, 0);
-#ifdef DIAGNOSTIC
- if (status == USBD_TIMEOUT)
- printf("ehci_abort_xfer: TIMEOUT while aborting\n");
-#endif
- /* Override the status which might be USBD_TIMEOUT. */
- xfer->ux_status = status;
- DPRINTF("waiting for abort to finish", 0, 0, 0, 0);
- xfer->ux_hcflags |= UXFER_ABORTWAIT;
- while (xfer->ux_hcflags & UXFER_ABORTING)
- cv_wait(&xfer->ux_hccv, &sc->sc_lock);
+ KASSERT(xfer->ux_status != USBD_CANCELLED);
+
+ /* Only the timeout, which runs only once, can time it out. */
+ KASSERT(xfer->ux_status != USBD_TIMEOUT);
+
+ /* If anyone else beat us, we're done. */
+ if (xfer->ux_status != USBD_IN_PROGRESS)
return;
- }
- xfer->ux_hcflags |= UXFER_ABORTING;
+
+ /* We beat everyone else. Claim the status. */
+ xfer->ux_status = status;
/*
- * Step 1: Make interrupt routine and hardware ignore xfer.
+ * If we're dying, skip the hardware action and just notify the
+ * software that we're done.
*/
- xfer->ux_status = status; /* make software ignore it */
- callout_stop(&xfer->ux_callout);
+ if (sc->sc_dying) {
+ goto dying;
+ }
+
+ /*
+ * HC Step 1: Make interrupt routine and hardware ignore xfer.
+ */
ehci_del_intr_list(sc, exfer);
usb_syncmem(&sqh->dma,
@@ -3215,17 +3238,13 @@
}
/*
- * Step 2: Wait until we know hardware has finished any possible
- * use of the xfer. Also make sure the soft interrupt routine
- * has run.
+ * HC Step 2: Wait until we know hardware has finished any possible
+ * use of the xfer.
*/
ehci_sync_hc(sc);
- sc->sc_softwake = 1;
- usb_schedsoftintr(&sc->sc_bus);
- cv_wait(&sc->sc_softwake_cv, &sc->sc_lock);
/*
- * Step 3: Remove any vestiges of the xfer from the hardware.
+ * HC Step 3: Remove any vestiges of the xfer from the hardware.
* The complication here is that the hardware may have executed
* beyond the xfer we're trying to abort. So as we're scanning
* the TDs of this xfer we check if the hardware points to
@@ -3266,17 +3285,14 @@
}
/*
- * Step 4: Execute callback.
+ * Final step: Notify completion to waiting xfers.
*/
+dying:
#ifdef DIAGNOSTIC
exfer->ex_isdone = true;
#endif
- wake = xfer->ux_hcflags & UXFER_ABORTWAIT;
- xfer->ux_hcflags &= ~(UXFER_ABORTING | UXFER_ABORTWAIT);
usb_transfer_complete(xfer);
- if (wake) {
Home |
Main Index |
Thread Index |
Old Index