The L1 substates support requires additional steps to work, see e.g. section '11.6.6.4 L1 Substate' in the RK3588 TRM V1.0.
These steps are currently missing from the driver.
While this has always been a problem when using e.g. CONFIG_PCIEASPM_POWER_SUPERSAVE=y, the problem became more apparent after commit f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree platforms"), which enabled ASPM also for CONFIG_PCIEASPM_DEFAULT=y.
Disable L1 substates until proper support is added.
Cc: stable@vger.kernel.org Fixes: 0e898eb8df4e ("PCI: rockchip-dwc: Add Rockchip RK356X host controller driver") Fixes: f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree platforms") Signed-off-by: Niklas Cassel cassel@kernel.org --- Changes since v1: -Remove superfluous dw_pcie_readl_dbi()
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c index 3e2752c7dd09..84f882abbca5 100644 --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c @@ -200,6 +200,25 @@ static bool rockchip_pcie_link_up(struct dw_pcie *pci) return FIELD_GET(PCIE_LINKUP_MASK, val) == PCIE_LINKUP; }
+/* + * See e.g. section '11.6.6.4 L1 Substate' in the RK3588 TRM V1.0 for the steps + * needed to support L1 substates. Currently, not a single rockchip platform + * performs these steps, so disable L1 substates until there is proper support. + */ +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_L1_PM_SS | PCI_L1SS_CAP_ASPM_L1_1 | + PCI_L1SS_CAP_ASPM_L1_2 | PCI_L1SS_CAP_PCIPM_L1_1 | + PCI_L1SS_CAP_PCIPM_L1_2); + dw_pcie_writel_dbi(pci, cap + PCI_L1SS_CAP, l1subcap); + } +} + static void rockchip_pcie_enable_l0s(struct dw_pcie *pci) { u32 cap, lnkcap; @@ -264,6 +283,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); rockchip_pcie_enable_l0s(pci);
return 0; @@ -301,6 +321,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);
在 2025/10/16 星期四 17:04, Niklas Cassel 写道:
The L1 substates support requires additional steps to work, see e.g. section '11.6.6.4 L1 Substate' in the RK3588 TRM V1.0.
These steps are currently missing from the driver.
Yes, we could add them later if concerns about supports-clkreq is fully discussed.
While this has always been a problem when using e.g. CONFIG_PCIEASPM_POWER_SUPERSAVE=y, the problem became more apparent after commit f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree platforms"), which enabled ASPM also for CONFIG_PCIEASPM_DEFAULT=y.
Disable L1 substates until proper support is added.
Thanks for the patch.
Acked-by: Shawn Lin shawn.lin@rock-chips.com
Cc: stable@vger.kernel.org Fixes: 0e898eb8df4e ("PCI: rockchip-dwc: Add Rockchip RK356X host controller driver") Fixes: f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree platforms") Signed-off-by: Niklas Cassel cassel@kernel.org
Changes since v1: -Remove superfluous dw_pcie_readl_dbi()
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c index 3e2752c7dd09..84f882abbca5 100644 --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c @@ -200,6 +200,25 @@ static bool rockchip_pcie_link_up(struct dw_pcie *pci) return FIELD_GET(PCIE_LINKUP_MASK, val) == PCIE_LINKUP; } +/*
- See e.g. section '11.6.6.4 L1 Substate' in the RK3588 TRM V1.0 for the steps
- needed to support L1 substates. Currently, not a single rockchip platform
- performs these steps, so disable L1 substates until there is proper support.
- */
+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_L1_PM_SS | PCI_L1SS_CAP_ASPM_L1_1 |
PCI_L1SS_CAP_ASPM_L1_2 | PCI_L1SS_CAP_PCIPM_L1_1 |
PCI_L1SS_CAP_PCIPM_L1_2);
dw_pcie_writel_dbi(pci, cap + PCI_L1SS_CAP, l1subcap);
- }
+}
- static void rockchip_pcie_enable_l0s(struct dw_pcie *pci) { u32 cap, lnkcap;
@@ -264,6 +283,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); rockchip_pcie_enable_l0s(pci);
return 0; @@ -301,6 +321,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);
On Thu, Oct 16, 2025 at 11:04:22AM +0200, Niklas Cassel wrote:
The L1 substates support requires additional steps to work, see e.g. section '11.6.6.4 L1 Substate' in the RK3588 TRM V1.0.
These steps are currently missing from the driver.
Can we outline here specifically what is missing?
While this has always been a problem when using e.g. CONFIG_PCIEASPM_POWER_SUPERSAVE=y, the problem became more apparent after commit f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree platforms"), which enabled ASPM also for CONFIG_PCIEASPM_DEFAULT=y.
Should also be able to trigger this problem regardless of CONFIG_PCIEASPM_* by using /sys/bus/pci/devices/.../link/l1_2_aspm.
Disable L1 substates until proper support is added.
I would word this more like "prevent advertising L1 Substates support" since we're not actually *disabling* anything here.
If the RK3588 TRM is publicly available, a URL here would be helpful.
Cc: stable@vger.kernel.org Fixes: 0e898eb8df4e ("PCI: rockchip-dwc: Add Rockchip RK356X host controller driver") Fixes: f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree platforms") Signed-off-by: Niklas Cassel cassel@kernel.org
Changes since v1: -Remove superfluous dw_pcie_readl_dbi()
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c index 3e2752c7dd09..84f882abbca5 100644 --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c @@ -200,6 +200,25 @@ static bool rockchip_pcie_link_up(struct dw_pcie *pci) return FIELD_GET(PCIE_LINKUP_MASK, val) == PCIE_LINKUP; } +/*
- See e.g. section '11.6.6.4 L1 Substate' in the RK3588 TRM V1.0 for the steps
- needed to support L1 substates. Currently, not a single rockchip platform
- performs these steps, so disable L1 substates until there is proper support.
- */
+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_L1_PM_SS | PCI_L1SS_CAP_ASPM_L1_1 |
PCI_L1SS_CAP_ASPM_L1_2 | PCI_L1SS_CAP_PCIPM_L1_1 |
PCI_L1SS_CAP_PCIPM_L1_2);
I suspect this problem is specifically related to L1.2 and CLKREQ#, and L1.1 might work fine. If so, can we update this so we still advertise L1.1 support?
dw_pcie_writel_dbi(pci, cap + PCI_L1SS_CAP, l1subcap);
- }
+}
static void rockchip_pcie_enable_l0s(struct dw_pcie *pci) { u32 cap, lnkcap; @@ -264,6 +283,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); rockchip_pcie_enable_l0s(pci);
return 0; @@ -301,6 +321,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);
2.51.0
On Thu, Oct 16, 2025 at 12:25:04PM -0500, Bjorn Helgaas wrote:
On Thu, Oct 16, 2025 at 11:04:22AM +0200, Niklas Cassel wrote:
The L1 substates support requires additional steps to work, see e.g. section '11.6.6.4 L1 Substate' in the RK3588 TRM V1.0.
+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_L1_PM_SS | PCI_L1SS_CAP_ASPM_L1_1 |
PCI_L1SS_CAP_ASPM_L1_2 | PCI_L1SS_CAP_PCIPM_L1_1 |
PCI_L1SS_CAP_PCIPM_L1_2);
I suspect this problem is specifically related to L1.2 and CLKREQ#, and L1.1 might work fine. If so, can we update this so we still advertise L1.1 support?
Hmm, looking again at the spec (PCIe r7.0, sec 5.5), it looks like CLKREQ# is required for both L1.1 and L1.2 (but not L1.0, the basic L1), so ignore my comment here.
On Thu, Oct 16, 2025 at 12:25:04PM -0500, Bjorn Helgaas wrote:
On Thu, Oct 16, 2025 at 11:04:22AM +0200, Niklas Cassel wrote:
The L1 substates support requires additional steps to work, see e.g. section '11.6.6.4 L1 Substate' in the RK3588 TRM V1.0.
These steps are currently missing from the driver.
Can we outline here specifically what is missing?
Sure.
While this has always been a problem when using e.g. CONFIG_PCIEASPM_POWER_SUPERSAVE=y, the problem became more apparent after commit f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree platforms"), which enabled ASPM also for CONFIG_PCIEASPM_DEFAULT=y.
Should also be able to trigger this problem regardless of CONFIG_PCIEASPM_* by using /sys/bus/pci/devices/.../link/l1_2_aspm.
Disable L1 substates until proper support is added.
I would word this more like "prevent advertising L1 Substates support" since we're not actually *disabling* anything here.
Sure.
If the RK3588 TRM is publicly available, a URL here would be helpful.
I couldn't find it on any official rockchip or radxa URL.
Will submit a v3 that fixes your comments.
Kind regards, Niklas
linux-stable-mirror@lists.linaro.org