2010-12-22 15:44:31

by Senthil Balasubramanian

[permalink] [raw]
Subject: [PATCH v3 1/2] ath9k_hw: Fix incorrect macversion and macrev checks

There are few places where we are checking for macversion and revsions
before RTC is powered ON. However we are reading the macversion and
revisions only after RTC is powered ON and so both macversion and
revisions are actully zero and this leads to incorrect srev checks

Incorrect srev checks can cause registers to be configured wrongly and can
cause unexpected behavior. Fixing this seems to address the ASPM issue that
we have observed. The laptop becomes very slow and hangs mostly with ASPM L1
enabled without this fix.

fix this by reading the macversion and revisisons even before we start
using them. There is no reason why should we delay reading this info
until RTC is powered on as this is just a register information.

Cc: Stable Kernel <[email protected]>
Signed-off-by: Senthil Balasubramanian <[email protected]>
---
v2 -- fixed reading revisions unnecessarily during every reset.
v3 -- added more information to the commit log.

drivers/net/wireless/ath/ath9k/hw.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index 4b51ed4..0a0ba80 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -491,6 +491,8 @@ static int __ath9k_hw_init(struct ath_hw *ah)
if (ah->hw_version.devid == AR5416_AR9100_DEVID)
ah->hw_version.macVersion = AR_SREV_VERSION_9100;

+ ath9k_hw_read_revisions(ah);
+
if (!ath9k_hw_set_reset_reg(ah, ATH9K_RESET_POWER_ON)) {
ath_err(common, "Couldn't reset chip\n");
return -EIO;
@@ -1078,8 +1080,6 @@ static bool ath9k_hw_set_reset_power_on(struct ath_hw *ah)
return false;
}

- ath9k_hw_read_revisions(ah);
-
return ath9k_hw_set_reset(ah, ATH9K_RESET_WARM);
}

--
1.7.3.4



2010-12-22 15:52:26

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] ath9k_hw: Fix incorrect macversion and macrev checks

On Wed, Dec 22, 2010 at 10:44 AM, Senthil Balasubramanian
<[email protected]> wrote:
> There are few places where we are checking for macversion and revsions
> before RTC is powered ON. However we are reading the macversion and
> revisions only after RTC is powered ON and so both macversion and
> revisions are actully zero and this leads to incorrect srev checks
>
> Incorrect srev checks can cause registers to be configured wrongly and can
> cause unexpected behavior. Fixing this seems to address the ASPM issue that
> we have observed. The laptop becomes very slow and hangs mostly with ASPM L1
> enabled without this fix.

On what chipset? Or did we see this on one chipset and suspect it can
affect others?

Luis

2010-12-23 02:04:47

by Senthil Balasubramanian

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] ath9k_hw: Fix incorrect macversion and macrev checks

On Thu, Dec 23, 2010 at 07:25:42AM +0530, Senthilkumar Balasubramanian wrote:
> On Wed, Dec 22, 2010 at 09:22:06PM +0530, Luis R. Rodriguez wrote:
> > On Wed, Dec 22, 2010 at 10:44 AM, Senthil Balasubramanian
> > <[email protected]> wrote:
> > > There are few places where we are checking for macversion and revsions
> > > before RTC is powered ON. However we are reading the macversion and
> > > revisions only after RTC is powered ON and so both macversion and
> > > revisions are actully zero and this leads to incorrect srev checks
> > >
> > > Incorrect srev checks can cause registers to be configured wrongly and can
> > > cause unexpected behavior. Fixing this seems to address the ASPM issue that
> > > we have observed. The laptop becomes very slow and hangs mostly with ASPM L1
> > > enabled without this fix.
> >
> > On what chipset? Or did we see this on one chipset and suspect it can
> > affect others?
> We have noticed on Merlin and it is obviously incorrect for other chipsets
This should be read as "we have noticed this issue with ar9380/ar9382"
> also :-(. Please feel free to rephrase/rework on this patch if you think
> something is still lacking/missing...
> >
> > Luis

2010-12-23 02:08:21

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] ath9k_hw: Fix incorrect macversion and macrev checks

On Wed, Dec 22, 2010 at 9:04 PM, Senthil Balasubramanian
<[email protected]> wrote:
> On Thu, Dec 23, 2010 at 07:25:42AM +0530, Senthilkumar Balasubramanian wrote:
>> On Wed, Dec 22, 2010 at 09:22:06PM +0530, Luis R. Rodriguez wrote:
>> > On Wed, Dec 22, 2010 at 10:44 AM, Senthil Balasubramanian
>> > <[email protected]> wrote:
>> > > There are few places where we are checking for macversion and revsions
>> > > before RTC is powered ON. However we are reading the macversion and
>> > > revisions only after RTC is powered ON and so both macversion and
>> > > revisions are actully zero and this leads to incorrect srev checks
>> > >
>> > > Incorrect srev checks can cause registers to be configured wrongly and can
>> > > cause unexpected behavior. Fixing this seems to address the ASPM issue that
>> > > we have observed. The laptop becomes very slow and hangs mostly with ASPM L1
>> > > enabled without this fix.
>> >
>> > On what chipset? Or did we see this on one chipset and suspect it can
>> > affect others?
>> We have noticed on Merlin and it is obviously incorrect for other chipsets
> This should be read as "we have noticed this issue with ar9380/ar9382"

The reason I asked about the chipset is ar9380 stuff won't be relevant
for longterm 2.6.35.

Luis

2010-12-23 01:55:50

by Senthil Balasubramanian

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] ath9k_hw: Fix incorrect macversion and macrev checks

On Wed, Dec 22, 2010 at 09:22:06PM +0530, Luis R. Rodriguez wrote:
> On Wed, Dec 22, 2010 at 10:44 AM, Senthil Balasubramanian
> <[email protected]> wrote:
> > There are few places where we are checking for macversion and revsions
> > before RTC is powered ON. However we are reading the macversion and
> > revisions only after RTC is powered ON and so both macversion and
> > revisions are actully zero and this leads to incorrect srev checks
> >
> > Incorrect srev checks can cause registers to be configured wrongly and can
> > cause unexpected behavior. Fixing this seems to address the ASPM issue that
> > we have observed. The laptop becomes very slow and hangs mostly with ASPM L1
> > enabled without this fix.
>
> On what chipset? Or did we see this on one chipset and suspect it can
> affect others?
We have noticed on Merlin and it is obviously incorrect for other chipsets
also :-(. Please feel free to rephrase/rework on this patch if you think
something is still lacking/missing...
>
> Luis