Re: [PATCH v2 1/2] PCI/ASPM: Override the ASPM and Clock PM states set by BIOS for devicetree platforms
From: Shawn Lin
Date: Wed Oct 15 2025 - 09:00:47 EST
在 2025/10/15 星期三 20:17, Niklas Cassel 写道:
On Wed, Oct 15, 2025 at 04:03:53PM +0530, Manivannan Sadhasivam wrote:
On Wed, Oct 15, 2025 at 11:46:02AM +0200, Niklas Cassel wrote:
Hello Shawn,
On Wed, Oct 15, 2025 at 05:11:39PM +0800, Shawn Lin wrote:
Thanks! Could you please try the below diff with f3ac2ff14834 applied?
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 214ed060ca1b..0069d06c282d 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -2525,6 +2525,15 @@ static void quirk_disable_aspm_l0s_l1(struct pci_dev *dev)
*/
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ASMEDIA, 0x1080, quirk_disable_aspm_l0s_l1);
+
+static void quirk_disable_aspm_all(struct pci_dev *dev)
+{
+ pci_info(dev, "Disabling ASPM\n");
+ pci_disable_link_state(dev, PCIE_LINK_STATE_ALL);
+}
+
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ROCKCHIP, 0x3588, quirk_disable_aspm_all);
That's not true from my POV. Rockchip platform supports all ASPM policy
after mass production verification. I also verified current upstream
code this morning with RK3588-EVB and can check L0s/L1/L1ss work fine.
The log and lspci output could be found here:
https://pastebin.com/qizeYED7
Moreover, I disscussed this issue with FUKAUMI today off-list and his
board seems to work when only disable L1ss by patching:
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -813,7 +813,7 @@ static void pcie_aspm_override_default_link_state(struct
pcie_link_state *link)
/* For devicetree platforms, enable all ASPM states by default */
if (of_have_populated_dt()) {
- link->aspm_default = PCIE_LINK_STATE_ASPM_ALL;
+ link->aspm_default = PCIE_LINK_STATE_L0S |
PCIE_LINK_STATE_L1;
override = link->aspm_default & ~link->aspm_enabled;
if (override)
pci_info(pdev, "ASPM: DT platform,
So, is there a proper way to just disable this feature for spec boards
instead of this Soc?
This fix seems do the trick, without needing to patch common code (aspm.c):
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 3e2752c7dd09..f5e1aaa97719 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -200,6 +200,19 @@ static bool rockchip_pcie_link_up(struct dw_pcie *pci)
return FIELD_GET(PCIE_LINKUP_MASK, val) == PCIE_LINKUP;
}
+static void rockchip_pcie_disable_l1sub(struct dw_pcie *pci)
+{
+ u32 cap, l1subcap;
+
+ cap = dw_pcie_find_ext_capability(pci, PCI_EXT_CAP_ID_L1SS);
+ if (cap) {
+ l1subcap = dw_pcie_readl_dbi(pci, cap + PCI_L1SS_CAP);
+ l1subcap &= ~(PCI_L1SS_CAP_ASPM_L1_1 | PCI_L1SS_CAP_ASPM_L1_2 | PCI_L1SS_CAP_L1_PM_SS);
+ dw_pcie_writel_dbi(pci, cap + PCI_L1SS_CAP, l1subcap);
+ l1subcap = dw_pcie_readl_dbi(pci, cap + PCI_L1SS_CAP);
+ }
+}
+
static void rockchip_pcie_enable_l0s(struct dw_pcie *pci)
{
u32 cap, lnkcap;
@@ -264,6 +277,7 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
rockchip);
+ rockchip_pcie_disable_l1sub(pci);
For now, this is a acceptable option if default ASPM policy enable L1ss
w/o checking if the HW could supports it... But how about adding
supports-clkreq stuff to upstream host driver directly? That would help
folks enable L1ss if the HW is ready and they just need adding property
to the DT.
rockchip_pcie_enable_l0s(pci);
return 0;
@@ -301,6 +315,7 @@ static void rockchip_pcie_ep_init(struct dw_pcie_ep *ep)
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
enum pci_barno bar;
+ rockchip_pcie_disable_l1sub(pci);
rockchip_pcie_enable_l0s(pci);
rockchip_pcie_ep_hide_broken_ats_cap_rk3588(ep);
But this patch removes the L1SS CAP for all boards, isn't it?
Yes, all boards supported by pcie-dw-rockchip.c, which matches what their
downstream driver does.
(Their downstream driver disables L1 substates for all boards that have
not defined 'supports-clkreq', and a grep through their downstream tree,
for all their all their different branches, shows that not a since rockchip
DTS has this property specified.)
The L1ss support is quite strict and need several steps to check, so we
didn't add supports-clkreq for them unless the HW is ready to go...
For adding supports of L1ss,
[1] the HW should support CLKREQ#, expecially for PCIe3.0 case on Rockchip SoCs , since both CLKREQ# of RC and EP should connect to the
100MHz crystal generator's enable pin, as L1.2 need to disable refclk as
well. If the enable pin is high active, the HW even need a invertor....
[2] define proper clkreq iomux to pinctrl of pcie node
[3] make sure the devices work fine with L1ss.(It's hard to check the slot case with random devices in the wild )
[4] add supports-clkreq to the DT and enable
CONFIG_PCIEASPM_POWER_SUPERSAVE
So, let me submit a real patch with the above.
Kind regards,
Niklas