[PATCH v3 0/7] PCI: Add common helper for 100 ms delay after link training

[PATCH v3 0/7] PCI: Add common helper for 100 ms delay after link training

[Hans Zhang]

PCIe r6.0, sec 6.6.1 (Conventional Reset) requires that for a Downstream
Port supporting Link speeds greater than 5.0 GT/s, software must wait a
minimum of 100 ms after Link training completes before sending any
Configuration Request.

Several PCIe host controller drivers currently omit this 100 ms delay
when the negotiated link speed is Gen3 (8 GT/s) or higher. Only the DWC
driver already implements it. The missing delay can lead to violations
of the PCIe specification and cause enumeration failures with high-speed
devices (e.g., NVIDIA RTX5070 GPU, PCIe 5.0 NVMe SSDs).

To fix this consistently and avoid code duplication, this series:

  1. Adds a static inline helper `pci_host_common_link_train_delay()`
     in `drivers/pci/controller/pci-host-common.h`. The helper checks
     the given max_link_speed and calls msleep(100) if speed > 5 GT/s.

  2. Converts the DWC driver to use this helper.

  3. Adds the missing 100 ms delay to the Cadence PCIe controller
     (both LGA and HPA IPs). A `max_link_speed` field is introduced
     in `struct cdns_pcie`. The j721e glue driver sets this field;
     other LGA glue drivers fall back to reading DT property
     "max-link-speed". HPA also reads the DT property.

  4. Adds the delay to the Aardvark and MediaTek Gen3 host drivers,
     and replaces the existing unconditional delay in the Renesas
     RZ/G3S driver with the conditional helper (per spec).

All changes are placed immediately after link training completes and
before any Configuration Request would be issued.

---
Our company's product is based on the HPA IP from Cadence. When connecting
to different devices, we encountered issues with the enumeration failure
when connecting to the NVIDIA RTX5070 GPU and the NVMe SSD with PCIe 5.0
interface. Our code is based on: 80dc18a0cba8d ("PCI: dwc: Ensure that
dw_pcie_wait_for_link() waits 100 ms after link up").
---
Changes since v3:
- Renamed helper from pcie_wait_after_link_train() to
  pci_host_common_link_train_delay() and moved to pci-host-common.h. (Mani)
- Reorganized Cadence patches: LGA + j721e glue in one patch, HPA separate.
- Added missing DT property reading for HPA and LGA fallback.
- Replace the existing msleep(100) call with the common helper
  pci_host_common_link_train_delay(). (Claudiu)

Changes since v2:
- Add pcie_wait_after_link_train() helper
- Reduce repetitive code comments and have each Root Port driver use the
  helper function instead.
- Increase the delay to 100ms after enabling the link-up that distinguishes
  between Cadence LGA and HPA IPs.
- Add the Aardvark, MediaTek Gen3, and Renesas RZ/G3S Root Port driver. When
  the speed is greater than GEN2, a delay of 100ms should be applied.

v2:
https://patchwork.kernel.org/project/linux-pci/cover/20260506152346.166056-1-18255117159@163.com/

v1:
https://patchwork.kernel.org/project/linux-pci/patch/20260501153553.66382-1-18255117159@163.com/
---
Hans Zhang (7):
  PCI: Add pci_host_common_link_train_delay() helper
  PCI: cadence: Add post-link delay for LGA and j721e glue driver
  PCI: cadence: HPA: Add post-link delay
  PCI: dwc: Use common pci_host_common_link_train_delay() helper
  PCI: aardvark: Add 100 ms delay after link training
  PCI: mediatek-gen3: Add 100 ms delay after link up
  PCI: rzg3s-host: Use common pci_host_common_link_train_delay() helper

 drivers/pci/controller/cadence/pci-j721e.c      |  1 +
 .../cadence/pcie-cadence-host-common.c          |  4 ++++
 .../controller/cadence/pcie-cadence-host-hpa.c  |  8 ++++++++
 .../pci/controller/cadence/pcie-cadence-host.c  |  4 ++++
 drivers/pci/controller/cadence/pcie-cadence.h   |  2 ++
 drivers/pci/controller/dwc/pcie-designware.c    |  9 ++-------
 drivers/pci/controller/pci-aardvark.c           |  5 ++++-
 drivers/pci/controller/pci-host-common.h        | 17 +++++++++++++++++
 drivers/pci/controller/pcie-mediatek-gen3.c     |  3 +++
 drivers/pci/controller/pcie-rzg3s-host.c        |  3 ++-
 10 files changed, 47 insertions(+), 9 deletions(-)


base-commit: 70390501d1944d4e5b8f7352be180fceb3a44132
-- 
2.34.1

[PATCH v3 1/7] PCI: Add pci_host_common_link_train_delay() helper

[Hans Zhang]

PCIe r6.0, sec 6.6.1 (Conventional Reset) requires that for a Downstream
Port supporting Link speeds greater than 5.0 GT/s, software must wait a
minimum of 100 ms after Link training completes before sending any
Configuration Request.

Introduce a static inline helper pci_host_common_link_train_delay() that
checks the given max_link_speed (2 = 5.0 GT/s, 3 = 8.0 GT/s, etc.) and
calls msleep(100) only when the speed is greater than 5.0 GT/s.

This allows multiple host controller drivers to share the same mandatory
delay without duplicating the logic.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/pci-host-common.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/pci/controller/pci-host-common.h b/drivers/pci/controller/pci-host-common.h
index b5075d4bd7eb..d709f7e3e11a 100644
--- a/drivers/pci/controller/pci-host-common.h
+++ b/drivers/pci/controller/pci-host-common.h
@@ -10,6 +10,9 @@
 #ifndef _PCI_HOST_COMMON_H
 #define _PCI_HOST_COMMON_H
 
+#include <linux/delay.h>
+#include "../pci.h"
+
 struct pci_ecam_ops;
 
 int pci_host_common_probe(struct platform_device *pdev);
@@ -20,4 +23,18 @@ void pci_host_common_remove(struct platform_device *pdev);
 
 struct pci_config_window *pci_host_common_ecam_create(struct device *dev,
 	struct pci_host_bridge *bridge, const struct pci_ecam_ops *ops);
+
+/**
+ * pci_host_common_link_train_delay - Wait 100 ms if link speed > 5 GT/s
+ * @max_link_speed: the maximum link speed (2 = 5.0 GT/s, 3 = 8.0 GT/s, ...)
+ *
+ * Must be called after Link training completes and before the first
+ * Configuration Request is sent.
+ */
+static inline void pci_host_common_link_train_delay(int max_link_speed)
+{
+	if (max_link_speed > 2)
+		msleep(PCIE_RESET_CONFIG_WAIT_MS);
+}
+
 #endif
-- 
2.34.1

[PATCH v3 2/7] PCI: cadence: Add post-link delay for LGA and j721e glue driver

[Hans Zhang]

The Cadence LGA (Legacy Architecture IP) PCIe host controller currently
lacks the mandatory 100 ms delay after link training completes for speeds
> 5.0 GT/s, as required by PCIe r6.0 sec 6.6.1.

Add a 'max_link_speed' field to struct cdns_pcie. In the common host
layer function cdns_pcie_host_start_link(), after the link has been
successfully established, call pci_host_common_link_train_delay() to
insert the required delay.

For the j721e glue driver, set cdns_pcie.max_link_speed from the existing
link speed logic. For other LGA-based glue drivers (sky1, sg2042), the
common LGA host setup (pcie-cadence-host.c) provides a fallback reading
of the device tree property "max-link-speed" when available. This ensures
that the delay is not missed on those platforms once they enable the
property.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/cadence/pci-j721e.c                | 1 +
 drivers/pci/controller/cadence/pcie-cadence-host-common.c | 4 ++++
 drivers/pci/controller/cadence/pcie-cadence-host.c        | 4 ++++
 drivers/pci/controller/cadence/pcie-cadence.h             | 2 ++
 4 files changed, 11 insertions(+)

diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
index bfdfe98d5aba..ee85b8e04f5b 100644
--- a/drivers/pci/controller/cadence/pci-j721e.c
+++ b/drivers/pci/controller/cadence/pci-j721e.c
@@ -206,6 +206,7 @@ static int j721e_pcie_set_link_speed(struct j721e_pcie *pcie,
 	    (pcie_get_link_speed(link_speed) == PCI_SPEED_UNKNOWN))
 		link_speed = 2;
 
+	pcie->cdns_pcie.max_link_speed = link_speed;
 	val = link_speed - 1;
 	ret = regmap_update_bits(syscon, offset, GENERATION_SEL_MASK, val);
 	if (ret)
diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-common.c b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
index 2b0211870f02..18e4b6c760b5 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host-common.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
@@ -14,6 +14,7 @@
 
 #include "pcie-cadence.h"
 #include "pcie-cadence-host-common.h"
+#include "../pci-host-common.h"
 
 #define LINK_RETRAIN_TIMEOUT HZ
 
@@ -115,6 +116,9 @@ int cdns_pcie_host_start_link(struct cdns_pcie_rc *rc,
 	if (!ret && rc->quirk_retrain_flag)
 		ret = cdns_pcie_retrain(pcie, pcie_link_up);
 
+	if (!ret)
+		pci_host_common_link_train_delay(pcie->max_link_speed);
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(cdns_pcie_host_start_link);
diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
index 0bc9e6e90e0e..058e4e619654 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
@@ -13,6 +13,7 @@
 
 #include "pcie-cadence.h"
 #include "pcie-cadence-host-common.h"
+#include "../../pci.h"
 
 static u8 bar_aperture_mask[] = {
 	[RP_BAR0] = 0x1F,
@@ -397,6 +398,9 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
 	rc->device_id = 0xffff;
 	of_property_read_u32(np, "device-id", &rc->device_id);
 
+	if (pcie->max_link_speed < 1)
+		pcie->max_link_speed = of_pci_get_max_link_speed(np);
+
 	pcie->reg_base = devm_platform_ioremap_resource_byname(pdev, "reg");
 	if (IS_ERR(pcie->reg_base)) {
 		dev_err(dev, "missing \"reg\"\n");
diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
index 574e9cf4d003..042a4c49bb9a 100644
--- a/drivers/pci/controller/cadence/pcie-cadence.h
+++ b/drivers/pci/controller/cadence/pcie-cadence.h
@@ -86,6 +86,7 @@ struct cdns_plat_pcie_of_data {
  * @ops: Platform-specific ops to control various inputs from Cadence PCIe
  *       wrapper
  * @cdns_pcie_reg_offsets: Register bank offsets for different SoC
+ * @max_link_speed: Maximum supported link speed
  */
 struct cdns_pcie {
 	void __iomem		             *reg_base;
@@ -98,6 +99,7 @@ struct cdns_pcie {
 	struct device_link	             **link;
 	const  struct cdns_pcie_ops          *ops;
 	const  struct cdns_plat_pcie_of_data *cdns_pcie_reg_offsets;
+	int				     max_link_speed;
 };
 
 /**
-- 
2.34.1

[PATCH v3 3/7] PCI: cadence: HPA: Add post-link delay

[Hans Zhang]

The Cadence HPA (High Performance Architecture IP) specific link setup
function cdns_pcie_hpa_host_link_setup() waits for the link to come up
but does not implement the required 100 ms delay after link training
completes for speeds > 5.0 GT/s (PCIe r6.0 sec 6.6.1).

Add a call to pci_host_common_link_train_delay() immediately after the
link is confirmed to be up, using the max_link_speed field. Also, in the
HPA host setup function, read the device tree property "max-link-speed"
to initialize max_link_speed if not already set by a glue driver.

This ensures compliance for HPA-based platforms.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/cadence/pcie-cadence-host-hpa.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c b/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
index 0f540bed58e8..8ef58ed01daa 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
@@ -15,6 +15,8 @@
 
 #include "pcie-cadence.h"
 #include "pcie-cadence-host-common.h"
+#include "../pci-host-common.h"
+#include "../../pci.h"
 
 static u8 bar_aperture_mask[] = {
 	[RP_BAR0] = 0x3F,
@@ -304,6 +306,8 @@ int cdns_pcie_hpa_host_link_setup(struct cdns_pcie_rc *rc)
 	ret = cdns_pcie_host_wait_for_link(pcie, cdns_pcie_hpa_link_up);
 	if (ret)
 		dev_dbg(dev, "PCIe link never came up\n");
+	else
+		pci_host_common_link_train_delay(pcie->max_link_speed);
 
 	return ret;
 }
@@ -313,6 +317,7 @@ int cdns_pcie_hpa_host_setup(struct cdns_pcie_rc *rc)
 {
 	struct device *dev = rc->pcie.dev;
 	struct platform_device *pdev = to_platform_device(dev);
+	struct device_node *np = dev->of_node;
 	struct pci_host_bridge *bridge;
 	enum   cdns_pcie_rp_bar bar;
 	struct cdns_pcie *pcie;
@@ -343,6 +348,9 @@ int cdns_pcie_hpa_host_setup(struct cdns_pcie_rc *rc)
 		rc->cfg_res = res;
 	}
 
+	if (pcie->max_link_speed < 1)
+		pcie->max_link_speed = of_pci_get_max_link_speed(np);
+
 	/* Put EROM Bar aperture to 0 */
 	cdns_pcie_hpa_writel(pcie, REG_BANK_IP_CFG_CTRL_REG, CDNS_PCIE_EROM, 0x0);
 
-- 
2.34.1

[PATCH v3 4/7] PCI: dwc: Use common pci_host_common_link_train_delay() helper

[Hans Zhang]

The DWC driver already implements the 100 ms delay required by PCIe
r6.0 sec 6.6.1 by checking pci->max_link_speed and calling msleep(100).

Replace the open-coded msleep() with the new common helper
pci_host_common_link_train_delay() to reduce code duplication and
improve maintainability. No functional change intended.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/dwc/pcie-designware.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index c11cf61b8319..7021d21bb601 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -22,6 +22,7 @@
 #include <linux/sizes.h>
 #include <linux/types.h>
 
+#include "../pci-host-common.h"
 #include "../../pci.h"
 #include "pcie-designware.h"
 
@@ -799,13 +800,7 @@ 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);
+	pci_host_common_link_train_delay(pci->max_link_speed);
 
 	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
 	val = dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKSTA);
-- 
2.34.1

[PATCH v3 5/7] PCI: aardvark: Add 100 ms delay after link training

[Hans Zhang]

The Aardvark PCIe controller driver waits for the link to come up but
does not implement the mandatory 100 ms delay after link training
completes for speeds greater than 5.0 GT/s (PCIe r6.0 sec 6.6.1).

The driver already maintains a 'link_gen' field that holds the negotiated
link speed. Use it together with pci_host_common_link_train_delay() to
insert the required delay immediately after confirming that the link
is up.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/pci-aardvark.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index e34bea1ff0ac..fd9c7d53e8a7 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -26,6 +26,7 @@
 #include <linux/of_address.h>
 #include <linux/of_pci.h>
 
+#include "pci-host-common.h"
 #include "../pci.h"
 #include "../pci-bridge-emul.h"
 
@@ -350,8 +351,10 @@ static int advk_pcie_wait_for_link(struct advk_pcie *pcie)
 
 	/* check if the link is up or not */
 	for (retries = 0; retries < LINK_WAIT_MAX_RETRIES; retries++) {
-		if (advk_pcie_link_up(pcie))
+		if (advk_pcie_link_up(pcie)) {
+			pci_host_common_link_train_delay(pcie->link_gen);
 			return 0;
+		}
 
 		usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
 	}
-- 
2.34.1

[PATCH v3 6/7] PCI: mediatek-gen3: Add 100 ms delay after link up

[Hans Zhang]

The MediaTek Gen3 PCIe host driver lacks the required 100 ms delay after
link training completes for speeds > 5.0 GT/s, as specified in PCIe r6.0
sec 6.6.1.

The driver already stores max_link_speed (from the device tree). After
mtk_pcie_startup_port() successfully brings up the link, call
pci_host_common_link_train_delay() to comply with the specification.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/pcie-mediatek-gen3.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
index b0accd828589..5abddec4e9be 100644
--- a/drivers/pci/controller/pcie-mediatek-gen3.c
+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
@@ -30,6 +30,7 @@
 #include <linux/regmap.h>
 #include <linux/reset.h>
 
+#include "pci-host-common.h"
 #include "../pci.h"
 
 #define PCIE_BASE_CFG_REG		0x14
@@ -570,6 +571,8 @@ static int mtk_pcie_startup_port(struct mtk_gen3_pcie *pcie)
 		goto err_power_down_device;
 	}
 
+	pci_host_common_link_train_delay(pcie->max_link_speed);
+
 	return 0;
 
 err_power_down_device:
-- 
2.34.1

[PATCH v3 7/7] PCI: rzg3s-host: Use common pci_host_common_link_train_delay() helper

[Hans Zhang]

Replace the unconditional msleep(100) with the common helper
pci_host_common_link_train_delay(). The helper only waits when
max_link_speed > 2, as required by PCIe r6.0 sec 6.6.1.

This avoids unnecessary delay for Gen1/Gen2 links while retaining
the mandatory 100 ms for higher speeds.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/pcie-rzg3s-host.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
index d86e7516dcc2..66f687304c1c 100644
--- a/drivers/pci/controller/pcie-rzg3s-host.c
+++ b/drivers/pci/controller/pcie-rzg3s-host.c
@@ -35,6 +35,7 @@
 #include <linux/slab.h>
 #include <linux/units.h>
 
+#include "pci-host-common.h"
 #include "../pci.h"
 
 /* AXI registers */
@@ -1663,7 +1664,7 @@ rzg3s_pcie_host_setup(struct rzg3s_pcie_host *host,
 	if (ret)
 		dev_info(dev, "Failed to set max link speed\n");
 
-	msleep(PCIE_RESET_CONFIG_WAIT_MS);
+	pci_host_common_link_train_delay(host->max_link_speed);
 
 	return 0;
 
-- 
2.34.1

Re: [PATCH v3 4/7] PCI: dwc: Use common pci_host_common_link_train_delay() helper

[Krzysztof Wilczyński]

Hello,

> -	/*
> -	 * 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);
> +	pci_host_common_link_train_delay(pci->max_link_speed);

This comment could move to the helper you added.

Thank you!

	Krzysztof

Re: [PATCH v3 1/7] PCI: Add pci_host_common_link_train_delay() helper

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Negative error codes passed as `max_link_speed` silently bypass the mandatory PCIe delay.
- [Medium] Architectural modularity violation by placing a generic PCIe helper in an ECAM framework header.
- [Low] The helper API relies on raw integer values for link speed instead of the typed `enum pci_bus_speed`, creating a brittle interface.
--

commit 47720e87ef05a9f0115d64b2e2d6eaa190242b88
Author: Hans Zhang <18255117159@163.com>

PCI: Add pci_host_common_link_train_delay() helper

This commit introduces a static inline helper to wait 100 ms after link
training completes if the maximum link speed is greater than 5.0 GT/s,
satisfying the PCIe r6.0 specification.

> diff --git a/drivers/pci/controller/pci-host-common.h b/drivers/pci/controller/pci-host-common.h
> index b5075d4bd7eb3..d709f7e3e11a3 100644
> --- a/drivers/pci/controller/pci-host-common.h
> +++ b/drivers/pci/controller/pci-host-common.h

This isn't a bug, but this places a generic PCIe helper inside an ECAM
framework header. Unrelated host controller drivers will have to include
an ECAM-specific header just to use this delay function.

Should this generic helper be placed in a more widely accessible internal
header like drivers/pci/pci.h?

[ ... ]

> @@ -20,4 +23,18 @@ void pci_host_common_remove(struct platform_device *pdev);
>  
>  struct pci_config_window *pci_host_common_ecam_create(struct device *dev,
>  	struct pci_host_bridge *bridge, const struct pci_ecam_ops *ops);
> +
> +/**
> + * pci_host_common_link_train_delay - Wait 100 ms if link speed > 5 GT/s
> + * @max_link_speed: the maximum link speed (2 = 5.0 GT/s, 3 = 8.0 GT/s, ...)
> + *
> + * Must be called after Link training completes and before the first
> + * Configuration Request is sent.
> + */
> +static inline void pci_host_common_link_train_delay(int max_link_speed)

This isn't a bug, but relying on raw integer values for link speed instead
of enum pci_bus_speed creates a brittle interface.

The PCI subsystem uses enum pci_bus_speed for link speeds, where
PCIE_SPEED_2_5GT is 20 and PCIE_SPEED_5_0GT is 21. If a driver mistakenly
passes an enum pci_bus_speed, the condition greater than 2 will evaluate
to true for all speeds, applying the delay unconditionally.

> +{
> +	if (max_link_speed > 2)

Will negative error codes passed as max_link_speed silently bypass the
mandatory PCIe delay?

Drivers retrieve the maximum link speed using of_pci_get_max_link_speed(),
which returns -EINVAL if the max-link-speed property is missing from the
device tree.

If -EINVAL is passed directly to this helper, the condition evaluates to
false and the 100 ms delay is skipped. If the hardware supports greater
than 5.0 GT/s, skipping this delay violates the PCIe r6.0 specification.

> +		msleep(PCIE_RESET_CONFIG_WAIT_MS);
> +}
> +
>  #endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511055923.37117-1-18255117159@163.com?part=1

Re: [PATCH v3 2/7] PCI: cadence: Add post-link delay for LGA and j721e glue driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Compilation error in `j721e_pcie_set_link_speed` due to incorrect structure member access on a pointer.
--

commit b3897a7e50194e1f9cc94b232c615c7f8f651006
Author: Hans Zhang <18255117159@163.com>

PCI: cadence: Add post-link delay for LGA and j721e glue driver

The Cadence LGA PCIe host controller lacks the mandatory 100 ms delay after
link training completes for speeds > 5.0 GT/s. This commit adds a
max_link_speed field to struct cdns_pcie and inserts the required delay
after the link has been successfully established.

> diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
> index bfdfe98d5aba7..ee85b8e04f5b8 100644
> --- a/drivers/pci/controller/cadence/pci-j721e.c
> +++ b/drivers/pci/controller/cadence/pci-j721e.c
> @@ -206,6 +206,7 @@ static int j721e_pcie_set_link_speed(struct j721e_pcie *pcie,
>  	    (pcie_get_link_speed(link_speed) == PCI_SPEED_UNKNOWN))
>  		link_speed = 2;
>  
> +	pcie->cdns_pcie.max_link_speed = link_speed;

Will this cause a compilation error?

Looking at struct j721e_pcie, cdns_pcie is defined as a pointer:

struct j721e_pcie {
	struct cdns_pcie	*cdns_pcie;
	struct clk		*refclk;
	...
};

Does this need to use the arrow operator instead of a dot
(pcie->cdns_pcie->max_link_speed) so it correctly accesses the
max_link_speed member?

>  	val = link_speed - 1;
>  	ret = regmap_update_bits(syscon, offset, GENERATION_SEL_MASK, val);
>  	if (ret)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511055923.37117-1-18255117159@163.com?part=2

Re: [PATCH v3 4/7] PCI: dwc: Use common pci_host_common_link_train_delay() helper

[Hans Zhang]

On 5/11/26 15:02, Krzysztof Wilczyński wrote:
> Hello,
> 
>> -	/*
>> -	 * 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);
>> +	pci_host_common_link_train_delay(pci->max_link_speed);
> 
> This comment could move to the helper you added.

Hi Krzysztof,

Will add.

Best regards,
Hans


> 
> Thank you!
> 
> 	Krzysztof

Re: [PATCH v3 3/7] PCI: cadence: HPA: Add post-link delay

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing device tree property silently disables the mandatory 100 ms PCIe link training delay.
- [High] The function `cdns_pcie_hpa_host_link_setup()` aborts the entire PCIe root complex probe if no downstream endpoint is connected.
--

commit 53fefd8ead01e523abe2d99bb9e1db2677c7f7ec
Author: Hans Zhang <18255117159@163.com>

PCI: cadence: HPA: Add post-link delay

This commit updates the Cadence HPA link setup function to include a 
100 ms delay after link training completes for speeds greater than 
5.0 GT/s. It also reads the max-link-speed device tree property to 
initialize the max link speed if it hasn't been set by a glue driver.

> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c b/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
> index 0f540bed58e88..8ef58ed01daaa 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
[ ... ]
> @@ -304,6 +306,8 @@ int cdns_pcie_hpa_host_link_setup(struct cdns_pcie_rc *rc)
>  	ret = cdns_pcie_host_wait_for_link(pcie, cdns_pcie_hpa_link_up);
>  	if (ret)
>  		dev_dbg(dev, "PCIe link never came up\n");
> +	else
> +		pci_host_common_link_train_delay(pcie->max_link_speed);
>  
>  	return ret;

This isn't a regression introduced by this patch, but since we are modifying 
this function, does returning ret here abort the entire PCIe root complex 
probe if no downstream endpoint is connected?

When no endpoint is connected, cdns_pcie_host_wait_for_link() returns 
-ETIMEDOUT. Returning that error back to the caller causes 
cdns_pcie_hpa_host_setup() to fail initialization. 

Could this break hotplug functionality or standard empty-slot handling 
when the slot is unoccupied at boot?

>  }
> @@ -313,6 +317,7 @@ int cdns_pcie_hpa_host_setup(struct cdns_pcie_rc *rc)
[ ... ]
> @@ -343,6 +348,9 @@ int cdns_pcie_hpa_host_setup(struct cdns_pcie_rc *rc)
>  		rc->cfg_res = res;
>  	}
>  
> +	if (pcie->max_link_speed < 1)
> +		pcie->max_link_speed = of_pci_get_max_link_speed(np);

If the max-link-speed property is missing from the device tree, 
of_pci_get_max_link_speed() will return -EINVAL.

Will assigning this negative error code directly to pcie->max_link_speed 
cause the post-link delay check to fail?

When pci_host_common_link_train_delay() is called:

static inline void pci_host_common_link_train_delay(int max_link_speed)
{
        if (max_link_speed > 2)
                msleep(PCIE_RESET_CONFIG_WAIT_MS);
}

Since -EINVAL is a negative value, the condition max_link_speed > 2 
evaluates to false. This appears to silently bypass the required 100 ms 
delay for Gen3+ speeds entirely.

Should the return value of of_pci_get_max_link_speed() be validated 
before assignment?

> +
>  	/* Put EROM Bar aperture to 0 */
>  	cdns_pcie_hpa_writel(pcie, REG_BANK_IP_CFG_CTRL_REG, CDNS_PCIE_EROM, 0x0);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511055923.37117-1-18255117159@163.com?part=3

Re: [PATCH v3 2/7] PCI: cadence: Add post-link delay for LGA and j721e glue driver

[Hans Zhang]

On 5/12/26 08:24, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Compilation error in `j721e_pcie_set_link_speed` due to incorrect structure member access on a pointer.
> --
> 
> commit b3897a7e50194e1f9cc94b232c615c7f8f651006
> Author: Hans Zhang <18255117159@163.com>
> 
> PCI: cadence: Add post-link delay for LGA and j721e glue driver
> 
> The Cadence LGA PCIe host controller lacks the mandatory 100 ms delay after
> link training completes for speeds > 5.0 GT/s. This commit adds a
> max_link_speed field to struct cdns_pcie and inserts the required delay
> after the link has been successfully established.
> 
>> diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c
>> index bfdfe98d5aba7..ee85b8e04f5b8 100644
>> --- a/drivers/pci/controller/cadence/pci-j721e.c
>> +++ b/drivers/pci/controller/cadence/pci-j721e.c
>> @@ -206,6 +206,7 @@ static int j721e_pcie_set_link_speed(struct j721e_pcie *pcie,
>>   	    (pcie_get_link_speed(link_speed) == PCI_SPEED_UNKNOWN))
>>   		link_speed = 2;
>>   
>> +	pcie->cdns_pcie.max_link_speed = link_speed;
> 
> Will this cause a compilation error?

Sorry. Will change.

Best regards,
Hans

> 
> Looking at struct j721e_pcie, cdns_pcie is defined as a pointer:
> 
> struct j721e_pcie {
> 	struct cdns_pcie	*cdns_pcie;
> 	struct clk		*refclk;
> 	...
> };
> 
> Does this need to use the arrow operator instead of a dot
> (pcie->cdns_pcie->max_link_speed) so it correctly accesses the
> max_link_speed member?
> 
>>   	val = link_speed - 1;
>>   	ret = regmap_update_bits(syscon, offset, GENERATION_SEL_MASK, val);
>>   	if (ret)
> 

Re: [PATCH v3 1/7] PCI: Add pci_host_common_link_train_delay() helper

[Hans Zhang]

On 5/12/26 08:07, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [High] Negative error codes passed as `max_link_speed` silently bypass the mandatory PCIe delay.
> - [Medium] Architectural modularity violation by placing a generic PCIe helper in an ECAM framework header.
> - [Low] The helper API relies on raw integer values for link speed instead of the typed `enum pci_bus_speed`, creating a brittle interface.
> --
> 
> commit 47720e87ef05a9f0115d64b2e2d6eaa190242b88
> Author: Hans Zhang <18255117159@163.com>
> 
> PCI: Add pci_host_common_link_train_delay() helper
> 
> This commit introduces a static inline helper to wait 100 ms after link
> training completes if the maximum link speed is greater than 5.0 GT/s,
> satisfying the PCIe r6.0 specification.
> 
>> diff --git a/drivers/pci/controller/pci-host-common.h b/drivers/pci/controller/pci-host-common.h
>> index b5075d4bd7eb3..d709f7e3e11a3 100644
>> --- a/drivers/pci/controller/pci-host-common.h
>> +++ b/drivers/pci/controller/pci-host-common.h
> 
> This isn't a bug, but this places a generic PCIe helper inside an ECAM
> framework header. Unrelated host controller drivers will have to include
> an ECAM-specific header just to use this delay function.
> 
> Should this generic helper be placed in a more widely accessible internal
> header like drivers/pci/pci.h?

No, Mani suggested placing it in the pci-host-common.h file.

> 
> [ ... ]
> 
>> @@ -20,4 +23,18 @@ void pci_host_common_remove(struct platform_device *pdev);
>>   
>>   struct pci_config_window *pci_host_common_ecam_create(struct device *dev,
>>   	struct pci_host_bridge *bridge, const struct pci_ecam_ops *ops);
>> +
>> +/**
>> + * pci_host_common_link_train_delay - Wait 100 ms if link speed > 5 GT/s
>> + * @max_link_speed: the maximum link speed (2 = 5.0 GT/s, 3 = 8.0 GT/s, ...)
>> + *
>> + * Must be called after Link training completes and before the first
>> + * Configuration Request is sent.
>> + */
>> +static inline void pci_host_common_link_train_delay(int max_link_speed)
> 
> This isn't a bug, but relying on raw integer values for link speed instead
> of enum pci_bus_speed creates a brittle interface.
> 
> The PCI subsystem uses enum pci_bus_speed for link speeds, where
> PCIE_SPEED_2_5GT is 20 and PCIE_SPEED_5_0GT is 21. If a driver mistakenly
> passes an enum pci_bus_speed, the condition greater than 2 will evaluate
> to true for all speeds, applying the delay unconditionally.
> 
>> +{
>> +	if (max_link_speed > 2)
> 
> Will negative error codes passed as max_link_speed silently bypass the
> mandatory PCIe delay?
> 
> Drivers retrieve the maximum link speed using of_pci_get_max_link_speed(),
> which returns -EINVAL if the max-link-speed property is missing from the
> device tree.
> 
> If -EINVAL is passed directly to this helper, the condition evaluates to
> false and the 100 ms delay is skipped. If the hardware supports greater
> than 5.0 GT/s, skipping this delay violates the PCIe r6.0 specification.
> 
>> +		msleep(PCIE_RESET_CONFIG_WAIT_MS);
>> +}
>> +
>>   #endif
> 

Re: [PATCH v3 4/7] PCI: dwc: Use common pci_host_common_link_train_delay() helper

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Improper cross-controller coupling by including `../pci-host-common.h` in the DesignWare core.
--

commit f5801e5d9e35ace16e87610771ef7a5939957245
Author: Hans Zhang <18255117159@163.com>

PCI: dwc: Use common pci_host_common_link_train_delay() helper

This commit replaces the open-coded 100 ms PCIe spec delay in the
DesignWare driver with the pci_host_common_link_train_delay() helper
to reduce code duplication.

> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index c11cf61b8319e..7021d21bb6011 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -22,6 +22,7 @@
>  #include <linux/sizes.h>
>  #include <linux/types.h>
>  
> +#include "../pci-host-common.h"

Does including this header create an architectural entanglement between the
DesignWare library and the generic PCI ECAM host controller library?

Since the delay implements a generic PCIe specification requirement (PCIe r6.0
sec 6.6.1) applicable to all Downstream Ports > 5.0 GT/s, should this helper
be placed in a shared subsystem-wide header (like drivers/pci/pci.h or
include/linux/pci.h) with a generic name, rather than relying on another
controller's header?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511055923.37117-1-18255117159@163.com?part=4

Re: [PATCH v3 5/7] PCI: aardvark: Add 100 ms delay after link training

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Potential Use-After-Free (UAF) in the IRQ handler during advk_pcie_probe() failure due to unbalanced teardown symmetry.
- [High] Memory and hardware resources are leaked in the error paths of advk_pcie_probe().
- [Medium] The commit message incorrectly claims that `link_gen` holds the negotiated link speed.
--

commit 1f92b585affbc960f9cf75de361f24796312c8bb
Author: Hans Zhang <18255117159@163.com>

PCI: aardvark: Add 100 ms delay after link training

This commit introduces a 100 ms delay after PCIe link training completes for
speeds greater than 5.0 GT/s. It utilizes the existing pcie->link_gen field
and pci_host_common_link_train_delay() to implement this requirement.

> The driver already maintains a 'link_gen' field that holds the negotiated
> link speed. Use it together with pci_host_common_link_train_delay() to
> insert the required delay immediately after confirming that the link
> is up.

This isn't a bug, but is this description accurate? The pcie->link_gen field
is populated during driver probe via of_pci_get_max_link_speed() and does
not appear to be dynamically updated with the actual negotiated link speed
from the hardware.

[ ... ]

Looking at the error paths in advk_pcie_probe(), if steps like
advk_pcie_init_irq_domain() or pci_host_probe() fail, the driver returns
without calling pci_bridge_emul_cleanup() to free the emulated bridge memory.
It also appears to skip calling advk_pcie_disable_phy() to power off the PHY.

Does this code leak the PHY and emulated bridge memory resources in these
error paths?

Also in advk_pcie_probe(), the driver registers advk_pcie_irq_handler using
devm_request_irq() and enables hardware interrupts via advk_pcie_setup_hw().

If a later step like pci_host_probe() fails, the error path calls
advk_pcie_remove_irq_domain(), which frees the irq domain.

Since the hardware interrupts are not masked and the IRQ handler is not
deregistered in this error path, devm_request_irq() defers unregistration
until after the probe function returns.

Could a hardware interrupt fire in this window, causing
advk_pcie_irq_handler() to call generic_handle_domain_irq() and dereference
the freed pcie->irq_domain pointer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511055923.37117-1-18255117159@163.com?part=5

Re: [PATCH v3 6/7] PCI: mediatek-gen3: Add 100 ms delay after link up

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The patch completely bypasses the mandatory 100 ms link training delay when the 'max-link-speed' property is omitted from the Device Tree.
--

commit a93442038658e8edd35c1ce148da1ebe3644bfd1
Author: Hans Zhang <18255117159@163.com>

PCI: mediatek-gen3: Add 100 ms delay after link up

The MediaTek Gen3 PCIe host driver lacks the required 100 ms delay after
link training completes for speeds > 5.0 GT/s, as specified in PCIe r6.0
sec 6.6.1. This commit adds the pci_host_common_link_train_delay() call
after the link is successfully brought up.

> diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
> index b0accd8285892..5abddec4e9bec 100644
> --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
[ ... ]
> @@ -570,6 +571,8 @@ static int mtk_pcie_startup_port(struct mtk_gen3_pcie *pcie)
>  		goto err_power_down_device;
>  	}
>  
> +	pci_host_common_link_train_delay(pcie->max_link_speed);

Does this code bypass the delay when the max-link-speed property is omitted
from the device tree?

If pcie->max_link_speed is 0 (unspecified in the device tree), the controller
defaults to its native hardware capability (Gen3). However, passing 0 to
pci_host_common_link_train_delay() will cause its internal check to evaluate
to false and skip the delay entirely:

static inline void pci_host_common_link_train_delay(int max_link_speed)
{
	if (max_link_speed > 2)
		msleep(PCIE_RESET_CONFIG_WAIT_MS);
}

Should the driver evaluate the actual controller capability or pass the known
maximum speed when pcie->max_link_speed is 0?

> +
>  	return 0;
>  
>  err_power_down_device:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511055923.37117-1-18255117159@163.com?part=6

Re: [PATCH v3 7/7] PCI: rzg3s-host: Use common pci_host_common_link_train_delay() helper

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The mandatory link training delay is bypassed when 'max-link-speed' is missing from the device tree, violating PCIe r6.0 requirements for Gen3+ devices.
--

commit afb29b744bdf8f1be85fd9c879cf2af33e736d5a
Author: Hans Zhang <18255117159@163.com>

PCI: rzg3s-host: Use common pci_host_common_link_train_delay() helper

Replace the unconditional msleep(100) with the common helper
pci_host_common_link_train_delay(). The helper only waits when
max_link_speed > 2, as required by PCIe r6.0 sec 6.6.1.

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index d86e7516dcc29..66f687304c1cb 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c

[ ... ]

> @@ -1663,7 +1664,7 @@ rzg3s_pcie_host_setup(struct rzg3s_pcie_host *host,
>  	if (ret)
>  		dev_info(dev, "Failed to set max link speed\n");
>  
> -	msleep(PCIE_RESET_CONFIG_WAIT_MS);
> +	pci_host_common_link_train_delay(host->max_link_speed);

If the max-link-speed property is missing from the device tree,
of_pci_get_max_link_speed() returns -EINVAL.

While rzg3s_pcie_set_max_link_speed() correctly handles this negative
value by falling back to hardware capabilities, host->max_link_speed
remains -EINVAL.

When this negative value is passed to pci_host_common_link_train_delay(),
the internal check (max_link_speed > 2) evaluates to false.

Could this cause a regression by inadvertently bypassing the mandatory link
training delay for Gen3 or higher devices that lack the max-link-speed
device tree property?

>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511055923.37117-1-18255117159@163.com?part=7

Re: [PATCH v3 4/7] PCI: dwc: Use common pci_host_common_link_train_delay() helper

[Manivannan Sadhasivam]

On Mon, May 11, 2026 at 04:02:42PM +0900, Krzysztof Wilczyński wrote:
> Hello,
> 
> > -	/*
> > -	 * 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);
> > +	pci_host_common_link_train_delay(pci->max_link_speed);
> 
> This comment could move to the helper you added.
> 

Not required, as the macro definition in drivers/pci/pci.h already has an
elaborative comment with spec citation.

- Mani

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

Re: [PATCH v3 1/7] PCI: Add pci_host_common_link_train_delay() helper

[Claudiu Beznea]

Hi, Hans,

On 5/11/26 08:59, Hans Zhang wrote:
> PCIe r6.0, sec 6.6.1 (Conventional Reset) requires that for a Downstream
> Port supporting Link speeds greater than 5.0 GT/s, software must wait a
> minimum of 100 ms after Link training completes before sending any
> Configuration Request.
> 
> Introduce a static inline helper pci_host_common_link_train_delay() that
> checks the given max_link_speed (2 = 5.0 GT/s, 3 = 8.0 GT/s, etc.) and
> calls msleep(100) only when the speed is greater than 5.0 GT/s.
> 
> This allows multiple host controller drivers to share the same mandatory
> delay without duplicating the logic.
> 
> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---
>   drivers/pci/controller/pci-host-common.h | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/pci/controller/pci-host-common.h b/drivers/pci/controller/pci-host-common.h
> index b5075d4bd7eb..d709f7e3e11a 100644
> --- a/drivers/pci/controller/pci-host-common.h
> +++ b/drivers/pci/controller/pci-host-common.h
> @@ -10,6 +10,9 @@
>   #ifndef _PCI_HOST_COMMON_H
>   #define _PCI_HOST_COMMON_H
>   
> +#include <linux/delay.h>
> +#include "../pci.h"
> +
>   struct pci_ecam_ops;
>   
>   int pci_host_common_probe(struct platform_device *pdev);
> @@ -20,4 +23,18 @@ void pci_host_common_remove(struct platform_device *pdev);
>   
>   struct pci_config_window *pci_host_common_ecam_create(struct device *dev,
>   	struct pci_host_bridge *bridge, const struct pci_ecam_ops *ops);
> +
> +/**
> + * pci_host_common_link_train_delay - Wait 100 ms if link speed > 5 GT/s
> + * @max_link_speed: the maximum link speed (2 = 5.0 GT/s, 3 = 8.0 GT/s, ...)
> + *
> + * Must be called after Link training completes and before the first
> + * Configuration Request is sent.
> + */
> +static inline void pci_host_common_link_train_delay(int max_link_speed)
> +{
> +	if (max_link_speed > 2)
> +		msleep(PCIE_RESET_CONFIG_WAIT_MS);

In case of RZ/G3S driver the max_link_speed is populated based on 
"max-link-speed" DT property (by calling of_pci_get_max_link_speed()). My 
understanding from [1] (and the review of the initial RZ/G3S driver support) is 
that this is not a mandatory property (note also the "Host drivers *could* add 
this" from [1]). At least for the RZ/G3S driver, in case the "max-link-speed" DT 
property is not present in DT but the controller supports more than 5GT/s (that 
is possible as the driver supports more controller variants), the max_link_speed 
argument will be negative. In that case the msleep() will not be called. This 
looks like an opposite of what the patch set is trying to achieve.

Also, if I'm not wrong, there is also the possibility of having the 
max-link-speed > 2 but the downstream port to not support more than 5GT/s. In 
that case the mspeep() would also be executed (but I think that wouldn't be 
really an issue).

Thank you,
Claudiu

[1] 
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/pci/pci-bus-common.yaml#L117

Thank you,
Claudiu

Re: [PATCH v3 4/7] PCI: dwc: Use common pci_host_common_link_train_delay() helper

[Krzysztof Wilczyński]

Hello,

> > > -	/*
> > > -	 * 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);
> > > +	pci_host_common_link_train_delay(pci->max_link_speed);
> > 
> > This comment could move to the helper you added.
> 
> Hi Krzysztof,
> 
> Will add.

No need.  Per Mani's feedback about macro being well documented.

Thank you nonetheless!

	Krzysztof

Re: [PATCH v3 4/7] PCI: dwc: Use common pci_host_common_link_train_delay() helper

[Krzysztof Wilczyński]

Hello,

> > > -	/*
> > > -	 * 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);
> > > +	pci_host_common_link_train_delay(pci->max_link_speed);
> > 
> > This comment could move to the helper you added.
> > 
> 
> Not required, as the macro definition in drivers/pci/pci.h already has an
> elaborative comment with spec citation.

So it does, indeed.  Thank you for checking!

	Krzysztof

Re: [PATCH v3 1/7] PCI: Add pci_host_common_link_train_delay() helper

[Hans Zhang]

On 5/12/26 15:05, Claudiu Beznea wrote:
> Hi, Hans,
> 
> On 5/11/26 08:59, Hans Zhang wrote:
>> PCIe r6.0, sec 6.6.1 (Conventional Reset) requires that for a Downstream
>> Port supporting Link speeds greater than 5.0 GT/s, software must wait a
>> minimum of 100 ms after Link training completes before sending any
>> Configuration Request.
>>
>> Introduce a static inline helper pci_host_common_link_train_delay() that
>> checks the given max_link_speed (2 = 5.0 GT/s, 3 = 8.0 GT/s, etc.) and
>> calls msleep(100) only when the speed is greater than 5.0 GT/s.
>>
>> This allows multiple host controller drivers to share the same mandatory
>> delay without duplicating the logic.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>>   drivers/pci/controller/pci-host-common.h | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/pci/controller/pci-host-common.h b/drivers/pci/ 
>> controller/pci-host-common.h
>> index b5075d4bd7eb..d709f7e3e11a 100644
>> --- a/drivers/pci/controller/pci-host-common.h
>> +++ b/drivers/pci/controller/pci-host-common.h
>> @@ -10,6 +10,9 @@
>>   #ifndef _PCI_HOST_COMMON_H
>>   #define _PCI_HOST_COMMON_H
>> +#include <linux/delay.h>
>> +#include "../pci.h"
>> +
>>   struct pci_ecam_ops;
>>   int pci_host_common_probe(struct platform_device *pdev);
>> @@ -20,4 +23,18 @@ void pci_host_common_remove(struct platform_device 
>> *pdev);
>>   struct pci_config_window *pci_host_common_ecam_create(struct device 
>> *dev,
>>       struct pci_host_bridge *bridge, const struct pci_ecam_ops *ops);
>> +
>> +/**
>> + * pci_host_common_link_train_delay - Wait 100 ms if link speed > 5 GT/s
>> + * @max_link_speed: the maximum link speed (2 = 5.0 GT/s, 3 = 8.0 GT/ 
>> s, ...)
>> + *
>> + * Must be called after Link training completes and before the first
>> + * Configuration Request is sent.
>> + */
>> +static inline void pci_host_common_link_train_delay(int max_link_speed)
>> +{
>> +    if (max_link_speed > 2)
>> +        msleep(PCIE_RESET_CONFIG_WAIT_MS);
> 
> In case of RZ/G3S driver the max_link_speed is populated based on "max- 
> link-speed" DT property (by calling of_pci_get_max_link_speed()). My 
> understanding from [1] (and the review of the initial RZ/G3S driver 
> support) is that this is not a mandatory property (note also the "Host 
> drivers *could* add this" from [1]). At least for the RZ/G3S driver, in 
> case the "max-link-speed" DT property is not present in DT but the 
> controller supports more than 5GT/s (that is possible as the driver 
> supports more controller variants), the max_link_speed argument will be 
> negative. In that case the msleep() will not be called. This looks like 
> an opposite of what the patch set is trying to achieve.

Hi Claudiu,

The situation you mentioned also exists in the dwc common driver. My 
understanding is that we are writing this driver at the normal rate 
which is greater than GEN2. For some exceptions, or when the support is 
greater than GEN2 but the actual operation is less than or equal to 
GEN2, this situation might be unavoidable. Furthermore, for RZ/G3S, the 
"max-link-speed" attribute can be added to the DT.


> 
> Also, if I'm not wrong, there is also the possibility of having the max- 
> link-speed > 2 but the downstream port to not support more than 5GT/s. 
> In that case the mspeep() would also be executed (but I think that 
> wouldn't be really an issue).


Before this patch, the RZ/G3S driver would always perform a msleep(100) 
regardless of whether it was greater than GEN2 or less than or equal to 
GEN2.

Best regards,
Hans

> 
> Thank you,
> Claudiu
> 
> [1] https://github.com/devicetree-org/dt-schema/blob/main/dtschema/ 
> schemas/pci/pci-bus-common.yaml#L117
> 
> Thank you,
> Claudiu

Re: [PATCH v3 4/7] PCI: dwc: Use common pci_host_common_link_train_delay() helper

[Hans Zhang]

On 5/12/26 15:14, Krzysztof Wilczyński wrote:
> Hello,
> 
>>>> -	/*
>>>> -	 * 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);
>>>> +	pci_host_common_link_train_delay(pci->max_link_speed);
>>>
>>> This comment could move to the helper you added.
>>
>> Hi Krzysztof,
>>
>> Will add.
> 
> No need.  Per Mani's feedback about macro being well documented.
> 

Hi Krzysztof,

Okay.

Best regards,
Hans

> Thank you nonetheless!
> 
> 	Krzysztof