Date: Mon, 2 Jul 2007 14:45:58 GMT From: Sepherosa Ziehau <sephe@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 122723 for review Message-ID: <200707021445.l62Ejwgn042059@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=122723 Change 122723 by sephe@sephe_zealot:sam_wifi on 2007/07/02 14:45:19 - Switch from mtx lock to sx lock, so we can drain taskqueue in ath_newstate(). This promises that after ieee80211_new_state(INIT) driver will not touch PCI clock domain registers so that HAL power can be set to SLEEP safely. - Nuke ath_stop(). - Set HAL power to AWAKE in ath_resume(). Obtained from: sam@ - Move bus_teardown_intr() from ath_pci_detach() into ath_detach(), but after ath_stop_locked(). This is used to make sure that intr handler will not be called after HAL power is set to SLEEP. - Drain TX taskqueue in ath_newstate(). - Don't explicitly set HAL power to SLEEP in ath_detach(), since HAL detaching routine will do that too. Confirmed by: sam@ # Sx lock still has trouble to play with multicast code. Affected files ... .. //depot/projects/wifi/sys/dev/ath/ath_rate/sample/sample.c#13 edit .. //depot/projects/wifi/sys/dev/ath/if_ath.c#148 edit .. //depot/projects/wifi/sys/dev/ath/if_ath_pci.c#15 edit .. //depot/projects/wifi/sys/dev/ath/if_athvar.h#59 edit Differences ... ==== //depot/projects/wifi/sys/dev/ath/ath_rate/sample/sample.c#13 (text+ko) ==== @@ -49,7 +49,7 @@ #include <sys/module.h> #include <sys/kernel.h> #include <sys/lock.h> -#include <sys/mutex.h> +#include <sys/sx.h> #include <sys/errno.h> #include <machine/bus.h> ==== //depot/projects/wifi/sys/dev/ath/if_ath.c#148 (text+ko) ==== @@ -49,7 +49,7 @@ #include <sys/mbuf.h> #include <sys/malloc.h> #include <sys/lock.h> -#include <sys/mutex.h> +#include <sys/sx.h> #include <sys/kernel.h> #include <sys/socket.h> #include <sys/sockio.h> @@ -104,7 +104,6 @@ static void ath_init(void *); static void ath_stop_locked(struct ifnet *); -static void ath_stop(struct ifnet *); static void ath_start(struct ifnet *); static int ath_reset(struct ifnet *); static int ath_media_change(struct ifnet *); @@ -656,7 +655,7 @@ } int -ath_detach(struct ath_softc *sc) +ath_detach(struct ath_softc *sc, struct resource *irq, void *irq_handler) { struct ifnet *ifp = sc->sc_ifp; @@ -667,7 +666,12 @@ if (ifp->if_capenable & IFCAP_POLLING) ether_poll_deregister(ifp); #endif - ath_stop(ifp); + ATH_LOCK(sc); + ath_stop_locked(ifp); + ATH_UNLOCK(sc); + + bus_teardown_intr(sc->sc_dev, irq, irq_handler); + bpfdetach(ifp); /* * NB: the order of these is important: @@ -704,25 +708,31 @@ DPRINTF(sc, ATH_DEBUG_ANY, "%s: if_flags %x\n", __func__, ifp->if_flags); - ath_stop(ifp); + ATH_LOCK(sc); + ath_stop_locked(ifp); + ath_hal_setpower(sc->sc_ah, HAL_PM_FULL_SLEEP); + ATH_UNLOCK(sc); } void ath_resume(struct ath_softc *sc) { struct ifnet *ifp = sc->sc_ifp; + struct ath_hal *ah = sc->sc_ah; DPRINTF(sc, ATH_DEBUG_ANY, "%s: if_flags %x\n", __func__, ifp->if_flags); + ath_hal_setpower(ah, HAL_PM_AWAKE); + if (ifp->if_flags & IFF_UP) { ath_init(sc); if (ifp->if_drv_flags & IFF_DRV_RUNNING) ath_start(ifp); } if (sc->sc_softled) { - ath_hal_gpioCfgOutput(sc->sc_ah, sc->sc_ledpin); - ath_hal_gpioset(sc->sc_ah, sc->sc_ledpin, !sc->sc_ledon); + ath_hal_gpioCfgOutput(ah, sc->sc_ledpin); + ath_hal_gpioset(ah, sc->sc_ledpin, !sc->sc_ledon); } } @@ -734,7 +744,21 @@ DPRINTF(sc, ATH_DEBUG_ANY, "%s: if_flags %x\n", __func__, ifp->if_flags); - ath_stop(ifp); + ATH_LOCK(sc); + ath_stop_locked(ifp); + if (!sc->sc_invalid) { + /* + * Set the chip in full sleep mode. Note that we are + * careful to do this only when bringing the interface + * completely to a stop. When the chip is in this state + * it must be carefully woken up or references to + * registers in the PCI clock domain may freeze the bus + * (and system). This varies by chip and is mostly an + * issue with newer parts that go to sleep more quickly. + */ + ath_hal_setpower(sc->sc_ah, HAL_PM_FULL_SLEEP); + } + ATH_UNLOCK(sc); } /* @@ -1155,28 +1179,6 @@ } } -static void -ath_stop(struct ifnet *ifp) -{ - struct ath_softc *sc = ifp->if_softc; - - ATH_LOCK(sc); - ath_stop_locked(ifp); - if (!sc->sc_invalid) { - /* - * Set the chip in full sleep mode. Note that we are - * careful to do this only when bringing the interface - * completely to a stop. When the chip is in this state - * it must be carefully woken up or references to - * registers in the PCI clock domain may freeze the bus - * (and system). This varies by chip and is mostly an - * issue with newer parts that go to sleep more quickly. - */ - ath_hal_setpower(sc->sc_ah, HAL_PM_FULL_SLEEP); - } - ATH_UNLOCK(sc); -} - /* * Reset the hardware w/o losing operational state. This is * basically a more efficient way of doing ath_stop, ath_init, @@ -5318,13 +5320,11 @@ */ sc->sc_imask &= ~(HAL_INT_SWBA | HAL_INT_BMISS); ath_intrset(sc, sc->sc_imask &~ HAL_INT_GLOBAL); -#if 0 - /* XXX can't use taskqueue_drain 'cuz we're holding sc_mtx */ taskqueue_drain(sc->sc_tq, &sc->sc_rxtask); + taskqueue_drain(sc->sc_tq, &sc->sc_txtask); taskqueue_drain(sc->sc_tq, &sc->sc_rxorntask); taskqueue_drain(sc->sc_tq, &sc->sc_bmisstask); taskqueue_drain(sc->sc_tq, &sc->sc_bstucktask); -#endif ath_rate_newstate(sc, nstate); goto done; } ==== //depot/projects/wifi/sys/dev/ath/if_ath_pci.c#15 (text+ko) ==== @@ -39,7 +39,7 @@ #include <sys/module.h> #include <sys/kernel.h> #include <sys/lock.h> -#include <sys/mutex.h> +#include <sys/sx.h> #include <sys/errno.h> #include <machine/bus.h> @@ -213,10 +213,9 @@ /* check if device was removed */ sc->sc_invalid = !bus_child_present(dev); - ath_detach(sc); + ath_detach(sc, psc->sc_irq, psc->sc_ih); bus_generic_detach(dev); - bus_teardown_intr(dev, psc->sc_irq, psc->sc_ih); bus_release_resource(dev, SYS_RES_IRQ, 0, psc->sc_irq); bus_dma_tag_destroy(sc->sc_dmat); ==== //depot/projects/wifi/sys/dev/ath/if_athvar.h#59 (text+ko) ==== @@ -197,7 +197,7 @@ HAL_BUS_TAG sc_st; /* bus space tag */ HAL_BUS_HANDLE sc_sh; /* bus space handle */ bus_dma_tag_t sc_dmat; /* bus DMA tag */ - struct mtx sc_mtx; /* master lock (recursive) */ + struct sx sc_mtx; /* master lock (recursive) */ struct taskqueue *sc_tq; /* private task queue */ struct ath_hal *sc_ah; /* Atheros HAL */ struct ath_ratectrl *sc_rc; /* tx rate control support */ @@ -321,12 +321,12 @@ #define sc_rx_th u_rx_rt.th #define ATH_LOCK_INIT(_sc) \ - mtx_init(&(_sc)->sc_mtx, device_get_nameunit((_sc)->sc_dev), \ - NULL, MTX_DEF | MTX_RECURSE) -#define ATH_LOCK_DESTROY(_sc) mtx_destroy(&(_sc)->sc_mtx) -#define ATH_LOCK(_sc) mtx_lock(&(_sc)->sc_mtx) -#define ATH_UNLOCK(_sc) mtx_unlock(&(_sc)->sc_mtx) -#define ATH_LOCK_ASSERT(_sc) mtx_assert(&(_sc)->sc_mtx, MA_OWNED) + sx_init_flags(&(_sc)->sc_mtx, device_get_nameunit((_sc)->sc_dev), \ + SX_RECURSE) +#define ATH_LOCK_DESTROY(_sc) sx_destroy(&(_sc)->sc_mtx) +#define ATH_LOCK(_sc) sx_xlock(&(_sc)->sc_mtx) +#define ATH_UNLOCK(_sc) sx_xunlock(&(_sc)->sc_mtx) +#define ATH_LOCK_ASSERT(_sc) sx_assert(&(_sc)->sc_mtx, SA_LOCKED) #define ATH_TXQ_SETUP(sc, i) ((sc)->sc_txqsetup & (1<<i)) @@ -342,7 +342,7 @@ mtx_assert(&(_sc)->sc_txbuflock, MA_OWNED) int ath_attach(u_int16_t, struct ath_softc *); -int ath_detach(struct ath_softc *); +int ath_detach(struct ath_softc *, struct resource *, void *); void ath_resume(struct ath_softc *); void ath_suspend(struct ath_softc *); void ath_shutdown(struct ath_softc *);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200707021445.l62Ejwgn042059>