* [PATCH v4 1/9] PCI: dw-rockchip: Rename rockchip_pcie_get_ltssm function
2025-10-29 17:56 [PATCH v4 0/9] PCI: dw-rockchip: add system suspend support Sebastian Reichel
@ 2025-10-29 17:56 ` Sebastian Reichel
2025-10-29 23:07 ` Bjorn Helgaas
2025-10-29 17:56 ` [PATCH v4 2/9] PCI: dw-rockchip: Support get_ltssm operation Sebastian Reichel
` (9 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Sebastian Reichel @ 2025-10-29 17:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin
Cc: linux-pci, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
Sebastian Reichel
Rename rockchip_pcie_get_ltssm to rockchip_pcie_get_ltssm_status_reg
to avoid confusion after introducing the .get_ltssm operation support,
which requires further processing of the register.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 3e2752c7dd09..58427db1cc65 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -175,7 +175,7 @@ static int rockchip_pcie_init_irq_domain(struct rockchip_pcie *rockchip)
return 0;
}
-static u32 rockchip_pcie_get_ltssm(struct rockchip_pcie *rockchip)
+static u32 rockchip_pcie_get_ltssm_status_reg(struct rockchip_pcie *rockchip)
{
return rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_LTSSM_STATUS);
}
@@ -195,7 +195,7 @@ static void rockchip_pcie_disable_ltssm(struct rockchip_pcie *rockchip)
static bool rockchip_pcie_link_up(struct dw_pcie *pci)
{
struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
- u32 val = rockchip_pcie_get_ltssm(rockchip);
+ u32 val = rockchip_pcie_get_ltssm_status_reg(rockchip);
return FIELD_GET(PCIE_LINKUP_MASK, val) == PCIE_LINKUP;
}
@@ -460,7 +460,7 @@ static irqreturn_t rockchip_pcie_rc_sys_irq_thread(int irq, void *arg)
rockchip_pcie_writel_apb(rockchip, reg, PCIE_CLIENT_INTR_STATUS_MISC);
dev_dbg(dev, "PCIE_CLIENT_INTR_STATUS_MISC: %#x\n", reg);
- dev_dbg(dev, "LTSSM_STATUS: %#x\n", rockchip_pcie_get_ltssm(rockchip));
+ dev_dbg(dev, "LTSSM_STATUS: %#x\n", rockchip_pcie_get_ltssm_status_reg(rockchip));
if (reg & PCIE_RDLH_LINK_UP_CHGED) {
if (rockchip_pcie_link_up(pci)) {
@@ -487,7 +487,7 @@ static irqreturn_t rockchip_pcie_ep_sys_irq_thread(int irq, void *arg)
rockchip_pcie_writel_apb(rockchip, reg, PCIE_CLIENT_INTR_STATUS_MISC);
dev_dbg(dev, "PCIE_CLIENT_INTR_STATUS_MISC: %#x\n", reg);
- dev_dbg(dev, "LTSSM_STATUS: %#x\n", rockchip_pcie_get_ltssm(rockchip));
+ dev_dbg(dev, "LTSSM_STATUS: %#x\n", rockchip_pcie_get_ltssm_status_reg(rockchip));
if (reg & PCIE_LINK_REQ_RST_NOT_INT) {
dev_dbg(dev, "hot reset or link-down reset\n");
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 1/9] PCI: dw-rockchip: Rename rockchip_pcie_get_ltssm function
2025-10-29 17:56 ` [PATCH v4 1/9] PCI: dw-rockchip: Rename rockchip_pcie_get_ltssm function Sebastian Reichel
@ 2025-10-29 23:07 ` Bjorn Helgaas
0 siblings, 0 replies; 22+ messages in thread
From: Bjorn Helgaas @ 2025-10-29 23:07 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin, linux-pci, linux-arm-kernel,
linux-rockchip, linux-kernel, kernel
On Wed, Oct 29, 2025 at 06:56:40PM +0100, Sebastian Reichel wrote:
> Rename rockchip_pcie_get_ltssm to rockchip_pcie_get_ltssm_status_reg
> to avoid confusion after introducing the .get_ltssm operation support,
> which requires further processing of the register.
If you repost for other reasons, please add "()" after function names,
including .get_ltssm(). Then you can drop "function" from the
subject.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 2/9] PCI: dw-rockchip: Support get_ltssm operation
2025-10-29 17:56 [PATCH v4 0/9] PCI: dw-rockchip: add system suspend support Sebastian Reichel
2025-10-29 17:56 ` [PATCH v4 1/9] PCI: dw-rockchip: Rename rockchip_pcie_get_ltssm function Sebastian Reichel
@ 2025-10-29 17:56 ` Sebastian Reichel
2025-10-29 17:56 ` [PATCH v4 3/9] PCI: dw-rockchip: Move devm_phy_get out of phy_init Sebastian Reichel
` (8 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Sebastian Reichel @ 2025-10-29 17:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin
Cc: linux-pci, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
Sebastian Reichel
Implement .get_ltssm operation function pointer, which will be needed
for system suspend support. As the driver used to have a function
called rockchip_pcie_get_ltssm() with different behavior the new
function is named rockchip_pcie_get_ltssm_state(), so that issues
can easily be detected when porting patches.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 58427db1cc65..e3d7792f7819 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -180,6 +180,14 @@ static u32 rockchip_pcie_get_ltssm_status_reg(struct rockchip_pcie *rockchip)
return rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_LTSSM_STATUS);
}
+static u32 rockchip_pcie_get_ltssm_state(struct dw_pcie *pci)
+{
+ struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
+ u32 val = rockchip_pcie_get_ltssm_status_reg(rockchip);
+
+ return FIELD_GET(PCIE_LTSSM_STATUS_MASK, val);
+}
+
static void rockchip_pcie_enable_ltssm(struct rockchip_pcie *rockchip)
{
rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_ENABLE_LTSSM,
@@ -446,6 +454,7 @@ static const struct dw_pcie_ops dw_pcie_ops = {
.link_up = rockchip_pcie_link_up,
.start_link = rockchip_pcie_start_link,
.stop_link = rockchip_pcie_stop_link,
+ .get_ltssm = rockchip_pcie_get_ltssm_state,
};
static irqreturn_t rockchip_pcie_rc_sys_irq_thread(int irq, void *arg)
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v4 3/9] PCI: dw-rockchip: Move devm_phy_get out of phy_init
2025-10-29 17:56 [PATCH v4 0/9] PCI: dw-rockchip: add system suspend support Sebastian Reichel
2025-10-29 17:56 ` [PATCH v4 1/9] PCI: dw-rockchip: Rename rockchip_pcie_get_ltssm function Sebastian Reichel
2025-10-29 17:56 ` [PATCH v4 2/9] PCI: dw-rockchip: Support get_ltssm operation Sebastian Reichel
@ 2025-10-29 17:56 ` Sebastian Reichel
2025-10-29 17:56 ` [PATCH v4 4/9] PCI: dw-rockchip: Add helper function for enhanced LTSSM control mode Sebastian Reichel
` (7 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Sebastian Reichel @ 2025-10-29 17:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin
Cc: linux-pci, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
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 e3d7792f7819..8e584016e244 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -425,14 +425,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;
@@ -674,6 +668,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.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v4 4/9] PCI: dw-rockchip: Add helper function for enhanced LTSSM control mode
2025-10-29 17:56 [PATCH v4 0/9] PCI: dw-rockchip: add system suspend support Sebastian Reichel
` (2 preceding siblings ...)
2025-10-29 17:56 ` [PATCH v4 3/9] PCI: dw-rockchip: Move devm_phy_get out of phy_init Sebastian Reichel
@ 2025-10-29 17:56 ` Sebastian Reichel
2025-10-29 23:11 ` Bjorn Helgaas
2025-10-29 17:56 ` [PATCH v4 5/9] PCI: dw-rockchip: Add helper function for controller mode Sebastian Reichel
` (6 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Sebastian Reichel @ 2025-10-29 17:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin
Cc: linux-pci, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
Sebastian Reichel
Remove code duplocation 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 | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 8e584016e244..45586a964ead 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -511,13 +511,24 @@ 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)
{
struct device *dev = &pdev->dev;
struct dw_pcie_rp *pp;
int irq, ret;
- u32 val;
if (!IS_ENABLED(CONFIG_PCIE_ROCKCHIP_DW_HOST))
return -ENODEV;
@@ -534,10 +545,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),
PCIE_CLIENT_GENERAL_CON);
@@ -581,14 +589,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),
PCIE_CLIENT_GENERAL_CON);
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 4/9] PCI: dw-rockchip: Add helper function for enhanced LTSSM control mode
2025-10-29 17:56 ` [PATCH v4 4/9] PCI: dw-rockchip: Add helper function for enhanced LTSSM control mode Sebastian Reichel
@ 2025-10-29 23:11 ` Bjorn Helgaas
0 siblings, 0 replies; 22+ messages in thread
From: Bjorn Helgaas @ 2025-10-29 23:11 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin, linux-pci, linux-arm-kernel,
linux-rockchip, linux-kernel, kernel
On Wed, Oct 29, 2025 at 06:56:43PM +0100, Sebastian Reichel wrote:
> Remove code duplocation and improve readability by introducing a new
> function to setup the enhanced LTSSM mode.
s/duplocation/duplication/
s/setup/set up/
Maybe also include the actual function name instead of just "a new
function". "rockchip_pcie_enable_enhanced_ltssm_control_mode()" is
pretty long; I wouldn't object to a shorter version :)
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 5/9] PCI: dw-rockchip: Add helper function for controller mode
2025-10-29 17:56 [PATCH v4 0/9] PCI: dw-rockchip: add system suspend support Sebastian Reichel
` (3 preceding siblings ...)
2025-10-29 17:56 ` [PATCH v4 4/9] PCI: dw-rockchip: Add helper function for enhanced LTSSM control mode Sebastian Reichel
@ 2025-10-29 17:56 ` Sebastian Reichel
2025-10-29 17:56 ` [PATCH v4 6/9] PCI: dw-rockchip: Add helper function for DDL indicator Sebastian Reichel
` (5 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Sebastian Reichel @ 2025-10-29 17:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin
Cc: linux-pci, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
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 | 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 45586a964ead..5c8d30e15a44 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -523,6 +523,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)
{
@@ -546,9 +551,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;
@@ -590,9 +593,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.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v4 6/9] PCI: dw-rockchip: Add helper function for DDL indicator
2025-10-29 17:56 [PATCH v4 0/9] PCI: dw-rockchip: add system suspend support Sebastian Reichel
` (4 preceding siblings ...)
2025-10-29 17:56 ` [PATCH v4 5/9] PCI: dw-rockchip: Add helper function for controller mode Sebastian Reichel
@ 2025-10-29 17:56 ` Sebastian Reichel
2025-10-29 17:56 ` [PATCH v4 7/9] PCI: dw-rockchip: Add pme_turn_off support Sebastian Reichel
` (4 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Sebastian Reichel @ 2025-10-29 17:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin
Cc: linux-pci, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
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 | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 5c8d30e15a44..ad4a907c991f 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -528,6 +528,15 @@ 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 */
+ val = FIELD_PREP_WM16(PCIE_RDLH_LINK_UP_CHGED, 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)
{
@@ -563,9 +572,7 @@ static int rockchip_pcie_configure_rc(struct platform_device *pdev,
return ret;
}
- /* unmask DLL up/down indicator */
- val = FIELD_PREP_WM16(PCIE_RDLH_LINK_UP_CHGED, 0);
- rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_INTR_MASK_MISC);
+ rockchip_pcie_unmask_dll_indicator(rockchip);
return ret;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v4 7/9] PCI: dw-rockchip: Add pme_turn_off support
2025-10-29 17:56 [PATCH v4 0/9] PCI: dw-rockchip: add system suspend support Sebastian Reichel
` (5 preceding siblings ...)
2025-10-29 17:56 ` [PATCH v4 6/9] PCI: dw-rockchip: Add helper function for DDL indicator Sebastian Reichel
@ 2025-10-29 17:56 ` Sebastian Reichel
2025-10-29 18:36 ` Frank Li
2025-10-29 17:56 ` [PATCH v4 8/9] PCI: dw-rockchip: Add system PM support Sebastian Reichel
` (3 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Sebastian Reichel @ 2025-10-29 17:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin
Cc: linux-pci, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
Sebastian Reichel
Prepare Rockchip PCIe controller for system suspend support by
adding the PME turn off operation.
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 | 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 ad4a907c991f..d887513a63d6 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -42,6 +42,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
@@ -61,6 +62,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)
/* Hot Reset Control Register */
#define PCIE_CLIENT_HOT_RESET_CTRL 0x180
@@ -277,8 +283,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.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 7/9] PCI: dw-rockchip: Add pme_turn_off support
2025-10-29 17:56 ` [PATCH v4 7/9] PCI: dw-rockchip: Add pme_turn_off support Sebastian Reichel
@ 2025-10-29 18:36 ` Frank Li
0 siblings, 0 replies; 22+ messages in thread
From: Frank Li @ 2025-10-29 18:36 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin, linux-pci, linux-arm-kernel,
linux-rockchip, linux-kernel, kernel
On Wed, Oct 29, 2025 at 06:56:46PM +0100, Sebastian Reichel wrote:
> Prepare Rockchip PCIe controller for system suspend support by
> adding the PME turn off operation.
>
> 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 | 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 ad4a907c991f..d887513a63d6 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -42,6 +42,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
> @@ -61,6 +62,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)
>
> /* Hot Reset Control Register */
> #define PCIE_CLIENT_HOT_RESET_CTRL 0x180
> @@ -277,8 +283,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,
Does common dw_pcie_pme_turn_off() work at your platform? which use iATU to
generate PME message?
I know some old chip don't support it, but I think rockchip's PCIe should
new enough.
Frank
> };
>
> /*
>
> --
> 2.51.0
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 8/9] PCI: dw-rockchip: Add system PM support
2025-10-29 17:56 [PATCH v4 0/9] PCI: dw-rockchip: add system suspend support Sebastian Reichel
` (6 preceding siblings ...)
2025-10-29 17:56 ` [PATCH v4 7/9] PCI: dw-rockchip: Add pme_turn_off support Sebastian Reichel
@ 2025-10-29 17:56 ` Sebastian Reichel
2025-10-29 17:56 ` [PATCH v4 9/9] PCI: dwc: support missing PCIe device on resume Sebastian Reichel
` (2 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Sebastian Reichel @ 2025-10-29 17:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin
Cc: linux-pci, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
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 | 93 +++++++++++++++++++++++++++
1 file changed, 93 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index d887513a63d6..cc917bb69c85 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -90,6 +90,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;
};
@@ -770,6 +771,92 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
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);
+ 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,
};
@@ -800,11 +887,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.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v4 9/9] PCI: dwc: support missing PCIe device on resume
2025-10-29 17:56 [PATCH v4 0/9] PCI: dw-rockchip: add system suspend support Sebastian Reichel
` (7 preceding siblings ...)
2025-10-29 17:56 ` [PATCH v4 8/9] PCI: dw-rockchip: Add system PM support Sebastian Reichel
@ 2025-10-29 17:56 ` Sebastian Reichel
2025-10-29 23:17 ` Bjorn Helgaas
2025-10-30 5:37 ` Krishna Chaitanya Chundru
2025-11-01 11:19 ` [PATCH v4 0/9] PCI: dw-rockchip: add system suspend support Anand Moon
2025-11-01 13:59 ` Manivannan Sadhasivam
10 siblings, 2 replies; 22+ messages in thread
From: Sebastian Reichel @ 2025-10-29 17:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin
Cc: linux-pci, linux-arm-kernel, linux-rockchip, linux-kernel, kernel,
Sebastian Reichel
When dw_pcie_resume_noirq() is called for a PCIe root complex for a PCIe
slot with no device plugged on Rockchip RK3576, dw_pcie_wait_for_link()
will return -ETIMEDOUT. During probe time this does not happen, since
the platform sets 'use_linkup_irq'.
This adds the same logic from dw_pcie_host_init() to the PM resume
function to avoid the problem.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/pci/controller/dwc/pcie-designware-host.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index e92513c5bda5..f25f1c136900 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -1215,9 +1215,16 @@ int dw_pcie_resume_noirq(struct dw_pcie *pci)
if (ret)
return ret;
- ret = dw_pcie_wait_for_link(pci);
- if (ret)
- return ret;
+ /*
+ * Note: Skip the link up delay only when a Link Up IRQ is present.
+ * If there is no Link Up IRQ, we should not bypass the delay
+ * because that would require users to manually rescan for devices.
+ */
+ if (!pci->pp.use_linkup_irq) {
+ ret = dw_pcie_wait_for_link(pci);
+ if (ret)
+ return ret;
+ }
return ret;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v4 9/9] PCI: dwc: support missing PCIe device on resume
2025-10-29 17:56 ` [PATCH v4 9/9] PCI: dwc: support missing PCIe device on resume Sebastian Reichel
@ 2025-10-29 23:17 ` Bjorn Helgaas
2025-10-30 5:37 ` Krishna Chaitanya Chundru
1 sibling, 0 replies; 22+ messages in thread
From: Bjorn Helgaas @ 2025-10-29 23:17 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin, linux-pci, linux-arm-kernel,
linux-rockchip, linux-kernel, kernel
On Wed, Oct 29, 2025 at 06:56:48PM +0100, Sebastian Reichel wrote:
> When dw_pcie_resume_noirq() is called for a PCIe root complex for a PCIe
> slot with no device plugged on Rockchip RK3576, dw_pcie_wait_for_link()
> will return -ETIMEDOUT. During probe time this does not happen, since
> the platform sets 'use_linkup_irq'.
>
> This adds the same logic from dw_pcie_host_init() to the PM resume
> function to avoid the problem.
s/PCI: dwc: support/PCI: dwc: Support/ (in subject; capitalize first word)
s/This adds/Add/
Can you mention 8d3bf19f1b58 ("PCI: dwc: Don't wait for link up if
driver can detect Link Up event") here? I think that's what added the
similar probe-time code.
I see you did copy the comment from 8d3bf19f1b58, thanks for that!
Maybe also word your subject and commit log along the same lines so
the two commits are easier to connect to each other.
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
> drivers/pci/controller/dwc/pcie-designware-host.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index e92513c5bda5..f25f1c136900 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -1215,9 +1215,16 @@ int dw_pcie_resume_noirq(struct dw_pcie *pci)
> if (ret)
> return ret;
>
> - ret = dw_pcie_wait_for_link(pci);
> - if (ret)
> - return ret;
> + /*
> + * Note: Skip the link up delay only when a Link Up IRQ is present.
> + * If there is no Link Up IRQ, we should not bypass the delay
> + * because that would require users to manually rescan for devices.
> + */
> + if (!pci->pp.use_linkup_irq) {
> + ret = dw_pcie_wait_for_link(pci);
> + if (ret)
> + return ret;
> + }
>
> return ret;
> }
>
> --
> 2.51.0
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v4 9/9] PCI: dwc: support missing PCIe device on resume
2025-10-29 17:56 ` [PATCH v4 9/9] PCI: dwc: support missing PCIe device on resume Sebastian Reichel
2025-10-29 23:17 ` Bjorn Helgaas
@ 2025-10-30 5:37 ` Krishna Chaitanya Chundru
2025-11-01 14:20 ` Manivannan Sadhasivam
1 sibling, 1 reply; 22+ messages in thread
From: Krishna Chaitanya Chundru @ 2025-10-30 5:37 UTC (permalink / raw)
To: Sebastian Reichel, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin
Cc: linux-pci, linux-arm-kernel, linux-rockchip, linux-kernel, kernel
On 10/29/2025 11:26 PM, Sebastian Reichel wrote:
> When dw_pcie_resume_noirq() is called for a PCIe root complex for a PCIe
> slot with no device plugged on Rockchip RK3576, dw_pcie_wait_for_link()
> will return -ETIMEDOUT. During probe time this does not happen, since
> the platform sets 'use_linkup_irq'.
>
> This adds the same logic from dw_pcie_host_init() to the PM resume
> function to avoid the problem.
>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
> drivers/pci/controller/dwc/pcie-designware-host.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index e92513c5bda5..f25f1c136900 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -1215,9 +1215,16 @@ int dw_pcie_resume_noirq(struct dw_pcie *pci)
> if (ret)
> return ret;
>
> - ret = dw_pcie_wait_for_link(pci);
> - if (ret)
> - return ret;
> + /*
> + * Note: Skip the link up delay only when a Link Up IRQ is present.
> + * If there is no Link Up IRQ, we should not bypass the delay
> + * because that would require users to manually rescan for devices.
> + */
In the resume scenario, we should explicitly wait for the link to be up,
there is no IRQ support at this resume phase
and secondly after controller resume pm framework will start resuming
the bridges & endpoints. what happens
if the link is not up by the time endpoint is resume is called. And
entire save & restore states might also gets messed up.
There will be no way to recover from this.
- Krishna Chaitanya.
> + if (!pci->pp.use_linkup_irq) {
> + ret = dw_pcie_wait_for_link(pci);
> + if (ret)
> + return ret;
> + }
>
> return ret;
> }
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v4 9/9] PCI: dwc: support missing PCIe device on resume
2025-10-30 5:37 ` Krishna Chaitanya Chundru
@ 2025-11-01 14:20 ` Manivannan Sadhasivam
2025-11-03 19:00 ` Sebastian Reichel
0 siblings, 1 reply; 22+ messages in thread
From: Manivannan Sadhasivam @ 2025-11-01 14:20 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: Sebastian Reichel, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, Heiko Stuebner, Philipp Zabel,
Jingoo Han, Shawn Lin, linux-pci, linux-arm-kernel,
linux-rockchip, linux-kernel, kernel
On Thu, Oct 30, 2025 at 11:07:19AM +0530, Krishna Chaitanya Chundru wrote:
>
> On 10/29/2025 11:26 PM, Sebastian Reichel wrote:
> > When dw_pcie_resume_noirq() is called for a PCIe root complex for a PCIe
> > slot with no device plugged on Rockchip RK3576, dw_pcie_wait_for_link()
> > will return -ETIMEDOUT. During probe time this does not happen, since
> > the platform sets 'use_linkup_irq'.
> >
> > This adds the same logic from dw_pcie_host_init() to the PM resume
> > function to avoid the problem.
> >
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > ---
> > drivers/pci/controller/dwc/pcie-designware-host.c | 13 ++++++++++---
> > 1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > index e92513c5bda5..f25f1c136900 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > @@ -1215,9 +1215,16 @@ int dw_pcie_resume_noirq(struct dw_pcie *pci)
> > if (ret)
> > return ret;
> > - ret = dw_pcie_wait_for_link(pci);
> > - if (ret)
> > - return ret;
> > + /*
> > + * Note: Skip the link up delay only when a Link Up IRQ is present.
> > + * If there is no Link Up IRQ, we should not bypass the delay
> > + * because that would require users to manually rescan for devices.
> > + */
>
> In the resume scenario, we should explicitly wait for the link to be up,
> there is no IRQ support at this resume phase
This could be fixed if the PM handlers are moved to non-no_irq ones.
> and secondly after controller resume pm framework will start resuming the
> bridges & endpoints. what happens
> if the link is not up by the time endpoint is resume is called. And entire
> save & restore states might also gets messed up.
> There will be no way to recover from this.
>
This is true if there were any devices connected to the bus during suspend. If
there were no devices, then it is fine to skip waiting for the link to be up.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 9/9] PCI: dwc: support missing PCIe device on resume
2025-11-01 14:20 ` Manivannan Sadhasivam
@ 2025-11-03 19:00 ` Sebastian Reichel
0 siblings, 0 replies; 22+ messages in thread
From: Sebastian Reichel @ 2025-11-03 19:00 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Krishna Chaitanya Chundru, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Heiko Stuebner, Philipp Zabel, Jingoo Han, Shawn Lin, linux-pci,
linux-arm-kernel, linux-rockchip, linux-kernel, kernel
[-- Attachment #1: Type: text/plain, Size: 2503 bytes --]
Hi,
On Sat, Nov 01, 2025 at 07:50:58PM +0530, Manivannan Sadhasivam wrote:
> On Thu, Oct 30, 2025 at 11:07:19AM +0530, Krishna Chaitanya Chundru wrote:
> >
> > On 10/29/2025 11:26 PM, Sebastian Reichel wrote:
> > > When dw_pcie_resume_noirq() is called for a PCIe root complex for a PCIe
> > > slot with no device plugged on Rockchip RK3576, dw_pcie_wait_for_link()
> > > will return -ETIMEDOUT. During probe time this does not happen, since
> > > the platform sets 'use_linkup_irq'.
> > >
> > > This adds the same logic from dw_pcie_host_init() to the PM resume
> > > function to avoid the problem.
> > >
> > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> > > ---
> > > drivers/pci/controller/dwc/pcie-designware-host.c | 13 ++++++++++---
> > > 1 file changed, 10 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > index e92513c5bda5..f25f1c136900 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > @@ -1215,9 +1215,16 @@ int dw_pcie_resume_noirq(struct dw_pcie *pci)
> > > if (ret)
> > > return ret;
> > > - ret = dw_pcie_wait_for_link(pci);
> > > - if (ret)
> > > - return ret;
> > > + /*
> > > + * Note: Skip the link up delay only when a Link Up IRQ is present.
> > > + * If there is no Link Up IRQ, we should not bypass the delay
> > > + * because that would require users to manually rescan for devices.
> > > + */
> >
> > In the resume scenario, we should explicitly wait for the link to be up,
> > there is no IRQ support at this resume phase
>
> This could be fixed if the PM handlers are moved to non-no_irq ones.
>
> > and secondly after controller resume pm framework will start resuming the
> > bridges & endpoints. what happens
> > if the link is not up by the time endpoint is resume is called. And entire
> > save & restore states might also gets messed up.
> > There will be no way to recover from this.
> >
>
> This is true if there were any devices connected to the bus during suspend. If
> there were no devices, then it is fine to skip waiting for the link to be up.
I thought about setting a flag in the suspend routine, that a device
is expected to be there at resume time. But I suppose it might also
have been removed during the system suspend?
Greetings,
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 0/9] PCI: dw-rockchip: add system suspend support
2025-10-29 17:56 [PATCH v4 0/9] PCI: dw-rockchip: add system suspend support Sebastian Reichel
` (8 preceding siblings ...)
2025-10-29 17:56 ` [PATCH v4 9/9] PCI: dwc: support missing PCIe device on resume Sebastian Reichel
@ 2025-11-01 11:19 ` Anand Moon
2025-11-01 13:59 ` Manivannan Sadhasivam
10 siblings, 0 replies; 22+ messages in thread
From: Anand Moon @ 2025-11-01 11:19 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin, linux-pci, linux-arm-kernel,
linux-rockchip, linux-kernel, kernel
Hi Sebastian,
On Wed, 29 Oct 2025 at 23:27, Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:
>
> I've recently been working on fixing up at least basic system suspend
> support on the Rockchip RK3576 platform. Currently the biggest 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.
>
> In opposite to Shawn Lin I did not test with different peripherals as my
> main goal is getting basic suspend to ram working in the first place. I
> did notice issues with the Broadcom WLAN card on the RK3576 EVB.
> Suspending that platform without a driver being probed works, but after
> probing brcmfmac suspend is aborted because brcmf_pcie_pm_enter_D3()
> does not work. As far as I can tell the problem is unrelated to the
> Rockchip PCIe driver.
>
Well, I gave it a try on Radxa Rock 5b,
I am observing the falling warning.
PM: noirq suspend of devices failed
alarm@rockpi-5b:~$ sudo systemctl suspend
[sudo] password for alarm:
alarm@rockpi-5b:~$ [ 459.301536][ T6149] wlan0: deauthenticating from
78:d2:94:85:bb:b2 by local choice (Reason: 3=DEAUTH_LEAVING)
[ 459.383823][ T6056] r8169 0004:41:00.0 enP4p65s0: Link is Down
[ 459.384568][ T6056] r8169 0004:41:00.0: disabling bus mastering
[ 459.862229][ T6291] PM: suspend entry (deep)
[ 459.935040][ T6291] Filesystems sync: 0.072 seconds
[ 459.941444][ T6253] (NULL device *): loading
/lib/firmware/6.18.0-rc3-3-ARM64-GCC/intel/ibt-12-16.sfi failed with
error -20
[ 459.943775][ T6253] (NULL device *): loading
/lib/firmware/6.18.0-rc3-3-ARM64-GCC/intel/ibt-12-16.ddc failed with
error -20
[ 459.945047][ T59] (NULL device *): loading
/lib/firmware/6.18.0-rc3-3-ARM64-GCC/arm/mali/arch10.8/mali_csffw.bin
failed with error -20
[ 459.946854][ T6253] (NULL device *): loading
/lib/firmware/6.18.0-rc3-3-ARM64-GCC/iwlwifi-8265-36.ucode failed with
error -20
[ 459.953052][ T6291] Freezing user space processes
[ 460.001306][ T6291] Freezing user space processes completed
(elapsed 0.047 seconds)
[ 460.001993][ T6291] OOM killer disabled.
[ 460.002341][ T6291] Freezing remaining freezable tasks
[ 460.004202][ T6291] Freezing remaining freezable tasks completed
(elapsed 0.001 seconds)
[ 460.004953][ T6291] printk: Suspending console(s) (use
no_console_suspend to debug)
[ 460.070242][ T6253] nvme 0000:01:00.0: save config 0x00: 0xa808144d
[ 460.070261][ T6253] nvme 0000:01:00.0: save config 0x04: 0x00100406
[ 460.070272][ T6253] nvme 0000:01:00.0: save config 0x08: 0x01080200
[ 460.070281][ T6253] nvme 0000:01:00.0: save config 0x0c: 0x00000000
[ 460.070291][ T6253] nvme 0000:01:00.0: save config 0x10: 0xf0200004
[ 460.070300][ T6253] nvme 0000:01:00.0: save config 0x14: 0x00000000
[ 460.070309][ T6253] nvme 0000:01:00.0: save config 0x18: 0x00000000
[ 460.070319][ T6253] nvme 0000:01:00.0: save config 0x1c: 0x00000000
[ 460.070328][ T6253] nvme 0000:01:00.0: save config 0x20: 0x00000000
[ 460.070337][ T6253] nvme 0000:01:00.0: save config 0x24: 0x00000000
[ 460.070346][ T6253] nvme 0000:01:00.0: save config 0x28: 0x00000000
[ 460.070355][ T6253] nvme 0000:01:00.0: save config 0x2c: 0xa801144d
[ 460.070364][ T6253] nvme 0000:01:00.0: save config 0x30: 0x00000000
[ 460.070373][ T6253] nvme 0000:01:00.0: save config 0x34: 0x00000040
[ 460.070383][ T6253] nvme 0000:01:00.0: save config 0x38: 0x00000000
[ 460.070392][ T6253] nvme 0000:01:00.0: save config 0x3c: 0x0000016d
[ 460.274027][ T6253] iwlwifi 0002:21:00.0: save config 0x00: 0x24fd8086
[ 460.274058][ T6253] iwlwifi 0002:21:00.0: save config 0x04: 0x00100406
[ 460.274080][ T6253] iwlwifi 0002:21:00.0: save config 0x08: 0x02800078
[ 460.274102][ T6253] iwlwifi 0002:21:00.0: save config 0x0c: 0x00000000
[ 460.274124][ T6253] iwlwifi 0002:21:00.0: save config 0x10: 0xf2200004
[ 460.274146][ T6253] iwlwifi 0002:21:00.0: save config 0x14: 0x00000000
[ 460.274168][ T6253] iwlwifi 0002:21:00.0: save config 0x18: 0x00000000
[ 460.274189][ T6253] iwlwifi 0002:21:00.0: save config 0x1c: 0x00000000
[ 460.274210][ T6253] iwlwifi 0002:21:00.0: save config 0x20: 0x00000000
[ 460.274232][ T6253] iwlwifi 0002:21:00.0: save config 0x24: 0x00000000
[ 460.274253][ T6253] iwlwifi 0002:21:00.0: save config 0x28: 0x00000000
[ 460.274275][ T6253] iwlwifi 0002:21:00.0: save config 0x2c: 0x10108086
[ 460.274296][ T6253] iwlwifi 0002:21:00.0: save config 0x30: 0x00000000
[ 460.274318][ T6253] iwlwifi 0002:21:00.0: save config 0x34: 0x000000c8
[ 460.274340][ T6253] iwlwifi 0002:21:00.0: save config 0x38: 0x00000000
[ 460.274361][ T6253] iwlwifi 0002:21:00.0: save config 0x3c: 0x00000183
[ 460.274917][ T6300] r8169 0004:41:00.0: save config 0x00: 0x812510ec
[ 460.274927][ T6300] r8169 0004:41:00.0: save config 0x04: 0x00100403
[ 460.274937][ T6300] r8169 0004:41:00.0: save config 0x08: 0x02000005
[ 460.274947][ T6300] r8169 0004:41:00.0: save config 0x0c: 0x00000010
[ 460.274957][ T6300] r8169 0004:41:00.0: save config 0x10: 0xf4100001
[ 460.274967][ T6300] r8169 0004:41:00.0: save config 0x14: 0x00000000
[ 460.274977][ T6300] r8169 0004:41:00.0: save config 0x18: 0xf4200004
[ 460.274987][ T6300] r8169 0004:41:00.0: save config 0x1c: 0x00000000
[ 460.274996][ T6300] r8169 0004:41:00.0: save config 0x20: 0xf4210004
[ 460.275006][ T6300] r8169 0004:41:00.0: save config 0x24: 0x00000000
[ 460.275016][ T6300] r8169 0004:41:00.0: save config 0x28: 0x00000000
[ 460.275026][ T6300] r8169 0004:41:00.0: save config 0x2c: 0x012310ec
[ 460.275036][ T6300] r8169 0004:41:00.0: save config 0x30: 0x00000000
[ 460.275045][ T6300] r8169 0004:41:00.0: save config 0x34: 0x00000040
[ 460.275055][ T6300] r8169 0004:41:00.0: save config 0x38: 0x00000000
[ 460.275065][ T6300] r8169 0004:41:00.0: save config 0x3c: 0x00000197
[ 460.285117][ T6253] iwlwifi 0002:21:00.0: PCI PM: Suspend power state: D3hot
[ 460.287498][ T6300] r8169 0004:41:00.0: PCI PM: Suspend power state: D3hot
[ 460.287631][ T6296] pcieport 0004:40:00.0: save config 0x00: 0x35881d87
[ 460.287644][ T6296] pcieport 0004:40:00.0: save config 0x04: 0x00100507
[ 460.287651][ T6296] pcieport 0004:40:00.0: save config 0x08: 0x06040001
[ 460.287658][ T6296] pcieport 0004:40:00.0: save config 0x0c: 0x00010000
[ 460.287664][ T6296] pcieport 0004:40:00.0: save config 0x10: 0x00000000
[ 460.287671][ T6296] pcieport 0004:40:00.0: save config 0x14: 0x00000000
[ 460.287677][ T6296] pcieport 0004:40:00.0: save config 0x18: 0x00414140
[ 460.287683][ T6296] pcieport 0004:40:00.0: save config 0x1c: 0x00000000
[ 460.287689][ T6296] pcieport 0004:40:00.0: save config 0x20: 0xf420f420
[ 460.287696][ T6296] pcieport 0004:40:00.0: save config 0x24: 0x0001fff1
[ 460.287702][ T6296] pcieport 0004:40:00.0: save config 0x28: 0x00000000
[ 460.287708][ T6296] pcieport 0004:40:00.0: save config 0x2c: 0x00000000
[ 460.287714][ T6296] pcieport 0004:40:00.0: save config 0x30: 0x00000000
[ 460.287720][ T6296] pcieport 0004:40:00.0: save config 0x34: 0x00000040
[ 460.287727][ T6296] pcieport 0004:40:00.0: save config 0x38: 0x00000000
[ 460.287733][ T6296] pcieport 0004:40:00.0: save config 0x3c: 0x00020197
[ 460.299895][ T6296] pcieport 0004:40:00.0: PCI PM: Suspend power state: D3hot
[ 460.311927][ T6291] rockchip-dw-pcie a41000000.pcie: Timeout
waiting for L2 entry! LTSSM: 0x12
[ 460.311935][ T6291] rockchip-dw-pcie a41000000.pcie: PM:
dpm_run_callback(): genpd_suspend_noirq returns -110
[ 460.311950][ T6291] rockchip-dw-pcie a41000000.pcie: PM: failed to
suspend noirq: error -110
[ 460.328691][ T57] pcieport 0004:40:00.0: restore config 0x2c:
0x00000000 -> 0x00000000
[ 460.328706][ T57] pcieport 0004:40:00.0: restore config 0x28:
0x00000000 -> 0x00000000
[ 460.328714][ T57] pcieport 0004:40:00.0: restore config 0x24:
0x0001fff1 -> 0x0001fff1
[ 460.329363][ T6299] iwlwifi 0002:21:00.0: restore config 0x3c:
0x00000100 -> 0x00000183
[ 460.329558][ T6299] iwlwifi 0002:21:00.0: restore config 0x10:
0x00000004 -> 0xf2200004
[ 460.329643][ T6299] iwlwifi 0002:21:00.0: restore config 0x04:
0x00100000 -> 0x00100406
[ 460.341978][ T6291] PM: noirq suspend of devices failed
[ 460.352925][ T6303] xhci-hcd xhci-hcd.7.auto: xHC error in resume,
USBSTS 0x401, Reinit
[ 460.352937][ T6303] usb usb5: root hub lost power or was reset
[ 460.352941][ T6303] usb usb6: root hub lost power or was reset
[ 460.517043][ T56] xhci-hcd xhci-hcd.8.auto: xHC error in resume,
USBSTS 0x401, Reinit
[ 460.517056][ T56] usb usb7: root hub lost power or was reset
[ 460.517060][ T56] usb usb8: root hub lost power or was reset
[ 460.833660][ T6291] OOM killer enabled.
[ 460.834002][ T6291] Restarting tasks: Starting
[ 460.835432][ T6291] Restarting tasks: Done
[ 460.835846][ T6291] random: crng reseeded on system resumption
[ 460.837377][ T6291] PM: suspend exit
[ 460.838167][ T6291] PM: suspend entry (s2idle)
[ 460.976541][ T6291] Filesystems sync: 0.138 seconds
[ 460.978668][ T6291] Freezing user space processes
[ 460.980928][ T6291] Freezing user space processes completed
(elapsed 0.001 seconds)
[ 460.981607][ T6291] OOM killer disabled.
[ 460.981954][ T6291] Freezing remaining freezable tasks
[ 460.997394][ T6291] Freezing remaining freezable tasks completed
(elapsed 0.014 seconds)
[ 460.998116][ T6291] printk: Suspending console(s) (use
no_console_suspend to debug)
[ 461.041501][ T6299] nvme 0000:01:00.0: save config 0x00: 0xa808144d
[ 461.041520][ T6299] nvme 0000:01:00.0: save config 0x04: 0x00100406
[ 461.041530][ T6299] nvme 0000:01:00.0: save config 0x08: 0x01080200
[ 461.041540][ T6299] nvme 0000:01:00.0: save config 0x0c: 0x00000000
[ 461.041549][ T6299] nvme 0000:01:00.0: save config 0x10: 0xf0200004
[ 461.041559][ T6299] nvme 0000:01:00.0: save config 0x14: 0x00000000
[ 461.041568][ T6299] nvme 0000:01:00.0: save config 0x18: 0x00000000
[ 461.041577][ T6299] nvme 0000:01:00.0: save config 0x1c: 0x00000000
[ 461.041586][ T6299] nvme 0000:01:00.0: save config 0x20: 0x00000000
[ 461.041595][ T6299] nvme 0000:01:00.0: save config 0x24: 0x00000000
[ 461.041604][ T6299] nvme 0000:01:00.0: save config 0x28: 0x00000000
[ 461.041613][ T6299] nvme 0000:01:00.0: save config 0x2c: 0xa801144d
[ 461.041622][ T6299] nvme 0000:01:00.0: save config 0x30: 0x00000000
[ 461.041632][ T6299] nvme 0000:01:00.0: save config 0x34: 0x00000040
[ 461.041641][ T6299] nvme 0000:01:00.0: save config 0x38: 0x00000000
[ 461.041650][ T6299] nvme 0000:01:00.0: save config 0x3c: 0x0000016d
[ 461.211199][ T6307] r8169 0004:41:00.0: save config 0x00: 0x812510ec
[ 461.211220][ T6307] r8169 0004:41:00.0: save config 0x04: 0x00100403
[ 461.211233][ T6307] r8169 0004:41:00.0: save config 0x08: 0x02000005
[ 461.211246][ T6307] r8169 0004:41:00.0: save config 0x0c: 0x00000010
[ 461.211257][ T6307] r8169 0004:41:00.0: save config 0x10: 0xf4100001
[ 461.211269][ T6307] r8169 0004:41:00.0: save config 0x14: 0x00000000
[ 461.211294][ T6297] iwlwifi 0002:21:00.0: save config 0x00: 0x24fd8086
[ 461.211298][ T6307] r8169 0004:41:00.0: save config 0x18: 0xf4200004
[ 461.211311][ T6307] r8169 0004:41:00.0: save config 0x1c: 0x00000000
[ 461.211327][ T6297] iwlwifi 0002:21:00.0: save config 0x04: 0x00100406
[ 461.211333][ T6307] r8169 0004:41:00.0: save config 0x20: 0xf4210004
[ 461.211350][ T6297] iwlwifi 0002:21:00.0: save config 0x08: 0x02800078
[ 461.211355][ T6307] r8169 0004:41:00.0: save config 0x24: 0x00000000
[ 461.211372][ T6297] iwlwifi 0002:21:00.0: save config 0x0c: 0x00000000
[ 461.211379][ T6307] r8169 0004:41:00.0: save config 0x28: 0x00000000
[ 461.211395][ T6297] iwlwifi 0002:21:00.0: save config 0x10: 0xf2200004
[ 461.211401][ T6307] r8169 0004:41:00.0: save config 0x2c: 0x012310ec
[ 461.211417][ T6297] iwlwifi 0002:21:00.0: save config 0x14: 0x00000000
[ 461.211421][ T6307] r8169 0004:41:00.0: save config 0x30: 0x00000000
[ 461.211439][ T6297] iwlwifi 0002:21:00.0: save config 0x18: 0x00000000
[ 461.211445][ T6307] r8169 0004:41:00.0: save config 0x34: 0x00000040
[ 461.211461][ T6297] iwlwifi 0002:21:00.0: save config 0x1c: 0x00000000
[ 461.211468][ T6307] r8169 0004:41:00.0: save config 0x38: 0x00000000
[ 461.211483][ T6297] iwlwifi 0002:21:00.0: save config 0x20: 0x00000000
[ 461.211487][ T6307] r8169 0004:41:00.0: save config 0x3c: 0x00000197
[ 461.211505][ T6297] iwlwifi 0002:21:00.0: save config 0x24: 0x00000000
[ 461.211527][ T6297] iwlwifi 0002:21:00.0: save config 0x28: 0x00000000
[ 461.211550][ T6297] iwlwifi 0002:21:00.0: save config 0x2c: 0x10108086
[ 461.211574][ T6297] iwlwifi 0002:21:00.0: save config 0x30: 0x00000000
[ 461.211596][ T6297] iwlwifi 0002:21:00.0: save config 0x34: 0x000000c8
[ 461.211618][ T6297] iwlwifi 0002:21:00.0: save config 0x38: 0x00000000
[ 461.211644][ T6297] iwlwifi 0002:21:00.0: save config 0x3c: 0x00000183
[ 461.224517][ T6297] iwlwifi 0002:21:00.0: PCI PM: Suspend power state: D3hot
[ 461.224566][ T6307] r8169 0004:41:00.0: PCI PM: Suspend power state: D3hot
[ 461.224614][ T6299] pcieport 0004:40:00.0: save config 0x00: 0x35881d87
[ 461.224622][ T6299] pcieport 0004:40:00.0: save config 0x04: 0x00100507
[ 461.224629][ T6299] pcieport 0004:40:00.0: save config 0x08: 0x06040001
[ 461.224636][ T6299] pcieport 0004:40:00.0: save config 0x0c: 0x00010000
[ 461.224642][ T6299] pcieport 0004:40:00.0: save config 0x10: 0x00000000
[ 461.224649][ T6299] pcieport 0004:40:00.0: save config 0x14: 0x00000000
[ 461.224655][ T6299] pcieport 0004:40:00.0: save config 0x18: 0x00414140
[ 461.224661][ T6299] pcieport 0004:40:00.0: save config 0x1c: 0x00000000
[ 461.224667][ T6299] pcieport 0004:40:00.0: save config 0x20: 0xf420f420
[ 461.224673][ T6299] pcieport 0004:40:00.0: save config 0x24: 0x0001fff1
[ 461.224680][ T6299] pcieport 0004:40:00.0: save config 0x28: 0x00000000
[ 461.224686][ T6299] pcieport 0004:40:00.0: save config 0x2c: 0x00000000
[ 461.224692][ T6299] pcieport 0004:40:00.0: save config 0x30: 0x00000000
[ 461.224698][ T6299] pcieport 0004:40:00.0: save config 0x34: 0x00000040
[ 461.224705][ T6299] pcieport 0004:40:00.0: save config 0x38: 0x00000000
[ 461.224711][ T6299] pcieport 0004:40:00.0: save config 0x3c: 0x00020197
[ 461.236873][ T6299] pcieport 0004:40:00.0: PCI PM: Suspend power state: D3hot
[ 461.247945][ T6291] rockchip-dw-pcie a41000000.pcie: Failed to
receive PME_TO_Ack
[ 461.258929][ T6291] rockchip-dw-pcie a41000000.pcie: Timeout
waiting for L2 entry! LTSSM: 0x12
[ 461.258937][ T6291] rockchip-dw-pcie a41000000.pcie: PM:
dpm_run_callback(): genpd_suspend_noirq returns -110
[ 461.258952][ T6291] rockchip-dw-pcie a41000000.pcie: PM: failed to
suspend noirq: error -110
[ 461.276608][ T6303] iwlwifi 0002:21:00.0: restore config 0x3c:
0x00000100 -> 0x00000183
[ 461.276655][ T6302] pcieport 0004:40:00.0: restore config 0x2c:
0x00000000 -> 0x00000000
[ 461.276683][ T6302] pcieport 0004:40:00.0: restore config 0x28:
0x00000000 -> 0x00000000
[ 461.276714][ T6302] pcieport 0004:40:00.0: restore config 0x24:
0x0001fff1 -> 0x0001fff1
[ 461.276805][ T6303] iwlwifi 0002:21:00.0: restore config 0x10:
0x00000004 -> 0xf2200004
[ 461.276908][ T6303] iwlwifi 0002:21:00.0: restore config 0x04:
0x00100000 -> 0x00100406
[ 461.289977][ T6291] PM: noirq suspend of devices failed
[ 461.302571][ T6298] xhci-hcd xhci-hcd.7.auto: xHC error in resume,
USBSTS 0x401, Reinit
[ 461.302581][ T6298] usb usb5: root hub lost power or was reset
[ 461.302585][ T6298] usb usb6: root hub lost power or was reset
[ 461.425101][ T6307] xhci-hcd xhci-hcd.8.auto: xHC error in resume,
USBSTS 0x401, Reinit
[ 461.425111][ T6307] usb usb7: root hub lost power or was reset
[ 461.425115][ T6307] usb usb8: root hub lost power or was reset
[ 461.746324][ T6291] OOM killer enabled.
[ 461.746666][ T6291] Restarting tasks: Starting
[ 461.748345][ T6291] Restarting tasks: Done
[ 461.748757][ T6291] random: crng reseeded on system resumption
[ 461.749533][ T6291] PM: suspend exit
[ 461.809104][ T6056] Realtek Internal NBASE-T PHY r8169-4-4100:00:
attached PHY driver (mii_bus:phy_addr=r8169-4-4100:00, irq=MAC)
[ 461.810148][ T6056] r8169 0004:41:00.0: enabling bus mastering
[ 461.989335][ T6298] r8169 0004:41:00.0 enP4p65s0: Link is Down
[ 464.915207][ T6311] r8169 0004:41:00.0 enP4p65s0: Link is Up -
1Gbps/Full - flow control rx/tx
[ 465.527599][ T6149] wlan0: authenticate with 78:d2:94:85:bb:b2
(local address=a4:6b:b6:06:5c:8e)
[ 465.529749][ T6149] wlan0: send auth to 78:d2:94:85:bb:b2 (try 1/3)
[ 465.543850][ T6298] wlan0: authenticated
[ 465.545190][ T6298] wlan0: associate with 78:d2:94:85:bb:b2 (try 1/3)
[ 465.555534][ T6298] wlan0: RX AssocResp from 78:d2:94:85:bb:b2
(capab=0x31 status=0 aid=1)
[ 465.560431][ T6298] wlan0: associated
Thanks
-Anand
> 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 (9):
> PCI: dw-rockchip: Rename rockchip_pcie_get_ltssm function
> PCI: dw-rockchip: Support get_ltssm operation
> 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: Add system PM support
> PCI: dwc: support missing PCIe device on resume
>
> drivers/pci/controller/dwc/pcie-designware-host.c | 13 +-
> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 220 ++++++++++++++++++----
> 2 files changed, 198 insertions(+), 35 deletions(-)
> ---
> base-commit: dcb6fa37fd7bc9c3d2b066329b0d27dedf8becaa
> change-id: 20251028-rockchip-pcie-system-suspend-86cf08a7b229
>
> Best regards,
> --
> Sebastian Reichel <sebastian.reichel@collabora.com>
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v4 0/9] PCI: dw-rockchip: add system suspend support
2025-10-29 17:56 [PATCH v4 0/9] PCI: dw-rockchip: add system suspend support Sebastian Reichel
` (9 preceding siblings ...)
2025-11-01 11:19 ` [PATCH v4 0/9] PCI: dw-rockchip: add system suspend support Anand Moon
@ 2025-11-01 13:59 ` Manivannan Sadhasivam
2025-11-03 18:58 ` Sebastian Reichel
10 siblings, 1 reply; 22+ messages in thread
From: Manivannan Sadhasivam @ 2025-11-01 13:59 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, Heiko Stuebner, Philipp Zabel, Jingoo Han,
Shawn Lin, linux-pci, linux-arm-kernel, linux-rockchip,
linux-kernel, kernel
On Wed, Oct 29, 2025 at 06:56:39PM +0100, Sebastian Reichel wrote:
> I've recently been working on fixing up at least basic system suspend
> support on the Rockchip RK3576 platform. Currently the biggest 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.
>
> In opposite to Shawn Lin I did not test with different peripherals as my
> main goal is getting basic suspend to ram working in the first place.
Wouldn't it break users who have connected endpoint devices and suspend their
platform? I don't want to have an untested feature that could potentially cause
regressions, just for the sake of getting basic system PM.
But if your goal is to just add basic system PM operations for CI testing, then
I would suggest you to do something minimal in the suspend/resume path that
don't disrupt the operation of a device.
But this also should be tested with some devices for sanity.
- Mani
> I
> did notice issues with the Broadcom WLAN card on the RK3576 EVB.
> Suspending that platform without a driver being probed works, but after
> probing brcmfmac suspend is aborted because brcmf_pcie_pm_enter_D3()
> does not work. As far as I can tell the problem is unrelated to the
> Rockchip PCIe driver.
>
> 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 (9):
> PCI: dw-rockchip: Rename rockchip_pcie_get_ltssm function
> PCI: dw-rockchip: Support get_ltssm operation
> 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: Add system PM support
> PCI: dwc: support missing PCIe device on resume
>
> drivers/pci/controller/dwc/pcie-designware-host.c | 13 +-
> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 220 ++++++++++++++++++----
> 2 files changed, 198 insertions(+), 35 deletions(-)
> ---
> base-commit: dcb6fa37fd7bc9c3d2b066329b0d27dedf8becaa
> change-id: 20251028-rockchip-pcie-system-suspend-86cf08a7b229
>
> Best regards,
> --
> Sebastian Reichel <sebastian.reichel@collabora.com>
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v4 0/9] PCI: dw-rockchip: add system suspend support
2025-11-01 13:59 ` Manivannan Sadhasivam
@ 2025-11-03 18:58 ` Sebastian Reichel
2025-11-06 19:01 ` Anand Moon
0 siblings, 1 reply; 22+ messages in thread
From: Sebastian Reichel @ 2025-11-03 18:58 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas, Heiko Stuebner, Philipp Zabel, Jingoo Han,
Shawn Lin, linux-pci, linux-arm-kernel, linux-rockchip,
linux-kernel, kernel
[-- Attachment #1: Type: text/plain, Size: 4699 bytes --]
Hi,
On Sat, Nov 01, 2025 at 07:29:41PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Oct 29, 2025 at 06:56:39PM +0100, Sebastian Reichel wrote:
> > I've recently been working on fixing up at least basic system suspend
> > support on the Rockchip RK3576 platform. Currently the biggest 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.
> >
> > In opposite to Shawn Lin I did not test with different peripherals as my
> > main goal is getting basic suspend to ram working in the first place.
>
> Wouldn't it break users who have connected endpoint devices and suspend their
> platform? I don't want to have an untested feature that could potentially cause
> regressions, just for the sake of getting basic system PM.
>
> But if your goal is to just add basic system PM operations for CI
> testing, then I would suggest you to do something minimal in the
> suspend/resume path that don't disrupt the operation of a device.
>
> But this also should be tested with some devices for sanity.
My goal is proper system PM support, but I would like to go step by
step. Right now system suspend on the Rockchip RK3576 EVB just hangs
the board and it has to be power cycled afterwards. In parallel to
this series I've send a bunch of fixes to get it working. It surely
isn't perfect, but I fear things regressing again in other areas while
the complex PCIe system sleep is being worked on - simply blocking system
suspend is not very helpful, since it effectively hides suspend problems.
Greetings,
-- Sebastian
> - Mani
>
> > I did notice issues with the Broadcom WLAN card on the RK3576 EVB.
> > Suspending that platform without a driver being probed works, but after
> > probing brcmfmac suspend is aborted because brcmf_pcie_pm_enter_D3()
> > does not work. As far as I can tell the problem is unrelated to the
> > Rockchip PCIe driver.
> >
> > 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 (9):
> > PCI: dw-rockchip: Rename rockchip_pcie_get_ltssm function
> > PCI: dw-rockchip: Support get_ltssm operation
> > 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: Add system PM support
> > PCI: dwc: support missing PCIe device on resume
> >
> > drivers/pci/controller/dwc/pcie-designware-host.c | 13 +-
> > drivers/pci/controller/dwc/pcie-dw-rockchip.c | 220 ++++++++++++++++++----
> > 2 files changed, 198 insertions(+), 35 deletions(-)
> > ---
> > base-commit: dcb6fa37fd7bc9c3d2b066329b0d27dedf8becaa
> > change-id: 20251028-rockchip-pcie-system-suspend-86cf08a7b229
> >
> > Best regards,
> > --
> > Sebastian Reichel <sebastian.reichel@collabora.com>
> >
>
> --
> மணிவண்ணன் சதாசிவம்
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 0/9] PCI: dw-rockchip: add system suspend support
2025-11-03 18:58 ` Sebastian Reichel
@ 2025-11-06 19:01 ` Anand Moon
2025-11-10 8:40 ` Anand Moon
0 siblings, 1 reply; 22+ messages in thread
From: Anand Moon @ 2025-11-06 19:01 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Heiko Stuebner, Philipp Zabel, Jingoo Han, Shawn Lin, linux-pci,
linux-arm-kernel, linux-rockchip, linux-kernel, kernel
Hi Sebastian,
On Tue, 4 Nov 2025 at 00:29, Sebastian Reichel
<sebastian.reichel@collabora.com> wrote:
>
> Hi,
>
> On Sat, Nov 01, 2025 at 07:29:41PM +0530, Manivannan Sadhasivam wrote:
> > On Wed, Oct 29, 2025 at 06:56:39PM +0100, Sebastian Reichel wrote:
> > > I've recently been working on fixing up at least basic system suspend
> > > support on the Rockchip RK3576 platform. Currently the biggest 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.
> > >
> > > In opposite to Shawn Lin I did not test with different peripherals as my
> > > main goal is getting basic suspend to ram working in the first place.
> >
> > Wouldn't it break users who have connected endpoint devices and suspend their
> > platform? I don't want to have an untested feature that could potentially cause
> > regressions, just for the sake of getting basic system PM.
> >
> > But if your goal is to just add basic system PM operations for CI
> > testing, then I would suggest you to do something minimal in the
> > suspend/resume path that don't disrupt the operation of a device.
> >
> > But this also should be tested with some devices for sanity.
>
> My goal is proper system PM support, but I would like to go step by
> step. Right now system suspend on the Rockchip RK3576 EVB just hangs
> the board and it has to be power cycled afterwards. In parallel to
> this series I've send a bunch of fixes to get it working. It surely
> isn't perfect, but I fear things regressing again in other areas while
> the complex PCIe system sleep is being worked on - simply blocking system
> suspend is not very helpful, since it effectively hides suspend problems.
>
As per my understanding, the current DTS configuration is missing definitions
for critical PCIe power management GPIOs (clkreq-gpios, perst-gpios, wake-gpios)
clkreq-gpios, such as PCIE30x1_0_CLKREQn_M1_L (not sure if it is used ?)
perst-gpios such as PCIE30x1_0_PERSTn_M1_L
wake-gpios, such as PCIE30x1_0_WAKEn_M1_L.
However, the RK3588 TRM indicates that these power management functions
can be controlled programmatically using specific memory-mapped registers:
The PCIE_CLIENT_POWER_CON register at 0x002C provides comprehensive
power management controls, including link-state management, wake-up
event handling,
and clock management.
In PHY GRF, we have the PCIe3PHY_GRF_PHY0_LN0_CON0 register at 0x1000 allows
direct control over the PHY's power state (P-states like P1, P2),
enabling transitions into
deep suspend (L2/L3) via register writes
clkreq_n Clock request for lane X. This is a side-band signal that a
PIPE 4.2 controller needs
to enter and exit P1.CPM, P1.1, and P1.2 power states.
My thought is that using rockchip_pcie_phy_deinit() in the suspend
routine and rockchip_pcie_phy_init() in the resume routine are
incorrect; these functions
likely perform full resets or power cuts rather than managed power
state transitions,
thus disrupting the desired suspended state of the PCIe link
I tried a few things on my own, but I am not moving forward.
Thanks
-Anand
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 0/9] PCI: dw-rockchip: add system suspend support
2025-11-06 19:01 ` Anand Moon
@ 2025-11-10 8:40 ` Anand Moon
0 siblings, 0 replies; 22+ messages in thread
From: Anand Moon @ 2025-11-10 8:40 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Heiko Stuebner, Philipp Zabel, Jingoo Han, Shawn Lin, linux-pci,
linux-arm-kernel, linux-rockchip, linux-kernel, kernel
Hi All,
On Fri, 7 Nov 2025 at 00:31, Anand Moon <linux.amoon@gmail.com> wrote:
>
> Hi Sebastian,
>
> On Tue, 4 Nov 2025 at 00:29, Sebastian Reichel
> <sebastian.reichel@collabora.com> wrote:
> >
> > Hi,
> >
> > On Sat, Nov 01, 2025 at 07:29:41PM +0530, Manivannan Sadhasivam wrote:
> > > On Wed, Oct 29, 2025 at 06:56:39PM +0100, Sebastian Reichel wrote:
> > > > I've recently been working on fixing up at least basic system suspend
> > > > support on the Rockchip RK3576 platform. Currently the biggest 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.
> > > >
> > > > In opposite to Shawn Lin I did not test with different peripherals as my
> > > > main goal is getting basic suspend to ram working in the first place.
> > >
> > > Wouldn't it break users who have connected endpoint devices and suspend their
> > > platform? I don't want to have an untested feature that could potentially cause
> > > regressions, just for the sake of getting basic system PM.
> > >
> > > But if your goal is to just add basic system PM operations for CI
> > > testing, then I would suggest you to do something minimal in the
> > > suspend/resume path that don't disrupt the operation of a device.
> > >
> > > But this also should be tested with some devices for sanity.
> >
> > My goal is proper system PM support, but I would like to go step by
> > step. Right now system suspend on the Rockchip RK3576 EVB just hangs
> > the board and it has to be power cycled afterwards. In parallel to
> > this series I've send a bunch of fixes to get it working. It surely
> > isn't perfect, but I fear things regressing again in other areas while
> > the complex PCIe system sleep is being worked on - simply blocking system
> > suspend is not very helpful, since it effectively hides suspend problems.
> >
> As per my understanding, the current DTS configuration is missing definitions
> for critical PCIe power management GPIOs (clkreq-gpios, perst-gpios, wake-gpios)
>
> clkreq-gpios, such as PCIE30x1_0_CLKREQn_M1_L (not sure if it is used ?)
> perst-gpios such as PCIE30x1_0_PERSTn_M1_L
> wake-gpios, such as PCIE30x1_0_WAKEn_M1_L.
>
As per the TRM 11.5 Interface Description
Signal Name Direction IO Attribute Description
------------ --------- -----------------------
------------------------------------------------------------
BUTTON_RSTN I Pull-down External
reset button input; pulled low to initiate reset
WAKEN I/O Open-drain, pull-up Wake
signal; enables device or host to signal wake events
PERSTN I/O Pull-down PCIe
reset signal; asserted low to reset endpoint or root complex
CLKREQN I/O Open-drain, pull-up Clock
request signal; used for dynamic clock management
See the following commit 4294e3211178 ("arm64: dts: rockchip: Split up
RK3588's PCIe pinctrls")
These pinctrls manage the low-speed PCIe signals:
- CLKREQ#: An output on the RK3588 (both RC or EP modes), used to
request that external clock-generation circuitry provide a clock.
- PERST#: An input on the RK3588 in EP mode, used to detect a reset
signal from the RC. In RC mode, the hardware does not use this signal:
Linux itself generates it by putting the pin in GPIO mode.
- WAKE#: In EP mode, this is an output; in RC mode, this is an input.
Each of these signals serves a distinct purpose, and more importantly,
PERST# should not be muxed when the RK3588 is in the RC role. Bundling
them together in pinctrl groups prevents proper use: indeed, almost none
of the current board-specific .dts files make any use of them.
(Exception: Rock 5A recently had a patch land that misuses _pins; this
patch corrects that.)
However, on some RK3588 boards, the PCIe 3 controller will indefinitely
stall the boot if CLKREQ# is not muxed (details in the next patch).
This patch unbundles the signals to allow them to be used.
So we can use these pinctrl to perform different tasks
like PERST# to reset and WAKE# to wake the PCIE in suspend / resume state.
Is this a good way to split the rk368 PCIe pinctrl into separate components?
Here is the example user wake gpio.
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
index 19a08f7794e6..13a7aa3ec1fc 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
@@ -359,9 +359,10 @@ rgmii_phy1: ethernet-phy@1 {
};
&pcie2x1l2 {
- pinctrl-0 = <&pcie2_reset>, <&pcie20x1m0_clkreqn>, <&pcie20x1m0_waken>;
+ pinctrl-0 = <&pcie2_reset>, <&pcie20x1m0_clkreqn>, <&pcie2wakeup>;
pinctrl-names = "default";
reset-gpios = <&gpio3 RK_PD1 GPIO_ACTIVE_HIGH>;
+ wakeup-gpio = <&gpio3 RK_PD0 GPIO_ACTIVE_HIGH>;
vpcie3v3-supply = <&vcc3v3_wf>;
status = "okay";
};
I haven’t come across a working example for this in RC mode.
Is there any confirmation that this approach functions as expected?
> However, the RK3588 TRM indicates that these power management functions
> can be controlled programmatically using specific memory-mapped registers:
>
> The PCIE_CLIENT_POWER_CON register at 0x002C provides comprehensive
> power management controls, including link-state management, wake-up
> event handling,
> and clock management.
>
> In PHY GRF, we have the PCIe3PHY_GRF_PHY0_LN0_CON0 register at 0x1000 allows
> direct control over the PHY's power state (P-states like P1, P2),
> enabling transitions into
> deep suspend (L2/L3) via register writes
> clkreq_n Clock request for lane X. This is a side-band signal that a
> PIPE 4.2 controller needs
> to enter and exit P1.CPM, P1.1, and P1.2 power states.
>
> My thought is that using rockchip_pcie_phy_deinit() in the suspend
> routine and rockchip_pcie_phy_init() in the resume routine are
> incorrect; these functions
> likely perform full resets or power cuts rather than managed power
> state transitions,
> thus disrupting the desired suspended state of the PCIe link
>
> I tried a few things on my own, but I am not moving forward.
>
Thanks
-Anand
^ permalink raw reply related [flat|nested] 22+ messages in thread