Skip to content

Commit 0c20720

Browse files
Ajay Agarwalpopcornmix
Ajay Agarwal
authored and
popcornmix
committed
PCI/ASPM: Disable L1 before disabling L1 PM Substates
commit 7447990 upstream. PCIe r6.2, sec 5.5.4, requires that: If setting either or both of the enable bits for ASPM L1 PM Substates, both ports must be configured as described in this section while ASPM L1 is disabled. Previously, pcie_config_aspm_l1ss() assumed that "setting enable bits" meant "setting them to 1", and it configured L1SS as follows: - Clear L1SS enable bits - Disable L1 - Configure L1SS enable bits as required - Enable L1 if required With this sequence, when disabling L1SS on an ARM A-core with a Synopsys DesignWare PCIe core, the CPU occasionally hangs when reading PCI_L1SS_CTL1, leading to a reboot when the CPU watchdog expires. Move the L1 disable to the caller (pcie_config_aspm_link(), where L1 was already enabled) so L1 is always disabled while updating the L1SS bits: - Disable L1 - Clear L1SS enable bits - Configure L1SS enable bits as required - Enable L1 if required Change pcie_aspm_cap_init() similarly. Link: https://lore.kernel.org/r/20241007032917.872262-1-ajayagarwal@google.com Signed-off-by: Ajay Agarwal <ajayagarwal@google.com> [bhelgaas: comments, commit log, compute L1SS setting before config access] Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> Tested-by: Johnny-CC Chang <Johnny-CC.Chang@mediatek.com> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 70eb23d commit 0c20720

File tree

1 file changed

+50
-42
lines changed

1 file changed

+50
-42
lines changed

‎drivers/pci/pcie/aspm.c

Lines changed: 50 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -805,6 +805,15 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
805805
pcie_capability_read_word(parent, PCI_EXP_LNKCTL, &parent_lnkctl);
806806
pcie_capability_read_word(child, PCI_EXP_LNKCTL, &child_lnkctl);
807807

808+
/* Disable L0s/L1 before updating L1SS config */
809+
if (FIELD_GET(PCI_EXP_LNKCTL_ASPMC, child_lnkctl) ||
810+
FIELD_GET(PCI_EXP_LNKCTL_ASPMC, parent_lnkctl)) {
811+
pcie_capability_write_word(child, PCI_EXP_LNKCTL,
812+
child_lnkctl & ~PCI_EXP_LNKCTL_ASPMC);
813+
pcie_capability_write_word(parent, PCI_EXP_LNKCTL,
814+
parent_lnkctl & ~PCI_EXP_LNKCTL_ASPMC);
815+
}
816+
808817
/*
809818
* Setup L0s state
810819
*
@@ -829,6 +838,13 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
829838

830839
aspm_l1ss_init(link);
831840

841+
/* Restore L0s/L1 if they were enabled */
842+
if (FIELD_GET(PCI_EXP_LNKCTL_ASPMC, child_lnkctl) ||
843+
FIELD_GET(PCI_EXP_LNKCTL_ASPMC, parent_lnkctl)) {
844+
pcie_capability_write_word(parent, PCI_EXP_LNKCTL, parent_lnkctl);
845+
pcie_capability_write_word(child, PCI_EXP_LNKCTL, child_lnkctl);
846+
}
847+
832848
/* Save default state */
833849
link->aspm_default = link->aspm_enabled;
834850

@@ -845,52 +861,35 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
845861
}
846862
}
847863

848-
/* Configure the ASPM L1 substates */
864+
/* Configure the ASPM L1 substates. Caller must disable L1 first. */
849865
static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state)
850866
{
851-
u32 val, enable_req;
867+
u32 val;
852868
struct pci_dev *child = link->downstream, *parent = link->pdev;
853869

854-
enable_req = (link->aspm_enabled ^ state) & state;
870+
val = 0;
871+
if (state & PCIE_LINK_STATE_L1_1)
872+
val |= PCI_L1SS_CTL1_ASPM_L1_1;
873+
if (state & PCIE_LINK_STATE_L1_2)
874+
val |= PCI_L1SS_CTL1_ASPM_L1_2;
875+
if (state & PCIE_LINK_STATE_L1_1_PCIPM)
876+
val |= PCI_L1SS_CTL1_PCIPM_L1_1;
877+
if (state & PCIE_LINK_STATE_L1_2_PCIPM)
878+
val |= PCI_L1SS_CTL1_PCIPM_L1_2;
855879

856880
/*
857-
* Here are the rules specified in the PCIe spec for enabling L1SS:
858-
* - When enabling L1.x, enable bit at parent first, then at child
859-
* - When disabling L1.x, disable bit at child first, then at parent
860-
* - When enabling ASPM L1.x, need to disable L1
861-
* (at child followed by parent).
862-
* - The ASPM/PCIPM L1.2 must be disabled while programming timing
881+
* PCIe r6.2, sec 5.5.4, rules for enabling L1 PM Substates:
882+
* - Clear L1.x enable bits at child first, then at parent
883+
* - Set L1.x enable bits at parent first, then at child
884+
* - ASPM/PCIPM L1.2 must be disabled while programming timing
863885
* parameters
864-
*
865-
* To keep it simple, disable all L1SS bits first, and later enable
866-
* what is needed.
867886
*/
868887

869888
/* Disable all L1 substates */
870889
pci_clear_and_set_config_dword(child, child->l1ss + PCI_L1SS_CTL1,
871890
PCI_L1SS_CTL1_L1SS_MASK, 0);
872891
pci_clear_and_set_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1,
873892
PCI_L1SS_CTL1_L1SS_MASK, 0);
874-
/*
875-
* If needed, disable L1, and it gets enabled later
876-
* in pcie_config_aspm_link().
877-
*/
878-
if (enable_req & (PCIE_LINK_STATE_L1_1 | PCIE_LINK_STATE_L1_2)) {
879-
pcie_capability_clear_word(child, PCI_EXP_LNKCTL,
880-
PCI_EXP_LNKCTL_ASPM_L1);
881-
pcie_capability_clear_word(parent, PCI_EXP_LNKCTL,
882-
PCI_EXP_LNKCTL_ASPM_L1);
883-
}
884-
885-
val = 0;
886-
if (state & PCIE_LINK_STATE_L1_1)
887-
val |= PCI_L1SS_CTL1_ASPM_L1_1;
888-
if (state & PCIE_LINK_STATE_L1_2)
889-
val |= PCI_L1SS_CTL1_ASPM_L1_2;
890-
if (state & PCIE_LINK_STATE_L1_1_PCIPM)
891-
val |= PCI_L1SS_CTL1_PCIPM_L1_1;
892-
if (state & PCIE_LINK_STATE_L1_2_PCIPM)
893-
val |= PCI_L1SS_CTL1_PCIPM_L1_2;
894893

895894
/* Enable what we need to enable */
896895
pci_clear_and_set_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1,
@@ -937,21 +936,30 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
937936
dwstream |= PCI_EXP_LNKCTL_ASPM_L1;
938937
}
939938

939+
/*
940+
* Per PCIe r6.2, sec 5.5.4, setting either or both of the enable
941+
* bits for ASPM L1 PM Substates must be done while ASPM L1 is
942+
* disabled. Disable L1 here and apply new configuration after L1SS
943+
* configuration has been completed.
944+
*
945+
* Per sec 7.5.3.7, when disabling ASPM L1, software must disable
946+
* it in the Downstream component prior to disabling it in the
947+
* Upstream component, and ASPM L1 must be enabled in the Upstream
948+
* component prior to enabling it in the Downstream component.
949+
*
950+
* Sec 7.5.3.7 also recommends programming the same ASPM Control
951+
* value for all functions of a multi-function device.
952+
*/
953+
list_for_each_entry(child, &linkbus->devices, bus_list)
954+
pcie_config_aspm_dev(child, 0);
955+
pcie_config_aspm_dev(parent, 0);
956+
940957
if (link->aspm_capable & PCIE_LINK_STATE_L1SS)
941958
pcie_config_aspm_l1ss(link, state);
942959

943-
/*
944-
* Spec 2.0 suggests all functions should be configured the
945-
* same setting for ASPM. Enabling ASPM L1 should be done in
946-
* upstream component first and then downstream, and vice
947-
* versa for disabling ASPM L1. Spec doesn't mention L0S.
948-
*/
949-
if (state & PCIE_LINK_STATE_L1)
950-
pcie_config_aspm_dev(parent, upstream);
960+
pcie_config_aspm_dev(parent, upstream);
951961
list_for_each_entry(child, &linkbus->devices, bus_list)
952962
pcie_config_aspm_dev(child, dwstream);
953-
if (!(state & PCIE_LINK_STATE_L1))
954-
pcie_config_aspm_dev(parent, upstream);
955963

956964
link->aspm_enabled = state;
957965

0 commit comments

Comments
 (0)