Re: [PATCH v2] arm64: dts: qcom: kodiak: Move PCIe GPIOs and PHYs to root ports

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Pre-existing bug in the legacy fallback parsing of the Qualcomm PCIe driver where a devm-managed GPIO request causes an -EBUSY failure.
--

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

arm64: dts: qcom: kodiak: Move PCIe GPIOs and PHYs to root ports

This commit moves the PCIe PHY and GPIO references from the host bridge to
the root ports for Kodiak devices. It aligns the device trees with the new
Qualcomm PCIe bindings where perst-gpios becomes reset-gpios on the root
port.

> The Qualcomm PCIe binding deprecates perst-gpios and wake-gpios
> on the host bridge and expects board reset and wake GPIOs to be
> described on the root port. PERST# is described there as
> reset-gpios.
> 
> Move the PCIe PHY references in kodiak.dtsi to the PCIe0 and PCIe1
> root port nodes, and move the board-specific PCIe reset and wake
> GPIOs in the Kodiak DTs to the corresponding root ports.
> 
> Keep the PHY and GPIO resources on the same root port nodes so the
> Qualcomm PCIe driver can parse the root port binding instead of
> falling back to the legacy host bridge GPIO parsing.

[Severity: High]
This isn't a bug introduced by this patch, but looking at the fallback
to the legacy host bridge GPIO parsing mentioned here, is there a
resource leak in the driver's fallback mechanics?

In drivers/pci/controller/dwc/pcie-qcom.c:qcom_pcie_parse_ports(), the
driver requests the perst GPIO exclusively:

qcom_pcie_parse_ports() {
    ...
    pcie->reset = devm_gpiod_get_optional(dev, "perst",
                                          GPIOD_OUT_HIGH);
    ...
}

If it fails to parse the child port nodes, it returns -ENODEV and jumps
to err_port_del:

err_port_del:
    list_for_each_entry_safe(port, tmp_port, &pcie->ports, list) {
        ...
        
However, it does not free the devm-allocated perst GPIO before returning.

When probe catches the -ENODEV and falls back to
qcom_pcie_parse_legacy_binding(), it tries to acquire the exact same GPIO:

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

Will this second request fail with -EBUSY and cause a regression for
existing boards running legacy device trees?

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