Re: [PATCH] arm64: dts: qcom: qcs6490-rubikpi3: Move PCIe GPIOs to root ports

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Moving reset-gpios to the root port node without moving `phys` breaks PCIe reset initialization, leaving the endpoints stuck in reset.
--

commit 061c47f23072e39770a5af921fae6283ad631fb1
Author: Hongyang Zhao <hongyang.zhao@thundersoft.com>

arm64: dts: qcom: qcs6490-rubikpi3: Move PCIe GPIOs to root ports

This commit moves the PCIe0 and PCIe1 reset and wake GPIOs to their
root port nodes in the qcs6490-thundercomm-rubikpi3 device tree. This
keeps the GPIO ownership with the device below the root port to match
the updated PCIe binding expectations.

> diff --git a/arch/arm64/boot/dts/qcom/qcs6490-thundercomm-rubikpi3.dts b/arch/arm64/boot/dts/qcom/qcs6490-thundercomm-rubikpi3.dts
> index f47efca42d48d..5c08ab53cdbd9 100644
> --- a/arch/arm64/boot/dts/qcom/qcs6490-thundercomm-rubikpi3.dts
> +++ b/arch/arm64/boot/dts/qcom/qcs6490-thundercomm-rubikpi3.dts
[ ... ]
> @@ -830,10 +827,12 @@ &pcie0_phy {
>  	status = "okay";
>  };
>  
> -&pcie1 {
> -	perst-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
> -	wake-gpios = <&tlmm 3 GPIO_ACTIVE_LOW>;
> +&pcie0_port {
> +	reset-gpios = <&tlmm 87 GPIO_ACTIVE_LOW>;
> +	wake-gpios = <&tlmm 89 GPIO_ACTIVE_HIGH>;
> +};

[Severity: High]
Does moving reset-gpios to the root port node without also moving the
phys property break PCIe reset initialization?

In drivers/pci/controller/dwc/pcie-qcom.c, qcom_pcie_parse_port() expects
to find the phys property on the port node:

qcom_pcie_parse_port() {
        ...
        phy = devm_of_phy_get(dev, node, NULL);
        if (IS_ERR(phy))
                return PTR_ERR(phy);
        ...
}

Because phys remains defined on the host node in the underlying base
dtsi, this returns -ENODEV and falls back to qcom_pcie_parse_legacy_binding().

The legacy parser then looks for perst-gpios on the host node:

qcom_pcie_parse_legacy_binding() {
        ...
        reset = devm_gpiod_get_optional(dev, "perst", GPIOD_OUT_HIGH);
        if (IS_ERR(reset))
                return PTR_ERR(reset);
        ...
}

Since this patch removes perst-gpios from the host node, this silently
returns NULL and the driver proceeds without acquiring the reset GPIO.

With pinctrl configuring the pin as output-low, won't the endpoints remain
stuck in reset indefinitely?

>  
> +&pcie1 {
>  	pinctrl-0 = <&pcie1_clkreq_n>,
>  		    <&pcie1_reset_n>,
>  		    <&pcie1_wake_n>;
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260607-rubikpi-bugfix-next-20260605-v1-1-ff97c5e35bf6@thundersoft.com?part=1