[ath9k-devel] [PATCH] ath10k: avoid infinite loop

[ath9k-devel] [PATCH] ath10k: avoid infinite loop

[Michal Kazior]

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/pci.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index ca09a44..41e58da 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -457,9 +457,11 @@ void ath10k_do_pci_wake(struct ath10k *ar)
 			break;
 		}
 
-		if (tot_delay > PCIE_WAKE_TIMEOUT)
-			ath10k_warn("keep_awake_count %d\n",
+		if (tot_delay > PCIE_WAKE_TIMEOUT) {
+			ath10k_warn("target takes too long to wake up (awake count %d)\n",
 				    atomic_read(&ar_pci->keep_awake_count));
+			break;
+		}
 
 		udelay(curr_delay);
 		tot_delay += curr_delay;
-- 
1.7.9.5

[ath9k-devel] [RFT] ath10k: retry target reset

[Michal Kazior]

Sometimes the device just won't reset cleanly
without retrying. This seems to depend on the host
hardware.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---

I can't reproduce the issue on my hw. Can someone
confirm if this fixes the problem?


 drivers/net/wireless/ath/ath10k/pci.c |   48 +++++++++++++++++++++++++--------
 1 file changed, 37 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 41e58da..3ae9298 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -422,15 +422,19 @@ static bool ath10k_pci_target_is_awake(struct ath10k *ar)
 	return (RTC_STATE_V_GET(val) == RTC_STATE_V_ON);
 }
 
-static void ath10k_pci_wait(struct ath10k *ar)
+static int ath10k_pci_wait(struct ath10k *ar)
 {
 	int n = 100;
 
 	while (n-- && !ath10k_pci_target_is_awake(ar))
 		msleep(10);
 
-	if (n < 0)
+	if (n < 0) {
 		ath10k_warn("Unable to wakeup target\n");
+		return -EIO;
+	}
+
+	return 0;
 }
 
 void ath10k_do_pci_wake(struct ath10k *ar)
@@ -1931,7 +1935,12 @@ static int ath10k_pci_start_intr_legacy(struct ath10k *ar)
 		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
 		  PCIE_SOC_WAKE_ADDRESS);
 
-	ath10k_pci_wait(ar);
+	ret = ath10k_pci_wait(ar);
+	if (ret) {
+		ath10k_err("target did not wake up\n");
+		free_irq(ar_pci->pdev->irq, ar);
+		return ret;
+	}
 
 	/*
 	 * A potential race occurs here: The CORE_BASE write
@@ -2019,13 +2028,18 @@ static int ath10k_pci_reset_target(struct ath10k *ar)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 	int wait_limit = 300; /* 3 sec */
+	int ret = 0;
 
 	/* Wait for Target to finish initialization before we proceed. */
 	iowrite32(PCIE_SOC_WAKE_V_MASK,
 		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
 		  PCIE_SOC_WAKE_ADDRESS);
 
-	ath10k_pci_wait(ar);
+	ret = ath10k_pci_wait(ar);
+	if (ret) {
+		ath10k_warn("target did not wake up\n");
+		goto exit;
+	}
 
 	while (wait_limit-- &&
 	       !(ioread32(ar_pci->mem + FW_INDICATOR_ADDRESS) &
@@ -2040,18 +2054,19 @@ static int ath10k_pci_reset_target(struct ath10k *ar)
 	}
 
 	if (wait_limit < 0) {
-		ath10k_err("Target stalled\n");
+		ath10k_warn("target stalled\n");
 		iowrite32(PCIE_SOC_WAKE_RESET,
 			  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
 			  PCIE_SOC_WAKE_ADDRESS);
-		return -EIO;
+		ret = -EIO;
+		goto exit;
 	}
 
+exit:
 	iowrite32(PCIE_SOC_WAKE_RESET,
 		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
 		  PCIE_SOC_WAKE_ADDRESS);
-
-	return 0;
+	return ret;
 }
 
 static void ath10k_pci_device_reset(struct ath10k_pci *ar_pci)
@@ -2128,6 +2143,7 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 	struct ath10k *ar;
 	struct ath10k_pci *ar_pci;
 	u32 lcr_val;
+	int reset_retries = 3;
 
 	ath10k_dbg(ATH10K_DBG_PCI, "%s\n", __func__);
 
@@ -2252,11 +2268,21 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
 	 * is in an unexpected state. We try to catch that here in order to
 	 * reset the Target and retry the probe.
 	 */
-	ath10k_pci_device_reset(ar_pci);
+	while (reset_retries-- > 0) {
+		ath10k_pci_device_reset(ar_pci);
 
-	ret = ath10k_pci_reset_target(ar);
-	if (ret)
+		ret = ath10k_pci_reset_target(ar);
+		if (ret == 0)
+			break;
+
+		ath10k_warn("retrying to reset target (%d tries left)\n",
+			    reset_retries);
+	}
+
+	if (ret) {
+		ath10k_err("could not reset target (%d)\n", ret);
 		goto err_intr;
+	}
 
 	if (ath10k_target_ps) {
 		ath10k_dbg(ATH10K_DBG_PCI, "on-chip power save enabled\n");
-- 
1.7.9.5

[ath9k-devel] [PATCH] ath10k: avoid infinite loop

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

Thanks, applied.

-- 
Kalle Valo

[ath9k-devel] [RFT] ath10k: retry target reset

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> Sometimes the device just won't reset cleanly
> without retrying. This seems to depend on the host
> hardware.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
>
> I can't reproduce the issue on my hw. Can someone
> confirm if this fixes the problem?

If you are referencing to the crash I reported to you privately, I have
seen that problem only once and haven't been able to reproduce since. So
I can't really test this patch.

> @@ -1931,7 +1935,12 @@ static int ath10k_pci_start_intr_legacy(struct ath10k *ar)
>  		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
>  		  PCIE_SOC_WAKE_ADDRESS);
>  
> -	ath10k_pci_wait(ar);
> +	ret = ath10k_pci_wait(ar);
> +	if (ret) {
> +		ath10k_err("target did not wake up\n");
> +		free_irq(ar_pci->pdev->irq, ar);
> +		return ret;
> +	}
>  
>  	/*
>  	 * A potential race occurs here: The CORE_BASE write
> @@ -2019,13 +2028,18 @@ static int ath10k_pci_reset_target(struct ath10k *ar)
>  {
>  	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
>  	int wait_limit = 300; /* 3 sec */
> +	int ret = 0;
>  
>  	/* Wait for Target to finish initialization before we proceed. */
>  	iowrite32(PCIE_SOC_WAKE_V_MASK,
>  		  ar_pci->mem + PCIE_LOCAL_BASE_ADDRESS +
>  		  PCIE_SOC_WAKE_ADDRESS);
>  
> -	ath10k_pci_wait(ar);
> +	ret = ath10k_pci_wait(ar);
> +	if (ret) {
> +		ath10k_warn("target did not wake up\n");
> +		goto exit;

You have same warnings messages in different places now. I recommend
adding "..for start" and "...for reset", or something like that, to
exactly identify the warning location. Example:

ath10k_warn("target did not wake up for reset\n");

-- 
Kalle Valo

[ath9k-devel] [RFT] ath10k: retry target reset

[Michal Kazior]

On 09/05/13 08:57, Kalle Valo wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> Sometimes the device just won't reset cleanly
>> without retrying. This seems to depend on the host
>> hardware.
>>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>> ---
>>
>> I can't reproduce the issue on my hw. Can someone
>> confirm if this fixes the problem?
>
> If you are referencing to the crash I reported to you privately, I have
> seen that problem only once and haven't been able to reproduce since. So
> I can't really test this patch.

Yes. This is both fortunate, and unfortunate :) The patch is actually my 
best guess what that issue might've been related to.


> You have same warnings messages in different places now. I recommend
> adding "..for start" and "...for reset", or something like that, to
> exactly identify the warning location. Example:
>
> ath10k_warn("target did not wake up for reset\n");

Good catch. Thanks!


-- Pozdrawiam / Best regards, Michal Kazior.

[ath9k-devel] [RFT] ath10k: retry target reset

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> On 09/05/13 08:57, Kalle Valo wrote:
>> Michal Kazior <michal.kazior@tieto.com> writes:
>>
>>> Sometimes the device just won't reset cleanly
>>> without retrying. This seems to depend on the host
>>> hardware.
>>>
>>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>>> ---
>>>
>>> I can't reproduce the issue on my hw. Can someone
>>> confirm if this fixes the problem?
>>
>> If you are referencing to the crash I reported to you privately, I have
>> seen that problem only once and haven't been able to reproduce since. So
>> I can't really test this patch.
>
> Yes. This is both fortunate, and unfortunate :) The patch is actually
> my best guess what that issue might've been related to.

At least your patch should help with the symptoms after the problem
appears. I will let you know if I see the problem again.

-- 
Kalle Valo