Add runtime PM support to Rockchip DW PCIe driver
Subsystems (3)
cover Add runtime PM support to Rockchip DW PCIe driver
Introduce runtime power management support in the Rockchip DesignWare PCIe controller driver. These changes allow the PCIe controller to suspend and resume dynamically, improving power efficiency on supported platforms.
Can Patch 1 can be backpoted to stable? It helps clean shutdown of PCIe.
Clarification: the series is based on
git://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git
branch : controller/dw-rockchip
Thanks
-Anand
Anand Moon (2):
PCI: dw-rockchip: Add remove callback for resource cleanup
PCI: dw-rockchip: Add runtime PM support to Rockchip PCIe driver
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+)
base-commit: 7ad31f88429369ada44710176e176256a2812c3f
Signature · 1 line
2.50.1
PATCH 1/2 PCI: dw-rockchip: Add remove callback for resource cleanup
Introduce a .remove() callback to the Rockchip DesignWare PCIe controller driver to ensure proper resource deinitialization during device removal. This includes disabling clocks and deinitializing the PCIe PHY.
--- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
drivers/pci/controller/dwc/pcie-dw-rockchip.c +11 -0
@@ -717,6 +717,16 @@ static int rockchip_pcie_probe(struct platform_device *pdev)return ret; } +static void rockchip_pcie_remove (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct rockchip_pcie *rockchip =dev_get_drvdata (dev); + +/* Perform other cleanups as necessary */ +clk_bulk_disable_unprepare (rockchip->clk_cnt, rockchip->clks); +rockchip_pcie_phy_deinit (rockchip); +} +static const struct rockchip_pcie_of_data rockchip_pcie_rc_of_data_rk3568 = { .mode =DW_PCIE_RC_TYPE , };
@@ -754,5 +764,6 @@ static struct platform_driver rockchip_pcie_driver = { .suppress_bind_attrs = true, }, .probe = rockchip_pcie_probe, + .remove = rockchip_pcie_remove, };builtin_platform_driver (rockchip_pcie_driver);
Signature · 1 line
2.50.1
Re: [PATCH v1 1/2] PCI: dw-rockchip: Add remove callback for resource cleanup
Quoted 16 lines · On Monday, 27 October 2025 15:55:29 Central European Standard Time Anand Moon…
Introduce a .remove() callback to the Rockchip DesignWare PCIe controller driver to ensure proper resource deinitialization during device removal. This includes disabling clocks and deinitializing the PCIe PHY.
Signed-off-by Anand Moon <[email protected]>--- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 11 +++++++++++ 1 file changed, 11 insertions(+)drivers/pci/controller/dwc/pcie-dw-rockchip.c +0
-0@@ -717,6 +717,16 @@ static int rockchip_pcie_probe(struct platform_device *pdev)return ret;
drivers/pci/controller/dwc/pcie-dw-rockchip.c static int rockchip_pcie_probe(struct platform_device *pdev) · line 727
Quoted 10 lines
drivers/pci/controller/dwc/pcie-dw-rockchip.c +8
-0} +static void rockchip_pcie_remove (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct rockchip_pcie *rockchip =dev_get_drvdata (dev); + +/* Perform other cleanups as necessary */ +clk_bulk_disable_unprepare (rockchip->clk_cnt, rockchip->clks); +rockchip_pcie_phy_deinit (rockchip);
You may want to add a
if (rockchip->vpcie3v3)
regulator_disable(rockchip->vpcie3v3);here, since it's enabled in the probe function if it's found.
Not doing so means the regulator core will produce a warning splat when devres removes it I'm fairly sure.
Kind regards,
Nicolas Frattaroli
Quoted 13 lines
drivers/pci/controller/dwc/pcie-dw-rockchip.c +3
-0+} +static const struct rockchip_pcie_of_data rockchip_pcie_rc_of_data_rk3568 = { .mode =DW_PCIE_RC_TYPE , };@@ -754,5 +764,6 @@ static struct platform_driver rockchip_pcie_driver = { .suppress_bind_attrs = true, }, .probe = rockchip_pcie_probe, + .remove = rockchip_pcie_remove, };builtin_platform_driver (rockchip_pcie_driver);
Re: [PATCH v1 1/2] PCI: dw-rockchip: Add remove callback for resource cleanup
Hi Nicolas,
Thanks for your review comments.
Quoted 31 lines · Nicolas Frattaroli
Quoted 16 lines · On Monday, 27 October 2025 15:55:29 Central European Standard Time Anand Moon…
Introduce a .remove() callback to the Rockchip DesignWare PCIe controller driver to ensure proper resource deinitialization during device removal. This includes disabling clocks and deinitializing the PCIe PHY.
Signed-off-by Anand Moon <[email protected]>--- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 11 +++++++++++ 1 file changed, 11 insertions(+)drivers/pci/controller/dwc/pcie-dw-rockchip.c +0
-0@@ -717,6 +717,16 @@ static int rockchip_pcie_probe(struct platform_device *pdev)return ret;drivers/pci/controller/dwc/pcie-dw-rockchip.c static int rockchip_pcie_probe(struct platform_device *pdev) · line 727
Quoted 10 lines
drivers/pci/controller/dwc/pcie-dw-rockchip.c +8
-0} +static void rockchip_pcie_remove (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct rockchip_pcie *rockchip =dev_get_drvdata (dev); + +/* Perform other cleanups as necessary */ +clk_bulk_disable_unprepare (rockchip->clk_cnt, rockchip->clks); +rockchip_pcie_phy_deinit (rockchip);You may want to add a
Quoted 8 lines
if (rockchip->vpcie3v3) regulator_disable(rockchip->vpcie3v3);here, since it's enabled in the probe function if it's found.
Not doing so means the regulator core will produce a warning splat when devres removes it I'm fairly sure.
I've removed the dependency on vpcie3v3 in the following commit:
c930b10f17c0 ("PCI: dw-rockchip: Simplify regulator setup withdevm_regulator_get_enable_optional()")
Please review this commit and confirm if everything looks good.
Quoted 3 lines
Kind regards,
Nicolas Frattaroli
Thanks
-Anand
Nicolas Frattaroli
Reviewed-byRe: [PATCH v1 1/2] PCI: dw-rockchip: Add remove callback for resource cleanup
Quoted 42 lines · On Monday, 27 October 2025 17:31:23 Central European Standard Time Anand Moon…
Hi Nicolas,
Thanks for your review comments.
Quoted 36 lines · Nicolas Frattaroli
Quoted 16 lines · On Monday, 27 October 2025 15:55:29 Central European Standard Time Anand Moon…
Introduce a .remove() callback to the Rockchip DesignWare PCIe controller driver to ensure proper resource deinitialization during device removal. This includes disabling clocks and deinitializing the PCIe PHY.
Signed-off-by Anand Moon <[email protected]>--- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 11 +++++++++++ 1 file changed, 11 insertions(+)drivers/pci/controller/dwc/pcie-dw-rockchip.c +0
-0@@ -717,6 +717,16 @@ static int rockchip_pcie_probe(struct platform_device *pdev)return ret;drivers/pci/controller/dwc/pcie-dw-rockchip.c static int rockchip_pcie_probe(struct platform_device *pdev) · line 727
Quoted 10 lines
drivers/pci/controller/dwc/pcie-dw-rockchip.c +8
-0} +static void rockchip_pcie_remove (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct rockchip_pcie *rockchip =dev_get_drvdata (dev); + +/* Perform other cleanups as necessary */ +clk_bulk_disable_unprepare (rockchip->clk_cnt, rockchip->clks); +rockchip_pcie_phy_deinit (rockchip);You may want to add a
if (rockchip->vpcie3v3) regulator_disable(rockchip->vpcie3v3);here, since it's enabled in the probe function if it's found.
Quoted 7 lines
Quoted 3 lines
Not doing so means the regulator core will produce a warning splat when devres removes it I'm fairly sure.
I've removed the dependency on vpcie3v3 in the following commit:
c930b10f17c0("PCI: dw-rockchip: Simplify regulator setup withdevm_regulator_get_enable_optional()")
Please review this commit and confirm if everything looks good.
I see. In that case, your code is indeed correct, thank you for pointing this out.
Kind regards,
Nicolas Frattaroli
Quoted 7 lines
Quoted 4 lines
Kind regards,
Nicolas FrattaroliThanks
-Anand
Re: [PATCH v1 1/2] PCI: dw-rockchip: Add remove callback for resource cleanup
In reply to Anand MoonQuoted 17 lines · Anand Moon
Introduce a .remove() callback to the Rockchip DesignWare PCIe controller driver to ensure proper resource deinitialization during device removal. This includes disabling clocks and deinitializing the PCIe PHY.
Signed-off-by Anand Moon <[email protected]>--- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 11 +++++++++++ 1 file changed, 11 insertions(+)drivers/pci/controller/dwc/pcie-dw-rockchip.c +0
-0@@ -717,6 +717,16 @@ static int rockchip_pcie_probe(struct platform_device *pdev)return ret; }
drivers/pci/controller/dwc/pcie-dw-rockchip.c static int rockchip_pcie_probe(struct platform_device *pdev) · line 728
Quoted 10 lines
drivers/pci/controller/dwc/pcie-dw-rockchip.c +9
-0+static void rockchip_pcie_remove (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct rockchip_pcie *rockchip =dev_get_drvdata (dev); + +/* Perform other cleanups as necessary */ +clk_bulk_disable_unprepare (rockchip->clk_cnt, rockchip->clks); +rockchip_pcie_phy_deinit (rockchip); +}
Thanks for the patch.
Dou you need to call dw_pcie_host_deinit()?
And I think you should also try to mask PCIE_CLIENT_INTR_MASK_MISC and remove the irq domain as well.
if (rockchip->irq_domain) {
int virq, j;
for (j = 0; j < PCI_NUM_INTX; j++) {
virq = irq_find_mapping(rockchip->irq_domain, j);
if (virq > 0)
irq_dispose_mapping(virq);
}
irq_set_chained_handler_and_data(rockchip->irq, NULL, NULL);
irq_domain_remove(rockchip->irq_domain);
}Another thin I noticed is should we call pm_runtime_* here for hope that genpd could be powered donw once removed?
drivers/pci/controller/dwc/pcie-dw-rockchip.c static struct platform_driver rockchip_pcie_driver = { · line 769
Quoted 11 lines
drivers/pci/controller/dwc/pcie-dw-rockchip.c +2
-0+static const struct rockchip_pcie_of_data rockchip_pcie_rc_of_data_rk3568 = { .mode =DW_PCIE_RC_TYPE , };@@ -754,5 +764,6 @@ static struct platform_driver rockchip_pcie_driver = { .suppress_bind_attrs = true, }, .probe = rockchip_pcie_probe, + .remove = rockchip_pcie_remove, };builtin_platform_driver (rockchip_pcie_driver);
_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip
Re: [PATCH v1 1/2] PCI: dw-rockchip: Add remove callback for resource cleanup
Hi Shawn,
Thanks for your review comments.
Quoted 32 lines · Shawn Lin
Quoted 17 lines · Anand Moon
Introduce a .remove() callback to the Rockchip DesignWare PCIe controller driver to ensure proper resource deinitialization during device removal. This includes disabling clocks and deinitializing the PCIe PHY.
Signed-off-by Anand Moon <[email protected]>--- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 11 +++++++++++ 1 file changed, 11 insertions(+)drivers/pci/controller/dwc/pcie-dw-rockchip.c +0
-0@@ -717,6 +717,16 @@ static int rockchip_pcie_probe(struct platform_device *pdev)return ret; }drivers/pci/controller/dwc/pcie-dw-rockchip.c static int rockchip_pcie_probe(struct platform_device *pdev) · line 728
Quoted 10 lines
drivers/pci/controller/dwc/pcie-dw-rockchip.c +9
-0+static void rockchip_pcie_remove (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct rockchip_pcie *rockchip =dev_get_drvdata (dev); + +/* Perform other cleanups as necessary */ +clk_bulk_disable_unprepare (rockchip->clk_cnt, rockchip->clks); +rockchip_pcie_phy_deinit (rockchip); +}Thanks for the patch.
Quoted 1 line
Dou you need to call
dw_pcie_host_deinit()?
I feel the rockchip_pcie_phy_deinit will power off the phy
Quoted 14 lines
And I think you should also try to mask
PCIE_CLIENT_INTR_MASK_MISCand remove the irq domain as well.if (rockchip->irq_domain) { int virq, j; for (j = 0; j < PCI_NUM_INTX; j++) { virq = irq_find_mapping(rockchip->irq_domain, j); if (virq > 0) irq_dispose_mapping(virq); } irq_set_chained_handler_and_data(rockchip->irq, NULL, NULL); irq_domain_remove(rockchip->irq_domain); }
I have implemented resource cleanup in rockchip_pcie_remove, which is invoked when the system is shutting down. Your feedback on the updated code is welcome.
static void rockchip_pcie_remove(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
int irq;
irq = of_irq_get_byname(dev->of_node, "legacy");
if (irq < 0)
return;
/* Perform other cleanups as necessary */
/* clear up INTR staatus register */
rockchip_pcie_writel_apb(rockchip, 0xffffffff,
PCIE_CLIENT_INTR_STATUS_MISC);
if (rockchip->irq_domain) {
int virq, j;
for (j = 0; j < PCI_NUM_INTX; j++) {
virq = irq_find_mapping(rockchip->irq_domain, j);
if (virq > 0)
irq_dispose_mapping(virq);
}
irq_set_chained_handler_and_data(irq, NULL, NULL);
irq_domain_remove(rockchip->irq_domain);
}
clk_bulk_disable_unprepare(rockchip->clk_cnt, rockchip->clks);
/* poweroff the phy */
rockchip_pcie_phy_deinit(rockchip);
/* release the reset */
reset_control_assert(rockchip->rst);
pm_runtime_put_sync(dev);
pm_runtime_disable(dev);
pm_runtime_no_callbacks(dev);
}Quoted 3 lines
Another thin I noticed is should we call pm_runtime_* here for hope that genpd could be powered donw once removed?
I could not find 'genpd' (power domain) used in the PCIe driver If we have an example to use genpd I will update this.
I am also looking into adding NOIRQ_SYSTEM_SLEEP_PM_OPS
Thanks
-Anand
Re: [PATCH v1 1/2] PCI: dw-rockchip: Add remove callback for resource cleanup
Quoted 78 lines · Anand Moon
Hi Shawn,
Thanks for your review comments.
Quoted 32 lines · Shawn Lin
Quoted 17 lines · Anand Moon
Introduce a .remove() callback to the Rockchip DesignWare PCIe controller driver to ensure proper resource deinitialization during device removal. This includes disabling clocks and deinitializing the PCIe PHY.
Signed-off-by Anand Moon <[email protected]>--- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 11 +++++++++++ 1 file changed, 11 insertions(+)drivers/pci/controller/dwc/pcie-dw-rockchip.c +0
-0@@ -717,6 +717,16 @@ static int rockchip_pcie_probe(struct platform_device *pdev)return ret; }drivers/pci/controller/dwc/pcie-dw-rockchip.c static int rockchip_pcie_probe(struct platform_device *pdev) · line 728
Quoted 10 lines
drivers/pci/controller/dwc/pcie-dw-rockchip.c +9
-0+static void rockchip_pcie_remove (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct rockchip_pcie *rockchip =dev_get_drvdata (dev); + +/* Perform other cleanups as necessary */ +clk_bulk_disable_unprepare (rockchip->clk_cnt, rockchip->clks); +rockchip_pcie_phy_deinit (rockchip); +}Thanks for the patch.
Quoted 1 line
Dou you need to call
dw_pcie_host_deinit()?I feel the
rockchip_pcie_phy_deinitwill power off the phyQuoted 14 lines
And I think you should also try to mask
PCIE_CLIENT_INTR_MASK_MISCand remove the irq domain as well.if (rockchip->irq_domain) { int virq, j; for (j = 0; j < PCI_NUM_INTX; j++) { virq = irq_find_mapping(rockchip->irq_domain, j); if (virq > 0) irq_dispose_mapping(virq); } irq_set_chained_handler_and_data(rockchip->irq, NULL, NULL); irq_domain_remove(rockchip->irq_domain); }I have implemented resource cleanup in
rockchip_pcie_remove, which is invoked when the system is shutting down. Your feedback on the updated code is welcome.static void
rockchip_pcie_remove(struct platform_device *pdev){ struct device *dev = &pdev->dev; struct rockchip_pcie *rockchip = dev_get_drvdata(dev); int irq; irq = of_irq_get_byname(dev->of_node, "legacy"); if (irq < 0) return; /* Perform other cleanups as necessary */ /* clear up INTR staatus register */ rockchip_pcie_writel_apb(rockchip, 0xffffffff, PCIE_CLIENT_INTR_STATUS_MISC); if (rockchip->irq_domain) { int virq, j; for (j = 0; j < PCI_NUM_INTX; j++) { virq = irq_find_mapping(rockchip->irq_domain, j); if (virq > 0) irq_dispose_mapping(virq); }
Quoted 8 lines
irq_set_chained_handler_and_data(irq, NULL, NULL);
irq_domain_remove(rockchip->irq_domain);
}
clk_bulk_disable_unprepare(rockchip->clk_cnt, rockchip->clks);
/* poweroff the phy */
rockchip_pcie_phy_deinit(rockchip);
/* release the reset */release? Or "reset the controller"?
Quoted 6 lines
reset_control_assert(rockchip->rst);
pm_runtime_put_sync(dev);
pm_runtime_disable(dev);
pm_runtime_no_callbacks(dev);
}Quoted 6 lines
Quoted 3 lines
Another thin I noticed is should we call pm_runtime_* here for hope that genpd could be powered donw once removed?
I could not find 'genpd' (power domain) used in the PCIe driver If we have an example to use genpd I will update this.
Quoted 1 line
I am also looking into adding
NOIRQ_SYSTEM_SLEEP_PM_OPS
That sounds good, you can pick up my patch[1] if you'd like to continue addressing the comments that I haven't had time to think more.
[1] https://www.spinics.net/lists/linux-pci/msg171846.html
Quoted 4 lines
Thanks
-Anand
_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip
Re: [PATCH v1 1/2] PCI: dw-rockchip: Add remove callback for resource cleanup
Hi Shawn,
Quoted 85 lines · Shawn Lin
Quoted 82 lines · Anand Moon
Hi Shawn,
Thanks for your review comments.
Quoted 32 lines · Shawn Lin
Quoted 17 lines · Anand Moon
Introduce a .remove() callback to the Rockchip DesignWare PCIe controller driver to ensure proper resource deinitialization during device removal. This includes disabling clocks and deinitializing the PCIe PHY.
Signed-off-by Anand Moon <[email protected]>--- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 11 +++++++++++ 1 file changed, 11 insertions(+)drivers/pci/controller/dwc/pcie-dw-rockchip.c +0
-0@@ -717,6 +717,16 @@ static int rockchip_pcie_probe(struct platform_device *pdev)return ret; }drivers/pci/controller/dwc/pcie-dw-rockchip.c static int rockchip_pcie_probe(struct platform_device *pdev) · line 728
Quoted 10 lines
drivers/pci/controller/dwc/pcie-dw-rockchip.c +9
-0+static void rockchip_pcie_remove (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct rockchip_pcie *rockchip =dev_get_drvdata (dev); + +/* Perform other cleanups as necessary */ +clk_bulk_disable_unprepare (rockchip->clk_cnt, rockchip->clks); +rockchip_pcie_phy_deinit (rockchip); +}Thanks for the patch.
Quoted 1 line
Dou you need to call
dw_pcie_host_deinit()?I feel the
rockchip_pcie_phy_deinitwill power off the phyQuoted 14 lines
And I think you should also try to mask
PCIE_CLIENT_INTR_MASK_MISCand remove the irq domain as well.if (rockchip->irq_domain) { int virq, j; for (j = 0; j < PCI_NUM_INTX; j++) { virq = irq_find_mapping(rockchip->irq_domain, j); if (virq > 0) irq_dispose_mapping(virq); } irq_set_chained_handler_and_data(rockchip->irq, NULL, NULL); irq_domain_remove(rockchip->irq_domain); }I have implemented resource cleanup in
rockchip_pcie_remove, which is invoked when the system is shutting down. Your feedback on the updated code is welcome.static void
rockchip_pcie_remove(struct platform_device *pdev){ struct device *dev = &pdev->dev; struct rockchip_pcie *rockchip = dev_get_drvdata(dev); int irq; irq = of_irq_get_byname(dev->of_node, "legacy"); if (irq < 0) return; /* Perform other cleanups as necessary */ /* clear up INTR staatus register */ rockchip_pcie_writel_apb(rockchip, 0xffffffff, PCIE_CLIENT_INTR_STATUS_MISC); if (rockchip->irq_domain) { int virq, j; for (j = 0; j < PCI_NUM_INTX; j++) { virq = irq_find_mapping(rockchip->irq_domain, j); if (virq > 0) irq_dispose_mapping(virq); } irq_set_chained_handler_and_data(irq, NULL, NULL); irq_domain_remove(rockchip->irq_domain); }
Quoted 7 lines
Quoted 4 lines
clk_bulk_disable_unprepare(rockchip->clk_cnt, rockchip->clks); /* poweroff the phy */ rockchip_pcie_phy_deinit(rockchip); /* release the reset */release? Or "reset the controller"?
Ok, I will fix this.
Quoted 9 lines
Quoted 9 lines
reset_control_assert(rockchip->rst); pm_runtime_put_sync(dev); pm_runtime_disable(dev); pm_runtime_no_callbacks(dev); }Quoted 3 lines
Another thin I noticed is should we call pm_runtime_* here for hope that genpd could be powered donw once removed?
Quoted 9 lines
Quoted 3 lines
I could not find 'genpd' (power domain) used in the PCIe driver If we have an example to use genpd I will update this.
Quoted 1 line
I am also looking into adding
NOIRQ_SYSTEM_SLEEP_PM_OPSThat sounds good, you can pick up my patch[1] if you'd like to continue addressing the comments that I haven't had time to think more.
Ok, thanks. I will carefully study and address the comments.
Thanks
-Anand
Manivannan Sadhasivam
maintainerRe: [PATCH v1 1/2] PCI: dw-rockchip: Add remove callback for resource cleanup
In reply to Anand MoonQuoted 5 lines · Anand Moon
Introduce a .remove() callback to the Rockchip DesignWare PCIe controller driver to ensure proper resource deinitialization during device removal. This includes disabling clocks and deinitializing the PCIe PHY.
How can you remove a driver that is only built-in? You are just sending some pointless patches that were not tested and does not make sense at all.
Please stop wasting others time.
- Mani
Quoted 36 lines
Signed-off-by Anand Moon <[email protected]>--- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 11 +++++++++++ 1 file changed, 11 insertions(+)drivers/pci/controller/dwc/pcie-dw-rockchip.c +11
-0@@ -717,6 +717,16 @@ static int rockchip_pcie_probe(struct platform_device *pdev)return ret; } +static void rockchip_pcie_remove (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct rockchip_pcie *rockchip =dev_get_drvdata (dev); + +/* Perform other cleanups as necessary */ +clk_bulk_disable_unprepare (rockchip->clk_cnt, rockchip->clks); +rockchip_pcie_phy_deinit (rockchip); +} +static const struct rockchip_pcie_of_data rockchip_pcie_rc_of_data_rk3568 = { .mode =DW_PCIE_RC_TYPE , };@@ -754,5 +764,6 @@ static struct platform_driver rockchip_pcie_driver = { .suppress_bind_attrs = true, }, .probe = rockchip_pcie_probe, + .remove = rockchip_pcie_remove, };builtin_platform_driver (rockchip_pcie_driver);Signature · 1 line
2.50.1
Signature · 1 line
மணிவண்ணன் சதாசிவம்
Re: [PATCH v1 1/2] PCI: dw-rockchip: Add remove callback for resource cleanup
Quoted 9 lines · On Friday, 31 October 2025 09:36:05 Central European Standard Time Manivannan…
Quoted 5 lines · Anand Moon
Introduce a .remove() callback to the Rockchip DesignWare PCIe controller driver to ensure proper resource deinitialization during device removal. This includes disabling clocks and deinitializing the PCIe PHY.
How can you remove a driver that is only built-in? You are just sending some pointless patches that were not tested and does not make sense at all.
The better question would be: why does Kconfig make PCIE_ROCKCHIP_DW a bool rather than a tristate? I see other PCIE_DW drivers using tristate, so this doesn't seem like a technical limitation with the IP.
Quoted 40 lines
Please stop wasting others time.
- Mani
Quoted 33 lines
Signed-off-by Anand Moon <[email protected]>--- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 11 +++++++++++ 1 file changed, 11 insertions(+)drivers/pci/controller/dwc/pcie-dw-rockchip.c +11
-0@@ -717,6 +717,16 @@ static int rockchip_pcie_probe(struct platform_device *pdev)return ret; } +static void rockchip_pcie_remove (struct platform_device *pdev) +{ +struct device *dev = &pdev->dev; +struct rockchip_pcie *rockchip =dev_get_drvdata (dev); + +/* Perform other cleanups as necessary */ +clk_bulk_disable_unprepare (rockchip->clk_cnt, rockchip->clks); +rockchip_pcie_phy_deinit (rockchip); +} +static const struct rockchip_pcie_of_data rockchip_pcie_rc_of_data_rk3568 = { .mode =DW_PCIE_RC_TYPE , };@@ -754,5 +764,6 @@ static struct platform_driver rockchip_pcie_driver = { .suppress_bind_attrs = true, }, .probe = rockchip_pcie_probe, + .remove = rockchip_pcie_remove, };builtin_platform_driver (rockchip_pcie_driver);
Manivannan Sadhasivam
maintainerRe: [PATCH v1 1/2] PCI: dw-rockchip: Add remove callback for resource cleanup
Quoted 8 lines · Nicolas Frattaroli
Quoted 7 lines · On Friday, 31 October 2025 09:36:05 Central European Standard Time Manivannan…
Quoted 5 lines · Anand Moon
Introduce a .remove() callback to the Rockchip DesignWare PCIe controller driver to ensure proper resource deinitialization during device removal. This includes disabling clocks and deinitializing the PCIe PHY.
Quoted 8 lines
Quoted 2 lines
How can you remove a driver that is only built-in? You are just sending some pointless patches that were not tested and does not make sense at all.
The better question would be: why does
KconfigmakePCIE_ROCKCHIP_DWa bool rather than a tristate? I see otherPCIE_DWdrivers using tristate, so this doesn't seem like a technical limitation with the IP.
The limitation is due to irqchip maintainers not allowing (or recommending) to remove a controller driver which implements an irqchip. So if any controller driver that does so, we make them built-in. Recently, I suggested some controller drivers to be tristate, but without the remove() callback. This will allow the controller drivers to be built as a module, but not get removed during runtime.
The reason why an irqchip controller should not be removed during runtime is that the concerns around disposing the IRQs consumed by the client drivers. Current irqchip framework doesn't guarantee that all IRQs would be disposed when the controller is removed.
But irrespective of the above explanation, my statement was correct that this patch is pointless.
- Mani
Signature · 1 line
மணிவண்ணன் சதாசிவம்
PATCH 2/2 PCI: dw-rockchip: Add runtime PM support to Rockchip PCIe driver
In reply to Anand MoonAdd runtime power management support to the Rockchip DesignWare PCIe controller driver by enabling devm_pm_runtime() in the probe function. These changes allow the PCIe controller to suspend and resume dynamically, improving power efficiency on supported platforms.
--- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+)
drivers/pci/controller/dwc/pcie-dw-rockchip.c +21 -0
@@ -20,6 +20,7 @@#include <linux/of_irq.h> #include <linux/phy/phy.h> #include <linux/platform_device.h> +#include <linux/pm_runtime.h> #include <linux/regmap.h> #include <linux/reset.h>
@@ -690,6 +691,20 @@ static int rockchip_pcie_probe(struct platform_device *pdev)if (ret) goto deinit_phy; + ret =pm_runtime_set_suspended (dev); +if (ret) + goto disable_pm_runtime; + + ret =devm_pm_runtime_enable (dev); +if (ret) { + ret =dev_err_probe (dev, ret,"Failed to enable runtime PM\n" ); + goto deinit_clk; + } + + ret =pm_runtime_resume_and_get (dev); +if (ret) + goto disable_pm_runtime; +switch (data->mode) {case DW_PCIE_RC_TYPE : ret =rockchip_pcie_configure_rc (pdev, rockchip);
@@ -709,7 +724,10 @@ static int rockchip_pcie_probe(struct platform_device *pdev)return 0 ; +disable_pm_runtime: +pm_runtime_disable (dev); deinit_clk: +pm_runtime_no_callbacks (dev);clk_bulk_disable_unprepare (rockchip->clk_cnt, rockchip->clks); deinit_phy:rockchip_pcie_phy_deinit (rockchip);
@@ -725,6 +743,9 @@ static void rockchip_pcie_remove(struct platform_device *pdev)/* Perform other cleanups as necessary */ clk_bulk_disable_unprepare (rockchip->clk_cnt, rockchip->clks);rockchip_pcie_phy_deinit (rockchip); +pm_runtime_put_sync (dev); +pm_runtime_disable (dev); +pm_runtime_no_callbacks (dev); }static const struct rockchip_pcie_of_data rockchip_pcie_rc_of_data_rk3568 = {
Signature · 1 line
2.50.1
Re: [PATCH v1 2/2] PCI: dw-rockchip: Add runtime PM support to Rockchip PCIe driver
Quoted 44 lines · Anand Moon
Add runtime power management support to the Rockchip DesignWare PCIe controller driver by enabling
devm_pm_runtime()in the probe function. These changes allow the PCIe controller to suspend and resume dynamically, improving power efficiency on supported platforms.Signed-off-by Anand Moon <[email protected]>--- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+)drivers/pci/controller/dwc/pcie-dw-rockchip.c +15
-0@@ -20,6 +20,7 @@#include <linux/of_irq.h> #include <linux/phy/phy.h> #include <linux/platform_device.h> +#include <linux/pm_runtime.h> #include <linux/regmap.h> #include <linux/reset.h> @@ -690,6 +691,20 @@ static int rockchip_pcie_probe(struct platform_device *pdev)if (ret) goto deinit_phy; + ret =pm_runtime_set_suspended (dev); +if (ret) + goto disable_pm_runtime; + + ret =devm_pm_runtime_enable (dev); +if (ret) { + ret =dev_err_probe (dev, ret,"Failed to enable runtime PM\n" ); + goto deinit_clk; + } + + ret =pm_runtime_resume_and_get (dev); +if (ret) + goto disable_pm_runtime; +switch (data->mode) {case DW_PCIE_RC_TYPE : ret =rockchip_pcie_configure_rc (pdev, rockchip);
drivers/pci/controller/dwc/pcie-dw-rockchip.c static int rockchip_pcie_probe(struct platform_device *pdev) · line 727
Quoted 4 lines
drivers/pci/controller/dwc/pcie-dw-rockchip.c +1
-0@@ -709,7 +724,10 @@ static int rockchip_pcie_probe(struct platform_device *pdev)return 0 ; +disable_pm_runtime:
We need to call reset_control_assert(rockchip->rst) before releasing the
the pm refcount. The problem we faced on vendor kernel is there might be still on-going transaction from IP to the AXI which blocks genpd to be powered down.
Quoted 16 lines
drivers/pci/controller/dwc/pcie-dw-rockchip.c +5
-0+pm_runtime_disable (dev); deinit_clk: +pm_runtime_no_callbacks (dev);clk_bulk_disable_unprepare (rockchip->clk_cnt, rockchip->clks); deinit_phy:rockchip_pcie_phy_deinit (rockchip);@@ -725,6 +743,9 @@ static void rockchip_pcie_remove(struct platform_device *pdev)/* Perform other cleanups as necessary */ clk_bulk_disable_unprepare (rockchip->clk_cnt, rockchip->clks);rockchip_pcie_phy_deinit (rockchip); +pm_runtime_put_sync (dev); +pm_runtime_disable (dev); +pm_runtime_no_callbacks (dev); }static const struct rockchip_pcie_of_data rockchip_pcie_rc_of_data_rk3568 = {
Re: [PATCH v1 2/2] PCI: dw-rockchip: Add runtime PM support to Rockchip PCIe driver
Hi Shawn,
Thanks for your review comments.
Quoted 47 lines · Shawn Lin
Quoted 45 lines · Anand Moon
Add runtime power management support to the Rockchip DesignWare PCIe controller driver by enabling
devm_pm_runtime()in the probe function. These changes allow the PCIe controller to suspend and resume dynamically, improving power efficiency on supported platforms.Signed-off-by Anand Moon <[email protected]>--- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+)drivers/pci/controller/dwc/pcie-dw-rockchip.c +15
-0@@ -20,6 +20,7 @@#include <linux/of_irq.h> #include <linux/phy/phy.h> #include <linux/platform_device.h> +#include <linux/pm_runtime.h> #include <linux/regmap.h> #include <linux/reset.h> @@ -690,6 +691,20 @@ static int rockchip_pcie_probe(struct platform_device *pdev)if (ret) goto deinit_phy; + ret =pm_runtime_set_suspended (dev); +if (ret) + goto disable_pm_runtime; + + ret =devm_pm_runtime_enable (dev); +if (ret) { + ret =dev_err_probe (dev, ret,"Failed to enable runtime PM\n" ); + goto deinit_clk; + } + + ret =pm_runtime_resume_and_get (dev); +if (ret) + goto disable_pm_runtime; +switch (data->mode) {case DW_PCIE_RC_TYPE : ret =rockchip_pcie_configure_rc (pdev, rockchip);@@ -709,7 +724,10 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
Quoted 8 lines
drivers/pci/controller/dwc/pcie-dw-rockchip.c static int rockchip_pcie_probe(struct platform_device *pdev) · line 727
Quoted 3 lines
drivers/pci/controller/dwc/pcie-dw-rockchip.c +1
-0return 0 ; +disable_pm_runtime:We need to call
reset_control_assert(rockchip->rst)before releasing the the pm refcount. The problem we faced on vendor kernel is there might be still on-going transaction from IP to the AXI which blocks genpd to be powered down.
Thanks, I will try to fix this in the next version.
Thanks
-Anand
Manivannan Sadhasivam
maintainerRe: [PATCH v1 2/2] PCI: dw-rockchip: Add runtime PM support to Rockchip PCIe driver
In reply to Anand MoonQuoted 5 lines · Anand Moon
Add runtime power management support to the Rockchip DesignWare PCIe controller driver by enabling
devm_pm_runtime()in the probe function. These changes allow the PCIe controller to suspend and resume dynamically, improving power efficiency on supported platforms.
Seriously? How can this patch improve the power efficiency if it is not doing any PM operation on its own?
Again a pointless patch.
- Mani
Quoted 62 lines
Signed-off-by Anand Moon <[email protected]>--- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+)drivers/pci/controller/dwc/pcie-dw-rockchip.c +21
-0@@ -20,6 +20,7 @@#include <linux/of_irq.h> #include <linux/phy/phy.h> #include <linux/platform_device.h> +#include <linux/pm_runtime.h> #include <linux/regmap.h> #include <linux/reset.h> @@ -690,6 +691,20 @@ static int rockchip_pcie_probe(struct platform_device *pdev)if (ret) goto deinit_phy; + ret =pm_runtime_set_suspended (dev); +if (ret) + goto disable_pm_runtime; + + ret =devm_pm_runtime_enable (dev); +if (ret) { + ret =dev_err_probe (dev, ret,"Failed to enable runtime PM\n" ); + goto deinit_clk; + } + + ret =pm_runtime_resume_and_get (dev); +if (ret) + goto disable_pm_runtime; +switch (data->mode) {case DW_PCIE_RC_TYPE : ret =rockchip_pcie_configure_rc (pdev, rockchip);@@ -709,7 +724,10 @@ static int rockchip_pcie_probe(struct platform_device *pdev)return 0 ; +disable_pm_runtime: +pm_runtime_disable (dev); deinit_clk: +pm_runtime_no_callbacks (dev);clk_bulk_disable_unprepare (rockchip->clk_cnt, rockchip->clks); deinit_phy:rockchip_pcie_phy_deinit (rockchip);@@ -725,6 +743,9 @@ static void rockchip_pcie_remove(struct platform_device *pdev)/* Perform other cleanups as necessary */ clk_bulk_disable_unprepare (rockchip->clk_cnt, rockchip->clks);rockchip_pcie_phy_deinit (rockchip); +pm_runtime_put_sync (dev); +pm_runtime_disable (dev); +pm_runtime_no_callbacks (dev); }static const struct rockchip_pcie_of_data rockchip_pcie_rc_of_data_rk3568 = {Signature · 1 line
2.50.1
Signature · 1 line
மணிவண்ணன் சதாசிவம்
Re: [PATCH v1 2/2] PCI: dw-rockchip: Add runtime PM support to Rockchip PCIe driver
Hi Manivannan
Thanks for your review comment.
Quoted 11 lines · Manivannan Sadhasivam
Quoted 5 lines · Anand Moon
Add runtime power management support to the Rockchip DesignWare PCIe controller driver by enabling
devm_pm_runtime()in the probe function. These changes allow the PCIe controller to suspend and resume dynamically, improving power efficiency on supported platforms.Seriously? How can this patch improve the power efficiency if it is not doing any PM operation on its own?
I could verify that runtime power management is active
[root@rockpi-5b alarm]# cat/sys/devices/platform/a41000000.pcie/power/runtime_status
active
[root@rockpi-5b alarm]# find /sys -name runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/pci_bus/0004:41/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/net/enP4p65s0/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/net/enP4p65s0/enP4p65s0-3::lan/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/net/enP4p65s0/enP4p65s0-2::lan/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/net/enP4p65s0/enP4p65s0-1::lan/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/net/enP4p65s0/enP4p65s0-0::lan/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/mdio_bus/r8169-4-4100/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/mdio_bus/r8169-4-4100/r8169-4-4100:00/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/mdio_bus/r8169-4-4100/r8169-4-4100:00/hwmon/hwmon11/power/runtime_status
Well, the powertop shows that the runtime power management is enabled on Radxa Rock 5b,
PowerTOP 2.15 Overview Idle stats Frequency stats Device
stats Device Freq stats Tunables WakeUp
Quoted 1 line
Quoted 1 line
Good Wireless Power Saving for interface wlan0
Good VM writeback timeout Good Bluetooth device interface status Good NMI watchdog should be turned off Good Autosuspend for unknown USB device 2-1.3 (8087:0a2b) Good Autosuspend for USB device USB 2.0 Hub [2-1] Good Autosuspend for USB device Generic Platform OHCI
controller [usb1]
Good Autosuspend for USB device xHCI Host Controller [usb8]
Good Autosuspend for USB device Generic Platform OHCI
controller [usb4]
Good Autosuspend for USB device EHCI Host Controller [usb2] Good Autosuspend for USB device xHCI Host Controller [usb6] Good Autosuspend for USB device EHCI Host Controller [usb3] Good Autosuspend for USB device xHCI Host Controller [usb5] Good Autosuspend for USB device xHCI Host Controller [usb7] Good Runtime PM for PCI Device Intel Corporation Wireless 8265 / 8275 Good Runtime PM for PCI Device Rockchip Electronics Co., Ltd RK3588 Good Runtime PM for PCI Device Rockchip Electronics Co., Ltd RK3588 Good Runtime PM for PCI Device Realtek Semiconductor Co.,
Ltd. RTL8125 2.5GbE Controller
Good Runtime PM for PCI Device Rockchip Electronics Co., Ltd RK3588
Good Runtime PM for PCI Device Samsung Electronics Co Ltd
NVMe SSD Controller SM981/PM981/PM983
PowerTOP 2.15 Overview Idle stats Frequency stats Device
stats Device Freq stats Tunables WakeUp
Usage Device name
1.1% CPU use
100.0% Radio device: rfkill_gpio
100.0% runtime-rockchip-gate-link-clk.712
100.0% PCI Device: Realtek Semiconductor Co., Ltd.
RTL8125 2.5GbE Controller
100.0% runtime-rockchip-gate-link-clk.717
100.0% runtime-rockchip-gate-link-clk.714
100.0% runtime-rockchip-gate-link-clk.489
100.0% runtime-a40000000.pcie
100.0% runtime-a40800000.pcie
100.0% runtime-rockchip-gate-link-clk.718
100.0% runtime-rockchip-gate-link-clk.706
100.0% runtime-rockchip-gate-link-clk.708
100.0% PCI Device: Intel Corporation Wireless 8265 / 8275
100.0% Radio device: btusb
100.0% runtime-fcd00000.usb
100.0% PCI Device: Samsung Electronics Co Ltd NVMe
SSD Controller SM981/PM981/PM983
100.0% Radio device: rfkill_gpio
100.0% runtime-fc000000.usb
100.0% Radio device: iwlwifi
100.0% PCI Device: Rockchip Electronics Co., Ltd RK3588
100.0% PCI Device: Rockchip Electronics Co., Ltd RK3588
100.0% PCI Device: Rockchip Electronics Co., Ltd RK3588
100.0% runtime-rockchip-gate-link-clk.711
100.0% runtime-fc400000.usb
100.0% runtime-rockchip-gate-link-clk.704
100.0% runtime-rockchip-gate-link-clk.701
100.0% runtime-rockchip-gate-link-clk.716
100.0% runtime-rockchip-gate-link-clk.707
100.0% runtime-rockchip-gate-link-clk.709
100.0% runtime-rockchip-gate-link-clk.719
100.0% runtime-xhci-hcd.1.auto
100.0% runtime-feb50000.serial
100.0% runtime-rockchip-gate-link-clk.715
100.0% runtime-rockchip-gate-link-clk.710Quoted 1 line
Again, a pointless patch.
I implemented a .remove patch to ensure proper resource cleanup, which is a necessary step for successfully enabling and managing runtime power for the device.
Quoted 2 lines
- Mani
Thanks
-Anand
Manivannan Sadhasivam
maintainerRe: [PATCH v1 2/2] PCI: dw-rockchip: Add runtime PM support to Rockchip PCIe driver
Quoted 13 lines · Anand Moon
Hi Manivannan
Thanks for your review comment.
Quoted 8 lines · Manivannan Sadhasivam
Quoted 5 lines · Anand Moon
Add runtime power management support to the Rockchip DesignWare PCIe controller driver by enabling
devm_pm_runtime()in the probe function. These changes allow the PCIe controller to suspend and resume dynamically, improving power efficiency on supported platforms.
Quoted 5 lines
Quoted 3 lines
Seriously? How can this patch improve the power efficiency if it is not doing any PM operation on its own?
I could verify that runtime power management is active
This is runtime status being active, which is a different thing as it only allows the runtime PM hierarchy to be maintained. But the way you described the commit message sounds like the patch is enabling runtime PM of the controller and that improves efficiency (as if the controller driver is actively doing runtime PM operations).
Quoted 78 lines
[root@rockpi-5b alarm]# cat
/sys/devices/platform/a41000000.pcie/power/runtime_status
active
[root@rockpi-5b alarm]# find /sys -nameruntime_status/sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/pci_bus/0004:41/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/net/enP4p65s0/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/net/enP4p65s0/enP4p65s0-3::lan/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/net/enP4p65s0/enP4p65s0-2::lan/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/net/enP4p65s0/enP4p65s0-1::lan/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/net/enP4p65s0/enP4p65s0-0::lan/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/mdio_bus/r8169-4-4100/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/mdio_bus/r8169-4-4100/r8169-4-4100:00/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/mdio_bus/r8169-4-4100/r8169-4-4100:00/hwmon/hwmon11/power/runtime_statusWell, the powertop shows that the runtime power management is enabled on Radxa Rock 5b,
PowerTOP 2.15 Overview Idle stats Frequency stats Device
stats Device Freq stats Tunables WakeUpQuoted 1 line
Quoted 1 line
Good Wireless Power Saving for interface wlan0
Good VM writeback timeout Good Bluetooth device interface status Good NMI watchdog should be turned off Good Autosuspend for unknown USB device 2-1.3 (8087:0a2b) Good Autosuspend for USB device USB 2.0 Hub [2-1] Good Autosuspend for USB device Generic Platform OHCIcontroller [usb1]
Good Autosuspend for USB device xHCI Host Controller [usb8]
Good Autosuspend for USB device Generic Platform OHCI
controller [usb4]Good Autosuspend for USB device EHCI Host Controller [usb2] Good Autosuspend for USB device xHCI Host Controller [usb6] Good Autosuspend for USB device EHCI Host Controller [usb3] Good Autosuspend for USB device xHCI Host Controller [usb5] Good Autosuspend for USB device xHCI Host Controller [usb7] Good Runtime PM for PCI Device Intel Corporation Wireless 8265 / 8275 Good Runtime PM for PCI Device Rockchip Electronics Co., Ltd RK3588 Good Runtime PM for PCI Device Rockchip Electronics Co., Ltd RK3588 Good Runtime PM for PCI Device Realtek Semiconductor Co.,Ltd. RTL8125 2.5GbE Controller
Good Runtime PM for PCI Device Rockchip Electronics Co., Ltd RK3588
Good Runtime PM for PCI Device Samsung Electronics Co Ltd
NVMe SSD Controller SM981/PM981/PM983PowerTOP 2.15 Overview Idle stats Frequency stats Device
stats Device Freq stats Tunables WakeUp
Usage Device name
1.1% CPU use
100.0% Radio device:rfkill_gpio
100.0% runtime-rockchip-gate-link-clk.712
100.0% PCI Device: Realtek Semiconductor Co., Ltd.
RTL8125 2.5GbE Controller100.0% runtime-rockchip-gate-link-clk.717 100.0% runtime-rockchip-gate-link-clk.714 100.0% runtime-rockchip-gate-link-clk.489 100.0% runtime-a40000000.pcie 100.0% runtime-a40800000.pcie 100.0% runtime-rockchip-gate-link-clk.718 100.0% runtime-rockchip-gate-link-clk.706 100.0% runtime-rockchip-gate-link-clk.708 100.0% PCI Device: Intel Corporation Wireless 8265 / 8275 100.0% Radio device: btusb 100.0% runtime-fcd00000.usb 100.0% PCI Device: Samsung Electronics Co Ltd NVMe SSD Controller SM981/PM981/PM983 100.0% Radio device: rfkill_gpio 100.0% runtime-fc000000.usb 100.0% Radio device: iwlwifi 100.0% PCI Device: Rockchip Electronics Co., Ltd RK3588 100.0% PCI Device: Rockchip Electronics Co., Ltd RK3588 100.0% PCI Device: Rockchip Electronics Co., Ltd RK3588 100.0% runtime-rockchip-gate-link-clk.711 100.0% runtime-fc400000.usb 100.0% runtime-rockchip-gate-link-clk.704 100.0% runtime-rockchip-gate-link-clk.701
Quoted 10 lines
100.0% runtime-rockchip-gate-link-clk.716 100.0% runtime-rockchip-gate-link-clk.707 100.0% runtime-rockchip-gate-link-clk.709 100.0% runtime-rockchip-gate-link-clk.719 100.0% runtime-xhci-hcd.1.auto 100.0% runtime-feb50000.serial 100.0% runtime-rockchip-gate-link-clk.715 100.0% runtime-rockchip-gate-link-clk.710Quoted 1 line
Again, a pointless patch.
This patch might make sense on its own, to enable runtime PM status of the controller so that the runtime PM could be applied to the entire PCIe hierarchy.
Quoted 3 lines
I implemented a .remove patch to ensure proper resource cleanup, which is a necessary step for successfully enabling and managing runtime power for the device.
How a dead code (remove callback for a always built-in driver) becomes necessary for runtime PM.
- Mani
Signature · 1 line
மணிவண்ணன் சதாசிவம்
Re: [PATCH v1 2/2] PCI: dw-rockchip: Add runtime PM support to Rockchip PCIe driver
Hi Manivannan
Quoted 114 lines · Manivannan Sadhasivam
Quoted 13 lines · Anand Moon
Hi Manivannan
Thanks for your review comment.
Quoted 8 lines · Manivannan Sadhasivam
Quoted 5 lines · Anand Moon
Add runtime power management support to the Rockchip DesignWare PCIe controller driver by enabling
devm_pm_runtime()in the probe function. These changes allow the PCIe controller to suspend and resume dynamically, improving power efficiency on supported platforms.Quoted 5 lines
Quoted 3 lines
Seriously? How can this patch improve the power efficiency if it is not doing any PM operation on its own?
I could verify that runtime power management is active
This is runtime status being active, which is a different thing as it only allows the runtime PM hierarchy to be maintained. But the way you described the commit message sounds like the patch is enabling runtime PM of the controller and that improves efficiency (as if the controller driver is actively doing runtime PM operations).
Quoted 87 lines
[root@rockpi-5b alarm]# cat
/sys/devices/platform/a41000000.pcie/power/runtime_status
active
[root@rockpi-5b alarm]# find /sys -nameruntime_status/sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/pci_bus/0004:41/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/net/enP4p65s0/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/net/enP4p65s0/enP4p65s0-3::lan/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/net/enP4p65s0/enP4p65s0-2::lan/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/net/enP4p65s0/enP4p65s0-1::lan/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/net/enP4p65s0/enP4p65s0-0::lan/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/mdio_bus/r8169-4-4100/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/mdio_bus/r8169-4-4100/r8169-4-4100:00/power/runtime_status /sys/devices/platform/a41000000.pcie/pci0004:40/0004:40:00.0/0004:41:00.0/mdio_bus/r8169-4-4100/r8169-4-4100:00/hwmon/hwmon11/power/runtime_statusWell, the powertop shows that the runtime power management is enabled on Radxa Rock 5b,
PowerTOP 2.15 Overview Idle stats Frequency stats Device
stats Device Freq stats Tunables WakeUpQuoted 1 line
Quoted 1 line
Good Wireless Power Saving for interface wlan0
Good VM writeback timeout Good Bluetooth device interface status Good NMI watchdog should be turned off Good Autosuspend for unknown USB device 2-1.3 (8087:0a2b) Good Autosuspend for USB device USB 2.0 Hub [2-1] Good Autosuspend for USB device Generic Platform OHCIcontroller [usb1]
Good Autosuspend for USB device xHCI Host Controller [usb8]
Good Autosuspend for USB device Generic Platform OHCI
controller [usb4]Good Autosuspend for USB device EHCI Host Controller [usb2] Good Autosuspend for USB device xHCI Host Controller [usb6] Good Autosuspend for USB device EHCI Host Controller [usb3] Good Autosuspend for USB device xHCI Host Controller [usb5] Good Autosuspend for USB device xHCI Host Controller [usb7] Good Runtime PM for PCI Device Intel Corporation Wireless 8265 / 8275 Good Runtime PM for PCI Device Rockchip Electronics Co., Ltd RK3588 Good Runtime PM for PCI Device Rockchip Electronics Co., Ltd RK3588 Good Runtime PM for PCI Device Realtek Semiconductor Co.,Ltd. RTL8125 2.5GbE Controller
Good Runtime PM for PCI Device Rockchip Electronics Co., Ltd RK3588
Good Runtime PM for PCI Device Samsung Electronics Co Ltd
NVMe SSD Controller SM981/PM981/PM983PowerTOP 2.15 Overview Idle stats Frequency stats Device
stats Device Freq stats Tunables WakeUp
Usage Device name
1.1% CPU use
100.0% Radio device:rfkill_gpio
100.0% runtime-rockchip-gate-link-clk.712
100.0% PCI Device: Realtek Semiconductor Co., Ltd.
RTL8125 2.5GbE Controller100.0% runtime-rockchip-gate-link-clk.717 100.0% runtime-rockchip-gate-link-clk.714 100.0% runtime-rockchip-gate-link-clk.489 100.0% runtime-a40000000.pcie 100.0% runtime-a40800000.pcie 100.0% runtime-rockchip-gate-link-clk.718 100.0% runtime-rockchip-gate-link-clk.706 100.0% runtime-rockchip-gate-link-clk.708 100.0% PCI Device: Intel Corporation Wireless 8265 / 8275 100.0% Radio device: btusb 100.0% runtime-fcd00000.usb 100.0% PCI Device: Samsung Electronics Co Ltd NVMe SSD Controller SM981/PM981/PM983 100.0% Radio device: rfkill_gpio 100.0% runtime-fc000000.usb 100.0% Radio device: iwlwifi 100.0% PCI Device: Rockchip Electronics Co., Ltd RK3588 100.0% PCI Device: Rockchip Electronics Co., Ltd RK3588 100.0% PCI Device: Rockchip Electronics Co., Ltd RK3588 100.0% runtime-rockchip-gate-link-clk.711 100.0% runtime-fc400000.usb 100.0% runtime-rockchip-gate-link-clk.704 100.0% runtime-rockchip-gate-link-clk.701 100.0% runtime-rockchip-gate-link-clk.716 100.0% runtime-rockchip-gate-link-clk.707 100.0% runtime-rockchip-gate-link-clk.709 100.0% runtime-rockchip-gate-link-clk.719 100.0% runtime-xhci-hcd.1.auto 100.0% runtime-feb50000.serial 100.0% runtime-rockchip-gate-link-clk.715 100.0% runtime-rockchip-gate-link-clk.710
Quoted 5 lines
Quoted 1 line
Quoted 1 line
Again, a pointless patch.
This patch might make sense on its own, to enable runtime PM status of the controller so that the runtime PM could be applied to the entire PCIe hierarchy.
I will update this in the commit message.
Quoted 7 lines
Quoted 3 lines
I implemented a .remove patch to ensure proper resource cleanup, which is a necessary step for successfully enabling and managing runtime power for the device.
How a dead code (remove callback for a always built-in driver) becomes necessary for runtime PM.
Ok, understood we don't have platform_driver_unregister in builtin_platform_driver
I will drop the first patch.
Quoted 2 lines
- Mani
Thanks
-Annad
Quoted 2 lines
--
மணிவண்ணன் சதாசிவம்
Quoted 5 lines · On Monday, 27 October 2025 15:55:28 Central European Standard Time Anand Moon…
Introduce runtime power management support in the Rockchip DesignWare PCIe controller driver. These changes allow the PCIe controller to suspend and resume dynamically, improving power efficiency on supported platforms.
Can Patch 1 can be backpoted to stable? It helps clean shutdown of PCIe.
You can do this by adding a Fixes tag to your patch. In your case, it might be fixing whatever introduced the clk_bulk_prepare_enable, i.e.:
This would be put above your Signed-off-by in the first patch, after the empty line.
To generate fixes tags like this, I use the following pretty format in my .git/config:
[pretty]
fixes = Fixes: %h (\"%s\")I can then do git log --pretty=fixes to show commits formatted the right way. To find which commit to pick, git blame and some sleuthing are helpful.
With this tag, stable bots can pick the commit into any release that the commit it fixes is in.
Kind regards,
Nicolas Frattaroli
Quoted 19 lines
Clarification: the series is based on
git://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git
branch : controller/dw-rockchipThanks
-AnandAnand Moon (2):
PCI: dw-rockchip: Add remove callback for resource cleanup
PCI: dw-rockchip: Add runtime PM support to Rockchip PCIe driverdrivers/pci/controller/dwc/pcie-dw-rockchip.c | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+)base-commit:
7ad31f88429369ada44710176e176256a2812c3f
Hi Nicolas,
Quoted 22 lines · Nicolas Frattaroli
Quoted 5 lines · On Monday, 27 October 2025 15:55:28 Central European Standard Time Anand Moon…
Introduce runtime power management support in the Rockchip DesignWare PCIe controller driver. These changes allow the PCIe controller to suspend and resume dynamically, improving power efficiency on supported platforms.
Can Patch 1 can be backpoted to stable? It helps clean shutdown of PCIe.
You can do this by adding a Fixes tag to your patch. In your case, it might be fixing whatever introduced the
clk_bulk_prepare_enable, i.e.:Fixes 0e898eb8df4e ("PCI: rockchip-dwc: Add Rockchip RK356X host controller driver")This would be put above your Signed-off-by in the first patch, after the empty line.
To generate fixes tags like this, I use the following pretty format in my .git/config:
[pretty] fixes = Fixes: %h (\"%s\")
Quoted 7 lines
I can then do
git log --pretty=fixesto show commits formatted the right way. To find which commit to pick,git blameand some sleuthing are helpful.With this tag, stable bots can pick the commit into any release that the commit it fixes is in.
Thanks for your input.
I will leave this to the maintainers to decide on this.
Quoted 3 lines
Kind regards,
Nicolas Frattaroli
Thanks
-Anand