[PATCH v3 0/6] PCI: dwc: Do not enumerate bus before endpoint devices are ready

[PATCH v3 0/6] PCI: dwc: Do not enumerate bus before endpoint devices are ready

[Niklas Cassel]

Hello all,

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 1/4 addresses a regression where a Plextor
NVMe drive fails to be configured correctly. With this series, the
Plextor NVMe drive works once again.


Kind regards,
Niklas


Changes since v2:
-Rename PCIE_RESET_CONFIG_DEVICE_WAIT_MS to PCIE_RESET_CONFIG_WAIT_MS.


Niklas Cassel (6):
  PCI: Rename PCIE_RESET_CONFIG_DEVICE_WAIT_MS to
    PCIE_RESET_CONFIG_WAIT_MS
  PCI: rockchip-host: Use macro PCIE_RESET_CONFIG_WAIT_MS
  PCI: dw-rockchip: Wait PCIE_RESET_CONFIG_WAIT_MS after link-up IRQ
  PCI: qcom: Wait PCIE_RESET_CONFIG_WAIT_MS after link-up IRQ
  PCI: dwc: Ensure that dw_pcie_wait_for_link() waits 100 ms after link
    up
  PCI: dwc: Reduce LINK_WAIT_SLEEP_MS

 drivers/pci/controller/dwc/pcie-designware.c  | 13 ++++++++++++-
 drivers/pci/controller/dwc/pcie-designware.h  | 13 +++++++++----
 drivers/pci/controller/dwc/pcie-dw-rockchip.c |  1 +
 drivers/pci/controller/dwc/pcie-qcom.c        |  1 +
 drivers/pci/controller/pcie-rockchip-host.c   |  2 +-
 drivers/pci/controller/plda/pcie-starfive.c   |  2 +-
 drivers/pci/pci.h                             |  9 +--------
 7 files changed, 26 insertions(+), 15 deletions(-)

-- 
2.49.0

[PATCH v3 4/6] PCI: qcom: Wait PCIE_RESET_CONFIG_WAIT_MS after link-up IRQ

[Niklas Cassel]

Per PCIe r6.0, sec 6.6.1, software must generally wait a minimum of
100ms (PCIE_RESET_CONFIG_WAIT_MS) after Link training completes before
sending a Configuration Request.

Prior to 36971d6c5a9a ("PCI: qcom: Don't wait for link if we can detect
Link Up"), qcom used dw_pcie_wait_for_link(), which waited between 0
and 90ms after the link came up before we enumerate the bus, and this
was apparently enough for most devices.

After 36971d6c5a9a, qcom_pcie_global_irq_thread() started enumeration
immediately when handling the link-up IRQ, and devices (e.g., Laszlo
Fiat's PLEXTOR PX-256M8PeGN NVMe SSD) may not be ready to handle config
requests yet.

Delay PCIE_RESET_CONFIG_WAIT_MS after the link-up IRQ before starting
enumeration.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
Fixes: 82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller driver")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index c789e3f85655..9b12f2f02042 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1564,6 +1564,7 @@ static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
 	writel_relaxed(status, pcie->parf + PARF_INT_ALL_CLEAR);
 
 	if (FIELD_GET(PARF_INT_ALL_LINK_UP, status)) {
+		msleep(PCIE_RESET_CONFIG_WAIT_MS);
 		dev_dbg(dev, "Received Link up event. Starting enumeration!\n");
 		/* Rescan the bus to enumerate endpoint devices */
 		pci_lock_rescan_remove();
-- 
2.49.0

Re: [PATCH v3 0/6] PCI: dwc: Do not enumerate bus before endpoint devices are ready

[Niklas Cassel]

On Fri, Jun 13, 2025 at 02:48:39PM +0200, Niklas Cassel wrote:
> Hello all,
> 
> 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 1/4 addresses a regression where a Plextor
> NVMe drive fails to be configured correctly. With this series, the
> Plextor NVMe drive works once again.
> 
> 
> Kind regards,
> Niklas
> 
> 
> Changes since v2:
> -Rename PCIE_RESET_CONFIG_DEVICE_WAIT_MS to PCIE_RESET_CONFIG_WAIT_MS.
> 
> 
> Niklas Cassel (6):
>   PCI: Rename PCIE_RESET_CONFIG_DEVICE_WAIT_MS to
>     PCIE_RESET_CONFIG_WAIT_MS
>   PCI: rockchip-host: Use macro PCIE_RESET_CONFIG_WAIT_MS
>   PCI: dw-rockchip: Wait PCIE_RESET_CONFIG_WAIT_MS after link-up IRQ
>   PCI: qcom: Wait PCIE_RESET_CONFIG_WAIT_MS after link-up IRQ
>   PCI: dwc: Ensure that dw_pcie_wait_for_link() waits 100 ms after link
>     up
>   PCI: dwc: Reduce LINK_WAIT_SLEEP_MS
> 
>  drivers/pci/controller/dwc/pcie-designware.c  | 13 ++++++++++++-
>  drivers/pci/controller/dwc/pcie-designware.h  | 13 +++++++++----
>  drivers/pci/controller/dwc/pcie-dw-rockchip.c |  1 +
>  drivers/pci/controller/dwc/pcie-qcom.c        |  1 +
>  drivers/pci/controller/pcie-rockchip-host.c   |  2 +-
>  drivers/pci/controller/plda/pcie-starfive.c   |  2 +-
>  drivers/pci/pci.h                             |  9 +--------
>  7 files changed, 26 insertions(+), 15 deletions(-)
> 
> -- 
> 2.49.0
> 

Gentle ping

Re: [PATCH v3 4/6] PCI: qcom: Wait PCIE_RESET_CONFIG_WAIT_MS after link-up IRQ

[Manivannan Sadhasivam]

On Fri, Jun 13, 2025 at 02:48:43PM +0200, Niklas Cassel wrote:
> Per PCIe r6.0, sec 6.6.1, software must generally wait a minimum of
> 100ms (PCIE_RESET_CONFIG_WAIT_MS) after Link training completes before
> sending a Configuration Request.
> 
> Prior to 36971d6c5a9a ("PCI: qcom: Don't wait for link if we can detect
> Link Up"), qcom used dw_pcie_wait_for_link(), which waited between 0
> and 90ms after the link came up before we enumerate the bus, and this
> was apparently enough for most devices.
> 
> After 36971d6c5a9a, qcom_pcie_global_irq_thread() started enumeration
> immediately when handling the link-up IRQ, and devices (e.g., Laszlo
> Fiat's PLEXTOR PX-256M8PeGN NVMe SSD) may not be ready to handle config
> requests yet.
> 
> Delay PCIE_RESET_CONFIG_WAIT_MS after the link-up IRQ before starting
> enumeration.
> 
> Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
> Fixes: 82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller driver")

Shouldn't 36971d6c5a9a be the fixes commit?

- Mani

> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index c789e3f85655..9b12f2f02042 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1564,6 +1564,7 @@ static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
>  	writel_relaxed(status, pcie->parf + PARF_INT_ALL_CLEAR);
>  
>  	if (FIELD_GET(PARF_INT_ALL_LINK_UP, status)) {
> +		msleep(PCIE_RESET_CONFIG_WAIT_MS);
>  		dev_dbg(dev, "Received Link up event. Starting enumeration!\n");
>  		/* Rescan the bus to enumerate endpoint devices */
>  		pci_lock_rescan_remove();
> -- 
> 2.49.0
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v3 4/6] PCI: qcom: Wait PCIE_RESET_CONFIG_WAIT_MS after link-up IRQ

[Niklas Cassel]

On Mon, Jun 23, 2025 at 08:27:19AM -0600, Manivannan Sadhasivam wrote:
> On Fri, Jun 13, 2025 at 02:48:43PM +0200, Niklas Cassel wrote:
> > Per PCIe r6.0, sec 6.6.1, software must generally wait a minimum of
> > 100ms (PCIE_RESET_CONFIG_WAIT_MS) after Link training completes before
> > sending a Configuration Request.
> > 
> > Prior to 36971d6c5a9a ("PCI: qcom: Don't wait for link if we can detect
> > Link Up"), qcom used dw_pcie_wait_for_link(), which waited between 0
> > and 90ms after the link came up before we enumerate the bus, and this
> > was apparently enough for most devices.
> > 
> > After 36971d6c5a9a, qcom_pcie_global_irq_thread() started enumeration
> > immediately when handling the link-up IRQ, and devices (e.g., Laszlo
> > Fiat's PLEXTOR PX-256M8PeGN NVMe SSD) may not be ready to handle config
> > requests yet.
> > 
> > Delay PCIE_RESET_CONFIG_WAIT_MS after the link-up IRQ before starting
> > enumeration.
> > 
> > Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
> > Reviewed-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
> > Fixes: 82a823833f4e ("PCI: qcom: Add Qualcomm PCIe controller driver")
> 
> Shouldn't 36971d6c5a9a be the fixes commit?

See Bjorn's comment:
https://lore.kernel.org/linux-pci/20250611211456.GA869983@bhelgaas/

	I would argue that 0e898eb8df4e ("PCI: rockchip-dwc: Add Rockchip
	RK356X host controller driver") is the right Fixes: commit here
	because dw_pcie_wait_for_link() *never* waited the required time, and
	it's quite possible that other devices don't work correctly.  The
	delay was about 90ms - <time required for link training>, so could be
	significantly less than 100ms.


Thus, following Bjorn's comment, to put the commit that introduced the driver
as the Fixes tag for dw-rockchip, I did the same thing for qcom.


Kind regards,
Niklas