Previously, ASPM service driver has ignored link state management
requests when OS is not authorized to touch LNKCTL (or ASPM is not
configured at all). Because the core interface has not been reliable,
drivers have create workarounds to force ASPM state by directly
writing into LNKCTL themselves.
A second problem is lack of symmetric pair for
pci_disable_link_state(). Any link state disable is permanent (NOTE:
pci_enable_link_state() despite its name is not a symmetric pair for
pci_disable_link_state()). The lack of way to re-enable ASPM prevents
drivers from using pci_disable_link_state() to disabling ASPM for
certain phases of driver operation and re-enabling it later.
Both cases are problematic because when ASPM is working normally
through the service driver, it is not aware of the extra link state
changes drivers perform directly causing the service driver to have
incorrect view about the ASPM state.
Address these problems by making pci_disable_link_state() reliable and
by providing proper pci_enable_link_state() pair for it (the function
currently on the way is renamed first to a more descriptive name).
After core improvements, convert drivers to use the new interface and
drop the workarounds.
v2:
- Rebased the series
- Reorder patches (rename patch first)
Ilpo Järvinen (13):
PCI/ASPM: Rename pci_enable_link_state() to
pci_set_default_link_state()
PCI/ASPM: Improve pci_set_default_link_state() kerneldoc
PCI/ASPM: Disable ASPM when driver requests it
PCI/ASPM: Move L0S/L1/sub states mask calculation into a helper
PCI/ASPM: Add pci_enable_link_state()
Bluetooth: hci_bcm4377: Convert aspm disable to quirk
mt76: Remove unreliable pci_disable_link_state() workaround
e1000e: Remove unreliable pci_disable_link_state{,_locked}()
workaround
wifi: ath10k: Use pci_disable/enable_link_state()
wifi: ath11k: Use pci_disable/enable_link_state()
wifi: ath12k: Use pci_disable/enable_link_state()
RDMA/hfi1: Use pci_disable/enable_link_state()
misc: rtsx: Use pci_disable/enable_link_state()
drivers/bluetooth/hci_bcm4377.c | 20 ---
drivers/infiniband/hw/hfi1/aspm.c | 38 +-----
drivers/infiniband/hw/hfi1/pcie.c | 2 +-
drivers/misc/cardreader/rts5228.c | 6 +-
drivers/misc/cardreader/rts5261.c | 6 +-
drivers/misc/cardreader/rtsx_pcr.c | 8 +-
drivers/net/ethernet/intel/e1000e/netdev.c | 77 +----------
drivers/net/wireless/ath/ath10k/pci.c | 8 +-
drivers/net/wireless/ath/ath11k/pci.c | 10 +-
drivers/net/wireless/ath/ath12k/pci.c | 10 +-
drivers/net/wireless/mediatek/mt76/Makefile | 1 -
drivers/net/wireless/mediatek/mt76/mt76.h | 1 -
.../net/wireless/mediatek/mt76/mt7615/pci.c | 2 +-
.../net/wireless/mediatek/mt76/mt76x0/pci.c | 2 +-
.../net/wireless/mediatek/mt76/mt76x2/pci.c | 2 +-
.../net/wireless/mediatek/mt76/mt7915/pci.c | 2 +-
.../net/wireless/mediatek/mt76/mt7921/pci.c | 2 +-
.../net/wireless/mediatek/mt76/mt7996/pci.c | 2 +-
drivers/net/wireless/mediatek/mt76/pci.c | 47 -------
drivers/pci/controller/vmd.c | 2 +-
drivers/pci/pcie/Makefile | 1 +
drivers/pci/pcie/aspm.c | 126 +++++++++++++-----
drivers/pci/pcie/aspm_minimal.c | 66 +++++++++
drivers/pci/quirks.c | 3 +
include/linux/pci.h | 10 +-
25 files changed, 199 insertions(+), 255 deletions(-)
delete mode 100644 drivers/net/wireless/mediatek/mt76/pci.c
create mode 100644 drivers/pci/pcie/aspm_minimal.c
--
2.30.2
rtsx driver adjusts ASPM state itself which leaves ASPM service
driver in PCI core unaware of the link state changes the driver
implemented.
Call pci_disable_link_state() and pci_enable_link_state() instead of
adjusting ASPMC field in LNKCTL directly in the driver and let PCI core
handle the ASPM state management.
Signed-off-by: Ilpo Järvinen <[email protected]>
---
drivers/misc/cardreader/rts5228.c | 6 ++----
drivers/misc/cardreader/rts5261.c | 6 ++----
drivers/misc/cardreader/rtsx_pcr.c | 8 +++++---
3 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/misc/cardreader/rts5228.c b/drivers/misc/cardreader/rts5228.c
index f4ab09439da7..8d3216c64ad1 100644
--- a/drivers/misc/cardreader/rts5228.c
+++ b/drivers/misc/cardreader/rts5228.c
@@ -497,8 +497,7 @@ static void rts5228_enable_aspm(struct rtsx_pcr *pcr, bool enable)
val = FORCE_ASPM_CTL0 | FORCE_ASPM_CTL1;
val |= (pcr->aspm_en & 0x02);
rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, mask, val);
- pcie_capability_clear_and_set_word(pcr->pci, PCI_EXP_LNKCTL,
- PCI_EXP_LNKCTL_ASPMC, pcr->aspm_en);
+ pci_enable_link_state(pcr->pci, pcr->aspm_en);
pcr->aspm_enabled = enable;
}
@@ -509,8 +508,7 @@ static void rts5228_disable_aspm(struct rtsx_pcr *pcr, bool enable)
if (pcr->aspm_enabled == enable)
return;
- pcie_capability_clear_and_set_word(pcr->pci, PCI_EXP_LNKCTL,
- PCI_EXP_LNKCTL_ASPMC, 0);
+ pci_disable_link_state(pcr->pci, PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1);
mask = FORCE_ASPM_VAL_MASK | FORCE_ASPM_CTL0 | FORCE_ASPM_CTL1;
val = FORCE_ASPM_CTL0 | FORCE_ASPM_CTL1;
rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, mask, val);
diff --git a/drivers/misc/cardreader/rts5261.c b/drivers/misc/cardreader/rts5261.c
index 94af6bf8a25a..f1ef15683a2f 100644
--- a/drivers/misc/cardreader/rts5261.c
+++ b/drivers/misc/cardreader/rts5261.c
@@ -578,8 +578,7 @@ static void rts5261_enable_aspm(struct rtsx_pcr *pcr, bool enable)
val |= (pcr->aspm_en & 0x02);
rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, mask, val);
- pcie_capability_clear_and_set_word(pcr->pci, PCI_EXP_LNKCTL,
- PCI_EXP_LNKCTL_ASPMC, pcr->aspm_en);
+ pci_enable_link_state(pcr->pci, pcr->aspm_en);
pcr->aspm_enabled = enable;
}
@@ -591,8 +590,7 @@ static void rts5261_disable_aspm(struct rtsx_pcr *pcr, bool enable)
if (pcr->aspm_enabled == enable)
return;
- pcie_capability_clear_and_set_word(pcr->pci, PCI_EXP_LNKCTL,
- PCI_EXP_LNKCTL_ASPMC, 0);
+ pci_disable_link_state(pcr->pci, PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1);
rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, mask, val);
rtsx_pci_write_register(pcr, SD_CFG1, SD_ASYNC_FIFO_NOT_RST, 0);
udelay(10);
diff --git a/drivers/misc/cardreader/rtsx_pcr.c b/drivers/misc/cardreader/rtsx_pcr.c
index a3f4b52bb159..6efb792152f2 100644
--- a/drivers/misc/cardreader/rtsx_pcr.c
+++ b/drivers/misc/cardreader/rtsx_pcr.c
@@ -86,9 +86,11 @@ static void rtsx_comm_set_aspm(struct rtsx_pcr *pcr, bool enable)
return;
if (pcr->aspm_mode == ASPM_MODE_CFG) {
- pcie_capability_clear_and_set_word(pcr->pci, PCI_EXP_LNKCTL,
- PCI_EXP_LNKCTL_ASPMC,
- enable ? pcr->aspm_en : 0);
+ if (enable)
+ pci_enable_link_state(pcr->pci, pcr->aspm_en);
+ else
+ pci_disable_link_state(pcr->pci, PCIE_LINK_STATE_L0S |
+ PCIE_LINK_STATE_L1);
} else if (pcr->aspm_mode == ASPM_MODE_REG) {
if (pcr->aspm_en & 0x02)
rtsx_pci_write_register(pcr, ASPM_FORCE_CTL, FORCE_ASPM_CTL0 |
--
2.30.2
ath10k driver adjusts ASPM state itself which leaves ASPM service
driver in PCI core unaware of the link state changes the driver
implemented.
Call pci_disable_link_state() and pci_enable_link_state() instead of
adjusting ASPMC field in LNKCTL directly in the driver and let PCI core
handle the ASPM state management.
Signed-off-by: Ilpo Järvinen <[email protected]>
---
drivers/net/wireless/ath/ath10k/pci.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 23f366221939..64f7133ce122 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1963,9 +1963,8 @@ 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_enable_link_state(ar_pci->pdev, ar_pci->link_ctl &
+ (PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1));
return 0;
}
@@ -2822,8 +2821,7 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar,
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);
+ pci_disable_link_state(ar_pci->pdev, PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1);
/*
* Bring the target up cleanly.
--
2.30.2
pci_disable_link_state() and pci_disable_link_state_locked() were made
reliable regardless of ASPM CONFIG and OS being disallowed to change
ASPM states to allow drivers to rely on them working.
Remove driver working around unreliable
pci_disable_link_state{,_locked}() from e1000e driver and just call the
functions directly.
Signed-off-by: Ilpo Järvinen <[email protected]>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 77 +---------------------
1 file changed, 2 insertions(+), 75 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index f536c856727c..fbe468061591 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -6765,79 +6765,6 @@ static int __e1000_shutdown(struct pci_dev *pdev, bool runtime)
return 0;
}
-/**
- * __e1000e_disable_aspm - Disable ASPM states
- * @pdev: pointer to PCI device struct
- * @state: bit-mask of ASPM states to disable
- * @locked: indication if this context holds pci_bus_sem locked.
- *
- * Some devices *must* have certain ASPM states disabled per hardware errata.
- **/
-static void __e1000e_disable_aspm(struct pci_dev *pdev, u16 state, int locked)
-{
- struct pci_dev *parent = pdev->bus->self;
- u16 aspm_dis_mask = 0;
- u16 pdev_aspmc, parent_aspmc;
-
- switch (state) {
- case PCIE_LINK_STATE_L0S:
- case PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1:
- aspm_dis_mask |= PCI_EXP_LNKCTL_ASPM_L0S;
- fallthrough; /* can't have L1 without L0s */
- case PCIE_LINK_STATE_L1:
- aspm_dis_mask |= PCI_EXP_LNKCTL_ASPM_L1;
- break;
- default:
- return;
- }
-
- pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &pdev_aspmc);
- pdev_aspmc &= PCI_EXP_LNKCTL_ASPMC;
-
- if (parent) {
- pcie_capability_read_word(parent, PCI_EXP_LNKCTL,
- &parent_aspmc);
- parent_aspmc &= PCI_EXP_LNKCTL_ASPMC;
- }
-
- /* Nothing to do if the ASPM states to be disabled already are */
- if (!(pdev_aspmc & aspm_dis_mask) &&
- (!parent || !(parent_aspmc & aspm_dis_mask)))
- return;
-
- dev_info(&pdev->dev, "Disabling ASPM %s %s\n",
- (aspm_dis_mask & pdev_aspmc & PCI_EXP_LNKCTL_ASPM_L0S) ?
- "L0s" : "",
- (aspm_dis_mask & pdev_aspmc & PCI_EXP_LNKCTL_ASPM_L1) ?
- "L1" : "");
-
-#ifdef CONFIG_PCIEASPM
- if (locked)
- pci_disable_link_state_locked(pdev, state);
- else
- pci_disable_link_state(pdev, state);
-
- /* Double-check ASPM control. If not disabled by the above, the
- * BIOS is preventing that from happening (or CONFIG_PCIEASPM is
- * not enabled); override by writing PCI config space directly.
- */
- pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &pdev_aspmc);
- pdev_aspmc &= PCI_EXP_LNKCTL_ASPMC;
-
- if (!(aspm_dis_mask & pdev_aspmc))
- return;
-#endif
-
- /* Both device and parent should have the same ASPM setting.
- * Disable ASPM in downstream component first and then upstream.
- */
- pcie_capability_clear_word(pdev, PCI_EXP_LNKCTL, aspm_dis_mask);
-
- if (parent)
- pcie_capability_clear_word(parent, PCI_EXP_LNKCTL,
- aspm_dis_mask);
-}
-
/**
* e1000e_disable_aspm - Disable ASPM states.
* @pdev: pointer to PCI device struct
@@ -6848,7 +6775,7 @@ static void __e1000e_disable_aspm(struct pci_dev *pdev, u16 state, int locked)
**/
static void e1000e_disable_aspm(struct pci_dev *pdev, u16 state)
{
- __e1000e_disable_aspm(pdev, state, 0);
+ pci_disable_link_state(pdev, state);
}
/**
@@ -6861,7 +6788,7 @@ static void e1000e_disable_aspm(struct pci_dev *pdev, u16 state)
**/
static void e1000e_disable_aspm_locked(struct pci_dev *pdev, u16 state)
{
- __e1000e_disable_aspm(pdev, state, 1);
+ pci_disable_link_state_locked(pdev, state);
}
static int e1000e_pm_thaw(struct device *dev)
--
2.30.2
Ilpo Järvinen <[email protected]> writes:
> ath10k driver adjusts ASPM state itself which leaves ASPM service
> driver in PCI core unaware of the link state changes the driver
> implemented.
>
> Call pci_disable_link_state() and pci_enable_link_state() instead of
> adjusting ASPMC field in LNKCTL directly in the driver and let PCI core
> handle the ASPM state management.
>
> Signed-off-by: Ilpo Järvinen <[email protected]>
Acked-by: Kalle Valo <[email protected]>
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches