When ath_drain_all_txq fails to stop DMA, it issues a hw reset. This reset
happens at a very problematic point in time, when the hardware rx path has
not been stopped yet. This could lead to memory corruption, hardware hangs
or other issues.
To fix these issues, simply remove the reset entirely and check the tx DMA
stop status to prevent problems with fast channel changes.
Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 2 +-
drivers/net/wireless/ath/ath9k/main.c | 5 +++--
drivers/net/wireless/ath/ath9k/xmit.c | 22 ++++++----------------
3 files changed, 10 insertions(+), 19 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 0d0bec3..0963071 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -329,7 +329,7 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp);
struct ath_txq *ath_txq_setup(struct ath_softc *sc, int qtype, int subtype);
void ath_tx_cleanupq(struct ath_softc *sc, struct ath_txq *txq);
int ath_tx_setup(struct ath_softc *sc, int haltype);
-void ath_drain_all_txq(struct ath_softc *sc, bool retry_tx);
+bool ath_drain_all_txq(struct ath_softc *sc, bool retry_tx);
void ath_draintxq(struct ath_softc *sc,
struct ath_txq *txq, bool retry_tx);
void ath_tx_node_init(struct ath_softc *sc, struct ath_node *an);
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index dace215..928ef68 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -244,11 +244,12 @@ int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw,
* the relevant bits of the h/w.
*/
ath9k_hw_set_interrupts(ah, 0);
- ath_drain_all_txq(sc, false);
+ stopped = ath_drain_all_txq(sc, false);
spin_lock_bh(&sc->rx.pcu_lock);
- stopped = ath_stoprecv(sc);
+ if (!ath_stoprecv(sc))
+ stopped = false;
/* XXX: do not flush receive queue here. We don't want
* to flush data frames already in queue because of
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index f2ade24..aff0478 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -1120,7 +1120,7 @@ void ath_draintxq(struct ath_softc *sc, struct ath_txq *txq, bool retry_tx)
}
}
-void ath_drain_all_txq(struct ath_softc *sc, bool retry_tx)
+bool ath_drain_all_txq(struct ath_softc *sc, bool retry_tx)
{
struct ath_hw *ah = sc->sc_ah;
struct ath_common *common = ath9k_hw_common(sc->sc_ah);
@@ -1128,7 +1128,7 @@ void ath_drain_all_txq(struct ath_softc *sc, bool retry_tx)
int i, npend = 0;
if (sc->sc_flags & SC_OP_INVALID)
- return;
+ return true;
/* Stop beacon queue */
ath9k_hw_stoptxdma(sc->sc_ah, sc->beacon.beaconq);
@@ -1142,25 +1142,15 @@ void ath_drain_all_txq(struct ath_softc *sc, bool retry_tx)
}
}
- if (npend) {
- int r;
-
- ath_print(common, ATH_DBG_FATAL,
- "Failed to stop TX DMA. Resetting hardware!\n");
-
- spin_lock_bh(&sc->sc_resetlock);
- r = ath9k_hw_reset(ah, sc->sc_ah->curchan, ah->caldata, false);
- if (r)
- ath_print(common, ATH_DBG_FATAL,
- "Unable to reset hardware; reset status %d\n",
- r);
- spin_unlock_bh(&sc->sc_resetlock);
- }
+ if (npend)
+ ath_print(common, ATH_DBG_FATAL, "Failed to stop TX DMA!\n");
for (i = 0; i < ATH9K_NUM_TX_QUEUES; i++) {
if (ATH_TXQ_SETUP(sc, i))
ath_draintxq(sc, &sc->tx.txq[i], retry_tx);
}
+
+ return !npend;
}
void ath_tx_cleanupq(struct ath_softc *sc, struct ath_txq *txq)
--
1.7.3.2
On 12/05/2010 11:17 AM, Felix Fietkau wrote:
> When ath_drain_all_txq fails to stop DMA, it issues a hw reset. This reset
> happens at a very problematic point in time, when the hardware rx path has
> not been stopped yet. This could lead to memory corruption, hardware hangs
> or other issues.
> To fix these issues, simply remove the reset entirely and check the tx DMA
> stop status to prevent problems with fast channel changes.
Should we put this change into wireless-testing as well?
Thanks,
Ben
>
> Signed-off-by: Felix Fietkau<[email protected]>
> ---
> drivers/net/wireless/ath/ath9k/ath9k.h | 2 +-
> drivers/net/wireless/ath/ath9k/main.c | 5 +++--
> drivers/net/wireless/ath/ath9k/xmit.c | 22 ++++++----------------
> 3 files changed, 10 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
> index 0d0bec3..0963071 100644
> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
> @@ -329,7 +329,7 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp);
> struct ath_txq *ath_txq_setup(struct ath_softc *sc, int qtype, int subtype);
> void ath_tx_cleanupq(struct ath_softc *sc, struct ath_txq *txq);
> int ath_tx_setup(struct ath_softc *sc, int haltype);
> -void ath_drain_all_txq(struct ath_softc *sc, bool retry_tx);
> +bool ath_drain_all_txq(struct ath_softc *sc, bool retry_tx);
> void ath_draintxq(struct ath_softc *sc,
> struct ath_txq *txq, bool retry_tx);
> void ath_tx_node_init(struct ath_softc *sc, struct ath_node *an);
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index dace215..928ef68 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -244,11 +244,12 @@ int ath_set_channel(struct ath_softc *sc, struct ieee80211_hw *hw,
> * the relevant bits of the h/w.
> */
> ath9k_hw_set_interrupts(ah, 0);
> - ath_drain_all_txq(sc, false);
> + stopped = ath_drain_all_txq(sc, false);
>
> spin_lock_bh(&sc->rx.pcu_lock);
>
> - stopped = ath_stoprecv(sc);
> + if (!ath_stoprecv(sc))
> + stopped = false;
>
> /* XXX: do not flush receive queue here. We don't want
> * to flush data frames already in queue because of
> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
> index f2ade24..aff0478 100644
> --- a/drivers/net/wireless/ath/ath9k/xmit.c
> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
> @@ -1120,7 +1120,7 @@ void ath_draintxq(struct ath_softc *sc, struct ath_txq *txq, bool retry_tx)
> }
> }
>
> -void ath_drain_all_txq(struct ath_softc *sc, bool retry_tx)
> +bool ath_drain_all_txq(struct ath_softc *sc, bool retry_tx)
> {
> struct ath_hw *ah = sc->sc_ah;
> struct ath_common *common = ath9k_hw_common(sc->sc_ah);
> @@ -1128,7 +1128,7 @@ void ath_drain_all_txq(struct ath_softc *sc, bool retry_tx)
> int i, npend = 0;
>
> if (sc->sc_flags& SC_OP_INVALID)
> - return;
> + return true;
>
> /* Stop beacon queue */
> ath9k_hw_stoptxdma(sc->sc_ah, sc->beacon.beaconq);
> @@ -1142,25 +1142,15 @@ void ath_drain_all_txq(struct ath_softc *sc, bool retry_tx)
> }
> }
>
> - if (npend) {
> - int r;
> -
> - ath_print(common, ATH_DBG_FATAL,
> - "Failed to stop TX DMA. Resetting hardware!\n");
> -
> - spin_lock_bh(&sc->sc_resetlock);
> - r = ath9k_hw_reset(ah, sc->sc_ah->curchan, ah->caldata, false);
> - if (r)
> - ath_print(common, ATH_DBG_FATAL,
> - "Unable to reset hardware; reset status %d\n",
> - r);
> - spin_unlock_bh(&sc->sc_resetlock);
> - }
> + if (npend)
> + ath_print(common, ATH_DBG_FATAL, "Failed to stop TX DMA!\n");
>
> for (i = 0; i< ATH9K_NUM_TX_QUEUES; i++) {
> if (ATH_TXQ_SETUP(sc, i))
> ath_draintxq(sc,&sc->tx.txq[i], retry_tx);
> }
> +
> + return !npend;
> }
>
> void ath_tx_cleanupq(struct ath_softc *sc, struct ath_txq *txq)
--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com