2011-03-10 13:41:45

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH v2 1/4] ath9k_hw: fix REG_SET_BIT and REG_CLR_BIT for multiple bits

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/hw.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index ef79f4c..6650fd4 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -95,9 +95,9 @@
#define REG_READ_FIELD(_a, _r, _f) \
(((REG_READ(_a, _r) & _f) >> _f##_S))
#define REG_SET_BIT(_a, _r, _f) \
- REG_WRITE(_a, _r, REG_READ(_a, _r) | _f)
+ REG_WRITE(_a, _r, REG_READ(_a, _r) | (_f))
#define REG_CLR_BIT(_a, _r, _f) \
- REG_WRITE(_a, _r, REG_READ(_a, _r) & ~_f)
+ REG_WRITE(_a, _r, REG_READ(_a, _r) & ~(_f))

#define DO_DELAY(x) do { \
if ((++(x) % 64) == 0) \
--
1.7.3.2



2011-03-10 21:55:22

by Mark Mentovai

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] ath9k: fix the .flush driver op implementation

Felix Fietkau wrote:
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
[...]
> ?static void ath9k_flush(struct ieee80211_hw *hw, bool drop)
> ?{
> ? ? ? ?struct ath_softc *sc = hw->priv;
> + ? ? ? int timeout = 200; /* ms */
> + ? ? ? int i, j;
>
> + ? ? ? ath9k_ps_wakeup(sc);
> ? ? ? ?mutex_lock(&sc->mutex);
>
> ? ? ? ?cancel_delayed_work_sync(&sc->tx_complete_work);
>
> + ? ? ? if (drop)
> + ? ? ? ? ? ? ? timeout = 1;
>
> + ? ? ? for (j = 0; j < timeout; j++) {
> + ? ? ? ? ? ? ? int npend = 0;
> + ? ? ? ? ? ? ? for (i = 0; i < ATH9K_NUM_TX_QUEUES; i++) {
> + ? ? ? ? ? ? ? ? ? ? ? if (!ATH_TXQ_SETUP(sc, i))
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? continue;
>
> + ? ? ? ? ? ? ? ? ? ? ? npend += ath9k_has_pending_frames(sc, &sc->tx.txq[i]);
> ? ? ? ? ? ? ? ?}
> +
> + ? ? ? ? ? ? ? if (!npend)
> + ? ? ? ? ? ? ? ? ? goto out;
> +
> + ? ? ? ? ? ? ? usleep_range(1000, 2000);
> ? ? ? ?}
>
> + ? ? ? if (!ath_drain_all_txq(sc, false))
> ? ? ? ? ? ? ? ?ath_reset(sc, false);
>
> +out:
> ? ? ? ?ieee80211_queue_delayed_work(hw, &sc->tx_complete_work, 0);
> ? ? ? ?mutex_unlock(&sc->mutex);
> + ? ? ? ath9k_ps_restore(sc);
> ?}

My only comment here is that you still hit the sleep even on the last
pass through the loop when it isn?t strictly necessary. Practically,
the only place this would be realized is the case where |drop| is set.
In this scenario, the code formerly didn?t sleep at all, but now can
sleep for 1-2ms. I couldn?t find any calls to drv_flush with |drop|
set, though, so it might just be an academic concern.

The delay loops in v2 patches 2/4 and 4/4 also share this
characteristic (although with shorter timeouts, and delays instead of
sleeps). Have you considered restructuring these loops to avoid the
final waits?

This is admittedly a pretty minor thing.

2011-03-10 22:46:41

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] ath9k: fix the .flush driver op implementation

On 2011-03-10 10:55 PM, Mark Mentovai wrote:
> Felix Fietkau wrote:
>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> [...]
>> static void ath9k_flush(struct ieee80211_hw *hw, bool drop)
>> {
>> struct ath_softc *sc = hw->priv;
>> + int timeout = 200; /* ms */
>> + int i, j;
>>
>> + ath9k_ps_wakeup(sc);
>> mutex_lock(&sc->mutex);
>>
>> cancel_delayed_work_sync(&sc->tx_complete_work);
>>
>> + if (drop)
>> + timeout = 1;
>>
>> + for (j = 0; j < timeout; j++) {
>> + int npend = 0;
>> + for (i = 0; i < ATH9K_NUM_TX_QUEUES; i++) {
>> + if (!ATH_TXQ_SETUP(sc, i))
>> + continue;
>>
>> + npend += ath9k_has_pending_frames(sc, &sc->tx.txq[i]);
>> }
>> +
>> + if (!npend)
>> + goto out;
>> +
>> + usleep_range(1000, 2000);
>> }
>>
>> + if (!ath_drain_all_txq(sc, false))
>> ath_reset(sc, false);
>>
>> +out:
>> ieee80211_queue_delayed_work(hw, &sc->tx_complete_work, 0);
>> mutex_unlock(&sc->mutex);
>> + ath9k_ps_restore(sc);
>> }
>
> My only comment here is that you still hit the sleep even on the last
> pass through the loop when it isn?t strictly necessary. Practically,
> the only place this would be realized is the case where |drop| is set.
> In this scenario, the code formerly didn?t sleep at all, but now can
> sleep for 1-2ms. I couldn?t find any calls to drv_flush with |drop|
> set, though, so it might just be an academic concern.
>
> The delay loops in v2 patches 2/4 and 4/4 also share this
> characteristic (although with shorter timeouts, and delays instead of
> sleeps). Have you considered restructuring these loops to avoid the
> final waits?
>
> This is admittedly a pretty minor thing.
Makes sense, I'll send a v3.

- Felix

2011-03-10 22:13:46

by Felix Fietkau

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] ath9k: improve reliability of beacon transmission and stuck beacon handling

On 2011-03-10 10:55 PM, Mark Mentovai wrote:
> Felix Fietkau wrote:
>> diff --git a/drivers/net/wireless/ath/ath9k/mac.c b/drivers/net/wireless/ath/ath9k/mac.c
> [...]
>> +bool ath9k_hw_stop_dma_queue(struct ath_hw *ah, u32 q)
> [...]
>> - ath_err(common,
>> - "Failed to stop TX DMA in 100 msec after killing last frame\n");
>
> Are you concerned about getting rid of this error that
> ath9k_hw_stoptxdma used to print? It doesn?t seem like anything else
> in the remaining code that uses it (ath_beacon_tasklet and
> ath9k_set_beaconing_status) will produce an error.
The error message is in a code path that never triggered (or at least
never should have) for the beacon code, based on the fact that it was
only supposed to be called when the number of pending frames in the
queue was already 0.
I removed the entire block of code because attempting to stop DMA by
messing with the queue parameters and doing busy-waiting is completely
bogus and counterproductive for anything beacon related.

> Based on the message you sent with patch 2/4, ?I can no longer trigger
> these messages on AR9380, and on AR9280 they become much more rare,?
> and the fact that you left the message intact in that patch?s
> ath_drain_all_txq, I wonder it?s appropriate to remove the message
> from this function.
Anything beacon related will report stuck beacons if frames stay in the
queue until the next interval, so there is some degree of reporting
there already.

- Felix

2011-03-10 22:38:08

by Mark Mentovai

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] ath9k: improve reliability of beacon transmission and stuck beacon handling

Felix Fietkau wrote:
> On 2011-03-10 10:55 PM, Mark Mentovai wrote:
>> Based on the message you sent with patch 2/4, ?I can no longer trigger
>> these messages on AR9380, and on AR9280 they become much more rare,?
>> and the fact that you left the message intact in that patch?s
>> ath_drain_all_txq, I wonder it?s appropriate to remove the message
>> from this function.
> Anything beacon related will report stuck beacons if frames stay in the
> queue until the next interval, so there is some degree of reporting
> there already.

OK, that makes sense. The stuck-beacon logging is ath_dbg, but that?s
probably enough given what ath_beacon_tasklet tries to do to recover
from a stuck beacon. I think I just get antsy when I see that
ath_reset might be called quietly, because it?s a big hammer.

No Big Deal.

2011-03-10 13:41:46

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH v2 4/4] ath9k: improve reliability of beacon transmission and stuck beacon handling

ath9k calls ath9k_hw_stoptxdma every time it sends a beacon, however there
is not much point in doing that if the previous beacon and mcast traffic
went out properly. On AR9380, calling that function too often can result
in an increase of stuck beacons due to differences in the handling of the
queue enable/disable functionality.

With this patch, the queue will only be explicitly stopped if the previous
data frames were not sent successfully. With the beacon code being the
only remaining user of ath9k_hw_stoptxdma, this function can be simplified
in order to remove the now pointless attempts at waiting for transmission
completion, which would never happen at this point due to the different
method of tx scheduling of the beacon queue.

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/beacon.c | 13 +-----
drivers/net/wireless/ath/ath9k/mac.c | 68 +++---------------------------
drivers/net/wireless/ath/ath9k/mac.h | 2 +-
3 files changed, 10 insertions(+), 73 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
index a4bdfdb..6d2a545f 100644
--- a/drivers/net/wireless/ath/ath9k/beacon.c
+++ b/drivers/net/wireless/ath/ath9k/beacon.c
@@ -373,6 +373,7 @@ void ath_beacon_tasklet(unsigned long data)
ath_dbg(common, ATH_DBG_BSTUCK,
"missed %u consecutive beacons\n",
sc->beacon.bmisscnt);
+ ath9k_hw_stop_dma_queue(ah, sc->beacon.beaconq);
ath9k_hw_bstuck_nfcal(ah);
} else if (sc->beacon.bmisscnt >= BSTUCK_THRESH) {
ath_dbg(common, ATH_DBG_BSTUCK,
@@ -450,16 +451,6 @@ void ath_beacon_tasklet(unsigned long data)
sc->beacon.updateslot = OK;
}
if (bfaddr != 0) {
- /*
- * Stop any current dma and put the new frame(s) on the queue.
- * This should never fail since we check above that no frames
- * are still pending on the queue.
- */
- if (!ath9k_hw_stoptxdma(ah, sc->beacon.beaconq)) {
- ath_err(common, "beacon queue %u did not stop?\n",
- sc->beacon.beaconq);
- }
-
/* NB: cabq traffic should already be queued and primed */
ath9k_hw_puttxbuf(ah, sc->beacon.beaconq, bfaddr);
ath9k_hw_txstart(ah, sc->beacon.beaconq);
@@ -780,7 +771,7 @@ void ath9k_set_beaconing_status(struct ath_softc *sc, bool status)
ah->imask &= ~ATH9K_INT_SWBA;
ath9k_hw_set_interrupts(ah, ah->imask);
tasklet_kill(&sc->bcon_tasklet);
- ath9k_hw_stoptxdma(ah, sc->beacon.beaconq);
+ ath9k_hw_stop_dma_queue(ah, sc->beacon.beaconq);
}
ath9k_ps_restore(sc);
}
diff --git a/drivers/net/wireless/ath/ath9k/mac.c b/drivers/net/wireless/ath/ath9k/mac.c
index 188af17..0a734f1 100644
--- a/drivers/net/wireless/ath/ath9k/mac.c
+++ b/drivers/net/wireless/ath/ath9k/mac.c
@@ -170,84 +170,30 @@ void ath9k_hw_abort_tx_dma(struct ath_hw *ah)
}
EXPORT_SYMBOL(ath9k_hw_abort_tx_dma);

-bool ath9k_hw_stoptxdma(struct ath_hw *ah, u32 q)
+bool ath9k_hw_stop_dma_queue(struct ath_hw *ah, u32 q)
{
-#define ATH9K_TX_STOP_DMA_TIMEOUT 4000 /* usec */
+#define ATH9K_TX_STOP_DMA_TIMEOUT 1000 /* usec */
#define ATH9K_TIME_QUANTUM 100 /* usec */
- struct ath_common *common = ath9k_hw_common(ah);
- struct ath9k_hw_capabilities *pCap = &ah->caps;
- struct ath9k_tx_queue_info *qi;
- u32 tsfLow, j, wait;
- u32 wait_time = ATH9K_TX_STOP_DMA_TIMEOUT / ATH9K_TIME_QUANTUM;
-
- if (q >= pCap->total_queues) {
- ath_dbg(common, ATH_DBG_QUEUE,
- "Stopping TX DMA, invalid queue: %u\n", q);
- return false;
- }
-
- qi = &ah->txq[q];
- if (qi->tqi_type == ATH9K_TX_QUEUE_INACTIVE) {
- ath_dbg(common, ATH_DBG_QUEUE,
- "Stopping TX DMA, inactive queue: %u\n", q);
- return false;
- }
+ int wait_time = ATH9K_TX_STOP_DMA_TIMEOUT / ATH9K_TIME_QUANTUM;
+ int wait;

REG_WRITE(ah, AR_Q_TXD, 1 << q);

for (wait = wait_time; wait != 0; wait--) {
if (ath9k_hw_numtxpending(ah, q) == 0)
break;
- udelay(ATH9K_TIME_QUANTUM);
- }
-
- if (ath9k_hw_numtxpending(ah, q)) {
- ath_dbg(common, ATH_DBG_QUEUE,
- "%s: Num of pending TX Frames %d on Q %d\n",
- __func__, ath9k_hw_numtxpending(ah, q), q);
-
- for (j = 0; j < 2; j++) {
- tsfLow = REG_READ(ah, AR_TSF_L32);
- REG_WRITE(ah, AR_QUIET2,
- SM(10, AR_QUIET2_QUIET_DUR));
- REG_WRITE(ah, AR_QUIET_PERIOD, 100);
- REG_WRITE(ah, AR_NEXT_QUIET_TIMER, tsfLow >> 10);
- REG_SET_BIT(ah, AR_TIMER_MODE,
- AR_QUIET_TIMER_EN);
-
- if ((REG_READ(ah, AR_TSF_L32) >> 10) == (tsfLow >> 10))
- break;
-
- ath_dbg(common, ATH_DBG_QUEUE,
- "TSF has moved while trying to set quiet time TSF: 0x%08x\n",
- tsfLow);
- }
-
- REG_SET_BIT(ah, AR_DIAG_SW, AR_DIAG_FORCE_CH_IDLE_HIGH);

- udelay(200);
- REG_CLR_BIT(ah, AR_TIMER_MODE, AR_QUIET_TIMER_EN);
-
- wait = wait_time;
- while (ath9k_hw_numtxpending(ah, q)) {
- if ((--wait) == 0) {
- ath_err(common,
- "Failed to stop TX DMA in 100 msec after killing last frame\n");
- break;
- }
- udelay(ATH9K_TIME_QUANTUM);
- }
-
- REG_CLR_BIT(ah, AR_DIAG_SW, AR_DIAG_FORCE_CH_IDLE_HIGH);
+ udelay(ATH9K_TIME_QUANTUM);
}

REG_WRITE(ah, AR_Q_TXD, 0);
+
return wait != 0;

#undef ATH9K_TX_STOP_DMA_TIMEOUT
#undef ATH9K_TIME_QUANTUM
}
-EXPORT_SYMBOL(ath9k_hw_stoptxdma);
+EXPORT_SYMBOL(ath9k_hw_stop_dma_queue);

void ath9k_hw_gettxintrtxqs(struct ath_hw *ah, u32 *txqs)
{
diff --git a/drivers/net/wireless/ath/ath9k/mac.h b/drivers/net/wireless/ath/ath9k/mac.h
index d37c6d4..b2b2ff8 100644
--- a/drivers/net/wireless/ath/ath9k/mac.h
+++ b/drivers/net/wireless/ath/ath9k/mac.h
@@ -676,7 +676,7 @@ void ath9k_hw_txstart(struct ath_hw *ah, u32 q);
void ath9k_hw_cleartxdesc(struct ath_hw *ah, void *ds);
u32 ath9k_hw_numtxpending(struct ath_hw *ah, u32 q);
bool ath9k_hw_updatetxtriglevel(struct ath_hw *ah, bool bIncTrigLevel);
-bool ath9k_hw_stoptxdma(struct ath_hw *ah, u32 q);
+bool ath9k_hw_stop_dma_queue(struct ath_hw *ah, u32 q);
void ath9k_hw_abort_tx_dma(struct ath_hw *ah);
void ath9k_hw_gettxintrtxqs(struct ath_hw *ah, u32 *txqs);
bool ath9k_hw_set_txq_props(struct ath_hw *ah, int q,
--
1.7.3.2


2011-03-10 21:56:18

by Mark Mentovai

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] ath9k: improve reliability of beacon transmission and stuck beacon handling

Felix Fietkau wrote:
> diff --git a/drivers/net/wireless/ath/ath9k/mac.c b/drivers/net/wireless/ath/ath9k/mac.c
[...]
> +bool ath9k_hw_stop_dma_queue(struct ath_hw *ah, u32 q)
[...]
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ath_err(common,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Failed to stop TX DMA in 100 msec after killing last frame\n");

Are you concerned about getting rid of this error that
ath9k_hw_stoptxdma used to print? It doesn?t seem like anything else
in the remaining code that uses it (ath_beacon_tasklet and
ath9k_set_beaconing_status) will produce an error.

Based on the message you sent with patch 2/4, ?I can no longer trigger
these messages on AR9380, and on AR9280 they become much more rare,?
and the fact that you left the message intact in that patch?s
ath_drain_all_txq, I wonder it?s appropriate to remove the message
from this function.

2011-03-10 13:41:46

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH v2 3/4] ath9k: fix the .flush driver op implementation

This patch simplifies the flush op and reuses ath_drain_all_txq for
flushing out pending frames if necessary. It also uses a global timeout
of 200ms instead of the per-queue 60ms timeout.

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 1 -
drivers/net/wireless/ath/ath9k/main.c | 54 +++++++++++--------------------
drivers/net/wireless/ath/ath9k/xmit.c | 28 ++++++++---------
3 files changed, 32 insertions(+), 51 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index c718ab5..099bd41 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -189,7 +189,6 @@ struct ath_txq {
u32 axq_ampdu_depth;
bool stopped;
bool axq_tx_inprogress;
- bool txq_flush_inprogress;
struct list_head axq_acq;
struct list_head txq_fifo[ATH_TXFIFO_DEPTH];
struct list_head txq_fifo_pending;
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 2e228aa..903f2a4 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -2128,56 +2128,40 @@ static void ath9k_set_coverage_class(struct ieee80211_hw *hw, u8 coverage_class)

static void ath9k_flush(struct ieee80211_hw *hw, bool drop)
{
-#define ATH_FLUSH_TIMEOUT 60 /* ms */
struct ath_softc *sc = hw->priv;
- struct ath_txq *txq = NULL;
- struct ath_hw *ah = sc->sc_ah;
- struct ath_common *common = ath9k_hw_common(ah);
- int i, j, npend = 0;
+ int timeout = 200; /* ms */
+ int i, j;

+ ath9k_ps_wakeup(sc);
mutex_lock(&sc->mutex);

cancel_delayed_work_sync(&sc->tx_complete_work);

- for (i = 0; i < ATH9K_NUM_TX_QUEUES; i++) {
- if (!ATH_TXQ_SETUP(sc, i))
- continue;
- txq = &sc->tx.txq[i];
+ if (drop)
+ timeout = 1;

- if (!drop) {
- for (j = 0; j < ATH_FLUSH_TIMEOUT; j++) {
- if (!ath9k_has_pending_frames(sc, txq))
- break;
- usleep_range(1000, 2000);
- }
- }
+ for (j = 0; j < timeout; j++) {
+ int npend = 0;
+ for (i = 0; i < ATH9K_NUM_TX_QUEUES; i++) {
+ if (!ATH_TXQ_SETUP(sc, i))
+ continue;

- if (drop || ath9k_has_pending_frames(sc, txq)) {
- ath_dbg(common, ATH_DBG_QUEUE, "Drop frames from hw queue:%d\n",
- txq->axq_qnum);
- spin_lock_bh(&txq->axq_lock);
- txq->txq_flush_inprogress = true;
- spin_unlock_bh(&txq->axq_lock);
-
- ath9k_ps_wakeup(sc);
- ath9k_hw_stoptxdma(ah, txq->axq_qnum);
- npend = ath9k_hw_numtxpending(ah, txq->axq_qnum);
- ath9k_ps_restore(sc);
- if (npend)
- break;
-
- ath_draintxq(sc, txq, false);
- txq->txq_flush_inprogress = false;
+ npend += ath9k_has_pending_frames(sc, &sc->tx.txq[i]);
}
+
+ if (!npend)
+ goto out;
+
+ usleep_range(1000, 2000);
}

- if (npend) {
+ if (!ath_drain_all_txq(sc, false))
ath_reset(sc, false);
- txq->txq_flush_inprogress = false;
- }

+out:
ieee80211_queue_delayed_work(hw, &sc->tx_complete_work, 0);
mutex_unlock(&sc->mutex);
+ ath9k_ps_restore(sc);
}

struct ieee80211_ops ath9k_ops = {
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index bb1d29e..f977f80 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -2012,8 +2012,7 @@ static void ath_tx_processq(struct ath_softc *sc, struct ath_txq *txq)
spin_lock_bh(&txq->axq_lock);
if (list_empty(&txq->axq_q)) {
txq->axq_link = NULL;
- if (sc->sc_flags & SC_OP_TXAGGR &&
- !txq->txq_flush_inprogress)
+ if (sc->sc_flags & SC_OP_TXAGGR)
ath_txq_schedule(sc, txq);
spin_unlock_bh(&txq->axq_lock);
break;
@@ -2094,7 +2093,7 @@ static void ath_tx_processq(struct ath_softc *sc, struct ath_txq *txq)

spin_lock_bh(&txq->axq_lock);

- if (sc->sc_flags & SC_OP_TXAGGR && !txq->txq_flush_inprogress)
+ if (sc->sc_flags & SC_OP_TXAGGR)
ath_txq_schedule(sc, txq);
spin_unlock_bh(&txq->axq_lock);
}
@@ -2265,18 +2264,17 @@ void ath_tx_edma_tasklet(struct ath_softc *sc)

spin_lock_bh(&txq->axq_lock);

- if (!txq->txq_flush_inprogress) {
- if (!list_empty(&txq->txq_fifo_pending)) {
- INIT_LIST_HEAD(&bf_head);
- bf = list_first_entry(&txq->txq_fifo_pending,
- struct ath_buf, list);
- list_cut_position(&bf_head,
- &txq->txq_fifo_pending,
- &bf->bf_lastbf->list);
- ath_tx_txqaddbuf(sc, txq, &bf_head);
- } else if (sc->sc_flags & SC_OP_TXAGGR)
- ath_txq_schedule(sc, txq);
- }
+ if (!list_empty(&txq->txq_fifo_pending)) {
+ INIT_LIST_HEAD(&bf_head);
+ bf = list_first_entry(&txq->txq_fifo_pending,
+ struct ath_buf, list);
+ list_cut_position(&bf_head,
+ &txq->txq_fifo_pending,
+ &bf->bf_lastbf->list);
+ ath_tx_txqaddbuf(sc, txq, &bf_head);
+ } else if (sc->sc_flags & SC_OP_TXAGGR)
+ ath_txq_schedule(sc, txq);
+
spin_unlock_bh(&txq->axq_lock);
}
}
--
1.7.3.2


2011-03-10 13:41:46

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH v2 2/4] ath9k: fix stopping tx dma on reset

In some situations, stopping Tx DMA frequently fails, leading to messages
like this:

ath: Failed to stop TX DMA in 100 msec after killing last frame
ath: Failed to stop TX DMA!

This patch uses a few MAC features to abort DMA globally instead of iterating
over all hardware queues and attempting to stop them individually.
Not only is that faster and works with a shorter timeout, it also makes the
process much more reliable.

With this change, I can no longer trigger these messages on AR9380,
and on AR9280 they become much more rare.

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/mac.c | 27 +++++++++++++++++++++++++++
drivers/net/wireless/ath/ath9k/mac.h | 1 +
drivers/net/wireless/ath/ath9k/xmit.c | 14 ++++++--------
3 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/mac.c b/drivers/net/wireless/ath/ath9k/mac.c
index 5efc869..188af17 100644
--- a/drivers/net/wireless/ath/ath9k/mac.c
+++ b/drivers/net/wireless/ath/ath9k/mac.c
@@ -143,6 +143,33 @@ bool ath9k_hw_updatetxtriglevel(struct ath_hw *ah, bool bIncTrigLevel)
}
EXPORT_SYMBOL(ath9k_hw_updatetxtriglevel);

+void ath9k_hw_abort_tx_dma(struct ath_hw *ah)
+{
+ int i, q;
+
+ REG_WRITE(ah, AR_Q_TXD, AR_Q_TXD_M);
+
+ REG_SET_BIT(ah, AR_PCU_MISC, AR_PCU_FORCE_QUIET_COLL | AR_PCU_CLEAR_VMF);
+ REG_SET_BIT(ah, AR_DIAG_SW, AR_DIAG_FORCE_CH_IDLE_HIGH);
+ REG_SET_BIT(ah, AR_D_GBL_IFS_MISC, AR_D_GBL_IFS_MISC_IGNORE_BACKOFF);
+
+ for (q = 0; q < AR_NUM_QCU; q++) {
+ for (i = 1000; i > 0; i--) {
+ if (!ath9k_hw_numtxpending(ah, q))
+ break;
+
+ udelay(5);
+ }
+ }
+
+ REG_CLR_BIT(ah, AR_PCU_MISC, AR_PCU_FORCE_QUIET_COLL | AR_PCU_CLEAR_VMF);
+ REG_CLR_BIT(ah, AR_DIAG_SW, AR_DIAG_FORCE_CH_IDLE_HIGH);
+ REG_CLR_BIT(ah, AR_D_GBL_IFS_MISC, AR_D_GBL_IFS_MISC_IGNORE_BACKOFF);
+
+ REG_WRITE(ah, AR_Q_TXD, 0);
+}
+EXPORT_SYMBOL(ath9k_hw_abort_tx_dma);
+
bool ath9k_hw_stoptxdma(struct ath_hw *ah, u32 q)
{
#define ATH9K_TX_STOP_DMA_TIMEOUT 4000 /* usec */
diff --git a/drivers/net/wireless/ath/ath9k/mac.h b/drivers/net/wireless/ath/ath9k/mac.h
index 04d58ae..d37c6d4 100644
--- a/drivers/net/wireless/ath/ath9k/mac.h
+++ b/drivers/net/wireless/ath/ath9k/mac.h
@@ -677,6 +677,7 @@ void ath9k_hw_cleartxdesc(struct ath_hw *ah, void *ds);
u32 ath9k_hw_numtxpending(struct ath_hw *ah, u32 q);
bool ath9k_hw_updatetxtriglevel(struct ath_hw *ah, bool bIncTrigLevel);
bool ath9k_hw_stoptxdma(struct ath_hw *ah, u32 q);
+void ath9k_hw_abort_tx_dma(struct ath_hw *ah);
void ath9k_hw_gettxintrtxqs(struct ath_hw *ah, u32 *txqs);
bool ath9k_hw_set_txq_props(struct ath_hw *ah, int q,
const struct ath9k_tx_queue_info *qinfo);
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index e16136d..bb1d29e 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -1194,16 +1194,14 @@ bool ath_drain_all_txq(struct ath_softc *sc, bool retry_tx)
if (sc->sc_flags & SC_OP_INVALID)
return true;

- /* Stop beacon queue */
- ath9k_hw_stoptxdma(sc->sc_ah, sc->beacon.beaconq);
+ ath9k_hw_abort_tx_dma(ah);

- /* Stop data queues */
+ /* Check if any queue remains active */
for (i = 0; i < ATH9K_NUM_TX_QUEUES; i++) {
- if (ATH_TXQ_SETUP(sc, i)) {
- txq = &sc->tx.txq[i];
- ath9k_hw_stoptxdma(ah, txq->axq_qnum);
- npend += ath9k_hw_numtxpending(ah, txq->axq_qnum);
- }
+ if (!ATH_TXQ_SETUP(sc, i))
+ continue;
+
+ npend += ath9k_hw_numtxpending(ah, sc->tx.txq[i].axq_qnum);
}

if (npend)
--
1.7.3.2