wlan - if_ath driver - Make some adjustments to ath (preliminary)
authorMatthew Dillon <dillon@apollo.backplane.com>
Wed, 8 Sep 2010 07:20:23 +0000 (00:20 -0700)
committerMatthew Dillon <dillon@apollo.backplane.com>
Wed, 8 Sep 2010 07:20:23 +0000 (00:20 -0700)
These adjustments correct some chip races and appear to fix issues
related to running the wlan in AP mode with the atheros driver.

* Fix a bug in ath_txqmove().  This bug was hidden due to a queue
  length check in the one place that used the routine but fix it anyway.

* Call stoptxdma() before messing with the beacon linkages rather than
  afterwords (ap mode).  I'm a bit unclear as to whether the previously
  installed beacon should have been allowed to continue to run if no
  new beacons are found.  For now it isn't.

* Redo the qbusy logic.  The old logic had at least 2 chipset/driver
  races related to the link field.  The new logic makes no assumptions
  and only reactivates the txdma if the queue is clearly idle and
  we are adding the first (bf) to it.  Otherwise leave it to the INT_TX
  code to detect where the txdma stopped and restart it at the
  appropriate place.

  This bit of code needs more work as the INT_TX for tx completion may
  be delayed indefinitely (we might need a callout check in there too,
  I'm not sure).

* For the moment use MB_WAIT when loading or replentishing the receive ring.
  There does not appear to be a proper mechanism to deal with stalls that
  might be created if a mbuf fails to allocate.

  What we really need here is proper rx ring mbuf replacement logic where
  the filled mbuf is NOT removed if no new mbuf can be allocated to take
  its place.  Using MB_WAIT is a bad hack.  It isn't entirely trivial
  due to the DMA load and the 32 bit address space restriction.

* Add a few cpu_sfence()s when poking the (bf) link field.  This probably
  isn't correct.

* Cleanup, add some debugging kprintf()s for a few unexpected conditions

sys/dev/netif/ath/ath/if_ath.c
sys/dev/netif/ath/ath/if_ath_pci.c
sys/dev/netif/ath/hal/ath_hal/ar5210/ar5210_xmit.c
sys/dev/netif/ath/hal/ath_hal/ar5211/ar5211_xmit.c
sys/dev/netif/ath/hal/ath_hal/ar5212/ar5212_xmit.c

index eb5a9e2..279fc7c 100644 (file)
@@ -1285,6 +1285,7 @@ ath_intr(void *arg)
        struct ifnet *ifp = sc->sc_ifp;
        struct ath_hal *ah = sc->sc_ah;
        HAL_INT status;
+       HAL_INT ostatus;
 
        if (sc->sc_invalid) {
                /*
@@ -1294,6 +1295,7 @@ ath_intr(void *arg)
                DPRINTF(sc, ATH_DEBUG_ANY, "%s: invalid; ignored\n", __func__);
                return;
        }
+
        if (!ath_hal_intrpend(ah))              /* shared irq, not for us */
                return;
        if ((ifp->if_flags & IFF_UP) == 0 ||
@@ -1312,9 +1314,9 @@ ath_intr(void *arg)
         * bits we haven't explicitly enabled so we mask the
         * value to insure we only process bits we requested.
         */
-       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 */
+       ath_hal_getisr(ah, &ostatus);           /* NB: clears ISR too */
+       DPRINTF(sc, ATH_DEBUG_INTR, "%s: status 0x%x\n", __func__, ostatus);
+       status = ostatus & 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 */
@@ -1352,28 +1354,34 @@ ath_intr(void *arg)
 #endif
                        }
                }
+
+               /*
+                * NB: The hardware should re-read the link when the RXE
+                *     bit is written, but it doesn't work at least on
+                *     older chipsets.
+                */
                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)
                        taskqueue_enqueue(sc->sc_tq, &sc->sc_rxtask);
+
                if (status & HAL_INT_TX)
                        taskqueue_enqueue(sc->sc_tq, &sc->sc_txtask);
+
                if (status & HAL_INT_BMISS) {
                        sc->sc_stats.ast_bmiss++;
                        taskqueue_enqueue(sc->sc_tq, &sc->sc_bmisstask);
                }
+
                if (status & HAL_INT_MIB) {
                        sc->sc_stats.ast_mib++;
                        /*
@@ -1388,6 +1396,7 @@ ath_intr(void *arg)
                        ath_hal_mibevent(ah, &sc->sc_halstats);
                        ath_hal_intrset(ah, sc->sc_imask);
                }
+
                if (status & HAL_INT_RXORN) {
                        /* NB: hal marks HAL_INT_FATAL when RXORN is fatal */
                        sc->sc_stats.ast_rxorn++;
@@ -1726,6 +1735,7 @@ _ath_getbuf_locked(struct ath_softc *sc)
        else
                bf = NULL;
        if (bf == NULL) {
+               kprintf("ath: ran out of descriptors\n");
                DPRINTF(sc, ATH_DEBUG_XMIT, "%s: %s\n", __func__,
                    STAILQ_FIRST(&sc->sc_txbuf) == NULL ?
                        "out of xmit buffers" : "xmit buffer busy");
@@ -2781,7 +2791,8 @@ static void
 ath_txqmove(struct ath_txq *dst, struct ath_txq *src)
 {
        STAILQ_CONCAT(&dst->axq_q, &src->axq_q);
-       dst->axq_link = src->axq_link;
+       if (src->axq_depth)
+               dst->axq_link = src->axq_link;
        src->axq_link = NULL;
        dst->axq_depth += src->axq_depth;
        src->axq_depth = 0;
@@ -2827,6 +2838,15 @@ ath_beacon_proc(void *arg, int pending)
                sc->sc_bmisscount = 0;
        }
 
+       /*
+        * Stop any current dma before messing with the beacon linkages.
+        */
+       if (!ath_hal_stoptxdma(ah, sc->sc_bhalq)) {
+               DPRINTF(sc, ATH_DEBUG_ANY,
+                       "%s: beacon queue %u did not stop?\n",
+                       __func__, sc->sc_bhalq);
+       }
+
        if (sc->sc_stagbeacons) {                       /* staggered beacons */
                struct ieee80211com *ic = sc->sc_ifp->if_l2com;
                uint32_t tsftu;
@@ -2885,22 +2905,12 @@ ath_beacon_proc(void *arg, int pending)
        }
 
        if (bfaddr != 0) {
-               /*
-                * Stop any current dma and put the new frame on the queue.
-                * This should never fail since we check above that no frames
-                * are still pending on the queue.
-                */
-               if (!ath_hal_stoptxdma(ah, sc->sc_bhalq)) {
-                       DPRINTF(sc, ATH_DEBUG_ANY,
-                               "%s: beacon queue %u did not stop?\n",
-                               __func__, sc->sc_bhalq);
-               }
                /* NB: cabq traffic should already be queued and primed */
                ath_hal_puttxbuf(ah, sc->sc_bhalq, bfaddr);
-               ath_hal_txstart(ah, sc->sc_bhalq);
-
                sc->sc_stats.ast_be_xmit++;
+               ath_hal_txstart(ah, sc->sc_bhalq);
        }
+       /* else no beacon will be generated */
 }
 
 static struct ath_buf *
@@ -2967,17 +2977,31 @@ ath_beacon_generate(struct ath_softc *sc, struct ieee80211vap *vap)
                /* NB: only at DTIM */
                if (nmcastq) {
                        struct ath_buf *bfm;
+                       int qbusy;
 
                        /*
                         * Move frames from the s/w mcast q to the h/w cab q.
                         * XXX MORE_DATA bit
                         */
                        bfm = STAILQ_FIRST(&avp->av_mcastq.axq_q);
-                       if (cabq->axq_link != NULL) {
-                               *cabq->axq_link = bfm->bf_daddr;
-                       } else
-                               ath_hal_puttxbuf(ah, cabq->axq_qnum,
-                                       bfm->bf_daddr);
+                       qbusy = ath_hal_txqenabled(ah, cabq->axq_qnum);
+                       if (qbusy == 0) {
+                               if (cabq->axq_link != NULL) {
+                                       cpu_sfence();
+                                       *cabq->axq_link = bfm->bf_daddr;
+                                       cabq->axq_flags |= ATH_TXQ_PUTPENDING;
+                               } else {
+                                       cpu_sfence();
+                                       ath_hal_puttxbuf(ah, cabq->axq_qnum,
+                                               bfm->bf_daddr);
+                               }
+                       } else {
+                               if (cabq->axq_link != NULL) {
+                                       cpu_sfence();
+                                       *cabq->axq_link = bfm->bf_daddr;
+                               }
+                               cabq->axq_flags |= ATH_TXQ_PUTPENDING;
+                       }
                        ath_txqmove(cabq, &avp->av_mcastq);
 
                        sc->sc_stats.ast_cabq_xmit += nmcastq;
@@ -3539,8 +3563,9 @@ ath_rxbuf_init(struct ath_softc *sc, struct ath_buf *bf)
                 * multiple of the cache line size.  Not doing this
                 * causes weird stuff to happen (for the 5210 at least).
                 */
-               m = m_getcl(MB_DONTWAIT, MT_DATA, M_PKTHDR);
+               m = m_getcl(MB_WAIT, MT_DATA, M_PKTHDR);
                if (m == NULL) {
+                       kprintf("ath_rxbuf_init: no mbuf\n");
                        DPRINTF(sc, ATH_DEBUG_ANY,
                                "%s: no mbuf/cluster\n", __func__);
                        sc->sc_stats.ast_rx_nombuf++;
@@ -4361,59 +4386,60 @@ ath_tx_handoff(struct ath_softc *sc, struct ath_txq *txq, struct ath_buf *bf)
             ("busy status 0x%x", bf->bf_flags));
        if (txq->axq_qnum != ATH_TXQ_SWQ) {
 #ifdef IEEE80211_SUPPORT_TDMA
+               /*
+                * Supporting transmit dma.  If the queue is busy it is
+                * impossible to determine if we've won the race against
+                * the chipset checking the link field or not, so we don't
+                * try.  Instead we let the TX interrupt detect the case
+                * and restart the transmitter.
+                *
+                * If the queue is not busy we can start things rolling
+                * right here.
+                */
                int qbusy;
 
                ATH_TXQ_INSERT_TAIL(txq, bf, bf_list);
                qbusy = ath_hal_txqenabled(ah, txq->axq_qnum);
-               if (txq->axq_link == NULL) {
-                       /*
-                        * Be careful writing the address to TXDP.  If
-                        * the tx q is enabled then this write will be
-                        * ignored.  Normally this is not an issue but
-                        * when tdma is in use and the q is beacon gated
-                        * this race can occur.  If the q is busy then
-                        * defer the work to later--either when another
-                        * packet comes along or when we prepare a beacon
-                        * frame at SWBA.
-                        */
-                       if (!qbusy) {
-                               ath_hal_puttxbuf(ah, txq->axq_qnum, bf->bf_daddr);
-                               txq->axq_flags &= ~ATH_TXQ_PUTPENDING;
-                               DPRINTF(sc, ATH_DEBUG_XMIT,
-                                   "%s: TXDP[%u] = %p (%p) depth %d\n",
-                                   __func__, txq->axq_qnum,
-                                   (caddr_t)bf->bf_daddr, bf->bf_desc,
-                                   txq->axq_depth);
-                       } else {
+
+               if (qbusy == 0) {
+                       if (txq->axq_link != NULL) {
+                               /*
+                                * We had already started one previously but
+                                * not yet processed the TX interrupt.  Don't
+                                * try to race a restart because we do not
+                                * know where it stopped, let the TX interrupt
+                                * restart us when it figures out where we
+                                * stopped.
+                                */
+                               cpu_sfence();
+                               *txq->axq_link = bf->bf_daddr;
                                txq->axq_flags |= ATH_TXQ_PUTPENDING;
-                               DPRINTF(sc, ATH_DEBUG_TDMA | ATH_DEBUG_XMIT,
-                                   "%s: Q%u busy, defer enable\n", __func__,
-                                   txq->axq_qnum);
-                       }
-               } else {
-                       *txq->axq_link = bf->bf_daddr;
-                       DPRINTF(sc, ATH_DEBUG_XMIT,
-                           "%s: link[%u](%p)=%p (%p) depth %d\n", __func__,
-                           txq->axq_qnum, txq->axq_link,
-                           (caddr_t)bf->bf_daddr, bf->bf_desc, txq->axq_depth);
-                       if ((txq->axq_flags & ATH_TXQ_PUTPENDING) && !qbusy) {
+                       } else {
                                /*
-                                * The q was busy when we previously tried
-                                * to write the address of the first buffer
-                                * in the chain.  Since it's not busy now
-                                * handle this chore.  We are certain the
-                                * buffer at the front is the right one since
-                                * axq_link is NULL only when the buffer list
-                                * is/was empty.
+                                * We are first in line, we can safely start
+                                * at this address.
                                 */
+                               cpu_sfence();
                                ath_hal_puttxbuf(ah, txq->axq_qnum,
-                                       STAILQ_FIRST(&txq->axq_q)->bf_daddr);
-                               txq->axq_flags &= ~ATH_TXQ_PUTPENDING;
-                               DPRINTF(sc, ATH_DEBUG_TDMA | ATH_DEBUG_XMIT,
-                                   "%s: Q%u restarted\n", __func__,
-                                   txq->axq_qnum);
+                                                bf->bf_daddr);
                        }
+               } else {
+                       /*
+                        * The queue is busy, go ahead and link us in but
+                        * do not try to start/restart the tx.  We just
+                        * don't know whether it will pick up our link
+                        * or not and we don't want to double-xmit.
+                        */
+                       if (txq->axq_link != NULL) {
+                               cpu_sfence();
+                               *txq->axq_link = bf->bf_daddr;
+                       }
+                       txq->axq_flags |= ATH_TXQ_PUTPENDING;
                }
+#if 0
+                               ath_hal_puttxbuf(ah, txq->axq_qnum,
+                                       STAILQ_FIRST(&txq->axq_q)->bf_daddr);
+#endif
 #else
                ATH_TXQ_INSERT_TAIL(txq, bf, bf_list);
                if (txq->axq_link == NULL) {
@@ -4886,6 +4912,8 @@ ath_tx_processq(struct ath_softc *sc, struct ath_txq *txq)
                txq->axq_link);
        nacked = 0;
        for (;;) {
+               int qbusy;
+
                txq->axq_intrcnt = 0;   /* reset periodic desc intr count */
                bf = STAILQ_FIRST(&txq->axq_q);
                if (bf == NULL)
@@ -4893,14 +4921,29 @@ ath_tx_processq(struct ath_softc *sc, struct ath_txq *txq)
                ds0 = &bf->bf_desc[0];
                ds = &bf->bf_desc[bf->bf_nseg - 1];
                ts = &bf->bf_status.ds_txstat;
+               qbusy = ath_hal_txqenabled(ah, txq->axq_qnum);
                status = ath_hal_txprocdesc(ah, ds, ts);
 #ifdef ATH_DEBUG
                if (sc->sc_debug & ATH_DEBUG_XMIT_DESC)
                        ath_printtxbuf(sc, bf, txq->axq_qnum, 0,
                            status == HAL_OK);
 #endif
-               if (status == HAL_EINPROGRESS)
+               if (status == HAL_EINPROGRESS) {
+#ifdef IEEE80211_SUPPORT_TDMA
+                       /*
+                        * If not done and the queue is not busy then the
+                        * transmitter raced the hardware on the link field
+                        * and we have to restart it.
+                        */
+                       if (!qbusy) {
+                               cpu_sfence();
+                               ath_hal_puttxbuf(ah, txq->axq_qnum,
+                                                bf->bf_daddr);
+                               ath_hal_txstart(ah, txq->axq_qnum);
+                       }
+#endif
                        break;
+               }
                ATH_TXQ_REMOVE_HEAD(txq, bf_list);
 #ifdef IEEE80211_SUPPORT_TDMA
                if (txq->axq_depth > 0) {
@@ -7345,19 +7388,20 @@ ath_tdma_beacon_send(struct ath_softc *sc, struct ieee80211vap *vap)
                sc->sc_ant_tx[1] = sc->sc_ant_tx[2] = 0;
        }
 
+       /*
+        * Stop any current dma before messing with the beacon linkages.
+        *
+        * This should never fail since we check above that no frames
+        * are still pending on the queue.
+        */
+       if (!ath_hal_stoptxdma(ah, sc->sc_bhalq)) {
+               DPRINTF(sc, ATH_DEBUG_ANY,
+                       "%s: beacon queue %u did not stop?\n",
+                       __func__, sc->sc_bhalq);
+               /* NB: the HAL still stops DMA, so proceed */
+       }
        bf = ath_beacon_generate(sc, vap);
        if (bf != NULL) {
-               /*
-                * Stop any current dma and put the new frame on the queue.
-                * This should never fail since we check above that no frames
-                * are still pending on the queue.
-                */
-               if (!ath_hal_stoptxdma(ah, sc->sc_bhalq)) {
-                       DPRINTF(sc, ATH_DEBUG_ANY,
-                               "%s: beacon queue %u did not stop?\n",
-                               __func__, sc->sc_bhalq);
-                       /* NB: the HAL still stops DMA, so proceed */
-               }
                ath_hal_puttxbuf(ah, sc->sc_bhalq, bf->bf_daddr);
                ath_hal_txstart(ah, sc->sc_bhalq);
 
@@ -7368,6 +7412,8 @@ ath_tdma_beacon_send(struct ath_softc *sc, struct ieee80211vap *vap)
                 * in arbitrating slot collisions.
                 */
                vap->iv_bss->ni_tstamp.tsf = ath_hal_gettsf64(ah);
+       } else {
+               device_printf(sc->sc_dev, "tdma beacon gen failed!\n");
        }
 }
 #endif /* IEEE80211_SUPPORT_TDMA */
index 644debd..c3d87e5 100644 (file)
@@ -152,7 +152,7 @@ ath_pci_attach(device_t dev)
         * Setup DMA descriptor area.
         */
        if (bus_dma_tag_create(sc->sc_dmat,     /* parent */
-                              1, 0,                    /* alignment, bounds */
+                              4, 0,                    /* alignment, bounds */
                               BUS_SPACE_MAXADDR_32BIT, /* lowaddr */
                               BUS_SPACE_MAXADDR,       /* highaddr */
                               NULL, NULL,              /* filter, filterarg */
index 1f1b1ae..dd2f62c 100644 (file)
@@ -372,6 +372,7 @@ ar5210StartTxDma(struct ath_hal *ah, u_int q)
 
        HALASSERT(q < HAL_NUM_TX_QUEUES);
 
+       cpu_sfence();
        HALDEBUG(ah, HAL_DEBUG_TXQUEUE, "%s: queue %u\n", __func__, q);
        qi = &ahp->ah_txq[q];
        switch (qi->tqi_type) {
index 7dd2eb2..d578f13 100644 (file)
@@ -435,6 +435,7 @@ ar5211StartTxDma(struct ath_hal *ah, u_int q)
        HALASSERT(q < HAL_NUM_TX_QUEUES);
        HALASSERT(AH5211(ah)->ah_txq[q].tqi_type != HAL_TX_QUEUE_INACTIVE);
 
+       cpu_sfence();
        /* Check that queue is not already active */
        HALASSERT((OS_REG_READ(ah, AR_Q_TXD) & (1<<q)) == 0);
 
index 7461ade..0533987 100644 (file)
@@ -526,6 +526,7 @@ ar5212StartTxDma(struct ath_hal *ah, u_int q)
 
        HALDEBUG(ah, HAL_DEBUG_TXQUEUE, "%s: queue %u\n", __func__, q);
 
+       cpu_sfence();
        /* Check to be sure we're not enabling a q that has its TXD bit set. */
        HALASSERT((OS_REG_READ(ah, AR_Q_TXD) & (1 << q)) == 0);
 
@@ -859,6 +860,7 @@ ar5212ProcTxDesc(struct ath_hal *ah,
        ts->ts_tstamp = MS(ads->ds_txstatus0, AR_SendTimestamp);
        ts->ts_status = 0;
        if ((ads->ds_txstatus0 & AR_FrmXmitOK) == 0) {
+               kprintf("bad status on tx queue\n");
                if (ads->ds_txstatus0 & AR_ExcessiveRetries)
                        ts->ts_status |= HAL_TXERR_XRETRY;
                if (ads->ds_txstatus0 & AR_Filtered)