Revert "PCI/ASPM: Disable only ASPM_STATE_L1 when driver, disables L1"
commit 3cb4f534 upstream. This reverts commit fb097dcd. After fb097dcd ("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 fb097dcd, 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 fb097dcd +++ After fb097dcd # 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 fb097dcd, the substates were not added to the mask when disabling L1, so enabling one substate implicitly enables all of them. Revert fb097dcd so enabling one substate doesn't enable the others. Link: https://lore.kernel.org/r/c75931ac-7208-4200-9ca1-821629cf5e28@gmail.com Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> [bhelgaas: work through example in commit log] Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> Cc: stable@vger.kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Please register or sign in to comment