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

[zhangsenchuan]

> > > > > 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@xxxxxxxxxxxxxxxxxx wrote:
> > > > > > From: Senchuan Zhang <zhangsenchuan@xxxxxxxxxxxxxxxxxx>
> > > > > >
> > > > > > +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?
> > 
> 
> Absolutely! I forgot my own rework ;) I'll cook a patch for the above. Then if
> you base your controller driver patch on top of it, we can merge both in a
> single tree (if Bjorn agrees).
> 

Okey,thanks!

I'm a little unsure. Do I need to send the v11 patch here first? Or should I wait
until you release the new fix patch, and then send the v11 patch?

Kind regards,
Senchuan