Re: Re: [PATCH v10 2/2] PCI: eic7700: Add Eswin PCIe host controller driver

[zhangsenchuan <zhangsenchuan@eswincomputing.com>]

> > > Subject: Re: [PATCH v10 2/2] PCI: eic7700: Add Eswin PCIe host controller driver
> > > 
> > > On Thu, Jan 29, 2026 at 05:29:00PM +0800, zhangsenchuan@eswincomputing.com wrote:
> > > > From: Senchuan Zhang <zhangsenchuan@eswincomputing.com>
> > > >
> > > > +static int eic7700_pcie_suspend_noirq(struct device *dev)
> > > > +{
> > > > +	struct eic7700_pcie *pcie = dev_get_drvdata(dev);
> > > > +
> > > > +	/*
> > > > +	 * The ESWIN EIC7700 SoC lacks hardware support for the L2/L3 low-power
> > > > +	 * link states. It cannot enter the L2/L3 Ready state through the
> > > > +	 * PME_Turn_Off/PME_To_Ack handshake protocol. To avoid this problem,
> > > > +	 * the dw_pcie_suspend_noirq API is not used.
> > > > +	 */
> > > 
> > > With 7.0, you can provide a dummy pme_turn_off() API and set
> > > 'pci->pp.skip_l23_ready' to reuse the dw_pcie_{suspend/resume}_noirq APIs.
> > > 
> > 
> > Hi Mani,
> > 
> > Setting pci->pp.skip_l23_ready does indeed allow us to reuse the
> > dw_pcie_suspend_noirq function. However, for the dw_pcie_resume_noirq
> > function, if the dw_pcie_start_link and dw_pcie_wait_for_link APIs fail to
> > execute, the clk/reset resources in the pci->pp.ops->init function cannot
> > be released. Perhaps the dw_pcie_resume_noirq function needs to be optimized.
> 
> Will this help?
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 6ae6189e9b8a..38ad79bbeab1 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -1300,15 +1300,24 @@ int dw_pcie_resume_noirq(struct dw_pcie *pci)
>  
>         ret = dw_pcie_start_link(pci);
>         if (ret)
> -               return ret;
> +               goto err_deinit;
>  
>         ret = dw_pcie_wait_for_link(pci);
>         if (ret)
> -               return ret;
> +               goto err_stop_link;
>  
>         if (pci->pp.ops->post_init)
>                 pci->pp.ops->post_init(&pci->pp);
>  
> +       return 0;
> +
> +err_stop_link:
> +       dw_pcie_stop_link(pci);
> +
> +err_deinit:
> +       if (pci->pp.ops->deinit)
> +               pci->pp.ops->deinit(&pci->pp);
> +
>         return ret;
>  }
>  EXPORT_SYMBOL_GPL(dw_pcie_resume_noirq);
> 

Yes, this can release the resources after init, after optimizing the 
resume function, i can reuse the dw_pcie_{suspend/resume}_noirq APIs.

I noticed that the dw_pcie_wait_for_link function has been optimized. Is 
it necessary to release the resources only when it return -ETIMEOUT?
Perhaps it needs to be slightly improved：

 ret = dw_pcie_wait_for_link(pci);
 if (ret == -ETIMEOUT)
         goto err_stop_link;

What about your suggestion?

Kind regards,
Senchuan