Skip to contentSkip to conversation

ath9k: ASPM fixes

PATCH New Stanislaw Gruszka 3 participants 13 messages Aug 5, 2011 – Aug 17, 2011

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

cover ath9k: ASPM fixes

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 <[email protected]>
Cc linux-pci at vger.kernel.org
Cc Jeff Kirsher <[email protected]>
Cc e1000-devel at 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(-)
#1 lore raw

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

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.

---
 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(-)
drivers/net/wireless/ath/ath9k/ar9002_hw.c +2 -3
index 44d9d8d..70a18d1 100644
@@ -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
drivers/net/wireless/ath/ath9k/ar9003_hw.c +2 -3
index ad2bb2b..e3d58bd 100644
@@ -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);
 
drivers/net/wireless/ath/ath9k/hw-ops.h +2 -3
index cb29e88..8c12385 100644
@@ -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)
drivers/net/wireless/ath/ath9k/hw.h +1 -2
index c798890..a26a8c5 100644
@@ -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,
drivers/net/wireless/ath/ath9k/main.c +3 -3
index 9098aaa..651f633 100644
@@ -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
drivers/net/wireless/ath/ath9k/pci.c +2 -1
index be4ea13..8256e97 100644
@@ -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);
 	}
 }
 
Signature · 1 line
1.7.1
#2 lore raw

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

In reply to Stanislaw Gruszka

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

---
 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(-)
drivers/net/wireless/ath/ath9k/ar9002_hw.c +0 -3
index 70a18d1..b54ab78 100644
@@ -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)) {
drivers/net/wireless/ath/ath9k/ar9003_hw.c +0 -3
index e3d58bd..9cf5d13 100644
@@ -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 */
drivers/net/wireless/ath/ath9k/hw-ops.h +3 -0
index 8c12385..dd9003e 100644
@@ -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);
 }
 
Signature · 1 line
1.7.1
#3 lore raw

PATCH 3/8 ath9k: do btcoex ASPM disabling at initialization time

In reply to 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 ...

---
 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(-)
drivers/net/wireless/ath/ath9k/hw.c +4 -4
index 8dcefe7..71cfe1a 100644
@@ -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");
drivers/net/wireless/ath/ath9k/hw.h +0 -4
index a26a8c5..f28f339 100644
@@ -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
drivers/net/wireless/ath/ath9k/main.c +0 -2
index 651f633..b0389c3 100644
@@ -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);
 	}
drivers/net/wireless/ath/ath9k/pci.c +25 -19

71 lines not on this page · raw message

Signature · 1 line
1.7.1
#4 lore raw

PATCH 4/8 pci: aspm: add function for disabling ASPM

In reply to Stanislaw Gruszka

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

Cc Jesse Barnes <[email protected]>
Cc linux-pci at vger.kernel.org
Cc Jeff Kirsher <[email protected]>
Cc e1000-devel at lists.sourceforge.net
---
 drivers/pci/pcie/Makefile |    3 +--
 drivers/pci/pcie/aspm.c   |   41 +++++++++++++++++++++++++++++++++++++++++
 include/linux/pci-aspm.h  |    2 ++
 3 files changed, 44 insertions(+), 2 deletions(-)
drivers/pci/pcie/Makefile +1 -2
index 00c62df..6e20c59 100644
@@ -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
drivers/pci/pcie/aspm.c +41 -0

50 lines not on this page · raw message

include/linux/pci-aspm.h +2 -0
index 7cea7b6..f9722e6 100644
@@ -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);
Signature · 1 line
1.7.1
#5 lore raw

PATCH 5/8 ath9k: use common function for disabling ASPM

In reply to Stanislaw Gruszka

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

---
 drivers/net/wireless/ath/ath9k/pci.c |   23 +++--------------------
 1 files changed, 3 insertions(+), 20 deletions(-)
drivers/net/wireless/ath/ath9k/pci.c +3 -20
index 7d94b64..67a2c96 100644
@@ -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;
 
Signature · 1 line
1.7.1
#6 lore raw

PATCH 6/8 e1000e: use common function for disabling ASPM

In reply to Stanislaw Gruszka
Cc Jeff Kirsher <[email protected]>
Cc e1000-devel at lists.sourceforge.net
---
 drivers/net/e1000e/netdev.c |   31 +------------------------------
 1 files changed, 1 insertions(+), 30 deletions(-)
drivers/net/e1000e/netdev.c +1 -30
index 3310c3d..62ee7e4 100644
@@ -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
Signature · 1 line
1.7.1
#7 lore raw

PATCH 7/8 pci: aspm: add settings changed callback

In reply to 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 <[email protected]>
Cc linux-pci at vger.kernel.org
---
 drivers/pci/pcie/aspm.c |    5 ++++-
 include/linux/pci.h     |    3 +++
 2 files changed, 7 insertions(+), 1 deletions(-)
drivers/pci/pcie/aspm.c +4 -1
index 2883fc3..d7acec8 100644
@@ -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);
 
include/linux/pci.h +3 -0
index c446b5c..2110d63 100644
@@ -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;
Signature · 1 line
1.7.1
#8 lore raw

PATCH 8/8 ath9k: be prepare for dynamic ASPM settings change

In reply to 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.

---
 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(-)
drivers/net/wireless/ath/ath9k/hw-ops.h +1 -0
index dd9003e..0743e82 100644
@@ -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;
 
drivers/net/wireless/ath/ath9k/hw.h +1 -0
index f28f339..cb388ff 100644
@@ -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;
drivers/net/wireless/ath/ath9k/main.c +2 -1
index b0389c3..a7e3c5c 100644
@@ -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,
drivers/net/wireless/ath/ath9k/pci.c +38 -0
index 67a2c96..c585180 100644
@@ -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,
 };
 
Signature · 1 line
1.7.1
#9 lore raw

cover ath9k: ASPM fixes

In reply to Stanislaw Gruszka
Quoted 25 lines · 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 <[email protected]>
Cc linux-pci at vger.kernel.org
Cc Jeff Kirsher <[email protected]>
Cc e1000-devel at lists.sourceforge.net
Quoted 14 lines
---
 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

Signature · 2 lines
John W. Linville		Someday the world will need a hero, and you
linville at tuxdriver.com			might be all we have.  Be ready.
#10 lore raw

cover ath9k: ASPM fixes

Quoted 41 lines · John W. Linville
Quoted 25 lines · 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 <[email protected]>
Cc linux-pci at vger.kernel.org
Cc Jeff Kirsher <[email protected]>
Cc e1000-devel at lists.sourceforge.net
Quoted 14 lines
---
 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(-)
Quoted 4 lines

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

#11 lore raw

cover ath9k: ASPM fixes

Quoted 47 lines · Stanislaw Gruszka
Quoted 41 lines · John W. Linville
Quoted 25 lines · 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 <[email protected]>
Cc linux-pci at vger.kernel.org
Cc Jeff Kirsher <[email protected]>
Cc e1000-devel at lists.sourceforge.net
Quoted 14 lines
---
 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(-)
Quoted 4 lines

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?

Quoted 7 lines

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 <[email protected]> to review the e1000e changes to ensure he did
not have any issues. To me the change looks fine based on the aspm
changes.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part
Url : http://lists.ath9k.org/pipermail/ath9k-devel/attachments/20110812/ed86114d/attachment.pgp

#12 lore raw

cover ath9k: ASPM fixes

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines

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