[PATCH 1/4] ath6kl: Fix bug in scheduling hb_timer

[PATCH 1/4] ath6kl: Fix bug in scheduling hb_timer

[Vasanthakumar Thiagarajan]

hb_timer should be scheduled only when hb_poll is non-zero.
But in ath6kl_recovery_work() the timer is scheduled based
on fw_recovery.enable instead which is wrong.

Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/recovery.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/recovery.c b/drivers/net/wireless/ath/ath6kl/recovery.c
index 03edeb8..c30df50 100644
--- a/drivers/net/wireless/ath/ath6kl/recovery.c
+++ b/drivers/net/wireless/ath/ath6kl/recovery.c
@@ -34,7 +34,7 @@ static void ath6kl_recovery_work(struct work_struct *work)
 
 	ar->fw_recovery.err_reason = 0;
 
-	if (ar->fw_recovery.enable)
+	if (ar->fw_recovery.hb_poll)
 		mod_timer(&ar->fw_recovery.hb_timer, jiffies +
 			  msecs_to_jiffies(ar->fw_recovery.hb_poll));
 }
-- 
1.7.0.4

[PATCH 2/4] ath6kl: Remove unnecessary recovery state check in ath6kl_recovery_hb_timer()

[Vasanthakumar Thiagarajan]

Checking for recovery state just before re-arming hb_timer is not
necessary, this should be done at the begining of the timer instead.

Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/recovery.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/recovery.c b/drivers/net/wireless/ath/ath6kl/recovery.c
index c30df50..1a82e8b 100644
--- a/drivers/net/wireless/ath/ath6kl/recovery.c
+++ b/drivers/net/wireless/ath/ath6kl/recovery.c
@@ -61,7 +61,7 @@ static void ath6kl_recovery_hb_timer(unsigned long data)
 	struct ath6kl *ar = (struct ath6kl *) data;
 	int err;
 
-	if (!ar->fw_recovery.enable)
+	if (!ar->fw_recovery.enable || (ar->state == ATH6KL_STATE_RECOVERY))
 		return;
 
 	if (ar->fw_recovery.hb_pending)
@@ -86,9 +86,6 @@ static void ath6kl_recovery_hb_timer(unsigned long data)
 		ath6kl_warn("Failed to send hb challenge request, err:%d\n",
 			    err);
 
-	if ((ar->state == ATH6KL_STATE_RECOVERY) || !ar->fw_recovery.enable)
-		return;
-
 	mod_timer(&ar->fw_recovery.hb_timer, jiffies +
 		  msecs_to_jiffies(ar->fw_recovery.hb_poll));
 }
-- 
1.7.0.4

[PATCH 3/4] ath6kl: Add a bit to ath6kl_dev_state for recovery cleanup state

[Vasanthakumar Thiagarajan]

Add a bit in ath6kl_dev_state to maintian the run time state
of firmware recovery configuration. This would help to have
user configuration in fw_recovery which will be added in
a separate patch.

Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/core.h     |    2 +-
 drivers/net/wireless/ath/ath6kl/recovery.c |   12 +++++++-----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/core.h b/drivers/net/wireless/ath/ath6kl/core.h
index ac90b31..40a7b19 100644
--- a/drivers/net/wireless/ath/ath6kl/core.h
+++ b/drivers/net/wireless/ath/ath6kl/core.h
@@ -641,6 +641,7 @@ enum ath6kl_dev_state {
 	SKIP_SCAN,
 	ROAM_TBL_PEND,
 	FIRST_BOOT,
+	RECOVERY_CLEANUP,
 };
 
 enum ath6kl_state {
@@ -807,7 +808,6 @@ struct ath6kl {
 	bool wiphy_registered;
 
 	struct ath6kl_fw_recovery {
-		bool enable;
 		struct work_struct recovery_work;
 		unsigned long err_reason;
 		unsigned long hb_poll;
diff --git a/drivers/net/wireless/ath/ath6kl/recovery.c b/drivers/net/wireless/ath/ath6kl/recovery.c
index 1a82e8b..98b6aa0 100644
--- a/drivers/net/wireless/ath/ath6kl/recovery.c
+++ b/drivers/net/wireless/ath/ath6kl/recovery.c
@@ -46,7 +46,8 @@ void ath6kl_recovery_err_notify(struct ath6kl *ar, enum ath6kl_fw_err reason)
 
 	set_bit(reason, &ar->fw_recovery.err_reason);
 
-	if (ar->fw_recovery.enable && ar->state != ATH6KL_STATE_RECOVERY)
+	if (!test_bit(RECOVERY_CLEANUP, &ar->flag) &&
+	    ar->state != ATH6KL_STATE_RECOVERY)
 		queue_work(ar->ath6kl_wq, &ar->fw_recovery.recovery_work);
 }
 
@@ -61,7 +62,8 @@ static void ath6kl_recovery_hb_timer(unsigned long data)
 	struct ath6kl *ar = (struct ath6kl *) data;
 	int err;
 
-	if (!ar->fw_recovery.enable || (ar->state == ATH6KL_STATE_RECOVERY))
+	if (test_bit(RECOVERY_CLEANUP, &ar->flag) ||
+	    (ar->state == ATH6KL_STATE_RECOVERY))
 		return;
 
 	if (ar->fw_recovery.hb_pending)
@@ -94,7 +96,7 @@ void ath6kl_recovery_init(struct ath6kl *ar)
 {
 	struct ath6kl_fw_recovery *recovery = &ar->fw_recovery;
 
-	recovery->enable = true;
+	clear_bit(RECOVERY_CLEANUP, &ar->flag);
 	INIT_WORK(&recovery->recovery_work, ath6kl_recovery_work);
 	recovery->seq_num = 0;
 	recovery->hb_misscnt = 0;
@@ -110,7 +112,7 @@ void ath6kl_recovery_init(struct ath6kl *ar)
 
 void ath6kl_recovery_cleanup(struct ath6kl *ar)
 {
-	ar->fw_recovery.enable = false;
+	set_bit(RECOVERY_CLEANUP, &ar->flag);
 
 	del_timer_sync(&ar->fw_recovery.hb_timer);
 	cancel_work_sync(&ar->fw_recovery.recovery_work);
@@ -133,7 +135,7 @@ void ath6kl_recovery_suspend(struct ath6kl *ar)
 
 void ath6kl_recovery_resume(struct ath6kl *ar)
 {
-	ar->fw_recovery.enable = true;
+	clear_bit(RECOVERY_CLEANUP, &ar->flag);
 
 	if (!ar->fw_recovery.hb_poll)
 		return;
-- 
1.7.0.4

[PATCH 4/4] ath6kl: Make fw error recovery configurable

[Vasanthakumar Thiagarajan]

Add a modparam to configure recovery. Recovery
from firmware error is disabled by default to debug
the actual issue further. To recovery from error,
modprobe ath6kl_core recovery_enable=1.

Reported-by: Jin Navy <nhjin@qca.qualcomm.com>
Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/core.c     |   12 ++++++++++--
 drivers/net/wireless/ath/ath6kl/core.h     |    1 +
 drivers/net/wireless/ath/ath6kl/recovery.c |   12 ++++++++++++
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/core.c b/drivers/net/wireless/ath/ath6kl/core.c
index fd5dd3a..4b46adb 100644
--- a/drivers/net/wireless/ath/ath6kl/core.c
+++ b/drivers/net/wireless/ath/ath6kl/core.c
@@ -33,6 +33,7 @@ static unsigned int wow_mode;
 static unsigned int uart_debug;
 static unsigned int ath6kl_p2p;
 static unsigned int testmode;
+static unsigned int recovery_enable;
 static unsigned int heart_beat_poll;
 
 module_param(debug_mask, uint, 0644);
@@ -41,9 +42,12 @@ module_param(wow_mode, uint, 0644);
 module_param(uart_debug, uint, 0644);
 module_param(ath6kl_p2p, uint, 0644);
 module_param(testmode, uint, 0644);
+module_param(recovery_enable, uint, 0644);
 module_param(heart_beat_poll, uint, 0644);
-MODULE_PARM_DESC(heart_beat_poll, "Enable fw error detection periodic" \
-		 "polling. This also specifies the polling interval in msecs");
+MODULE_PARM_DESC(recovery_enable, "Enable recovery from firmware error");
+MODULE_PARM_DESC(heart_beat_poll, "Enable fw error detection periodic"   \
+		 "polling. This also specifies the polling interval in"  \
+		 "msecs. Set reocvery_enable for this to be effective");
 
 void ath6kl_core_tx_complete(struct ath6kl *ar, struct sk_buff *skb)
 {
@@ -206,6 +210,10 @@ int ath6kl_core_init(struct ath6kl *ar, enum ath6kl_htc_type htc_type)
 	ath6kl_dbg(ATH6KL_DBG_TRC, "%s: name=%s dev=0x%p, ar=0x%p\n",
 		   __func__, wdev->netdev->name, wdev->netdev, ar);
 
+	ar->fw_recovery.enable = !!recovery_enable;
+	if (!ar->fw_recovery.enable)
+		return ret;
+
 	if (heart_beat_poll &&
 	    test_bit(ATH6KL_FW_CAPABILITY_HEART_BEAT_POLL,
 		     ar->fw_capabilities))
diff --git a/drivers/net/wireless/ath/ath6kl/core.h b/drivers/net/wireless/ath/ath6kl/core.h
index 40a7b19..3b2dfc1 100644
--- a/drivers/net/wireless/ath/ath6kl/core.h
+++ b/drivers/net/wireless/ath/ath6kl/core.h
@@ -815,6 +815,7 @@ struct ath6kl {
 		u32 seq_num;
 		bool hb_pending;
 		u8 hb_misscnt;
+		bool enable;
 	} fw_recovery;
 
 #ifdef CONFIG_ATH6KL_DEBUG
diff --git a/drivers/net/wireless/ath/ath6kl/recovery.c b/drivers/net/wireless/ath/ath6kl/recovery.c
index 98b6aa0..3a8d5e9 100644
--- a/drivers/net/wireless/ath/ath6kl/recovery.c
+++ b/drivers/net/wireless/ath/ath6kl/recovery.c
@@ -41,6 +41,9 @@ static void ath6kl_recovery_work(struct work_struct *work)
 
 void ath6kl_recovery_err_notify(struct ath6kl *ar, enum ath6kl_fw_err reason)
 {
+	if (!ar->fw_recovery.enable)
+		return;
+
 	ath6kl_dbg(ATH6KL_DBG_RECOVERY, "Fw error detected, reason:%d\n",
 		   reason);
 
@@ -112,6 +115,9 @@ void ath6kl_recovery_init(struct ath6kl *ar)
 
 void ath6kl_recovery_cleanup(struct ath6kl *ar)
 {
+	if (!ar->fw_recovery.enable)
+		return;
+
 	set_bit(RECOVERY_CLEANUP, &ar->flag);
 
 	del_timer_sync(&ar->fw_recovery.hb_timer);
@@ -120,6 +126,9 @@ void ath6kl_recovery_cleanup(struct ath6kl *ar)
 
 void ath6kl_recovery_suspend(struct ath6kl *ar)
 {
+	if (!ar->fw_recovery.enable)
+		return;
+
 	ath6kl_recovery_cleanup(ar);
 
 	if (!ar->fw_recovery.err_reason)
@@ -135,6 +144,9 @@ void ath6kl_recovery_suspend(struct ath6kl *ar)
 
 void ath6kl_recovery_resume(struct ath6kl *ar)
 {
+	if (!ar->fw_recovery.enable)
+		return;
+
 	clear_bit(RECOVERY_CLEANUP, &ar->flag);
 
 	if (!ar->fw_recovery.hb_poll)
-- 
1.7.0.4

Re: [PATCH 1/4] ath6kl: Fix bug in scheduling hb_timer

[Kalle Valo]

On 09/03/2012 10:19 AM, Vasanthakumar Thiagarajan wrote:
> hb_timer should be scheduled only when hb_poll is non-zero.
> But in ath6kl_recovery_work() the timer is scheduled based
> on fw_recovery.enable instead which is wrong.
> 
> Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>

Thanks, all four applied.

Kalle