While working on suspend/resume for another arm64 board, I ran into an
issue with xhci(4). We detach USB devices early on in the suspend
process. At this point we mark the USB bus as "dying" to make sure
devices don't re-attach. We look at this "dying" flag in the
interrupt handler and bail out early, before actually acking the
interrupt. The problem here is that if we actually get an interrupt
during this phase it will continue to fire, slowing us down or even
stop us from making progress altogether.
The reason why we bail out early is that we also use the "dying" flag
for when the xhci(4) controller disappears from under out feet (for
example when someone removes a USB ExpressCard). In that case we
don't really want to continue accessing hardware registers that aren't
there anymore.
I think using the flag for two purposes is a bad idea. The diff below
introduces another flag that signals that the controller is no longer
there. This fixes the issue with my arm64 board.
ok?
Index: dev/usb/xhci.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/xhci.c,v
retrieving revision 1.125
diff -u -p -r1.125 xhci.c
--- dev/usb/xhci.c 12 Apr 2022 19:41:11 -0000 1.125
+++ dev/usb/xhci.c 14 Jul 2022 20:20:16 -0000
@@ -593,7 +593,7 @@ xhci_intr(void *v)
{
struct xhci_softc *sc = v;
- if (sc == NULL || sc->sc_bus.dying)
+ if (sc->sc_dead)
return (0);
/* If we get an interrupt while polling, then just ignore it. */
@@ -613,6 +613,7 @@ xhci_intr1(struct xhci_softc *sc)
intrs = XOREAD4(sc, XHCI_USBSTS);
if (intrs == 0xffffffff) {
sc->sc_bus.dying = 1;
+ sc->sc_dead = 1;
return (0);
}
Index: dev/usb/xhcivar.h
===================================================================
RCS file: /cvs/src/sys/dev/usb/xhcivar.h,v
retrieving revision 1.12
diff -u -p -r1.12 xhcivar.h
--- dev/usb/xhcivar.h 24 Dec 2020 14:11:38 -0000 1.12
+++ dev/usb/xhcivar.h 14 Jul 2022 20:20:16 -0000
@@ -88,6 +88,8 @@ struct xhci_softc {
bus_space_handle_t ioh;
bus_size_t sc_size;
+ int sc_dead;
+
bus_size_t sc_oper_off; /* Operational Register space */
bus_size_t sc_runt_off; /* Runtime */
bus_size_t sc_door_off; /* Doorbell */
