All of lore.kernel.org
 help / color / mirror / Atom feed
* [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.