[PATCH -next 0/8] ath9k: ASPM fixes

[PATCH -next 0/8] ath9k: ASPM fixes

[Stanislaw Gruszka]

This patch series try to fix ath9k ASPM. Some of patches are cleanup
only or do merging common code (with e1000e driver).

With CONFIG_PCIEASPM it's possible to change ASPM settings on runtime
via /sys/module/pcie_aspm/parameters/policy . However most drivers
I looked at assume this setting is constant. I add a callback to pci
driver to inform about the change, and allow driver to do needed hw
related changes. Currently only ath9k implement the callback, but
I think it will be useful for other drivers i.e. iwlwifi, rtlwifi
that do some own ASPM handling.

Would be nice to get some testing on patches on different
platforms with ath9k devices on kernel compiled with CONFIG_PCIEASPM
(default) and mixing up commands:

echo powersave > /sys/module/pcie_aspm/parameters/policy  
echo performance > /sys/module/pcie_aspm/parameters/policy  
ifconfig wlan0 down
ifconfig wlan0 up
pm-suspend

Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: linux-pci@vger.kernel.org
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: e1000-devel@lists.sourceforge.net
---
 drivers/net/e1000e/netdev.c                |   31 ----------
 drivers/net/wireless/ath/ath9k/ar9002_hw.c |   12 ---
 drivers/net/wireless/ath/ath9k/ar9003_hw.c |   12 ---
 drivers/net/wireless/ath/ath9k/hw-ops.h    |    9 +-
 drivers/net/wireless/ath/ath9k/hw.c        |   17 +++--
 drivers/net/wireless/ath/ath9k/hw.h        |   12 +--
 drivers/net/wireless/ath/ath9k/main.c      |    8 --
 drivers/net/wireless/ath/ath9k/pci.c       |   88 ++++++++++++++++++++++-------
 drivers/pci/pcie/Makefile                  |    3 
 drivers/pci/pcie/aspm.c                    |   46 ++++++++++++++-
 include/linux/pci-aspm.h                   |    2 
 include/linux/pci.h                        |    3 
 12 files changed, 148 insertions(+), 95 deletions(-)

[PATCH -next 1/8] ath9k: remove ->config_pci_powersave() redundant argument

[Stanislaw Gruszka]

We always call ->config_pci_powersave() with both restore and power_off
arguments equal to 0 or both equal to 1, so merge them into one
argument.

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

diff --git a/drivers/net/wireless/ath/ath9k/ar9002_hw.c b/drivers/net/wireless/ath/ath9k/ar9002_hw.c
index 44d9d8d..70a18d1 100644
--- a/drivers/net/wireless/ath/ath9k/ar9002_hw.c
+++ b/drivers/net/wireless/ath/ath9k/ar9002_hw.c
@@ -303,8 +303,7 @@ static void ar9002_hw_init_mode_gain_regs(struct ath_hw *ah)
  * register as the other analog registers.  Hence the 9 writes.
  */
 static void ar9002_hw_configpcipowersave(struct ath_hw *ah,
-					 int restore,
-					 int power_off)
+					 bool power_off)
 {
 	u8 i;
 	u32 val;
@@ -313,7 +312,7 @@ static void ar9002_hw_configpcipowersave(struct ath_hw *ah,
 		return;
 
 	/* Nothing to do on restore for 11N */
-	if (!restore) {
+	if (!power_off /* !restore */) {
 		if (AR_SREV_9280_20_OR_LATER(ah)) {
 			/*
 			 * AR9280 2.0 or later chips use SerDes values from the
diff --git a/drivers/net/wireless/ath/ath9k/ar9003_hw.c b/drivers/net/wireless/ath/ath9k/ar9003_hw.c
index ad2bb2b..e3d58bd 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_hw.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_hw.c
@@ -516,14 +516,13 @@ static void ar9003_hw_init_mode_gain_regs(struct ath_hw *ah)
  * register as the other analog registers.  Hence the 9 writes.
  */
 static void ar9003_hw_configpcipowersave(struct ath_hw *ah,
-					 int restore,
-					 int power_off)
+					 bool power_off)
 {
 	if (ah->is_pciexpress != true || ah->aspm_enabled != true)
 		return;
 
 	/* Nothing to do on restore for 11N */
-	if (!restore) {
+	if (!power_off /* !restore */) {
 		/* set bit 19 to allow forcing of pcie core into L1 state */
 		REG_SET_BIT(ah, AR_PCIE_PM_CTRL, AR_PCIE_PM_CTRL_ENA);
 
diff --git a/drivers/net/wireless/ath/ath9k/hw-ops.h b/drivers/net/wireless/ath/ath9k/hw-ops.h
index cb29e88..8c12385 100644
--- a/drivers/net/wireless/ath/ath9k/hw-ops.h
+++ b/drivers/net/wireless/ath/ath9k/hw-ops.h
@@ -22,10 +22,9 @@
 /* Hardware core and driver accessible callbacks */
 
 static inline void ath9k_hw_configpcipowersave(struct ath_hw *ah,
-					       int restore,
-					       int power_off)
+					       bool power_off)
 {
-	ath9k_hw_ops(ah)->config_pci_powersave(ah, restore, power_off);
+	ath9k_hw_ops(ah)->config_pci_powersave(ah, power_off);
 }
 
 static inline void ath9k_hw_rxena(struct ath_hw *ah)
diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index c798890..a26a8c5 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -601,8 +601,7 @@ struct ath_hw_private_ops {
  */
 struct ath_hw_ops {
 	void (*config_pci_powersave)(struct ath_hw *ah,
-				     int restore,
-				     int power_off);
+				     bool power_off);
 	void (*rx_enable)(struct ath_hw *ah);
 	void (*set_desc_link)(void *ds, u32 link);
 	bool (*calibrate)(struct ath_hw *ah,
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 9098aaa..651f633 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -887,7 +887,7 @@ static void ath_radio_enable(struct ath_softc *sc, struct ieee80211_hw *hw)
 	ath9k_ps_wakeup(sc);
 	spin_lock_bh(&sc->sc_pcu_lock);
 
-	ath9k_hw_configpcipowersave(ah, 0, 0);
+	ath9k_hw_configpcipowersave(ah, false);
 
 	if (!ah->curchan)
 		ah->curchan = ath9k_cmn_get_curchannel(sc->hw, ah);
@@ -967,7 +967,7 @@ void ath_radio_disable(struct ath_softc *sc, struct ieee80211_hw *hw)
 
 	ath9k_hw_phy_disable(ah);
 
-	ath9k_hw_configpcipowersave(ah, 1, 1);
+	ath9k_hw_configpcipowersave(ah, true);
 
 	spin_unlock_bh(&sc->sc_pcu_lock);
 	ath9k_ps_restore(sc);
@@ -1066,7 +1066,7 @@ static int ath9k_start(struct ieee80211_hw *hw)
 	init_channel = ath9k_cmn_get_curchannel(hw, ah);
 
 	/* Reset SERDES registers */
-	ath9k_hw_configpcipowersave(ah, 0, 0);
+	ath9k_hw_configpcipowersave(ah, false);
 
 	/*
 	 * The basic interface to setting the hardware in a good
diff --git a/drivers/net/wireless/ath/ath9k/pci.c b/drivers/net/wireless/ath/ath9k/pci.c
index be4ea13..8256e97 100644
--- a/drivers/net/wireless/ath/ath9k/pci.c
+++ b/drivers/net/wireless/ath/ath9k/pci.c
@@ -35,6 +35,7 @@ static DEFINE_PCI_DEVICE_TABLE(ath_pci_id_table) = {
 	{ 0 }
 };
 
+
 /* return bus cachesize in 4B word units */
 static void ath_pci_read_cachesize(struct ath_common *common, int *csz)
 {
@@ -137,7 +138,7 @@ static void ath_pci_aspm_init(struct ath_common *common)
 	if (aspm & (PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1)) {
 		ah->aspm_enabled = true;
 		/* Initialize PCIe PM and SERDES registers. */
-		ath9k_hw_configpcipowersave(ah, 0, 0);
+		ath9k_hw_configpcipowersave(ah, false);
 	}
 }
 
-- 
1.7.1

[PATCH -next 2/8] ath9k: merge common ->config_pci_powersave() checks

[Stanislaw Gruszka]

Move common checks into wrapper function. Since ASPM can be only enabled
on PCIe devices ->is_pciexpress check is unneeded.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/ath/ath9k/ar9002_hw.c |    3 ---
 drivers/net/wireless/ath/ath9k/ar9003_hw.c |    3 ---
 drivers/net/wireless/ath/ath9k/hw-ops.h    |    3 +++
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ar9002_hw.c b/drivers/net/wireless/ath/ath9k/ar9002_hw.c
index 70a18d1..b54ab78 100644
--- a/drivers/net/wireless/ath/ath9k/ar9002_hw.c
+++ b/drivers/net/wireless/ath/ath9k/ar9002_hw.c
@@ -308,9 +308,6 @@ static void ar9002_hw_configpcipowersave(struct ath_hw *ah,
 	u8 i;
 	u32 val;
 
-	if (ah->is_pciexpress != true || ah->aspm_enabled != true)
-		return;
-
 	/* Nothing to do on restore for 11N */
 	if (!power_off /* !restore */) {
 		if (AR_SREV_9280_20_OR_LATER(ah)) {
diff --git a/drivers/net/wireless/ath/ath9k/ar9003_hw.c b/drivers/net/wireless/ath/ath9k/ar9003_hw.c
index e3d58bd..9cf5d13 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_hw.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_hw.c
@@ -518,9 +518,6 @@ static void ar9003_hw_init_mode_gain_regs(struct ath_hw *ah)
 static void ar9003_hw_configpcipowersave(struct ath_hw *ah,
 					 bool power_off)
 {
-	if (ah->is_pciexpress != true || ah->aspm_enabled != true)
-		return;
-
 	/* Nothing to do on restore for 11N */
 	if (!power_off /* !restore */) {
 		/* set bit 19 to allow forcing of pcie core into L1 state */
diff --git a/drivers/net/wireless/ath/ath9k/hw-ops.h b/drivers/net/wireless/ath/ath9k/hw-ops.h
index 8c12385..dd9003e 100644
--- a/drivers/net/wireless/ath/ath9k/hw-ops.h
+++ b/drivers/net/wireless/ath/ath9k/hw-ops.h
@@ -24,6 +24,9 @@
 static inline void ath9k_hw_configpcipowersave(struct ath_hw *ah,
 					       bool power_off)
 {
+	if (ah->aspm_enabled != true)
+		return;
+
 	ath9k_hw_ops(ah)->config_pci_powersave(ah, power_off);
 }
 
-- 
1.7.1

[PATCH -next 3/8] ath9k: do btcoex ASPM disabling at initialization time

[Stanislaw Gruszka]

Disable ASPM in pci ->probe on upstream (device) and downstream
(PCIe port) component. According to e1000e driver authors this is
required. I did not find that requirement in PCIe spec, but it seems
to be logical for me.

This need to be fixed for CONFIG_PCIEASPM, that will be done later ...

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/ath/ath9k/hw.c   |    8 +++---
 drivers/net/wireless/ath/ath9k/hw.h   |    4 ---
 drivers/net/wireless/ath/ath9k/main.c |    2 -
 drivers/net/wireless/ath/ath9k/pci.c  |   44 ++++++++++++++++++--------------
 4 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index 8dcefe7..71cfe1a 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -603,10 +603,7 @@ static int __ath9k_hw_init(struct ath_hw *ah)
 
 	ath9k_hw_init_mode_regs(ah);
 
-
-	if (ah->is_pciexpress)
-		ath9k_hw_aspm_init(ah);
-	else
+	if (!ah->is_pciexpress)
 		ath9k_hw_disablepcie(ah);
 
 	if (!AR_SREV_9300_20_OR_LATER(ah))
@@ -621,6 +618,9 @@ static int __ath9k_hw_init(struct ath_hw *ah)
 	if (r)
 		return r;
 
+	if (ah->is_pciexpress)
+		ath9k_hw_aspm_init(ah);
+
 	r = ath9k_hw_init_macaddr(ah);
 	if (r) {
 		ath_err(common, "Failed to initialize MAC address\n");
diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index a26a8c5..f28f339 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -1029,10 +1029,6 @@ void ath9k_ani_reset(struct ath_hw *ah, bool is_scanning);
 void ath9k_hw_proc_mib_event(struct ath_hw *ah);
 void ath9k_hw_ani_monitor(struct ath_hw *ah, struct ath9k_channel *chan);
 
-#define ATH_PCIE_CAP_LINK_CTRL	0x70
-#define ATH_PCIE_CAP_LINK_L0S	1
-#define ATH_PCIE_CAP_LINK_L1	2
-
 #define ATH9K_CLOCK_RATE_CCK		22
 #define ATH9K_CLOCK_RATE_5GHZ_OFDM	40
 #define ATH9K_CLOCK_RATE_2GHZ_OFDM	44
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 651f633..b0389c3 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1141,8 +1141,6 @@ static int ath9k_start(struct ieee80211_hw *hw)
 					   AR_STOMP_LOW_WLAN_WGHT);
 		ath9k_hw_btcoex_enable(ah);
 
-		if (common->bus_ops->bt_coex_prep)
-			common->bus_ops->bt_coex_prep(common);
 		if (ah->btcoex_hw.scheme == ATH_BTCOEX_CFG_3WIRE)
 			ath9k_btcoex_timer_resume(sc);
 	}
diff --git a/drivers/net/wireless/ath/ath9k/pci.c b/drivers/net/wireless/ath/ath9k/pci.c
index 8256e97..7d94b64 100644
--- a/drivers/net/wireless/ath/ath9k/pci.c
+++ b/drivers/net/wireless/ath/ath9k/pci.c
@@ -89,23 +89,6 @@ static bool ath_pci_eeprom_read(struct ath_common *common, u32 off, u16 *data)
 	return true;
 }
 
-/*
- * Bluetooth coexistance requires disabling ASPM.
- */
-static void ath_pci_bt_coex_prep(struct ath_common *common)
-{
-	struct ath_softc *sc = (struct ath_softc *) common->priv;
-	struct pci_dev *pdev = to_pci_dev(sc->dev);
-	u8 aspm;
-
-	if (!pci_is_pcie(pdev))
-		return;
-
-	pci_read_config_byte(pdev, ATH_PCIE_CAP_LINK_CTRL, &aspm);
-	aspm &= ~(ATH_PCIE_CAP_LINK_L0S | ATH_PCIE_CAP_LINK_L1);
-	pci_write_config_byte(pdev, ATH_PCIE_CAP_LINK_CTRL, aspm);
-}
-
 static void ath_pci_extn_synch_enable(struct ath_common *common)
 {
 	struct ath_softc *sc = (struct ath_softc *) common->priv;
@@ -117,6 +100,7 @@ static void ath_pci_extn_synch_enable(struct ath_common *common)
 	pci_write_config_byte(pdev, sc->sc_ah->caps.pcie_lcr_offset, lnkctl);
 }
 
+/* Need to be called after we discover btcoex capabilities */
 static void ath_pci_aspm_init(struct ath_common *common)
 {
 	struct ath_softc *sc = (struct ath_softc *) common->priv;
@@ -126,10 +110,33 @@ static void ath_pci_aspm_init(struct ath_common *common)
 	int pos;
 	u8 aspm;
 
-	if (!pci_is_pcie(pdev))
+	pos = pci_pcie_cap(pdev);
+	if (!pos)
 		return;
 
 	parent = pdev->bus->self;
+
+	if (ah->btcoex_hw.scheme != ATH_BTCOEX_CFG_NONE) {
+		/* Bluetooth coexistance requires disabling ASPM. */
+		pci_read_config_byte(pdev, pos + PCI_EXP_LNKCTL, &aspm);
+		aspm &= ~(PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1);
+		pci_write_config_byte(pdev, pos + PCI_EXP_LNKCTL, aspm);
+
+		/*
+		 * Both upstream and downstream PCIe components should
+		 * have the same ASPM settings.
+		 */
+		if (WARN_ON(!parent))
+			return;
+
+		pos = pci_pcie_cap(parent);
+		pci_read_config_byte(parent, pos + PCI_EXP_LNKCTL, &aspm);
+		aspm &= ~(PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1);
+		pci_write_config_byte(parent, pos + PCI_EXP_LNKCTL, aspm);
+
+		return;
+	}
+
 	if (WARN_ON(!parent))
 		return;
 
@@ -146,7 +153,6 @@ static const struct ath_bus_ops ath_pci_bus_ops = {
 	.ath_bus_type = ATH_PCI,
 	.read_cachesize = ath_pci_read_cachesize,
 	.eeprom_read = ath_pci_eeprom_read,
-	.bt_coex_prep = ath_pci_bt_coex_prep,
 	.extn_synch_en = ath_pci_extn_synch_enable,
 	.aspm_init = ath_pci_aspm_init,
 };
-- 
1.7.1

[PATCH -next 4/8] pci: aspm: add function for disabling ASPM

[Stanislaw Gruszka]

Add support for disabling ASPM if !CONFIG_PCIEASPM. Patch is based
on code from e1000e.

Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: linux-pci@vger.kernel.org
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: e1000-devel@lists.sourceforge.net
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/pci/pcie/Makefile |    3 +--
 drivers/pci/pcie/aspm.c   |   41 +++++++++++++++++++++++++++++++++++++++++
 include/linux/pci-aspm.h  |    2 ++
 3 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile
index 00c62df..6e20c59 100644
--- a/drivers/pci/pcie/Makefile
+++ b/drivers/pci/pcie/Makefile
@@ -2,8 +2,7 @@
 # Makefile for PCI-Express PORT Driver
 #
 
-# Build PCI Express ASPM if needed
-obj-$(CONFIG_PCIEASPM)		+= aspm.o
+obj-y				+= aspm.o
 
 pcieportdrv-y			:= portdrv_core.o portdrv_pci.o portdrv_bus.o
 pcieportdrv-$(CONFIG_ACPI)	+= portdrv_acpi.o
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 6892601..2883fc3 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -21,6 +21,8 @@
 #include <linux/pci-aspm.h>
 #include "../pci.h"
 
+#ifdef CONFIG_PCIEASPM
+
 #ifdef MODULE_PARAM_PREFIX
 #undef MODULE_PARAM_PREFIX
 #endif
@@ -976,3 +978,42 @@ bool pcie_aspm_support_enabled(void)
 	return aspm_support_enabled;
 }
 EXPORT_SYMBOL(pcie_aspm_support_enabled);
+
+void pcie_disable_aspm(struct pci_dev *pdev, u16 state)
+{
+	pci_disable_link_state_locked(pdev, state);
+}
+EXPORT_SYMBOL(pcie_disable_aspm);
+
+#else /* CONFIG_PCIEASPM */
+
+void pcie_disable_aspm(struct pci_dev *pdev, u16 state)
+{
+	int pos;
+	u16 reg16;
+	struct pci_dev *parent;
+
+	pos = pci_pcie_cap(pdev);
+	if (!pos)
+		return;
+	/*
+	 * Both device and parent should have the same ASPM setting.
+	 * Disable ASPM in downstream component first and then upstream.
+	 */
+
+	pci_read_config_word(pdev, pos + PCI_EXP_LNKCTL, &reg16);
+	reg16 &= ~state;
+	pci_write_config_word(pdev, pos + PCI_EXP_LNKCTL, reg16);
+
+	parent = pdev->bus->self;
+	if (WARN_ON(!parent))
+		return;
+
+	pos = pci_pcie_cap(parent);
+	pci_read_config_word(parent, pos + PCI_EXP_LNKCTL, &reg16);
+	reg16 &= ~state;
+	pci_write_config_word(parent, pos + PCI_EXP_LNKCTL, reg16);
+}
+EXPORT_SYMBOL(pcie_disable_aspm);
+
+#endif
diff --git a/include/linux/pci-aspm.h b/include/linux/pci-aspm.h
index 7cea7b6..f9722e6 100644
--- a/include/linux/pci-aspm.h
+++ b/include/linux/pci-aspm.h
@@ -55,6 +55,8 @@ static inline void pcie_no_aspm(void)
 }
 #endif
 
+extern void pcie_disable_aspm(struct pci_dev *pdev, u16 state);
+
 #ifdef CONFIG_PCIEASPM_DEBUG /* this depends on CONFIG_PCIEASPM */
 extern void pcie_aspm_create_sysfs_dev_files(struct pci_dev *pdev);
 extern void pcie_aspm_remove_sysfs_dev_files(struct pci_dev *pdev);
-- 
1.7.1

[PATCH -next 5/8] ath9k: use common function for disabling ASPM

[Stanislaw Gruszka]

Disable ASPM using common function, this works also with
CONFIG_PCIEASPM=y.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/ath/ath9k/pci.c |   23 +++--------------------
 1 files changed, 3 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/pci.c b/drivers/net/wireless/ath/ath9k/pci.c
index 7d94b64..67a2c96 100644
--- a/drivers/net/wireless/ath/ath9k/pci.c
+++ b/drivers/net/wireless/ath/ath9k/pci.c
@@ -110,33 +110,16 @@ static void ath_pci_aspm_init(struct ath_common *common)
 	int pos;
 	u8 aspm;
 
-	pos = pci_pcie_cap(pdev);
-	if (!pos)
+	if (!pci_is_pcie(pdev))
 		return;
 
-	parent = pdev->bus->self;
-
 	if (ah->btcoex_hw.scheme != ATH_BTCOEX_CFG_NONE) {
 		/* Bluetooth coexistance requires disabling ASPM. */
-		pci_read_config_byte(pdev, pos + PCI_EXP_LNKCTL, &aspm);
-		aspm &= ~(PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1);
-		pci_write_config_byte(pdev, pos + PCI_EXP_LNKCTL, aspm);
-
-		/*
-		 * Both upstream and downstream PCIe components should
-		 * have the same ASPM settings.
-		 */
-		if (WARN_ON(!parent))
-			return;
-
-		pos = pci_pcie_cap(parent);
-		pci_read_config_byte(parent, pos + PCI_EXP_LNKCTL, &aspm);
-		aspm &= ~(PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1);
-		pci_write_config_byte(parent, pos + PCI_EXP_LNKCTL, aspm);
-
+		pcie_disable_aspm(pdev, PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1);
 		return;
 	}
 
+	parent = pdev->bus->self;
 	if (WARN_ON(!parent))
 		return;
 
-- 
1.7.1

[PATCH -next 6/8] e1000e: use common function for disabling ASPM

[Stanislaw Gruszka]

Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: e1000-devel@lists.sourceforge.net
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/e1000e/netdev.c |   31 +------------------------------
 1 files changed, 1 insertions(+), 30 deletions(-)

diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 3310c3d..62ee7e4 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -5358,42 +5358,13 @@ static void e1000_complete_shutdown(struct pci_dev *pdev, bool sleep,
 	}
 }
 
-#ifdef CONFIG_PCIEASPM
-static void __e1000e_disable_aspm(struct pci_dev *pdev, u16 state)
-{
-	pci_disable_link_state_locked(pdev, state);
-}
-#else
-static void __e1000e_disable_aspm(struct pci_dev *pdev, u16 state)
-{
-	int pos;
-	u16 reg16;
-
-	/*
-	 * Both device and parent should have the same ASPM setting.
-	 * Disable ASPM in downstream component first and then upstream.
-	 */
-	pos = pci_pcie_cap(pdev);
-	pci_read_config_word(pdev, pos + PCI_EXP_LNKCTL, &reg16);
-	reg16 &= ~state;
-	pci_write_config_word(pdev, pos + PCI_EXP_LNKCTL, reg16);
-
-	if (!pdev->bus->self)
-		return;
-
-	pos = pci_pcie_cap(pdev->bus->self);
-	pci_read_config_word(pdev->bus->self, pos + PCI_EXP_LNKCTL, &reg16);
-	reg16 &= ~state;
-	pci_write_config_word(pdev->bus->self, pos + PCI_EXP_LNKCTL, reg16);
-}
-#endif
 static void e1000e_disable_aspm(struct pci_dev *pdev, u16 state)
 {
 	dev_info(&pdev->dev, "Disabling ASPM %s %s\n",
 		 (state & PCIE_LINK_STATE_L0S) ? "L0s" : "",
 		 (state & PCIE_LINK_STATE_L1) ? "L1" : "");
 
-	__e1000e_disable_aspm(pdev, state);
+	pcie_disable_aspm(pdev, state);
 }
 
 #ifdef CONFIG_PM
-- 
1.7.1

[PATCH -next 7/8] pci: aspm: add settings changed callback

[Stanislaw Gruszka]

Inform drivers that register pci_drviver->aspm_changed(), about new ASPM
settings. Callback is only called for child (endpoint) devices.

Currently this will be used in ath9k driver. In the future we most
likely will use it in iwlwifi and rtlwifi drivers and possibly some
others that needs to do own ASPM related quirks.

Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: linux-pci@vger.kernel.org
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/pci/pcie/aspm.c |    5 ++++-
 include/linux/pci.h     |    3 +++
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 2883fc3..d7acec8 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -474,8 +474,11 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
 	 */
 	if (state & ASPM_STATE_L1)
 		pcie_config_aspm_dev(parent, upstream);
-	list_for_each_entry(child, &linkbus->devices, bus_list)
+	list_for_each_entry(child, &linkbus->devices, bus_list) {
 		pcie_config_aspm_dev(child, dwstream);
+		if (child->driver && child->driver->aspm_changed)
+			child->driver->aspm_changed(child, dwstream);
+	}
 	if (!(state & ASPM_STATE_L1))
 		pcie_config_aspm_dev(parent, upstream);
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index c446b5c..2110d63 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -555,6 +555,9 @@ struct pci_driver {
 	int  (*resume_early) (struct pci_dev *dev);
 	int  (*resume) (struct pci_dev *dev);	                /* Device woken up */
 	void (*shutdown) (struct pci_dev *dev);
+#ifdef CONFIG_PCIEASPM
+	void (*aspm_changed) (struct pci_dev *dev, u32 state);
+#endif
 	struct pci_error_handlers *err_handler;
 	struct device_driver	driver;
 	struct pci_dynids dynids;
-- 
1.7.1

[PATCH -next 8/8] ath9k: be prepare for dynamic ASPM settings change

[Stanislaw Gruszka]

Kernel offer possibility to change ASPM settings using
/sys/module/pcie_aspm/parameters/policy interface. As settings can be
changed, we also need to change ath9k hw registers. Patch implement
this by monitoring power_off state, and based on that state setup
register when pci core inform us about ASPM change.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/ath/ath9k/hw-ops.h |    1 +
 drivers/net/wireless/ath/ath9k/hw.h     |    1 +
 drivers/net/wireless/ath/ath9k/main.c   |    3 +-
 drivers/net/wireless/ath/ath9k/pci.c    |   38 +++++++++++++++++++++++++++++++
 4 files changed, 42 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw-ops.h b/drivers/net/wireless/ath/ath9k/hw-ops.h
index dd9003e..0743e82 100644
--- a/drivers/net/wireless/ath/ath9k/hw-ops.h
+++ b/drivers/net/wireless/ath/ath9k/hw-ops.h
@@ -24,6 +24,7 @@
 static inline void ath9k_hw_configpcipowersave(struct ath_hw *ah,
 					       bool power_off)
 {
+	ah->power_off = power_off;
 	if (ah->aspm_enabled != true)
 		return;
 
diff --git a/drivers/net/wireless/ath/ath9k/hw.h b/drivers/net/wireless/ath/ath9k/hw.h
index f28f339..cb388ff 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -672,6 +672,7 @@ struct ath_hw {
 	bool sw_mgmt_crypto;
 	bool is_pciexpress;
 	bool aspm_enabled;
+	bool power_off;
 	bool is_monitoring;
 	bool need_an_top2_fixup;
 	u16 tx_trig_level;
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index b0389c3..a7e3c5c 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1065,6 +1065,8 @@ static int ath9k_start(struct ieee80211_hw *hw)
 
 	init_channel = ath9k_cmn_get_curchannel(hw, ah);
 
+	spin_lock_bh(&sc->sc_pcu_lock);
+
 	/* Reset SERDES registers */
 	ath9k_hw_configpcipowersave(ah, false);
 
@@ -1075,7 +1077,6 @@ static int ath9k_start(struct ieee80211_hw *hw)
 	 * be followed by initialization of the appropriate bits
 	 * and then setup of the interrupt mask.
 	 */
-	spin_lock_bh(&sc->sc_pcu_lock);
 	r = ath9k_hw_reset(ah, init_channel, ah->caldata, false);
 	if (r) {
 		ath_err(common,
diff --git a/drivers/net/wireless/ath/ath9k/pci.c b/drivers/net/wireless/ath/ath9k/pci.c
index 67a2c96..c585180 100644
--- a/drivers/net/wireless/ath/ath9k/pci.c
+++ b/drivers/net/wireless/ath/ath9k/pci.c
@@ -285,6 +285,41 @@ static void ath_pci_remove(struct pci_dev *pdev)
 	pci_release_region(pdev, 0);
 }
 
+#ifdef CONFIG_PCIEASPM
+
+static void ath_pci_aspm_changed(struct pci_dev *pdev, u32 state)
+{
+	struct ieee80211_hw *hw = pci_get_drvdata(pdev);
+	struct ath_softc *sc = hw->priv;
+	struct ath_hw *ah = sc->sc_ah;
+
+	spin_lock_irq(&sc->sc_pcu_lock);
+
+	/*
+	 * ASPM settings were changed, we have to configure PCIe PM and SERDES
+	 * registers based on current state of device and new ASPM settings.
+	 */
+	if (state & (PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1)) {
+		ah->aspm_enabled = true;
+		ath9k_hw_configpcipowersave(ah, ah->power_off);
+	} else {
+		if (ah->power_off == false) {
+			/*
+			 * Device is enabled, but since we disable ASPM we also
+			 * setup PCIe PM registers in off state.
+			 */
+			ath9k_hw_configpcipowersave(ah, true);
+			/* But track real power_off value. */
+			ah->power_off = false;
+		}
+		ah->aspm_enabled = false;
+	}
+
+	spin_unlock_irq(&sc->sc_pcu_lock);
+}
+
+#endif
+
 #ifdef CONFIG_PM
 
 static int ath_pci_suspend(struct device *device)
@@ -364,6 +399,9 @@ static struct pci_driver ath_pci_driver = {
 	.id_table   = ath_pci_id_table,
 	.probe      = ath_pci_probe,
 	.remove     = ath_pci_remove,
+#ifdef CONFIG_PCIEASPM
+	.aspm_changed = ath_pci_aspm_changed,
+#endif
 	.driver.pm  = ATH9K_PM_OPS,
 };
 
-- 
1.7.1

Re: [PATCH -next 0/8] ath9k: ASPM fixes

[John W. Linville]

On Fri, Aug 05, 2011 at 01:10:31PM +0200, Stanislaw Gruszka wrote:
> This patch series try to fix ath9k ASPM. Some of patches are cleanup
> only or do merging common code (with e1000e driver).
> 
> With CONFIG_PCIEASPM it's possible to change ASPM settings on runtime
> via /sys/module/pcie_aspm/parameters/policy . However most drivers
> I looked at assume this setting is constant. I add a callback to pci
> driver to inform about the change, and allow driver to do needed hw
> related changes. Currently only ath9k implement the callback, but
> I think it will be useful for other drivers i.e. iwlwifi, rtlwifi
> that do some own ASPM handling.
> 
> Would be nice to get some testing on patches on different
> platforms with ath9k devices on kernel compiled with CONFIG_PCIEASPM
> (default) and mixing up commands:
> 
> echo powersave > /sys/module/pcie_aspm/parameters/policy  
> echo performance > /sys/module/pcie_aspm/parameters/policy  
> ifconfig wlan0 down
> ifconfig wlan0 up
> pm-suspend
> 
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Cc: linux-pci@vger.kernel.org
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: e1000-devel@lists.sourceforge.net
> ---
>  drivers/net/e1000e/netdev.c                |   31 ----------
>  drivers/net/wireless/ath/ath9k/ar9002_hw.c |   12 ---
>  drivers/net/wireless/ath/ath9k/ar9003_hw.c |   12 ---
>  drivers/net/wireless/ath/ath9k/hw-ops.h    |    9 +-
>  drivers/net/wireless/ath/ath9k/hw.c        |   17 +++--
>  drivers/net/wireless/ath/ath9k/hw.h        |   12 +--
>  drivers/net/wireless/ath/ath9k/main.c      |    8 --
>  drivers/net/wireless/ath/ath9k/pci.c       |   88 ++++++++++++++++++++++-------
>  drivers/pci/pcie/Makefile                  |    3 
>  drivers/pci/pcie/aspm.c                    |   46 ++++++++++++++-
>  include/linux/pci-aspm.h                   |    2 
>  include/linux/pci.h                        |    3 
>  12 files changed, 148 insertions(+), 95 deletions(-)

I'm happy to take patches 1-3 through my tree.  But maybe patches 4-8
should go through a PCI tree?  Or at least get some ACKS from the
appropriate maintainers to take these changes through the wireless
trees?

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH -next 0/8] ath9k: ASPM fixes

[Stanislaw Gruszka]

On Thu, Aug 11, 2011 at 12:08:48PM -0400, John W. Linville wrote:
> On Fri, Aug 05, 2011 at 01:10:31PM +0200, Stanislaw Gruszka wrote:
> > This patch series try to fix ath9k ASPM. Some of patches are cleanup
> > only or do merging common code (with e1000e driver).
> > 
> > With CONFIG_PCIEASPM it's possible to change ASPM settings on runtime
> > via /sys/module/pcie_aspm/parameters/policy . However most drivers
> > I looked at assume this setting is constant. I add a callback to pci
> > driver to inform about the change, and allow driver to do needed hw
> > related changes. Currently only ath9k implement the callback, but
> > I think it will be useful for other drivers i.e. iwlwifi, rtlwifi
> > that do some own ASPM handling.
> > 
> > Would be nice to get some testing on patches on different
> > platforms with ath9k devices on kernel compiled with CONFIG_PCIEASPM
> > (default) and mixing up commands:
> > 
> > echo powersave > /sys/module/pcie_aspm/parameters/policy  
> > echo performance > /sys/module/pcie_aspm/parameters/policy  
> > ifconfig wlan0 down
> > ifconfig wlan0 up
> > pm-suspend
> > 
> > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> > Cc: linux-pci@vger.kernel.org
> > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > Cc: e1000-devel@lists.sourceforge.net
> > ---
> >  drivers/net/e1000e/netdev.c                |   31 ----------
> >  drivers/net/wireless/ath/ath9k/ar9002_hw.c |   12 ---
> >  drivers/net/wireless/ath/ath9k/ar9003_hw.c |   12 ---
> >  drivers/net/wireless/ath/ath9k/hw-ops.h    |    9 +-
> >  drivers/net/wireless/ath/ath9k/hw.c        |   17 +++--
> >  drivers/net/wireless/ath/ath9k/hw.h        |   12 +--
> >  drivers/net/wireless/ath/ath9k/main.c      |    8 --
> >  drivers/net/wireless/ath/ath9k/pci.c       |   88 ++++++++++++++++++++++-------
> >  drivers/pci/pcie/Makefile                  |    3 
> >  drivers/pci/pcie/aspm.c                    |   46 ++++++++++++++-
> >  include/linux/pci-aspm.h                   |    2 
> >  include/linux/pci.h                        |    3 
> >  12 files changed, 148 insertions(+), 95 deletions(-)
> 
> I'm happy to take patches 1-3 through my tree.  But maybe patches 4-8
> should go through a PCI tree?  Or at least get some ACKS from the
> appropriate maintainers to take these changes through the wireless
> trees?

Getting ACKs from Jesse and Jeff would be best since ath9k patches
5 and 8 depends on 1-3. But, crap, seems my scripts does not eval Cc
from email body to actual Cc list. Not sure if pci and e1000 guys
saw these patches. If not, I'm gona repost this set and Cc it
fully to pci and e1000 guys.

Stanislaw

Re: [PATCH -next 0/8] ath9k: ASPM fixes

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 3073 bytes --]

On Fri, 2011-08-12 at 05:35 -0700, Stanislaw Gruszka wrote:
> On Thu, Aug 11, 2011 at 12:08:48PM -0400, John W. Linville wrote:
> > On Fri, Aug 05, 2011 at 01:10:31PM +0200, Stanislaw Gruszka wrote:
> > > This patch series try to fix ath9k ASPM. Some of patches are cleanup
> > > only or do merging common code (with e1000e driver).
> > > 
> > > With CONFIG_PCIEASPM it's possible to change ASPM settings on runtime
> > > via /sys/module/pcie_aspm/parameters/policy . However most drivers
> > > I looked at assume this setting is constant. I add a callback to pci
> > > driver to inform about the change, and allow driver to do needed hw
> > > related changes. Currently only ath9k implement the callback, but
> > > I think it will be useful for other drivers i.e. iwlwifi, rtlwifi
> > > that do some own ASPM handling.
> > > 
> > > Would be nice to get some testing on patches on different
> > > platforms with ath9k devices on kernel compiled with CONFIG_PCIEASPM
> > > (default) and mixing up commands:
> > > 
> > > echo powersave > /sys/module/pcie_aspm/parameters/policy  
> > > echo performance > /sys/module/pcie_aspm/parameters/policy  
> > > ifconfig wlan0 down
> > > ifconfig wlan0 up
> > > pm-suspend
> > > 
> > > Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> > > Cc: linux-pci@vger.kernel.org
> > > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > > Cc: e1000-devel@lists.sourceforge.net
> > > ---
> > >  drivers/net/e1000e/netdev.c                |   31 ----------
> > >  drivers/net/wireless/ath/ath9k/ar9002_hw.c |   12 ---
> > >  drivers/net/wireless/ath/ath9k/ar9003_hw.c |   12 ---
> > >  drivers/net/wireless/ath/ath9k/hw-ops.h    |    9 +-
> > >  drivers/net/wireless/ath/ath9k/hw.c        |   17 +++--
> > >  drivers/net/wireless/ath/ath9k/hw.h        |   12 +--
> > >  drivers/net/wireless/ath/ath9k/main.c      |    8 --
> > >  drivers/net/wireless/ath/ath9k/pci.c       |   88 ++++++++++++++++++++++-------
> > >  drivers/pci/pcie/Makefile                  |    3 
> > >  drivers/pci/pcie/aspm.c                    |   46 ++++++++++++++-
> > >  include/linux/pci-aspm.h                   |    2 
> > >  include/linux/pci.h                        |    3 
> > >  12 files changed, 148 insertions(+), 95 deletions(-)
> > 
> > I'm happy to take patches 1-3 through my tree.  But maybe patches 4-8
> > should go through a PCI tree?  Or at least get some ACKS from the
> > appropriate maintainers to take these changes through the wireless
> > trees?
> 
> Getting ACKs from Jesse and Jeff would be best since ath9k patches
> 5 and 8 depends on 1-3. But, crap, seems my scripts does not eval Cc
> from email body to actual Cc list. Not sure if pci and e1000 guys
> saw these patches. If not, I'm gona repost this set and Cc it
> fully to pci and e1000 guys.
> 
> Stanislaw

I did see the patches, sorry for not responding.  I wanted Bruce Allan
<bruce.w.allan@intel.com> to review the e1000e changes to ensure he did
not have any issues.  To me the change looks fine based on the aspm
changes.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

Re: [PATCH -next 0/8] ath9k: ASPM fixes

[Stanislaw Gruszka]

> > > I'm happy to take patches 1-3 through my tree.  But maybe patches 4-8
> > > should go through a PCI tree?  Or at least get some ACKS from the
> > > appropriate maintainers to take these changes through the wireless
> > > trees?

Ok, John please apply first 3 patches if you still have them, if
not I'll repost.

Remaining 5 and possibly other ASPM patches for other drivers
I'll repost against pci tree, when the first 3 lend upstream.

Thanks
Stanislaw