Re: [PATCH v5] PCI: imx6: fix resource leaks in probe error paths

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Replacing DL_FLAG_STATELESS with DL_FLAG_AUTOREMOVE_CONSUMER for device links to virtual PM domain devices creates managed links that violate the driver core state machine, triggering a WARN_ON during successful probe.
--

commit 36162fbf1662cbc9312c01c7d2c2113d6c77e624
Author: Zhijian Han <hanzhijian1991@gmail.com>

PCI: imx6: fix resource leaks in probe error paths

This commit fixes resource leaks involving pwrctrl devices and power domains
in the imx_pcie_probe() error paths. It adds a devm action to detach the
power domains and replaces the stateless device link flag with an autoremove
consumer flag so links are cleaned up automatically.

Link: https://lore.kernel.org/all/20260822013640.182C01F000E9@smtp.kernel.org/

> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 39790e66b98dc..ec8339e03d584 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c

[ ... ]

> @@ -655,24 +672,30 @@ static int imx_pcie_attach_pd(struct device *dev)
>  	if (!imx_pcie->pd_pcie)
>  		return 0;
>  	link = device_link_add(dev, imx_pcie->pd_pcie,
> -			DL_FLAG_STATELESS |
>  			DL_FLAG_PM_RUNTIME |
> -			DL_FLAG_RPM_ACTIVE);
> +			DL_FLAG_RPM_ACTIVE |
> +			DL_FLAG_AUTOREMOVE_CONSUMER);

[Severity: High]
Does changing this link from stateless to managed violate the driver core
state machine for virtual PM domain devices?

Virtual PM domain devices created on the genpd bus never bind to drivers,
so their driver status remains DL_DEV_NO_DRIVER. Because of this,
device_link_add() will initialize a managed link to DL_STATE_DORMANT.

When the consumer device successfully completes its probe, the driver core
calls device_links_driver_bound(). That function iterates over all managed
supplier links and evaluates:

WARN_ON(link->status != DL_STATE_CONSUMER_PROBE)

Since this link is dormant, won't this trigger the WARN_ON and turn a
successful normal boot sequence into a kernel panic on systems with
panic_on_warn enabled?

>  	if (!link) {
>  		dev_err(dev, "Failed to add device_link to pcie pd\n");
> +		imx_pcie_detach_pd(imx_pcie);
>  		return -EINVAL;
>  	}
>  
>  	imx_pcie->pd_pcie_phy = dev_pm_domain_attach_by_name(dev, "pcie_phy");
> -	if (IS_ERR(imx_pcie->pd_pcie_phy))
> -		return PTR_ERR(imx_pcie->pd_pcie_phy);
> +	if (IS_ERR(imx_pcie->pd_pcie_phy)) {
> +		int ret = PTR_ERR(imx_pcie->pd_pcie_phy);
> +
> +		imx_pcie_detach_pd(imx_pcie);
> +		return ret;
> +	}
>  
>  	link = device_link_add(dev, imx_pcie->pd_pcie_phy,
> -			DL_FLAG_STATELESS |
>  			DL_FLAG_PM_RUNTIME |
> -			DL_FLAG_RPM_ACTIVE);
> +			DL_FLAG_RPM_ACTIVE |
> +			DL_FLAG_AUTOREMOVE_CONSUMER);

[Severity: High]
This has the same issue as the pcie power domain link creation above.

Since DL_FLAG_AUTOREMOVE_CONSUMER implies a managed link, should these
links for virtual PM domain devices remain DL_FLAG_STATELESS to avoid
the WARN_ON during device_links_driver_bound()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902164313.2901302-1-hanzhijian1991@gmail.com?part=1