Skip to content

Commit 1bf238e

Browse files
l1kgregkh
authored andcommitted
PCI/ASPM: Fix pci_clear_and_set_config_dword() usage
commit cc33985 upstream. When aspm_calc_l12_info() programs the L1 PM Substates Control 1 register fields Common_Mode_Restore_Time, LTR_L1.2_THRESHOLD_Value and _Scale, it invokes pci_clear_and_set_config_dword() in an incorrect way: For the bits to clear it selects those corresponding to the field. So far so good. But for the bits to set it passes a full register value. pci_clear_and_set_config_dword() performs a boolean OR operation which sets all bits of that value, not just the ones that were just cleared. Thus, when setting the LTR_L1.2_THRESHOLD_Value and _Scale on the child of an ASPM link, aspm_calc_l12_info() also sets the Common_Mode_Restore_Time. That's a spec violation: PCIe r7.0 sec 7.8.3.3 says this field is RsvdP for Upstream Ports. On Adrià's Pixelbook Eve, Common_Mode_Restore_Time of the Intel 7265 "Stone Peak" wifi card is zero, yet aspm_calc_l12_info() does not preserve the zero bits but instead programs the value calculated for the Root Port into the wifi card. Likewise, when setting the Common_Mode_Restore_Time on the Root Port, aspm_calc_l12_info() also changes the LTR_L1.2_THRESHOLD_Value and _Scale from the initial 163840 nsec to 237568 nsec (due to ORing those fields), only to reduce it afterwards to 106496 nsec. Amend all invocations of pci_clear_and_set_config_dword() to only set bits which are cleared. Finally, when setting the T_POWER_ON_Value and _Scale on the Root Port and the wifi card, aspm_calc_l12_info() fails to preserve bits declared RsvdP and instead overwrites them with zeroes. Replace pci_write_config_dword() with pci_clear_and_set_config_dword() to avoid this. Fixes: aeda9ad ("PCI/ASPM: Configure L1 substate settings") Link: https://bugzilla.kernel.org/show_bug.cgi?id=220705#c22 Signed-off-by: Lukas Wunner <lukas@wunner.de> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> Tested-by: Adrià Vilanova Martínez <me@avm99963.com> Cc: stable@vger.kernel.org # v4.11+ Link: https://patch.msgid.link/5c1752d7512eed0f4ea57b84b12d7ee08ca61fc5.1771226659.git.lukas@wunner.de Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 24582f5 commit 1bf238e

1 file changed

Lines changed: 12 additions & 5 deletions

File tree

drivers/pci/pcie/aspm.c

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -706,22 +706,29 @@ static void aspm_calc_l12_info(struct pcie_link_state *link,
706706
}
707707

708708
/* Program T_POWER_ON times in both ports */
709-
pci_write_config_dword(parent, parent->l1ss + PCI_L1SS_CTL2, ctl2);
710-
pci_write_config_dword(child, child->l1ss + PCI_L1SS_CTL2, ctl2);
709+
pci_clear_and_set_config_dword(parent, parent->l1ss + PCI_L1SS_CTL2,
710+
PCI_L1SS_CTL2_T_PWR_ON_VALUE |
711+
PCI_L1SS_CTL2_T_PWR_ON_SCALE, ctl2);
712+
pci_clear_and_set_config_dword(child, child->l1ss + PCI_L1SS_CTL2,
713+
PCI_L1SS_CTL2_T_PWR_ON_VALUE |
714+
PCI_L1SS_CTL2_T_PWR_ON_SCALE, ctl2);
711715

712716
/* Program Common_Mode_Restore_Time in upstream device */
713717
pci_clear_and_set_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1,
714-
PCI_L1SS_CTL1_CM_RESTORE_TIME, ctl1);
718+
PCI_L1SS_CTL1_CM_RESTORE_TIME,
719+
ctl1 & PCI_L1SS_CTL1_CM_RESTORE_TIME);
715720

716721
/* Program LTR_L1.2_THRESHOLD time in both ports */
717722
pci_clear_and_set_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1,
718723
PCI_L1SS_CTL1_LTR_L12_TH_VALUE |
719724
PCI_L1SS_CTL1_LTR_L12_TH_SCALE,
720-
ctl1);
725+
ctl1 & (PCI_L1SS_CTL1_LTR_L12_TH_VALUE |
726+
PCI_L1SS_CTL1_LTR_L12_TH_SCALE));
721727
pci_clear_and_set_config_dword(child, child->l1ss + PCI_L1SS_CTL1,
722728
PCI_L1SS_CTL1_LTR_L12_TH_VALUE |
723729
PCI_L1SS_CTL1_LTR_L12_TH_SCALE,
724-
ctl1);
730+
ctl1 & (PCI_L1SS_CTL1_LTR_L12_TH_VALUE |
731+
PCI_L1SS_CTL1_LTR_L12_TH_SCALE));
725732

726733
if (pl1_2_enables || cl1_2_enables) {
727734
pci_clear_and_set_config_dword(parent,

0 commit comments

Comments
 (0)