Re: [PATCH v3 5/6] PCI: dwc: Ensure that dw_pcie_wait_for_link() waits 100 ms after link up

[Niklas Cassel <cassel@kernel.org>]

On Mon, Jun 23, 2025 at 08:28:55AM -0600, Manivannan Sadhasivam wrote:
> On Fri, Jun 13, 2025 at 02:48:44PM +0200, Niklas Cassel wrote:
> > As per PCIe r6.0, sec 6.6.1, a Downstream Port that supports Link speeds
> > greater than 5.0 GT/s, software must wait a minimum of 100 ms after Link
> > training completes before sending a Configuration Request.
> > 
> > Add this delay in dw_pcie_wait_for_link(), after the link is reported as
> > up. The delay will only be performed in the success case where the link
> > came up.
> > 
> > DWC glue drivers that have a link up IRQ (drivers that set
> > use_linkup_irq = true) do not call dw_pcie_wait_for_link(), instead they
> > perform this delay in their threaded link up IRQ handler.
> > 
> > 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 | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> > index 4d794964fa0f..24903f67d724 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -714,6 +714,13 @@ int dw_pcie_wait_for_link(struct dw_pcie *pci)
> >  		return -ETIMEDOUT;
> >  	}
> >  
> > +	/*
> > +	 * As per PCIe r6.0, sec 6.6.1, a Downstream Port that supports Link
> > +	 * speeds greater than 5.0 GT/s, software must wait a minimum of 100 ms
> > +	 * after Link training completes before sending a Configuration Request.
> > +	 */
> 
> As the comment clearly states, we should only wait if the downstream port
> supports link speed > 5.0 GT/s. So you should have the below check:
> 
> 	if (pci->max_link_speed > 1)
> 		msleep(PCIE_RESET_CONFIG_WAIT_MS);

PCIe 1.0 has 2.5 GT/s
PCIe 2.0 has 5.0 GT/s

Thus will assume that you actually meant:

if (pci->max_link_speed > 2)


Kind regards,
Niklas