All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mohammed Shafi Shajakhan <mohammed@qca.qualcomm.com>
To: ath9k-devel@lists.ath9k.org
Subject: [ath9k-devel] [RFC] ath9k: Fix softlockup in AR9485
Date: Thu, 31 May 2012 11:04:28 +0530	[thread overview]
Message-ID: <4FC702E4.9000906@qca.qualcomm.com> (raw)
In-Reply-To: <20422.56106.127697.13731@gargle.gargle.HOWL>

Hi Sujith,

On Thursday 31 May 2012 08:14 AM, Sujith Manoharan wrote:
> Mohammed Shafi Shajakhan wrote:
>> before the STA is associated PLL4(0x1618c) always seem to dump
>> zero, which causes a softlockup as we keep on polling infinitely
>> PLL4's 8th bit. Once PLL4'S 8th bit is set indicates we can take
>> PLL3(0x16188) readings which tells us whether PLL is locked or not.
>> if the PLL is not locked we have a rx hang.
>> It does not looks safe to poll PLL4 before the STA is associated.
>> so it is safe to check this  rx hang after association, where we might
>> have a possibility of rx hang under stress testing. digging into the
>> PLL stuff did not yield anything much better.
>>
>> previously the register dumped 0xdeadbeef and the hw_pll_work
>> itself is cancelled by ath_radio_disable(obselete)
>>
>> fixes https://bugzilla.redhat.com/show_bug.cgi?id=811142
>>
>> Reported-by: Rolf Offermanns<rolf.offermanns@gmx.net>
>> Cc: stable at vger.kernel.org
>> Signed-off-by: Mohammed Shafi Shajakhan<mohammed@qca.qualcomm.com>
>> ---
>>   drivers/net/wireless/ath/ath9k/main.c |    3 +++
>>   1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>> index dfa78e8..a03ad3e 100644
>> --- a/drivers/net/wireless/ath/ath9k/main.c
>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>> @@ -970,6 +970,9 @@ void ath_hw_pll_work(struct work_struct *work)
>>   					    hw_pll_work.work);
>>   	u32 pll_sqsum;
>>
>> +	if (!(sc->sc_flags&  SC_OP_PRIM_STA_VIF))
>> +		return;
>> +
>>   	if (AR_SREV_9485(sc->sc_ah)) {
>>
>>   		ath9k_ps_wakeup(sc);
>
>
> All this is really racy, I think. In the various callsites where we issue a HW
> reset by queuing a work, there is no guarantee that the caller would not be
> invoked once again before the reset work has had a chance to kick in. And there is
> nothing that prevents the various poll routines from checking the HW while a
> reset in progress.

should we check  if (work_pending(&sc->hw_reset_work)) like we do in
ath_beacon_tasklet in all the other work routine that access hardware 
registers in case the hw_reset_work is not cancelled(possible in 
ath_set_channel)


>
> And why are we using HZ for the pll_work ?

don't know, i put a printk to see the routine seems to get executed for 
every 200ms(HZ/5). scan work in mac80211 seems to use HZ
ieee80211_queue_delayed_work(&local->hw, &local->scan_work, next_delay);


>
> Sujith


-- 
thanks,
shafi

WARNING: multiple messages have this Message-ID (diff)
From: Mohammed Shafi Shajakhan <mohammed@qca.qualcomm.com>
To: Sujith Manoharan <c_manoha@qca.qualcomm.com>
Cc: "John W. Linville" <linville@tuxdriver.com>,
	<linux-wireless@vger.kernel.org>,
	Mohammed Shafi Shajakhan <mohammed@qca.qualcomm.com>,
	<stable@vger.kernel.org>,
	Rodriguez Luis <rodrigue@qca.qualcomm.com>,
	<ath9k-devel@lists.ath9k.org>
Subject: Re: [ath9k-devel] [RFC] ath9k: Fix softlockup in AR9485
Date: Thu, 31 May 2012 11:04:28 +0530	[thread overview]
Message-ID: <4FC702E4.9000906@qca.qualcomm.com> (raw)
In-Reply-To: <20422.56106.127697.13731@gargle.gargle.HOWL>

Hi Sujith,

On Thursday 31 May 2012 08:14 AM, Sujith Manoharan wrote:
> Mohammed Shafi Shajakhan wrote:
>> before the STA is associated PLL4(0x1618c) always seem to dump
>> zero, which causes a softlockup as we keep on polling infinitely
>> PLL4's 8th bit. Once PLL4'S 8th bit is set indicates we can take
>> PLL3(0x16188) readings which tells us whether PLL is locked or not.
>> if the PLL is not locked we have a rx hang.
>> It does not looks safe to poll PLL4 before the STA is associated.
>> so it is safe to check this  rx hang after association, where we might
>> have a possibility of rx hang under stress testing. digging into the
>> PLL stuff did not yield anything much better.
>>
>> previously the register dumped 0xdeadbeef and the hw_pll_work
>> itself is cancelled by ath_radio_disable(obselete)
>>
>> fixes https://bugzilla.redhat.com/show_bug.cgi?id=811142
>>
>> Reported-by: Rolf Offermanns<rolf.offermanns@gmx.net>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Mohammed Shafi Shajakhan<mohammed@qca.qualcomm.com>
>> ---
>>   drivers/net/wireless/ath/ath9k/main.c |    3 +++
>>   1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>> index dfa78e8..a03ad3e 100644
>> --- a/drivers/net/wireless/ath/ath9k/main.c
>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>> @@ -970,6 +970,9 @@ void ath_hw_pll_work(struct work_struct *work)
>>   					    hw_pll_work.work);
>>   	u32 pll_sqsum;
>>
>> +	if (!(sc->sc_flags&  SC_OP_PRIM_STA_VIF))
>> +		return;
>> +
>>   	if (AR_SREV_9485(sc->sc_ah)) {
>>
>>   		ath9k_ps_wakeup(sc);
>
>
> All this is really racy, I think. In the various callsites where we issue a HW
> reset by queuing a work, there is no guarantee that the caller would not be
> invoked once again before the reset work has had a chance to kick in. And there is
> nothing that prevents the various poll routines from checking the HW while a
> reset in progress.

should we check  if (work_pending(&sc->hw_reset_work)) like we do in
ath_beacon_tasklet in all the other work routine that access hardware 
registers in case the hw_reset_work is not cancelled(possible in 
ath_set_channel)


>
> And why are we using HZ for the pll_work ?

don't know, i put a printk to see the routine seems to get executed for 
every 200ms(HZ/5). scan work in mac80211 seems to use HZ
ieee80211_queue_delayed_work(&local->hw, &local->scan_work, next_delay);


>
> Sujith


-- 
thanks,
shafi

  reply	other threads:[~2012-05-31  5:34 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-30 16:10 [ath9k-devel] [RFC] ath9k: Fix softlockup in AR9485 Mohammed Shafi Shajakhan
2012-05-30 16:10 ` Mohammed Shafi Shajakhan
2012-05-30 18:48 ` [ath9k-devel] " Adrian Chadd
2012-05-30 18:48   ` Adrian Chadd
2012-05-31  4:49   ` [ath9k-devel] " Mohammed Shafi Shajakhan
2012-05-31  4:49     ` Mohammed Shafi Shajakhan
2012-05-31  2:44 ` [ath9k-devel] " Sujith Manoharan
2012-05-31  2:44   ` Sujith Manoharan
2012-05-31  5:34   ` Mohammed Shafi Shajakhan [this message]
2012-05-31  5:34     ` Mohammed Shafi Shajakhan
2012-05-31  5:41     ` Sujith Manoharan
2012-05-31  5:41       ` Sujith Manoharan
2012-05-31  5:52       ` Mohammed Shafi Shajakhan
2012-05-31  5:52         ` Mohammed Shafi Shajakhan
2012-06-08 12:16         ` Sujith Manoharan
2012-06-08 12:16           ` Sujith Manoharan
2012-06-11  5:03           ` Mohammed Shafi Shajakhan
2012-06-11  5:03             ` Mohammed Shafi Shajakhan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FC702E4.9000906@qca.qualcomm.com \
    --to=mohammed@qca.qualcomm.com \
    --cc=ath9k-devel@lists.ath9k.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.