Date: Mon, 21 Aug 2006 17:53:19 GMT From: Paolo Pisati <piso@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 104702 for review Message-ID: <200608211753.k7LHrJEM022945@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=104702 Change 104702 by piso@piso_longino on 2006/08/21 17:53:08 Restore the original driver: i keep getting a "kernel trap 12 with interrupt disabled" while calling ath_hal_intrpend() in the filter. Talk with jhb and sam about refactoring this driver. Affected files ... .. //depot/projects/soc2006/intr_filter/dev/ath/if_ath.c#6 edit .. //depot/projects/soc2006/intr_filter/dev/ath/if_ath_pci.c#4 edit .. //depot/projects/soc2006/intr_filter/dev/ath/if_athvar.h#5 edit Differences ... ==== //depot/projects/soc2006/intr_filter/dev/ath/if_ath.c#6 (text+ko) ==== @@ -115,8 +115,8 @@ static void ath_watchdog(struct ifnet *); static int ath_ioctl(struct ifnet *, u_long, caddr_t); static void ath_fatal_proc(void *, int); -static void ath_rxorn_proc(void *); -static void ath_bmiss_proc(void *); +static void ath_rxorn_proc(void *, int); +static void ath_bmiss_proc(void *, int); static void ath_radar_proc(void *, int); static int ath_key_alloc(struct ieee80211com *, const struct ieee80211_key *, @@ -149,7 +149,7 @@ struct ieee80211_node *ni, int subtype, int rssi, u_int32_t rstamp); static void ath_setdefantenna(struct ath_softc *, u_int); -static void ath_rx_proc(void *); +static void ath_rx_proc(void *, int); static void ath_txq_init(struct ath_softc *sc, struct ath_txq *, int); static struct ath_txq *ath_txq_setup(struct ath_softc*, int qtype, int subtype); static int ath_tx_setup(struct ath_softc *, int, int); @@ -158,9 +158,9 @@ static void ath_tx_cleanup(struct ath_softc *); static int ath_tx_start(struct ath_softc *, struct ieee80211_node *, struct ath_buf *, struct mbuf *); -static void ath_tx_proc_q0(void *); -static void ath_tx_proc_q0123(void *); -static void ath_tx_proc(void *); +static void ath_tx_proc_q0(void *, int); +static void ath_tx_proc_q0123(void *, int); +static void ath_tx_proc(void *, int); static int ath_chan_set(struct ath_softc *, struct ieee80211_channel *); static void ath_draintxq(struct ath_softc *); static void ath_stoprecv(struct ath_softc *); @@ -398,7 +398,10 @@ taskqueue_thread_enqueue, &sc->sc_tq); taskqueue_start_threads(&sc->sc_tq, 1, PI_NET, "%s taskq", ifp->if_xname); - + + TASK_INIT(&sc->sc_rxtask, 0, ath_rx_proc, sc); + TASK_INIT(&sc->sc_rxorntask, 0, ath_rxorn_proc, sc); + TASK_INIT(&sc->sc_bmisstask, 0, ath_bmiss_proc, sc); TASK_INIT(&sc->sc_bstucktask,0, ath_bstuck_proc, sc); TASK_INIT(&sc->sc_radartask, 0, ath_radar_proc, sc); @@ -456,13 +459,13 @@ */ switch (sc->sc_txqsetup &~ (1<<sc->sc_cabq->axq_qnum)) { case 0x01: - sc->tx_func = &ath_tx_proc_q0; + TASK_INIT(&sc->sc_txtask, 0, ath_tx_proc_q0, sc); break; case 0x0f: - sc->tx_func = &ath_tx_proc_q0123; + TASK_INIT(&sc->sc_txtask, 0, ath_tx_proc_q0123, sc); break; default: - sc->tx_func = &ath_tx_proc; + TASK_INIT(&sc->sc_txtask, 0, ath_tx_proc, sc); break; } @@ -711,13 +714,16 @@ ath_stop(ifp); } -int -ath_filter(void *arg) +/* + * Interrupt handler. Most of the actual processing is deferred. + */ +void +ath_intr(void *arg) { struct ath_softc *sc = arg; struct ifnet *ifp = sc->sc_ifp; struct ath_hal *ah = sc->sc_ah; - int ret = FILTER_HANDLED; + HAL_INT status; if (sc->sc_invalid) { /* @@ -725,19 +731,17 @@ * Note this can happen early on if the IRQ is shared. */ DPRINTF(sc, ATH_DEBUG_ANY, "%s: invalid; ignored\n", __func__); - return (FILTER_STRAY); + return; } - mtx_lock_spin(&sc->sc_smtx); if (!ath_hal_intrpend(ah)) /* shared irq, not for us */ - return (FILTER_STRAY); + return; if (!((ifp->if_flags & IFF_UP) && (ifp->if_drv_flags & - IFF_DRV_RUNNING))) { + IFF_DRV_RUNNING))) { DPRINTF(sc, ATH_DEBUG_ANY, "%s: if_flags 0x%x\n", __func__, ifp->if_flags); - ath_hal_getisr(ah, &sc->sc_status); /* clear ISR */ + ath_hal_getisr(ah, &status); /* clear ISR */ ath_hal_intrset(ah, 0); /* disable further intr's */ - mtx_unlock_spin(&sc->sc_smtx); - return (ret); + return; } /* * Figure out the reason(s) for the interrupt. Note @@ -745,40 +749,9 @@ * bits we haven't explicitly enabled so we mask the * value to insure we only process bits we requested. */ - ath_hal_getisr(ah, &sc->sc_status); /* NB: clears ISR too */ - DPRINTF(sc, ATH_DEBUG_INTR, "%s: status 0x%x\n", __func__, sc->sc_status); - sc->sc_status &= sc->sc_imask; /* discard unasked for bits */ - if (sc->sc_status & HAL_INT_RXEOL) { - /* - * NB: the hardware should re-read the link when - * RXE bit is written, but it doesn't work at - * least on older hardware revs. - */ - sc->sc_stats.ast_rxeol++; - sc->sc_rxlink = NULL; - sc->sc_status &= ~HAL_INT_RXEOL; - } - - if (sc->sc_status) { - ath_hal_intrset(ah, 0); - ret |= FILTER_SCHEDULE_THREAD; - } - mtx_unlock_spin(&sc->sc_smtx); - return (ret); -} - -/* - * Interrupt handler. Most of the actual processing is deferred. - */ -void -ath_intr(void *arg) -{ - struct ath_softc *sc = arg; - struct ath_hal *ah = sc->sc_ah; - HAL_INT status; - - mtx_lock_spin(&sc->sc_smtx); - status = sc->sc_status; + ath_hal_getisr(ah, &status); /* NB: clears ISR too */ + DPRINTF(sc, ATH_DEBUG_INTR, "%s: status 0x%x\n", __func__, status); + status &= sc->sc_imask; /* discard unasked for bits */ if (status & HAL_INT_FATAL) { sc->sc_stats.ast_hardware++; ath_hal_intrset(ah, 0); /* disable intr's until reset */ @@ -786,7 +759,7 @@ } else if (status & HAL_INT_RXORN) { sc->sc_stats.ast_rxorn++; ath_hal_intrset(ah, 0); /* disable intr's until reset */ - ath_rxorn_proc(sc); + taskqueue_enqueue(sc->sc_tq, &sc->sc_rxorntask); } else { if (status & HAL_INT_SWBA) { /* @@ -797,18 +770,27 @@ */ ath_beacon_proc(sc, 0); } + if (status & HAL_INT_RXEOL) { + /* + * NB: the hardware should re-read the link when + * RXE bit is written, but it doesn't work at + * least on older hardware revs. + */ + sc->sc_stats.ast_rxeol++; + sc->sc_rxlink = NULL; + } if (status & HAL_INT_TXURN) { sc->sc_stats.ast_txurn++; /* bump tx trigger level */ ath_hal_updatetxtriglevel(ah, AH_TRUE); } if (status & HAL_INT_RX) - ath_rx_proc(sc); + taskqueue_enqueue(sc->sc_tq, &sc->sc_rxtask); if (status & HAL_INT_TX) - sc->tx_func(sc); + taskqueue_enqueue(sc->sc_tq, &sc->sc_txtask); if (status & HAL_INT_BMISS) { sc->sc_stats.ast_bmiss++; - ath_bmiss_proc(sc); + taskqueue_enqueue(sc->sc_tq, &sc->sc_bmisstask); } if (status & HAL_INT_MIB) { sc->sc_stats.ast_mib++; @@ -825,9 +807,6 @@ ath_hal_intrset(ah, sc->sc_imask); } } - /* Enable interrupts */ - ath_hal_intrset(ah, sc->sc_imask); - mtx_unlock_spin(&sc->sc_smtx); } static void @@ -854,7 +833,7 @@ } static void -ath_rxorn_proc(void *arg) +ath_rxorn_proc(void *arg, int pending) { struct ath_softc *sc = arg; struct ifnet *ifp = sc->sc_ifp; @@ -864,12 +843,12 @@ } static void -ath_bmiss_proc(void *arg) +ath_bmiss_proc(void *arg, int pending) { struct ath_softc *sc = arg; struct ieee80211com *ic = &sc->sc_ic; - DPRINTF(sc, ATH_DEBUG_ANY, "%s\n", __func__); + DPRINTF(sc, ATH_DEBUG_ANY, "%s: pending %u\n", __func__, pending); KASSERT(ic->ic_opmode == IEEE80211_M_STA, ("unexpect operating mode %u", ic->ic_opmode)); if (ic->ic_state == IEEE80211_S_RUN) { @@ -1009,9 +988,7 @@ */ if (sc->sc_needmib && ic->ic_opmode == IEEE80211_M_STA) sc->sc_imask |= HAL_INT_MIB; - mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, sc->sc_imask); - mtx_unlock_spin(&sc->sc_smtx); ifp->if_drv_flags |= IFF_DRV_RUNNING; ic->ic_state = IEEE80211_S_INIT; @@ -1077,9 +1054,7 @@ !sc->sc_ledon); sc->sc_blinking = 0; } - mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, 0); - mtx_unlock_spin(&sc->sc_smtx); } ath_draintxq(sc); if (!sc->sc_invalid) { @@ -1137,10 +1112,8 @@ c = ic->ic_curchan; sc->sc_curchan.channel = c->ic_freq; sc->sc_curchan.channelFlags = ath_chan2flags(ic, c); - - mtx_lock_spin(&sc->sc_smtx); + ath_hal_intrset(ah, 0); /* disable interrupts */ - mtx_unlock_spin(&sc->sc_smtx); ath_draintxq(sc); /* stop xmit side */ ath_stoprecv(sc); /* stop recv side */ /* NB: indicate channel change so we do a full reset */ @@ -1161,9 +1134,7 @@ if_printf(ifp, "%s: unable to start recv logic\n", __func__); if (ic->ic_state == IEEE80211_S_RUN) ath_beacon_config(sc); /* restart beacons */ - mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, sc->sc_imask); - mtx_unlock_spin(&sc->sc_smtx); ath_start(ifp); /* restart xmit */ return 0; @@ -2369,18 +2340,12 @@ , bs.bs_cfpnext , bs.bs_timoffset ); - mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, 0); - mtx_unlock_spin(&sc->sc_smtx); ath_hal_beacontimers(ah, &bs); sc->sc_imask |= HAL_INT_BMISS; - mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, sc->sc_imask); - mtx_unlock_spin(&sc->sc_smtx); } else { - mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, 0); - mtx_unlock_spin(&sc->sc_smtx); if (nexttbtt == intval) intval |= HAL_BEACON_RESET_TSF; if (ic->ic_opmode == IEEE80211_M_IBSS) { @@ -2417,9 +2382,7 @@ } ath_hal_beaconinit(ah, nexttbtt, intval); sc->sc_bmisscount = 0; - mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, sc->sc_imask); - mtx_unlock_spin(&sc->sc_smtx); /* * When using a self-linked beacon descriptor in * ibss mode load it once here. @@ -2867,7 +2830,7 @@ } static void -ath_rx_proc(void *arg) +ath_rx_proc(void *arg, int npending) { #define PA2DESC(_sc, _pa) \ ((struct ath_desc *)((caddr_t)(_sc)->sc_rxdma.dd_desc + \ @@ -2889,7 +2852,7 @@ NET_LOCK_GIANT(); /* XXX */ - DPRINTF(sc, ATH_DEBUG_RX_PROC, "%s\n", __func__); + DPRINTF(sc, ATH_DEBUG_RX_PROC, "%s: pending %u\n", __func__, npending); ngood = 0; nf = ath_hal_getchannoise(ah, &sc->sc_curchan); tsf = ath_hal_gettsf64(ah); @@ -4010,7 +3973,7 @@ * for a single hardware transmit queue (e.g. 5210 and 5211). */ static void -ath_tx_proc_q0(void *arg) +ath_tx_proc_q0(void *arg, int npending) { struct ath_softc *sc = arg; struct ifnet *ifp = sc->sc_ifp; @@ -4033,7 +3996,7 @@ * for four hardware queues, 0-3 (e.g. 5212 w/ WME support). */ static void -ath_tx_proc_q0123(void *arg) +ath_tx_proc_q0123(void *arg, int npending) { struct ath_softc *sc = arg; struct ifnet *ifp = sc->sc_ifp; @@ -4069,7 +4032,7 @@ * Deferred processing of transmit interrupt. */ static void -ath_tx_proc(void *arg) +ath_tx_proc(void *arg, int npending) { struct ath_softc *sc = arg; struct ifnet *ifp = sc->sc_ifp; @@ -4373,9 +4336,7 @@ * hardware at the new frequency, and then re-enable * the relevant bits of the h/w. */ - mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, 0); /* disable interrupts */ - mtx_unlock_spin(&sc->sc_smtx); ath_draintxq(sc); /* clear pending tx frames */ ath_stoprecv(sc); /* turn off frame recv */ if (!ath_hal_reset(ah, sc->sc_opmode, &hchan, AH_TRUE, &status)) { @@ -4429,9 +4390,7 @@ /* * Re-enable interrupts. */ - mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, sc->sc_imask); - mtx_unlock_spin(&sc->sc_smtx); } return 0; } @@ -4537,9 +4496,7 @@ /* * NB: disable interrupts so we don't rx frames. */ - mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, sc->sc_imask &~ HAL_INT_GLOBAL); - mtx_unlock_spin(&sc->sc_smtx); /* * Notify the rate control algorithm. */ @@ -4648,10 +4605,8 @@ sc->sc_halstats.ns_avgrssi = ATH_RSSI_DUMMY_MARKER; sc->sc_halstats.ns_avgtxrssi = ATH_RSSI_DUMMY_MARKER; } else { - mtx_lock_spin(&sc->sc_smtx); ath_hal_intrset(ah, sc->sc_imask &~ (HAL_INT_SWBA | HAL_INT_BMISS)); - mtx_unlock_spin(&sc->sc_smtx); sc->sc_imask &= ~(HAL_INT_SWBA | HAL_INT_BMISS); } done: ==== //depot/projects/soc2006/intr_filter/dev/ath/if_ath_pci.c#4 (text+ko) ==== @@ -168,8 +168,8 @@ goto bad1; } if (bus_setup_intr(dev, psc->sc_irq, - INTR_TYPE_NET | INTR_MPSAFE, ath_filter, - ath_intr, sc, &psc->sc_ih)) { + INTR_TYPE_NET | INTR_MPSAFE, + NULL, ath_intr, sc, &psc->sc_ih)) { device_printf(dev, "could not establish interrupt\n"); goto bad2; } @@ -194,14 +194,11 @@ } ATH_LOCK_INIT(sc); - mtx_init(&sc->sc_smtx, device_get_nameunit(dev), MTX_NETWORK_LOCK, - MTX_SPIN); error = ath_attach(pci_get_device(dev), sc); if (error == 0) return error; - mtx_destroy(&sc->sc_smtx); ATH_LOCK_DESTROY(sc); bus_dma_tag_destroy(sc->sc_dmat); bad3: ==== //depot/projects/soc2006/intr_filter/dev/ath/if_athvar.h#5 (text+ko) ==== @@ -223,13 +223,6 @@ u_int sc_mcastrate; /* ieee rate for mcastrateix */ u_int sc_txantenna; /* tx antenna (fixed or auto) */ HAL_INT sc_imask; /* interrupt mask copy */ - HAL_INT sc_status; - struct mtx sc_smtx; /* - * guard access to sc_status - * and to the interrupt handling - * registers (?!?!?!?!) - */ - void (*tx_func)(void *); u_int sc_keymax; /* size of key cache */ u_int8_t sc_keymap[ATH_KEYBYTES];/* key use bit map */ @@ -261,6 +254,7 @@ struct ath_descdma sc_rxdma; /* RX descriptos */ ath_bufhead sc_rxbuf; /* receive buffer */ u_int32_t *sc_rxlink; /* link ptr in last RX desc */ + struct task sc_rxtask; /* rx int processing */ struct task sc_rxorntask; /* rxorn int processing */ struct task sc_radartask; /* radar processing */ u_int8_t sc_defant; /* current default antenna */ @@ -276,6 +270,7 @@ u_int sc_txintrperiod;/* tx interrupt batching */ struct ath_txq sc_txq[HAL_NUM_TX_QUEUES]; struct ath_txq *sc_ac2q[5]; /* WME AC -> h/w q map */ + struct task sc_txtask; /* tx int processing */ struct ath_descdma sc_bdma; /* beacon descriptors */ ath_bufhead sc_bbuf; /* beacon buffers */ @@ -329,7 +324,6 @@ void ath_resume(struct ath_softc *); void ath_suspend(struct ath_softc *); void ath_shutdown(struct ath_softc *); -int ath_filter(void *); void ath_intr(void *); /*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200608211753.k7LHrJEM022945>