* [PATCH 0/2] Manage the PCIe supplies with pwrctrl @ 2026-10-27 7:55 ` Shawn Lin 0 siblings, 0 replies; 8+ messages in thread From: Shawn Lin @ 2026-10-27 7:55 UTC (permalink / raw) To: Bjorn Helgaas, Manivannan Sadhasivam, Heiko Stuebner Cc: linux-pci, linux-rockchip, Shawn Lin The Rockchip PCIe host controller describes a single vpcie3v3 supply on the controller node to power whatever is plugged into the slot. That is enough for a board that only ever has one device there, but it cannot express per-device supplies, and it does not fit boards where the slot power is owned by a separate power sequencing or switch device. This series switches the pcie-dw-rockchip driver to the generic PCI pwrctrl framework, which lets the supplies of the devices below a host controller be described in their own DT nodes and be enabled before the PHY is initialized. Patch 1 makes the driver create and power on the pwrctrl devices below the controller ahead of the PHY init. The controller level vpcie3v3 supply is kept for backward compatibility, so existing boards keep working unchanged. Patch 2 moves the supply of the RK3588 EVB1 slot into a child node that describes the slot as a PCI bridge. Shawn Lin (2): PCI: dw-rockchip: Power on devices below the RC using pwrctrl arm64: dts: rockchip: rk3588-evb1: describe the PCIe3x4 slot supply .../boot/dts/rockchip/rk3588-evb1-v10.dts | 14 ++++++++- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 30 ++++++++++++++++--- 2 files changed, 39 insertions(+), 5 deletions(-) -- 2.25.1 _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 0/2] Manage the PCIe supplies with pwrctrl @ 2026-10-27 7:55 ` Shawn Lin 0 siblings, 0 replies; 8+ messages in thread From: Shawn Lin @ 2026-10-27 7:55 UTC (permalink / raw) To: Bjorn Helgaas, Manivannan Sadhasivam, Heiko Stuebner Cc: linux-pci, linux-rockchip, Shawn Lin The Rockchip PCIe host controller describes a single vpcie3v3 supply on the controller node to power whatever is plugged into the slot. That is enough for a board that only ever has one device there, but it cannot express per-device supplies, and it does not fit boards where the slot power is owned by a separate power sequencing or switch device. This series switches the pcie-dw-rockchip driver to the generic PCI pwrctrl framework, which lets the supplies of the devices below a host controller be described in their own DT nodes and be enabled before the PHY is initialized. Patch 1 makes the driver create and power on the pwrctrl devices below the controller ahead of the PHY init. The controller level vpcie3v3 supply is kept for backward compatibility, so existing boards keep working unchanged. Patch 2 moves the supply of the RK3588 EVB1 slot into a child node that describes the slot as a PCI bridge. Shawn Lin (2): PCI: dw-rockchip: Power on devices below the RC using pwrctrl arm64: dts: rockchip: rk3588-evb1: describe the PCIe3x4 slot supply .../boot/dts/rockchip/rk3588-evb1-v10.dts | 14 ++++++++- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 30 ++++++++++++++++--- 2 files changed, 39 insertions(+), 5 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] PCI: dw-rockchip: Power on devices below the RC using pwrctrl 2026-10-27 7:55 ` Shawn Lin @ 2026-10-27 7:55 ` Shawn Lin -1 siblings, 0 replies; 8+ messages in thread From: Shawn Lin @ 2026-10-27 7:55 UTC (permalink / raw) To: Bjorn Helgaas, Manivannan Sadhasivam, Heiko Stuebner Cc: linux-pci, linux-rockchip, Shawn Lin Devices below the Rockchip PCIe host controller may need resources (regulators, clocks) enabled before they can be detected on the bus. So far the driver only enabled a single vpcie3v3 supply described by the controller node itself, which cannot express per-device or per-slot supplies. Use the PCI pwrctrl framework: before initializing the PHY, create the pwrctrl devices for the DT nodes below the controller and power them on. The supplies of the slot must be up before PHY init, so this has to run ahead of rockchip_pcie_phy_init(). The controller node supply is kept for backward compatibility: boards that still describe vpcie3v3 on the controller keep working, and boards that move the supply into a child node get it enabled by pwrctrl. Only the RC mode is affected, the endpoint does not own the supplies of the devices below it. Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> --- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 30 ++++++++++++++++--- 1 file changed, 26 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..952b4c6cc4bf 100644 --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c @@ -19,6 +19,7 @@ #include <linux/of.h> #include <linux/of_irq.h> #include <linux/phy/phy.h> +#include <linux/pci-pwrctrl.h> #include <linux/platform_device.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h> @@ -841,16 +842,32 @@ static int rockchip_pcie_probe(struct platform_device *pdev) if (ret) return ret; + /* + * Power on the devices described below this controller before the PHY + * is initialized: the slot supplies must be up before PHY init. + */ + if (data->mode == DW_PCIE_RC_TYPE) { + ret = pci_pwrctrl_create_devices(dev); + if (ret) + return dev_err_probe(dev, ret, + "failed to create pwrctrl devices\n"); + + ret = pci_pwrctrl_power_on_devices(dev); + if (ret) { + pci_pwrctrl_destroy_devices(dev); + return dev_err_probe(dev, ret, + "failed to power on pwrctrl devices\n"); + } + } + /* 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"); + goto err_pwrctrl_off; ret = rockchip_pcie_phy_init(rockchip); if (ret) - return dev_err_probe(dev, ret, - "failed to initialize the phy\n"); + goto err_pwrctrl_off; ret = reset_control_deassert(rockchip->rst); if (ret) @@ -883,6 +900,11 @@ 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); +err_pwrctrl_off: + if (data->mode == DW_PCIE_RC_TYPE) { + pci_pwrctrl_power_off_devices(dev); + pci_pwrctrl_destroy_devices(dev); + } return ret; } -- 2.25.1 _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 1/2] PCI: dw-rockchip: Power on devices below the RC using pwrctrl @ 2026-10-27 7:55 ` Shawn Lin 0 siblings, 0 replies; 8+ messages in thread From: Shawn Lin @ 2026-10-27 7:55 UTC (permalink / raw) To: Bjorn Helgaas, Manivannan Sadhasivam, Heiko Stuebner Cc: linux-pci, linux-rockchip, Shawn Lin Devices below the Rockchip PCIe host controller may need resources (regulators, clocks) enabled before they can be detected on the bus. So far the driver only enabled a single vpcie3v3 supply described by the controller node itself, which cannot express per-device or per-slot supplies. Use the PCI pwrctrl framework: before initializing the PHY, create the pwrctrl devices for the DT nodes below the controller and power them on. The supplies of the slot must be up before PHY init, so this has to run ahead of rockchip_pcie_phy_init(). The controller node supply is kept for backward compatibility: boards that still describe vpcie3v3 on the controller keep working, and boards that move the supply into a child node get it enabled by pwrctrl. Only the RC mode is affected, the endpoint does not own the supplies of the devices below it. Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> --- drivers/pci/controller/dwc/pcie-dw-rockchip.c | 30 ++++++++++++++++--- 1 file changed, 26 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..952b4c6cc4bf 100644 --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c @@ -19,6 +19,7 @@ #include <linux/of.h> #include <linux/of_irq.h> #include <linux/phy/phy.h> +#include <linux/pci-pwrctrl.h> #include <linux/platform_device.h> #include <linux/regmap.h> #include <linux/regulator/consumer.h> @@ -841,16 +842,32 @@ static int rockchip_pcie_probe(struct platform_device *pdev) if (ret) return ret; + /* + * Power on the devices described below this controller before the PHY + * is initialized: the slot supplies must be up before PHY init. + */ + if (data->mode == DW_PCIE_RC_TYPE) { + ret = pci_pwrctrl_create_devices(dev); + if (ret) + return dev_err_probe(dev, ret, + "failed to create pwrctrl devices\n"); + + ret = pci_pwrctrl_power_on_devices(dev); + if (ret) { + pci_pwrctrl_destroy_devices(dev); + return dev_err_probe(dev, ret, + "failed to power on pwrctrl devices\n"); + } + } + /* 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"); + goto err_pwrctrl_off; ret = rockchip_pcie_phy_init(rockchip); if (ret) - return dev_err_probe(dev, ret, - "failed to initialize the phy\n"); + goto err_pwrctrl_off; ret = reset_control_deassert(rockchip->rst); if (ret) @@ -883,6 +900,11 @@ 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); +err_pwrctrl_off: + if (data->mode == DW_PCIE_RC_TYPE) { + pci_pwrctrl_power_off_devices(dev); + pci_pwrctrl_destroy_devices(dev); + } return ret; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] PCI: dw-rockchip: Power on devices below the RC using pwrctrl 2026-10-27 7:55 ` Shawn Lin (?) @ 2026-10-27 8:08 ` sashiko-bot -1 siblings, 0 replies; 8+ messages in thread From: sashiko-bot @ 2026-10-27 8:08 UTC (permalink / raw) To: Shawn Lin; +Cc: linux-pci Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] PCI: dw-rockchip: pwrctrl devices destroyed on -EPROBE_DEFER in probe - [Medium] PCI: dw-rockchip: link failure when CONFIG_PCI_PWRCTRL=m -- commit f0cd23e8e46914fb711657df22bcc2cf90053573 Author: Shawn Lin <shawn.lin@rock-chips.com> PCI: dw-rockchip: Power on devices below the RC using pwrctrl This commit updates rockchip_pcie_probe() to create and power on PCI pwrctrl devices for DT child nodes below the Rockchip PCIe host controller before initializing the PHY when operating in RC mode. > diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c > index bd86e32cc2b15..952b4c6cc4bf2 100644 > --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c > +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c > @@ -19,6 +19,7 @@ > #include <linux/of.h> > #include <linux/of_irq.h> > #include <linux/phy/phy.h> > +#include <linux/pci-pwrctrl.h> > #include <linux/platform_device.h> > #include <linux/regmap.h> > #include <linux/regulator/consumer.h> > @@ -841,16 +842,32 @@ static int rockchip_pcie_probe(struct platform_device *pdev) > if (ret) > return ret; > > + /* > + * Power on the devices described below this controller before the PHY > + * is initialized: the slot supplies must be up before PHY init. > + */ > + if (data->mode == DW_PCIE_RC_TYPE) { > + ret = pci_pwrctrl_create_devices(dev); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to create pwrctrl devices\n"); [Severity: Medium] Can this cause a vmlinux link failure when CONFIG_PCIE_ROCKCHIP_DW=y and CONFIG_PCI_PWRCTRL=m? In drivers/pci/controller/dwc/Kconfig, PCIE_ROCKCHIP_DW, PCIE_ROCKCHIP_DW_HOST, and PCIE_ROCKCHIP_DW_EP are bool options: config PCIE_ROCKCHIP_DW bool config PCIE_ROCKCHIP_DW_HOST bool "Rockchip DesignWare PCIe controller (host mode)" depends on PCI_MSI depends on ARCH_ROCKCHIP || COMPILE_TEST depends on OF select PCIE_DW_HOST select PCIE_ROCKCHIP_DW select PCI_HOST_COMMON In include/linux/pci-pwrctrl.h, the declarations for pci_pwrctrl_create_devices(), pci_pwrctrl_destroy_devices(), pci_pwrctrl_power_on_devices(), and pci_pwrctrl_power_off_devices() are guarded by #if IS_ENABLED(CONFIG_PCI_PWRCTRL): #if IS_ENABLED(CONFIG_PCI_PWRCTRL) int pci_pwrctrl_create_devices(struct device *parent); void pci_pwrctrl_destroy_devices(struct device *parent); int pci_pwrctrl_power_on_devices(struct device *parent); void pci_pwrctrl_power_off_devices(struct device *parent); Because data->mode == DW_PCIE_RC_TYPE in rockchip_pcie_probe() is a runtime check, built-in pcie-dw-rockchip.o emits references to the pci_pwrctrl_* symbols whenever PCIE_ROCKCHIP_DW_HOST=y or PCIE_ROCKCHIP_DW_EP=y is enabled, failing to link when pci-pwrctrl-core is built as a module (and compiling to no-op stubs if CONFIG_PCI_PWRCTRL is disabled). Should PCIE_ROCKCHIP_DW (or PCIE_ROCKCHIP_DW_HOST and PCIE_ROCKCHIP_DW_EP) select PCI_PWRCTRL_GENERIC in drivers/pci/controller/dwc/Kconfig, similar to PCI_IMX6_HOST, PCI_IMX6_EP, and PCIE_QCOM? > + > + ret = pci_pwrctrl_power_on_devices(dev); > + if (ret) { > + pci_pwrctrl_destroy_devices(dev); > + return dev_err_probe(dev, ret, > + "failed to power on pwrctrl devices\n"); > + } > + } > + > /* 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"); > + goto err_pwrctrl_off; > > ret = rockchip_pcie_phy_init(rockchip); > if (ret) > - return dev_err_probe(dev, ret, > - "failed to initialize the phy\n"); > + goto err_pwrctrl_off; > > ret = reset_control_deassert(rockchip->rst); > if (ret) > @@ -883,6 +900,11 @@ 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); > +err_pwrctrl_off: > + if (data->mode == DW_PCIE_RC_TYPE) { > + pci_pwrctrl_power_off_devices(dev); > + pci_pwrctrl_destroy_devices(dev); > + } [Severity: High] Does calling pci_pwrctrl_destroy_devices(dev) unconditionally in rockchip_pcie_probe() when ret == -EPROBE_DEFER prevent deferred child pwrctrl devices from binding and re-triggering probe? When pci_pwrctrl_power_on_devices(dev) is called in rockchip_pcie_probe(), drivers/pci/pwrctrl/core.c:pci_pwrctrl_power_on_device() checks whether each created child pwrctrl platform device has bound to its driver: scoped_guard(device, &pdev->dev) { if (device_is_bound(&pdev->dev)) { ret = __pci_pwrctrl_power_on_device(&pdev->dev); } else { /* FIXME: Use blocking wait instead of probe deferral */ dev_dbg(&pdev->dev, "driver is not bound\n"); ret = -EPROBE_DEFER; } } If a child pwrctrl device has not bound yet (for example because its supplier deferred probe, its driver probes asynchronously, or its module is not yet loaded), pci_pwrctrl_power_on_devices(dev) returns -EPROBE_DEFER (or a subsequent step in rockchip_pcie_probe() such as devm_regulator_get_enable_optional() or rockchip_pcie_phy_init() may return -EPROBE_DEFER and jump to err_pwrctrl_off). Both the error path of pci_pwrctrl_power_on_devices(dev) and the err_pwrctrl_off cleanup block call pci_pwrctrl_destroy_devices(dev) unconditionally, which unregisters the child pwrctrl platform devices via of_device_unregister() and removes them from deferred_probe_pending_list so they can never bind and re-trigger rockchip_pcie_probe(). Should pci_pwrctrl_destroy_devices(dev) only be called when ret != -EPROBE_DEFER (as done in pcie-qcom.c, pci-imx6.c, and pcie-mediatek-gen3.c)? -- Sashiko AI review · https://sashiko.dev/#/patchset/20261027075520.3591049-1-shawn.lin@rock-chips.com?part=1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] arm64: dts: rockchip: rk3588-evb1: describe the PCIe3x4 slot supply 2026-10-27 7:55 ` Shawn Lin @ 2026-10-27 7:55 ` Shawn Lin -1 siblings, 0 replies; 8+ messages in thread From: Shawn Lin @ 2026-10-27 7:55 UTC (permalink / raw) To: Bjorn Helgaas, Manivannan Sadhasivam, Heiko Stuebner Cc: linux-pci, linux-rockchip, Shawn Lin Move the vpcie3v3 supply of the PCIe3x4 controller into a child node describing the slot, so that the PCI pwrctrl framework manages it. The child node is modelled as a PCI bridge, which is what dtc expects for a node below a PCI host bridge, and carries the regulator that powers the devices in the slot. Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> --- arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts index 8969b56f3063..80eb36e783e0 100644 --- a/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts +++ b/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts @@ -569,8 +569,20 @@ &pcie3x4 { pinctrl-0 = <&pcie3_reset>, <&pcie30x4m1_clkreqn>; supports-clkreq; reset-gpios = <&gpio4 RK_PB6 GPIO_ACTIVE_HIGH>; - vpcie3v3-supply = <&vcc3v3_pcie30>; status = "okay"; + + pcie@0 { + device_type = "pci"; + compatible = "pciclass,0604"; + reg = <0x000000 0x0 0x0 0x0 0x0>; + bus-range = <0x00 0x0f>; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + + vpcie3v3-supply = <&vcc3v3_pcie30>; + }; }; &pd_gpu { -- 2.25.1 _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] arm64: dts: rockchip: rk3588-evb1: describe the PCIe3x4 slot supply @ 2026-10-27 7:55 ` Shawn Lin 0 siblings, 0 replies; 8+ messages in thread From: Shawn Lin @ 2026-10-27 7:55 UTC (permalink / raw) To: Bjorn Helgaas, Manivannan Sadhasivam, Heiko Stuebner Cc: linux-pci, linux-rockchip, Shawn Lin Move the vpcie3v3 supply of the PCIe3x4 controller into a child node describing the slot, so that the PCI pwrctrl framework manages it. The child node is modelled as a PCI bridge, which is what dtc expects for a node below a PCI host bridge, and carries the regulator that powers the devices in the slot. Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> --- arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts index 8969b56f3063..80eb36e783e0 100644 --- a/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts +++ b/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts @@ -569,8 +569,20 @@ &pcie3x4 { pinctrl-0 = <&pcie3_reset>, <&pcie30x4m1_clkreqn>; supports-clkreq; reset-gpios = <&gpio4 RK_PB6 GPIO_ACTIVE_HIGH>; - vpcie3v3-supply = <&vcc3v3_pcie30>; status = "okay"; + + pcie@0 { + device_type = "pci"; + compatible = "pciclass,0604"; + reg = <0x000000 0x0 0x0 0x0 0x0>; + bus-range = <0x00 0x0f>; + + #address-cells = <3>; + #size-cells = <2>; + ranges; + + vpcie3v3-supply = <&vcc3v3_pcie30>; + }; }; &pd_gpu { -- 2.25.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] arm64: dts: rockchip: rk3588-evb1: describe the PCIe3x4 slot supply 2026-10-27 7:55 ` Shawn Lin (?) @ 2026-10-27 8:13 ` sashiko-bot -1 siblings, 0 replies; 8+ messages in thread From: sashiko-bot @ 2026-10-27 8:13 UTC (permalink / raw) To: Shawn Lin; +Cc: linux-pci Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] PCI: dw-rockchip: missing select PCI_PWRCTRL_GENERIC breaks link and power - [High] PCI: dw-rockchip: rockchip_pcie_probe() destroys pwrctrl devices on deferral -- commit 78bab75f21b5749b90228d2891ea140c86ee2884 Author: Shawn Lin <shawn.lin@rock-chips.com> arm64: dts: rockchip: rk3588-evb1: describe the PCIe3x4 slot supply This commit moves the vpcie3v3-supply regulator of the PCIe3x4 controller on rk3588-evb1-v10 into a child pcie@0 bridge node so that it is managed by the PCI pwrctrl framework. > diff --git a/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts > index 8969b56f3063e..80eb36e783e0c 100644 > --- a/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts > +++ b/arch/arm64/boot/dts/rockchip/rk3588-evb1-v10.dts > @@ -569,8 +569,20 @@ &pcie3x4 { > pinctrl-0 = <&pcie3_reset>, <&pcie30x4m1_clkreqn>; > supports-clkreq; > reset-gpios = <&gpio4 RK_PB6 GPIO_ACTIVE_HIGH>; > - vpcie3v3-supply = <&vcc3v3_pcie30>; > status = "okay"; > + > + pcie@0 { > + device_type = "pci"; > + compatible = "pciclass,0604"; > + reg = <0x000000 0x0 0x0 0x0 0x0>; > + bus-range = <0x00 0x0f>; > + > + #address-cells = <3>; > + #size-cells = <2>; > + ranges; > + > + vpcie3v3-supply = <&vcc3v3_pcie30>; > + }; [Severity: High] With vpcie3v3-supply moved from &pcie3x4 into the pcie@0 child node (matched by compatible = "pciclass,0604" in pci-pwrctrl-slot under CONFIG_PCI_PWRCTRL_GENERIC), and commit f0cd23e8e469 ("PCI: dw-rockchip: Power on devices below the RC using pwrctrl") adding direct calls to the pci_pwrctrl_* helpers in rockchip_pcie_probe(), should PCIE_ROCKCHIP_DW_HOST select PCI_PWRCTRL_GENERIC in drivers/pci/controller/dwc/Kconfig? Currently in drivers/pci/controller/dwc/Kconfig, PCIE_ROCKCHIP_DW and PCIE_ROCKCHIP_DW_HOST are bool symbols built into vmlinux without selecting PCI_PWRCTRL_GENERIC: drivers/pci/controller/dwc/Kconfig: config PCIE_ROCKCHIP_DW bool config PCIE_ROCKCHIP_DW_HOST bool "Rockchip DesignWare PCIe controller (host mode)" depends on PCI_MSI depends on ARCH_ROCKCHIP || COMPILE_TEST depends on OF select PCIE_DW_HOST select PCIE_ROCKCHIP_DW select PCI_HOST_COMMON Meanwhile, include/linux/pci-pwrctrl.h exposes the non-stub declarations whenever CONFIG_PCI_PWRCTRL is enabled as either built-in or a module: include/linux/pci-pwrctrl.h: #if IS_ENABLED(CONFIG_PCI_PWRCTRL) int pci_pwrctrl_create_devices(struct device *parent); void pci_pwrctrl_destroy_devices(struct device *parent); int pci_pwrctrl_power_on_devices(struct device *parent); void pci_pwrctrl_power_off_devices(struct device *parent); #else If CONFIG_PCIE_ROCKCHIP_DW_HOST=y and CONFIG_PCI_PWRCTRL=m (for example when CONFIG_PCI_PWRCTRL_GENERIC=m as in arch/arm64/configs/defconfig and no other built-in driver selects PCI_PWRCTRL_GENERIC=y), linking vmlinux fails with undefined references from rockchip_pcie_probe() to pci_pwrctrl_create_devices(), pci_pwrctrl_power_on_devices(), pci_pwrctrl_power_off_devices(), and pci_pwrctrl_destroy_devices(). Additionally, if CONFIG_PCI_PWRCTRL_GENERIC=n, either CONFIG_PCI_PWRCTRL=n turns those calls into no-op stubs while &pcie3x4 no longer has vpcie3v3-supply on the controller node (leaving vcc3v3_pcie30 disabled when rockchip_pcie_phy_init() runs), or CONFIG_PCI_PWRCTRL=y via another pwrctrl driver causes pci_pwrctrl_power_on_devices() to return -EPROBE_DEFER permanently because no driver binds to "pciclass,0604". [Severity: High] What happens on rk3588-evb1-v10 when pci_pwrctrl_power_on_devices() or a subsequent step in rockchip_pcie_probe() returns -EPROBE_DEFER for this pcie@0 child node? In drivers/pci/pwrctrl/core.c, pci_pwrctrl_power_on_devices() calls pci_pwrctrl_power_on_device(), which returns -EPROBE_DEFER whenever the newly created pwrctrl platform device for pcie@0 has not yet bound to its driver (for example if slot_pwrctrl_probe() defers on vcc3v3_pcie30 or &gpio3, or if pci-pwrctrl-slot is a module not yet loaded): drivers/pci/pwrctrl/core.c:pci_pwrctrl_power_on_device() scoped_guard(device, &pdev->dev) { if (device_is_bound(&pdev->dev)) { ret = __pci_pwrctrl_power_on_device(&pdev->dev); } else { /* FIXME: Use blocking wait instead of probe deferral */ dev_dbg(&pdev->dev, "driver is not bound\n"); ret = -EPROBE_DEFER; } } However, in drivers/pci/controller/dwc/pcie-dw-rockchip.c, rockchip_pcie_probe() unconditionally destroys the pwrctrl child devices both when pci_pwrctrl_power_on_devices() fails and at the err_pwrctrl_off cleanup label: drivers/pci/controller/dwc/pcie-dw-rockchip.c:rockchip_pcie_probe() if (data->mode == DW_PCIE_RC_TYPE) { ret = pci_pwrctrl_create_devices(dev); if (ret) return dev_err_probe(dev, ret, "failed to create pwrctrl devices\n"); ret = pci_pwrctrl_power_on_devices(dev); if (ret) { pci_pwrctrl_destroy_devices(dev); return dev_err_probe(dev, ret, "failed to power on pwrctrl devices\n"); } } ... err_pwrctrl_off: if (data->mode == DW_PCIE_RC_TYPE) { pci_pwrctrl_power_off_devices(dev); pci_pwrctrl_destroy_devices(dev); } Calling pci_pwrctrl_destroy_devices(dev) on -EPROBE_DEFER unregisters the pcie@0 platform device via of_device_unregister(), removing it from sysfs and deferred_probe_pending_list so it cannot bind to slot_pwrctrl_driver and trigger a re-probe of &pcie3x4. Should rockchip_pcie_probe() avoid calling pci_pwrctrl_destroy_devices(dev) when ret == -EPROBE_DEFER (matching pcie-qcom.c, pci-imx6.c, and pcie-mediatek-gen3.c)? -- Sashiko AI review · https://sashiko.dev/#/patchset/20261027075520.3591049-1-shawn.lin@rock-chips.com?part=2 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-10-27 8:13 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-27 7:55 [PATCH 0/2] Manage the PCIe supplies with pwrctrl Shawn Lin 2026-10-27 7:55 ` Shawn Lin 2026-10-27 7:55 ` [PATCH 1/2] PCI: dw-rockchip: Power on devices below the RC using pwrctrl Shawn Lin 2026-10-27 7:55 ` Shawn Lin 2026-10-27 8:08 ` sashiko-bot 2026-10-27 7:55 ` [PATCH 2/2] arm64: dts: rockchip: rk3588-evb1: describe the PCIe3x4 slot supply Shawn Lin 2026-10-27 7:55 ` Shawn Lin 2026-10-27 8:13 ` sashiko-bot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.