The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v3 0/8] PCI/ASPM: Add pci_force_enable_link_state() API and convert Atheros drivers
@ 2026-07-08 14:30 Manivannan Sadhasivam
  2026-07-08 14:30 ` [PATCH v3 1/8] PCI/ASPM: Always disable ASPM when driver requests it Manivannan Sadhasivam
                   ` (7 more replies)
  0 siblings, 8 replies; 10+ messages in thread
From: Manivannan Sadhasivam @ 2026-07-08 14:30 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Nirmal Patel, Jonathan Derrick, Jeff Johnson
  Cc: linux-pci, linux-kernel, linux-arm-msm, linux-wireless, ath12k,
	ath11k, ath10k, Krishna Chaitanya Chundru, Qiang Yu, mani,
	Ilpo Järvinen, Manivannan Sadhasivam

Hi,

This series converts the atheros WLAN drivers (ath10k/ath11k/ath12k) to
use the PCI core ASPM APIs for enabling/disabling ASPM instead of writing
to the LNKCTL register directly. Touching LNKCTL behind the PCI core's
back leaves the ASPM service driver's view of the link state out of sync
with the hardware, which is not nice.

The catch is that these drivers save the currently enabled ASPM states,
disable ASPM before firmware download, and restore the saved states
afterwards. pci_enable_link_state() cannot restore states that were
disabled via pci_disable_link_state() or blacklisted during init (like
the pre-1.1 device blacklist, which is only meant to be overridden with
pcie_aspm=force), and that is intentional. So the save/restore sequence
cannot be expressed with the existing APIs.

In v2 I tried to fix this by making pci_enable_link_state*() symmetric
with the disable APIs, but as pointed out during review, that also ends
up overriding the init-time blacklist for every caller (including VMD),
which is not something we want. So instead, this version adds a separate
pci_force_enable_link_state() API (patch 2) that re-enables the requested
states unconditionally. The caller is responsible for only asking for
states the device actually supports, and the ath drivers do this by
restoring exactly what pcie_aspm_enabled() reported earlier.

The first five patches touch the PCI core: patch 1 (from Ilpo) makes
pci_disable_link_state() disable ASPM unconditionally, patch 2 adds the
new API, patch 3 moves the D0 transition needed before enabling PCI-PM
L1 PM Substates (PCIe r6.0, sec 5.5.4) into the shared helper and drops
it from the vmd and pcie-qcom callers, patch 4 cleans up the kernel-doc,
and patch 5 makes pcie_aspm_enabled() return the enabled state mask so it
can be used for save/restore. The last three patches convert the ath
drivers.

Testing
=======

I've tested this series on a Lenovo Thinkpad T14s with WCN7850 (so that's
just the ath12k driver). Rest of the drivers are compile tested only.

Merging Strategy
================

Even though there is no build dependency between the PCI core and atheros
patches, there is a functional dependency. So I'd recommend creating an
immutable branch with the PCI patches and merging that branch into both
the PCI and linux-wireless trees, and finally merging the atheros patches
into the linux-wireless tree.

If the immutable branch seems like a hassle, then the PCI core patches
could get merged first and the atheros patches can wait for the next
cycle.

- Mani

Changes in v3:
* Dropped the change that made pci_enable_link_state*() symmetric with the
  disable APIs. Instead added a new pci_force_enable_link_state() API used
  only by the ath drivers, so the init-time blacklist stays overridable
  only via pcie_aspm=force and existing callers like VMD are left
  untouched.
* Fixed the locked path to use pci_set_power_state_locked() for the D0
  transition. The previous version called the non-locked variant, which
  re-acquires pci_bus_sem.
* ath drivers now disable PCIE_LINK_STATE_ASPM_ALL instead of
  PCIE_LINK_STATE_ALL so that CLKPM is preserved and can be restored
  (pcie_aspm_enabled() doesn't report CLKPM).
* Reworked the kernel-doc patch to cover both the enable and disable APIs
  and reference the new API.
* Rebased on top of v7.2-rc1.

Changes in v2:
* Reworked the pcie_aspm_enabled() API to return the enabled states
  instead of bool and used it to save/restore the ASPM states in ath
  drivers.
* Added a patch from Ilpo to disable ASPM even if BIOS didn't grant
  permission.
* Added the CONFIG_PCIEASPM dependency to ath{10/11/12}k drivers.
* Rebased on top of v6.17-rc1.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
Ilpo Järvinen (1):
      PCI/ASPM: Always disable ASPM when driver requests it

Manivannan Sadhasivam (7):
      PCI/ASPM: Add pci_force_enable_link_state() API
      PCI/ASPM: Transition the device to D0 (if required) when enabling ASPM link states
      PCI/ASPM: Improve the kernel-doc for pci_{enable,disable}_link_state*() APIs
      PCI/ASPM: Return enabled ASPM states from pcie_aspm_enabled() API
      wifi: ath12k: Use pci_{enable/disable}_link_state() APIs to enable/disable ASPM states
      wifi: ath11k: Use pci_{enable/disable}_link_state() APIs to enable/disable ASPM states
      wifi: ath10k: Use pci_{enable/disable}_link_state() APIs to enable/disable ASPM states

 drivers/net/wireless/ath/ath10k/Kconfig |   2 +-
 drivers/net/wireless/ath/ath10k/pci.c   |  11 +--
 drivers/net/wireless/ath/ath10k/pci.h   |   5 +-
 drivers/net/wireless/ath/ath11k/Kconfig |   2 +-
 drivers/net/wireless/ath/ath11k/pci.c   |  19 +----
 drivers/net/wireless/ath/ath11k/pci.h   |   3 +-
 drivers/net/wireless/ath/ath12k/Kconfig |   2 +-
 drivers/net/wireless/ath/ath12k/pci.c   |  19 +----
 drivers/net/wireless/ath/ath12k/pci.h   |   4 +-
 drivers/pci/controller/dwc/pcie-qcom.c  |   5 --
 drivers/pci/controller/vmd.c            |   5 --
 drivers/pci/pcie/aspm.c                 | 140 ++++++++++++++++++++++++--------
 include/linux/pci.h                     |   7 +-
 13 files changed, 129 insertions(+), 95 deletions(-)
---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260708-pci-aspm-fix-331826a2cb5d

Best regards,
--  
மணிவண்ணன் சதாசிவம்



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v3 1/8] PCI/ASPM: Always disable ASPM when driver requests it
  2026-07-08 14:30 [PATCH v3 0/8] PCI/ASPM: Add pci_force_enable_link_state() API and convert Atheros drivers Manivannan Sadhasivam
@ 2026-07-08 14:30 ` Manivannan Sadhasivam
  2026-07-08 14:30 ` [PATCH v3 2/8] PCI/ASPM: Add pci_force_enable_link_state() API Manivannan Sadhasivam
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Manivannan Sadhasivam @ 2026-07-08 14:30 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Nirmal Patel, Jonathan Derrick, Jeff Johnson
  Cc: linux-pci, linux-kernel, linux-arm-msm, linux-wireless, ath12k,
	ath11k, ath10k, Krishna Chaitanya Chundru, Qiang Yu, mani,
	Ilpo Järvinen, Manivannan Sadhasivam

From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

PCI core/ASPM service driver allows controlling ASPM state through
pci_disable_link_state() API. It was decided earlier (see the Link
below), to not allow ASPM changes when OS does not have control over it
but only log a warning about the problem 'commit 2add0ec14c25 ("PCI/ASPM:
Warn when driver asks to disable ASPM, but we can't do it")'.

A number of drivers have added workarounds to force ASPM off with own
writes into the Link Control Register (some even with comments
explaining why PCI core does not disable it under some circumstances).
According to the comments, some drivers require ASPM to be off for
reliable operation.

Having custom ASPM handling in drivers is problematic because the state
kept in the ASPM service driver is not updated by the changes made
outside the link state management API.

As the first step to address this issue, make pci_disable_link_state()
to unconditionally disable ASPM so the motivation for drivers to come
up with custom ASPM handling code is eliminated.

To fully take advantage of the ASPM handling core provides, the drivers
that need to quirk ASPM have to be altered depend on PCIEASPM and the
custom ASPM code is removed. This is to be done separately. As PCIEASPM
is already behind EXPERT, it should be no problem to limit disabling it
for configurations that do not require touching ASPM.

Make pci_disable_link_state() function comment to comply kerneldoc
formatting while changing the description.

Link: https://lore.kernel.org/all/CANUX_P3F5YhbZX3WGU-j1AGpbXb_T9Bis2ErhvKkFMtDvzatVQ@mail.gmail.com/
Link: https://lore.kernel.org/all/20230511131441.45704-1-ilpo.jarvinen@linux.intel.com/
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
[mani: commit message fixup]
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/pcie/aspm.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 172783e7f519..1c15a14718e8 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1449,16 +1449,23 @@ static int __pci_disable_link_state(struct pci_dev *pdev, int state, bool locked
 		return -EINVAL;
 	/*
 	 * A driver requested that ASPM be disabled on this device, but
-	 * if we don't have permission to manage ASPM (e.g., on ACPI
+	 * if we might not have permission to manage ASPM (e.g., on ACPI
 	 * systems we have to observe the FADT ACPI_FADT_NO_ASPM bit and
-	 * the _OSC method), we can't honor that request.  Windows has
-	 * a similar mechanism using "PciASPMOptOut", which is also
-	 * ignored in this situation.
+	 * the _OSC method), previously we chose to not honor disable
+	 * request in that case. Windows has a similar mechanism using
+	 * "PciASPMOptOut", which is also ignored in this situation.
+	 *
+	 * Not honoring the requests to disable ASPM, however, led to
+	 * drivers forcing ASPM off on their own. As such changes of ASPM
+	 * state are not tracked by this service driver, the state kept here
+	 * became out of sync.
+	 *
+	 * Therefore, honor ASPM disable requests even when OS does not have
+	 * ASPM control. Plain disable for ASPM is assumed to be slightly
+	 * safer than fully managing it.
 	 */
-	if (aspm_disabled) {
-		pci_warn(pdev, "can't disable ASPM; OS doesn't have ASPM control\n");
-		return -EPERM;
-	}
+	if (aspm_disabled)
+		pci_warn(pdev, "OS doesn't have ASPM control, disabling ASPM anyway\n");
 
 	if (!locked)
 		down_read(&pci_bus_sem);
@@ -1485,13 +1492,13 @@ int pci_disable_link_state_locked(struct pci_dev *pdev, int state)
 EXPORT_SYMBOL(pci_disable_link_state_locked);
 
 /**
- * pci_disable_link_state - Disable device's link state, so the link will
- * never enter specific states.  Note that if the BIOS didn't grant ASPM
- * control to the OS, this does nothing because we can't touch the LNKCTL
- * register. Returns 0 or a negative errno.
- *
+ * pci_disable_link_state - Disable device's link state
  * @pdev: PCI device
  * @state: ASPM link state to disable
+ *
+ * Disable device's link state so the link will never enter specific states.
+ *
+ * Return: 0 or a negative errno
  */
 int pci_disable_link_state(struct pci_dev *pdev, int state)
 {

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 2/8] PCI/ASPM: Add pci_force_enable_link_state() API
  2026-07-08 14:30 [PATCH v3 0/8] PCI/ASPM: Add pci_force_enable_link_state() API and convert Atheros drivers Manivannan Sadhasivam
  2026-07-08 14:30 ` [PATCH v3 1/8] PCI/ASPM: Always disable ASPM when driver requests it Manivannan Sadhasivam
@ 2026-07-08 14:30 ` Manivannan Sadhasivam
  2026-07-08 14:30 ` [PATCH v3 3/8] PCI/ASPM: Transition the device to D0 (if required) when enabling ASPM link states Manivannan Sadhasivam
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Manivannan Sadhasivam @ 2026-07-08 14:30 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Nirmal Patel, Jonathan Derrick, Jeff Johnson
  Cc: linux-pci, linux-kernel, linux-arm-msm, linux-wireless, ath12k,
	ath11k, ath10k, Krishna Chaitanya Chundru, Qiang Yu, mani,
	Ilpo Järvinen, Manivannan Sadhasivam

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

pci_enable_link_state() and pci_enable_link_state_locked() APIs only enable
the ASPM states that were not previously disabled via
pci_disable_link_state*() API or blacklisted during init (e.g. the pre-1.1
device blacklist, which is only meant to be overridden with
'pcie_aspm=force'). This is an intentional behavior as these APIs must not
silently re-enable states that were deliberately disabled, since doing so
could enable ASPM on a link where it is known to be unsafe.

However, some drivers (e.g. the atheros WLAN drivers) save the currently
enabled ASPM states, disable all ASPM states before firmware download, and
then try to restore exactly the states that were enabled before. Restoring
those states requires re-enabling states that were just disabled via
pci_disable_link_state() API. But, this cannot be achieved using the
existing APIs.

Hence, add pci_force_enable_link_state() API for such callers. Unlike
pci_enable_link_state(), it re-enables the requested states even if they
were previously disabled via pci_disable_link_state() or disabled during
init. The caller is therefore responsible for only enabling states the
device actually supports, typically the ones previously reported by
pcie_aspm_enabled().

This API is implemented by adding a 'force' parameter to the shared
__pci_enable_link_state() helper. When 'force' is true, the requested
states are cleared from 'link->aspm_disable' before enabling.

Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/pcie/aspm.c | 37 ++++++++++++++++++++++++++++++++++---
 include/linux/pci.h     |  3 +++
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 1c15a14718e8..c04fb71de91c 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1506,7 +1506,8 @@ int pci_disable_link_state(struct pci_dev *pdev, int state)
 }
 EXPORT_SYMBOL(pci_disable_link_state);
 
-static int __pci_enable_link_state(struct pci_dev *pdev, int state, bool locked)
+static int __pci_enable_link_state(struct pci_dev *pdev, int state, bool locked,
+				   bool force)
 {
 	struct pcie_link_state *link = pcie_aspm_get_link(pdev);
 
@@ -1527,6 +1528,10 @@ static int __pci_enable_link_state(struct pci_dev *pdev, int state, bool locked)
 		down_read(&pci_bus_sem);
 	mutex_lock(&aspm_lock);
 	link->aspm_default = pci_calc_aspm_enable_mask(state);
+
+	/* Force enable states that were previously disabled */
+	if (force)
+		link->aspm_disable &= ~link->aspm_default;
 	pcie_config_aspm_link(link, policy_to_aspm_state(link));
 
 	link->clkpm_default = (state & PCIE_LINK_STATE_CLKPM) ? 1 : 0;
@@ -1553,7 +1558,7 @@ static int __pci_enable_link_state(struct pci_dev *pdev, int state, bool locked)
  */
 int pci_enable_link_state(struct pci_dev *pdev, int state)
 {
-	return __pci_enable_link_state(pdev, state, false);
+	return __pci_enable_link_state(pdev, state, false, false);
 }
 EXPORT_SYMBOL(pci_enable_link_state);
 
@@ -1576,10 +1581,36 @@ int pci_enable_link_state_locked(struct pci_dev *pdev, int state)
 {
 	lockdep_assert_held_read(&pci_bus_sem);
 
-	return __pci_enable_link_state(pdev, state, true);
+	return __pci_enable_link_state(pdev, state, true, false);
 }
 EXPORT_SYMBOL(pci_enable_link_state_locked);
 
+/**
+ * pci_force_enable_link_state - Force enable device link state
+ * @pdev: PCI device
+ * @state: Mask of ASPM link states to enable
+ *
+ * Enable device link state, so the link will enter the specified states.
+ * Unlike pci_enable_link_state(), this also re-enables states previously
+ * disabled by pci_disable_link_state() and overrides the ASPM states disabled
+ * during init (e.g., the pre-1.1 device blacklist). The caller is therefore
+ * responsible for only enabling states the device supports, typically the ones
+ * previously reported by pcie_aspm_enabled().
+ *
+ * Note that if the BIOS didn't grant ASPM control to the OS, this does nothing
+ * because we can't touch the LNKCTL register.
+ *
+ * Note: Ensure devices are in D0 before enabling PCI-PM L1 PM Substates, per
+ * PCIe r6.0, sec 5.5.4.
+ *
+ * Return: 0 on success, a negative errno otherwise.
+ */
+int pci_force_enable_link_state(struct pci_dev *pdev, int state)
+{
+	return __pci_enable_link_state(pdev, state, false, true);
+}
+EXPORT_SYMBOL(pci_force_enable_link_state);
+
 void pcie_aspm_remove_cap(struct pci_dev *pdev, u32 lnkcap)
 {
 	if (lnkcap & PCI_EXP_LNKCAP_ASPM_L0S)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index ebb5b9d76360..be6719626c8f 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1918,6 +1918,7 @@ int pci_disable_link_state(struct pci_dev *pdev, int state);
 int pci_disable_link_state_locked(struct pci_dev *pdev, int state);
 int pci_enable_link_state(struct pci_dev *pdev, int state);
 int pci_enable_link_state_locked(struct pci_dev *pdev, int state);
+int pci_force_enable_link_state(struct pci_dev *pdev, int state);
 void pcie_no_aspm(void);
 bool pcie_aspm_support_enabled(void);
 bool pcie_aspm_enabled(struct pci_dev *pdev);
@@ -1930,6 +1931,8 @@ static inline int pci_enable_link_state(struct pci_dev *pdev, int state)
 { return 0; }
 static inline int pci_enable_link_state_locked(struct pci_dev *pdev, int state)
 { return 0; }
+static inline int pci_force_enable_link_state(struct pci_dev *pdev, int state)
+{ return 0; }
 static inline void pcie_no_aspm(void) { }
 static inline bool pcie_aspm_support_enabled(void) { return false; }
 static inline bool pcie_aspm_enabled(struct pci_dev *pdev) { return false; }

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 3/8] PCI/ASPM: Transition the device to D0 (if required) when enabling ASPM link states
  2026-07-08 14:30 [PATCH v3 0/8] PCI/ASPM: Add pci_force_enable_link_state() API and convert Atheros drivers Manivannan Sadhasivam
  2026-07-08 14:30 ` [PATCH v3 1/8] PCI/ASPM: Always disable ASPM when driver requests it Manivannan Sadhasivam
  2026-07-08 14:30 ` [PATCH v3 2/8] PCI/ASPM: Add pci_force_enable_link_state() API Manivannan Sadhasivam
@ 2026-07-08 14:30 ` Manivannan Sadhasivam
  2026-07-08 14:30 ` [PATCH v3 4/8] PCI/ASPM: Improve the kernel-doc for pci_{enable,disable}_link_state*() APIs Manivannan Sadhasivam
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Manivannan Sadhasivam @ 2026-07-08 14:30 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Nirmal Patel, Jonathan Derrick, Jeff Johnson
  Cc: linux-pci, linux-kernel, linux-arm-msm, linux-wireless, ath12k,
	ath11k, ath10k, Krishna Chaitanya Chundru, Qiang Yu, mani,
	Ilpo Järvinen, Manivannan Sadhasivam

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

Per PCIe spec r6.0, sec 5.5.4:

"If setting either or both of the enable bits for PCI-PM L1 PM Substates,
both ports must be configured as described in this section while in D0."

Currently, the callers of pci_enable_link_state_locked() (vmd, pcie-qcom)
transition the device to D0 themselves before enabling the link state. But
this is easy to get wrong and has to be duplicated by every caller.

Move the D0 transition into the shared __pci_enable_link_state() helper so
that all three APIs pci_enable_link_state(), pci_enable_link_state_locked()
and pci_force_enable_link_state() perform it, and only when the PCI-PM L1
PM Substates are getting enabled.

Now that the helper handles the transition, drop the redundant D0 transition
from the vmd and pcie-qcom callers.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c |  5 -----
 drivers/pci/controller/vmd.c           |  5 -----
 drivers/pci/pcie/aspm.c                | 23 +++++++++++++++++------
 3 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index d8eb52857f69..45f4caeb0814 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1085,11 +1085,6 @@ static int qcom_pcie_post_init_2_7_0(struct qcom_pcie *pcie)
 
 static int qcom_pcie_enable_aspm(struct pci_dev *pdev, void *userdata)
 {
-	/*
-	 * Downstream devices need to be in D0 state before enabling PCI PM
-	 * substates.
-	 */
-	pci_set_power_state_locked(pdev, PCI_D0);
 	pci_enable_link_state_locked(pdev, PCIE_LINK_STATE_ALL);
 
 	return 0;
diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index d4ae250d4bc6..20597d2cf66e 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -762,11 +762,6 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
 	pci_info(pdev, "VMD: Default LTR value set by driver\n");
 
 out_state_change:
-	/*
-	 * Ensure devices are in D0 before enabling PCI-PM L1 PM Substates, per
-	 * PCIe r6.0, sec 5.5.4.
-	 */
-	pci_set_power_state_locked(pdev, PCI_D0);
 	pci_enable_link_state_locked(pdev, PCIE_LINK_STATE_ALL);
 	return 0;
 }
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index c04fb71de91c..6d6862fd2ebb 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1524,6 +1524,17 @@ static int __pci_enable_link_state(struct pci_dev *pdev, int state, bool locked,
 		return -EPERM;
 	}
 
+	/*
+	 * Ensure the device is in D0 before enabling PCI-PM L1 PM Substates, per
+	 * PCIe r6.0, sec 5.5.4.
+	 */
+	if (state & PCIE_LINK_STATE_L1_SS_PCIPM) {
+		if (locked)
+			pci_set_power_state_locked(pdev, PCI_D0);
+		else
+			pci_set_power_state(pdev, PCI_D0);
+	}
+
 	if (!locked)
 		down_read(&pci_bus_sem);
 	mutex_lock(&aspm_lock);
@@ -1550,8 +1561,8 @@ static int __pci_enable_link_state(struct pci_dev *pdev, int state, bool locked,
  * touch the LNKCTL register. Also note that this does not enable states
  * disabled by pci_disable_link_state(). Return 0 or a negative errno.
  *
- * Note: Ensure devices are in D0 before enabling PCI-PM L1 PM Substates, per
- * PCIe r6.0, sec 5.5.4.
+ * Note: The device will be transitioned to D0 state if the PCI-PM L1 Substates
+ * are getting enabled.
  *
  * @pdev: PCI device
  * @state: Mask of ASPM link states to enable
@@ -1569,8 +1580,8 @@ EXPORT_SYMBOL(pci_enable_link_state);
  * can't touch the LNKCTL register. Also note that this does not enable states
  * disabled by pci_disable_link_state(). Return 0 or a negative errno.
  *
- * Note: Ensure devices are in D0 before enabling PCI-PM L1 PM Substates, per
- * PCIe r6.0, sec 5.5.4.
+ * Note: The device will be transitioned to D0 state if the PCI-PM L1 Substates
+ * are getting enabled.
  *
  * @pdev: PCI device
  * @state: Mask of ASPM link states to enable
@@ -1600,8 +1611,8 @@ EXPORT_SYMBOL(pci_enable_link_state_locked);
  * Note that if the BIOS didn't grant ASPM control to the OS, this does nothing
  * because we can't touch the LNKCTL register.
  *
- * Note: Ensure devices are in D0 before enabling PCI-PM L1 PM Substates, per
- * PCIe r6.0, sec 5.5.4.
+ * Note: The device will be transitioned to D0 state if the PCI-PM L1 Substates
+ * are getting enabled.
  *
  * Return: 0 on success, a negative errno otherwise.
  */

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 4/8] PCI/ASPM: Improve the kernel-doc for pci_{enable,disable}_link_state*() APIs
  2026-07-08 14:30 [PATCH v3 0/8] PCI/ASPM: Add pci_force_enable_link_state() API and convert Atheros drivers Manivannan Sadhasivam
                   ` (2 preceding siblings ...)
  2026-07-08 14:30 ` [PATCH v3 3/8] PCI/ASPM: Transition the device to D0 (if required) when enabling ASPM link states Manivannan Sadhasivam
@ 2026-07-08 14:30 ` Manivannan Sadhasivam
  2026-07-08 14:30 ` [PATCH v3 5/8] PCI/ASPM: Return enabled ASPM states from pcie_aspm_enabled() API Manivannan Sadhasivam
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Manivannan Sadhasivam @ 2026-07-08 14:30 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Nirmal Patel, Jonathan Derrick, Jeff Johnson
  Cc: linux-pci, linux-kernel, linux-arm-msm, linux-wireless, ath12k,
	ath11k, ath10k, Krishna Chaitanya Chundru, Qiang Yu, mani,
	Ilpo Järvinen, Manivannan Sadhasivam

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

Add kernel-doc for pci_disable_link_state_locked() API and fix the
kernel-doc for pci_disable_link_state() API.

Also convert the kernel-doc of pci_enable_link_state() and
pci_enable_link_state_locked() APIs to the standard format and mention that
pci_force_enable_link_state() should be used to re-enable the states
disabled by pci_disable_link_state().

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/pcie/aspm.c | 47 ++++++++++++++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 6d6862fd2ebb..5e7e199c78d2 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1483,6 +1483,17 @@ static int __pci_disable_link_state(struct pci_dev *pdev, int state, bool locked
 	return 0;
 }
 
+/**
+ * pci_disable_link_state_locked - Disable device's link state
+ * @pdev: PCI device
+ * @state: ASPM link state to disable
+ *
+ * Disable device's link state so the link will never enter specific states.
+ *
+ * Context: Caller holds pci_bus_sem read lock.
+ *
+ * Return: 0 on success, a negative errno otherwise.
+ */
 int pci_disable_link_state_locked(struct pci_dev *pdev, int state)
 {
 	lockdep_assert_held_read(&pci_bus_sem);
@@ -1555,17 +1566,20 @@ static int __pci_enable_link_state(struct pci_dev *pdev, int state, bool locked,
 }
 
 /**
- * pci_enable_link_state - Clear and set the default device link state so that
- * the link may be allowed to enter the specified states. Note that if the
- * BIOS didn't grant ASPM control to the OS, this does nothing because we can't
- * touch the LNKCTL register. Also note that this does not enable states
- * disabled by pci_disable_link_state(). Return 0 or a negative errno.
+ * pci_enable_link_state - Enable device's link state
+ * @pdev: PCI device
+ * @state: Mask of ASPM link states to enable
+ *
+ * Enable device's link state, so the link will enter the specified states.
+ * Note that if the BIOS didn't grant ASPM control to the OS, this does
+ * nothing because we can't touch the LNKCTL register. Also note that this
+ * does not enable states disabled by pci_disable_link_state(). Use
+ * pci_force_enable_link_state() for that.
  *
  * Note: The device will be transitioned to D0 state if the PCI-PM L1 Substates
  * are getting enabled.
  *
- * @pdev: PCI device
- * @state: Mask of ASPM link states to enable
+ * Return: 0 on success, a negative errno otherwise.
  */
 int pci_enable_link_state(struct pci_dev *pdev, int state)
 {
@@ -1574,19 +1588,22 @@ int pci_enable_link_state(struct pci_dev *pdev, int state)
 EXPORT_SYMBOL(pci_enable_link_state);
 
 /**
- * pci_enable_link_state_locked - Clear and set the default device link state
- * so that the link may be allowed to enter the specified states. Note that if
- * the BIOS didn't grant ASPM control to the OS, this does nothing because we
- * can't touch the LNKCTL register. Also note that this does not enable states
- * disabled by pci_disable_link_state(). Return 0 or a negative errno.
+ * pci_enable_link_state_locked - Enable device's link state
+ * @pdev: PCI device
+ * @state: Mask of ASPM link states to enable
+ *
+ * Enable device's link state, so the link will enter the specified states.
+ * Note that if the BIOS didn't grant ASPM control to the OS, this does
+ * nothing because we can't touch the LNKCTL register. Also note that this
+ * does not enable states disabled by pci_disable_link_state(). Use
+ * pci_force_enable_link_state() for that.
  *
  * Note: The device will be transitioned to D0 state if the PCI-PM L1 Substates
  * are getting enabled.
  *
- * @pdev: PCI device
- * @state: Mask of ASPM link states to enable
- *
  * Context: Caller holds pci_bus_sem read lock.
+ *
+ * Return: 0 on success, a negative errno otherwise.
  */
 int pci_enable_link_state_locked(struct pci_dev *pdev, int state)
 {

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 5/8] PCI/ASPM: Return enabled ASPM states from pcie_aspm_enabled() API
  2026-07-08 14:30 [PATCH v3 0/8] PCI/ASPM: Add pci_force_enable_link_state() API and convert Atheros drivers Manivannan Sadhasivam
                   ` (3 preceding siblings ...)
  2026-07-08 14:30 ` [PATCH v3 4/8] PCI/ASPM: Improve the kernel-doc for pci_{enable,disable}_link_state*() APIs Manivannan Sadhasivam
@ 2026-07-08 14:30 ` Manivannan Sadhasivam
  2026-07-08 14:30 ` [PATCH v3 6/8] wifi: ath12k: Use pci_{enable/disable}_link_state() APIs to enable/disable ASPM states Manivannan Sadhasivam
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Manivannan Sadhasivam @ 2026-07-08 14:30 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Nirmal Patel, Jonathan Derrick, Jeff Johnson
  Cc: linux-pci, linux-kernel, linux-arm-msm, linux-wireless, ath12k,
	ath11k, ath10k, Krishna Chaitanya Chundru, Qiang Yu, mani,
	Ilpo Järvinen, Manivannan Sadhasivam

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

pcie_aspm_enabled() returns 'pcie_link_state::aspm_enabled' parameter which
contains the enabled states. But the API currently returns the 'bool' type
which is used by the callers to decide if ASPM is enabled or not.

To allow the future callers to also make use of the enabled ASPM states,
return the actual type of 'pcie_link_state::aspm_enabled' parameter, 'u32'.

Existing callers can still treat the return value as a 'bool' as the C11
standard guarantees the behavior (this API relied on the same behavior
before as well).

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/pcie/aspm.c | 6 ++++--
 include/linux/pci.h     | 4 ++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 5e7e199c78d2..3ce6b9319566 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1694,15 +1694,17 @@ module_param_call(policy, pcie_aspm_set_policy, pcie_aspm_get_policy,
 	NULL, 0644);
 
 /**
- * pcie_aspm_enabled - Check if PCIe ASPM has been enabled for a device.
+ * pcie_aspm_enabled - Get the enabled PCIe ASPM states for a device.
  * @pdev: Target device.
  *
  * Relies on the upstream bridge's link_state being valid.  The link_state
  * is deallocated only when the last child of the bridge (i.e., @pdev or a
  * sibling) is removed, and the caller should be holding a reference to
  * @pdev, so this should be safe.
+ *
+ * Return: Enabled PCIe ASPM states. 0 if ASPM is disabled.
  */
-bool pcie_aspm_enabled(struct pci_dev *pdev)
+u32 pcie_aspm_enabled(struct pci_dev *pdev)
 {
 	struct pcie_link_state *link = pcie_aspm_get_link(pdev);
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index be6719626c8f..61f6b78c19fd 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1921,7 +1921,7 @@ int pci_enable_link_state_locked(struct pci_dev *pdev, int state);
 int pci_force_enable_link_state(struct pci_dev *pdev, int state);
 void pcie_no_aspm(void);
 bool pcie_aspm_support_enabled(void);
-bool pcie_aspm_enabled(struct pci_dev *pdev);
+u32 pcie_aspm_enabled(struct pci_dev *pdev);
 #else
 static inline int pci_disable_link_state(struct pci_dev *pdev, int state)
 { return 0; }
@@ -1935,7 +1935,7 @@ static inline int pci_force_enable_link_state(struct pci_dev *pdev, int state)
 { return 0; }
 static inline void pcie_no_aspm(void) { }
 static inline bool pcie_aspm_support_enabled(void) { return false; }
-static inline bool pcie_aspm_enabled(struct pci_dev *pdev) { return false; }
+static inline u32 pcie_aspm_enabled(struct pci_dev *pdev) { return 0; }
 #endif
 
 #ifdef CONFIG_HOTPLUG_PCI

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 6/8] wifi: ath12k: Use pci_{enable/disable}_link_state() APIs to enable/disable ASPM states
  2026-07-08 14:30 [PATCH v3 0/8] PCI/ASPM: Add pci_force_enable_link_state() API and convert Atheros drivers Manivannan Sadhasivam
                   ` (4 preceding siblings ...)
  2026-07-08 14:30 ` [PATCH v3 5/8] PCI/ASPM: Return enabled ASPM states from pcie_aspm_enabled() API Manivannan Sadhasivam
@ 2026-07-08 14:30 ` Manivannan Sadhasivam
  2026-07-10 20:25   ` Bjorn Helgaas
  2026-07-08 14:30 ` [PATCH v3 7/8] wifi: ath11k: " Manivannan Sadhasivam
  2026-07-08 14:30 ` [PATCH v3 8/8] wifi: ath10k: " Manivannan Sadhasivam
  7 siblings, 1 reply; 10+ messages in thread
From: Manivannan Sadhasivam @ 2026-07-08 14:30 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Nirmal Patel, Jonathan Derrick, Jeff Johnson
  Cc: linux-pci, linux-kernel, linux-arm-msm, linux-wireless, ath12k,
	ath11k, ath10k, Krishna Chaitanya Chundru, Qiang Yu, mani,
	Ilpo Järvinen, Manivannan Sadhasivam

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

It is not recommended to enable/disable the ASPM states on the back of the
PCI core directly using the LNKCTL register. It will break the PCI core's
knowledge about the device ASPM states. So use the APIs exposed by the PCI
core to enable/disable ASPM states.

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Reported-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/net/wireless/ath/ath12k/Kconfig |  2 +-
 drivers/net/wireless/ath/ath12k/pci.c   | 19 +++----------------
 drivers/net/wireless/ath/ath12k/pci.h   |  4 +++-
 3 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/Kconfig b/drivers/net/wireless/ath/ath12k/Kconfig
index 4a2b240f967a..7852ede3eaea 100644
--- a/drivers/net/wireless/ath/ath12k/Kconfig
+++ b/drivers/net/wireless/ath/ath12k/Kconfig
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: BSD-3-Clause-Clear
 config ATH12K
 	tristate "Qualcomm Wi-Fi 7 support (ath12k)"
-	depends on MAC80211 && HAS_DMA && PCI
+	depends on MAC80211 && HAS_DMA && PCI && PCIEASPM
 	select QCOM_QMI_HELPERS
 	select MHI_BUS
 	select QRTR
diff --git a/drivers/net/wireless/ath/ath12k/pci.c b/drivers/net/wireless/ath/ath12k/pci.c
index d9a22d6afbb0..6d48fb25690d 100644
--- a/drivers/net/wireless/ath/ath12k/pci.c
+++ b/drivers/net/wireless/ath/ath12k/pci.c
@@ -884,19 +884,9 @@ static void ath12k_pci_free_region(struct ath12k_pci *ab_pci)
 
 static void ath12k_pci_aspm_disable(struct ath12k_pci *ab_pci)
 {
-	struct ath12k_base *ab = ab_pci->ab;
-
-	pcie_capability_read_word(ab_pci->pdev, PCI_EXP_LNKCTL,
-				  &ab_pci->link_ctl);
-
-	ath12k_dbg(ab, ATH12K_DBG_PCI, "pci link_ctl 0x%04x L0s %d L1 %d\n",
-		   ab_pci->link_ctl,
-		   u16_get_bits(ab_pci->link_ctl, PCI_EXP_LNKCTL_ASPM_L0S),
-		   u16_get_bits(ab_pci->link_ctl, PCI_EXP_LNKCTL_ASPM_L1));
+	ab_pci->aspm_states = pcie_aspm_enabled(ab_pci->pdev);
 
-	/* disable L0s and L1 */
-	pcie_capability_clear_word(ab_pci->pdev, PCI_EXP_LNKCTL,
-				   PCI_EXP_LNKCTL_ASPMC);
+	pci_disable_link_state(ab_pci->pdev, PCIE_LINK_STATE_ASPM_ALL);
 
 	set_bit(ATH12K_PCI_ASPM_RESTORE, &ab_pci->flags);
 }
@@ -925,10 +915,7 @@ static void ath12k_pci_aspm_restore(struct ath12k_pci *ab_pci)
 {
 	if (ab_pci->ab->hw_params->supports_aspm &&
 	    test_and_clear_bit(ATH12K_PCI_ASPM_RESTORE, &ab_pci->flags))
-		pcie_capability_clear_and_set_word(ab_pci->pdev, PCI_EXP_LNKCTL,
-						   PCI_EXP_LNKCTL_ASPMC,
-						   ab_pci->link_ctl &
-						   PCI_EXP_LNKCTL_ASPMC);
+		pci_force_enable_link_state(ab_pci->pdev, ab_pci->aspm_states);
 }
 
 static void ath12k_pci_cancel_workqueue(struct ath12k_base *ab)
diff --git a/drivers/net/wireless/ath/ath12k/pci.h b/drivers/net/wireless/ath/ath12k/pci.h
index 0e0e2020c6ae..409ef063cd69 100644
--- a/drivers/net/wireless/ath/ath12k/pci.h
+++ b/drivers/net/wireless/ath/ath12k/pci.h
@@ -128,7 +128,9 @@ struct ath12k_pci {
 
 	/* enum ath12k_pci_flags */
 	unsigned long flags;
-	u16 link_ctl;
+
+	/* Cached PCIe ASPM states */
+	u32 aspm_states;
 	unsigned long irq_flags;
 	const struct ath12k_pci_ops *pci_ops;
 	u32 qmi_instance;

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 7/8] wifi: ath11k: Use pci_{enable/disable}_link_state() APIs to enable/disable ASPM states
  2026-07-08 14:30 [PATCH v3 0/8] PCI/ASPM: Add pci_force_enable_link_state() API and convert Atheros drivers Manivannan Sadhasivam
                   ` (5 preceding siblings ...)
  2026-07-08 14:30 ` [PATCH v3 6/8] wifi: ath12k: Use pci_{enable/disable}_link_state() APIs to enable/disable ASPM states Manivannan Sadhasivam
@ 2026-07-08 14:30 ` Manivannan Sadhasivam
  2026-07-08 14:30 ` [PATCH v3 8/8] wifi: ath10k: " Manivannan Sadhasivam
  7 siblings, 0 replies; 10+ messages in thread
From: Manivannan Sadhasivam @ 2026-07-08 14:30 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Nirmal Patel, Jonathan Derrick, Jeff Johnson
  Cc: linux-pci, linux-kernel, linux-arm-msm, linux-wireless, ath12k,
	ath11k, ath10k, Krishna Chaitanya Chundru, Qiang Yu, mani,
	Ilpo Järvinen, Manivannan Sadhasivam

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

It is not recommended to enable/disable the ASPM states on the back of the
PCI core directly using the LNKCTL register. It will break the PCI core's
knowledge about the device ASPM states. So use the APIs exposed by the PCI
core to enable/disable ASPM states.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/net/wireless/ath/ath11k/Kconfig |  2 +-
 drivers/net/wireless/ath/ath11k/pci.c   | 19 +++----------------
 drivers/net/wireless/ath/ath11k/pci.h   |  3 ++-
 3 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/Kconfig b/drivers/net/wireless/ath/ath11k/Kconfig
index 122726f84492..c8b418521173 100644
--- a/drivers/net/wireless/ath/ath11k/Kconfig
+++ b/drivers/net/wireless/ath/ath11k/Kconfig
@@ -19,7 +19,7 @@ config ATH11K_AHB
 
 config ATH11K_PCI
 	tristate "Atheros ath11k PCI support"
-	depends on ATH11K && PCI
+	depends on ATH11K && PCI && PCIEASPM
 	select MHI_BUS
 	select QRTR
 	select QRTR_MHI
diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c
index 35bb9e7a63a2..da980e01d1fc 100644
--- a/drivers/net/wireless/ath/ath11k/pci.c
+++ b/drivers/net/wireless/ath/ath11k/pci.c
@@ -604,19 +604,9 @@ static void ath11k_pci_free_region(struct ath11k_pci *ab_pci)
 
 static void ath11k_pci_aspm_disable(struct ath11k_pci *ab_pci)
 {
-	struct ath11k_base *ab = ab_pci->ab;
-
-	pcie_capability_read_word(ab_pci->pdev, PCI_EXP_LNKCTL,
-				  &ab_pci->link_ctl);
-
-	ath11k_dbg(ab, ATH11K_DBG_PCI, "link_ctl 0x%04x L0s %d L1 %d\n",
-		   ab_pci->link_ctl,
-		   u16_get_bits(ab_pci->link_ctl, PCI_EXP_LNKCTL_ASPM_L0S),
-		   u16_get_bits(ab_pci->link_ctl, PCI_EXP_LNKCTL_ASPM_L1));
+	ab_pci->aspm_states = pcie_aspm_enabled(ab_pci->pdev);
 
-	/* disable L0s and L1 */
-	pcie_capability_clear_word(ab_pci->pdev, PCI_EXP_LNKCTL,
-				   PCI_EXP_LNKCTL_ASPMC);
+	pci_disable_link_state(ab_pci->pdev, PCIE_LINK_STATE_ASPM_ALL);
 
 	set_bit(ATH11K_PCI_ASPM_RESTORE, &ab_pci->flags);
 }
@@ -624,10 +614,7 @@ static void ath11k_pci_aspm_disable(struct ath11k_pci *ab_pci)
 static void ath11k_pci_aspm_restore(struct ath11k_pci *ab_pci)
 {
 	if (test_and_clear_bit(ATH11K_PCI_ASPM_RESTORE, &ab_pci->flags))
-		pcie_capability_clear_and_set_word(ab_pci->pdev, PCI_EXP_LNKCTL,
-						   PCI_EXP_LNKCTL_ASPMC,
-						   ab_pci->link_ctl &
-						   PCI_EXP_LNKCTL_ASPMC);
+		pci_force_enable_link_state(ab_pci->pdev, ab_pci->aspm_states);
 }
 
 #ifdef CONFIG_DEV_COREDUMP
diff --git a/drivers/net/wireless/ath/ath11k/pci.h b/drivers/net/wireless/ath/ath11k/pci.h
index 1e3005a4b64c..474c18741418 100644
--- a/drivers/net/wireless/ath/ath11k/pci.h
+++ b/drivers/net/wireless/ath/ath11k/pci.h
@@ -72,7 +72,8 @@ struct ath11k_pci {
 
 	/* enum ath11k_pci_flags */
 	unsigned long flags;
-	u16 link_ctl;
+	/* Cached PCIe ASPM states */
+	u32 aspm_states;
 	u64 dma_mask;
 };
 

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 8/8] wifi: ath10k: Use pci_{enable/disable}_link_state() APIs to enable/disable ASPM states
  2026-07-08 14:30 [PATCH v3 0/8] PCI/ASPM: Add pci_force_enable_link_state() API and convert Atheros drivers Manivannan Sadhasivam
                   ` (6 preceding siblings ...)
  2026-07-08 14:30 ` [PATCH v3 7/8] wifi: ath11k: " Manivannan Sadhasivam
@ 2026-07-08 14:30 ` Manivannan Sadhasivam
  7 siblings, 0 replies; 10+ messages in thread
From: Manivannan Sadhasivam @ 2026-07-08 14:30 UTC (permalink / raw)
  To: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Nirmal Patel, Jonathan Derrick, Jeff Johnson
  Cc: linux-pci, linux-kernel, linux-arm-msm, linux-wireless, ath12k,
	ath11k, ath10k, Krishna Chaitanya Chundru, Qiang Yu, mani,
	Ilpo Järvinen, Manivannan Sadhasivam

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

It is not recommended to enable/disable the ASPM states on the back of the
PCI core directly using the LNKCTL register. It will break the PCI core's
knowledge about the device ASPM states. So use the APIs exposed by the PCI
core to enable/disable ASPM states.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/Kconfig |  2 +-
 drivers/net/wireless/ath/ath10k/pci.c   | 11 ++++-------
 drivers/net/wireless/ath/ath10k/pci.h   |  5 ++---
 3 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/Kconfig b/drivers/net/wireless/ath/ath10k/Kconfig
index efb9f022d8c6..eac3ec5a4e78 100644
--- a/drivers/net/wireless/ath/ath10k/Kconfig
+++ b/drivers/net/wireless/ath/ath10k/Kconfig
@@ -17,7 +17,7 @@ config ATH10K_CE
 
 config ATH10K_PCI
 	tristate "Atheros ath10k PCI support"
-	depends on ATH10K && PCI
+	depends on ATH10K && PCI && PCIEASPM
 	help
 	  This module adds support for PCIE bus
 
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 335bc7c488e4..2e27a70065cc 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1965,9 +1965,7 @@ static int ath10k_pci_hif_start(struct ath10k *ar)
 	ath10k_pci_irq_enable(ar);
 	ath10k_pci_rx_post(ar);
 
-	pcie_capability_clear_and_set_word(ar_pci->pdev, PCI_EXP_LNKCTL,
-					   PCI_EXP_LNKCTL_ASPMC,
-					   ar_pci->link_ctl & PCI_EXP_LNKCTL_ASPMC);
+	pci_force_enable_link_state(ar_pci->pdev, ar_pci->aspm_states);
 
 	return 0;
 }
@@ -2822,10 +2820,9 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar,
 
 	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot hif power up\n");
 
-	pcie_capability_read_word(ar_pci->pdev, PCI_EXP_LNKCTL,
-				  &ar_pci->link_ctl);
-	pcie_capability_clear_word(ar_pci->pdev, PCI_EXP_LNKCTL,
-				   PCI_EXP_LNKCTL_ASPMC);
+	ar_pci->aspm_states = pcie_aspm_enabled(ar_pci->pdev);
+
+	pci_disable_link_state(ar_pci->pdev, PCIE_LINK_STATE_ASPM_ALL);
 
 	/*
 	 * Bring the target up cleanly.
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index d114778edb41..122e365b7cdd 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -128,10 +128,9 @@ struct ath10k_pci {
 	struct timer_list rx_post_retry;
 
 	/* Due to HW quirks it is recommended to disable ASPM during device
-	 * bootup. To do that the original PCI-E Link Control is stored before
-	 * device bootup is executed and re-programmed later.
+	 * bootup. To do that the ASPM states are saved and re-programmed later.
 	 */
-	u16 link_ctl;
+	u32 aspm_states;
 
 	/* Protects ps_awake and ps_wake_refcount */
 	spinlock_t ps_lock;

-- 
2.43.0



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 6/8] wifi: ath12k: Use pci_{enable/disable}_link_state() APIs to enable/disable ASPM states
  2026-07-08 14:30 ` [PATCH v3 6/8] wifi: ath12k: Use pci_{enable/disable}_link_state() APIs to enable/disable ASPM states Manivannan Sadhasivam
@ 2026-07-10 20:25   ` Bjorn Helgaas
  0 siblings, 0 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2026-07-10 20:25 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Bjorn Helgaas, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Nirmal Patel, Jonathan Derrick, Jeff Johnson,
	linux-pci, linux-kernel, linux-arm-msm, linux-wireless, ath12k,
	ath11k, ath10k, Krishna Chaitanya Chundru, Qiang Yu,
	Ilpo Järvinen, Manivannan Sadhasivam

On Wed, Jul 08, 2026 at 04:30:20PM +0200, Manivannan Sadhasivam wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> 
> It is not recommended to enable/disable the ASPM states on the back of the
> PCI core directly using the LNKCTL register. It will break the PCI core's
> knowledge about the device ASPM states. So use the APIs exposed by the PCI
> core to enable/disable ASPM states.
> 
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> 
> Reported-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>  drivers/net/wireless/ath/ath12k/Kconfig |  2 +-
>  drivers/net/wireless/ath/ath12k/pci.c   | 19 +++----------------
>  drivers/net/wireless/ath/ath12k/pci.h   |  4 +++-
>  3 files changed, 7 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/Kconfig b/drivers/net/wireless/ath/ath12k/Kconfig
> index 4a2b240f967a..7852ede3eaea 100644
> --- a/drivers/net/wireless/ath/ath12k/Kconfig
> +++ b/drivers/net/wireless/ath/ath12k/Kconfig
> @@ -1,7 +1,7 @@
>  # SPDX-License-Identifier: BSD-3-Clause-Clear
>  config ATH12K
>  	tristate "Qualcomm Wi-Fi 7 support (ath12k)"
> -	depends on MAC80211 && HAS_DMA && PCI
> +	depends on MAC80211 && HAS_DMA && PCI && PCIEASPM

As Sashiko pointed out [1], I think adding the PCIEASPM dependency is
an issue.  The PCI core should provide stubs or whatever is necessary
to make ath12k work even if PCIEASPM isn't selected.

I'm guessing this hardware depends on the ability to disable ASPM even
without PCIEASPM so we probably would need more than just empty stubs.
We do build aspm.c unconditionally now and we provide LTR and L1SS
save/restore regardless of PCIEASPM.  Maybe we need something similar
here.

[1] https://lore.kernel.org/all/20260708144904.0CC4F1F000E9@smtp.kernel.org

>  	select QCOM_QMI_HELPERS
>  	select MHI_BUS
>  	select QRTR
> diff --git a/drivers/net/wireless/ath/ath12k/pci.c b/drivers/net/wireless/ath/ath12k/pci.c
> index d9a22d6afbb0..6d48fb25690d 100644
> --- a/drivers/net/wireless/ath/ath12k/pci.c
> +++ b/drivers/net/wireless/ath/ath12k/pci.c
> @@ -884,19 +884,9 @@ static void ath12k_pci_free_region(struct ath12k_pci *ab_pci)
>  
>  static void ath12k_pci_aspm_disable(struct ath12k_pci *ab_pci)
>  {
> -	struct ath12k_base *ab = ab_pci->ab;
> -
> -	pcie_capability_read_word(ab_pci->pdev, PCI_EXP_LNKCTL,
> -				  &ab_pci->link_ctl);
> -
> -	ath12k_dbg(ab, ATH12K_DBG_PCI, "pci link_ctl 0x%04x L0s %d L1 %d\n",
> -		   ab_pci->link_ctl,
> -		   u16_get_bits(ab_pci->link_ctl, PCI_EXP_LNKCTL_ASPM_L0S),
> -		   u16_get_bits(ab_pci->link_ctl, PCI_EXP_LNKCTL_ASPM_L1));
> +	ab_pci->aspm_states = pcie_aspm_enabled(ab_pci->pdev);
>  
> -	/* disable L0s and L1 */
> -	pcie_capability_clear_word(ab_pci->pdev, PCI_EXP_LNKCTL,
> -				   PCI_EXP_LNKCTL_ASPMC);
> +	pci_disable_link_state(ab_pci->pdev, PCIE_LINK_STATE_ASPM_ALL);
>  
>  	set_bit(ATH12K_PCI_ASPM_RESTORE, &ab_pci->flags);
>  }
> @@ -925,10 +915,7 @@ static void ath12k_pci_aspm_restore(struct ath12k_pci *ab_pci)
>  {
>  	if (ab_pci->ab->hw_params->supports_aspm &&
>  	    test_and_clear_bit(ATH12K_PCI_ASPM_RESTORE, &ab_pci->flags))
> -		pcie_capability_clear_and_set_word(ab_pci->pdev, PCI_EXP_LNKCTL,
> -						   PCI_EXP_LNKCTL_ASPMC,
> -						   ab_pci->link_ctl &
> -						   PCI_EXP_LNKCTL_ASPMC);
> +		pci_force_enable_link_state(ab_pci->pdev, ab_pci->aspm_states);
>  }
>  
>  static void ath12k_pci_cancel_workqueue(struct ath12k_base *ab)
> diff --git a/drivers/net/wireless/ath/ath12k/pci.h b/drivers/net/wireless/ath/ath12k/pci.h
> index 0e0e2020c6ae..409ef063cd69 100644
> --- a/drivers/net/wireless/ath/ath12k/pci.h
> +++ b/drivers/net/wireless/ath/ath12k/pci.h
> @@ -128,7 +128,9 @@ struct ath12k_pci {
>  
>  	/* enum ath12k_pci_flags */
>  	unsigned long flags;
> -	u16 link_ctl;
> +
> +	/* Cached PCIe ASPM states */
> +	u32 aspm_states;
>  	unsigned long irq_flags;
>  	const struct ath12k_pci_ops *pci_ops;
>  	u32 qmi_instance;
> 
> -- 
> 2.43.0
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-07-10 20:25 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-08 14:30 [PATCH v3 0/8] PCI/ASPM: Add pci_force_enable_link_state() API and convert Atheros drivers Manivannan Sadhasivam
2026-07-08 14:30 ` [PATCH v3 1/8] PCI/ASPM: Always disable ASPM when driver requests it Manivannan Sadhasivam
2026-07-08 14:30 ` [PATCH v3 2/8] PCI/ASPM: Add pci_force_enable_link_state() API Manivannan Sadhasivam
2026-07-08 14:30 ` [PATCH v3 3/8] PCI/ASPM: Transition the device to D0 (if required) when enabling ASPM link states Manivannan Sadhasivam
2026-07-08 14:30 ` [PATCH v3 4/8] PCI/ASPM: Improve the kernel-doc for pci_{enable,disable}_link_state*() APIs Manivannan Sadhasivam
2026-07-08 14:30 ` [PATCH v3 5/8] PCI/ASPM: Return enabled ASPM states from pcie_aspm_enabled() API Manivannan Sadhasivam
2026-07-08 14:30 ` [PATCH v3 6/8] wifi: ath12k: Use pci_{enable/disable}_link_state() APIs to enable/disable ASPM states Manivannan Sadhasivam
2026-07-10 20:25   ` Bjorn Helgaas
2026-07-08 14:30 ` [PATCH v3 7/8] wifi: ath11k: " Manivannan Sadhasivam
2026-07-08 14:30 ` [PATCH v3 8/8] wifi: ath10k: " Manivannan Sadhasivam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox