Re: APSM, AR9285 and bus hangs

Re: APSM, AR9285 and bus hangs

[Adrian Chadd]

On 13 June 2011 19:00, Stanislaw Gruszka <sgruszka@redhat.com> wrote:

> cam a fedora user, confirms that patch fixes the locks-ups
> https://bugzilla.redhat.com/show_bug.cgi?id=697157#c26
>
> We are waiting for fix now :-)

Cool.

The problem is, the initial modification and subsequent changes to the
powersave function were documented to fix an APSM hang, not cause it.

I'm happy to submit a patch to undo that particular change but I'm not
sure if it'll -break- things for other people. :)

Someone with PCIe/APCI/APSM/general clue in the area of CPU + bus
sleep states needs to step in and help. I'm just not equipped to fix
the problem at the moment.

Thanks,


Adrian

Re: APSM, AR9285 and bus hangs

[Camilo Mesias]

Hi,

On Mon, Jun 13, 2011 at 2:01 PM, Adrian Chadd <adrian@freebsd.org> wrote:
> I'm happy to submit a patch to undo that particular change but I'm not
> sure if it'll -break- things for other people. :)

I was wondering if it would be a pragmatic approach to let people turn
off APSM somehow. During testing I tried the pcie_aspm=off boot option
but got the impression the ath9k driver didn't honour it. Maybe an
option to the driver specifically to let people configure the
workaround?

With the tweaked driver my netbook has gone from practically useless
to almost completely satisfactory, so I see this as an important fix
to make available to the general public somehow.

-Cam

Re: APSM, AR9285 and bus hangs

[Stanislaw Gruszka]

On Mon, Jun 13, 2011 at 09:01:57PM +0800, Adrian Chadd wrote:
> On 13 June 2011 19:00, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> 
> > cam a fedora user, confirms that patch fixes the locks-ups
> > https://bugzilla.redhat.com/show_bug.cgi?id=697157#c26
> >
> > We are waiting for fix now :-)
> 
> Cool.
> 
> The problem is, the initial modification and subsequent changes to the
> powersave function were documented to fix an APSM hang, not cause it.
> 
> I'm happy to submit a patch to undo that particular change but I'm not
> sure if it'll -break- things for other people. :)
> 
> Someone with PCIe/APCI/APSM/general clue in the area of CPU + bus
> sleep states needs to step in and help. I'm just not equipped to fix
> the problem at the moment.

I see this commit in 2.6.39, missed in 2.6.38 and 2.6.35 currently.
Is possible this is the fix which do not break other things?

commit 5b64aa72ead6f8be488d2be7af579f0d69fb7a6e
Author: Rajkumar Manoharan <rmanoharan@atheros.com>
Date:   Thu Jan 27 18:39:37 2011 +0530

    ath9k_hw: Fix system hang when resuming from S3/S4
    
    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: stable@kernel.org
    Cc: Jack Lee <jlee@atheros.com>
    Signed-off-by: Rajkumar Manoharan <rmanoharan@atheros.com>
    Signed-off-by: John W. Linville <linville@tuxdriver.com>

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;

Re: APSM, AR9285 and bus hangs

[Mohammed Shafi]

On Mon, Jun 13, 2011 at 6:50 PM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> On Mon, Jun 13, 2011 at 09:01:57PM +0800, Adrian Chadd wrote:
>> On 13 June 2011 19:00, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
>>
>> > cam a fedora user, confirms that patch fixes the locks-ups
>> > https://bugzilla.redhat.com/show_bug.cgi?id=697157#c26
>> >
>> > We are waiting for fix now :-)
>>
>> Cool.
>>
>> The problem is, the initial modification and subsequent changes to the
>> powersave function were documented to fix an APSM hang, not cause it.
>>
>> I'm happy to submit a patch to undo that particular change but I'm not
>> sure if it'll -break- things for other people. :)
>>
>> Someone with PCIe/APCI/APSM/general clue in the area of CPU + bus
>> sleep states needs to step in and help. I'm just not equipped to fix
>> the problem at the moment.
>
> I see this commit in 2.6.39, missed in 2.6.38 and 2.6.35 currently.
> Is possible this is the fix which do not break other things?

hi,
no, this fix only affects AR9280 and it does not affects AR9285.

>
> commit 5b64aa72ead6f8be488d2be7af579f0d69fb7a6e
> Author: Rajkumar Manoharan <rmanoharan@atheros.com>
> Date:   Thu Jan 27 18:39:37 2011 +0530
>
>    ath9k_hw: Fix system hang when resuming from S3/S4
>
>    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: stable@kernel.org
>    Cc: Jack Lee <jlee@atheros.com>
>    Signed-off-by: Rajkumar Manoharan <rmanoharan@atheros.com>
>    Signed-off-by: John W. Linville <linville@tuxdriver.com>
>
> 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;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
shafi

Re: APSM, AR9285 and bus hangs

[Adrian Chadd]

I think the problem on these laptops is that -disabling- the APSM bits
causes some systems to hang.

That's what the commit earlier in the year did - it disabled those
bits by default. Later code then enabled those bits only on poweron,
and disabled them on suspend. My dirty patch just has them enabled by
default again as per the previous behaviour.

I guess someone should establish whether APSM is even supported on those boards?

Adrian

On 13 June 2011 21:20, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> On Mon, Jun 13, 2011 at 09:01:57PM +0800, Adrian Chadd wrote:
>> On 13 June 2011 19:00, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
>>
>> > cam a fedora user, confirms that patch fixes the locks-ups
>> > https://bugzilla.redhat.com/show_bug.cgi?id=697157#c26
>> >
>> > We are waiting for fix now :-)
>>
>> Cool.
>>
>> The problem is, the initial modification and subsequent changes to the
>> powersave function were documented to fix an APSM hang, not cause it.
>>
>> I'm happy to submit a patch to undo that particular change but I'm not
>> sure if it'll -break- things for other people. :)
>>
>> Someone with PCIe/APCI/APSM/general clue in the area of CPU + bus
>> sleep states needs to step in and help. I'm just not equipped to fix
>> the problem at the moment.
>
> I see this commit in 2.6.39, missed in 2.6.38 and 2.6.35 currently.
> Is possible this is the fix which do not break other things?
>
> commit 5b64aa72ead6f8be488d2be7af579f0d69fb7a6e
> Author: Rajkumar Manoharan <rmanoharan@atheros.com>
> Date:   Thu Jan 27 18:39:37 2011 +0530
>
>    ath9k_hw: Fix system hang when resuming from S3/S4
>
>    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: stable@kernel.org
>    Cc: Jack Lee <jlee@atheros.com>
>    Signed-off-by: Rajkumar Manoharan <rmanoharan@atheros.com>
>    Signed-off-by: John W. Linville <linville@tuxdriver.com>
>
> 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;
>

Re: APSM, AR9285 and bus hangs

[Stanislaw Gruszka]

On Mon, Jun 13, 2011 at 07:10:56PM +0530, Mohammed Shafi wrote:
> > I see this commit in 2.6.39, missed in 2.6.38 and 2.6.35 currently.
> > Is possible this is the fix which do not break other things?
> 
> hi,
> no, this fix only affects AR9280 and it does not affects AR9285.

That could be another stupid question, but could we do not touch
these bits on older AR9285s and disable them only on revision, that
was added by bad commit 53bc7aa08b48e5cd745f986731cc7dc24eef2a9f

diff --git a/drivers/net/wireless/ath/ath9k/ar9002_hw.c b/drivers/net/wireless/ath/ath9k/ar9002_hw.c
index f344cc2..91d99eb 100644
--- a/drivers/net/wireless/ath/ath9k/ar9002_hw.c
+++ b/drivers/net/wireless/ath/ath9k/ar9002_hw.c
@@ -384,7 +384,7 @@ static void ar9002_hw_configpcipowersave(struct ath_hw *ah,
 			}
 		}
 
-		if (AR_SREV_9280(ah) || AR_SREV_9285(ah) || AR_SREV_9287(ah)) {
+		if (AR_SREV_9280(ah) || AR_SREV_9285E_20(ah) || AR_SREV_9287(ah)) {
 			/*
 			 * Disable bit 6 and 7 before entering D3 to
 			 * prevent system hang.

Re: APSM, AR9285 and bus hangs

[Mohammed Shafi]

On Tue, Jun 14, 2011 at 3:39 PM, Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> On Mon, Jun 13, 2011 at 07:10:56PM +0530, Mohammed Shafi wrote:
>> > I see this commit in 2.6.39, missed in 2.6.38 and 2.6.35 currently.
>> > Is possible this is the fix which do not break other things?
>>
>> hi,
>> no, this fix only affects AR9280 and it does not affects AR9285.
>
> That could be another stupid question, but could we do not touch
> these bits on older AR9285s and disable them only on revision, that
> was added by bad commit 53bc7aa08b48e5cd745f986731cc7dc24eef2a9f
>
> diff --git a/drivers/net/wireless/ath/ath9k/ar9002_hw.c b/drivers/net/wireless/ath/ath9k/ar9002_hw.c
> index f344cc2..91d99eb 100644
> --- a/drivers/net/wireless/ath/ath9k/ar9002_hw.c
> +++ b/drivers/net/wireless/ath/ath9k/ar9002_hw.c
> @@ -384,7 +384,7 @@ static void ar9002_hw_configpcipowersave(struct ath_hw *ah,
>                        }
>                }
>
> -               if (AR_SREV_9280(ah) || AR_SREV_9285(ah) || AR_SREV_9287(ah)) {
> +               if (AR_SREV_9280(ah) || AR_SREV_9285E_20(ah) || AR_SREV_9287(ah)) {
>                        /*
>                         * Disable bit 6 and 7 before entering D3 to
>                         * prevent system hang.


I had verified with the internal code base whether this is being done,
unfortunately no and at least I had tested this patch in my lenovo
T400 machine it did not cause any issue. this might fix the issue for
the specific platform but I am not sure whether this will not cause
any problems in other platforms. these sort for Workarounds were done
based on testing in different platforms.
Adrian you have any thoughts?




-- 
shafi

Re: APSM, AR9285 and bus hangs

[Adrian Chadd]

On 14 June 2011 19:44, Mohammed Shafi <shafi.wireless@gmail.com> wrote:

> I had verified with the internal code base whether this is being done,
> unfortunately no and at least I had tested this patch in my lenovo
> T400 machine it did not cause any issue. this might fix the issue for
> the specific platform but I am not sure whether this will not cause
> any problems in other platforms. these sort for Workarounds were done
> based on testing in different platforms.
> Adrian you have any thoughts?

I don't have any new thoughts, no. I think it's more likely something
to do with how the bus is being setup, how APSM is being used, how
power saving in general is handled, quirks of that chipset (all the
laptops with problems have been a specific AMD chipset, right?)

Unfortunately I don't know anywhere near enough about the goings on
with power saving/APSM on Linux. If someone would like to get us in
contact with the relevant developers, we could likely have a more
constructive discussion.

(And after exams please, I don't have brain cycles to spend thoroughly
thinking about this until late June..)


Adrian

Re: APSM, AR9285 and bus hangs

[Tony Houghton]

On Tue, 14 Jun 2011 19:47:35 +0800
Adrian Chadd <adrian@freebsd.org> wrote:

> I don't have any new thoughts, no. I think it's more likely something
> to do with how the bus is being setup, how APSM is being used, how
> power saving in general is handled, quirks of that chipset (all the
> laptops with problems have been a specific AMD chipset, right?)

No, mine's Intel Atom/NVidia ION LE. According to lspci it has an NVidia
MCP79 running the PCI Express etc.

Re: APSM, AR9285 and bus hangs

[Stanislaw Gruszka]

On Tue, Jun 14, 2011 at 08:02:54AM +0800, Adrian Chadd wrote:
> I think the problem on these laptops is that -disabling- the APSM bits
> causes some systems to hang.

It is interesting if below patch works, people who can reproduce the
problem please give it a try. Except testing, I'm also interested
on comments :-)

---
>From 6f6f488156d60b44adeb68bbd0cb474b92111681 Mon Sep 17 00:00:00 2001
From: Stanislaw Gruszka <sgruszka@redhat.com>
Date: Fri, 15 Jul 2011 17:50:48 +0200
Subject: [PATCH] ath9k: skip ->config_pci_powersave quirks if PCIe port has ASPM disabled

We receive many bug reports about system hang during suspend/resume
when ath9k driver is in use. Adrian Chadd remarked that this problem
happens on systems that have ASPM disabled.

To do not hit the bug, try to skip doing ->config_pci_powersave magic
if PCIe downstream port device, which ath9k device is connected to, has
ASPM disabled.

This is test only patch, for now I'm interesting if it works. If so
proper patch will need to cope with runtime ASPM setting change, and
probably setup some ath9k registers correctly in both case ASPM is
disabled.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/ath/ath9k/ar9002_hw.c |    6 +-----
 drivers/net/wireless/ath/ath9k/ar9003_hw.c |    6 +-----
 drivers/net/wireless/ath/ath9k/hw.c        |    1 -
 drivers/net/wireless/ath/ath9k/hw.h        |    2 +-
 drivers/net/wireless/ath/ath9k/pci.c       |   20 ++++++++++++++++++++
 5 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ar9002_hw.c b/drivers/net/wireless/ath/ath9k/ar9002_hw.c
index 9ff7c30..44d9d8d 100644
--- a/drivers/net/wireless/ath/ath9k/ar9002_hw.c
+++ b/drivers/net/wireless/ath/ath9k/ar9002_hw.c
@@ -309,11 +309,7 @@ static void ar9002_hw_configpcipowersave(struct ath_hw *ah,
 	u8 i;
 	u32 val;
 
-	if (ah->is_pciexpress != true)
-		return;
-
-	/* Do not touch SerDes registers */
-	if (ah->config.pcie_powersave_enable == 2)
+	if (ah->is_pciexpress != true || ah->aspm_enabled != true)
 		return;
 
 	/* Nothing to do on restore for 11N */
diff --git a/drivers/net/wireless/ath/ath9k/ar9003_hw.c b/drivers/net/wireless/ath/ath9k/ar9003_hw.c
index 8efdec2..ad2bb2b 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_hw.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_hw.c
@@ -519,11 +519,7 @@ static void ar9003_hw_configpcipowersave(struct ath_hw *ah,
 					 int restore,
 					 int power_off)
 {
-	if (ah->is_pciexpress != true)
-		return;
-
-	/* Do not touch SerDes registers */
-	if (ah->config.pcie_powersave_enable == 2)
+	if (ah->is_pciexpress != true || ah->aspm_enabled != true)
 		return;
 
 	/* Nothing to do on restore for 11N */
diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index 2a5f908..8a80e7d 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -378,7 +378,6 @@ static void ath9k_hw_init_config(struct ath_hw *ah)
 	ah->config.additional_swba_backoff = 0;
 	ah->config.ack_6mb = 0x0;
 	ah->config.cwm_ignore_extcca = 0;
-	ah->config.pcie_powersave_enable = 0;
 	ah->config.pcie_clock_req = 0;
 	ah->config.pcie_waen = 0;
 	ah->config.analog_shiftreg = 1;
diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index 6acd0f9..7dd78e7 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -219,7 +219,6 @@ struct ath9k_ops_config {
 	int additional_swba_backoff;
 	int ack_6mb;
 	u32 cwm_ignore_extcca;
-	u8 pcie_powersave_enable;
 	bool pcieSerDesWrite;
 	u8 pcie_clock_req;
 	u32 pcie_waen;
@@ -673,6 +672,7 @@ struct ath_hw {
 
 	bool sw_mgmt_crypto;
 	bool is_pciexpress;
+	bool aspm_enabled;
 	bool is_monitoring;
 	bool need_an_top2_fixup;
 	u16 tx_trig_level;
diff --git a/drivers/net/wireless/ath/ath9k/pci.c b/drivers/net/wireless/ath/ath9k/pci.c
index 3bad0b2..2f34503 100644
--- a/drivers/net/wireless/ath/ath9k/pci.c
+++ b/drivers/net/wireless/ath/ath9k/pci.c
@@ -123,6 +123,24 @@ static const struct ath_bus_ops ath_pci_bus_ops = {
 	.extn_synch_en = ath_pci_extn_synch_enable,
 };
 
+static void ath_pci_check_aspm(struct ath_softc *sc)
+{
+	struct ath_hw *ah = sc->sc_ah;
+	struct pci_dev *pdev = to_pci_dev(sc->dev);
+	struct pci_dev *parent;
+	u8 aspm;
+
+	ah->aspm_enabled = false;
+
+	if (!pci_is_pcie(pdev))
+		return;
+
+	parent = pdev->bus->self;
+	pci_read_config_byte(parent, ATH_PCIE_CAP_LINK_CTRL, &aspm);
+	if (aspm & (ATH_PCIE_CAP_LINK_L0S | ATH_PCIE_CAP_LINK_L1))
+		ah->aspm_enabled = true;
+}
+
 static int ath_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	void __iomem *mem;
@@ -230,6 +248,8 @@ static int ath_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		goto err_init;
 	}
 
+	ath_pci_check_aspm(sc);
+
 	ath9k_hw_name(sc->sc_ah, hw_name, sizeof(hw_name));
 	wiphy_info(hw->wiphy, "%s mem=0x%lx, irq=%d\n",
 		   hw_name, (unsigned long)mem, pdev->irq);
-- 
1.7.1

Re: APSM, AR9285 and bus hangs

[Stanislaw Gruszka]

On Fri, Jul 15, 2011 at 06:18:58PM +0200, Stanislaw Gruszka wrote:
> On Tue, Jun 14, 2011 at 08:02:54AM +0800, Adrian Chadd wrote:
> > I think the problem on these laptops is that -disabling- the APSM bits
> > causes some systems to hang.
> 
> It is interesting if below patch works, people who can reproduce the
> problem please give it a try. Except testing, I'm also interested
> on comments :-)

I got only feedback from Camilo, patch stops to hung system regularly,
but still there was rare random hangs.

I'm going to post some more patches on linux-wireles mailing list.
Please check them out!

Thanks
Stanislaw