Re: [PATCH v2 4/4] PCI: qcom: enable Link retain logic for Hamoa

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Setting `link_retain = true` in `cfg_x1e80100` exposes a flawed error handling path in `qcom_pcie_host_init()` that silently ignores `-EPROBE_DEFER` and bypasses critical host configuration.
--

commit 868652002bbdb8222b28dbec2b0655af97d3ceb8
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

    PCI: qcom: enable Link retain logic for Hamoa

    The Hamoa platform supports keeping the PCIe link active across
    bootloader and kernel handoff. To take advantage of this, introduce a
    specific configuration (cfg_x1e80100) with link_retain = true and
    update the device match table to use it.

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index b061eaa227b39..8496eb87debbc 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1557,6 +1557,12 @@ static const struct qcom_pcie_cfg cfg_sc8280xp = {
>  	.no_l0s = true,
>  };
>  
> +static const struct qcom_pcie_cfg cfg_x1e80100 = {
> +	.ops = &ops_1_21_0,
> +	.no_l0s = true,
> +	.link_retain = true,
> +};
> +

[Severity: High]
This isn't a bug introduced by this patch, but setting link_retain to true
here exposes a pre-existing flawed error handling path in qcom_pcie_host_init()
that silently ignores -EPROBE_DEFER.

If pci_pwrctrl_power_on_devices() returns -EPROBE_DEFER when the power control
driver hasn't bound yet, the error handler intercepts the deferral:

qcom_pcie_host_init() {
    ...
	ret = pci_pwrctrl_power_on_devices(pci->dev);
	if (ret)
		goto err_pwrctrl_destroy;
    ...
err_pwrctrl_destroy:
	if (pp->link_retain && ret == -EPROBE_DEFER)
		return 0;
    ...
}

Will returning 0 here trick the DWC core (dw_pcie_host_init()) into assuming
host initialization completed successfully?

Since the probe completes without retrying, could this leave the PCIe slot
regulators unmanaged by the kernel, causing them to be disabled later when
regulator_init_complete() runs at late initcall?

Additionally, does returning early skip mandatory initialization steps like
qcom_pcie_clear_aspm_l0s() (which is required by cfg_x1e80100.no_l0s = true),
hiding MSI-X/DPC capabilities, and performing SID configuration? Skipping these
could result in link instability, AER errors, and hardware malfunction.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-link_retain-v2-0-08ed448b081c@oss.qualcomm.com?part=4

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy