Re: [PATCH v2 34/37] arm64: dts: qcom: talos: Move PCIe phy and GPIOs to root port node

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] A preexisting error-handling flaw in `qcom_pcie_parse_port()` causes a resource leak of the initialized PHY. This leak will now be frequently triggered due to this commit moving `reset-gpios` to the port node.
--

commit 689313d243e7ec1e4f35d3db8dcb7b5d6e32d186
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

arm64: dts: qcom: talos: Move PCIe phy and GPIOs to root port node

Moves the PCIe phy reference and the perst/wake GPIO properties from the
RC controller node to the root port node. Also renames perst-gpios to
reset-gpios to match the root port binding context.

> diff --git a/arch/arm64/boot/dts/qcom/qcs615-ride.dts b/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> --- a/arch/arm64/boot/dts/qcom/qcs615-ride.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs615-ride.dts

[ ... ]

> @@ -444,15 +444,17 @@ &mdss_dsi0_phy {
>  };
>  
>  &pcie {
> -	perst-gpios = <&tlmm 101 GPIO_ACTIVE_LOW>;
> -	wake-gpios = <&tlmm 100 GPIO_ACTIVE_LOW>;
> -
>  	pinctrl-0 = <&pcie_default_state>;
>  	pinctrl-names = "default";
>  
>  	status = "okay";
>  };
>  
> +&pcie_port0 {
> +	reset-gpios = <&tlmm 101 GPIO_ACTIVE_LOW>;

[Severity: High]
This is a pre-existing issue, but will moving these GPIOs here make it
highly likely to trigger a PHY resource leak during probe deferral?

In drivers/pci/controller/dwc/pcie-qcom.c, qcom_pcie_parse_port()
initializes the PHY before parsing the perst GPIOs:

drivers/pci/controller/dwc/pcie-qcom.c:qcom_pcie_parse_port() {
	...
	ret = phy_init(phy);
	if (ret)
		return ret;

	INIT_LIST_HEAD(&port->perst);

	ret = qcom_pcie_parse_perst(pcie, port, node);
	if (ret)
		return ret;

	port->phy = phy;
	INIT_LIST_HEAD(&port->list);
	list_add_tail(&port->list, &pcie->ports);
}

If the newly moved reset-gpios cause qcom_pcie_parse_perst() to return
-EPROBE_DEFER, the function exits before adding the newly allocated port
to the pcie->ports list.

Since the cleanup routine qcom_pcie_parse_ports() iterates over the
pcie->ports list to call phy_exit(), does this mean the initialized PHY
reference (and its underlying clocks/regulators) will be permanently
leaked on every probe deferral?

> +	wake-gpios = <&tlmm 100 GPIO_ACTIVE_LOW>;
> +};
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611-wake-v2-0-2744251b1181@oss.qualcomm.com?part=34