Re: [PATCH AUTOSEL 6.18] PCI: cadence: Protect root bus removal with rescan lock

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Medium] Missing cleanup of host init and link setup on pci_host_probe failure.
- [Medium] Double pm_runtime_disable() due to mixing devm_pm_runtime_enable() and manual pm_runtime_disable().
--

commit 9d7375f353a2f9bf3cd5f3ff9c612cd84f44e32f
Author: Hans Zhang <18255117159@163.com>

PCI: cadence: Protect root bus removal with rescan lock
    
Hold the pci_rescan_remove_lock lock while stopping and removing a root bus
to avoid racing with concurrent rescan or hotplug operations triggered via
sysfs.

Link: https://patch.msgid.link/20260521161822.132996-2-18255117159@163.com

> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
> index fffd63d6665e8..c8daa5847c0f3 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence-host.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
> @@ -620,8 +620,10 @@ void cdns_pcie_host_disable(struct cdns_pcie_rc *rc)
>  	struct pci_host_bridge *bridge;
>  
>  	bridge = pci_host_bridge_from_priv(rc);
> +	pci_lock_rescan_remove();
>  	pci_stop_root_bus(bridge->bus);
>  	pci_remove_root_bus(bridge->bus);
> +	pci_unlock_rescan_remove();
>  
>  	cdns_pcie_host_deinit(rc);
>  	cdns_pcie_host_link_disable(rc);

[Severity: Medium]
This isn't a bug introduced by this patch, but does cdns_pcie_host_setup() leak
hardware resources if a failure occurs during the probe phase?

If probe fails in cdns_pcie_host_init() due to a resource conflict:

drivers/pci/controller/cadence/pcie-cadence-host.c:cdns_pcie_host_setup() {
...
	ret = cdns_pcie_host_init(rc);
	if (ret)
		return ret;

Can this return without calling cdns_pcie_host_link_disable() to undo the prior
link setup?

Similarly, if the failure happens later in the same function:

	return pci_host_probe(bridge);
}

Will this return the error code without undoing the setup performed by both
cdns_pcie_host_link_setup() and cdns_pcie_host_init()?

This could leave the PCIe link started and root port BARs configured,
potentially preventing a clean re-initialization on subsequent probe attempts.

[Severity: Medium]
This is also a pre-existing issue, but could the driver reload path in
pcie-sg2042.c leave runtime PM permanently disabled?

When sg2042_pcie_probe() initializes power management:

drivers/pci/controller/cadence/pcie-sg2042.c:sg2042_pcie_probe() {
...
	pm_runtime_no_callbacks(dev);
	devm_pm_runtime_enable(dev);

This automatically registers a devres cleanup function to call
pm_runtime_disable() on driver unbind.

However, the remove function manually disables it again:

drivers/pci/controller/cadence/pcie-sg2042.c:sg2042_pcie_remove() {
...
	pm_runtime_disable(dev);
}

If an administrator unbinds and rebinds the driver, does this manual call
increment the disable_depth counter a second time alongside the devres cleanup?

On subsequent driver reprobe, devm_pm_runtime_enable() would only decrement
it by one, leaving disable_depth at 1. Would this prevent the device from
entering low-power states after a driver reload?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-408-sashal@kernel.org?part=1