2012-06-18 07:43:43

by Mohammed Shafi Shajakhan

Subject: [PATCH] ath9k_hw: avoid possible infinite loop in ar9003_get_pll_sqsum_dvc

From: Mohammed Shafi Shajakhan <[email protected]>

"ath9k: Fix softlockup in AR9485" with commit id
64bc1239c790e051ff677e023435d770d2ffa174 fixed the reported
issue, yet its better to avoid the possible infinite loop
in ar9003_get_pll_sqsum_dvc by having a timeout as suggested
by ath9k maintainers.
http://www.spinics.net/lists/linux-wireless/msg92126.html.
Based on my testing PLL's locking measurement is done in
~200us (2 iterations).

Cc: [email protected]
Cc: Rolf Offermanns <[email protected]>
Cc: Sujith Manoharan <[email protected]>
Cc: Senthil Balasubramanian <[email protected]>
Signed-off-by: Mohammed Shafi Shajakhan <[email protected]>
---
drivers/net/wireless/ath/ath9k/hw.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index 784baee..a42c26f 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -773,13 +773,25 @@ static void ath9k_hw_init_qos(struct ath_hw *ah)

u32 ar9003_get_pll_sqsum_dvc(struct ath_hw *ah)
{
+ struct ath_common *common = ath9k_hw_common(ah);
+ int i = 0;
+
REG_CLR_BIT(ah, PLL3, PLL3_DO_MEAS_MASK);
udelay(100);
REG_SET_BIT(ah, PLL3, PLL3_DO_MEAS_MASK);

- while ((REG_READ(ah, PLL4) & PLL4_MEAS_DONE) == 0)
+ while ((REG_READ(ah, PLL4) & PLL4_MEAS_DONE) == 0) {
+
udelay(100);

+ if (WARN_ON_ONCE(i >= 100)) {
+ ath_err(common, "PLL4 meaurement not done\n");
+ break;
+ }
+
+ i++;
+ }
+
return (REG_READ(ah, PLL3) & SQSUM_DVC_MASK) >> 3;
}
EXPORT_SYMBOL(ar9003_get_pll_sqsum_dvc);
--
1.7.0.4

2012-06-18 08:03:45

by Rajkumar Manoharan

Subject: Re: [PATCH] ath9k_hw: avoid possible infinite loop in ar9003_get_pll_sqsum_dvc

On Mon, Jun 18, 2012 at 01:13:30PM +0530, Mohammed Shafi Shajakhan wrote:
> From: Mohammed Shafi Shajakhan <[email protected]>
>
> "ath9k: Fix softlockup in AR9485" with commit id
> 64bc1239c790e051ff677e023435d770d2ffa174 fixed the reported
> issue, yet its better to avoid the possible infinite loop
> in ar9003_get_pll_sqsum_dvc by having a timeout as suggested
> by ath9k maintainers.
> http://www.spinics.net/lists/linux-wireless/msg92126.html.
> Based on my testing PLL's locking measurement is done in
> ~200us (2 iterations).
>
> Cc: [email protected]
> Cc: Rolf Offermanns <[email protected]>
> Cc: Sujith Manoharan <[email protected]>
> Cc: Senthil Balasubramanian <[email protected]>
> Signed-off-by: Mohammed Shafi Shajakhan <[email protected]>
> ---
> drivers/net/wireless/ath/ath9k/hw.c | 14 +++++++++++++-
> 1 files changed, 13 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
> index 784baee..a42c26f 100644
> --- a/drivers/net/wireless/ath/ath9k/hw.c
> +++ b/drivers/net/wireless/ath/ath9k/hw.c
> @@ -773,13 +773,25 @@ static void ath9k_hw_init_qos(struct ath_hw *ah)
>
> u32 ar9003_get_pll_sqsum_dvc(struct ath_hw *ah)
> {
> + struct ath_common *common = ath9k_hw_common(ah);
> + int i = 0;
> +
> REG_CLR_BIT(ah, PLL3, PLL3_DO_MEAS_MASK);
> udelay(100);
> REG_SET_BIT(ah, PLL3, PLL3_DO_MEAS_MASK);
>
> - while ((REG_READ(ah, PLL4) & PLL4_MEAS_DONE) == 0)
> + while ((REG_READ(ah, PLL4) & PLL4_MEAS_DONE) == 0) {
> +
> udelay(100);
>
> + if (WARN_ON_ONCE(i >= 100)) {
> + ath_err(common, "PLL4 meaurement not done\n");
> + break;
> + }
> +
> + i++;
How did you come up with 100 as max limit to break the loop?

-Rajkumar

2012-06-18 09:58:11

by Mohammed Shafi Shajakhan

Subject: Re: [PATCH] ath9k_hw: avoid possible infinite loop in ar9003_get_pll_sqsum_dvc

Hi Raj,

On Monday 18 June 2012 01:34 PM, Rajkumar Manoharan wrote:
> On Mon, Jun 18, 2012 at 01:13:30PM +0530, Mohammed Shafi Shajakhan wrote:
>> From: Mohammed Shafi Shajakhan<[email protected]>
>>
>> "ath9k: Fix softlockup in AR9485" with commit id
>> 64bc1239c790e051ff677e023435d770d2ffa174 fixed the reported
>> issue, yet its better to avoid the possible infinite loop
>> in ar9003_get_pll_sqsum_dvc by having a timeout as suggested
>> by ath9k maintainers.
>> http://www.spinics.net/lists/linux-wireless/msg92126.html.
>> Based on my testing PLL's locking measurement is done in
>> ~200us (2 iterations).
>>
>> Cc: [email protected]
>> Cc: Rolf Offermanns<[email protected]>
>> Cc: Sujith Manoharan<[email protected]>
>> Cc: Senthil Balasubramanian<[email protected]>
>> Signed-off-by: Mohammed Shafi Shajakhan<[email protected]>
>> ---
>> drivers/net/wireless/ath/ath9k/hw.c | 14 +++++++++++++-
>> 1 files changed, 13 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
>> index 784baee..a42c26f 100644
>> --- a/drivers/net/wireless/ath/ath9k/hw.c
>> +++ b/drivers/net/wireless/ath/ath9k/hw.c
>> @@ -773,13 +773,25 @@ static void ath9k_hw_init_qos(struct ath_hw *ah)
>>
>> u32 ar9003_get_pll_sqsum_dvc(struct ath_hw *ah)
>> {
>> + struct ath_common *common = ath9k_hw_common(ah);
>> + int i = 0;
>> +
>> REG_CLR_BIT(ah, PLL3, PLL3_DO_MEAS_MASK);
>> udelay(100);
>> REG_SET_BIT(ah, PLL3, PLL3_DO_MEAS_MASK);
>>
>> - while ((REG_READ(ah, PLL4)& PLL4_MEAS_DONE) == 0)
>> + while ((REG_READ(ah, PLL4)& PLL4_MEAS_DONE) == 0) {
>> +
>> udelay(100);
>>
>> + if (WARN_ON_ONCE(i>= 100)) {
>> + ath_err(common, "PLL4 meaurement not done\n");
>> + break;
>> + }
>> +
>> + i++;
> How did you come up with 100 as max limit to break the loop?
>

i assumed its a sufficient arbitrary max value to confirm that the PLL
locking measurement does not happens. Based on my quite a few of
tests(idle associated, with traffic)PLL immediately locksup in 200us (2
iterations). As per your suggestion i will quickly check with the
System's team if we can give a proper value, rather than 100 * 100 us as
timeout. thanks for reviewing it!

--
thanks,
shafi