The L1 substates support requires additional steps to work, namely: -Proper handling of the CLKREQ# sideband signal. (It is mostly handled by hardware, but software still needs to set the clkreq fields in the PCIE_CLIENT_POWER_CON register to match the hardware implementation.) -Program the frequency of the aux clock into the DSP_PCIE_PL_AUX_CLK_FREQ_OFF register. (During L1 substates the core_clk is turned off and the aux_clk is used instead.)
These steps are currently missing from the driver.
For more details, see section '18.6.6.4 L1 Substate' in the RK3658 TRM 1.1 Part 2, or section '11.6.6.4 L1 Substate' in the RK3588 TRM 1.0 Part2.
While this has always been a problem when using e.g. CONFIG_PCIEASPM_POWER_SUPERSAVE=y, or when modifying /sys/bus/pci/devices/.../link/l1_2_aspm, the lacking driver support for L1 substates 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.
When using e.g. an NVMe drive connected to the PCIe controller, the problem will be seen as: nvme nvme0: controller is down; will reset: CSTS=0xffffffff, PCI_STATUS=0x10 nvme nvme0: Does your device have a faulty power saving mode enabled? nvme nvme0: Try "nvme_core.default_ps_max_latency_us=0 pcie_aspm=off pcie_port_pm=off" and report a bug
Thus, prevent advertising L1 Substates support until proper driver 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") Acked-by: Shawn Lin shawn.lin@rock-chips.com Signed-off-by: Niklas Cassel cassel@kernel.org --- Changes since v2: -Improve commit message (Bjorn)
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 Fri, Oct 17, 2025 at 06:32:53PM +0200, Niklas Cassel wrote:
The L1 substates support requires additional steps to work, namely: -Proper handling of the CLKREQ# sideband signal. (It is mostly handled by hardware, but software still needs to set the clkreq fields in the PCIE_CLIENT_POWER_CON register to match the hardware implementation.) -Program the frequency of the aux clock into the DSP_PCIE_PL_AUX_CLK_FREQ_OFF register. (During L1 substates the core_clk is turned off and the aux_clk is used instead.)
These steps are currently missing from the driver.
For more details, see section '18.6.6.4 L1 Substate' in the RK3658 TRM 1.1 Part 2, or section '11.6.6.4 L1 Substate' in the RK3588 TRM 1.0 Part2.
While this has always been a problem when using e.g. CONFIG_PCIEASPM_POWER_SUPERSAVE=y, or when modifying /sys/bus/pci/devices/.../link/l1_2_aspm, the lacking driver support for L1 substates 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.
When using e.g. an NVMe drive connected to the PCIe controller, the problem will be seen as: nvme nvme0: controller is down; will reset: CSTS=0xffffffff, PCI_STATUS=0x10 nvme nvme0: Does your device have a faulty power saving mode enabled? nvme nvme0: Try "nvme_core.default_ps_max_latency_us=0 pcie_aspm=off pcie_port_pm=off" and report a bug
Thus, prevent advertising L1 Substates support until proper driver support is added.
I think Mani is planning a change so we don't try to enable L1 Substates by default, which should avoid the regression even without a patch like this.
That will still leave the existing CONFIG_PCIEASPM_POWER_SUPERSAVE=y and sysfs l1_1_aspm problems.
And we'll need to figure out a way to allow L1.x to be enabled based on 'supports-clkreq' and possibly other info. That would likely be v6.19 material since it's new functionality.
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") Acked-by: Shawn Lin shawn.lin@rock-chips.com Signed-off-by: Niklas Cassel cassel@kernel.org
Changes since v2: -Improve commit message (Bjorn)
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);
2.51.0
On 17 October 2025 18:45:58 CEST, Bjorn Helgaas helgaas@kernel.org wrote:
(snip)
Thus, prevent advertising L1 Substates support until proper driver support is added.
I think Mani is planning a change so we don't try to enable L1 Substates by default, which should avoid the regression even without a patch like this.
Sounds good, I suggested the same: https://lore.kernel.org/linux-pci/aO9tWjgHnkATroNa@ryzen/
That will still leave the existing CONFIG_PCIEASPM_POWER_SUPERSAVE=y and sysfs l1_1_aspm problems.
Indeed, which is why I think that this patch is v6.18 material.
And we'll need to figure out a way to allow L1.x to be enabled based on 'supports-clkreq' and possibly other info. That would likely be v6.19 material since it's new functionality.
I agree.
Kind regards, Niklas
On Sat, Oct 18, 2025 at 07:07:35AM +0200, Niklas Cassel wrote:
On 17 October 2025 18:45:58 CEST, Bjorn Helgaas helgaas@kernel.org wrote:
(snip)
Thus, prevent advertising L1 Substates support until proper driver support is added.
I think Mani is planning a change so we don't try to enable L1 Substates by default, which should avoid the regression even without a patch like this.
Sounds good, I suggested the same: https://lore.kernel.org/linux-pci/aO9tWjgHnkATroNa@ryzen/
That will still leave the existing CONFIG_PCIEASPM_POWER_SUPERSAVE=y and sysfs l1_1_aspm problems.
Indeed, which is why I think that this patch is v6.18 material.
Not strictly a 6.18 material as the Kconfig/sysfs/cmdline issues existed even before we enabled the ASPM states by default in v6.18-rc1.
The default ASPM issue will be fixed by a separate patch which will be generic for all platforms. I'll apply this patch for v6.19.
- Mani
Hi Niklas,
Thank you for your work.
On 10/18/25 01:32, Niklas Cassel wrote:
The L1 substates support requires additional steps to work, namely: -Proper handling of the CLKREQ# sideband signal. (It is mostly handled by hardware, but software still needs to set the clkreq fields in the PCIE_CLIENT_POWER_CON register to match the hardware implementation.) -Program the frequency of the aux clock into the DSP_PCIE_PL_AUX_CLK_FREQ_OFF register. (During L1 substates the core_clk is turned off and the aux_clk is used instead.)
These steps are currently missing from the driver.
For more details, see section '18.6.6.4 L1 Substate' in the RK3658 TRM 1.1 Part 2, or section '11.6.6.4 L1 Substate' in the RK3588 TRM 1.0 Part2.
While this has always been a problem when using e.g. CONFIG_PCIEASPM_POWER_SUPERSAVE=y, or when modifying /sys/bus/pci/devices/.../link/l1_2_aspm, the lacking driver support for L1 substates 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.
When using e.g. an NVMe drive connected to the PCIe controller, the problem will be seen as: nvme nvme0: controller is down; will reset: CSTS=0xffffffff, PCI_STATUS=0x10 nvme nvme0: Does your device have a faulty power saving mode enabled? nvme nvme0: Try "nvme_core.default_ps_max_latency_us=0 pcie_aspm=off pcie_port_pm=off" and report a bug
Thus, prevent advertising L1 Substates support until proper driver 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") Acked-by: Shawn Lin shawn.lin@rock-chips.com Signed-off-by: Niklas Cassel cassel@kernel.org
I've confirmed this patch resolves the issue in v6.18-rc1 using the following configuration:
ROCK 5A & M.2 RTL8852BE ROCK 5B & M.2 MT7921E, NVMe SSD ROCK 5T & on-board AX210, NVMe SSD x2 ROCK 5 ITX+ & M.2 MT7922E, NVMe SSD x2
Therefore,
Tested-by: FUKAUMI Naoki naoki@radxa.com
Best regards,
(P.S. I couldn't test on the ROCK 5B & M.2 RTL8852BE, ROCK 5B+ & on-board RTL8852BE, and ROCK 5C & ASM2806 due to separate issues.)
-- FUKAUMI Naoki Radxa Computer (Shenzhen) Co., Ltd.
Changes since v2: -Improve commit message (Bjorn)
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 Fri, 17 Oct 2025 18:32:53 +0200, Niklas Cassel wrote:
The L1 substates support requires additional steps to work, namely: -Proper handling of the CLKREQ# sideband signal. (It is mostly handled by hardware, but software still needs to set the clkreq fields in the PCIE_CLIENT_POWER_CON register to match the hardware implementation.) -Program the frequency of the aux clock into the DSP_PCIE_PL_AUX_CLK_FREQ_OFF register. (During L1 substates the core_clk is turned off and the aux_clk is used instead.)
[...]
Applied, thanks!
[1/1] PCI: dw-rockchip: Prevent advertising L1 Substates support commit: 40331c63e7901a2cc75ce6b5d24d50601efb833d
Best regards,
linux-stable-mirror@lists.linaro.org
-
Bjorn Helgaas -
FUKAUMI Naoki -
Manivannan Sadhasivam -
Niklas Cassel