* [PATCH 1/3] ath9k: fix enabling interrupts after a hardware error interrupt @ 2011-09-14 19:23 Felix Fietkau 2011-09-14 19:23 ` [PATCH 2/3] ath9k: make beacon timer initialization more reliable Felix Fietkau 2011-09-15 7:14 ` [PATCH 1/3] ath9k: fix enabling interrupts after a hardware error interrupt Mohammed Shafi 0 siblings, 2 replies; 6+ messages in thread From: Felix Fietkau @ 2011-09-14 19:23 UTC (permalink / raw) To: linux-wireless; +Cc: linville, mcgrof The interrupt handler increases the interrupt disable refcount, so the tasklet needs to always call ath9k_hw_enable_interrupts. Signed-off-by: Felix Fietkau <nbd@openwrt.org> --- drivers/net/wireless/ath/ath9k/main.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c index 8a17cff5..a3c3316 100644 --- a/drivers/net/wireless/ath/ath9k/main.c +++ b/drivers/net/wireless/ath/ath9k/main.c @@ -669,15 +669,15 @@ void ath9k_tasklet(unsigned long data) u32 status = sc->intrstatus; u32 rxmask; + ath9k_ps_wakeup(sc); + spin_lock(&sc->sc_pcu_lock); + if ((status & ATH9K_INT_FATAL) || (status & ATH9K_INT_BB_WATCHDOG)) { ieee80211_queue_work(sc->hw, &sc->hw_reset_work); - return; + goto out; } - ath9k_ps_wakeup(sc); - spin_lock(&sc->sc_pcu_lock); - /* * Only run the baseband hang check if beacons stop working in AP or * IBSS mode, because it has a high false positive rate. For station @@ -725,6 +725,7 @@ void ath9k_tasklet(unsigned long data) if (status & ATH9K_INT_GENTIMER) ath_gen_timer_isr(sc->sc_ah); +out: /* re-enable hardware interrupt */ ath9k_hw_enable_interrupts(ah); -- 1.7.3.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] ath9k: make beacon timer initialization more reliable 2011-09-14 19:23 [PATCH 1/3] ath9k: fix enabling interrupts after a hardware error interrupt Felix Fietkau @ 2011-09-14 19:23 ` Felix Fietkau 2011-09-14 19:23 ` [PATCH 3/3] ath9k: ensure that rx is not enabled during a reset Felix Fietkau 2011-09-15 7:14 ` [PATCH 1/3] ath9k: fix enabling interrupts after a hardware error interrupt Mohammed Shafi 1 sibling, 1 reply; 6+ messages in thread From: Felix Fietkau @ 2011-09-14 19:23 UTC (permalink / raw) To: linux-wireless; +Cc: linville, mcgrof When starting the AP beacon timer, it assumes that the TSF has recently been cleared. Set the SC_OP_TSF_RESET flag to ensure that this is always the case. Signed-off-by: Felix Fietkau <nbd@openwrt.org> --- drivers/net/wireless/ath/ath9k/beacon.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c index 22e8e25..6d7088b 100644 --- a/drivers/net/wireless/ath/ath9k/beacon.c +++ b/drivers/net/wireless/ath/ath9k/beacon.c @@ -517,6 +517,7 @@ static void ath_beacon_config_ap(struct ath_softc *sc, /* Set the computed AP beacon timers */ ath9k_hw_disable_interrupts(ah); + sc->sc_flags |= SC_OP_TSF_RESET; ath9k_beacon_init(sc, nexttbtt, intval); sc->beacon.bmisscnt = 0; ath9k_hw_set_interrupts(ah, ah->imask); -- 1.7.3.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] ath9k: ensure that rx is not enabled during a reset 2011-09-14 19:23 ` [PATCH 2/3] ath9k: make beacon timer initialization more reliable Felix Fietkau @ 2011-09-14 19:23 ` Felix Fietkau 0 siblings, 0 replies; 6+ messages in thread From: Felix Fietkau @ 2011-09-14 19:23 UTC (permalink / raw) To: linux-wireless; +Cc: linville, mcgrof During a reset, rx buffers are flushed after rx has been disabled. To avoid race conditions, rx needs to stay disabled during the reset, so avoid any calls to ath9k_hw_rxena in that case. Signed-off-by: Felix Fietkau <nbd@openwrt.org> --- drivers/net/wireless/ath/ath9k/main.c | 4 ++-- drivers/net/wireless/ath/ath9k/recv.c | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c index a3c3316..a75810a 100644 --- a/drivers/net/wireless/ath/ath9k/main.c +++ b/drivers/net/wireless/ath/ath9k/main.c @@ -247,8 +247,8 @@ static bool ath_prepare_reset(struct ath_softc *sc, bool retry_tx, bool flush) if (!flush) { if (ah->caps.hw_caps & ATH9K_HW_CAP_EDMA) - ath_rx_tasklet(sc, 0, true); - ath_rx_tasklet(sc, 0, false); + ath_rx_tasklet(sc, 1, true); + ath_rx_tasklet(sc, 1, false); } else { ath_flushrecv(sc); } diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c index 8d3e19d..bcc0b22 100644 --- a/drivers/net/wireless/ath/ath9k/recv.c +++ b/drivers/net/wireless/ath/ath9k/recv.c @@ -1839,7 +1839,7 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp) * If we're asked to flush receive queue, directly * chain it back at the queue without processing it. */ - if (flush) + if (sc->sc_flags & SC_OP_RXFLUSH) goto requeue_drop_frag; retval = ath9k_rx_skb_preprocess(common, hw, hdr, &rs, @@ -1967,7 +1967,8 @@ requeue: } else { list_move_tail(&bf->list, &sc->rx.rxbuf); ath_rx_buf_link(sc, bf); - ath9k_hw_rxena(ah); + if (!flush) + ath9k_hw_rxena(ah); } } while (1); -- 1.7.3.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] ath9k: fix enabling interrupts after a hardware error interrupt 2011-09-14 19:23 [PATCH 1/3] ath9k: fix enabling interrupts after a hardware error interrupt Felix Fietkau 2011-09-14 19:23 ` [PATCH 2/3] ath9k: make beacon timer initialization more reliable Felix Fietkau @ 2011-09-15 7:14 ` Mohammed Shafi 2011-09-15 7:19 ` Felix Fietkau 1 sibling, 1 reply; 6+ messages in thread From: Mohammed Shafi @ 2011-09-15 7:14 UTC (permalink / raw) To: Felix Fietkau; +Cc: linux-wireless, linville, mcgrof Hi Felix, On Thu, Sep 15, 2011 at 12:53 AM, Felix Fietkau <nbd@openwrt.org> wrote: > The interrupt handler increases the interrupt disable refcount, so the > tasklet needs to always call ath9k_hw_enable_interrupts. > > Signed-off-by: Felix Fietkau <nbd@openwrt.org> > --- > drivers/net/wireless/ath/ath9k/main.c | 9 +++++---- > 1 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c > index 8a17cff5..a3c3316 100644 > --- a/drivers/net/wireless/ath/ath9k/main.c > +++ b/drivers/net/wireless/ath/ath9k/main.c > @@ -669,15 +669,15 @@ void ath9k_tasklet(unsigned long data) > u32 status = sc->intrstatus; > u32 rxmask; > > + ath9k_ps_wakeup(sc); this is done in ath_reset, is there any corner cases that I had missed ? > + spin_lock(&sc->sc_pcu_lock); spin_lock_bh(&sc->sc_pcu_lock) in ath_reset_internal, won't be a problem know, or should we need to move this lock > + > if ((status & ATH9K_INT_FATAL) || > (status & ATH9K_INT_BB_WATCHDOG)) { > ieee80211_queue_work(sc->hw, &sc->hw_reset_work); > - return; > + goto out; > } > > - ath9k_ps_wakeup(sc); > - spin_lock(&sc->sc_pcu_lock); > - > /* > * Only run the baseband hang check if beacons stop working in AP or > * IBSS mode, because it has a high false positive rate. For station > @@ -725,6 +725,7 @@ void ath9k_tasklet(unsigned long data) > if (status & ATH9K_INT_GENTIMER) > ath_gen_timer_isr(sc->sc_ah); > > +out: > /* re-enable hardware interrupt */ > ath9k_hw_enable_interrupts(ah); this is done in ath_complete_reset, should we enable interrupts if some times ath9k_hw_reset fails? please correct me if I had understood wrongly. thanks! > > -- > 1.7.3.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- shafi ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] ath9k: fix enabling interrupts after a hardware error interrupt 2011-09-15 7:14 ` [PATCH 1/3] ath9k: fix enabling interrupts after a hardware error interrupt Mohammed Shafi @ 2011-09-15 7:19 ` Felix Fietkau 2011-09-15 7:57 ` Mohammed Shafi 0 siblings, 1 reply; 6+ messages in thread From: Felix Fietkau @ 2011-09-15 7:19 UTC (permalink / raw) To: Mohammed Shafi; +Cc: linux-wireless, linville, mcgrof On 2011-09-15 9:14 AM, Mohammed Shafi wrote: > Hi Felix, > > On Thu, Sep 15, 2011 at 12:53 AM, Felix Fietkau<nbd@openwrt.org> wrote: >> The interrupt handler increases the interrupt disable refcount, so the >> tasklet needs to always call ath9k_hw_enable_interrupts. >> >> Signed-off-by: Felix Fietkau<nbd@openwrt.org> >> --- >> drivers/net/wireless/ath/ath9k/main.c | 9 +++++---- >> 1 files changed, 5 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c >> index 8a17cff5..a3c3316 100644 >> --- a/drivers/net/wireless/ath/ath9k/main.c >> +++ b/drivers/net/wireless/ath/ath9k/main.c >> @@ -669,15 +669,15 @@ void ath9k_tasklet(unsigned long data) >> u32 status = sc->intrstatus; >> u32 rxmask; >> >> + ath9k_ps_wakeup(sc); > > this is done in ath_reset, is there any corner cases that I had missed ? > >> + spin_lock(&sc->sc_pcu_lock); > > spin_lock_bh(&sc->sc_pcu_lock) in ath_reset_internal, won't be a > problem know, or should we need to move this lock > >> + >> if ((status& ATH9K_INT_FATAL) || >> (status& ATH9K_INT_BB_WATCHDOG)) { >> ieee80211_queue_work(sc->hw,&sc->hw_reset_work); >> - return; >> + goto out; >> } >> >> - ath9k_ps_wakeup(sc); >> - spin_lock(&sc->sc_pcu_lock); >> - >> /* >> * Only run the baseband hang check if beacons stop working in AP or >> * IBSS mode, because it has a high false positive rate. For station >> @@ -725,6 +725,7 @@ void ath9k_tasklet(unsigned long data) >> if (status& ATH9K_INT_GENTIMER) >> ath_gen_timer_isr(sc->sc_ah); >> >> +out: >> /* re-enable hardware interrupt */ >> ath9k_hw_enable_interrupts(ah); > > > this is done in ath_complete_reset, should we enable interrupts if > some times ath9k_hw_reset fails? > please correct me if I had understood wrongly. thanks! ath9k_hw_enable_interrupts was changed to use refcounting to keep interrupts blocked. The interrupt handler increses the refcount, so the tasklet needs to decrease it. When a reset is issued, it starts by disabling interrupts, then re-enables them after completing the reset. Disabling interrupts in the ISR, but not enabling them in the tasklet causes an imbalance which leaves interrupts disabled after the reset is done. - Felix ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] ath9k: fix enabling interrupts after a hardware error interrupt 2011-09-15 7:19 ` Felix Fietkau @ 2011-09-15 7:57 ` Mohammed Shafi 0 siblings, 0 replies; 6+ messages in thread From: Mohammed Shafi @ 2011-09-15 7:57 UTC (permalink / raw) To: Felix Fietkau; +Cc: linux-wireless, linville, mcgrof On Thu, Sep 15, 2011 at 12:49 PM, Felix Fietkau <nbd@openwrt.org> wrote: > On 2011-09-15 9:14 AM, Mohammed Shafi wrote: >> >> Hi Felix, >> >> On Thu, Sep 15, 2011 at 12:53 AM, Felix Fietkau<nbd@openwrt.org> wrote: >>> >>> The interrupt handler increases the interrupt disable refcount, so the >>> tasklet needs to always call ath9k_hw_enable_interrupts. >>> >>> Signed-off-by: Felix Fietkau<nbd@openwrt.org> >>> --- >>> drivers/net/wireless/ath/ath9k/main.c | 9 +++++---- >>> 1 files changed, 5 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/net/wireless/ath/ath9k/main.c >>> b/drivers/net/wireless/ath/ath9k/main.c >>> index 8a17cff5..a3c3316 100644 >>> --- a/drivers/net/wireless/ath/ath9k/main.c >>> +++ b/drivers/net/wireless/ath/ath9k/main.c >>> @@ -669,15 +669,15 @@ void ath9k_tasklet(unsigned long data) >>> u32 status = sc->intrstatus; >>> u32 rxmask; >>> >>> + ath9k_ps_wakeup(sc); >> >> this is done in ath_reset, is there any corner cases that I had missed ? >> >>> + spin_lock(&sc->sc_pcu_lock); >> >> spin_lock_bh(&sc->sc_pcu_lock) in ath_reset_internal, won't be a >> problem know, or should we need to move this lock >> >>> + >>> if ((status& ATH9K_INT_FATAL) || >>> (status& ATH9K_INT_BB_WATCHDOG)) { >>> ieee80211_queue_work(sc->hw,&sc->hw_reset_work); >>> - return; >>> + goto out; >>> } >>> >>> - ath9k_ps_wakeup(sc); >>> - spin_lock(&sc->sc_pcu_lock); >>> - >>> /* >>> * Only run the baseband hang check if beacons stop working in AP >>> or >>> * IBSS mode, because it has a high false positive rate. For >>> station >>> @@ -725,6 +725,7 @@ void ath9k_tasklet(unsigned long data) >>> if (status& ATH9K_INT_GENTIMER) >>> ath_gen_timer_isr(sc->sc_ah); >>> >>> +out: >>> /* re-enable hardware interrupt */ >>> ath9k_hw_enable_interrupts(ah); >> >> >> this is done in ath_complete_reset, should we enable interrupts if >> some times ath9k_hw_reset fails? >> please correct me if I had understood wrongly. thanks! > > ath9k_hw_enable_interrupts was changed to use refcounting to keep interrupts > blocked. The interrupt handler increses the refcount, so the tasklet needs > to decrease it. When a reset is issued, it starts by disabling interrupts, > then re-enables them after completing the reset. > Disabling interrupts in the ISR, but not enabling them in the tasklet causes > an imbalance which leaves interrupts disabled after the reset is done. Felix, thanks for your explanation. i understood it somewhat :). i will take a look at this more carefully, especially intr_ref_cnt. > > - Felix > -- shafi ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-09-15 7:57 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-09-14 19:23 [PATCH 1/3] ath9k: fix enabling interrupts after a hardware error interrupt Felix Fietkau 2011-09-14 19:23 ` [PATCH 2/3] ath9k: make beacon timer initialization more reliable Felix Fietkau 2011-09-14 19:23 ` [PATCH 3/3] ath9k: ensure that rx is not enabled during a reset Felix Fietkau 2011-09-15 7:14 ` [PATCH 1/3] ath9k: fix enabling interrupts after a hardware error interrupt Mohammed Shafi 2011-09-15 7:19 ` Felix Fietkau 2011-09-15 7:57 ` Mohammed Shafi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.