* [PATCH v7 01/10] PCI: dw-rockchip: Fix LTSSM set functions
2026-11-04 18:56 [PATCH v7 00/10] PCI: dw-rockchip: add system suspend support Sebastian Reichel
@ 2026-11-04 18:56 ` Sebastian Reichel
2026-11-04 19:02 ` sashiko-bot
2026-11-04 18:56 ` [PATCH v7 02/10] PCI: dw-rockchip: Restore vpcie3v3 regulator handle Sebastian Reichel
` (9 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Sebastian Reichel @ 2026-11-04 18:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin, Liam Girdwood, Mark Brown,
Yury Norov (NVIDIA), Nicolas Frattaroli
Cc: Alexey Charkov, linux-pci, linux-arm-kernel, linux-rockchip,
linux-kernel, kernel, 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")
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 | 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 bd86e32cc2b1..c35c5583c797 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -322,14 +322,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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v7 02/10] PCI: dw-rockchip: Restore vpcie3v3 regulator handle
2026-11-04 18:56 [PATCH v7 00/10] PCI: dw-rockchip: add system suspend support Sebastian Reichel
2026-11-04 18:56 ` [PATCH v7 01/10] PCI: dw-rockchip: Fix LTSSM set functions Sebastian Reichel
@ 2026-11-04 18:56 ` Sebastian Reichel
2026-11-04 19:07 ` sashiko-bot
2026-11-04 18:56 ` [PATCH v7 03/10] PCI: dw-rockchip: Move devm_phy_get out of phy_init Sebastian Reichel
` (8 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Sebastian Reichel @ 2026-11-04 18:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin, Liam Girdwood, Mark Brown,
Yury Norov (NVIDIA), Nicolas Frattaroli
Cc: Alexey Charkov, linux-pci, linux-arm-kernel, linux-rockchip,
linux-kernel, kernel, 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 | 27 ++++++++++++++++++++-------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index c35c5583c797..574d3dcfcf8b 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -113,6 +113,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,24 @@ 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");
+ if (ret) {
+ dev_err_probe(dev, ret, "failed to initialize the phy\n");
+ goto disable_regulator;
+ }
ret = reset_control_deassert(rockchip->rst);
if (ret)
@@ -885,6 +895,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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v7 03/10] PCI: dw-rockchip: Move devm_phy_get out of phy_init
2026-11-04 18:56 [PATCH v7 00/10] PCI: dw-rockchip: add system suspend support Sebastian Reichel
2026-11-04 18:56 ` [PATCH v7 01/10] PCI: dw-rockchip: Fix LTSSM set functions Sebastian Reichel
2026-11-04 18:56 ` [PATCH v7 02/10] PCI: dw-rockchip: Restore vpcie3v3 regulator handle Sebastian Reichel
@ 2026-11-04 18:56 ` Sebastian Reichel
2026-11-04 19:15 ` sashiko-bot
2026-11-04 18:56 ` [PATCH v7 04/10] PCI: dw-rockchip: Add helper function for enhanced LTSSM control mode Sebastian Reichel
` (7 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Sebastian Reichel @ 2026-11-04 18:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin, Liam Girdwood, Mark Brown,
Yury Norov (NVIDIA), Nicolas Frattaroli
Cc: Alexey Charkov, 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.
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 574d3dcfcf8b..a3b0fb918479 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -599,14 +599,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) {
dev_err_probe(dev, ret, "failed to initialize the phy\n");
--
2.53.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v7 04/10] PCI: dw-rockchip: Add helper function for enhanced LTSSM control mode
2026-11-04 18:56 [PATCH v7 00/10] PCI: dw-rockchip: add system suspend support Sebastian Reichel
` (2 preceding siblings ...)
2026-11-04 18:56 ` [PATCH v7 03/10] PCI: dw-rockchip: Move devm_phy_get out of phy_init Sebastian Reichel
@ 2026-11-04 18:56 ` Sebastian Reichel
2026-11-04 19:20 ` sashiko-bot
2026-11-04 18:56 ` [PATCH v7 05/10] 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 @ 2026-11-04 18:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin, Liam Girdwood, Mark Brown,
Yury Norov (NVIDIA), Nicolas Frattaroli
Cc: Alexey Charkov, 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 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 a3b0fb918479..7645b0895bf9 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -683,6 +683,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,
+ bool delay_link_training_after_reset)
+{
+ u32 val;
+
+ /* Enable the enhanced control mode of signal app_ltssm_enable */
+ val = FIELD_PREP_WM16(PCIE_LTSSM_ENABLE_ENHANCE, 1);
+ if (delay_link_training_after_reset)
+ val |= FIELD_PREP_WM16(PCIE_LTSSM_APP_DLY2_EN, 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)
{
@@ -706,9 +718,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, false);
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, true);
rockchip_pcie_writel_apb(rockchip,
PCIE_CLIENT_SET_MODE(PCIE_CLIENT_MODE_EP),
--
2.53.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v7 04/10] PCI: dw-rockchip: Add helper function for enhanced LTSSM control mode
2026-11-04 18:56 ` [PATCH v7 04/10] PCI: dw-rockchip: Add helper function for enhanced LTSSM control mode Sebastian Reichel
@ 2026-11-04 19:20 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-11-04 19:20 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-pci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] PCI: dw-rockchip: missed LTSSM helper in rockchip_pcie_rc_reset_root_port()
--
commit 01b1475be69dbc97dabfa0df29b029fb455894d6
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
PCI: dw-rockchip: Add helper function for enhanced LTSSM control mode
This commit introduces a rockchip_pcie_enable_enhanced_ltssm_control_mode()
helper function to deduplicate setting up the enhanced LTSSM mode in
rockchip_pcie_configure_rc() and rockchip_pcie_configure_ep().
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index a3b0fb918479f..7645b0895bf91 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -683,6 +683,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,
> + bool delay_link_training_after_reset)
> +{
> + u32 val;
> +
> + /* Enable the enhanced control mode of signal app_ltssm_enable */
> + val = FIELD_PREP_WM16(PCIE_LTSSM_ENABLE_ENHANCE, 1);
> + if (delay_link_training_after_reset)
> + val |= FIELD_PREP_WM16(PCIE_LTSSM_APP_DLY2_EN, 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)
> {
> @@ -706,9 +718,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, false);
[Severity: Low]
As reported by Sashiko on v6 [1], rockchip_pcie_rc_reset_root_port() in
drivers/pci/controller/dwc/pcie-dw-rockchip.c still open-codes the same
PCIE_CLIENT_HOT_RESET_CTRL write:
/* LTSSM enable control mode */
val = FIELD_PREP_WM16(PCIE_LTSSM_ENABLE_ENHANCE, 1);
rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_HOT_RESET_CTRL);
Should rockchip_pcie_rc_reset_root_port() also be converted to call
rockchip_pcie_enable_enhanced_ltssm_control_mode(rockchip, false)?
[1] https://lore.kernel.org/r/20260908-rockchip-pcie-system-suspend-v6-4-fbabcca9921b@collabora.com
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-rockchip-pcie-system-suspend-v7-0-6ae6875939a2@collabora.com?part=4
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v7 05/10] PCI: dw-rockchip: Add helper function for controller mode
2026-11-04 18:56 [PATCH v7 00/10] PCI: dw-rockchip: add system suspend support Sebastian Reichel
` (3 preceding siblings ...)
2026-11-04 18:56 ` [PATCH v7 04/10] PCI: dw-rockchip: Add helper function for enhanced LTSSM control mode Sebastian Reichel
@ 2026-11-04 18:56 ` Sebastian Reichel
2026-11-04 19:24 ` sashiko-bot
2026-11-04 18:56 ` [PATCH v7 06/10] PCI: dw-rockchip: Add helper function for DDL indicator Sebastian Reichel
` (5 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Sebastian Reichel @ 2026-11-04 18:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin, Liam Girdwood, Mark Brown,
Yury Norov (NVIDIA), Nicolas Frattaroli
Cc: Alexey Charkov, 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.
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 7645b0895bf9..18dce2f52a27 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -695,6 +695,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)
{
@@ -719,10 +724,7 @@ static int rockchip_pcie_configure_rc(struct platform_device *pdev,
}
rockchip_pcie_enable_enhanced_ltssm_control_mode(rockchip, false);
-
- 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);
irq = of_irq_get_byname(dev->of_node, "legacy");
if (irq < 0)
@@ -776,10 +778,7 @@ static int rockchip_pcie_configure_ep(struct platform_device *pdev,
}
rockchip_pcie_enable_enhanced_ltssm_control_mode(rockchip, true);
-
- 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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v7 06/10] PCI: dw-rockchip: Add helper function for DDL indicator
2026-11-04 18:56 [PATCH v7 00/10] PCI: dw-rockchip: add system suspend support Sebastian Reichel
` (4 preceding siblings ...)
2026-11-04 18:56 ` [PATCH v7 05/10] PCI: dw-rockchip: Add helper function for controller mode Sebastian Reichel
@ 2026-11-04 18:56 ` Sebastian Reichel
2026-11-04 19:31 ` sashiko-bot
2026-11-04 18:56 ` [PATCH v7 07/10] PCI: dw-rockchip: Add pme_turn_off support Sebastian Reichel
` (4 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Sebastian Reichel @ 2026-11-04 18:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin, Liam Girdwood, Mark Brown,
Yury Norov (NVIDIA), Nicolas Frattaroli
Cc: Alexey Charkov, 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.
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 18dce2f52a27..f089dab4acf0 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -700,6 +700,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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v7 07/10] PCI: dw-rockchip: Add pme_turn_off support
2026-11-04 18:56 [PATCH v7 00/10] PCI: dw-rockchip: add system suspend support Sebastian Reichel
` (5 preceding siblings ...)
2026-11-04 18:56 ` [PATCH v7 06/10] PCI: dw-rockchip: Add helper function for DDL indicator Sebastian Reichel
@ 2026-11-04 18:56 ` Sebastian Reichel
2026-11-04 19:37 ` sashiko-bot
2026-11-04 18:56 ` [PATCH v7 08/10] PCI: dw-rockchip: Set broken L1SS resume flag Sebastian Reichel
` (3 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Sebastian Reichel @ 2026-11-04 18:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin, Liam Girdwood, Mark Brown,
Yury Norov (NVIDIA), Nicolas Frattaroli
Cc: Alexey Charkov, 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.
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 47 +++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index f089dab4acf0..417e6fbbfe09 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -47,6 +47,10 @@
#define PCIE_CLIENT_ENABLE_LTSSM FIELD_PREP_WM16(BIT(2), 1)
#define PCIE_CLIENT_DISABLE_LTSSM FIELD_PREP_WM16(BIT(2), 0)
+/* Interrupt Status Register for Received Messages */
+#define PCIE_CLIENT_INTR_STATUS_MSG_RX 0x04
+#define PME_TO_ACK BIT(9)
+
/* Interrupt Status Register Related to Legacy Interrupt */
#define PCIE_CLIENT_INTR_STATUS_LEGACY 0x8
@@ -71,6 +75,11 @@
#define PCIE_CLKREQ_READY FIELD_PREP_WM16(BIT(0), 1)
#define PCIE_CLKREQ_NOT_READY FIELD_PREP_WM16(BIT(0), 0)
#define PCIE_CLKREQ_PULL_DOWN FIELD_PREP_WM16(GENMASK(13, 12), 1)
+#define PME_READY_ENTER_L23 BIT(3)
+
+/* Message Request Register */
+#define PCIE_CLIENT_MSG_GEN 0x34
+#define PME_TURN_OFF FIELD_PREP_WM16(BIT(4), 1)
/* RASDES TBA information */
#define PCIE_CLIENT_CDM_RASDES_TBA_INFO_CMN 0x154
@@ -439,8 +448,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_CON,
+ 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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v7 08/10] PCI: dw-rockchip: Set broken L1SS resume flag
2026-11-04 18:56 [PATCH v7 00/10] PCI: dw-rockchip: add system suspend support Sebastian Reichel
` (6 preceding siblings ...)
2026-11-04 18:56 ` [PATCH v7 07/10] PCI: dw-rockchip: Add pme_turn_off support Sebastian Reichel
@ 2026-11-04 18:56 ` Sebastian Reichel
2026-11-04 19:42 ` sashiko-bot
2026-11-04 18:56 ` [PATCH v7 09/10] PCI: dw-rockchip: Add system PM support Sebastian Reichel
` (2 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Sebastian Reichel @ 2026-11-04 18:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin, Liam Girdwood, Mark Brown,
Yury Norov (NVIDIA), Nicolas Frattaroli
Cc: Alexey Charkov, linux-pci, linux-arm-kernel, linux-rockchip,
linux-kernel, kernel, Sebastian Reichel
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 broken_l1ss_resume flag to indicate this limitation to the PCIe
framework. This results in pci_suspend_retains_context() returning false
and PCIe client can avoid relying on device state being preserved
during system suspend.
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 | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 417e6fbbfe09..2e759f92b5c6 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -448,6 +448,11 @@ 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)
+{
+ 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);
@@ -487,6 +492,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
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH v7 09/10] PCI: dw-rockchip: Add system PM support
2026-11-04 18:56 [PATCH v7 00/10] PCI: dw-rockchip: add system suspend support Sebastian Reichel
` (7 preceding siblings ...)
2026-11-04 18:56 ` [PATCH v7 08/10] PCI: dw-rockchip: Set broken L1SS resume flag Sebastian Reichel
@ 2026-11-04 18:56 ` Sebastian Reichel
2026-11-04 19:57 ` sashiko-bot
2026-11-04 18:56 ` [PATCH v7 10/10] PCI: dw-rockchip: Clear debug buffer before entering L2 Sebastian Reichel
2026-11-04 22:44 ` [PATCH v7 00/10] PCI: dw-rockchip: add system suspend support Igor Velkov
10 siblings, 1 reply; 22+ messages in thread
From: Sebastian Reichel @ 2026-11-04 18:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin, Liam Girdwood, Mark Brown,
Yury Norov (NVIDIA), Nicolas Frattaroli
Cc: Alexey Charkov, 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 | 104 ++++++++++++++++++++++++++
1 file changed, 104 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 2e759f92b5c6..75b40091bb95 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -124,6 +124,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;
@@ -804,6 +805,7 @@ static int rockchip_pcie_configure_rc(struct platform_device *pdev,
pp = &rockchip->pci.pp;
pp->ops = &rockchip_pcie_host_ops;
+ pp->suspend_poweroff = true;
ret = dw_pcie_host_init(pp);
if (ret) {
@@ -1037,6 +1039,102 @@ 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)
+ return 0;
+
+ 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)
+ return 0;
+
+ if (rockchip->vpcie3v3) {
+ ret = regulator_enable(rockchip->vpcie3v3);
+ if (ret) {
+ dev_err(dev, "regulator enable failed: %d\n", ret);
+ return ret;
+ }
+ }
+
+ ret = rockchip_pcie_phy_init(rockchip);
+ if (ret) {
+ dev_err(dev, "phy init failed: %d\n", ret);
+ goto err_disable_regulator;
+ }
+
+ ret = reset_control_deassert(rockchip->rst);
+ if (ret) {
+ dev_err(dev, "reset deassert failed: %d\n", ret);
+ goto err_deinit_phy;
+ }
+
+ ret = clk_bulk_prepare_enable(rockchip->clk_cnt, rockchip->clks);
+ if (ret) {
+ dev_err(dev, "clock init failed: %d\n", ret);
+ goto err_assert_reset;
+ }
+
+ 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);
+
+ ret = gpiod_set_value_cansleep(rockchip->rst_gpio, 1);
+ if (ret) {
+ dev_err(dev, "failed to set reset gpio: %d\n", ret);
+ goto err_disable_clk;
+ }
+
+ ret = dw_pcie_resume_noirq(pci);
+ if (ret) {
+ dev_err(dev, "failed to resume: %d\n", ret);
+ goto err_clear_rst_gpio;
+ }
+
+ return 0;
+
+err_clear_rst_gpio:
+ gpiod_set_value_cansleep(rockchip->rst_gpio, 0);
+err_disable_clk:
+ clk_bulk_disable_unprepare(rockchip->clk_cnt, rockchip->clks);
+err_assert_reset:
+ reset_control_assert(rockchip->rst);
+err_deinit_phy:
+ rockchip_pcie_phy_deinit(rockchip);
+err_disable_regulator:
+ if (rockchip->vpcie3v3)
+ regulator_disable(rockchip->vpcie3v3);
+
+ return ret;
+}
+
static const struct rockchip_pcie_of_data rockchip_pcie_rc_of_data_rk3568 = {
.mode = DW_PCIE_RC_TYPE,
};
@@ -1067,11 +1165,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
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v7 09/10] PCI: dw-rockchip: Add system PM support
2026-11-04 18:56 ` [PATCH v7 09/10] PCI: dw-rockchip: Add system PM support Sebastian Reichel
@ 2026-11-04 19:57 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-11-04 19:57 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-pci
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] PCI: dw-rockchip: premature rst_gpio deassertion in rockchip_pcie_resume()
- [High] PCI: dw-rockchip: unclocked MMIO and double disable in rockchip_pcie_suspend()
- [Medium] PCI: dw-rockchip: wrong IRQ mask and order in rockchip_pcie_resume()
- [Low] PCI: dw-rockchip: signed shift overflow in rockchip_pcie_resume()
--
commit 941dec7d380bdb3a689bc84885404d2f2e8eef67
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
PCI: dw-rockchip: Add system PM support
This commit adds noirq system suspend and resume callbacks for Rockchip
DesignWare PCIe controllers in Root Complex mode. It saves and restores the
legacy interrupt mask and sequences the PCIe link, PERST# GPIO, PHY, clocks,
reset line, and 3.3V regulator across suspend and resume.
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 2e759f92b5c6f..75b40091bb953 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
[ ... ]
> @@ -1037,6 +1039,102 @@ 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)
> + return 0;
> +
> + 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);
[Severity: High]
What happens on a subsequent suspend if rockchip_pcie_resume() fails?
During suspend, dw_pcie_suspend_noirq() sets pci->suspended = true, and
rockchip_pcie_suspend() then powers off the PHY, disables clocks, asserts
reset, and disables rockchip->vpcie3v3.
If a later call to rockchip_pcie_resume() fails (for example, if
dw_pcie_resume_noirq() returns -ETIMEDOUT when dw_pcie_wait_for_link() times
out, or if regulator, PHY, reset, or clock initialization fails), the error
unwind path in rockchip_pcie_resume() disables the clocks, asserts reset,
deinitializes the PHY, and disables rockchip->vpcie3v3 while pci->suspended
remains true:
drivers/pci/controller/dwc/pcie-dw-rockchip.c:rockchip_pcie_resume() {
...
ret = dw_pcie_resume_noirq(pci);
if (ret) {
dev_err(dev, "failed to resume: %d\n", ret);
goto err_clear_rst_gpio;
}
return 0;
err_clear_rst_gpio:
gpiod_set_value_cansleep(rockchip->rst_gpio, 0);
err_disable_clk:
clk_bulk_disable_unprepare(rockchip->clk_cnt, rockchip->clks);
err_assert_reset:
reset_control_assert(rockchip->rst);
err_deinit_phy:
rockchip_pcie_phy_deinit(rockchip);
err_disable_regulator:
if (rockchip->vpcie3v3)
regulator_disable(rockchip->vpcie3v3);
return ret;
}
Because the PM core logs the noirq resume error, clears is_noirq_suspended,
and continues resuming the system, won't child PCI device resume and the next
call to rockchip_pcie_suspend() read APB/DBI registers while the controller
clocks are disabled and reset is asserted (causing an SError on ARM64), and
then call rockchip_pcie_phy_deinit(), clk_bulk_disable_unprepare(), and
regulator_disable() a second time?
> +
> + 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)
> + return 0;
> +
> + if (rockchip->vpcie3v3) {
> + ret = regulator_enable(rockchip->vpcie3v3);
> + if (ret) {
> + dev_err(dev, "regulator enable failed: %d\n", ret);
> + return ret;
> + }
> + }
> +
> + ret = rockchip_pcie_phy_init(rockchip);
> + if (ret) {
> + dev_err(dev, "phy init failed: %d\n", ret);
> + goto err_disable_regulator;
> + }
> +
> + ret = reset_control_deassert(rockchip->rst);
> + if (ret) {
> + dev_err(dev, "reset deassert failed: %d\n", ret);
> + goto err_deinit_phy;
> + }
> +
> + ret = clk_bulk_prepare_enable(rockchip->clk_cnt, rockchip->clks);
> + if (ret) {
> + dev_err(dev, "clock init failed: %d\n", ret);
> + goto err_assert_reset;
> + }
> +
> + rockchip_pcie_writel_apb(rockchip, FIELD_PREP_WM16(0xffff, rockchip->intx),
> + PCIE_CLIENT_INTR_MASK_LEGACY);
[Severity: Low]
Could passing 0xffff to FIELD_PREP_WM16() trigger a signed shift overflow in
rockchip_pcie_resume()?
FIELD_PREP_WM16() preserves the type of _mask and shifts it left by 16:
include/linux/hw_bitfield.h:
#define FIELD_PREP_WM16(_mask, _val) \
({ \
typeof(_val) __val = _val; \
typeof(_mask) __mask = _mask; \
__BF_FIELD_CHECK(__mask, ((u16)0U), __val, \
"HWORD_UPDATE: "); \
(((typeof(__mask))(__val) << __bf_shf(__mask)) & (__mask)) | \
((__mask) << 16); \
})
Because 0xffff is a signed int literal with bit 15 set, (__mask) << 16 shifts
into the sign bit of a 32-bit signed int, triggering a UBSAN
shift-out-of-bounds warning when CONFIG_UBSAN_SHIFT is enabled. Should this
use PCIE_INTR_MASK (GENMASK(7, 0)) or an unsigned mask instead?
> +
> + 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);
[Severity: Medium]
Should rockchip_pcie_resume() be calling rockchip_pcie_unmask_dll_indicator()
here in Root Complex mode and before dw_pcie_resume_noirq()?
Looking at rockchip_pcie_unmask_dll_indicator():
drivers/pci/controller/dwc/pcie-dw-rockchip.c:rockchip_pcie_unmask_dll_indicator() {
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);
}
In RC mode, rockchip_pcie_configure_rc() and
rockchip_pcie_rc_reset_root_port() only unmask PCIE_LINK_REQ_RST_NOT_INT, and
rockchip_pcie_configure_rc() only does so after dw_pcie_host_init() has
configured the RC and brought up the link:
drivers/pci/controller/dwc/pcie-dw-rockchip.c:rockchip_pcie_configure_rc() {
...
ret = dw_pcie_host_init(pp);
if (ret) {
dev_err(dev, "failed to initialize host\n");
return ret;
}
/* unmask hot reset/link-down reset */
val = FIELD_PREP_WM16(PCIE_LINK_REQ_RST_NOT_INT, 0);
rockchip_pcie_writel_apb(rockchip, val, PCIE_CLIENT_INTR_MASK_MISC);
}
Because rockchip_pcie_rc_sys_irq_thread() only handles
PCIE_LINK_REQ_RST_NOT_INT (whereas PCIE_RDLH_LINK_UP_CHGED is handled by
rockchip_pcie_ep_sys_irq_thread()), won't calling
rockchip_pcie_unmask_dll_indicator() before dw_pcie_resume_noirq() cause
spurious pcie-sys-rc interrupts on link-up changes and allow link transitions
during resume link training to latch PCIE_LINK_REQ_RST_NOT_INT and trigger
pci_host_handle_link_down() once IRQs are re-enabled?
> +
> + ret = gpiod_set_value_cansleep(rockchip->rst_gpio, 1);
> + if (ret) {
> + dev_err(dev, "failed to set reset gpio: %d\n", ret);
> + goto err_disable_clk;
> + }
> +
> + ret = dw_pcie_resume_noirq(pci);
[Severity: High]
Does setting rockchip->rst_gpio to 1 before dw_pcie_resume_noirq() cause a
premature PERST# deassertion and reset glitch on resume?
In rockchip_pcie_resume(), gpiod_set_value_cansleep(rockchip->rst_gpio, 1)
deasserts PERST# before dw_pcie_resume_noirq() runs. Immediately afterward,
dw_pcie_resume_noirq() calls dw_pcie_setup_rc() and
dw_pcie_start_link() -> rockchip_pcie_start_link(), which drives
rockchip->rst_gpio back to 0, enables LTSSM, waits PCIE_T_PVPERL_MS (100ms),
and then drives rockchip->rst_gpio to 1:
drivers/pci/controller/dwc/pcie-dw-rockchip.c:rockchip_pcie_start_link() {
struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
/* Reset device */
gpiod_set_value_cansleep(rockchip->rst_gpio, 0);
rockchip_pcie_enable_ltssm(rockchip);
...
msleep(PCIE_T_PVPERL_MS);
rockchip_pcie_ltssm_trace(rockchip, true);
gpiod_set_value_cansleep(rockchip->rst_gpio, 1);
return 0;
}
Won't driving rockchip->rst_gpio high before rockchip_pcie_start_link()
produce a 0 -> 1 -> 0 -> 1 glitch on PERST# before the RC is configured,
before LTSSM and refclk are enabled, and before PCIE_T_PVPERL_MS has elapsed?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-rockchip-pcie-system-suspend-v7-0-6ae6875939a2@collabora.com?part=9
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v7 10/10] PCI: dw-rockchip: Clear debug buffer before entering L2
2026-11-04 18:56 [PATCH v7 00/10] PCI: dw-rockchip: add system suspend support Sebastian Reichel
` (8 preceding siblings ...)
2026-11-04 18:56 ` [PATCH v7 09/10] PCI: dw-rockchip: Add system PM support Sebastian Reichel
@ 2026-11-04 18:56 ` Sebastian Reichel
2026-11-04 20:03 ` sashiko-bot
2026-11-04 22:44 ` [PATCH v7 00/10] PCI: dw-rockchip: add system suspend support Igor Velkov
10 siblings, 1 reply; 22+ messages in thread
From: Sebastian Reichel @ 2026-11-04 18:56 UTC (permalink / raw)
To: Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin, Liam Girdwood, Mark Brown,
Yury Norov (NVIDIA), Nicolas Frattaroli
Cc: Alexey Charkov, linux-pci, linux-arm-kernel, linux-rockchip,
linux-kernel, kernel, Sebastian Reichel
Clear the debug buffer before entering the L2 state to ease debugging
link training issues on resume. The register latches link/LTSSM status.
Clearing it at suspend means post-resume diagnostics reflect the new
link bring-up, and not pre-suspend residue.
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 | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
index 75b40091bb95..66b3d31d7bc9 100644
--- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
+++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
@@ -81,6 +81,9 @@
#define PCIE_CLIENT_MSG_GEN 0x34
#define PME_TURN_OFF FIELD_PREP_WM16(BIT(4), 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)
@@ -1039,6 +1042,11 @@ static int rockchip_pcie_rc_reset_root_port(struct pci_host_bridge *bridge,
return ret;
}
+static void rockchip_pcie_link_status_clear(struct rockchip_pcie *rockchip)
+{
+ rockchip_pcie_writel_apb(rockchip, 0x0, PCIE_CLIENT_GENERAL_DEBUG);
+}
+
static int rockchip_pcie_suspend(struct device *dev)
{
struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
@@ -1050,6 +1058,8 @@ static int rockchip_pcie_suspend(struct device *dev)
rockchip->intx = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_INTR_MASK_LEGACY);
+ rockchip_pcie_link_status_clear(rockchip);
+
ret = dw_pcie_suspend_noirq(pci);
if (ret)
return ret;
--
2.53.0
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH v7 10/10] PCI: dw-rockchip: Clear debug buffer before entering L2
2026-11-04 18:56 ` [PATCH v7 10/10] PCI: dw-rockchip: Clear debug buffer before entering L2 Sebastian Reichel
@ 2026-11-04 20:03 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-11-04 20:03 UTC (permalink / raw)
To: Sebastian Reichel; +Cc: linux-pci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] PCI: dw-rockchip: ineffective debug register clear in rockchip_pcie_suspend()
--
commit f0e5805bf8d7dc88d910a870a6063138c20497af
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
PCI: dw-rockchip: Clear debug buffer before entering L2
This commit adds rockchip_pcie_link_status_clear() to write 0x0 to
PCIE_CLIENT_GENERAL_DEBUG during system suspend in rockchip_pcie_suspend() so
that post-resume diagnostics reflect the new link bring-up.
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 75b40091bb953..66b3d31d7bc9c 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -81,6 +81,9 @@
> #define PCIE_CLIENT_MSG_GEN 0x34
> #define PME_TURN_OFF FIELD_PREP_WM16(BIT(4), 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)
> @@ -1039,6 +1042,11 @@ static int rockchip_pcie_rc_reset_root_port(struct pci_host_bridge *bridge,
> return ret;
> }
>
> +static void rockchip_pcie_link_status_clear(struct rockchip_pcie *rockchip)
> +{
> + rockchip_pcie_writel_apb(rockchip, 0x0, PCIE_CLIENT_GENERAL_DEBUG);
> +}
> +
> static int rockchip_pcie_suspend(struct device *dev)
> {
> struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
> @@ -1050,6 +1058,8 @@ static int rockchip_pcie_suspend(struct device *dev)
>
> rockchip->intx = rockchip_pcie_readl_apb(rockchip, PCIE_CLIENT_INTR_MASK_LEGACY);
>
> + rockchip_pcie_link_status_clear(rockchip);
[Severity: Medium]
Does clearing PCIE_CLIENT_GENERAL_DEBUG at this point in
rockchip_pcie_suspend() have any effect on link or LTSSM diagnostics in the
upstream driver?
In drivers/pci/controller/dwc/pcie-dw-rockchip.c, PCIE_CLIENT_GENERAL_DEBUG
(0x104) is never written during link bring-up and is never read by any status,
diagnostic, or trace path. Live LTSSM state is read from
PCIE_CLIENT_LTSSM_STATUS and PCIE_CLIENT_CDM_RASDES_TBA_INFO_CMN in
rockchip_pcie_get_ltssm(), while hardware LTSSM state transition history is
recorded in the debug FIFO at PCIE_CLIENT_DBG_FIFO_MODE_CON through
PCIE_CLIENT_DBG_FIFO_STATUS managed by rockchip_pcie_ltssm_trace() in
rockchip_pcie_start_link() and rockchip_pcie_stop_link(). (In the downstream
vendor kernel, PCIE_CLIENT_GENERAL_DEBUG is used as a software scratchpad
register written by vendor-specific link-up code that is not present
upstream.)
Furthermore, even if PCIE_CLIENT_GENERAL_DEBUG latched hardware LTSSM
transitions, rockchip_pcie_link_status_clear() is called before
dw_pcie_suspend_noirq() while the link is still active:
rockchip_pcie_suspend()
rockchip_pcie_link_status_clear()
dw_pcie_suspend_noirq()
rockchip_pcie_pme_turn_off()
dw_pcie_get_ltssm() /* polls for L2_IDLE */
dw_pcie_stop_link()
...
clk_bulk_disable_unprepare(rockchip->clk_cnt, rockchip->clks);
reset_control_assert(rockchip->rst);
Because dw_pcie_suspend_noirq() subsequently broadcasts PME_Turn_Off,
transitions LTSSM to L2_IDLE, and stops the link, and rockchip_pcie_suspend()
then disables clocks and asserts reset_control_assert(rockchip->rst) (which
resets the APB client registers across suspend/resume, requiring
rockchip->intx to be saved and restored), is this write to
PCIE_CLIENT_GENERAL_DEBUG needed or intended to target a different mechanism?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-rockchip-pcie-system-suspend-v7-0-6ae6875939a2@collabora.com?part=10
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v7 00/10] PCI: dw-rockchip: add system suspend support
2026-11-04 18:56 [PATCH v7 00/10] PCI: dw-rockchip: add system suspend support Sebastian Reichel
` (9 preceding siblings ...)
2026-11-04 18:56 ` [PATCH v7 10/10] PCI: dw-rockchip: Clear debug buffer before entering L2 Sebastian Reichel
@ 2026-11-04 22:44 ` Igor Velkov
10 siblings, 0 replies; 22+ messages in thread
From: Igor Velkov @ 2026-11-04 22:44 UTC (permalink / raw)
To: Sebastian Reichel
Cc: iav, Lorenzo Pieralisi, Krzysztof Wilczyński,
Manivannan Sadhasivam, Rob Herring, Bjorn Helgaas, Heiko Stuebner,
Philipp Zabel, Jingoo Han, Shawn Lin, Liam Girdwood, Mark Brown,
Yury Norov (NVIDIA), Nicolas Frattaroli, Alexey Charkov,
linux-pci, linux-arm-kernel, linux-rockchip, linux-kernel, kernel
Tested on ODROID-M1 (RK3568, NVMe on pcie3x2) with 7.3.1, Jonas'
EHCI/OHCI clock patch and f845c1e2d2d4 ("PCI: dwc: Add suspend_poweroff
flag for platforms with RC power loss") backported: s2idle 10/10,
deep 10/10, the NVMe stays usable after resume.
Tested-by: Igor Velkov <iav@iav.lv>
^ permalink raw reply [flat|nested] 22+ messages in thread