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);