Module Name:    src
Committed By:   riastradh
Date:           Mon Mar 31 14:46:42 UTC 2025

Modified Files:
        src/sys/arch/arm/xscale: pxa2x0_ohci.c
        src/sys/arch/mips/ralink: ralink_ohci.c
        src/sys/dev/cardbus: ohci_cardbus.c
        src/sys/dev/pci: ohci_pci.c
        src/sys/dev/usb: ohci.c ohcivar.h

Log Message:
ohci(4): Rework detach logic and justify the ordering.

Handle failed attach when we detach.

This changes the signature of the ohci_detach function, but it is
only ever used by statically linked ohci bus attachments, never by
modules so far, so no kernel revbump.

PR port-amd64/59180: System reboots instead of shutting down


To generate a diff of this commit:
cvs rdiff -u -r1.13 -r1.14 src/sys/arch/arm/xscale/pxa2x0_ohci.c
cvs rdiff -u -r1.7 -r1.8 src/sys/arch/mips/ralink/ralink_ohci.c
cvs rdiff -u -r1.46 -r1.47 src/sys/dev/cardbus/ohci_cardbus.c
cvs rdiff -u -r1.59 -r1.60 src/sys/dev/pci/ohci_pci.c
cvs rdiff -u -r1.329 -r1.330 src/sys/dev/usb/ohci.c
cvs rdiff -u -r1.63 -r1.64 src/sys/dev/usb/ohcivar.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/arch/arm/xscale/pxa2x0_ohci.c
diff -u src/sys/arch/arm/xscale/pxa2x0_ohci.c:1.13 src/sys/arch/arm/xscale/pxa2x0_ohci.c:1.14
--- src/sys/arch/arm/xscale/pxa2x0_ohci.c:1.13	Sat Aug  7 16:18:46 2021
+++ src/sys/arch/arm/xscale/pxa2x0_ohci.c	Mon Mar 31 14:46:42 2025
@@ -1,4 +1,4 @@
-/*	$NetBSD: pxa2x0_ohci.c,v 1.13 2021/08/07 16:18:46 thorpej Exp $	*/
+/*	$NetBSD: pxa2x0_ohci.c,v 1.14 2025/03/31 14:46:42 riastradh Exp $	*/
 /*	$OpenBSD: pxa2x0_ohci.c,v 1.19 2005/04/08 02:32:54 dlg Exp $ */
 
 /*
@@ -123,11 +123,7 @@ pxaohci_attach(device_t parent, device_t
 	}
 
 #if 0
-	sc->sc.sc_powerhook = powerhook_establish(device_xname(sc->sc.sc_bus.bdev),
-	    pxaohci_power, sc);
-	if (sc->sc.sc_powerhook == NULL) {
-		aprint_error_dev(sc->sc.sc_dev->sc_bus.bdev, "cannot establish powerhook\n");
-	}
+	pmf_device_register1(self, ohci_suspend, ohci_resume, ohci_shutdown);
 #endif
 
 	sc->sc.sc_child = config_found(self, &sc->sc.sc_bus, usbctlprint,
@@ -151,27 +147,57 @@ pxaohci_detach(device_t self, int flags)
 	struct pxaohci_softc *sc = device_private(self);
 	int error;
 
-	error = ohci_detach(&sc->sc, flags);
+	/*
+	 * Detach the USB child first.  Disconnects all USB devices and
+	 * prevents connecting new ones.
+	 */
+	error = config_detach_children(self, flags);
 	if (error)
 		return error;
 
+	/*
+	 * Shut down the controller and block interrupts at the device
+	 * level.  Once we have shut down the controller, the shutdown
+	 * handler no longer needed -- deregister it from PMF.
+	 * (Harmless to call ohci_shutdown more than once, so no
+	 * synchronization needed.)
+	 */
+	ohci_shutdown(self, 0);
 #if 0
-	if (sc->sc.sc_powerhook) {
-		powerhook_disestablish(sc->sc.sc_powerhook);
-		sc->sc.sc_powerhook = NULL;
-	}
+	pmf_device_deregister(self);
 #endif
 
+	/*
+	 * Interrupts are blocked at the device level by ohci_shutdown.
+	 * Disestablish the interrupt handler.  This waits for it to
+	 * complete on all CPUs.
+	 */
 	if (sc->sc_ih) {
 		pxa2x0_intr_disestablish(sc->sc_ih);
 		sc->sc_ih = NULL;
 	}
 
+	/*
+	 * Free the bus-independent ohci(4) state now that the
+	 * interrupt handler has ceased to run on all CPUs.
+	 */
+	ohci_detach(&sc->sc);
+
+	/*
+	 * Issue a Full Host Reset to disable the host controller
+	 * interface.
+	 *
+	 * XXX Is this necessary or is it redundant with ohci_shutdown?
+	 * Should it be done in ohci_shutdown as well?
+	 */
 	pxaohci_disable(sc);
 
 	/* stop clock */
 	pxa2x0_clkman_config(CKEN_USBHC, 0);
 
+	/*
+	 * Unmap the registers now that we're all done with them.
+	 */
 	if (sc->sc.sc_size) {
 		bus_space_unmap(sc->sc.iot, sc->sc.ioh, sc->sc.sc_size);
 		sc->sc.sc_size = 0;

Index: src/sys/arch/mips/ralink/ralink_ohci.c
diff -u src/sys/arch/mips/ralink/ralink_ohci.c:1.7 src/sys/arch/mips/ralink/ralink_ohci.c:1.8
--- src/sys/arch/mips/ralink/ralink_ohci.c:1.7	Sat Aug  7 16:18:59 2021
+++ src/sys/arch/mips/ralink/ralink_ohci.c	Mon Mar 31 14:46:42 2025
@@ -1,4 +1,4 @@
-/*	$NetBSD: ralink_ohci.c,v 1.7 2021/08/07 16:18:59 thorpej Exp $	*/
+/*	$NetBSD: ralink_ohci.c,v 1.8 2025/03/31 14:46:42 riastradh Exp $	*/
 /*-
  * Copyright (c) 2011 CradlePoint Technology, Inc.
  * All rights reserved.
@@ -31,7 +31,7 @@
 #include "ehci.h"
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ralink_ohci.c,v 1.7 2021/08/07 16:18:59 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ralink_ohci.c,v 1.8 2025/03/31 14:46:42 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/bus.h>
@@ -162,28 +162,58 @@ static int
 ralink_ohci_detach(device_t self, int flags)
 {
 	struct ralink_ohci_softc *sc = device_private(self);
-	int rv;
+	int error;
 
-	pmf_device_deregister(self);
+	/*
+	 * Detach the USB child first.  Disconnects all USB devices and
+	 * prevents connecting new ones.
+	 */
+	error = config_detach_children(self, flags);
+	if (error)
+		return error;
+
+	/*
+	 * Stop listing this as a possible companion controller for
+	 * ehci(4).
+	 */
+#if NEHCI > 0
+	ralink_usb_hc_rem(&sc->sc_hc);
+#endif
 
-	rv = ohci_detach(&sc->sc_ohci, flags);
-	if (rv != 0)
-		return rv;
+	/*
+	 * Shut down the controller and block interrupts at the device
+	 * level.  Once we have shut down the controller, the shutdown
+	 * handler no longer needed -- deregister it from PMF.
+	 * (Harmless to call ohci_shutdown more than once, so no
+	 * synchronization needed.)
+	 */
+	ohci_shutdown(self, 0);
+	pmf_device_deregister(self);
 
+	/*
+	 * Interrupts are blocked at the device level by ohci_shutdown.
+	 * Disestablish the interrupt handler.  This waits for it to
+	 * complete on all CPUs.
+	 */
 	if (sc->sc_ih != NULL) {
 		ra_intr_disestablish(sc->sc_ih);
 		sc->sc_ih = NULL;
 	}
 
-	if (sc->sc_ohci.sc_size == 0) {
+	/*
+	 * Free the bus-independent ohci(4) state now that the
+	 * interrupt handler has ceased to run on all CPUs.
+	 */
+	ohci_detach(&sc->sc_ohci);
+
+	/*
+	 * Unmap the registers now that we're all done with them.
+	 */
+	if (sc->sc_ohci.sc_size) {
 		bus_space_unmap(sc->sc_ohci.iot, sc->sc_ohci.ioh,
-			sc->sc_ohci.sc_size);
+		    sc->sc_ohci.sc_size);
 		sc->sc_ohci.sc_size = 0;
 	}
 
-#if NEHCI > 0
-	ralink_usb_hc_rem(&sc->sc_hc);
-#endif
-
 	return 0;
 }

Index: src/sys/dev/cardbus/ohci_cardbus.c
diff -u src/sys/dev/cardbus/ohci_cardbus.c:1.46 src/sys/dev/cardbus/ohci_cardbus.c:1.47
--- src/sys/dev/cardbus/ohci_cardbus.c:1.46	Sat Aug  7 16:19:10 2021
+++ src/sys/dev/cardbus/ohci_cardbus.c	Mon Mar 31 14:46:42 2025
@@ -1,4 +1,4 @@
-/*	$NetBSD: ohci_cardbus.c,v 1.46 2021/08/07 16:19:10 thorpej Exp $	*/
+/*	$NetBSD: ohci_cardbus.c,v 1.47 2025/03/31 14:46:42 riastradh Exp $	*/
 
 /*
  * Copyright (c) 1998 The NetBSD Foundation, Inc.
@@ -38,7 +38,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ohci_cardbus.c,v 1.46 2021/08/07 16:19:10 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ohci_cardbus.c,v 1.47 2025/03/31 14:46:42 riastradh Exp $");
 
 #include "ehci_cardbus.h"
 
@@ -175,22 +175,58 @@ ohci_cardbus_detach(device_t self, int f
 {
 	struct ohci_cardbus_softc *sc = device_private(self);
 	struct cardbus_devfunc *ct = sc->sc_ct;
-	int rv;
+	int error;
 
-	rv = ohci_detach(&sc->sc, flags);
-	if (rv)
-		return rv;
+	/*
+	 * Detach the USB child first.  Disconnects all USB devices and
+	 * prevents connecting new ones.
+	 */
+	error = config_detach_children(self, flags);
+	if (error)
+		return error;
+
+	/*
+	 * Stop listing this as a possible companion controller for
+	 * ehci(4).
+	 */
+#if NEHCI_CARDBUS > 0
+	usb_cardbus_rem(&sc->sc_cardbus);
+#endif
+
+	/*
+	 * Shut down the controller and block interrupts at the device
+	 * level.  Once we have shut down the controller, the shutdown
+	 * handler no longer needed -- deregister it from PMF.
+	 * (Harmless to call ohci_shutdown more than once, so no
+	 * synchronization needed.)
+	 */
+	ohci_shutdown(self, 0);
+	pmf_device_deregister(self);
+
+	/*
+	 * Interrupts are blocked at the device level by ohci_shutdown.
+	 * Disestablish the interrupt handler.  This waits for it to
+	 * complete on all CPUs.
+	 */
 	if (sc->sc_ih != NULL) {
 		Cardbus_intr_disestablish(ct, sc->sc_ih);
 		sc->sc_ih = NULL;
 	}
+
+	/*
+	 * Free the bus-independent ohci(4) state now that the
+	 * interrupt handler has ceased to run on all CPUs.
+	 */
+	ohci_detach(&sc->sc);
+
+	/*
+	 * Unmap the registers now that we're all done with them.
+	 */
 	if (sc->sc.sc_size) {
 		Cardbus_mapreg_unmap(ct, PCI_CBMEM, sc->sc.iot,
 		    sc->sc.ioh, sc->sc.sc_size);
 		sc->sc.sc_size = 0;
 	}
-#if NEHCI_CARDBUS > 0
-	usb_cardbus_rem(&sc->sc_cardbus);
-#endif
+
 	return 0;
 }

Index: src/sys/dev/pci/ohci_pci.c
diff -u src/sys/dev/pci/ohci_pci.c:1.59 src/sys/dev/pci/ohci_pci.c:1.60
--- src/sys/dev/pci/ohci_pci.c:1.59	Sat Aug  7 16:19:14 2021
+++ src/sys/dev/pci/ohci_pci.c	Mon Mar 31 14:46:42 2025
@@ -1,4 +1,4 @@
-/*	$NetBSD: ohci_pci.c,v 1.59 2021/08/07 16:19:14 thorpej Exp $	*/
+/*	$NetBSD: ohci_pci.c,v 1.60 2025/03/31 14:46:42 riastradh Exp $	*/
 
 /*
  * Copyright (c) 1998 The NetBSD Foundation, Inc.
@@ -31,7 +31,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ohci_pci.c,v 1.59 2021/08/07 16:19:14 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ohci_pci.c,v 1.60 2025/03/31 14:46:42 riastradh Exp $");
 
 #include "ehci.h"
 
@@ -187,33 +187,58 @@ static int
 ohci_pci_detach(device_t self, int flags)
 {
 	struct ohci_pci_softc *sc = device_private(self);
-	int rv;
+	int error;
 
-	rv = ohci_detach(&sc->sc, flags);
-	if (rv)
-		return rv;
-
-	pmf_device_deregister(self);
+	/*
+	 * Detach the USB child first.  Disconnects all USB devices and
+	 * prevents connecting new ones.
+	 */
+	error = config_detach_children(self, flags);
+	if (error)
+		return error;
 
-	ohci_shutdown(self, flags);
+	/*
+	 * Stop listing this as a possible companion controller for
+	 * ehci(4).
+	 */
+#if NEHCI > 0
+	usb_pci_rem(&sc->sc_pci);
+#endif
 
-	if (sc->sc.sc_size) {
-		/* Disable interrupts, so we don't get any spurious ones. */
-		bus_space_write_4(sc->sc.iot, sc->sc.ioh,
-				  OHCI_INTERRUPT_DISABLE, OHCI_ALL_INTRS);
-	}
+	/*
+	 * Shut down the controller and block interrupts at the device
+	 * level.  Once we have shut down the controller, the shutdown
+	 * handler no longer needed -- deregister it from PMF.
+	 * (Harmless to call ohci_shutdown more than once, so no
+	 * synchronization needed.)
+	 */
+	ohci_shutdown(self, 0);
+	pmf_device_deregister(self);
 
+	/*
+	 * Interrupts are blocked at the device level by ohci_shutdown.
+	 * Disestablish the interrupt handler.  This waits for it to
+	 * complete on all CPUs.
+	 */
 	if (sc->sc_ih != NULL) {
 		pci_intr_disestablish(sc->sc_pc, sc->sc_ih);
 		sc->sc_ih = NULL;
 	}
+
+	/*
+	 * Free the bus-independent ohci(4) state now that the
+	 * interrupt handler has ceased to run on all CPUs.
+	 */
+	ohci_detach(&sc->sc);
+
+	/*
+	 * Unmap the registers now that we're all done with them.
+	 */
 	if (sc->sc.sc_size) {
 		bus_space_unmap(sc->sc.iot, sc->sc.ioh, sc->sc.sc_size);
 		sc->sc.sc_size = 0;
 	}
-#if NEHCI > 0
-	usb_pci_rem(&sc->sc_pci);
-#endif
+
 	return 0;
 }
 

Index: src/sys/dev/usb/ohci.c
diff -u src/sys/dev/usb/ohci.c:1.329 src/sys/dev/usb/ohci.c:1.330
--- src/sys/dev/usb/ohci.c:1.329	Sun Sep 22 14:05:47 2024
+++ src/sys/dev/usb/ohci.c	Mon Mar 31 14:46:42 2025
@@ -1,4 +1,4 @@
-/*	$NetBSD: ohci.c,v 1.329 2024/09/22 14:05:47 jmcneill Exp $	*/
+/*	$NetBSD: ohci.c,v 1.330 2025/03/31 14:46:42 riastradh Exp $	*/
 
 /*
  * Copyright (c) 1998, 2004, 2005, 2012, 2016, 2020 The NetBSD Foundation, Inc.
@@ -42,7 +42,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ohci.c,v 1.329 2024/09/22 14:05:47 jmcneill Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ohci.c,v 1.330 2025/03/31 14:46:42 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -368,16 +368,14 @@ ohci_childdet(device_t self, device_t ch
 	sc->sc_child = NULL;
 }
 
-int
-ohci_detach(struct ohci_softc *sc, int flags)
+void
+ohci_detach(struct ohci_softc *sc)
 {
-	int rv = 0;
 
-	if (sc->sc_child != NULL)
-		rv = config_detach(sc->sc_child, flags);
+	KASSERT(sc->sc_child == NULL);
 
-	if (rv != 0)
-		return rv;
+	if (!sc->sc_attached)
+		return;
 
 	softint_disestablish(sc->sc_rhsc_si);
 
@@ -391,8 +389,6 @@ ohci_detach(struct ohci_softc *sc, int f
 		usb_freemem(&sc->sc_hccadma);
 	pool_cache_destroy(sc->sc_xferpool);
 	cv_destroy(&sc->sc_abort_cv);
-
-	return rv;
 }
 
 ohci_soft_ed_t *
@@ -1092,6 +1088,7 @@ ohci_init(ohci_softc_t *sc)
 	DPRINTF("enabling %#jx", sc->sc_eintrs | OHCI_MIE, 0, 0, 0);
 	OWRITE4(sc, OHCI_INTERRUPT_ENABLE, sc->sc_eintrs | OHCI_MIE);
 
+	sc->sc_attached = true;
 	return 0;
 
  bad5:
@@ -1166,6 +1163,9 @@ ohci_shutdown(device_t self, int flags)
 
 	OHCIHIST_FUNC(); OHCIHIST_CALLED();
 
+	if (!sc->sc_attached)
+		return true;
+
 	DPRINTF("stopping the HC", 0, 0, 0, 0);
 	OWRITE4(sc, OHCI_INTERRUPT_DISABLE, OHCI_ALL_INTRS);
 	OWRITE4(sc, OHCI_CONTROL, OHCI_SET_HCFS(OHCI_HCFS_RESET));

Index: src/sys/dev/usb/ohcivar.h
diff -u src/sys/dev/usb/ohcivar.h:1.63 src/sys/dev/usb/ohcivar.h:1.64
--- src/sys/dev/usb/ohcivar.h:1.63	Sun Sep 22 14:05:47 2024
+++ src/sys/dev/usb/ohcivar.h	Mon Mar 31 14:46:42 2025
@@ -1,4 +1,4 @@
-/*	$NetBSD: ohcivar.h,v 1.63 2024/09/22 14:05:47 jmcneill Exp $	*/
+/*	$NetBSD: ohcivar.h,v 1.64 2025/03/31 14:46:42 riastradh Exp $	*/
 
 /*
  * Copyright (c) 1998 The NetBSD Foundation, Inc.
@@ -90,6 +90,7 @@ typedef struct ohci_softc {
 	bus_space_tag_t iot;
 	bus_space_handle_t ioh;
 	bus_size_t sc_size;
+	bool sc_attached;
 
 	kmutex_t sc_lock;
 	kmutex_t sc_intr_lock;
@@ -172,7 +173,7 @@ struct ohci_xfer {
 
 int		ohci_init(ohci_softc_t *);
 int		ohci_intr(void *);
-int		ohci_detach(ohci_softc_t *, int);
+void		ohci_detach(ohci_softc_t *);
 bool		ohci_shutdown(device_t, int);
 void		ohci_childdet(device_t, device_t);
 int		ohci_activate(device_t, enum devact);

Reply via email to