* [PATCH v2 09/22] ath9k: Handle fatal interrupts properly
@ 2012-05-24 4:02 Sujith Manoharan
2012-05-29 12:29 ` Felix Fietkau
0 siblings, 1 reply; 6+ messages in thread
From: Sujith Manoharan @ 2012-05-24 4:02 UTC (permalink / raw)
To: linville; +Cc: linux-wireless
When a fatal interrupt is received or it is detected that the baseband
has hung, the chip has to be reset immediately. Otherwise, we end up
processing spurious interrupts. Ensure that we bail out properly in
the ISR when the reset work hasn't completed yet.
Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 2 ++
drivers/net/wireless/ath/ath9k/init.c | 1 +
drivers/net/wireless/ath/ath9k/main.c | 14 ++++++++++++++
3 files changed, 17 insertions(+)
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index a277cf6..70504b7 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -593,6 +593,7 @@ struct ath_ant_comb {
#define SC_OP_BT_SCAN BIT(6)
#define SC_OP_ANI_RUN BIT(7)
#define SC_OP_PRIM_STA_VIF BIT(8)
+#define SC_OP_HW_RESET BIT(9)
/* Powersave flags */
#define PS_WAIT_FOR_BEACON BIT(0)
@@ -631,6 +632,7 @@ struct ath_softc {
spinlock_t sc_serial_rw;
spinlock_t sc_pm_lock;
spinlock_t sc_pcu_lock;
+ spinlock_t sc_bb_lock;
struct mutex mutex;
struct work_struct paprd_work;
struct work_struct hw_check_work;
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 9dfce1a..d1d016c 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -549,6 +549,7 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
spin_lock_init(&sc->sc_serial_rw);
spin_lock_init(&sc->sc_pm_lock);
+ spin_lock_init(&sc->sc_bb_lock);
mutex_init(&sc->mutex);
#ifdef CONFIG_ATH9K_DEBUGFS
spin_lock_init(&sc->nodes_lock);
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 3f79923..672aefe 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -270,6 +270,7 @@ static bool ath_complete_reset(struct ath_softc *sc, bool start)
{
struct ath_hw *ah = sc->sc_ah;
struct ath_common *common = ath9k_hw_common(ah);
+ unsigned long flags;
if (ath_startrecv(sc) != 0) {
ath_err(common, "Unable to restart recv logic\n");
@@ -310,6 +311,10 @@ static bool ath_complete_reset(struct ath_softc *sc, bool start)
ieee80211_wake_queues(sc->hw);
+ spin_lock_irqsave(&sc->sc_bb_lock, flags);
+ sc->sc_flags &= ~SC_OP_HW_RESET;
+ spin_unlock_irqrestore(&sc->sc_bb_lock, flags);
+
return true;
}
@@ -692,6 +697,9 @@ void ath9k_tasklet(unsigned long data)
RESET_STAT_INC(sc, type);
#endif
+ spin_lock(&sc->sc_bb_lock);
+ sc->sc_flags |= SC_OP_HW_RESET;
+ spin_unlock(&sc->sc_bb_lock);
ieee80211_queue_work(sc->hw, &sc->hw_reset_work);
goto out;
}
@@ -768,6 +776,12 @@ irqreturn_t ath_isr(int irq, void *dev)
if (sc->sc_flags & SC_OP_INVALID)
return IRQ_NONE;
+ spin_lock(&sc->sc_bb_lock);
+ if (sc->sc_flags & SC_OP_HW_RESET) {
+ spin_unlock(&sc->sc_bb_lock);
+ return IRQ_NONE;
+ }
+ spin_unlock(&sc->sc_bb_lock);
/* shared irq, not for us */
--
1.7.10.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 09/22] ath9k: Handle fatal interrupts properly
2012-05-24 4:02 [PATCH v2 09/22] ath9k: Handle fatal interrupts properly Sujith Manoharan
@ 2012-05-29 12:29 ` Felix Fietkau
2012-05-29 12:36 ` Johannes Berg
0 siblings, 1 reply; 6+ messages in thread
From: Felix Fietkau @ 2012-05-29 12:29 UTC (permalink / raw)
To: Sujith Manoharan; +Cc: linville, linux-wireless
On 2012-05-24 6:02 AM, Sujith Manoharan wrote:
> When a fatal interrupt is received or it is detected that the baseband
> has hung, the chip has to be reset immediately. Otherwise, we end up
> processing spurious interrupts. Ensure that we bail out properly in
> the ISR when the reset work hasn't completed yet.
>
> Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
Wouldn't it be much easier to just use a bool variable instead of the
combination of lock+flag? A simple variable assignment is atomic...
- Felix
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 09/22] ath9k: Handle fatal interrupts properly
2012-05-29 12:29 ` Felix Fietkau
@ 2012-05-29 12:36 ` Johannes Berg
2012-05-29 12:43 ` Felix Fietkau
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Berg @ 2012-05-29 12:36 UTC (permalink / raw)
To: Felix Fietkau; +Cc: Sujith Manoharan, linville, linux-wireless
On Tue, 2012-05-29 at 14:29 +0200, Felix Fietkau wrote:
> On 2012-05-24 6:02 AM, Sujith Manoharan wrote:
> > When a fatal interrupt is received or it is detected that the baseband
> > has hung, the chip has to be reset immediately. Otherwise, we end up
> > processing spurious interrupts. Ensure that we bail out properly in
> > the ISR when the reset work hasn't completed yet.
> >
> > Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
> Wouldn't it be much easier to just use a bool variable instead of the
> combination of lock+flag? A simple variable assignment is atomic...
Is that guaranteed for bool variables?
johannes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 09/22] ath9k: Handle fatal interrupts properly
2012-05-29 12:36 ` Johannes Berg
@ 2012-05-29 12:43 ` Felix Fietkau
2012-05-29 12:50 ` Sujith Manoharan
2012-06-02 11:53 ` Sujith Manoharan
0 siblings, 2 replies; 6+ messages in thread
From: Felix Fietkau @ 2012-05-29 12:43 UTC (permalink / raw)
To: Johannes Berg; +Cc: Sujith Manoharan, linville, linux-wireless
On 2012-05-29 2:36 PM, Johannes Berg wrote:
> On Tue, 2012-05-29 at 14:29 +0200, Felix Fietkau wrote:
>> On 2012-05-24 6:02 AM, Sujith Manoharan wrote:
>> > When a fatal interrupt is received or it is detected that the baseband
>> > has hung, the chip has to be reset immediately. Otherwise, we end up
>> > processing spurious interrupts. Ensure that we bail out properly in
>> > the ISR when the reset work hasn't completed yet.
>> >
>> > Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
>> Wouldn't it be much easier to just use a bool variable instead of the
>> combination of lock+flag? A simple variable assignment is atomic...
>
> Is that guaranteed for bool variables?
Hm, good point. Maybe use u32 then. Either way, the lock in this patch
does almost nothing to protect against race conditions, since it doesn't
apply to other places in the code that change sc->sc_flags.
Maybe we should change that entire thing to just use set_bit/test_bit
instead.
- Felix
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 09/22] ath9k: Handle fatal interrupts properly
2012-05-29 12:43 ` Felix Fietkau
@ 2012-05-29 12:50 ` Sujith Manoharan
2012-06-02 11:53 ` Sujith Manoharan
1 sibling, 0 replies; 6+ messages in thread
From: Sujith Manoharan @ 2012-05-29 12:50 UTC (permalink / raw)
To: Felix Fietkau; +Cc: Johannes Berg, Sujith Manoharan, linville, linux-wireless
Felix Fietkau wrote:
> Hm, good point. Maybe use u32 then. Either way, the lock in this patch
> does almost nothing to protect against race conditions, since it doesn't
> apply to other places in the code that change sc->sc_flags.
> Maybe we should change that entire thing to just use set_bit/test_bit
> instead.
That is because the locking for all the other flags has to be cleaned up.
Especially the powersave/beacon flags, and the usage of other locks
are hazy (sc_pcu_lock etc.). We can clean all of them, but I think it
will be an involved process - and the watchdog race is a crippling issue
as far as general usage is concerned. Disconnects occur in regular usage
quite frequently, breaking stress tests.
Sujith
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 09/22] ath9k: Handle fatal interrupts properly
2012-05-29 12:43 ` Felix Fietkau
2012-05-29 12:50 ` Sujith Manoharan
@ 2012-06-02 11:53 ` Sujith Manoharan
1 sibling, 0 replies; 6+ messages in thread
From: Sujith Manoharan @ 2012-06-02 11:53 UTC (permalink / raw)
To: Felix Fietkau; +Cc: Johannes Berg, Sujith Manoharan, linville, linux-wireless
Felix Fietkau wrote:
> Hm, good point. Maybe use u32 then. Either way, the lock in this patch
> does almost nothing to protect against race conditions, since it doesn't
> apply to other places in the code that change sc->sc_flags.
> Maybe we should change that entire thing to just use set_bit/test_bit
> instead.
That's a good idea. I'll convert the flags to atomic ops.
Sujith
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-06-02 11:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-24 4:02 [PATCH v2 09/22] ath9k: Handle fatal interrupts properly Sujith Manoharan
2012-05-29 12:29 ` Felix Fietkau
2012-05-29 12:36 ` Johannes Berg
2012-05-29 12:43 ` Felix Fietkau
2012-05-29 12:50 ` Sujith Manoharan
2012-06-02 11:53 ` Sujith Manoharan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).