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?
> > 
> 
> I've just sent the fix:
> https://lore.kernel.org/linux-pci/20260226133951.296743-1-mani@xxxxxxxxxx
> 
> You can post your series on top of it. There should be no build dependency, but
> there is a functional dependency. So we may put this patch and your series in a
> single branch.

Okey,thanks! I have sent the v11 patch.

Kind regards,
Senchuan