Re: [PATCH 25/34] arm64: dts: qcom: sdm845: 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] Missing phy_exit() on early error paths in PCIe port parsing functions, leading to leaked initialized PHY state.
--

commit 7aa07cedc6576c48d7f1dd8ce61fd5bb50362173
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

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

The PCIe phy reference and the perst/wake GPIO properties are
per root port and belong in the root port node (pcie@0), not in the
RC controller node. Move phys and phy-names from the controller to
pcie0_port0 and pcie1_port0, adding labels to these nodes to allow
board-level overrides. Move perst-gpios/wake-gpios from the
controller overrides to the respective port nodes in the board files,
renaming perst-gpios to reset-gpios to match the binding used in the
root port context.

> diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
> index 3e636926231d3..27a0b90d2b7ae 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
[ ... ]
> @@ -618,15 +618,17 @@ &mss_pil {
>  
>  &pcie0 {
>  	status = "okay";
> -	perst-gpios = <&tlmm 35 GPIO_ACTIVE_LOW>;
> -	wake-gpios = <&tlmm 134 GPIO_ACTIVE_LOW>;
> -
>  	vddpe-3v3-supply = <&pcie0_3p3v_dual>;
>  
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pcie0_default_state>;
>  };
>  
> +&pcie0_port0 {
> +	reset-gpios = <&tlmm 35 GPIO_ACTIVE_LOW>;
> +	wake-gpios = <&tlmm 134 GPIO_ACTIVE_LOW>;
> +};
> +

[Severity: High]
This isn't a bug introduced by this patch, but does this new device tree
configuration expose a resource leak in the driver's error handling paths?

Because the driver now dynamically evaluates qcom_pcie_parse_perst() during
qcom_pcie_parse_port() for these root port nodes, what happens if these GPIOs
defer probing?

Looking at drivers/pci/controller/dwc/pcie-qcom.c in qcom_pcie_parse_port():

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

If this fails (e.g., returning -EPROBE_DEFER because GPIOs are not yet
available), the function returns immediately. At this point, the newly parsed
port has not been added to pcie->ports, so the caller's error handler fails
to find it and doesn't call phy_exit(port->phy). Will this leak the initialized
PHY state (such as regulator enable counts or unbalanced clock states) on every
probe deferral attempt?

A similar issue appears to be present in qcom_pcie_parse_legacy_binding():

	port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
	if (!port)
		return -ENOMEM;

Does returning -ENOMEM here leak the PHY state without reverting the earlier
phy_init(phy) call?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910-root_port-v1-0-90a252ddbbfa@oss.qualcomm.com?part=25