The interrupt handler increases the interrupt disable refcount, so the
tasklet needs to always call ath9k_hw_enable_interrupts.
Signed-off-by: Felix Fietkau <[email protected]>
---
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
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 <[email protected]>
---
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
Hi Felix,
On Thu, Sep 15, 2011 at 12:53 AM, Felix Fietkau <[email protected]> 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 <[email protected]>
> ---
> ?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 [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>
--
shafi
On Thu, Sep 15, 2011 at 12:49 PM, Felix Fietkau <[email protected]> wrote:
> On 2011-09-15 9:14 AM, Mohammed Shafi wrote:
>>
>> Hi Felix,
>>
>> On Thu, Sep 15, 2011 at 12:53 AM, Felix Fietkau<[email protected]> ?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<[email protected]>
>>> ?---
>>> ? 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
On 2011-09-15 9:14 AM, Mohammed Shafi wrote:
> Hi Felix,
>
> On Thu, Sep 15, 2011 at 12:53 AM, Felix Fietkau<[email protected]> 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<[email protected]>
>> ---
>> 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
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 <[email protected]>
---
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