[PATCH 1/2] ath9k_hw: Fix system hang when resuming from S3/S4

[PATCH 1/2] ath9k_hw: Fix system hang when resuming from S3/S4

[Rajkumar Manoharan]

The bit 6 & 7 of AR_WA (0x4004) should be enabled only
for the chips that are supporting L0s functionality
while resuming back from S3/S4.

Enabling these bits for AR9280 is causing system hang
within a few S3/S4-resume cycles.

Cc: Jack Lee <jlee@atheros.com>
Signed-off-by: Rajkumar Manoharan <rmanoharan@atheros.com>
---
 drivers/net/wireless/ath/ath9k/ar9002_hw.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ar9002_hw.c b/drivers/net/wireless/ath/ath9k/ar9002_hw.c
index f8a7771..f44c84a 100644
--- a/drivers/net/wireless/ath/ath9k/ar9002_hw.c
+++ b/drivers/net/wireless/ath/ath9k/ar9002_hw.c
@@ -426,9 +426,8 @@ static void ar9002_hw_configpcipowersave(struct ath_hw *ah,
 		}
 
 		/* WAR for ASPM system hang */
-		if (AR_SREV_9280(ah) || AR_SREV_9285(ah) || AR_SREV_9287(ah)) {
+		if (AR_SREV_9285(ah) || AR_SREV_9287(ah))
 			val |= (AR_WA_BIT6 | AR_WA_BIT7);
-		}
 
 		if (AR_SREV_9285E_20(ah))
 			val |= AR_WA_BIT23;
-- 
1.7.3.5

[PATCH 2/2] ath9k: Fix power save usage count imbalance on deinit

[Rajkumar Manoharan]

Upon unloading the driver, the ps_usecount is incremented
before configuring gpio registers in deinit_device.
But it is failed to restore the ps_usecount after that.
The problem is that the chip is moved to FULL SLEEP
by radio_disable when mac80211 is reporting as idle
though ps_usecount is not zero.

This patch retores ps_usecount properly and ensures that
the chip is always moved to full sleep only if ps usage
counte is zero which helps in debugging. And also fixes
the following warning.

ath: DMA failed to stop in 10 ms AR_CR=0xdeadbeef AR_DIAG_SW=0xdeadbeef
ath: Could not stop RX, we could be confusing the DMA engine when we
start RX up
------------[ cut here ]------------
WARNING: at drivers/net/wireless/ath/ath9k/recv.c:536
ath_stoprecv+0xf4/0x100 [ath9k]()

Cc: stable@kernel.org
Signed-off-by: Rajkumar Manoharan <rmanoharan@atheros.com>
---
 drivers/net/wireless/ath/ath9k/init.c |    2 ++
 drivers/net/wireless/ath/ath9k/main.c |    2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 767d8b8..b38c874 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -824,6 +824,8 @@ void ath9k_deinit_device(struct ath_softc *sc)
 	wiphy_rfkill_stop_polling(sc->hw->wiphy);
 	ath_deinit_leds(sc);
 
+	ath9k_ps_restore(sc);
+
 	for (i = 0; i < sc->num_sec_wiphy; i++) {
 		struct ath_wiphy *aphy = sc->sec_wiphy[i];
 		if (aphy == NULL)
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index c03184e..aff1c72 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -966,8 +966,6 @@ void ath_radio_disable(struct ath_softc *sc, struct ieee80211_hw *hw)
 
 	spin_unlock_bh(&sc->sc_pcu_lock);
 	ath9k_ps_restore(sc);
-
-	ath9k_setpower(sc, ATH9K_PM_FULL_SLEEP);
 }
 
 int ath_reset(struct ath_softc *sc, bool retry_tx)
-- 
1.7.3.5

Re: [PATCH 2/2] ath9k: Fix power save usage count imbalance on deinit

[Luis R. Rodriguez]

On Wed, Jan 19, 2011 at 7:47 AM, Rajkumar Manoharan
<rmanoharan@atheros.com> wrote:
> Upon unloading the driver, the ps_usecount is incremented
> before configuring gpio registers in deinit_device.
> But it is failed to restore the ps_usecount after that.
> The problem is that the chip is moved to FULL SLEEP
> by radio_disable when mac80211 is reporting as idle
> though ps_usecount is not zero.
>
> This patch retores ps_usecount properly and ensures that
> the chip is always moved to full sleep only if ps usage
> counte is zero which helps in debugging. And also fixes
> the following warning.
>
> ath: DMA failed to stop in 10 ms AR_CR=0xdeadbeef AR_DIAG_SW=0xdeadbeef
> ath: Could not stop RX, we could be confusing the DMA engine when we
> start RX up
> ------------[ cut here ]------------
> WARNING: at drivers/net/wireless/ath/ath9k/recv.c:536
> ath_stoprecv+0xf4/0x100 [ath9k]()
>
> Cc: stable@kernel.org
> Signed-off-by: Rajkumar Manoharan <rmanoharan@atheros.com>
> ---
>  drivers/net/wireless/ath/ath9k/init.c |    2 ++
>  drivers/net/wireless/ath/ath9k/main.c |    2 --
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
> index 767d8b8..b38c874 100644
> --- a/drivers/net/wireless/ath/ath9k/init.c
> +++ b/drivers/net/wireless/ath/ath9k/init.c
> @@ -824,6 +824,8 @@ void ath9k_deinit_device(struct ath_softc *sc)
>        wiphy_rfkill_stop_polling(sc->hw->wiphy);
>        ath_deinit_leds(sc);
>
> +       ath9k_ps_restore(sc);
> +
>        for (i = 0; i < sc->num_sec_wiphy; i++) {
>                struct ath_wiphy *aphy = sc->sec_wiphy[i];
>                if (aphy == NULL)
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index c03184e..aff1c72 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -966,8 +966,6 @@ void ath_radio_disable(struct ath_softc *sc, struct ieee80211_hw *hw)
>
>        spin_unlock_bh(&sc->sc_pcu_lock);
>        ath9k_ps_restore(sc);
> -
> -       ath9k_setpower(sc, ATH9K_PM_FULL_SLEEP);

Are you sure this hunk does not regress the suspend/resume case when
using the new dbus API?

  Luis

Re: [PATCH 2/2] ath9k: Fix power save usage count imbalance on deinit

[Rajkumar Manoharan]

On Thu, Jan 20, 2011 at 12:54:56AM +0530, Luis R. Rodriguez wrote:
> On Wed, Jan 19, 2011 at 7:47 AM, Rajkumar Manoharan
> <rmanoharan@atheros.com> wrote:
> > Upon unloading the driver, the ps_usecount is incremented
> > before configuring gpio registers in deinit_device.
> > But it is failed to restore the ps_usecount after that.
> > The problem is that the chip is moved to FULL SLEEP
> > by radio_disable when mac80211 is reporting as idle
> > though ps_usecount is not zero.
> >
> > This patch retores ps_usecount properly and ensures that
> > the chip is always moved to full sleep only if ps usage
> > counte is zero which helps in debugging. And also fixes
> > the following warning.
> >
> > ath: DMA failed to stop in 10 ms AR_CR=0xdeadbeef AR_DIAG_SW=0xdeadbeef
> > ath: Could not stop RX, we could be confusing the DMA engine when we
> > start RX up
> > ------------[ cut here ]------------
> > WARNING: at drivers/net/wireless/ath/ath9k/recv.c:536
> > ath_stoprecv+0xf4/0x100 [ath9k]()
> >
> 
> Are you sure this hunk does not regress the suspend/resume case when
> using the new dbus API?
>
I verfied the suspend/resume case with AR9280 card. But I don't
understand how the new dbus API is related with this patch?

--
Rajkumar

Re: [PATCH 2/2] ath9k: Fix power save usage count imbalance on deinit

[Luis R. Rodriguez]

On Wed, Jan 19, 2011 at 11:51 PM, Rajkumar Manoharan
<rmanoharan@atheros.com> wrote:
> On Thu, Jan 20, 2011 at 12:54:56AM +0530, Luis R. Rodriguez wrote:
>> On Wed, Jan 19, 2011 at 7:47 AM, Rajkumar Manoharan
>> <rmanoharan@atheros.com> wrote:
>> > Upon unloading the driver, the ps_usecount is incremented
>> > before configuring gpio registers in deinit_device.
>> > But it is failed to restore the ps_usecount after that.
>> > The problem is that the chip is moved to FULL SLEEP
>> > by radio_disable when mac80211 is reporting as idle
>> > though ps_usecount is not zero.
>> >
>> > This patch retores ps_usecount properly and ensures that
>> > the chip is always moved to full sleep only if ps usage
>> > counte is zero which helps in debugging. And also fixes
>> > the following warning.
>> >
>> > ath: DMA failed to stop in 10 ms AR_CR=0xdeadbeef AR_DIAG_SW=0xdeadbeef
>> > ath: Could not stop RX, we could be confusing the DMA engine when we
>> > start RX up
>> > ------------[ cut here ]------------
>> > WARNING: at drivers/net/wireless/ath/ath9k/recv.c:536
>> > ath_stoprecv+0xf4/0x100 [ath9k]()
>> >
>>
>> Are you sure this hunk does not regress the suspend/resume case when
>> using the new dbus API?
>>
> I verfied the suspend/resume case with AR9280 card. But I don't
> understand how the new dbus API is related with this patch?

Paul, if you get a chance to give this a spin before it gets merged
it'd be appreciated.

  Luis

Re: [PATCH 2/2] ath9k: Fix power save usage count imbalance on deinit

[Rajkumar Manoharan]

On Fri, Jan 21, 2011 at 04:32:20AM +0530, Luis R. Rodriguez wrote:
> On Wed, Jan 19, 2011 at 11:51 PM, Rajkumar Manoharan
> <rmanoharan@atheros.com> wrote:
> > On Thu, Jan 20, 2011 at 12:54:56AM +0530, Luis R. Rodriguez wrote:
> >> On Wed, Jan 19, 2011 at 7:47 AM, Rajkumar Manoharan
> >> <rmanoharan@atheros.com> wrote:
> >> > Upon unloading the driver, the ps_usecount is incremented
> >> > before configuring gpio registers in deinit_device.
> >> > But it is failed to restore the ps_usecount after that.
> >> > The problem is that the chip is moved to FULL SLEEP
> >> > by radio_disable when mac80211 is reporting as idle
> >> > though ps_usecount is not zero.
> >> >
> >> > This patch retores ps_usecount properly and ensures that
> >> > the chip is always moved to full sleep only if ps usage
> >> > counte is zero which helps in debugging. And also fixes
> >> > the following warning.
> >> >
> >> > ath: DMA failed to stop in 10 ms AR_CR=0xdeadbeef AR_DIAG_SW=0xdeadbeef
> >> > ath: Could not stop RX, we could be confusing the DMA engine when we
> >> > start RX up
> >> > ------------[ cut here ]------------
> >> > WARNING: at drivers/net/wireless/ath/ath9k/recv.c:536
> >> > ath_stoprecv+0xf4/0x100 [ath9k]()
> >> >
> >>
> >> Are you sure this hunk does not regress the suspend/resume case when
> >> using the new dbus API?
> >>
> > I verfied the suspend/resume case with AR9280 card. But I don't
> > understand how the new dbus API is related with this patch?
> 
> Paul, if you get a chance to give this a spin before it gets merged
> it'd be appreciated.
Paul,
Did you get a chance to evaluate this patch? Shall we proceed further?

--
Rajkumar

Re: [PATCH 2/2] ath9k: Fix power save usage count imbalance on deinit

[Paul Stewart]

This change doesn't regress the issue with scan-during-suspend.  The
other regression you should test for is that the system really does
continue to honor powersave after a suspend-resume cycle (verify
null-functions and drop in power consumption).  You should be able to
do that yourself, and I'd suggest testing for this case as well before
committing any changes to powersave.

--
Paul

On Sat, Jan 22, 2011 at 11:21 PM, Rajkumar Manoharan
<rmanoharan@atheros.com> wrote:
> On Fri, Jan 21, 2011 at 04:32:20AM +0530, Luis R. Rodriguez wrote:
>> On Wed, Jan 19, 2011 at 11:51 PM, Rajkumar Manoharan
>> <rmanoharan@atheros.com> wrote:
>> > On Thu, Jan 20, 2011 at 12:54:56AM +0530, Luis R. Rodriguez wrote:
>> >> On Wed, Jan 19, 2011 at 7:47 AM, Rajkumar Manoharan
>> >> <rmanoharan@atheros.com> wrote:
>> >> > Upon unloading the driver, the ps_usecount is incremented
>> >> > before configuring gpio registers in deinit_device.
>> >> > But it is failed to restore the ps_usecount after that.
>> >> > The problem is that the chip is moved to FULL SLEEP
>> >> > by radio_disable when mac80211 is reporting as idle
>> >> > though ps_usecount is not zero.
>> >> >
>> >> > This patch retores ps_usecount properly and ensures that
>> >> > the chip is always moved to full sleep only if ps usage
>> >> > counte is zero which helps in debugging. And also fixes
>> >> > the following warning.
>> >> >
>> >> > ath: DMA failed to stop in 10 ms AR_CR=0xdeadbeef AR_DIAG_SW=0xdeadbeef
>> >> > ath: Could not stop RX, we could be confusing the DMA engine when we
>> >> > start RX up
>> >> > ------------[ cut here ]------------
>> >> > WARNING: at drivers/net/wireless/ath/ath9k/recv.c:536
>> >> > ath_stoprecv+0xf4/0x100 [ath9k]()
>> >> >
>> >>
>> >> Are you sure this hunk does not regress the suspend/resume case when
>> >> using the new dbus API?
>> >>
>> > I verfied the suspend/resume case with AR9280 card. But I don't
>> > understand how the new dbus API is related with this patch?
>>
>> Paul, if you get a chance to give this a spin before it gets merged
>> it'd be appreciated.
> Paul,
> Did you get a chance to evaluate this patch? Shall we proceed further?
>
> --
> Rajkumar
>

Re: [PATCH 2/2] ath9k: Fix power save usage count imbalance on deinit

[Rajkumar Manoharan]

On Tue, Jan 25, 2011 at 05:33:31AM +0530, Paul Stewart wrote:
> This change doesn't regress the issue with scan-during-suspend.  The
> other regression you should test for is that the system really does
> continue to honor powersave after a suspend-resume cycle (verify
> null-functions and drop in power consumption).  You should be able to
> do that yourself, and I'd suggest testing for this case as well before
> committing any changes to powersave.
>
Thanks a lot Paul for validating the patch. Sure I will do more testing
with this patch.

--
Rajkumar