Re: [PATCH 6/9] PCI: mediatek: Protect root bus removal with rescan lock

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unbalanced clock disable and PM runtime disable when PCIe links are down.
--

commit 92b7c7047c2235e7192b983ddc544d37ac83ba04
Author: Hans Zhang <18255117159@163.com>

PCI: mediatek: Protect root bus removal with rescan lock

Secure mtk_pcie_remove() by adding pci_lock_rescan_remove() /
pci_unlock_rescan_remove() around pci_stop_root_bus() and
pci_remove_root_bus().

> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index 75722524fe744..2fedb6d2939de 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -1172,8 +1172,10 @@ static void mtk_pcie_remove(struct platform_device *pdev)
>  	struct mtk_pcie *pcie = platform_get_drvdata(pdev);
>  	struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie);
>  
> +	pci_lock_rescan_remove();
>  	pci_stop_root_bus(host->bus);
>  	pci_remove_root_bus(host->bus);
> +	pci_unlock_rescan_remove();
>  	mtk_pcie_free_resources(pcie);
>  
>  	mtk_pcie_irq_teardown(pcie);

[Severity: High]
This is a pre-existing issue, but does the unconditional call to
mtk_pcie_put_resources(pcie) at the end of this function cause unbalanced
disables if all port links failed to come up?

In mtk_pcie_setup(), if all port links fail to come up (for instance, if no
PCIe endpoint is connected), list_empty(&pcie->ports) evaluates to true.
When this happens, mtk_pcie_subsys_powerdown(pcie) is called early,
disabling the clock and PM runtime. However, mtk_pcie_probe() successfully
completes and registers the empty root bus.

When the driver is later unbound or removed, mtk_pcie_remove()
unconditionally executes mtk_pcie_put_resources(pcie), without the necessary
!list_empty(&pcie->ports) guard that correctly exists in the probe error
path. This calls mtk_pcie_subsys_powerdown() a second time.

Will this double clk_disable_unprepare() trigger a clock framework WARN,
and increment the PM disable depth to 2?

If the driver is later re-probed, pm_runtime_enable() only decrements the
depth by 1, leaving the device disabled.

Can this result in pm_runtime_get_sync() silently failing to power on the
PCIe power domain, causing subsequent MMIO register accesses in
mtk_pcie_enable_port() to trigger a synchronous external abort?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521161822.132996-1-18255117159@163.com?part=6