PCI/ASPM: Disable L1 before disabling L1 PM Substates

commit 7447990137 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>
This commit is contained in:
Ajay Agarwal 2024-10-07 08:59:17 +05:30 committed by Greg Kroah-Hartman
parent 4685153b12
commit 3c4fed940d

View File

@ -805,6 +805,15 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
pcie_capability_read_word(parent, PCI_EXP_LNKCTL, &parent_lnkctl);
pcie_capability_read_word(child, PCI_EXP_LNKCTL, &child_lnkctl);
/* Disable L0s/L1 before updating L1SS config */
if (FIELD_GET(PCI_EXP_LNKCTL_ASPMC, child_lnkctl) ||
FIELD_GET(PCI_EXP_LNKCTL_ASPMC, parent_lnkctl)) {
pcie_capability_write_word(child, PCI_EXP_LNKCTL,
child_lnkctl & ~PCI_EXP_LNKCTL_ASPMC);
pcie_capability_write_word(parent, PCI_EXP_LNKCTL,
parent_lnkctl & ~PCI_EXP_LNKCTL_ASPMC);
}
/*
* Setup L0s state
*
@ -829,6 +838,13 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
aspm_l1ss_init(link);
/* Restore L0s/L1 if they were enabled */
if (FIELD_GET(PCI_EXP_LNKCTL_ASPMC, child_lnkctl) ||
FIELD_GET(PCI_EXP_LNKCTL_ASPMC, parent_lnkctl)) {
pcie_capability_write_word(parent, PCI_EXP_LNKCTL, parent_lnkctl);
pcie_capability_write_word(child, PCI_EXP_LNKCTL, child_lnkctl);
}
/* Save default state */
link->aspm_default = link->aspm_enabled;
@ -845,43 +861,12 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
}
}
/* Configure the ASPM L1 substates */
/* Configure the ASPM L1 substates. Caller must disable L1 first. */
static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state)
{
u32 val, enable_req;
u32 val;
struct pci_dev *child = link->downstream, *parent = link->pdev;
enable_req = (link->aspm_enabled ^ state) & state;
/*
* Here are the rules specified in the PCIe spec for enabling L1SS:
* - When enabling L1.x, enable bit at parent first, then at child
* - When disabling L1.x, disable bit at child first, then at parent
* - When enabling ASPM L1.x, need to disable L1
* (at child followed by parent).
* - The ASPM/PCIPM L1.2 must be disabled while programming timing
* parameters
*
* To keep it simple, disable all L1SS bits first, and later enable
* what is needed.
*/
/* Disable all L1 substates */
pci_clear_and_set_config_dword(child, child->l1ss + PCI_L1SS_CTL1,
PCI_L1SS_CTL1_L1SS_MASK, 0);
pci_clear_and_set_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1,
PCI_L1SS_CTL1_L1SS_MASK, 0);
/*
* If needed, disable L1, and it gets enabled later
* in pcie_config_aspm_link().
*/
if (enable_req & (PCIE_LINK_STATE_L1_1 | PCIE_LINK_STATE_L1_2)) {
pcie_capability_clear_word(child, PCI_EXP_LNKCTL,
PCI_EXP_LNKCTL_ASPM_L1);
pcie_capability_clear_word(parent, PCI_EXP_LNKCTL,
PCI_EXP_LNKCTL_ASPM_L1);
}
val = 0;
if (state & PCIE_LINK_STATE_L1_1)
val |= PCI_L1SS_CTL1_ASPM_L1_1;
@ -892,6 +877,20 @@ static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state)
if (state & PCIE_LINK_STATE_L1_2_PCIPM)
val |= PCI_L1SS_CTL1_PCIPM_L1_2;
/*
* PCIe r6.2, sec 5.5.4, rules for enabling L1 PM Substates:
* - Clear L1.x enable bits at child first, then at parent
* - Set L1.x enable bits at parent first, then at child
* - ASPM/PCIPM L1.2 must be disabled while programming timing
* parameters
*/
/* Disable all L1 substates */
pci_clear_and_set_config_dword(child, child->l1ss + PCI_L1SS_CTL1,
PCI_L1SS_CTL1_L1SS_MASK, 0);
pci_clear_and_set_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1,
PCI_L1SS_CTL1_L1SS_MASK, 0);
/* Enable what we need to enable */
pci_clear_and_set_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1,
PCI_L1SS_CTL1_L1SS_MASK, val);
@ -937,21 +936,30 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
dwstream |= PCI_EXP_LNKCTL_ASPM_L1;
}
/*
* Per PCIe r6.2, sec 5.5.4, setting either or both of the enable
* bits for ASPM L1 PM Substates must be done while ASPM L1 is
* disabled. Disable L1 here and apply new configuration after L1SS
* configuration has been completed.
*
* Per sec 7.5.3.7, when disabling ASPM L1, software must disable
* it in the Downstream component prior to disabling it in the
* Upstream component, and ASPM L1 must be enabled in the Upstream
* component prior to enabling it in the Downstream component.
*
* Sec 7.5.3.7 also recommends programming the same ASPM Control
* value for all functions of a multi-function device.
*/
list_for_each_entry(child, &linkbus->devices, bus_list)
pcie_config_aspm_dev(child, 0);
pcie_config_aspm_dev(parent, 0);
if (link->aspm_capable & PCIE_LINK_STATE_L1SS)
pcie_config_aspm_l1ss(link, state);
/*
* Spec 2.0 suggests all functions should be configured the
* same setting for ASPM. Enabling ASPM L1 should be done in
* upstream component first and then downstream, and vice
* versa for disabling ASPM L1. Spec doesn't mention L0S.
*/
if (state & PCIE_LINK_STATE_L1)
pcie_config_aspm_dev(parent, upstream);
pcie_config_aspm_dev(parent, upstream);
list_for_each_entry(child, &linkbus->devices, bus_list)
pcie_config_aspm_dev(child, dwstream);
if (!(state & PCIE_LINK_STATE_L1))
pcie_config_aspm_dev(parent, upstream);
link->aspm_enabled = state;