PCI: dwc: Do not enumerate bus before endpoint devices are ready

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 <[email protected]>

Reviewed-by Wilfred Mallawa <[email protected]>

Signed-off-by Niklas Cassel <[email protected]>

---
 drivers/pci/controller/dwc/pcie-designware.c | 8 ++++++++
 1 file changed, 8 insertions(+)

drivers/pci/controller/dwc/pcie-designware.c +0 -0

index 4d794964fa0f..053e9c540439 100644

drivers/pci/controller/dwc/pcie-designware.c +0 -0

 	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
 	val = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA);
 

Signature · 1 line

2.49.0

Quoted 24 lines · Niklas Cassel

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 <[email protected]>

Reviewed-by Wilfred Mallawa <[email protected]>

Signed-off-by Niklas Cassel <[email protected]>

---
 drivers/pci/controller/dwc/pcie-designware.c | 8 ++++++++
 1 file changed, 8 insertions(+)

drivers/pci/controller/dwc/pcie-designware.c +0 -0

index 4d794964fa0f..053e9c540439 100644

drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_wait_for_link(struct dw_pcie *pci) · line 723

Quoted 10 lines

drivers/pci/controller/dwc/pcie-designware.c +7 -0

@@ -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?

Hello Bjorn, Mani,

Quoted 41 lines · Bjorn Helgaas

Quoted 24 lines · Niklas Cassel

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 <[email protected]>

Reviewed-by Wilfred Mallawa <[email protected]>

Signed-off-by Niklas Cassel <[email protected]>

---
 drivers/pci/controller/dwc/pcie-designware.c | 8 ++++++++
 1 file changed, 8 insertions(+)

drivers/pci/controller/dwc/pcie-designware.c +0 -0

index 4d794964fa0f..053e9c540439 100644

drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_wait_for_link(struct dw_pcie *pci) · line 723

Quoted 10 lines

drivers/pci/controller/dwc/pcie-designware.c +7 -0

@@ -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.

Quoted 1 line

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/[email protected]/

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.

So I think the best thing is either (on top of pci/next):

Quoted 50 lines · Niklas Cassel

Quoted 41 lines · Bjorn Helgaas

Quoted 24 lines · Niklas Cassel

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 <[email protected]>

Reviewed-by Wilfred Mallawa <[email protected]>

Signed-off-by Niklas Cassel <[email protected]>

---
 drivers/pci/controller/dwc/pcie-designware.c | 8 ++++++++
 1 file changed, 8 insertions(+)

drivers/pci/controller/dwc/pcie-designware.c +0 -0

index 4d794964fa0f..053e9c540439 100644

drivers/pci/controller/dwc/pcie-designware.c int dw_pcie_wait_for_link(struct dw_pcie *pci) · line 723

Quoted 10 lines

drivers/pci/controller/dwc/pcie-designware.c +7 -0

@@ -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.

Quoted 1 line

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/[email protected]/

Quoted 7 lines

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).

Quoted 1 line

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.

Quoted 5 lines

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.

It ends up being a little bit weird that the delay is in the DWC core (dw_pcie_wait_for_link()) for ports that support fast links, and in the glue drivers otherwise. It would be easier to verify and maintain if the delay were always in the DWC core.

If we had a dw_pcie_host_ops callback for PERST# deassertion, the delay could be in the DWC core, but I don't know if there's enough consistency across drivers for that to be practical.

Quoted 11 lines · Bjorn Helgaas

Quoted 5 lines

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.

It ends up being a little bit weird that the delay is in the DWC core (dw_pcie_wait_for_link()) for ports that support fast links, and in the glue drivers otherwise. It would be easier to verify and maintain if the delay were always in the DWC core.

Quoted 3 lines

If we had a dw_pcie_host_ops callback for PERST# deassertion, the delay could be in the DWC core, but I don't know if there's enough consistency across drivers for that to be practical.

Currently, there is not much consistency between the glue drivers, so adding a DWC core API to assert/deassert PERST# sounds like a good idea to me. The callback could even be supplied a struct gpio_desc pointer.

Like I mentioned in my previous email:
https://github.com/torvalds/linux/blob/v6.16-rc4/drivers/pci/controller/dwc/pci-imx6.c#L885
https://github.com/torvalds/linux/blob/v6.16-rc4/drivers/pci/controller/dwc/pcie-qcom.c#L294
https://github.com/torvalds/linux/blob/v6.16-rc4/drivers/pci/controller/dwc/pcie-keembay.c#L89

these drivers seem to have a 100 ms delay after PERST# has been deasserted, but there are of course more glue drivers, so a lot of them will not have a 100 ms wait _after_ PERST# is deasserted. (All glue drivers seem to have a delay between asserting and deasserting PERST#.)

Right now, e.g. qcom will have a 100 ms delay both after deasserting PERST# and after link up. (However, based on the supported link speed, only one of the delays should be needed.)

However, my main concern is not that qcom waits twice, it is those drivers that do not have a 100 ms delay after PERST# has been deasserted, because before commit 470f10f18b48 ("PCI: Reduce PCIE_LINK_WAIT_SLEEP_MS"), those drivers might have been "saved" by the ridiculously long PCIE_LINK_WAIT_SLEEP_MS.

However, now when we sleep less in each iteration when polling for link up, those drivers that do not have a 100 ms delay after PERST# has been deasserted might actually see regressions, because (the now reduced) PCIE_LINK_WAIT_SLEEP_MS time is no longer "saving" them.

Quoted 6 lines · Niklas Cassel

However, my main concern is not that qcom waits twice, it is those drivers that do not have a 100 ms delay after PERST# has been deasserted, because before commit 470f10f18b48 ("PCI: Reduce PCIE_LINK_WAIT_SLEEP_MS"), those drivers might have been "saved" by the ridiculously long PCIE_LINK_WAIT_SLEEP_MS.

Quoted 4 lines

However, now when we sleep less in each iteration when polling for link up, those drivers that do not have a 100 ms delay after PERST# has been deasserted might actually see regressions, because (the now reduced) PCIE_LINK_WAIT_SLEEP_MS time is no longer "saving" them.

Why can't you just add the delay to those drivers now? They can be consolidated later.

Hello Mani,

Quoted 7 lines · Manivannan Sadhasivam

Quoted 6 lines · Niklas Cassel

However, my main concern is not that qcom waits twice, it is those drivers that do not have a 100 ms delay after PERST# has been deasserted, because before commit 470f10f18b48 ("PCI: Reduce PCIE_LINK_WAIT_SLEEP_MS"), those drivers might have been "saved" by the ridiculously long PCIE_LINK_WAIT_SLEEP_MS.

Quoted 8 lines

Quoted 4 lines

However, now when we sleep less in each iteration when polling for link up, those drivers that do not have a 100 ms delay after PERST# has been deasserted might actually see regressions, because (the now reduced) PCIE_LINK_WAIT_SLEEP_MS time is no longer "saving" them.

Why can't you just add the delay to those drivers now? They can be consolidated later.

Right now, I don't have the extra cycles needed to read all of these drivers:

$ git grep -l gpio drivers/pci/controller/dwc/
drivers/pci/controller/dwc/pci-dra7xx.c
drivers/pci/controller/dwc/pci-imx6.c
drivers/pci/controller/dwc/pci-keystone.c
drivers/pci/controller/dwc/pci-meson.c
drivers/pci/controller/dwc/pcie-amd-mdb.c
drivers/pci/controller/dwc/pcie-bt1.c
drivers/pci/controller/dwc/pcie-designware-plat.c
drivers/pci/controller/dwc/pcie-designware.c
drivers/pci/controller/dwc/pcie-designware.h
drivers/pci/controller/dwc/pcie-dw-rockchip.c
drivers/pci/controller/dwc/pcie-fu740.c
drivers/pci/controller/dwc/pcie-histb.c
drivers/pci/controller/dwc/pcie-intel-gw.c
drivers/pci/controller/dwc/pcie-keembay.c
drivers/pci/controller/dwc/pcie-kirin.c
drivers/pci/controller/dwc/pcie-qcom-ep.c
drivers/pci/controller/dwc/pcie-qcom.c
drivers/pci/controller/dwc/pcie-rcar-gen4.c
drivers/pci/controller/dwc/pcie-tegra194.c
drivers/pci/controller/dwc/pcie-visconti.c

then understanding them properly to feel that I am confident in my changes, waiting for reviews from each glue driver maintainer, and then waiting for someone to pick it up.

Also, all of the above has to be done when consolidating the PERST# handling anyway.

This whole thing started because someone reported a regression on a random Plextor NVMe drive. Since it was my commit ec9fd499b9c6 ("PCI: dw-rockchip:
Don't wait for link since we can detect Link Up") that introduced the regression, I obviously helped debugging the issue.

The regression was solved by adding a 100 ms delay after receiving the link up IRQ, before enumerating the bus.

This fix was sent May 5th:
https://lore.kernel.org/linux-pci/[email protected]/
(This series had up to v2, the series was then renamed and had up to v4, so basically v6 in total.)

While my responsibility was done two months ago, I still wanted to make sure that no one else got bit by the same bug, thus I also improved the generic dw_pcie_wait_for_link() (for those drivers not using link up IRQ):
https://lore.kernel.org/linux-pci/[email protected]/

Sure, that will only help PCIe controllers that support Link speeds greater than 5.0 GT/s, and do not use a link up IRQ, but still something.

PCIe controllers that only support Link speeds <= 5.0 GT/s, and do not use a link up IRQ, and do not have a delay after deasserting PERST#, can still send config requests too early.

However, if we drop 470f10f18b48 ("PCI: Reduce PCIE_LINK_WAIT_SLEEP_MS"), PCIe controllers that only support Link speeds <= 5.0 GT/s, and do not use a link up IRQ, and do not have a delay after deasserting PERST#, will be no worse off than what is already in mainline.

PCIe controllers that support Link speeds greater than 5.0 GT/s, and do not use a link up IRQ, will still be more robust than ever :)
(Rome wasn't built in a day.)

On Wed, Jul 02, 2025 at 06:17:41PM GMT, Niklas Cassel wrote:

Sounds fair! I've now dropped 470f10f18b48 from controller/linkup-fix branch.

Do you have cycles for consolidating PERST# handling?

Hello Mani,

Quoted 5 lines · Manivannan Sadhasivam

On Wed, Jul 02, 2025 at 06:17:41PM GMT, Niklas Cassel wrote:

Sounds fair! I've now dropped 470f10f18b48 from controller/linkup-fix branch.

Do you have cycles for consolidating PERST# handling?

While I think that consolidating PERST# handling is a great idea, I already have 3-4 things with higher priority on my TODO list (and some things have been there for quite a while).

So unless something has rather high priority, I currently don't know when I will have some spare cycles.

I really hope that someone will have some spare cycles to work on this, because the PERST# handling in the controller drivers really is a mess.

Quoted 12 lines · Niklas Cassel

Quoted 11 lines · Bjorn Helgaas

Quoted 5 lines

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.

It ends up being a little bit weird that the delay is in the DWC core (dw_pcie_wait_for_link()) for ports that support fast links, and in the glue drivers otherwise. It would be easier to verify and maintain if the delay were always in the DWC core.

Quoted 8 lines

Quoted 3 lines

If we had a dw_pcie_host_ops callback for PERST# deassertion, the delay could be in the DWC core, but I don't know if there's enough consistency across drivers for that to be practical.

Currently, there is not much consistency between the glue drivers, so adding a DWC core API to assert/deassert PERST# sounds like a good idea to me. The callback could even be supplied a struct gpio_desc pointer.

+1 for consolidating the PERST# handling. But just FYI, I'm going to move the PERST# deassert handling from controller drivers to pwrctrl drivers as once pwrctrl drivers are used to control the power supplies, they should control PERST# as well. Currently, this is the missing piece for pwrctrl framework.

Also, we cannot entirely get rid of the PERST# handling in controller drivers, since they need to assert PERST# before resetting the RC during init.

The DWC PCIe controller driver currently does not follow the PCIe specification with regards to the delays after link training, before sending out configuration requests. This series fixes this.

At the same time, PATCH 3/7 addresses a regression where a Plextor NVMe drive fails to be configured correctly. With this series, the Plextor NVMe drive works once again.

[...]