Skip to content

Commit 3cb4f53

Browse files
hkallweitbjorn-helgaas
authored andcommitted
Revert "PCI/ASPM: Disable only ASPM_STATE_L1 when driver, disables L1"
This reverts commit fb097dc. After fb097dc ("PCI/ASPM: Disable only ASPM_STATE_L1 when driver disables L1"), disabling L1 via pci_disable_link_state(PCIE_LINK_STATE_L1), then enabling one substate, e.g., L1.1, via sysfs actually enables *all* the substates. For example, r8169 disables L1 because of hardware issues on a number of systems, which implicitly disables the L1.1 and L1.2 substates. On some systems, L1 and L1.1 work fine, but L1.2 causes missed rx packets. Enabling L1.1 via the sysfs "aspm_l1_1" attribute unexpectedly enables L1.2 as well as L1.1. After fb097dc, pci_disable_link_state(PCIE_LINK_STATE_L1) adds only ASPM_L1 (but not any of the L1.x substates) to the "aspm_disable" mask: --- Before fb097dc +++ After fb097dc # r8169 disables L1: pci_disable_link_state(PCIE_LINK_STATE_L1) - disable |= ASPM_L1 | ASPM_L1_1 | ASPM_L1_2 | ... # disable L1, L1.x + disable |= ASPM_L1 # disable L1 only # write "1" to sysfs "aspm_l1_1" attribute: l1_1_aspm aspm_attr_store_common(state = ASPM_L1_1) disable &= ~ASPM_L1_1 # enable L1.1 if (state & (ASPM_L1_1 | ...)) # if enabling any substate disable &= ~ASPM_L1 # enable L1 # final state: - disable = ASPM_L1_2 | ... # L1, L1.1 enabled; L1.2 disabled + disable = 0 # L1, L1.1, L1.2 all enabled Enabling an L1.x substate removes the substate and L1 from the "aspm_disable" mask. After fb097dc, the substates were not added to the mask when disabling L1, so enabling one substate implicitly enables all of them. Revert fb097dc so enabling one substate doesn't enable the others. Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Heiner Kallweit <[email protected]> [bhelgaas: work through example in commit log] Signed-off-by: Bjorn Helgaas <[email protected]> Cc: [email protected]
1 parent 3be31e9 commit 3cb4f53

File tree

1 file changed

+2
-1
lines changed

1 file changed

+2
-1
lines changed

drivers/pci/pcie/aspm.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1066,7 +1066,8 @@ static int __pci_disable_link_state(struct pci_dev *pdev, int state, bool sem)
10661066
if (state & PCIE_LINK_STATE_L0S)
10671067
link->aspm_disable |= ASPM_STATE_L0S;
10681068
if (state & PCIE_LINK_STATE_L1)
1069-
link->aspm_disable |= ASPM_STATE_L1;
1069+
/* L1 PM substates require L1 */
1070+
link->aspm_disable |= ASPM_STATE_L1 | ASPM_STATE_L1SS;
10701071
if (state & PCIE_LINK_STATE_L1_1)
10711072
link->aspm_disable |= ASPM_STATE_L1_1;
10721073
if (state & PCIE_LINK_STATE_L1_2)

0 commit comments

Comments
 (0)