Re: [PATCH] PCI: imx6: fix pwrctrl device leak on PM runtime setup failure

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

Pre-existing issues:
- [High] Unbalanced `dev_pm_domain_attach_by_name()` and stateless `device_link_add()` calls in `imx_pcie_attach_pd()` cause persistent resource leaks on probe failure or probe deferral.
- [High] `imx_add_pcie_ep()` ignores the return value of `imx_pcie_host_init()`, causing hardware access on unpowered registers and a potential kernel panic.
--

--- Patch [1]: [PATCH] PCI: imx6: fix pwrctrl device leak on PM runtime setup failure ---
commit 0d55b3d7e1c6b8341c6d4f378c6f80b85c73e417
Author: hanzhijian <[email protected]>

PCI: imx6: fix pwrctrl device leak on PM runtime setup failure

This commit fixes a resource leak in imx_pcie_probe() by redirecting a failure
path to err_pwrctrl_destroy rather than returning directly. It ensures pwrctrl
devices are properly cleaned up when PM runtime setup fails.

> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 39790e66b98dc..616c42932faa1 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1975,7 +1975,7 @@ static int imx_pcie_probe(struct platform_device *pdev)
>  			pm_runtime_no_callbacks(dev);
>  			ret = devm_pm_runtime_set_active_enabled(dev);
>  			if (ret < 0)
> -				return ret;
> +				goto err_pwrctrl_destroy;

[Severity: High]
This is a pre-existing issue, but does the err_pwrctrl_destroy error path leak
PM domains and device links?

In imx_pcie_probe(), imx_pcie_attach_pd() explicitly allocates PM domains via
dev_pm_domain_attach_by_name() and creates stateless device links:

drivers/pci/controller/dwc/pci-imx6.c:imx_pcie_attach_pd() {
	...
	imx_pcie->pd_pcie = dev_pm_domain_attach_by_name(dev, "pcie");
	...
}

If a failure occurs and the code jumps to err_pwrctrl_destroy, it appears these
resources are not explicitly cleaned up using dev_pm_domain_detach() and
device_link_del() before returning.  Since this can happen on probe deferrals
(e.g., if pci_pwrctrl_create_devices() returns -EPROBE_DEFER), will these
resources leak repeatedly on every deferral?

>  		}
>  
>  		if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_SKIP_L23_READY))

[Severity: High]
This is also a pre-existing issue, but does imx_add_pcie_ep() ignore the
return value of imx_pcie_host_init()?

Looking at imx_add_pcie_ep(), which can be called earlier in imx_pcie_probe()
before reaching this code:

drivers/pci/controller/dwc/pci-imx6.c:imx_add_pcie_ep() {
	...
	imx_pcie_host_init(pp);
	ep = &pci->ep;
	ep->ops = &pcie_ep_ops;
	...
	ret = dw_pcie_ep_init(ep);
	...
}

If imx_pcie_host_init() fails (e.g., if regulators or clocks fail or defer),
the return value is discarded.  Will the code blindly proceed to
dw_pcie_ep_init() and access unpowered or unclocked hardware registers,
potentially causing a kernel panic due to an SError on ARM?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1