[PATCH 0/2] Move PHYs and PERST# properties to PCIe RP node

[PATCH 0/2] Move PHYs and PERST# properties to PCIe RP node

[Kathiravan Thirumoorthy]

Follow the new binding by placing the PHYs and PERST# (now RESET#)
under the Root Port node instead of the Root Complex node. Although
IPQ5332 and IPQ5424 are intended to follow the new binding style, they
still define these properties under the RC node.

Move these properties to the RP node to avoid mixed configurations.
Also, drop the phy-names property which is deprecated in the new style.

Other IPQ targets still follow the complete old binding style (no mixed
configuraions). Convert them to the new binding in a subsequent release,
which will also allow me with enough time to arrange the devices for the
sanity checks.

Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
---
Kathiravan Thirumoorthy (2):
      arm64: dts: qcom: ipq5332: Move PHYs and PERST# to Root Port node
      arm64: dts: qcom: ipq5424: Move PHYs and PERST# to Root Port node

 arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts | 16 ++++++++++------
 arch/arm64/boot/dts/qcom/ipq5332.dtsi       | 12 ++++--------
 arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts | 12 ++++++++----
 arch/arm64/boot/dts/qcom/ipq5424.dtsi       | 20 ++++++++------------
 4 files changed, 30 insertions(+), 30 deletions(-)
---
base-commit: 08484c504b55a98bd100527fbe10a3caf55ff3ff
change-id: 20260602-move_perst_to_rp-63b4fab37bc5

Best regards,
--  
Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>

[PATCH 1/2] arm64: dts: qcom: ipq5332: Move PHYs and PERST# to Root Port node

[Kathiravan Thirumoorthy]

Follow the new binding style by defining PHYs and PERST# (now RESET#)
under the Root Port node. Avoid mixing styles and move these properties
to the RP node.

Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts | 16 ++++++++++------
 arch/arm64/boot/dts/qcom/ipq5332.dtsi       | 12 ++++--------
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts b/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts
index 79ec77cfe552..7fcf632e289f 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts
+++ b/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts
@@ -36,9 +36,6 @@ &pcie0 {
 	pinctrl-0 = <&pcie0_default>;
 	pinctrl-names = "default";
 
-	perst-gpios = <&tlmm 38 GPIO_ACTIVE_LOW>;
-	wake-gpios = <&tlmm 39 GPIO_ACTIVE_LOW>;
-
 	status = "okay";
 };
 
@@ -46,13 +43,15 @@ &pcie0_phy {
 	status = "okay";
 };
 
+&pcie0_port0 {
+	reset-gpios = <&tlmm 38 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 39 GPIO_ACTIVE_LOW>;
+};
+
 &pcie1 {
 	pinctrl-0 = <&pcie1_default>;
 	pinctrl-names = "default";
 
-	perst-gpios = <&tlmm 47 GPIO_ACTIVE_LOW>;
-	wake-gpios = <&tlmm 48 GPIO_ACTIVE_LOW>;
-
 	status = "okay";
 };
 
@@ -60,6 +59,11 @@ &pcie1_phy {
 	status = "okay";
 };
 
+&pcie1_port0 {
+	reset-gpios = <&tlmm 47 GPIO_ACTIVE_LOW>;
+	wake-gpios = <&tlmm 48 GPIO_ACTIVE_LOW>;
+};
+
 &tlmm {
 	i2c_1_pins: i2c-1-state {
 		pins = "gpio29", "gpio30";
diff --git a/arch/arm64/boot/dts/qcom/ipq5332.dtsi b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
index e227730d99a6..bff5e3ea7831 100644
--- a/arch/arm64/boot/dts/qcom/ipq5332.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5332.dtsi
@@ -704,22 +704,20 @@ pcie1: pcie@18000000 {
 				      "aux",
 				      "ahb";
 
-			phys = <&pcie1_phy>;
-			phy-names = "pciephy";
-
 			interconnects = <&gcc MASTER_SNOC_PCIE3_2_M &gcc SLAVE_SNOC_PCIE3_2_M>,
 					<&gcc MASTER_ANOC_PCIE3_2_S &gcc SLAVE_ANOC_PCIE3_2_S>;
 			interconnect-names = "pcie-mem", "cpu-pcie";
 
 			status = "disabled";
 
-			pcie@0 {
+			pcie1_port0: pcie@0 {
 				device_type = "pci";
 				reg = <0x0 0x0 0x0 0x0 0x0>;
 
 				#address-cells = <3>;
 				#size-cells = <2>;
 				ranges;
+				phys = <&pcie1_phy>;
 			};
 		};
 
@@ -808,22 +806,20 @@ pcie0: pcie@20000000 {
 				      "aux",
 				      "ahb";
 
-			phys = <&pcie0_phy>;
-			phy-names = "pciephy";
-
 			interconnects = <&gcc MASTER_SNOC_PCIE3_1_M &gcc SLAVE_SNOC_PCIE3_1_M>,
 					<&gcc MASTER_ANOC_PCIE3_1_S &gcc SLAVE_ANOC_PCIE3_1_S>;
 			interconnect-names = "pcie-mem", "cpu-pcie";
 
 			status = "disabled";
 
-			pcie@0 {
+			pcie0_port0: pcie@0 {
 				device_type = "pci";
 				reg = <0x0 0x0 0x0 0x0 0x0>;
 
 				#address-cells = <3>;
 				#size-cells = <2>;
 				ranges;
+				phys = <&pcie0_phy>;
 			};
 		};
 	};

-- 
2.34.1

Re: [PATCH 1/2] arm64: dts: qcom: ipq5332: Move PHYs and PERST# to Root Port node

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing `phy_exit()` on error path in `qcom_pcie_parse_port()` leaks PHY initialization state.
--

commit a3169d2cae3bc2449e374f733bcb358e726cc377
Author: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>

arm64: dts: qcom: ipq5332: Move PHYs and PERST# to Root Port node

This commit updates the device tree bindings for the IPQ5332 PCIe
interfaces. It moves the PHYs and PERST# properties under the
Root Port nodes to follow the new binding style.

> diff --git a/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts b/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts
> index 79ec77cfe5527..7fcf632e289f0 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts
> +++ b/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts

[ ... ]

> @@ -46,13 +43,15 @@ &pcie0_phy {
>  	status = "okay";
>  };
>  
> +&pcie0_port0 {
> +	reset-gpios = <&tlmm 38 GPIO_ACTIVE_LOW>;
> +	wake-gpios = <&tlmm 39 GPIO_ACTIVE_LOW>;
> +};

[Severity: High]
This is a pre-existing issue, but does moving these properties to the
Root Port node expose a PHY initialization leak on probe deferral?

When the driver processes the port via qcom_pcie_parse_port(), it calls
phy_init() and then immediately calls qcom_pcie_parse_perst(). If parsing
the PERST# GPIO (now added to the port node here) fails with -EPROBE_DEFER,
the function returns early:

drivers/pci/controller/dwc/pcie-qcom.c:qcom_pcie_parse_port() {
    ...
	ret = qcom_pcie_parse_perst(pcie, port, node);
	if (ret)
		return ret;
    ...
}

Because the port is not added to the pcie->ports list until later in
the function, the cleanup loop in the caller qcom_pcie_parse_ports() misses
it entirely:

err_port_del:
	list_for_each_entry_safe(port, tmp_port, &pcie->ports, list) {
		list_for_each_entry_safe(perst, tmp_perst, &port->perst, list)
			list_del(&perst->list);
		phy_exit(port->phy);
		list_del(&port->list);
	}

Could this lead to unbalanced clocks and regulator reference counts by
skipping phy_exit() if the GPIO driver is not yet ready?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602-move_perst_to_rp-v1-0-8ee5b80316da@oss.qualcomm.com?part=1

[PATCH 2/2] arm64: dts: qcom: ipq5424: Move PHYs and PERST# to Root Port node

[Kathiravan Thirumoorthy]

Follow the new binding style by defining PHYs and PERST# (now RESET#)
under the Root Port node. Avoid mixing styles and move these properties
to the RP node.

Signed-off-by: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts | 12 ++++++++----
 arch/arm64/boot/dts/qcom/ipq5424.dtsi       | 20 ++++++++------------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts b/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
index de71b72ae6dc..be8657239c46 100644
--- a/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
+++ b/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
@@ -86,8 +86,6 @@ &pcie2 {
 	pinctrl-0 = <&pcie2_default_state>;
 	pinctrl-names = "default";
 
-	perst-gpios = <&tlmm 31 GPIO_ACTIVE_LOW>;
-
 	status = "okay";
 };
 
@@ -95,12 +93,14 @@ &pcie2_phy {
 	status = "okay";
 };
 
+&pcie2_port0 {
+	reset-gpios = <&tlmm 31 GPIO_ACTIVE_LOW>;
+};
+
 &pcie3 {
 	pinctrl-0 = <&pcie3_default_state>;
 	pinctrl-names = "default";
 
-	perst-gpios = <&tlmm 34 GPIO_ACTIVE_LOW>;
-
 	status = "okay";
 };
 
@@ -108,6 +108,10 @@ &pcie3_phy {
 	status = "okay";
 };
 
+&pcie3_port0 {
+	reset-gpios = <&tlmm 34 GPIO_ACTIVE_LOW>;
+};
+
 &qusb_phy_0 {
 	vdd-supply = <&vreg_misc_0p925>;
 	vdda-pll-supply = <&vreg_misc_1p8>;
diff --git a/arch/arm64/boot/dts/qcom/ipq5424.dtsi b/arch/arm64/boot/dts/qcom/ipq5424.dtsi
index 876bf6a8b8ff..6635f9259bbb 100644
--- a/arch/arm64/boot/dts/qcom/ipq5424.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq5424.dtsi
@@ -968,15 +968,13 @@ pcie3: pcie@40000000 {
 				      "aux",
 				      "ahb";
 
-			phys = <&pcie3_phy>;
-			phy-names = "pciephy";
 			interconnects = <&gcc MASTER_ANOC_PCIE3 &gcc SLAVE_ANOC_PCIE3>,
 					<&gcc MASTER_CNOC_PCIE3 &gcc SLAVE_CNOC_PCIE3>;
 			interconnect-names = "pcie-mem", "cpu-pcie";
 
 			status = "disabled";
 
-			pcie@0 {
+			pcie3_port0: pcie@0 {
 				device_type = "pci";
 				reg = <0x0 0x0 0x0 0x0 0x0>;
 				bus-range = <0x01 0xff>;
@@ -984,6 +982,7 @@ pcie@0 {
 				#address-cells = <3>;
 				#size-cells = <2>;
 				ranges;
+				phys = <&pcie3_phy>;
 			};
 		};
 
@@ -1071,15 +1070,13 @@ pcie2: pcie@50000000 {
 				      "aux",
 				      "ahb";
 
-			phys = <&pcie2_phy>;
-			phy-names = "pciephy";
 			interconnects = <&gcc MASTER_ANOC_PCIE2 &gcc SLAVE_ANOC_PCIE2>,
 					<&gcc MASTER_CNOC_PCIE2 &gcc SLAVE_CNOC_PCIE2>;
 			interconnect-names = "pcie-mem", "cpu-pcie";
 
 			status = "disabled";
 
-			pcie@0 {
+			pcie2_port0: pcie@0 {
 				device_type = "pci";
 				reg = <0x0 0x0 0x0 0x0 0x0>;
 				bus-range = <0x01 0xff>;
@@ -1087,6 +1084,7 @@ pcie@0 {
 				#address-cells = <3>;
 				#size-cells = <2>;
 				ranges;
+				phys = <&pcie2_phy>;
 			};
 		};
 
@@ -1174,15 +1172,13 @@ pcie1: pcie@60000000 {
 				      "aux",
 				      "ahb";
 
-			phys = <&pcie1_phy>;
-			phy-names = "pciephy";
 			interconnects = <&gcc MASTER_ANOC_PCIE1	&gcc SLAVE_ANOC_PCIE1>,
 					<&gcc MASTER_CNOC_PCIE1	&gcc SLAVE_CNOC_PCIE1>;
 			interconnect-names = "pcie-mem", "cpu-pcie";
 
 			status = "disabled";
 
-			pcie@0 {
+			pcie1_port0: pcie@0 {
 				device_type = "pci";
 				reg = <0x0 0x0 0x0 0x0 0x0>;
 				bus-range = <0x01 0xff>;
@@ -1190,6 +1186,7 @@ pcie@0 {
 				#address-cells = <3>;
 				#size-cells = <2>;
 				ranges;
+				phys = <&pcie1_phy>;
 			};
 		};
 
@@ -1277,15 +1274,13 @@ pcie0: pcie@70000000 {
 				      "aux",
 				      "ahb";
 
-			phys = <&pcie0_phy>;
-			phy-names = "pciephy";
 			interconnects = <&gcc MASTER_ANOC_PCIE0 &gcc SLAVE_ANOC_PCIE0>,
 					<&gcc MASTER_CNOC_PCIE0	&gcc SLAVE_CNOC_PCIE0>;
 			interconnect-names = "pcie-mem", "cpu-pcie";
 
 			status = "disabled";
 
-			pcie@0 {
+			pcie0_port0: pcie@0 {
 				device_type = "pci";
 				reg = <0x0 0x0 0x0 0x0 0x0>;
 				bus-range = <0x01 0xff>;
@@ -1293,6 +1288,7 @@ pcie@0 {
 				#address-cells = <3>;
 				#size-cells = <2>;
 				ranges;
+				phys = <&pcie0_phy>;
 			};
 		};
 	};

-- 
2.34.1

Re: [PATCH 2/2] arm64: dts: qcom: ipq5424: Move PHYs and PERST# to Root Port node

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Legacy device trees fail to probe with -EBUSY because the 'perst' GPIO descriptor is requested twice without being released.
--

commit f286f42842ba6bb45e869489bbe2830fdd9be93c
Author: Kathiravan Thirumoorthy <kathiravan.thirumoorthy@oss.qualcomm.com>

arm64: dts: qcom: ipq5424: Move PHYs and PERST# to Root Port node

This commit updates the device tree to define PHYs and the PERST# 
(reset-gpios) under the PCIe Root Port node instead of the main controller 
node. This aligns with the new binding style.

> diff --git a/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts b/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
> index de71b72ae6dcc..be8657239c46b 100644
> --- a/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
> +++ b/arch/arm64/boot/dts/qcom/ipq5424-rdp466.dts
> @@ -86,8 +86,6 @@ &pcie2 {
>  	pinctrl-0 = <&pcie2_default_state>;
>  	pinctrl-names = "default";
>  
> -	perst-gpios = <&tlmm 31 GPIO_ACTIVE_LOW>;
> -

[Severity: Critical]
This is a pre-existing issue, but does moving these properties expose a
probe failure on legacy device trees?

When parsing a legacy DT, qcom_pcie_parse_ports() successfully requests the
"perst" GPIO:

drivers/pci/controller/dwc/pcie-qcom.c:qcom_pcie_parse_ports() {
...
	pcie->reset = devm_gpiod_get_optional(dev, "perst",
					      GPIOD_OUT_HIGH);
	if (IS_ERR(pcie->reset))
		return PTR_ERR(pcie->reset);
...
}

But it then returns -ENODEV due to the lack of a "phys" property on the child
node. The probe function then falls back to qcom_pcie_parse_legacy_binding(),
which attempts to request the same "perst" GPIO again:

drivers/pci/controller/dwc/pcie-qcom.c:qcom_pcie_parse_legacy_binding() {
...
	reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH);
	if (IS_ERR(reset))
		return PTR_ERR(reset);
...
}

Because the first request was never released using devm_gpiod_put(), will
the second devm_gpiod_get_optional() fail with -EBUSY and fatally abort the
probe, breaking backwards compatibility for older platforms?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260602-move_perst_to_rp-v1-0-8ee5b80316da@oss.qualcomm.com?part=2