[PATCH v2] PCI: eswin: Fix resource leak on dw_pcie_host_init() failure

[PATCH v2] PCI: eswin: Fix resource leak on dw_pcie_host_init() failure

[Felix Gu]

When dw_pcie_host_init() fails, the code jumps to err_init which
only calls pm_runtime_put() but skips the cleanup of port resources
allocated by eswin_pcie_parse_ports().

Additionally, the incorrect port cleanup code in
eswin_pcie_host_init() error path is removed since port resources
should be released in probe's error handler, not in host_init.

Fixes: b593c26d081a ("PCI: eswin: Add ESWIN PCIe Root Complex driver")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
   Changes in v2:
   - Fix Mani's comment.
   - Remove incorrect port cleanup code in eswin_pcie_host_init() error path.
   - Link to v1: https://lore.kernel.org/lkml/20260416-eswin-v1-1-a4991725b5ca@gmail.com/
---
 drivers/pci/controller/dwc/pcie-eswin.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-eswin.c b/drivers/pci/controller/dwc/pcie-eswin.c
index ce8d64f8a395..7a02ab91a190 100644
--- a/drivers/pci/controller/dwc/pcie-eswin.c
+++ b/drivers/pci/controller/dwc/pcie-eswin.c
@@ -182,7 +182,7 @@ static int eswin_pcie_host_init(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 	struct eswin_pcie *pcie = to_eswin_pcie(pci);
-	struct eswin_pcie_port *port, *tmp;
+	struct eswin_pcie_port *port;
 	u32 val;
 	int ret;
 
@@ -252,10 +252,6 @@ static int eswin_pcie_host_init(struct dw_pcie_rp *pp)
 	reset_control_bulk_assert(ESWIN_NUM_RSTS, pcie->resets);
 err_deassert:
 	clk_bulk_disable_unprepare(pcie->num_clks, pcie->clks);
-	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
-		reset_control_put(port->perst);
-		list_del(&port->list);
-	}
 
 	return ret;
 }
@@ -347,18 +343,17 @@ static int eswin_pcie_probe(struct platform_device *pdev)
 	ret = dw_pcie_host_init(&pci->pp);
 	if (ret) {
 		dev_err(dev, "Failed to init host\n");
-		goto err_init;
+		goto err_pm_runtime_put;
 	}
 
 	return 0;
 
 err_pm_runtime_put:
+	pm_runtime_put(dev);
 	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
 		reset_control_put(port->perst);
 		list_del(&port->list);
 	}
-err_init:
-	pm_runtime_put(dev);
 
 	return ret;
 }

---
base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
change-id: 20260519-eswin-997a8fa10245

Best regards,
--  
Felix Gu <ustc.gu@gmail.com>

Re: [PATCH v2] PCI: eswin: Fix resource leak on dw_pcie_host_init() failure

[zhangsenchuan]

> Fixes: b593c26d081a ("PCI: eswin: Add ESWIN PCIe Root Complex driver")
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> ---
>    Changes in v2:
>    - Fix Mani's comment.
>    - Remove incorrect port cleanup code in eswin_pcie_host_init() error path.
>    - Link to v1: https://lore.kernel.org/lkml/20260416-eswin-v1-1-a4991725b5ca@gmail.com/
> ---
>  drivers/pci/controller/dwc/pcie-eswin.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-eswin.c b/drivers/pci/controller/dwc/pcie-eswin.c
> index ce8d64f8a395..7a02ab91a190 100644
> --- a/drivers/pci/controller/dwc/pcie-eswin.c
> +++ b/drivers/pci/controller/dwc/pcie-eswin.c
> @@ -182,7 +182,7 @@ static int eswin_pcie_host_init(struct dw_pcie_rp *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>  	struct eswin_pcie *pcie = to_eswin_pcie(pci);
> -	struct eswin_pcie_port *port, *tmp;
> +	struct eswin_pcie_port *port;
>  	u32 val;
>  	int ret;
>  
> @@ -252,10 +252,6 @@ static int eswin_pcie_host_init(struct dw_pcie_rp *pp)
>  	reset_control_bulk_assert(ESWIN_NUM_RSTS, pcie->resets);
>  err_deassert:
>  	clk_bulk_disable_unprepare(pcie->num_clks, pcie->clks);
> -	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
> -		reset_control_put(port->perst);
> -		list_del(&port->list);
> -	}

Hi, Felix

If removed, in the dw_pcie_resume_noirq function, if the execution of 
"pci->pp.ops->init(&pci->pp)" fails, the resources will not be released.

Kind regards,
Senchuan

>  
>  	return ret;
>  }
> @@ -347,18 +343,17 @@ static int eswin_pcie_probe(struct platform_device *pdev)
>  	ret = dw_pcie_host_init(&pci->pp);
>  	if (ret) {
>  		dev_err(dev, "Failed to init host\n");
> -		goto err_init;
> +		goto err_pm_runtime_put;
>  	}
>  
>  	return 0;
>  
>  err_pm_runtime_put:
> +	pm_runtime_put(dev);
>  	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
>  		reset_control_put(port->perst);
>  		list_del(&port->list);
>  	}
> -err_init:
> -	pm_runtime_put(dev);
>  
>  	return ret;
>  }

Re: [PATCH v2] PCI: eswin: Fix resource leak on dw_pcie_host_init() failure

[Felix Gu]

On Thu, May 21, 2026 at 3:08 PM zhangsenchuan
<zhangsenchuan@eswincomputing.com> wrote:
> > --- a/drivers/pci/controller/dwc/pcie-eswin.c
> > +++ b/drivers/pci/controller/dwc/pcie-eswin.c
> > @@ -182,7 +182,7 @@ static int eswin_pcie_host_init(struct dw_pcie_rp *pp)
> >  {
> >       struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> >       struct eswin_pcie *pcie = to_eswin_pcie(pci);
> > -     struct eswin_pcie_port *port, *tmp;
> > +     struct eswin_pcie_port *port;
> >       u32 val;
> >       int ret;
> >
> > @@ -252,10 +252,6 @@ static int eswin_pcie_host_init(struct dw_pcie_rp *pp)
> >       reset_control_bulk_assert(ESWIN_NUM_RSTS, pcie->resets);
> >  err_deassert:
> >       clk_bulk_disable_unprepare(pcie->num_clks, pcie->clks);
> > -     list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
> > -             reset_control_put(port->perst);
> > -             list_del(&port->list);
> > -     }
>
> Hi, Felix
>
> If removed, in the dw_pcie_resume_noirq function, if the execution of
> "pci->pp.ops->init(&pci->pp)" fails, the resources will not be released.
>

Hi Senchuan,
Do we really need to release the port related resources in
dw_pcie_resume_noirq()
when pci->pp.ops->init(&pci->pp) fails?
And why not release the port related resources in the following error paths of
dw_pcie_resume_noirq()?

Best regards,
Felix
> Kind regards,
> Senchuan

Re: [PATCH v2] PCI: eswin: Fix resource leak on dw_pcie_host_init() failure

[Manivannan Sadhasivam]

On Wed, May 20, 2026 at 10:45:04PM +0800, Felix Gu wrote:
> When dw_pcie_host_init() fails, the code jumps to err_init which
> only calls pm_runtime_put() but skips the cleanup of port resources
> allocated by eswin_pcie_parse_ports().
> 
> Additionally, the incorrect port cleanup code in
> eswin_pcie_host_init() error path is removed since port resources
> should be released in probe's error handler, not in host_init.
> 
> Fixes: b593c26d081a ("PCI: eswin: Add ESWIN PCIe Root Complex driver")
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> ---
>    Changes in v2:
>    - Fix Mani's comment.
>    - Remove incorrect port cleanup code in eswin_pcie_host_init() error path.
>    - Link to v1: https://lore.kernel.org/lkml/20260416-eswin-v1-1-a4991725b5ca@gmail.com/
> ---
>  drivers/pci/controller/dwc/pcie-eswin.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-eswin.c b/drivers/pci/controller/dwc/pcie-eswin.c
> index ce8d64f8a395..7a02ab91a190 100644
> --- a/drivers/pci/controller/dwc/pcie-eswin.c
> +++ b/drivers/pci/controller/dwc/pcie-eswin.c
> @@ -182,7 +182,7 @@ static int eswin_pcie_host_init(struct dw_pcie_rp *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>  	struct eswin_pcie *pcie = to_eswin_pcie(pci);
> -	struct eswin_pcie_port *port, *tmp;
> +	struct eswin_pcie_port *port;
>  	u32 val;
>  	int ret;
>  
> @@ -252,10 +252,6 @@ static int eswin_pcie_host_init(struct dw_pcie_rp *pp)
>  	reset_control_bulk_assert(ESWIN_NUM_RSTS, pcie->resets);
>  err_deassert:
>  	clk_bulk_disable_unprepare(pcie->num_clks, pcie->clks);
> -	list_for_each_entry_safe(port, tmp, &pcie->ports, list) {
> -		reset_control_put(port->perst);
> -		list_del(&port->list);
> -	}

Unrelated change.

>  
>  	return ret;
>  }
> @@ -347,18 +343,17 @@ static int eswin_pcie_probe(struct platform_device *pdev)
>  	ret = dw_pcie_host_init(&pci->pp);
>  	if (ret) {
>  		dev_err(dev, "Failed to init host\n");
> -		goto err_init;
> +		goto err_pm_runtime_put;
>  	}
>  
>  	return 0;
>  
>  err_pm_runtime_put:
> +	pm_runtime_put(dev);

When pm_runtime_get_sync() fails, pm_runtime_put_noidle() should be called in
the error path. But now pm_runtime_put() will be called, which may attempt
suspend.

- Mani

-- 
மணிவண்ணன் சதாசிவம்