All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fix: PCI: dra7xx: dra7xx_pcie_probe: fix missing device_link_del on phy   error paths
@ 2026-06-26 15:48 WenTao Liang
  2026-06-26 15:57 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: WenTao Liang @ 2026-06-26 15:48 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, linux-pci
  Cc: Lorenzo Pieralisi, Krzysztof Wilczynski, Bjorn Helgaas, stable,
	linux-kernel, WenTao Liang

In dra7xx_pcie_probe(), when devm_phy_get() fails for a later phy device
  (i > 0), the function directly returns PTR_ERR(phy[i]) without calling
  device_link_del() on previously added device links. Similarly, when
  dra7xx_pcie_enable_phy() fails, all phy_count device links are leaked as
  the function returns directly without cleanup.

Change the error paths to jump to the existing err_link label which
  properly iterates through link[0..i) and calls device_link_del() for
  each.

Cc: stable@vger.kernel.org
Signed-off-by: WenTao Liang <vulab@iscas.ac.cn>
---
 drivers/pci/controller/dwc/pci-dra7xx.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
index cd904659c321..153ee6f7dbfc 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -743,8 +743,10 @@ static int dra7xx_pcie_probe(struct platform_device *pdev)
 	for (i = 0; i < phy_count; i++) {
 		snprintf(name, sizeof(name), "pcie-phy%d", i);
 		phy[i] = devm_phy_get(dev, name);
-		if (IS_ERR(phy[i]))
-			return PTR_ERR(phy[i]);
+		if (IS_ERR(phy[i])) {
+			ret = PTR_ERR(phy[i]);
+			goto err_link;
+		}
 
 		link[i] = device_link_add(dev, &phy[i]->dev, DL_FLAG_STATELESS);
 		if (!link[i]) {
@@ -767,7 +769,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;
 	}
 
 	platform_set_drvdata(pdev, dra7xx);
@@ -910,7 +913,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);
+		goto err_link;
 	}
 
 	return 0;
-- 
2.39.5 (Apple Git-154)


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] fix: PCI: dra7xx: dra7xx_pcie_probe: fix missing device_link_del on phy   error paths
  2026-06-26 15:48 [PATCH] fix: PCI: dra7xx: dra7xx_pcie_probe: fix missing device_link_del on phy error paths WenTao Liang
@ 2026-06-26 15:57 ` sashiko-bot
  2026-06-28  4:01 ` WenTao Liang
  2026-08-14 20:00 ` kernel test robot
  2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-06-26 15:57 UTC (permalink / raw)
  To: WenTao Liang; +Cc: linux-pci

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] fix: PCI: dra7xx: dra7xx_pcie_probe: fix missing device_link_del on phy   error paths
  2026-06-26 15:48 [PATCH] fix: PCI: dra7xx: dra7xx_pcie_probe: fix missing device_link_del on phy error paths WenTao Liang
  2026-06-26 15:57 ` sashiko-bot
@ 2026-06-28  4:01 ` WenTao Liang
  2026-08-14 20:00 ` kernel test robot
  2 siblings, 0 replies; 4+ messages in thread
From: WenTao Liang @ 2026-06-28  4:01 UTC (permalink / raw)
  To: Kishon Vijay Abraham I, linux-pci
  Cc: Lorenzo Pieralisi, Krzysztof Wilczynski, Bjorn Helgaas, stable,
	linux-kernel



> 2026年6月26日 23:48,WenTao Liang <vulab@iscas.ac.cn> 写道:
> 
> In dra7xx_pcie_probe(), when devm_phy_get() fails for a later phy device
>  (i > 0), the function directly returns PTR_ERR(phy[i]) without calling
>  device_link_del() on previously added device links. Similarly, when
>  dra7xx_pcie_enable_phy() fails, all phy_count device links are leaked as
>  the function returns directly without cleanup.
> 
> Change the error paths to jump to the existing err_link label which
>  properly iterates through link[0..i) and calls device_link_del() for
>  each.
> 
> Cc: stable@vger.kernel.org
> Signed-off-by: WenTao Liang <vulab@iscas.ac.cn>
> ---
> drivers/pci/controller/dwc/pci-dra7xx.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
> index cd904659c321..153ee6f7dbfc 100644
> --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> @@ -743,8 +743,10 @@ static int dra7xx_pcie_probe(struct platform_device *pdev)
> 	for (i = 0; i < phy_count; i++) {
> 		snprintf(name, sizeof(name), "pcie-phy%d", i);
> 		phy[i] = devm_phy_get(dev, name);
> -		if (IS_ERR(phy[i]))
> -			return PTR_ERR(phy[i]);
> +		if (IS_ERR(phy[i])) {
> +			ret = PTR_ERR(phy[i]);
> +			goto err_link;
> +		}
> 
> 		link[i] = device_link_add(dev, &phy[i]->dev, DL_FLAG_STATELESS);
> 		if (!link[i]) {
> @@ -767,7 +769,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;
> 	}
> 
> 	platform_set_drvdata(pdev, dra7xx);
> @@ -910,7 +913,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);
> +		goto err_link;
> 	}
> 
> 	return 0;
> -- 
> 2.39.5 (Apple Git-154)

Please ignore this patch. I will resend a proper version after
learning the kernel submission process.

Apologies for the noise.

Best regards,
WenTao Liang


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] fix: PCI: dra7xx: dra7xx_pcie_probe: fix missing device_link_del on phy   error paths
  2026-06-26 15:48 [PATCH] fix: PCI: dra7xx: dra7xx_pcie_probe: fix missing device_link_del on phy error paths WenTao Liang
  2026-06-26 15:57 ` sashiko-bot
  2026-06-28  4:01 ` WenTao Liang
@ 2026-08-14 20:00 ` kernel test robot
  2 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2026-08-14 20:00 UTC (permalink / raw)
  To: WenTao Liang, Kishon Vijay Abraham I, linux-pci
  Cc: oe-kbuild-all, Lorenzo Pieralisi, Krzysztof Wilczynski,
	Bjorn Helgaas, stable, linux-kernel, WenTao Liang

Hi WenTao,

kernel test robot noticed the following build errors:

[auto build test ERROR on pci/next]
[also build test ERROR on pci/for-linus linus/master v7.2-rc7 next-20260813]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/WenTao-Liang/fix-PCI-dra7xx-dra7xx_pcie_probe-fix-missing-device_link_del-on-phy-error-paths/20260815-023710
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/20260626154809.53770-1-vulab%40iscas.ac.cn
patch subject: [PATCH] fix: PCI: dra7xx: dra7xx_pcie_probe: fix missing device_link_del on phy   error paths
config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20260815/202608150306.dhMPPe3n-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 16.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260815/202608150306.dhMPPe3n-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202608150306.dhMPPe3n-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/pci/controller/dwc/pci-dra7xx.c: In function 'dra7xx_pcie_resume_noirq':
>> drivers/pci/controller/dwc/pci-dra7xx.c:918:17: error: label 'err_link' used but not defined
     918 |                 goto err_link;
         |                 ^~~~


vim +/err_link +918 drivers/pci/controller/dwc/pci-dra7xx.c

   908	
   909	static int dra7xx_pcie_resume_noirq(struct device *dev)
   910	{
   911		struct dra7xx_pcie *dra7xx = dev_get_drvdata(dev);
   912		int ret;
   913	
   914		ret = dra7xx_pcie_enable_phy(dra7xx);
   915		if (ret) {
   916			dev_err(dev, "failed to enable phy\n");
   917			dra7xx_pcie_disable_phy(dra7xx);
 > 918			goto err_link;
   919		}
   920	
   921		return 0;
   922	}
   923	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-14 20:00 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-26 15:48 [PATCH] fix: PCI: dra7xx: dra7xx_pcie_probe: fix missing device_link_del on phy error paths WenTao Liang
2026-06-26 15:57 ` sashiko-bot
2026-06-28  4:01 ` WenTao Liang
2026-08-14 20:00 ` kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.