[PATCH v6 00/10] PCI: dw-rockchip: add system suspend support

[PATCH v6 00/10] PCI: dw-rockchip: add system suspend support

[Sebastian Reichel]

I've been working on fixing up at least basic system suspend support on
the Rockchip RK3576 platform. The last open issue is missing support in
the PCIe driver. This series is a follow-up for Shawn Lin's series with
feedback from Niklas Cassel and Manivannan Sadhasivam being handled as
well as some of my own changes fixing up things I noticed.

I've tested this on multiple RK3576 and RK3588 boards and things look
good to me on the PCIe side.

On RK3588, I also needed [0] to fix PCIe IOMMU issues after resume. I
consider this an independent issue. Also on the Rockchip EVBs the
brcmfmac driver fails to suspend. The WIFI card is connected via PCIe,
but the problem is with the driver/firmware and unrelated to PCIe
itself. I will send a separate workaround patch for that. Additionally
on RK3588 the system will run into SError on resume due to a USB2
problem. This problem is also independent and will be handled
separately.

[0] https://lore.kernel.org/all/20260728210928.1050849-1-praan@google.com/

Changes since PATCHv5:
 * https://lore.kernel.org/r/20260316-rockchip-pcie-system-suspend-v5-0-5bb5ad37d643@collabora.com
 * Rebase to pci/next
 * Minor changes to make checkpatch happy
 * Update patch description of the last patch and drop RFC tag
   according to feedback from Shawn Lin
 * Prepend new patch, which fixes Nicolas Frattaroli's conversion
   to FIELD_PREP_WM16 accidently dropping a flag.
 * Add another new patch, which sets .broken_l1ss_resume to tell the
   PCI core that the controller is powered off during suspend following
   the Qcom example

Changes since PATCHv4:
 * https://lore.kernel.org/r/20251029-rockchip-pcie-system-suspend-v4-0-ce2e1b0692d2@collabora.com
 * rebase to v7.0-rc1
 * Add an RFC patch at the end with some bits I found in the Rockchip vendor kernel;
 * Made some tests on Rock 5B+ (RK3588); with this patch series suspending works
   when setting pm_test to devices. A full system suspend fails in a later step,
   but that is independent of PCIe.

Changes since PATCHv3:
 * https://lore.kernel.org/linux-pci/1744940759-23823-1-git-send-email-shawn.lin@rock-chips.com/
 * rename rockchip_pcie_get_ltssm to rockchip_pcie_get_ltssm_status_reg
   in a separate patch (Niklas Cassel)
 * rename rockchip_pcie_get_pure_ltssm to rockchip_pcie_get_ltssm_state
   in a separate patch (Niklas Cassel)
 * Move devm_phy_get out of phy_init to probe in a separate patch
   (Manivannan Sadhasivam)
 * Add helper function for enhanced LTSSM control mode in a separate patch
   (Niklas Cassel)
 * Add helper function for controller mode in a separate patch
   (Niklas Cassel)
 * Add helper function for DDL indicator in a separate patch
   (Niklas Cassel)
 * Move rockchip_pcie_pme_turn_off implementation in a separate patch
 * Rebase to v6.18-rc3 using new FIELD_PREP_WM16()
 * Improve readability of PME_TURN_OFF/PME_TO_ACK defines (Manivannan Sadhasivam)
 * Fix usage of reverse Xmas (Manivannan Sadhasivam)
 * Assert PERST# before turning off other resources (Manivannan Sadhasivam)
 * Improve some error messages (Manivannan Sadhasivam)
 * Rename goto labels as per their purpose (Manivannan Sadhasivam)
 * Add extra patch for dw_pcie_resume_noirq, since I've seen errors
   during resume on boards not having anything plugged into their PCIe
   port

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
Sebastian Reichel (10):
      PCI: dw-rockchip: Fix LTSSM set functions
      PCI: dw-rockchip: Restore vpcie3v3 regulator handle
      PCI: dw-rockchip: Move devm_phy_get out of phy_init
      PCI: dw-rockchip: Add helper function for enhanced LTSSM control mode
      PCI: dw-rockchip: Add helper function for controller mode
      PCI: dw-rockchip: Add helper function for DDL indicator
      PCI: dw-rockchip: Add pme_turn_off support
      PCI: dw-rockchip: Set broken L1SS resume flag
      PCI: dw-rockchip: Add system PM support
      PCI: dw-rockchip: Clear debug buffer before entering L2

 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 274 ++++++++++++++++++++++----
 1 file changed, 235 insertions(+), 39 deletions(-)
---
base-commit: 39ee38a9fa2eaeff030a6c865abb244597c091eb
change-id: 20251028-rockchip-pcie-system-suspend-86cf08a7b229

Best regards,
--  
Sebastian Reichel <sebastian.reichel@collabora.com>

[PATCH v6 01/10] PCI: dw-rockchip: Fix LTSSM set functions

[Sebastian Reichel]

Before the Rockchip PCIe driver has been switched over to the
FIELD_PREP_WM16 macro, PCIE_CLIENT_ENABLE_LTSSM and
PCIE_CLIENT_DISABLE_LTSSM were setting bits with a mask of 0xc = 0b1100,
which means BIT 2 and BIT 3. After the conversion it only sets bit 2,
with bit 3 being handled by a separate define named
PCIE_CLIENT_LD_RQ_RST_GRT. Apparently the conversion missed to make use
of this new macros resulting in the third bit not being set.

Fixes: 30e919570581 ("PCI: dw-rockchip: Switch to FIELD_PREP_WM16 macro")
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index af26a07c0c9e..dc85564e5001 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -321,14 +321,16 @@ static void rockchip_pcie_ltssm_trace(struct rockchip_pcie *rockchip,
 
 static void rockchip_pcie_enable_ltssm(struct rockchip_pcie *rockchip)
 {
-	rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_ENABLE_LTSSM,
-				 PCIE_CLIENT_GENERAL_CON);
+	u32 val = PCIE_CLIENT_ENABLE_LTSSM | PCIE_CLIENT_LD_RQ_RST_GRT;
+
+	rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_GENERAL_CON);
 }
 
 static void rockchip_pcie_disable_ltssm(struct rockchip_pcie *rockchip)
 {
-	rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_DISABLE_LTSSM,
-				 PCIE_CLIENT_GENERAL_CON);
+	u32 val = PCIE_CLIENT_DISABLE_LTSSM | PCIE_CLIENT_LD_RQ_RST_GRT;
+
+	rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_GENERAL_CON);
 }
 
 static bool rockchip_pcie_link_up(struct dw_pcie *pci)

-- 
2.53.0

[PATCH v6 02/10] PCI: dw-rockchip: Restore vpcie3v3 regulator handle

[Sebastian Reichel]

This reverts commit c930b10f17c0 ("PCI: dw-rockchip: Simplify regulator
setup with devm_regulator_get_enable_optional()"), which nicely cleaned
up the code.

The vpcie3v3 regulator handle is needed to disable the regulator during
system suspend (to be added in its own patch).

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index dc85564e5001..f67ad3c31c35 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -112,6 +112,7 @@ struct rockchip_pcie {
 	unsigned int clk_cnt;
 	struct reset_control *rst;
 	struct gpio_desc *rst_gpio;
+	struct regulator *vpcie3v3;
 	struct irq_domain *irq_domain;
 	const struct rockchip_pcie_of_data *data;
 	bool supports_clkreq;
@@ -844,15 +845,22 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 		return ret;
 
 	/* DON'T MOVE ME: must be enable before PHY init */
-	ret = devm_regulator_get_enable_optional(dev, "vpcie3v3");
-	if (ret < 0 && ret != -ENODEV)
-		return dev_err_probe(dev, ret,
-				     "failed to enable vpcie3v3 regulator\n");
+	rockchip->vpcie3v3 = devm_regulator_get_optional(dev, "vpcie3v3");
+	if (IS_ERR(rockchip->vpcie3v3)) {
+		if (PTR_ERR(rockchip->vpcie3v3) != -ENODEV)
+			return dev_err_probe(dev, PTR_ERR(rockchip->vpcie3v3),
+					"failed to get vpcie3v3 regulator\n");
+		rockchip->vpcie3v3 = NULL;
+	} else {
+		ret = regulator_enable(rockchip->vpcie3v3);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "failed to enable vpcie3v3 regulator\n");
+	}
 
 	ret = rockchip_pcie_phy_init(rockchip);
 	if (ret)
-		return dev_err_probe(dev, ret,
-				     "failed to initialize the phy\n");
+		goto disable_regulator;
 
 	ret = reset_control_deassert(rockchip->rst);
 	if (ret)
@@ -885,6 +893,9 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 	clk_bulk_disable_unprepare(rockchip->clk_cnt, rockchip->clks);
 deinit_phy:
 	rockchip_pcie_phy_deinit(rockchip);
+disable_regulator:
+	if (rockchip->vpcie3v3)
+		regulator_disable(rockchip->vpcie3v3);
 
 	return ret;
 }

-- 
2.53.0

[PATCH v6 03/10] PCI: dw-rockchip: Move devm_phy_get out of phy_init

[Sebastian Reichel]

By moving devm_phy_get() to the probe routine, rockchip_pcie_phy_init()
can be used to re-initialize the PCIe PHY, which is for example needed
after a system suspend/resume cycle.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index f67ad3c31c35..7472ec78b473 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -612,14 +612,8 @@ static int rockchip_pcie_resource_get(struct platform_device *pdev,
 
 static int rockchip_pcie_phy_init(struct rockchip_pcie *rockchip)
 {
-	struct device *dev = rockchip->pci.dev;
 	int ret;
 
-	rockchip->phy = devm_phy_get(dev, "pcie-phy");
-	if (IS_ERR(rockchip->phy))
-		return dev_err_probe(dev, PTR_ERR(rockchip->phy),
-				     "missing PHY\n");
-
 	ret = phy_init(rockchip->phy);
 	if (ret < 0)
 		return ret;
@@ -858,6 +852,13 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
 					     "failed to enable vpcie3v3 regulator\n");
 	}
 
+	rockchip->phy = devm_phy_get(dev, "pcie-phy");
+	if (IS_ERR(rockchip->phy)) {
+		ret = PTR_ERR(rockchip->phy);
+		dev_err_probe(dev, ret, "missing PHY\n");
+		goto disable_regulator;
+	}
+
 	ret = rockchip_pcie_phy_init(rockchip);
 	if (ret)
 		goto disable_regulator;

-- 
2.53.0

[PATCH v6 04/10] PCI: dw-rockchip: Add helper function for enhanced LTSSM control mode

[Sebastian Reichel]

Remove code duplication and improve readability by introducing a new
function to setup the enhanced LTSSM mode.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 7472ec78b473..6f84b07406d4 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -696,6 +696,18 @@ static irqreturn_t rockchip_pcie_ep_sys_irq_thread(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
+static void rockchip_pcie_enable_enhanced_ltssm_control_mode(struct rockchip_pcie *rockchip,
+							     u32 flags)
+{
+	u32 val;
+
+	/* Enable the enhanced control mode of signal app_ltssm_enable */
+	val = FIELD_PREP_WM16(PCIE_LTSSM_ENABLE_ENHANCE, 1);
+	if (flags)
+		val |= FIELD_PREP_WM16(flags, 1);
+	rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL);
+}
+
 static int rockchip_pcie_configure_rc(struct platform_device *pdev,
 				      struct rockchip_pcie *rockchip)
 {
@@ -719,9 +731,7 @@ static int rockchip_pcie_configure_rc(struct platform_device *pdev,
 		return ret;
 	}
 
-	/* LTSSM enable control mode */
-	val = FIELD_PREP_WM16(PCIE_LTSSM_ENABLE_ENHANCE, 1);
-	rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL);
+	rockchip_pcie_enable_enhanced_ltssm_control_mode(rockchip, 0);
 
 	rockchip_pcie_writel_apb(rockchip,
 				 PCIE_CLIENT_SET_MODE(PCIE_CLIENT_MODE_RC),
@@ -765,13 +775,7 @@ static int rockchip_pcie_configure_ep(struct platform_device *pdev,
 		return ret;
 	}
 
-	/*
-	 * LTSSM enable control mode, and automatically delay link training on
-	 * hot reset/link-down reset.
-	 */
-	val = FIELD_PREP_WM16(PCIE_LTSSM_ENABLE_ENHANCE, 1) |
-	      FIELD_PREP_WM16(PCIE_LTSSM_APP_DLY2_EN, 1);
-	rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL);
+	rockchip_pcie_enable_enhanced_ltssm_control_mode(rockchip, PCIE_LTSSM_APP_DLY2_EN);
 
 	rockchip_pcie_writel_apb(rockchip,
 				 PCIE_CLIENT_SET_MODE(PCIE_CLIENT_MODE_EP),

-- 
2.53.0

[PATCH v6 05/10] PCI: dw-rockchip: Add helper function for controller mode

[Sebastian Reichel]

Remove code duplication and improve readability by introducing a new
function to setup the controller mode.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 6f84b07406d4..211776bbcb99 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -708,6 +708,11 @@ static void rockchip_pcie_enable_enhanced_ltssm_control_mode(struct rockchip_pci
 	rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL);
 }
 
+static void rockchip_pcie_set_controller_mode(struct rockchip_pcie *rockchip, u32 mode)
+{
+	rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_SET_MODE(mode), PCIE_CLIENT_GENERAL_CON);
+}
+
 static int rockchip_pcie_configure_rc(struct platform_device *pdev,
 				      struct rockchip_pcie *rockchip)
 {
@@ -732,10 +737,7 @@ static int rockchip_pcie_configure_rc(struct platform_device *pdev,
 	}
 
 	rockchip_pcie_enable_enhanced_ltssm_control_mode(rockchip, 0);
-
-	rockchip_pcie_writel_apb(rockchip,
-				 PCIE_CLIENT_SET_MODE(PCIE_CLIENT_MODE_RC),
-				 PCIE_CLIENT_GENERAL_CON);
+	rockchip_pcie_set_controller_mode(rockchip, PCIE_CLIENT_MODE_RC);
 
 	pp = &rockchip->pci.pp;
 	pp->ops = &rockchip_pcie_host_ops;
@@ -776,10 +778,7 @@ static int rockchip_pcie_configure_ep(struct platform_device *pdev,
 	}
 
 	rockchip_pcie_enable_enhanced_ltssm_control_mode(rockchip, PCIE_LTSSM_APP_DLY2_EN);
-
-	rockchip_pcie_writel_apb(rockchip,
-				 PCIE_CLIENT_SET_MODE(PCIE_CLIENT_MODE_EP),
-				 PCIE_CLIENT_GENERAL_CON);
+	rockchip_pcie_set_controller_mode(rockchip, PCIE_CLIENT_MODE_EP);
 
 	rockchip->pci.ep.ops = &rockchip_pcie_ep_ops;
 	rockchip->pci.ep.page_size = SZ_64K;

-- 
2.53.0

[PATCH v6 06/10] PCI: dw-rockchip: Add helper function for DDL indicator

[Sebastian Reichel]

Remove code duplication and improve readability by introducing a new
function to setup the DLL indicator.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 211776bbcb99..1e48099e36e4 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -713,6 +713,16 @@ static void rockchip_pcie_set_controller_mode(struct rockchip_pcie *rockchip, u3
 	rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_SET_MODE(mode), PCIE_CLIENT_GENERAL_CON);
 }
 
+static void rockchip_pcie_unmask_dll_indicator(struct rockchip_pcie *rockchip)
+{
+	u32 val;
+
+	/* unmask DLL up/down indicator and hot reset/link-down reset */
+	val = FIELD_PREP_WM16(PCIE_RDLH_LINK_UP_CHGED, 0) |
+	      FIELD_PREP_WM16(PCIE_LINK_REQ_RST_NOT_INT, 0);
+	rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_INTR_MASK_MISC);
+}
+
 static int rockchip_pcie_configure_rc(struct platform_device *pdev,
 				      struct rockchip_pcie *rockchip)
 {
@@ -760,7 +770,6 @@ static int rockchip_pcie_configure_ep(struct platform_device *pdev,
 {
 	struct device *dev = &pdev->dev;
 	int irq, ret;
-	u32 val;
 
 	if (!IS_ENABLED(CONFIG_PCIE_ROCKCHIP_DW_EP))
 		return -ENODEV;
@@ -800,10 +809,7 @@ static int rockchip_pcie_configure_ep(struct platform_device *pdev,
 
 	pci_epc_init_notify(rockchip->pci.ep.epc);
 
-	/* unmask DLL up/down indicator and hot reset/link-down reset */
-	val = FIELD_PREP_WM16(PCIE_RDLH_LINK_UP_CHGED, 0) |
-	      FIELD_PREP_WM16(PCIE_LINK_REQ_RST_NOT_INT, 0);
-	rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_INTR_MASK_MISC);
+	rockchip_pcie_unmask_dll_indicator(rockchip);
 
 	return ret;
 }

-- 
2.53.0

[PATCH v6 07/10] PCI: dw-rockchip: Add pme_turn_off support

[Sebastian Reichel]

Prepare Rockchip PCIe controller for system suspend support by
adding the PME turn off operation.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 44 +++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 1e48099e36e4..17530507e41b 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -45,6 +45,7 @@
 #define  PCIE_CLIENT_LD_RQ_RST_GRT	FIELD_PREP_WM16(BIT(3), 1)
 #define  PCIE_CLIENT_ENABLE_LTSSM	FIELD_PREP_WM16(BIT(2), 1)
 #define  PCIE_CLIENT_DISABLE_LTSSM	FIELD_PREP_WM16(BIT(2), 0)
+#define PCIE_CLIENT_INTR_STATUS_MSG_RX	0x04
 
 /* Interrupt Status Register Related to Legacy Interrupt */
 #define PCIE_CLIENT_INTR_STATUS_LEGACY	0x8
@@ -64,6 +65,11 @@
 
 /* Interrupt Mask Register Related to Miscellaneous Operation */
 #define PCIE_CLIENT_INTR_MASK_MISC	0x24
+#define PCIE_CLIENT_POWER		0x2c
+#define PCIE_CLIENT_MSG_GEN		0x34
+#define PME_READY_ENTER_L23		BIT(3)
+#define PME_TURN_OFF			FIELD_PREP_WM16(BIT(4), 1)
+#define PME_TO_ACK			FIELD_PREP_WM16(BIT(9), 1)
 
 /* Power Management Control Register */
 #define PCIE_CLIENT_POWER_CON		0x2c
@@ -452,8 +458,46 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
 	return 0;
 }
 
+static void rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
+	struct device *dev = rockchip->pci.dev;
+	u32 status;
+	int ret;
+
+	/* 1. Broadcast PME_Turn_Off Message, bit 4 self-clear once done */
+	rockchip_pcie_writel_apb(rockchip, PME_TURN_OFF, PCIE_CLIENT_MSG_GEN);
+	ret = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_MSG_GEN,
+				 status, !(status & BIT(4)), PCIE_PME_TO_L2_TIMEOUT_US / 10,
+				 PCIE_PME_TO_L2_TIMEOUT_US);
+	if (ret) {
+		dev_warn(dev, "Failed to send PME_Turn_Off\n");
+		return;
+	}
+
+	/* 2. Wait for PME_TO_Ack, bit 9 will be set once received */
+	ret = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_INTR_STATUS_MSG_RX,
+				 status, status & BIT(9), PCIE_PME_TO_L2_TIMEOUT_US / 10,
+				 PCIE_PME_TO_L2_TIMEOUT_US);
+	if (ret) {
+		dev_warn(dev, "Failed to receive PME_TO_Ack\n");
+		return;
+	}
+
+	/* 3. Clear PME_TO_Ack and Wait for ready to enter L23 message */
+	rockchip_pcie_writel_apb(rockchip, PME_TO_ACK, PCIE_CLIENT_INTR_STATUS_MSG_RX);
+	ret = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_POWER,
+				 status, status & PME_READY_ENTER_L23,
+				 PCIE_PME_TO_L2_TIMEOUT_US / 10,
+				 PCIE_PME_TO_L2_TIMEOUT_US);
+	if (ret)
+		dev_err(dev, "Failed to get ready to enter L23 message\n");
+}
+
 static const struct dw_pcie_host_ops rockchip_pcie_host_ops = {
 	.init = rockchip_pcie_host_init,
+	.pme_turn_off = rockchip_pcie_pme_turn_off,
 };
 
 /*

-- 
2.53.0

[PATCH v6 08/10] PCI: dw-rockchip: Set broken L1SS resume flag

[Sebastian Reichel]

Setup the broken_l1ss_resume flag, so that pci_suspend_retains_context()
returns false on the Rockchip platform allowing PCIe peripheral device
drivers to act upon.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 17530507e41b..47d9cab909bc 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -458,6 +458,21 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
 	return 0;
 }
 
+static void rockchip_pcie_host_post_init(struct dw_pcie_rp *pp)
+{
+	/*
+	 * During system suspend, the Rockchip RC driver effectively powers off
+	 * the controller. If the link is in L1SS and the endpoint asserts
+	 * CLKREQ# to exit L1SS, the time required to wake the system and
+	 * restore the PHY/REFCLK may exceed the L1SS exit timing
+	 * (L10_REFCLK_ON + T_COMMONMODE), resulting in Link Down (LDn) and a
+	 * reset of the endpoint. Set this flag to indicate this limitation to
+	 * client drivers so that they can avoid relying on device state being
+	 * preserved during system suspend.
+	 */
+	pp->bridge->broken_l1ss_resume = true;
+}
+
 static void rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -497,6 +512,7 @@ static void rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
 
 static const struct dw_pcie_host_ops rockchip_pcie_host_ops = {
 	.init = rockchip_pcie_host_init,
+	.post_init = rockchip_pcie_host_post_init,
 	.pme_turn_off = rockchip_pcie_pme_turn_off,
 };
 

-- 
2.53.0

[PATCH v6 09/10] PCI: dw-rockchip: Add system PM support

[Sebastian Reichel]

Add system PM support for Rockchip PCIe Designware Controllers.

I've tested this on the Rockchip RK3576 EVB1, the Radxa ROCK 4D
and the ArmSom Sige5 boards.

While I haven't experienced any issues, most of my tests have been done
without any devices attached (i.e. default board without any extras), so
there _might_ still be some problems. As system suspend does not work at
all right now, I think it makes sense to get at least the basic
configurations working as soon as possible as it will allow us to catch
regressions by enabling system suspend in CI systems like KernelCI.

Co-developed-by: Shawn Lin <shawn.lin@rock-chips.com>
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 100 ++++++++++++++++++++++++++
 1 file changed, 100 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 47d9cab909bc..e675918ba184 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -120,6 +120,7 @@ struct rockchip_pcie {
 	struct gpio_desc *rst_gpio;
 	struct regulator *vpcie3v3;
 	struct irq_domain *irq_domain;
+	u32 intx;
 	const struct rockchip_pcie_of_data *data;
 	bool supports_clkreq;
 	struct delayed_work trace_work;
@@ -1042,6 +1043,99 @@ static int rockchip_pcie_rc_reset_root_port(struct pci_host_bridge *bridge,
 	return ret;
 }
 
+static int rockchip_pcie_suspend(struct device *dev)
+{
+	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
+	struct dw_pcie *pci = &rockchip->pci;
+	int ret;
+
+	if (rockchip->data->mode == DW_PCIE_EP_TYPE) {
+		dev_err(dev, "suspend is not supported in EP mode\n");
+		return -EOPNOTSUPP;
+	}
+
+	rockchip->intx = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_INTR_MASK_LEGACY);
+
+	ret = dw_pcie_suspend_noirq(pci);
+	if (ret)
+		return ret;
+
+	gpiod_set_value_cansleep(rockchip->rst_gpio, 0);
+	rockchip_pcie_phy_deinit(rockchip);
+	clk_bulk_disable_unprepare(rockchip->clk_cnt, rockchip->clks);
+	reset_control_assert(rockchip->rst);
+	if (rockchip->vpcie3v3)
+		regulator_disable(rockchip->vpcie3v3);
+
+	return 0;
+}
+
+static int rockchip_pcie_resume(struct device *dev)
+{
+	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
+	struct dw_pcie *pci = &rockchip->pci;
+	int ret;
+
+	if (rockchip->data->mode == DW_PCIE_EP_TYPE) {
+		dev_err(dev, "resume is not supported in EP mode\n");
+		return -EOPNOTSUPP;
+	}
+
+	ret = clk_bulk_prepare_enable(rockchip->clk_cnt, rockchip->clks);
+	if (ret) {
+		dev_err(dev, "clock init failed: %d\n", ret);
+		return ret;
+	}
+
+	if (rockchip->vpcie3v3) {
+		ret = regulator_enable(rockchip->vpcie3v3);
+		if (ret)
+			goto err_disable_clk;
+	}
+
+	ret = rockchip_pcie_phy_init(rockchip);
+	if (ret) {
+		dev_err(dev, "phy init failed: %d\n", ret);
+		goto err_disable_regulator;
+	}
+
+	reset_control_deassert(rockchip->rst);
+
+	rockchip_pcie_writel_apb(rockchip, FIELD_PREP_WM16(0xffff, rockchip->intx),
+				 PCIE_CLIENT_INTR_MASK_LEGACY);
+
+	rockchip_pcie_enable_enhanced_ltssm_control_mode(rockchip, 0);
+	rockchip_pcie_set_controller_mode(rockchip, PCIE_CLIENT_MODE_RC);
+	rockchip_pcie_unmask_dll_indicator(rockchip);
+
+	gpiod_set_value_cansleep(rockchip->rst_gpio, 1);
+
+	ret = dw_pcie_resume_noirq(pci);
+	if (ret) {
+		dev_err(dev, "failed to resume: %d\n", ret);
+		/*
+		 * During resume, dw_pcie_wait_for_link() is called and when
+		 * there is no device connected at all it returns -EIO with
+		 * the message "Device found, but not active". Ignore it for
+		 * now.
+		 */
+		if (ret != -EIO)
+			goto err_deinit_phy;
+	}
+
+	return 0;
+
+err_deinit_phy:
+	gpiod_set_value_cansleep(rockchip->rst_gpio, 0);
+	rockchip_pcie_phy_deinit(rockchip);
+err_disable_regulator:
+	if (rockchip->vpcie3v3)
+		regulator_disable(rockchip->vpcie3v3);
+err_disable_clk:
+	clk_bulk_disable_unprepare(rockchip->clk_cnt, rockchip->clks);
+	return ret;
+}
+
 static const struct rockchip_pcie_of_data rockchip_pcie_rc_of_data_rk3568 = {
 	.mode = DW_PCIE_RC_TYPE,
 };
@@ -1072,11 +1166,17 @@ static const struct of_device_id rockchip_pcie_of_match[] = {
 	{},
 };
 
+static const struct dev_pm_ops rockchip_pcie_pm_ops = {
+	NOIRQ_SYSTEM_SLEEP_PM_OPS(rockchip_pcie_suspend,
+				  rockchip_pcie_resume)
+};
+
 static struct platform_driver rockchip_pcie_driver = {
 	.driver = {
 		.name	= "rockchip-dw-pcie",
 		.of_match_table = rockchip_pcie_of_match,
 		.suppress_bind_attrs = true,
+		.pm = &rockchip_pcie_pm_ops,
 	},
 	.probe = rockchip_pcie_probe,
 };

-- 
2.53.0

[PATCH v6 10/10] PCI: dw-rockchip: Clear debug buffer before entering L2

[Sebastian Reichel]

Clear the debug buffer before entering the L2 state to ease debugging
link training issues on resume.

Cc: Shawn Lin <shawn.lin@rock-chips.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index e675918ba184..a508f4fffd2a 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -77,6 +77,9 @@
 #define  PCIE_CLKREQ_NOT_READY		FIELD_PREP_WM16(BIT(0), 0)
 #define  PCIE_CLKREQ_PULL_DOWN		FIELD_PREP_WM16(GENMASK(13, 12), 1)
 
+/* General Debug Register */
+#define PCIE_CLIENT_GENERAL_DEBUG	0x104
+
 /* RASDES TBA information */
 #define PCIE_CLIENT_CDM_RASDES_TBA_INFO_CMN	0x154
 #define  PCIE_CLIENT_CDM_RASDES_TBA_L1_1	BIT(4)
@@ -1043,6 +1046,11 @@ static int rockchip_pcie_rc_reset_root_port(struct pci_host_bridge *bridge,
 	return ret;
 }
 
+static inline void rockchip_pcie_link_status_clear(struct rockchip_pcie *rockchip)
+{
+	rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_GENERAL_DEBUG, 0x0);
+}
+
 static int rockchip_pcie_suspend(struct device *dev)
 {
 	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
@@ -1056,6 +1064,11 @@ static int rockchip_pcie_suspend(struct device *dev)
 
 	rockchip->intx = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_INTR_MASK_LEGACY);
 
+	/* All sub-devices are in D3hot by PCIe stack */
+	dw_pcie_dbi_ro_wr_dis(pci);
+
+	rockchip_pcie_link_status_clear(rockchip);
+
 	ret = dw_pcie_suspend_noirq(pci);
 	if (ret)
 		return ret;

-- 
2.53.0

Re: [PATCH v6 08/10] PCI: dw-rockchip: Set broken L1SS resume flag

[sashiko-bot]

> Setup the broken_l1ss_resume flag, so that pci_suspend_retains_context()
> returns false on the Rockchip platform allowing PCIe peripheral device
> drivers to act upon.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-rockchip-pcie-system-suspend-v6-0-fbabcca9921b@collabora.com?part=8

Re: [PATCH v6 01/10] PCI: dw-rockchip: Fix LTSSM set functions

[sashiko-bot]

> Before the Rockchip PCIe driver has been switched over to the
> FIELD_PREP_WM16 macro, PCIE_CLIENT_ENABLE_LTSSM and
> PCIE_CLIENT_DISABLE_LTSSM were setting bits with a mask of 0xc = 0b1100,
> which means BIT 2 and BIT 3. After the conversion it only sets bit 2,
> with bit 3 being handled by a separate define named
> PCIE_CLIENT_LD_RQ_RST_GRT. Apparently the conversion missed to make use
> of this new macros resulting in the third bit not being set.
> 
> Fixes: 30e919570581 ("PCI: dw-rockchip: Switch to FIELD_PREP_WM16 macro")
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-rockchip-pcie-system-suspend-v6-0-fbabcca9921b@collabora.com?part=1

Re: [PATCH v6 06/10] PCI: dw-rockchip: Add helper function for DDL indicator

[sashiko-bot]

> Remove code duplication and improve readability by introducing a new
> function to setup the DLL indicator.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-rockchip-pcie-system-suspend-v6-0-fbabcca9921b@collabora.com?part=6

Re: [PATCH v6 05/10] PCI: dw-rockchip: Add helper function for controller mode

[sashiko-bot]

> Remove code duplication and improve readability by introducing a new
> function to setup the controller mode.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-rockchip-pcie-system-suspend-v6-0-fbabcca9921b@collabora.com?part=5

Re: [PATCH v6 03/10] PCI: dw-rockchip: Move devm_phy_get out of phy_init

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but iterating through `bus->devices` without holding the `pci_bus_sem` read lock risks a Use-After-Free.
- [High] This problem wasn't introduced by this patch, but repeated devres allocations in rockchip_pcie_rc_reset_root_port() via rockchip_pcie_clk_init() cause a resource leak.
--

commit 2321e2492c7e546beab0727fbd3c7122dbf07abc
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

PCI: dw-rockchip: Move devm_phy_get out of phy_init

This patch moves the devm_phy_get() call from rockchip_pcie_phy_init() to
the probe routine. This allows the PCIe PHY to be re-initialized without
re-requesting the managed resource, which is necessary after operations
like system suspend/resume.

> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index f67ad3c31c35b..7472ec78b4738 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
[ ... ]
> @@ -858,6 +852,13 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  					     "failed to enable vpcie3v3 regulator\n");
>  	}
>  
> +	rockchip->phy = devm_phy_get(dev, "pcie-phy");
> +	if (IS_ERR(rockchip->phy)) {
> +		ret = PTR_ERR(rockchip->phy);
> +		dev_err_probe(dev, ret, "missing PHY\n");
> +		goto disable_regulator;
> +	}
> +

[Severity: High]
This isn't a bug introduced by this patch, but while moving devm_phy_get() 
to the probe function correctly prevents resource leaks during 
re-initialization, there is a similar leak involving devm_clk_bulk_get_all().

In rockchip_pcie_clk_init(), devm_clk_bulk_get_all() is called
unconditionally:

rockchip_pcie_clk_init() {
        ...
        ret = devm_clk_bulk_get_all(dev, &rockchip->clks);
        ...
}

This causes a leak because rockchip_pcie_rc_reset_root_port() calls
rockchip_pcie_clk_init() during a recurring reset flow:

rockchip_pcie_rc_reset_root_port() {
        ...
        ret = rockchip_pcie_clk_init(rockchip);
        ...
}

Since rockchip_pcie_rc_reset_root_port() is assigned to
pp->bridge->reset_root_port, it can be triggered dynamically via sysfs or
error recovery. Because devres resources are only freed when the device
unbinds, does every root port reset allocate a new set of clock handles
and leak the previous devres memory? Could the clock acquisition be moved
to the probe function as well, similar to what this patch does for the PHY?

[Severity: High]
This is also a pre-existing issue, but iterating through bus->devices
without holding the pci_bus_sem read lock risks a Use-After-Free.

In rockchip_pcie_rc_sys_irq_thread(), the thread processes hot reset or
link-down events and iterates over the bus:

rockchip_pcie_rc_sys_irq_thread() {
        ...
        for_each_pci_bridge(port, pp->bridge->bus) {
                if (pci_pcie_type(port) == PCI_EXP_TYPE_ROOT_PORT)
                        pci_host_handle_link_down(port);
        }
        ...
}

Because bus->devices can be dynamically modified concurrently via PCIe
hotplug or user-initiated unbinds via sysfs, if a device is hot-removed
while the thread is iterating, could it dereference an invalid list
pointer? Should this iteration be protected by down_read(&pci_bus_sem)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-rockchip-pcie-system-suspend-v6-0-fbabcca9921b@collabora.com?part=3

Re: [PATCH v6 10/10] PCI: dw-rockchip: Clear debug buffer before entering L2

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incorrect arguments passed to rockchip_pcie_writel_apb() swap register offset and value.
- [Medium] Undocumented modification of controller DBI state during suspend.
--

commit 8a75a527ec46d932d4c3b6e7ed428836e9df376b
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

PCI: dw-rockchip: Clear debug buffer before entering L2

This commit aims to clear the debug buffer before entering the L2 state to
ease debugging link training issues on resume. It introduces a helper to clear
the general debug register and invokes it during suspend.

> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index e675918ba1848..a508f4fffd2ac 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -77,6 +77,9 @@
>  #define  PCIE_CLKREQ_NOT_READY		FIELD_PREP_WM16(BIT(0), 0)
>  #define  PCIE_CLKREQ_PULL_DOWN		FIELD_PREP_WM16(GENMASK(13, 12), 1)
>  
> +/* General Debug Register */
> +#define PCIE_CLIENT_GENERAL_DEBUG	0x104
> +

[ ... ]

> @@ -1043,6 +1046,11 @@ static int rockchip_pcie_rc_reset_root_port(struct pci_host_bridge *bridge,
>  	return ret;
>  }
>  
> +static inline void rockchip_pcie_link_status_clear(struct rockchip_pcie *rockchip)
> +{
> +	rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_GENERAL_DEBUG, 0x0);

[Severity: High]
Are the arguments to rockchip_pcie_writel_apb() swapped here?

Looking at the function definition earlier in the file, the signature expects
the value before the register offset:

static void rockchip_pcie_writel_apb(struct rockchip_pcie *rockchip, u32 val,
				     u32 reg)

This call passes the register offset (PCIE_CLIENT_GENERAL_DEBUG, which is
0x104) as the data value to be written to register offset 0x0. This fails to
clear the intended debug register and might unexpectedly corrupt the controller
register at offset 0x0.

> +}
> +
>  static int rockchip_pcie_suspend(struct device *dev)
>  {
>  	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
> @@ -1056,6 +1064,11 @@ static int rockchip_pcie_suspend(struct device *dev)
>  
>  	rockchip->intx = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_INTR_MASK_LEGACY);
>  
> +	/* All sub-devices are in D3hot by PCIe stack */
> +	dw_pcie_dbi_ro_wr_dis(pci);

[Severity: Medium]
Is this modification of the DBI state intentional for this commit?

This introduces an undocumented change to disable the controller's DBI 
read-only writes unconditionally during the suspend sequence. Since there is no
explanation in the commit message and no matching enable call, could this be an
accidental inclusion unrelated to clearing the debug buffer?

> +
> +	rockchip_pcie_link_status_clear(rockchip);
> +
>  	ret = dw_pcie_suspend_noirq(pci);
>  	if (ret)
>  		return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-rockchip-pcie-system-suspend-v6-0-fbabcca9921b@collabora.com?part=10

Re: [PATCH v6 02/10] PCI: dw-rockchip: Restore vpcie3v3 regulator handle

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

New issues:
- [Medium] Silent failure when phy_init() or phy_power_on() fails due to dropped dev_err_probe().

Pre-existing issues:
- [High] Repeated devres memory and clock reference leaks during root port resets.
- [High] Failure to create IRQ domain causes potential arbitrary interrupt execution or NULL pointer dereference.
- [High] IRQ handler is active while clocks are disabled on probe error paths, causing a potential crash if the IRQ fires.
- [High] The chained legacy IRQ handler is not cleared on probe failure, leading to a permanent use-after-free and crash.
--

commit f70d8a03f10e09b0300fff37e8aa493a89895114
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

PCI: dw-rockchip: Restore vpcie3v3 regulator handle

This commit reverts a previous cleanup that removed the explicit vpcie3v3 
regulator handle. It restores storing the regulator in the rockchip struct 
and explicitly disables it in the probe error path. This change is needed 
to support disabling the regulator during system suspend in a future patch.

> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index dc85564e50013..f67ad3c31c35b 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
[ ... ]
> @@ -844,15 +845,22 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
[ ... ]
>  	ret = rockchip_pcie_phy_init(rockchip);
>  	if (ret)
> -		return dev_err_probe(dev, ret,
> -				     "failed to initialize the phy\n");
> +		goto disable_regulator;

[Severity: Medium]
Is it possible that removing this dev_err_probe() call causes failures in
phy_init() or phy_power_on() to happen silently? This seems like it could
make device initialization issues significantly harder to debug when they
occur.

>  
>  	ret = reset_control_deassert(rockchip->rst);
[ ... ]
> @@ -885,6 +893,9 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  	clk_bulk_disable_unprepare(rockchip->clk_cnt, rockchip->clks);

[Severity: High]
This is a pre-existing issue, but does manually disabling the clocks here on
the error path leave a window where the devm-managed IRQ handler (requested
via devm_request_threaded_irq() earlier in probe) is still active?

If the IRQ fires before devres cleans it up after the function returns, could
the handler attempt to read APB registers with clocks gated, causing a bus
fault and kernel panic?

>  deinit_phy:
>  	rockchip_pcie_phy_deinit(rockchip);
> +disable_regulator:
> +	if (rockchip->vpcie3v3)
> +		regulator_disable(rockchip->vpcie3v3);
>  
>  	return ret;
>  }

While reviewing this patch, I also noticed a few pre-existing issues in the
same file:

> @@ -570,9 +570,9 @@ static int rockchip_pcie_clk_init(struct rockchip_pcie *rockchip)
>  {
>  	struct device *dev = rockchip->pci.dev;
>  	int ret;
>  
>  	ret = devm_clk_bulk_get_all(dev, &rockchip->clks);
>  	if (ret < 0)
>  		return dev_err_probe(dev, ret, "failed to get clocks\n");

[Severity: High]
This isn't a bug introduced by this patch, but does calling
devm_clk_bulk_get_all() during root port resets cause a persistent memory
and clock reference leak?

Since rockchip_pcie_clk_init() is called by rockchip_pcie_rc_reset_root_port():

drivers/pci/controller/dwc/pcie-dw-rockchip.c:rockchip_pcie_rc_reset_root_port() {
	...
	ret = rockchip_pcie_clk_init(rockchip);
	if (ret)
		goto deinit_phy;
	...
}

Will repeated root port resets, such as those triggered by hardware
link-down events or userspace sysfs requests, continually allocate devres
structures and leak clock subsystem references, since they are only freed on
unbind?

> @@ -437,6 +437,6 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
>  {
[ ... ]
>  	ret = rockchip_pcie_init_irq_domain(rockchip);
>  	if (ret < 0)
>  		dev_err(dev, "failed to init irq domain\n");
>  
>  	irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
>  					 rockchip);

[Severity: High]
This is a pre-existing issue, but does ignoring the error code from
rockchip_pcie_init_irq_domain() allow rockchip_pcie_intx_handler to be
registered with a NULL irq_domain?

If the legacy interrupt triggers when rockchip->irq_domain is NULL:

drivers/pci/controller/dwc/pcie-dw-rockchip.c:rockchip_pcie_intx_handler() {
	...
	for_each_set_bit(hwirq, &reg, 4)
		generic_handle_domain_irq(rockchip->irq_domain, hwirq);
	...
}

Could generic_handle_domain_irq() fall back to irq_default_domain and
potentially execute completely unrelated interrupt handlers belonging to
other devices if the default domain resolves the hwirq (0-3)?

[Severity: High]
Additionally, this is a pre-existing issue, but is the chained legacy IRQ
handler correctly cleared on probe failure?

If initialization fails later in the probe path, the rockchip structure will
be freed by devres. Because irq_set_chained_handler_and_data() is used
without a corresponding devres cleanup or manual cleanup in the error path,
could a subsequent legacy IRQ trigger a use-after-free when
rockchip_pcie_intx_handler() dereferences the freed rockchip structure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-rockchip-pcie-system-suspend-v6-0-fbabcca9921b@collabora.com?part=2

Re: [PATCH v6 07/10] PCI: dw-rockchip: Add pme_turn_off support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Incorrect use of FIELD_PREP_WM16() on a Write-1-to-Clear (W1C) interrupt status register, causing unintended clearing of bit 25.
- [Low] Redundant macro definition for the Power Management Control Register.
--

commit 10a270e398b67f78bf174b063ed14cff0ce4e43b
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

    PCI: dw-rockchip: Add pme_turn_off support

This commit prepares the Rockchip PCIe controller for system suspend support
by adding the PME turn off operation.

> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
[ ... ]
> @@ -64,6 +65,11 @@
>  
>  /* Interrupt Mask Register Related to Miscellaneous Operation */
>  #define PCIE_CLIENT_INTR_MASK_MISC	0x24
> +#define PCIE_CLIENT_POWER		0x2c

[Severity: Low]
Does this code introduce a redundant macro definition? It appears that
PCIE_CLIENT_POWER_CON is already defined as 0x2c immediately below
this addition.

> +#define PCIE_CLIENT_MSG_GEN		0x34
> +#define PME_READY_ENTER_L23		BIT(3)
> +#define PME_TURN_OFF			FIELD_PREP_WM16(BIT(4), 1)
> +#define PME_TO_ACK			FIELD_PREP_WM16(BIT(9), 1)
>  
>  /* Power Management Control Register */
>  #define PCIE_CLIENT_POWER_CON		0x2c
[ ... ]
> @@ -452,8 +458,46 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
>  	return 0;
>  }
>  
> +static void rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
> +{
[ ... ]
> +	/* 3. Clear PME_TO_Ack and Wait for ready to enter L23 message */
> +	rockchip_pcie_writel_apb(rockchip, PME_TO_ACK, PCIE_CLIENT_INTR_STATUS_MSG_RX);

[Severity: Medium]
Since PME_TO_ACK uses FIELD_PREP_WM16(), it sets the write mask in the
upper 16 bits (asserting bit 25). If PCIE_CLIENT_INTR_STATUS_MSG_RX is
a Write-1-to-Clear interrupt status register, will writing this WM16
formatted value unintentionally clear the interrupt status for bit 25?
If bit 25 corresponds to an active interrupt, it could be silently lost.

> +	ret = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_POWER,
> +				 status, status & PME_READY_ENTER_L23,
> +				 PCIE_PME_TO_L2_TIMEOUT_US / 10,
> +				 PCIE_PME_TO_L2_TIMEOUT_US);
> +	if (ret)
> +		dev_err(dev, "Failed to get ready to enter L23 message\n");
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-rockchip-pcie-system-suspend-v6-0-fbabcca9921b@collabora.com?part=7

Re: [PATCH v6 09/10] PCI: dw-rockchip: Add system PM support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] System resume unconditionally re-initializes the host, leaking the existing IRQ domain and breaking legacy INTx interrupts while executing invalid GFP_KERNEL allocations in an atomic context.
- [High] The PM operations `rockchip_pcie_suspend` and `rockchip_pcie_resume` are registered as `noirq` callbacks but call functions that can sleep.
- [High] Missing check for the return value of reset_control_deassert() in rockchip_pcie_resume().
- [Medium] Missing `reset_control_assert` in the error handling path of `rockchip_pcie_resume`.
- [Medium] The order of initializing and tearing down the regulator, PHY, and clocks in system PM callbacks contradicts the proven sequence used during driver probe.
--

commit 8a8b70dde70e09797d6c8a3b9234fe5f8265aec7
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

PCI: dw-rockchip: Add system PM support

Add system PM support for Rockchip PCIe Designware Controllers.

> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 47d9cab909bce..e675918ba1848 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c

[ ... ]

> +static int rockchip_pcie_suspend(struct device *dev)
> +{

[ ... ]

> +	ret = dw_pcie_suspend_noirq(pci);
> +	if (ret)
> +		return ret;
> +
> +	gpiod_set_value_cansleep(rockchip->rst_gpio, 0);

[Severity: High]
Since rockchip_pcie_suspend() is registered via NOIRQ_SYSTEM_SLEEP_PM_OPS(),
it executes as a noirq callback where local interrupts are disabled.

Is it safe to call sleepable functions in this atomic context? Functions like
gpiod_set_value_cansleep() and clk_bulk_disable_unprepare() (which takes a
mutex) might trigger might_sleep splats or deadlocks.

> +	rockchip_pcie_phy_deinit(rockchip);
> +	clk_bulk_disable_unprepare(rockchip->clk_cnt, rockchip->clks);
> +	reset_control_assert(rockchip->rst);
> +	if (rockchip->vpcie3v3)
> +		regulator_disable(rockchip->vpcie3v3);
> +
> +	return 0;
> +}
> +
> +static int rockchip_pcie_resume(struct device *dev)
> +{

[ ... ]

> +	ret = clk_bulk_prepare_enable(rockchip->clk_cnt, rockchip->clks);

[Severity: Medium]
Does this sequence invert the hardware initialization dependencies used during
driver probe?

During probe, the driver enables the regulator, initializes the PHY, deasserts
reset, and then enables the clocks. Here in rockchip_pcie_resume(), the clocks
are enabled first, followed by the regulator and PHY. Could this inverted
order cause link-training bugs or PHY lockups?

> +	if (ret) {
> +		dev_err(dev, "clock init failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	if (rockchip->vpcie3v3) {
> +		ret = regulator_enable(rockchip->vpcie3v3);
> +		if (ret)
> +			goto err_disable_clk;
> +	}
> +
> +	ret = rockchip_pcie_phy_init(rockchip);
> +	if (ret) {
> +		dev_err(dev, "phy init failed: %d\n", ret);
> +		goto err_disable_regulator;
> +	}
> +
> +	reset_control_deassert(rockchip->rst);

[Severity: High]
Does this code handle reset_control_deassert() failures?

If reset_control_deassert() fails, the driver immediately proceeds to write to
the IP block's APB registers. If the hardware reset was not actually released,
accessing these registers could trigger a synchronous external abort and kernel
panic.

> +
> +	rockchip_pcie_writel_apb(rockchip, FIELD_PREP_WM16(0xffff, rockchip->intx),
> +				 PCIE_CLIENT_INTR_MASK_LEGACY);
> +
> +	rockchip_pcie_enable_enhanced_ltssm_control_mode(rockchip, 0);
> +	rockchip_pcie_set_controller_mode(rockchip, PCIE_CLIENT_MODE_RC);
> +	rockchip_pcie_unmask_dll_indicator(rockchip);
> +
> +	gpiod_set_value_cansleep(rockchip->rst_gpio, 1);
> +
> +	ret = dw_pcie_resume_noirq(pci);

[Severity: High]
Does system resume unconditionally re-initialize the host, leaking the existing
IRQ domain and breaking legacy INTx interrupts?

When dw_pcie_resume_noirq() is called, it invokes the host initialization
callback rockchip_pcie_host_init(), which then calls
rockchip_pcie_init_irq_domain():

rockchip_pcie_host_init()
  rockchip_pcie_init_irq_domain()
    rockchip->irq_domain = irq_domain_create_linear(of_fwnode_handle(intc),
                                                    PCI_NUM_INTX,
                                                    &intx_domain_ops,
                                                    rockchip);

This executes an invalid GFP_KERNEL allocation inside the atomic noirq phase,
overwrites the existing irq_domain pointer (leaking the old domain), and drops
existing INTx mappings.

> +	if (ret) {
> +		dev_err(dev, "failed to resume: %d\n", ret);

[ ... ]

> +err_deinit_phy:
> +	gpiod_set_value_cansleep(rockchip->rst_gpio, 0);
> +	rockchip_pcie_phy_deinit(rockchip);
> +err_disable_regulator:

[Severity: Medium]
Does this error path leave the hardware controller in an inconsistent,
permanently deasserted state?

If dw_pcie_resume_noirq() fails, the error handling block tears down the PHY,
clocks, and regulator, but it seems to miss calling
reset_control_assert(rockchip->rst).

> +	if (rockchip->vpcie3v3)
> +		regulator_disable(rockchip->vpcie3v3);
> +err_disable_clk:
> +	clk_bulk_disable_unprepare(rockchip->clk_cnt, rockchip->clks);
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-rockchip-pcie-system-suspend-v6-0-fbabcca9921b@collabora.com?part=9

Re: [PATCH v6 04/10] PCI: dw-rockchip: Add helper function for enhanced LTSSM control mode

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Variable parameter passed to FIELD_PREP_WM16() causes build failures and contains a latent logic bug.

Pre-existing issues:
- [Low] Incomplete refactoring of enhanced LTSSM control mode setup.
--

commit 754de1ab627f784507a4b699acc006187c6a91fb
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

PCI: dw-rockchip: Add helper function for enhanced LTSSM control mode

Remove code duplication and improve readability by introducing a new
function to setup the enhanced LTSSM mode.

> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 7472ec78b4738..6f84b07406d49 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -696,6 +696,18 @@ static irqreturn_t rockchip_pcie_ep_sys_irq_thread(int irq, void *arg)
>  	return IRQ_HANDLED;
>  }
>  
> +static void rockchip_pcie_enable_enhanced_ltssm_control_mode(struct rockchip_pcie *rockchip,
> +							     u32 flags)
> +{
> +	u32 val;
> +
> +	/* Enable the enhanced control mode of signal app_ltssm_enable */
> +	val = FIELD_PREP_WM16(PCIE_LTSSM_ENABLE_ENHANCE, 1);
> +	if (flags)
> +		val |= FIELD_PREP_WM16(flags, 1);

[Severity: Medium]
Does passing the variable flags to FIELD_PREP_WM16() violate the macro's
requirement for a compile-time constant mask?

If rockchip_pcie_enable_enhanced_ltssm_control_mode() is outlined, will this
trigger a BUILD_BUG_ON_MSG(!__builtin_constant_p) build failure?

Also, if a future caller passes a multi-bit mask to flags (e.g. BIT(1) |
BIT(3)), would the macro evaluate to (1 << 1) & mask, setting only BIT(1)
and writing 0 to BIT(3), despite enabling both in the upper 16 bits?

> +	rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL);
> +}

[Severity: Low]
This is a pre-existing issue, but since the goal of this helper is to remove
code duplication for setting up enhanced LTSSM mode, should the exact same
duplicate sequence in rockchip_pcie_rc_reset_root_port() also be replaced
with rockchip_pcie_enable_enhanced_ltssm_control_mode(rockchip, 0)?

drivers/pci/controller/dwc/pcie-dw-rockchip.c:rockchip_pcie_rc_reset_root_port() {
    ...
    /* LTSSM enable control mode */
    val = FIELD_PREP_WM16(PCIE_LTSSM_ENABLE_ENHANCE, 1);
    rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL);
    ...
}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-rockchip-pcie-system-suspend-v6-0-fbabcca9921b@collabora.com?part=4

Re: [PATCH v6 01/10] PCI: dw-rockchip: Fix LTSSM set functions

[Shawn Lin]

在 2026/09/08 星期二 22:51, Sebastian Reichel 写道:
> Before the Rockchip PCIe driver has been switched over to the
> FIELD_PREP_WM16 macro, PCIE_CLIENT_ENABLE_LTSSM and
> PCIE_CLIENT_DISABLE_LTSSM were setting bits with a mask of 0xc = 0b1100,
> which means BIT 2 and BIT 3. After the conversion it only sets bit 2,
> with bit 3 being handled by a separate define named
> PCIE_CLIENT_LD_RQ_RST_GRT. Apparently the conversion missed to make use
> of this new macros resulting in the third bit not being set.
> 

Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

> Fixes: 30e919570581 ("PCI: dw-rockchip: Switch to FIELD_PREP_WM16 macro")
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/pci/controller/dwc/pcie-dw-rockchip.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index af26a07c0c9e..dc85564e5001 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -321,14 +321,16 @@ static void rockchip_pcie_ltssm_trace(struct rockchip_pcie *rockchip,
>   
>   static void rockchip_pcie_enable_ltssm(struct rockchip_pcie *rockchip)
>   {
> -	rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_ENABLE_LTSSM,
> -				 PCIE_CLIENT_GENERAL_CON);
> +	u32 val = PCIE_CLIENT_ENABLE_LTSSM | PCIE_CLIENT_LD_RQ_RST_GRT;
> +
> +	rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_GENERAL_CON);
>   }
>   
>   static void rockchip_pcie_disable_ltssm(struct rockchip_pcie *rockchip)
>   {
> -	rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_DISABLE_LTSSM,
> -				 PCIE_CLIENT_GENERAL_CON);
> +	u32 val = PCIE_CLIENT_DISABLE_LTSSM | PCIE_CLIENT_LD_RQ_RST_GRT;
> +
> +	rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_GENERAL_CON);
>   }
>   
>   static bool rockchip_pcie_link_up(struct dw_pcie *pci)
> 

Re: [PATCH v6 02/10] PCI: dw-rockchip: Restore vpcie3v3 regulator handle

[Shawn Lin]

在 2026/09/08 星期二 22:51, Sebastian Reichel 写道:
> This reverts commit c930b10f17c0 ("PCI: dw-rockchip: Simplify regulator
> setup with devm_regulator_get_enable_optional()"), which nicely cleaned
> up the code.
> 
> The vpcie3v3 regulator handle is needed to disable the regulator during
> system suspend (to be added in its own patch).
> 

Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/pci/controller/dwc/pcie-dw-rockchip.c | 23 +++++++++++++++++------
>   1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index dc85564e5001..f67ad3c31c35 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -112,6 +112,7 @@ struct rockchip_pcie {
>   	unsigned int clk_cnt;
>   	struct reset_control *rst;
>   	struct gpio_desc *rst_gpio;
> +	struct regulator *vpcie3v3;
>   	struct irq_domain *irq_domain;
>   	const struct rockchip_pcie_of_data *data;
>   	bool supports_clkreq;
> @@ -844,15 +845,22 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>   		return ret;
>   
>   	/* DON'T MOVE ME: must be enable before PHY init */
> -	ret = devm_regulator_get_enable_optional(dev, "vpcie3v3");
> -	if (ret < 0 && ret != -ENODEV)
> -		return dev_err_probe(dev, ret,
> -				     "failed to enable vpcie3v3 regulator\n");
> +	rockchip->vpcie3v3 = devm_regulator_get_optional(dev, "vpcie3v3");
> +	if (IS_ERR(rockchip->vpcie3v3)) {
> +		if (PTR_ERR(rockchip->vpcie3v3) != -ENODEV)
> +			return dev_err_probe(dev, PTR_ERR(rockchip->vpcie3v3),
> +					"failed to get vpcie3v3 regulator\n");
> +		rockchip->vpcie3v3 = NULL;
> +	} else {
> +		ret = regulator_enable(rockchip->vpcie3v3);
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "failed to enable vpcie3v3 regulator\n");
> +	}
>   
>   	ret = rockchip_pcie_phy_init(rockchip);
>   	if (ret)
> -		return dev_err_probe(dev, ret,
> -				     "failed to initialize the phy\n");
> +		goto disable_regulator;
>   
>   	ret = reset_control_deassert(rockchip->rst);
>   	if (ret)
> @@ -885,6 +893,9 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>   	clk_bulk_disable_unprepare(rockchip->clk_cnt, rockchip->clks);
>   deinit_phy:
>   	rockchip_pcie_phy_deinit(rockchip);
> +disable_regulator:
> +	if (rockchip->vpcie3v3)
> +		regulator_disable(rockchip->vpcie3v3);
>   
>   	return ret;
>   }
> 

Re: [PATCH v6 03/10] PCI: dw-rockchip: Move devm_phy_get out of phy_init

[Shawn Lin]

在 2026/09/08 星期二 22:51, Sebastian Reichel 写道:
> By moving devm_phy_get() to the probe routine, rockchip_pcie_phy_init()
> can be used to re-initialize the PCIe PHY, which is for example needed
> after a system suspend/resume cycle.
> 

Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/pci/controller/dwc/pcie-dw-rockchip.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index f67ad3c31c35..7472ec78b473 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -612,14 +612,8 @@ static int rockchip_pcie_resource_get(struct platform_device *pdev,
>   
>   static int rockchip_pcie_phy_init(struct rockchip_pcie *rockchip)
>   {
> -	struct device *dev = rockchip->pci.dev;
>   	int ret;
>   
> -	rockchip->phy = devm_phy_get(dev, "pcie-phy");
> -	if (IS_ERR(rockchip->phy))
> -		return dev_err_probe(dev, PTR_ERR(rockchip->phy),
> -				     "missing PHY\n");
> -
>   	ret = phy_init(rockchip->phy);
>   	if (ret < 0)
>   		return ret;
> @@ -858,6 +852,13 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>   					     "failed to enable vpcie3v3 regulator\n");
>   	}
>   
> +	rockchip->phy = devm_phy_get(dev, "pcie-phy");
> +	if (IS_ERR(rockchip->phy)) {
> +		ret = PTR_ERR(rockchip->phy);
> +		dev_err_probe(dev, ret, "missing PHY\n");
> +		goto disable_regulator;
> +	}
> +
>   	ret = rockchip_pcie_phy_init(rockchip);
>   	if (ret)
>   		goto disable_regulator;
> 

Re: [PATCH v6 04/10] PCI: dw-rockchip: Add helper function for enhanced LTSSM control mode

[Shawn Lin]

在 2026/09/08 星期二 22:51, Sebastian Reichel 写道:
> Remove code duplication and improve readability by introducing a new
> function to setup the enhanced LTSSM mode.
> 

Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/pci/controller/dwc/pcie-dw-rockchip.c | 24 ++++++++++++++----------
>   1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 7472ec78b473..6f84b07406d4 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -696,6 +696,18 @@ static irqreturn_t rockchip_pcie_ep_sys_irq_thread(int irq, void *arg)
>   	return IRQ_HANDLED;
>   }
>   
> +static void rockchip_pcie_enable_enhanced_ltssm_control_mode(struct rockchip_pcie *rockchip,
> +							     u32 flags)
> +{
> +	u32 val;
> +
> +	/* Enable the enhanced control mode of signal app_ltssm_enable */
> +	val = FIELD_PREP_WM16(PCIE_LTSSM_ENABLE_ENHANCE, 1);
> +	if (flags)
> +		val |= FIELD_PREP_WM16(flags, 1);
> +	rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL);
> +}
> +
>   static int rockchip_pcie_configure_rc(struct platform_device *pdev,
>   				      struct rockchip_pcie *rockchip)
>   {
> @@ -719,9 +731,7 @@ static int rockchip_pcie_configure_rc(struct platform_device *pdev,
>   		return ret;
>   	}
>   
> -	/* LTSSM enable control mode */
> -	val = FIELD_PREP_WM16(PCIE_LTSSM_ENABLE_ENHANCE, 1);
> -	rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL);
> +	rockchip_pcie_enable_enhanced_ltssm_control_mode(rockchip, 0);
>   
>   	rockchip_pcie_writel_apb(rockchip,
>   				 PCIE_CLIENT_SET_MODE(PCIE_CLIENT_MODE_RC),
> @@ -765,13 +775,7 @@ static int rockchip_pcie_configure_ep(struct platform_device *pdev,
>   		return ret;
>   	}
>   
> -	/*
> -	 * LTSSM enable control mode, and automatically delay link training on
> -	 * hot reset/link-down reset.
> -	 */
> -	val = FIELD_PREP_WM16(PCIE_LTSSM_ENABLE_ENHANCE, 1) |
> -	      FIELD_PREP_WM16(PCIE_LTSSM_APP_DLY2_EN, 1);
> -	rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL);
> +	rockchip_pcie_enable_enhanced_ltssm_control_mode(rockchip, PCIE_LTSSM_APP_DLY2_EN);
>   
>   	rockchip_pcie_writel_apb(rockchip,
>   				 PCIE_CLIENT_SET_MODE(PCIE_CLIENT_MODE_EP),
> 

Re: [PATCH v6 05/10] PCI: dw-rockchip: Add helper function for controller mode

[Shawn Lin]

在 2026/09/08 星期二 22:51, Sebastian Reichel 写道:
> Remove code duplication and improve readability by introducing a new
> function to setup the controller mode.
> 

Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/pci/controller/dwc/pcie-dw-rockchip.c | 15 +++++++--------
>   1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 6f84b07406d4..211776bbcb99 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -708,6 +708,11 @@ static void rockchip_pcie_enable_enhanced_ltssm_control_mode(struct rockchip_pci
>   	rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL);
>   }
>   
> +static void rockchip_pcie_set_controller_mode(struct rockchip_pcie *rockchip, u32 mode)
> +{
> +	rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_SET_MODE(mode), PCIE_CLIENT_GENERAL_CON);
> +}
> +
>   static int rockchip_pcie_configure_rc(struct platform_device *pdev,
>   				      struct rockchip_pcie *rockchip)
>   {
> @@ -732,10 +737,7 @@ static int rockchip_pcie_configure_rc(struct platform_device *pdev,
>   	}
>   
>   	rockchip_pcie_enable_enhanced_ltssm_control_mode(rockchip, 0);
> -
> -	rockchip_pcie_writel_apb(rockchip,
> -				 PCIE_CLIENT_SET_MODE(PCIE_CLIENT_MODE_RC),
> -				 PCIE_CLIENT_GENERAL_CON);
> +	rockchip_pcie_set_controller_mode(rockchip, PCIE_CLIENT_MODE_RC);
>   
>   	pp = &rockchip->pci.pp;
>   	pp->ops = &rockchip_pcie_host_ops;
> @@ -776,10 +778,7 @@ static int rockchip_pcie_configure_ep(struct platform_device *pdev,
>   	}
>   
>   	rockchip_pcie_enable_enhanced_ltssm_control_mode(rockchip, PCIE_LTSSM_APP_DLY2_EN);
> -
> -	rockchip_pcie_writel_apb(rockchip,
> -				 PCIE_CLIENT_SET_MODE(PCIE_CLIENT_MODE_EP),
> -				 PCIE_CLIENT_GENERAL_CON);
> +	rockchip_pcie_set_controller_mode(rockchip, PCIE_CLIENT_MODE_EP);
>   
>   	rockchip->pci.ep.ops = &rockchip_pcie_ep_ops;
>   	rockchip->pci.ep.page_size = SZ_64K;
> 

Re: [PATCH v6 06/10] PCI: dw-rockchip: Add helper function for DDL indicator

[Shawn Lin]

在 2026/09/08 星期二 22:51, Sebastian Reichel 写道:
> Remove code duplication and improve readability by introducing a new
> function to setup the DLL indicator.
> 

Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/pci/controller/dwc/pcie-dw-rockchip.c | 16 +++++++++++-----
>   1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 211776bbcb99..1e48099e36e4 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -713,6 +713,16 @@ static void rockchip_pcie_set_controller_mode(struct rockchip_pcie *rockchip, u3
>   	rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_SET_MODE(mode), PCIE_CLIENT_GENERAL_CON);
>   }
>   
> +static void rockchip_pcie_unmask_dll_indicator(struct rockchip_pcie *rockchip)
> +{
> +	u32 val;
> +
> +	/* unmask DLL up/down indicator and hot reset/link-down reset */
> +	val = FIELD_PREP_WM16(PCIE_RDLH_LINK_UP_CHGED, 0) |
> +	      FIELD_PREP_WM16(PCIE_LINK_REQ_RST_NOT_INT, 0);
> +	rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_INTR_MASK_MISC);
> +}
> +
>   static int rockchip_pcie_configure_rc(struct platform_device *pdev,
>   				      struct rockchip_pcie *rockchip)
>   {
> @@ -760,7 +770,6 @@ static int rockchip_pcie_configure_ep(struct platform_device *pdev,
>   {
>   	struct device *dev = &pdev->dev;
>   	int irq, ret;
> -	u32 val;
>   
>   	if (!IS_ENABLED(CONFIG_PCIE_ROCKCHIP_DW_EP))
>   		return -ENODEV;
> @@ -800,10 +809,7 @@ static int rockchip_pcie_configure_ep(struct platform_device *pdev,
>   
>   	pci_epc_init_notify(rockchip->pci.ep.epc);
>   
> -	/* unmask DLL up/down indicator and hot reset/link-down reset */
> -	val = FIELD_PREP_WM16(PCIE_RDLH_LINK_UP_CHGED, 0) |
> -	      FIELD_PREP_WM16(PCIE_LINK_REQ_RST_NOT_INT, 0);
> -	rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_INTR_MASK_MISC);
> +	rockchip_pcie_unmask_dll_indicator(rockchip);
>   
>   	return ret;
>   }
> 

Re: [PATCH v6 07/10] PCI: dw-rockchip: Add pme_turn_off support

[Shawn Lin]

在 2026/09/08 星期二 22:51, Sebastian Reichel 写道:
> Prepare Rockchip PCIe controller for system suspend support by
> adding the PME turn off operation.
> 

Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/pci/controller/dwc/pcie-dw-rockchip.c | 44 +++++++++++++++++++++++++++
>   1 file changed, 44 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 1e48099e36e4..17530507e41b 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -45,6 +45,7 @@
>   #define  PCIE_CLIENT_LD_RQ_RST_GRT	FIELD_PREP_WM16(BIT(3), 1)
>   #define  PCIE_CLIENT_ENABLE_LTSSM	FIELD_PREP_WM16(BIT(2), 1)
>   #define  PCIE_CLIENT_DISABLE_LTSSM	FIELD_PREP_WM16(BIT(2), 0)
> +#define PCIE_CLIENT_INTR_STATUS_MSG_RX	0x04
>   
>   /* Interrupt Status Register Related to Legacy Interrupt */
>   #define PCIE_CLIENT_INTR_STATUS_LEGACY	0x8
> @@ -64,6 +65,11 @@
>   
>   /* Interrupt Mask Register Related to Miscellaneous Operation */
>   #define PCIE_CLIENT_INTR_MASK_MISC	0x24
> +#define PCIE_CLIENT_POWER		0x2c
> +#define PCIE_CLIENT_MSG_GEN		0x34
> +#define PME_READY_ENTER_L23		BIT(3)
> +#define PME_TURN_OFF			FIELD_PREP_WM16(BIT(4), 1)
> +#define PME_TO_ACK			FIELD_PREP_WM16(BIT(9), 1)
>   
>   /* Power Management Control Register */
>   #define PCIE_CLIENT_POWER_CON		0x2c
> @@ -452,8 +458,46 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
>   	return 0;
>   }
>   
> +static void rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
> +{
> +	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> +	struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
> +	struct device *dev = rockchip->pci.dev;
> +	u32 status;
> +	int ret;
> +
> +	/* 1. Broadcast PME_Turn_Off Message, bit 4 self-clear once done */
> +	rockchip_pcie_writel_apb(rockchip, PME_TURN_OFF, PCIE_CLIENT_MSG_GEN);
> +	ret = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_MSG_GEN,
> +				 status, !(status & BIT(4)), PCIE_PME_TO_L2_TIMEOUT_US / 10,
> +				 PCIE_PME_TO_L2_TIMEOUT_US);
> +	if (ret) {
> +		dev_warn(dev, "Failed to send PME_Turn_Off\n");
> +		return;
> +	}
> +
> +	/* 2. Wait for PME_TO_Ack, bit 9 will be set once received */
> +	ret = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_INTR_STATUS_MSG_RX,
> +				 status, status & BIT(9), PCIE_PME_TO_L2_TIMEOUT_US / 10,
> +				 PCIE_PME_TO_L2_TIMEOUT_US);
> +	if (ret) {
> +		dev_warn(dev, "Failed to receive PME_TO_Ack\n");
> +		return;
> +	}
> +
> +	/* 3. Clear PME_TO_Ack and Wait for ready to enter L23 message */
> +	rockchip_pcie_writel_apb(rockchip, PME_TO_ACK, PCIE_CLIENT_INTR_STATUS_MSG_RX);
> +	ret = readl_poll_timeout(rockchip->apb_base + PCIE_CLIENT_POWER,
> +				 status, status & PME_READY_ENTER_L23,
> +				 PCIE_PME_TO_L2_TIMEOUT_US / 10,
> +				 PCIE_PME_TO_L2_TIMEOUT_US);
> +	if (ret)
> +		dev_err(dev, "Failed to get ready to enter L23 message\n");
> +}
> +
>   static const struct dw_pcie_host_ops rockchip_pcie_host_ops = {
>   	.init = rockchip_pcie_host_init,
> +	.pme_turn_off = rockchip_pcie_pme_turn_off,
>   };
>   
>   /*
> 

Re: [PATCH v6 08/10] PCI: dw-rockchip: Set broken L1SS resume flag

[Shawn Lin]

在 2026/09/08 星期二 22:51, Sebastian Reichel 写道:
> Setup the broken_l1ss_resume flag, so that pci_suspend_retains_context()
> returns false on the Rockchip platform allowing PCIe peripheral device
> drivers to act upon.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/pci/controller/dwc/pcie-dw-rockchip.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 17530507e41b..47d9cab909bc 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -458,6 +458,21 @@ static int rockchip_pcie_host_init(struct dw_pcie_rp *pp)
>   	return 0;
>   }
>   
> +static void rockchip_pcie_host_post_init(struct dw_pcie_rp *pp)
> +{
> +	/*
> +	 * During system suspend, the Rockchip RC driver effectively powers off
> +	 * the controller. If the link is in L1SS and the endpoint asserts
> +	 * CLKREQ# to exit L1SS, the time required to wake the system and
> +	 * restore the PHY/REFCLK may exceed the L1SS exit timing
> +	 * (L10_REFCLK_ON + T_COMMONMODE), resulting in Link Down (LDn) and a
> +	 * reset of the endpoint. Set this flag to indicate this limitation to
> +	 * client drivers so that they can avoid relying on device state being
> +	 * preserved during system suspend.
> +	 */

Yes, this is ture for Rockchip platforms that powering off the
controller as well as PHY is a default action during system suspend.

Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

> +	pp->bridge->broken_l1ss_resume = true;
> +}
> +
>   static void rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
>   {
>   	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -497,6 +512,7 @@ static void rockchip_pcie_pme_turn_off(struct dw_pcie_rp *pp)
>   
>   static const struct dw_pcie_host_ops rockchip_pcie_host_ops = {
>   	.init = rockchip_pcie_host_init,
> +	.post_init = rockchip_pcie_host_post_init,
>   	.pme_turn_off = rockchip_pcie_pme_turn_off,
>   };
>   
> 

Re: [PATCH v6 10/10] PCI: dw-rockchip: Clear debug buffer before entering L2

[Shawn Lin]

在 2026/09/08 星期二 22:51, Sebastian Reichel 写道:
> Clear the debug buffer before entering the L2 state to ease debugging
> link training issues on resume.
> 

Reviewed-by: Shawn Lin <shawn.lin@rock-chips.com>

> Cc: Shawn Lin <shawn.lin@rock-chips.com>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/pci/controller/dwc/pcie-dw-rockchip.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index e675918ba184..a508f4fffd2a 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -77,6 +77,9 @@
>   #define  PCIE_CLKREQ_NOT_READY		FIELD_PREP_WM16(BIT(0), 0)
>   #define  PCIE_CLKREQ_PULL_DOWN		FIELD_PREP_WM16(GENMASK(13, 12), 1)
>   
> +/* General Debug Register */
> +#define PCIE_CLIENT_GENERAL_DEBUG	0x104
> +
>   /* RASDES TBA information */
>   #define PCIE_CLIENT_CDM_RASDES_TBA_INFO_CMN	0x154
>   #define  PCIE_CLIENT_CDM_RASDES_TBA_L1_1	BIT(4)
> @@ -1043,6 +1046,11 @@ static int rockchip_pcie_rc_reset_root_port(struct pci_host_bridge *bridge,
>   	return ret;
>   }
>   
> +static inline void rockchip_pcie_link_status_clear(struct rockchip_pcie *rockchip)
> +{
> +	rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_GENERAL_DEBUG, 0x0);
> +}
> +
>   static int rockchip_pcie_suspend(struct device *dev)
>   {
>   	struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
> @@ -1056,6 +1064,11 @@ static int rockchip_pcie_suspend(struct device *dev)
>   
>   	rockchip->intx = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_INTR_MASK_LEGACY);
>   
> +	/* All sub-devices are in D3hot by PCIe stack */
> +	dw_pcie_dbi_ro_wr_dis(pci);
> +
> +	rockchip_pcie_link_status_clear(rockchip);
> +
>   	ret = dw_pcie_suspend_noirq(pci);
>   	if (ret)
>   		return ret;
>