* [PATCH v2 1/3] ath9k: rename tx_complete_work to hw_check_work
@ 2017-02-02 9:14 Felix Fietkau
2017-02-02 9:14 ` [PATCH v2 2/3] ath9k_hw: check if the chip failed to wake up Felix Fietkau
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Felix Fietkau @ 2017-02-02 9:14 UTC (permalink / raw)
To: linux-wireless; +Cc: kvalo
Also include common MAC alive check. This should make the hang checks
more reliable for modes where beacons are not sent and is used as a
starting point for further hang check improvements
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 6 ++---
drivers/net/wireless/ath/ath9k/init.c | 1 +
drivers/net/wireless/ath/ath9k/link.c | 46 ++++++++++++++++++----------------
drivers/net/wireless/ath/ath9k/main.c | 10 +++++---
drivers/net/wireless/ath/ath9k/xmit.c | 2 --
5 files changed, 35 insertions(+), 30 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 331947b6a667..b2fa44adc60e 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -108,7 +108,7 @@ int ath_descdma_setup(struct ath_softc *sc, struct ath_descdma *dd,
#define ATH_AGGR_MIN_QDEPTH 2
/* minimum h/w qdepth for non-aggregated traffic */
#define ATH_NON_AGGR_MIN_QDEPTH 8
-#define ATH_TX_COMPLETE_POLL_INT 1000
+#define ATH_HW_CHECK_POLL_INT 1000
#define ATH_TXFIFO_DEPTH 8
#define ATH_TX_ERROR 0x01
@@ -745,7 +745,7 @@ void ath9k_csa_update(struct ath_softc *sc);
#define ATH_PAPRD_TIMEOUT 100 /* msecs */
#define ATH_PLL_WORK_INTERVAL 100
-void ath_tx_complete_poll_work(struct work_struct *work);
+void ath_hw_check_work(struct work_struct *work);
void ath_reset_work(struct work_struct *work);
bool ath_hw_check(struct ath_softc *sc);
void ath_hw_pll_work(struct work_struct *work);
@@ -1053,7 +1053,7 @@ struct ath_softc {
#ifdef CONFIG_ATH9K_DEBUGFS
struct ath9k_debug debug;
#endif
- struct delayed_work tx_complete_work;
+ struct delayed_work hw_check_work;
struct delayed_work hw_pll_work;
struct timer_list sleep_timer;
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 084ad1bd495f..a2f7f48bbb92 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -681,6 +681,7 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
INIT_WORK(&sc->hw_reset_work, ath_reset_work);
INIT_WORK(&sc->paprd_work, ath_paprd_calibrate);
INIT_DELAYED_WORK(&sc->hw_pll_work, ath_hw_pll_work);
+ INIT_DELAYED_WORK(&sc->hw_check_work, ath_hw_check_work);
ath9k_init_channel_context(sc);
diff --git a/drivers/net/wireless/ath/ath9k/link.c b/drivers/net/wireless/ath/ath9k/link.c
index 5ad0feeebc86..27c50562dc47 100644
--- a/drivers/net/wireless/ath/ath9k/link.c
+++ b/drivers/net/wireless/ath/ath9k/link.c
@@ -20,20 +20,13 @@
* TX polling - checks if the TX engine is stuck somewhere
* and issues a chip reset if so.
*/
-void ath_tx_complete_poll_work(struct work_struct *work)
+static bool ath_tx_complete_check(struct ath_softc *sc)
{
- struct ath_softc *sc = container_of(work, struct ath_softc,
- tx_complete_work.work);
struct ath_txq *txq;
int i;
- bool needreset = false;
-
- if (sc->tx99_state) {
- ath_dbg(ath9k_hw_common(sc->sc_ah), RESET,
- "skip tx hung detection on tx99\n");
- return;
- }
+ if (sc->tx99_state)
+ return true;
for (i = 0; i < IEEE80211_NUM_ACS; i++) {
txq = sc->tx.txq_map[i];
@@ -41,25 +34,36 @@ void ath_tx_complete_poll_work(struct work_struct *work)
ath_txq_lock(sc, txq);
if (txq->axq_depth) {
if (txq->axq_tx_inprogress) {
- needreset = true;
ath_txq_unlock(sc, txq);
- break;
- } else {
- txq->axq_tx_inprogress = true;
+ goto reset;
}
+
+ txq->axq_tx_inprogress = true;
}
ath_txq_unlock(sc, txq);
}
- if (needreset) {
- ath_dbg(ath9k_hw_common(sc->sc_ah), RESET,
- "tx hung, resetting the chip\n");
- ath9k_queue_reset(sc, RESET_TYPE_TX_HANG);
+ return true;
+
+reset:
+ ath_dbg(ath9k_hw_common(sc->sc_ah), RESET,
+ "tx hung, resetting the chip\n");
+ ath9k_queue_reset(sc, RESET_TYPE_TX_HANG);
+ return false;
+
+}
+
+void ath_hw_check_work(struct work_struct *work)
+{
+ struct ath_softc *sc = container_of(work, struct ath_softc,
+ hw_check_work.work);
+
+ if (!ath_hw_check(sc) ||
+ !ath_tx_complete_check(sc))
return;
- }
- ieee80211_queue_delayed_work(sc->hw, &sc->tx_complete_work,
- msecs_to_jiffies(ATH_TX_COMPLETE_POLL_INT));
+ ieee80211_queue_delayed_work(sc->hw, &sc->hw_check_work,
+ msecs_to_jiffies(ATH_HW_CHECK_POLL_INT));
}
/*
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 58f06ce9a4cf..0345d6ec564f 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -181,7 +181,7 @@ void ath9k_ps_restore(struct ath_softc *sc)
static void __ath_cancel_work(struct ath_softc *sc)
{
cancel_work_sync(&sc->paprd_work);
- cancel_delayed_work_sync(&sc->tx_complete_work);
+ cancel_delayed_work_sync(&sc->hw_check_work);
cancel_delayed_work_sync(&sc->hw_pll_work);
#ifdef CONFIG_ATH9K_BTCOEX_SUPPORT
@@ -198,7 +198,8 @@ void ath_cancel_work(struct ath_softc *sc)
void ath_restart_work(struct ath_softc *sc)
{
- ieee80211_queue_delayed_work(sc->hw, &sc->tx_complete_work, 0);
+ ieee80211_queue_delayed_work(sc->hw, &sc->hw_check_work,
+ ATH_HW_CHECK_POLL_INT);
if (AR_SREV_9340(sc->sc_ah) || AR_SREV_9330(sc->sc_ah))
ieee80211_queue_delayed_work(sc->hw, &sc->hw_pll_work,
@@ -2091,7 +2092,7 @@ void __ath9k_flush(struct ieee80211_hw *hw, u32 queues, bool drop,
int timeout;
bool drain_txq;
- cancel_delayed_work_sync(&sc->tx_complete_work);
+ cancel_delayed_work_sync(&sc->hw_check_work);
if (ah->ah_flags & AH_UNPLUGGED) {
ath_dbg(common, ANY, "Device has been unplugged!\n");
@@ -2129,7 +2130,8 @@ void __ath9k_flush(struct ieee80211_hw *hw, u32 queues, bool drop,
ath9k_ps_restore(sc);
}
- ieee80211_queue_delayed_work(hw, &sc->tx_complete_work, 0);
+ ieee80211_queue_delayed_work(hw, &sc->hw_check_work,
+ ATH_HW_CHECK_POLL_INT);
}
static bool ath9k_tx_frames_pending(struct ieee80211_hw *hw)
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index c35a192861ab..11073cf87909 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -2872,8 +2872,6 @@ int ath_tx_init(struct ath_softc *sc, int nbufs)
return error;
}
- INIT_DELAYED_WORK(&sc->tx_complete_work, ath_tx_complete_poll_work);
-
if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_EDMA)
error = ath_tx_edma_init(sc);
--
2.11.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v2 2/3] ath9k_hw: check if the chip failed to wake up
2017-02-02 9:14 [PATCH v2 1/3] ath9k: rename tx_complete_work to hw_check_work Felix Fietkau
@ 2017-02-02 9:14 ` Felix Fietkau
2017-02-02 9:14 ` [PATCH v2 3/3] ath9k: fix race condition in enabling/disabling IRQs Felix Fietkau
2017-02-07 9:01 ` [v2,1/3] ath9k: rename tx_complete_work to hw_check_work Kalle Valo
2 siblings, 0 replies; 4+ messages in thread
From: Felix Fietkau @ 2017-02-02 9:14 UTC (permalink / raw)
To: linux-wireless; +Cc: kvalo
In an RFC patch, Sven Eckelmann and Simon Wunderlich reported:
"QCA 802.11n chips (especially AR9330/AR9340) sometimes end up in a
state in which a read of AR_CFG always returns 0xdeadbeef.
This should not happen when when the power_mode of the device is
ATH9K_PM_AWAKE."
Include the check for the default register state in the existing MAC
hang check.
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
drivers/net/wireless/ath/ath9k/hw.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index ac36873d6da4..8c5c2dd8fa7f 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -1603,6 +1603,10 @@ bool ath9k_hw_check_alive(struct ath_hw *ah)
int count = 50;
u32 reg, last_val;
+ /* Check if chip failed to wake up */
+ if (REG_READ(ah, AR_CFG) == 0xdeadbeef)
+ return false;
+
if (AR_SREV_9300(ah))
return !ath9k_hw_detect_mac_hang(ah);
--
2.11.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 3/3] ath9k: fix race condition in enabling/disabling IRQs
2017-02-02 9:14 [PATCH v2 1/3] ath9k: rename tx_complete_work to hw_check_work Felix Fietkau
2017-02-02 9:14 ` [PATCH v2 2/3] ath9k_hw: check if the chip failed to wake up Felix Fietkau
@ 2017-02-02 9:14 ` Felix Fietkau
2017-02-07 9:01 ` [v2,1/3] ath9k: rename tx_complete_work to hw_check_work Kalle Valo
2 siblings, 0 replies; 4+ messages in thread
From: Felix Fietkau @ 2017-02-02 9:14 UTC (permalink / raw)
To: linux-wireless; +Cc: kvalo
The code currently relies on refcounting to disable IRQs from within the
IRQ handler and re-enabling them again after the tasklet has run.
However, due to race conditions sometimes the IRQ handler might be
called twice, or the tasklet may not run at all (if interrupted in the
middle of a reset).
This can cause nasty imbalances in the irq-disable refcount which will
get the driver permanently stuck until the entire radio has been stopped
and started again (ath_reset will not recover from this).
Instead of using this fragile logic, change the code to ensure that
running the irq handler during tasklet processing is safe, and leave the
refcount untouched.
Cc: stable@vger.kernel.org
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 1 +
drivers/net/wireless/ath/ath9k/init.c | 1 +
drivers/net/wireless/ath/ath9k/mac.c | 44 ++++++++++++++++++++++++++--------
drivers/net/wireless/ath/ath9k/mac.h | 1 +
drivers/net/wireless/ath/ath9k/main.c | 27 +++++++++------------
5 files changed, 48 insertions(+), 26 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index b2fa44adc60e..cf076719c27e 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -998,6 +998,7 @@ struct ath_softc {
struct survey_info *cur_survey;
struct survey_info survey[ATH9K_NUM_CHANNELS];
+ spinlock_t intr_lock;
struct tasklet_struct intr_tq;
struct tasklet_struct bcon_tasklet;
struct ath_hw *sc_ah;
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index a2f7f48bbb92..fa4b3cc1ba22 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -669,6 +669,7 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
common->bt_ant_diversity = 1;
spin_lock_init(&common->cc_lock);
+ spin_lock_init(&sc->intr_lock);
spin_lock_init(&sc->sc_serial_rw);
spin_lock_init(&sc->sc_pm_lock);
spin_lock_init(&sc->chan_lock);
diff --git a/drivers/net/wireless/ath/ath9k/mac.c b/drivers/net/wireless/ath/ath9k/mac.c
index bba85d1a6cd1..d937c39b3a0b 100644
--- a/drivers/net/wireless/ath/ath9k/mac.c
+++ b/drivers/net/wireless/ath/ath9k/mac.c
@@ -805,21 +805,12 @@ void ath9k_hw_disable_interrupts(struct ath_hw *ah)
}
EXPORT_SYMBOL(ath9k_hw_disable_interrupts);
-void ath9k_hw_enable_interrupts(struct ath_hw *ah)
+static void __ath9k_hw_enable_interrupts(struct ath_hw *ah)
{
struct ath_common *common = ath9k_hw_common(ah);
u32 sync_default = AR_INTR_SYNC_DEFAULT;
u32 async_mask;
- if (!(ah->imask & ATH9K_INT_GLOBAL))
- return;
-
- if (!atomic_inc_and_test(&ah->intr_ref_cnt)) {
- ath_dbg(common, INTERRUPT, "Do not enable IER ref count %d\n",
- atomic_read(&ah->intr_ref_cnt));
- return;
- }
-
if (AR_SREV_9340(ah) || AR_SREV_9550(ah) || AR_SREV_9531(ah) ||
AR_SREV_9561(ah))
sync_default &= ~AR_INTR_SYNC_HOST1_FATAL;
@@ -841,6 +832,39 @@ void ath9k_hw_enable_interrupts(struct ath_hw *ah)
ath_dbg(common, INTERRUPT, "AR_IMR 0x%x IER 0x%x\n",
REG_READ(ah, AR_IMR), REG_READ(ah, AR_IER));
}
+
+void ath9k_hw_resume_interrupts(struct ath_hw *ah)
+{
+ struct ath_common *common = ath9k_hw_common(ah);
+
+ if (!(ah->imask & ATH9K_INT_GLOBAL))
+ return;
+
+ if (atomic_read(&ah->intr_ref_cnt) != 0) {
+ ath_dbg(common, INTERRUPT, "Do not enable IER ref count %d\n",
+ atomic_read(&ah->intr_ref_cnt));
+ return;
+ }
+
+ __ath9k_hw_enable_interrupts(ah);
+}
+EXPORT_SYMBOL(ath9k_hw_resume_interrupts);
+
+void ath9k_hw_enable_interrupts(struct ath_hw *ah)
+{
+ struct ath_common *common = ath9k_hw_common(ah);
+
+ if (!(ah->imask & ATH9K_INT_GLOBAL))
+ return;
+
+ if (!atomic_inc_and_test(&ah->intr_ref_cnt)) {
+ ath_dbg(common, INTERRUPT, "Do not enable IER ref count %d\n",
+ atomic_read(&ah->intr_ref_cnt));
+ return;
+ }
+
+ __ath9k_hw_enable_interrupts(ah);
+}
EXPORT_SYMBOL(ath9k_hw_enable_interrupts);
void ath9k_hw_set_interrupts(struct ath_hw *ah)
diff --git a/drivers/net/wireless/ath/ath9k/mac.h b/drivers/net/wireless/ath/ath9k/mac.h
index 3bab01435a86..770fc11b41d1 100644
--- a/drivers/net/wireless/ath/ath9k/mac.h
+++ b/drivers/net/wireless/ath/ath9k/mac.h
@@ -744,6 +744,7 @@ void ath9k_hw_set_interrupts(struct ath_hw *ah);
void ath9k_hw_enable_interrupts(struct ath_hw *ah);
void ath9k_hw_disable_interrupts(struct ath_hw *ah);
void ath9k_hw_kill_interrupts(struct ath_hw *ah);
+void ath9k_hw_resume_interrupts(struct ath_hw *ah);
void ar9002_hw_attach_mac_ops(struct ath_hw *ah);
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 0345d6ec564f..9e65d14e7b1e 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -374,21 +374,20 @@ void ath9k_tasklet(unsigned long data)
struct ath_common *common = ath9k_hw_common(ah);
enum ath_reset_type type;
unsigned long flags;
- u32 status = sc->intrstatus;
+ u32 status;
u32 rxmask;
+ spin_lock_irqsave(&sc->intr_lock, flags);
+ status = sc->intrstatus;
+ sc->intrstatus = 0;
+ spin_unlock_irqrestore(&sc->intr_lock, flags);
+
ath9k_ps_wakeup(sc);
spin_lock(&sc->sc_pcu_lock);
if (status & ATH9K_INT_FATAL) {
type = RESET_TYPE_FATAL_INT;
ath9k_queue_reset(sc, type);
-
- /*
- * Increment the ref. counter here so that
- * interrupts are enabled in the reset routine.
- */
- atomic_inc(&ah->intr_ref_cnt);
ath_dbg(common, RESET, "FATAL: Skipping interrupts\n");
goto out;
}
@@ -404,11 +403,6 @@ void ath9k_tasklet(unsigned long data)
type = RESET_TYPE_BB_WATCHDOG;
ath9k_queue_reset(sc, type);
- /*
- * Increment the ref. counter here so that
- * interrupts are enabled in the reset routine.
- */
- atomic_inc(&ah->intr_ref_cnt);
ath_dbg(common, RESET,
"BB_WATCHDOG: Skipping interrupts\n");
goto out;
@@ -421,7 +415,6 @@ void ath9k_tasklet(unsigned long data)
if ((sc->gtt_cnt >= MAX_GTT_CNT) && !ath9k_hw_check_alive(ah)) {
type = RESET_TYPE_TX_GTT;
ath9k_queue_reset(sc, type);
- atomic_inc(&ah->intr_ref_cnt);
ath_dbg(common, RESET,
"GTT: Skipping interrupts\n");
goto out;
@@ -478,7 +471,7 @@ void ath9k_tasklet(unsigned long data)
ath9k_btcoex_handle_interrupt(sc, status);
/* re-enable hardware interrupt */
- ath9k_hw_enable_interrupts(ah);
+ ath9k_hw_resume_interrupts(ah);
out:
spin_unlock(&sc->sc_pcu_lock);
ath9k_ps_restore(sc);
@@ -542,7 +535,9 @@ irqreturn_t ath_isr(int irq, void *dev)
return IRQ_NONE;
/* Cache the status */
- sc->intrstatus = status;
+ spin_lock(&sc->intr_lock);
+ sc->intrstatus |= status;
+ spin_unlock(&sc->intr_lock);
if (status & SCHED_INTR)
sched = true;
@@ -588,7 +583,7 @@ irqreturn_t ath_isr(int irq, void *dev)
if (sched) {
/* turn off every interrupt */
- ath9k_hw_disable_interrupts(ah);
+ ath9k_hw_kill_interrupts(ah);
tasklet_schedule(&sc->intr_tq);
}
--
2.11.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [v2,1/3] ath9k: rename tx_complete_work to hw_check_work
2017-02-02 9:14 [PATCH v2 1/3] ath9k: rename tx_complete_work to hw_check_work Felix Fietkau
2017-02-02 9:14 ` [PATCH v2 2/3] ath9k_hw: check if the chip failed to wake up Felix Fietkau
2017-02-02 9:14 ` [PATCH v2 3/3] ath9k: fix race condition in enabling/disabling IRQs Felix Fietkau
@ 2017-02-07 9:01 ` Kalle Valo
2 siblings, 0 replies; 4+ messages in thread
From: Kalle Valo @ 2017-02-07 9:01 UTC (permalink / raw)
To: Felix Fietkau; +Cc: linux-wireless, kvalo
Felix Fietkau <nbd@nbd.name> wrote:
> Also include common MAC alive check. This should make the hang checks
> more reliable for modes where beacons are not sent and is used as a
> starting point for further hang check improvements
>
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
3 patches applied to ath-next branch of ath.git, thanks.
d63ffc45c5d3 ath9k: rename tx_complete_work to hw_check_work
a34d0a0da1ab ath9k_hw: check if the chip failed to wake up
3a5e969bb2f6 ath9k: fix race condition in enabling/disabling IRQs
--
https://patchwork.kernel.org/patch/9551439/
Documentation about submitting wireless patches and checking status
from patchwork:
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-02-07 9:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-02 9:14 [PATCH v2 1/3] ath9k: rename tx_complete_work to hw_check_work Felix Fietkau
2017-02-02 9:14 ` [PATCH v2 2/3] ath9k_hw: check if the chip failed to wake up Felix Fietkau
2017-02-02 9:14 ` [PATCH v2 3/3] ath9k: fix race condition in enabling/disabling IRQs Felix Fietkau
2017-02-07 9:01 ` [v2,1/3] ath9k: rename tx_complete_work to hw_check_work Kalle Valo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox