On Tue, Jul 01, 2025 at 01:55:43PM GMT, Niklas Cassel wrote: > Hello Bjorn, Mani, > > On Mon, Jun 30, 2025 at 03:19:02PM -0500, Bjorn Helgaas wrote: > > On Wed, Jun 25, 2025 at 12:23:51PM +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@xxxxxxxxxx> > > > Reviewed-by: Wilfred Mallawa <wilfred.mallawa@xxxxxxx> > > > Signed-off-by: Niklas Cassel <cassel@xxxxxxxxxx> > > > --- > > > drivers/pci/controller/dwc/pcie-designware.c | 8 ++++++++ > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c > > > index 4d794964fa0f..053e9c540439 100644 > > > --- a/drivers/pci/controller/dwc/pcie-designware.c > > > +++ b/drivers/pci/controller/dwc/pcie-designware.c > > > @@ -714,6 +714,14 @@ 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. > > > + */ > > > + if (pci->max_link_speed > 2) > > > + msleep(PCIE_RESET_CONFIG_WAIT_MS); > > > > Sec 6.6.1 also requires "100 ms following exit from a Conventional > > Reset before sending a Configuration Request to the device immediately > > below that Port" for Downstream Ports that do *not* support Link > > speeds greater than 5.0 GT/s. > > > > Where does that delay happen? > > Argh... > > In version 3 of this series, the wait was unconditional, so it handled both > cases: > https://lore.kernel.org/linux-pci/20250613124839.2197945-13-cassel@xxxxxxxxxx/ > > But I got a review comment from Mani: > https://lore.kernel.org/linux-pci/hmkx6vjoqshthk5rqakcyzneredcg6q45tqhnaoqvmvs36zmsk@tzd7f44qkydq/ > > That I should only care about the greater than 5.0 GT/s case. > > Perhaps the confusion came about since the comment only mentioned one of the > two the cases specified by PCIe r6.0, sec 6.6.1. > Yes, that was intentional since the DWC core code only deals with the link up and not PERST# (which is handled by the glue drivers). > So I think the best thing is either (on top of pci/next): > No. The PERST# delay should be handled by the glue drivers since they are the ones controlling the PERST# line. Doing an unconditional wait for both the cases in DWC core, seems wrong to me. - Mani -- மணிவண்ணன் சதாசிவம்
