Re: [PATCH] fix: PCI: dra7xx: dra7xx_pcie_probe: fix missing device_link_del on phy error paths

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Double cleanup of PHY resources on error path.
- [Low] Compilation error due to undefined label `err_link`.
--

commit 51479d1472b27aa065c936360d150e8e275d37b6
Author: WenTao Liang <vulab@iscas.ac.cn>

fix: PCI: dra7xx: dra7xx_pcie_probe: fix missing device_link_del on phy error paths
    
This commit addresses a device link leak in the phy error paths of
dra7xx_pcie_probe(). It changes the error paths to jump to the err_link
label to properly call device_link_del() for each link. It also updates
the phy enable error paths.

> diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
> index 3fc889944f021..c54d68cf0e412 100644
> --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> +++ b/drivers/pci/controller/dwc/pci-dra7xx.c

[ ... ]

> @@ -766,7 +768,8 @@ static int dra7xx_pcie_probe(struct platform_device *pdev)
>  	ret = dra7xx_pcie_enable_phy(dra7xx);
>  	if (ret) {
>  		dev_err(dev, "failed to enable phy\n");
> -		return ret;
> +		dra7xx_pcie_disable_phy(dra7xx);
> +		goto err_link;
>  	}

[Severity: High]
Does this cause a double cleanup of the PHY resources?

When dra7xx_pcie_enable_phy() fails, it already cleans up any partially 
enabled PHYs internally before returning. Calling dra7xx_pcie_disable_phy() 
here will repeat the cleanup on the same PHYs, which could lead to 
unbalanced reference counts in the PHY framework (underflowing power_count 
and init_count).

[ ... ]

> @@ -909,7 +912,8 @@ static int dra7xx_pcie_resume_noirq(struct device *dev)
>  	ret = dra7xx_pcie_enable_phy(dra7xx);
>  	if (ret) {
>  		dev_err(dev, "failed to enable phy\n");
> -		return ret;
> +		dra7xx_pcie_disable_phy(dra7xx);

[Severity: High]
Similar to the probe function, could this cause a double cleanup of the 
PHY resources since dra7xx_pcie_enable_phy() handles its own cleanup on 
failure?

> +		goto err_link;

[Severity: Low]
Does this label exist in this function?

It appears that the err_link label is not defined anywhere in 
dra7xx_pcie_resume_noirq(), which will result in a compilation error.

>  	}
>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626154809.53770-1-vulab@iscas.ac.cn?part=1