Re: [PATCH v3 6/6] PCI: dwc: Reduce LINK_WAIT_SLEEP_MS

[Niklas Cassel <cassel@kernel.org>]

On Mon, Jun 23, 2025 at 08:52:24AM -0600, Manivannan Sadhasivam wrote:
> On Fri, Jun 13, 2025 at 02:48:45PM +0200, Niklas Cassel wrote:
> > There is no reason for the delay, in each loop iteration, while polling for
> > link up (LINK_WAIT_SLEEP_MS), to be so long as 90 ms.
> > 
> > PCIe r6.0, sec 6.6.1, still require us to wait for up to 1.0 s for the link
> > to come up, thus the number of retries (LINK_WAIT_MAX_RETRIES) is increased
> > to keep the total timeout to 1.0 s.
> > 
> > PCIe r6.0, sec 6.6.1, also mandates that there is a 100 ms delay, after the
> > link has been established, before performing configuration requests (this
> > delay already exists in dw_pcie_wait_for_link() and is unchanged).
> > 
> > Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> > Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
> > Signed-off-by: Niklas Cassel <cassel@kernel.org>
> > ---
> >  drivers/pci/controller/dwc/pcie-designware.c |  6 +++++-
> >  drivers/pci/controller/dwc/pcie-designware.h | 13 +++++++++----
> >  2 files changed, 14 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 24903f67d724..ae6f0bfe3c56 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -701,7 +701,11 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
> >  	u32 offset, val;
> >  	int retries;
> >  
> > -	/* Check if the link is up or not */
> > +	/*
> > +	 * Check if the link is up or not. As per PCIe r6.0, sec 6.6.1, software
> > +	 * must allow at least 1.0 s following exit from a Conventional Reset of
> > +	 * a device, before determining that the device is broken.
> > +	 */
> >  	for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
> >  		if (dw_pcie_link_up(pci))
> >  			break;
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> > index ce9e18554e42..b225c4f3d36a 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > @@ -62,11 +62,16 @@
> >  #define dw_pcie_cap_set(_pci, _cap) \
> >  	set_bit(DW_PCIE_CAP_ ## _cap, &(_pci)->caps)
> >  
> > -/* Parameters for the waiting for link up routine */
> > -#define LINK_WAIT_MAX_RETRIES		10
> > -#define LINK_WAIT_SLEEP_MS		90
> > +/*
> > + * Parameters for waiting for a link to be established. As per PCIe r6.0,
> > + * sec 6.6.1, software must allow at least 1.0 s following exit from a
> > + * Conventional Reset of a device, before determining that the device is broken.
> > + * Therefore LINK_WAIT_MAX_RETRIES * LINK_WAIT_SLEEP_MS should equal 1.0 s.
> > + */
> > +#define LINK_WAIT_MAX_RETRIES		100
> > +#define LINK_WAIT_SLEEP_MS		10
> 
> These are not DWC specific. So I'd recommend moving it to drivers/pci/pci.h.

The total time to wait (LINK_WAIT_MAX_RETRIES * LINK_WAIT_SLEEP_MS) is not DWC
specific.

However, that we choose to wait 10 ms between polls is definitely DWC specific.

But sure, I can move these to drivers/pci/pci.h.


Kind regards,
Niklas