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

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

[Hans Zhang]

PCIe r6.0, sec 6.6.1 (Conventional Reset) states:

- For 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 to the device immediately below
  that Port.

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.

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

  1. Adds a static inline helper `pcie_wait_after_link_train()` in
     drivers/pci/pci.h. The helper checks the given max_link_speed
     (or negotiated speed) and calls msleep(100) if the speed is > 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 - Legacy Architecture IP - and HPA - High Performance
     Architecture IP) after introducing a `max_link_speed` field in
     struct cdns_pcie.

  4. Adds the delay to the Aardvark, MediaTek Gen3, and Renesas RZ/G3S
     host drivers, reusing their existing link speed fields.

All changes are placed exactly where the driver has just finished
waiting for the link to come up, i.e., 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 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.

v1:
https://patchwork.kernel.org/project/linux-pci/patch/20260501153553.66382-1-18255117159@163.com/
---
Hans Zhang (8):
  PCI: Add pcie_wait_after_link_train() helper
  PCI: cadence: LGA: Add max_link_speed field and 100 ms delay after
    link training
  PCI: cadence: HPA: Add 100 ms delay after link training
  PCI: j721e: Set max_link_speed to enable 100 ms delay after link up
  PCI: dwc: Use common pcie_wait_after_link_train() helper
  PCI: aardvark: Add 100 ms delay after link training
  PCI: mediatek-gen3: Add 100 ms delay after link training
  PCI: rzg3s-host: Add 100 ms delay after link training

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


base-commit: a293ec25d59dd96309058c70df5a4dd0f889a1e4
-- 
2.34.1

[PATCH v2 1/8] PCI: Add pcie_wait_after_link_train() 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 pcie_wait_after_link_train() 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. The helper uses
the existing PCIE_RESET_CONFIG_WAIT_MS macro defined in pci.h.

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/pci.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4a14f88e543a..a8705a2a2d85 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -60,6 +60,19 @@ struct pcie_tlp_log;
  */
 #define PCIE_RESET_CONFIG_WAIT_MS	100
 
+/**
+ * pcie_wait_after_link_train - 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 pcie_wait_after_link_train(int max_link_speed)
+{
+	if (max_link_speed > 2)
+		msleep(PCIE_RESET_CONFIG_WAIT_MS);
+}
+
 /* Parameters for the waiting for link up routine */
 #define PCIE_LINK_WAIT_MAX_RETRIES	10
 #define PCIE_LINK_WAIT_SLEEP_MS		90
-- 
2.34.1

[PATCH v2 2/8] PCI: cadence: LGA: Add max_link_speed field and 100 ms delay after link training

[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 to record the maximum
supported link speed (or the currently configured speed). In the common
host layer function cdns_pcie_host_start_link(), after the link has been
successfully established, call pcie_wait_after_link_train() to insert the
required delay if max_link_speed > 2.

Glue drivers must set max_link_speed appropriately (e.g., from the device
tree property "max-link-speed") to enable the delay.

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

diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-common.c b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
index 2b0211870f02..51376f69d007 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.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)
+		pcie_wait_after_link_train(pcie->max_link_speed);
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(cdns_pcie_host_start_link);
diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
index 574e9cf4d003..e222b095d2b6 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 v2 3/8] PCI: cadence: HPA: Add 100 ms delay after link training

[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 pcie_wait_after_link_train() immediately after the link is
confirmed to be up, using the max_link_speed previously stored in struct
cdns_pcie. This ensures compliance with the specification regardless of
whether the HPA or LGA path is used.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/cadence/pcie-cadence-host-hpa.c | 3 +++
 1 file changed, 3 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..62e939906785 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host-hpa.c
@@ -15,6 +15,7 @@
 
 #include "pcie-cadence.h"
 #include "pcie-cadence-host-common.h"
+#include "../../pci.h"
 
 static u8 bar_aperture_mask[] = {
 	[RP_BAR0] = 0x3F,
@@ -304,6 +305,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
+		pcie_wait_after_link_train(pcie->max_link_speed);
 
 	return ret;
 }
-- 
2.34.1

[PATCH v2 4/8] PCI: j721e: Set max_link_speed to enable 100 ms delay after link up

[Hans Zhang]

Set cdns_pcie.max_link_speed to the maximum supported link speed
(obtained from the device tree property "max-link-speed") in
j721e_pcie_set_link_speed(). This activates the post-link delay logic
added in cdns_pcie_host_start_link() when the controller supports
speeds greater than 5 GT/s.

As required by PCIe r6.0 sec 6.6.1, and following the same approach as
commit 80dc18a0cba8d ("PCI: dwc: Ensure that dw_pcie_wait_for_link()
waits 100 ms after link up"), this ensures a 100 ms delay after link
training completes before any Configuration Request is sent.

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

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

[PATCH v2 5/8] PCI: dwc: Use common pcie_wait_after_link_train() 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
pcie_wait_after_link_train() 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 | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index c11cf61b8319..e5808d4b3867 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -799,13 +799,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);
+	pcie_wait_after_link_train(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 v2 6/8] 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 pcie_wait_after_link_train() 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 | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
index e34bea1ff0ac..526351c21c49 100644
--- a/drivers/pci/controller/pci-aardvark.c
+++ b/drivers/pci/controller/pci-aardvark.c
@@ -350,8 +350,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)) {
+			pcie_wait_after_link_train(pcie->link_gen);
 			return 0;
+		}
 
 		usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
 	}
-- 
2.34.1

[PATCH v2 7/8] PCI: mediatek-gen3: Add 100 ms delay after link training

[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
pcie_wait_after_link_train() to comply with the specification.

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

diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c b/drivers/pci/controller/pcie-mediatek-gen3.c
index b0accd828589..7c5f2ba7157b 100644
--- a/drivers/pci/controller/pcie-mediatek-gen3.c
+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
@@ -570,6 +570,8 @@ static int mtk_pcie_startup_port(struct mtk_gen3_pcie *pcie)
 		goto err_power_down_device;
 	}
 
+	pcie_wait_after_link_train(pcie->max_link_speed);
+
 	return 0;
 
 err_power_down_device:
-- 
2.34.1

[PATCH v2 8/8] PCI: rzg3s-host: Add 100 ms delay after link training

[Hans Zhang]

The Renesas RZ/G3S PCIe host driver currently does not enforce the
mandatory 100 ms delay after link training completes for speeds > 5.0 GT/s,
required by PCIe r6.0 sec 6.6.1.

The driver already has a 'max_link_speed' field (derived from the device
tree). Add a call to pcie_wait_after_link_train() in
rzg3s_pcie_host_init() after reading the link status, ensuring that the
delay is applied before any Configuration Request is sent downstream.

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

diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
index d86e7516dcc2..6ab59c5464cf 100644
--- a/drivers/pci/controller/pcie-rzg3s-host.c
+++ b/drivers/pci/controller/pcie-rzg3s-host.c
@@ -1390,6 +1390,8 @@ static int rzg3s_pcie_host_init(struct rzg3s_pcie_host *host)
 	val = readl_relaxed(host->axi + RZG3S_PCI_PCSTAT2);
 	dev_info(host->dev, "PCIe link status [0x%x]\n", val);
 
+	pcie_wait_after_link_train(host->max_link_speed);
+
 	return 0;
 
 config_deinit_post:
-- 
2.34.1

RE: [PATCH v2 2/8] PCI: cadence: LGA: Add max_link_speed field and 100 ms delay after link training

[Biju Das]

> -----Original Message-----
> From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org> On Behalf Of Hans Zhang
> Sent: 06 May 2026 16:24
> Subject: [PATCH v2 2/8] PCI: cadence: LGA: Add max_link_speed field and 100 ms delay after link
> training
> 
> 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 to record the maximum supported link speed (or the
> currently configured speed). In the common host layer function cdns_pcie_host_start_link(), after the
> link has been successfully established, call pcie_wait_after_link_train() to insert the required delay
> if max_link_speed > 2.
> 
> Glue drivers must set max_link_speed appropriately (e.g., from the device tree property "max-link-
> speed") to enable the delay.
> 
> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---
>  drivers/pci/controller/cadence/pcie-cadence-host-common.c | 4 ++++
>  drivers/pci/controller/cadence/pcie-cadence.h             | 2 ++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-common.c
> b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
> index 2b0211870f02..51376f69d007 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.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)
> +		pcie_wait_after_link_train(pcie->max_link_speed);
> +
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(cdns_pcie_host_start_link);
> diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-
> cadence.h
> index 574e9cf4d003..e222b095d2b6 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

Maximum to make consistent with other comments?


>   */
>  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;

unsigned int as speed cannot be negative??

Cheers,
Biju

>  };
> 
>  /**
> --
> 2.34.1
> 

RE: [PATCH v2 1/8] PCI: Add pcie_wait_after_link_train() helper

[Biju Das]

> -----Original Message-----
> From: Hans Zhang <18255117159@163.com>
> Sent: 06 May 2026 16:24
> Subject: [PATCH v2 1/8] PCI: Add pcie_wait_after_link_train() helper
> 
> 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 pcie_wait_after_link_train() 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. The helper uses the existing
> PCIE_RESET_CONFIG_WAIT_MS macro defined in pci.h.
> 
> 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/pci.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 4a14f88e543a..a8705a2a2d85 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -60,6 +60,19 @@ struct pcie_tlp_log;
>   */
>  #define PCIE_RESET_CONFIG_WAIT_MS	100
> 
> +/**
> + * pcie_wait_after_link_train - 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 pcie_wait_after_link_train(int max_link_speed) {
> +	if (max_link_speed > 2)
> +		msleep(PCIE_RESET_CONFIG_WAIT_MS);
> +}

Maybe move this patch to the user??

Cheers,
Biju

> +
>  /* Parameters for the waiting for link up routine */
>  #define PCIE_LINK_WAIT_MAX_RETRIES	10
>  #define PCIE_LINK_WAIT_SLEEP_MS		90
> --
> 2.34.1
> 

Re: [PATCH v2 1/8] PCI: Add pcie_wait_after_link_train() helper

[Manivannan Sadhasivam]

On Wed, May 06, 2026 at 11:23:39PM +0800, 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 pcie_wait_after_link_train() 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. The helper uses
> the existing PCIE_RESET_CONFIG_WAIT_MS macro defined in pci.h.
> 
> 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/pci.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 4a14f88e543a..a8705a2a2d85 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h

Since this helper is for host controller drivers, this needs to be defined in
pci-host-common.h.

> @@ -60,6 +60,19 @@ struct pcie_tlp_log;
>   */
>  #define PCIE_RESET_CONFIG_WAIT_MS	100
>  
> +/**
> + * pcie_wait_after_link_train - 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 pcie_wait_after_link_train(int max_link_speed)

pci_host_common_link_train_delay()?

- Mani

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

Re: [PATCH v2 2/8] PCI: cadence: LGA: Add max_link_speed field and 100 ms delay after link training

[Manivannan Sadhasivam]

On Wed, May 06, 2026 at 11:23:40PM +0800, Hans Zhang wrote:
> 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 to record the maximum
> supported link speed (or the currently configured speed). In the common
> host layer function cdns_pcie_host_start_link(), after the link has been
> successfully established, call pcie_wait_after_link_train() to insert the
> required delay if max_link_speed > 2.
> 
> Glue drivers must set max_link_speed appropriately (e.g., from the device
> tree property "max-link-speed") to enable the delay.
> 

You need to club those glue driver patches into this one. Otherwise, you'll end
up breaking bisectability.

- Mani

> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---
>  drivers/pci/controller/cadence/pcie-cadence-host-common.c | 4 ++++
>  drivers/pci/controller/cadence/pcie-cadence.h             | 2 ++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-common.c b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
> index 2b0211870f02..51376f69d007 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.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)
> +		pcie_wait_after_link_train(pcie->max_link_speed);
> +
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(cdns_pcie_host_start_link);
> diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
> index 574e9cf4d003..e222b095d2b6 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
> 

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

Re: [PATCH v2 4/8] PCI: j721e: Set max_link_speed to enable 100 ms delay after link up

[Manivannan Sadhasivam]

On Wed, May 06, 2026 at 11:23:42PM +0800, Hans Zhang wrote:
> Set cdns_pcie.max_link_speed to the maximum supported link speed
> (obtained from the device tree property "max-link-speed") in
> j721e_pcie_set_link_speed(). This activates the post-link delay logic
> added in cdns_pcie_host_start_link() when the controller supports
> speeds greater than 5 GT/s.
> 
> As required by PCIe r6.0 sec 6.6.1, and following the same approach as
> commit 80dc18a0cba8d ("PCI: dwc: Ensure that dw_pcie_wait_for_link()
> waits 100 ms after link up"), this ensures a 100 ms delay after link
> training completes before any Configuration Request is sent.
> 
> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---
>  drivers/pci/controller/cadence/pci-j721e.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> 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;

What about other glue drivers?

- Mani

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

Re: [PATCH v2 4/8] PCI: j721e: Set max_link_speed to enable 100 ms delay after link up

[Hans Zhang]

On 5/7/26 00:04, Manivannan Sadhasivam wrote:
> On Wed, May 06, 2026 at 11:23:42PM +0800, Hans Zhang wrote:
>> Set cdns_pcie.max_link_speed to the maximum supported link speed
>> (obtained from the device tree property "max-link-speed") in
>> j721e_pcie_set_link_speed(). This activates the post-link delay logic
>> added in cdns_pcie_host_start_link() when the controller supports
>> speeds greater than 5 GT/s.
>>
>> As required by PCIe r6.0 sec 6.6.1, and following the same approach as
>> commit 80dc18a0cba8d ("PCI: dwc: Ensure that dw_pcie_wait_for_link()
>> waits 100 ms after link up"), this ensures a 100 ms delay after link
>> training completes before any Configuration Request is sent.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>>   drivers/pci/controller/cadence/pci-j721e.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> 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;
> 
> What about other glue drivers?

Hi Mani,

pci-sky1.c:
There is no time to handle the corresponding DTS yet, and the attribute 
"max-link-speed" has not been parsed either. There will be a plan for 
the subsequent upstream.

pcie-sg2042.c：
I'm not familiar with this product.


Do you mean something like a dwc driver, similar to being placed in the 
file drivers/pci/controller/dwc/pcie-designware.c?
pci->max_link_speed = of_pci_get_max_link_speed(np);

For the Cadence driver, it is located in 
drivers/pci/controller/cadence/pcie-cadence-host-common.c.


Best regards,
Hans

> 
> - Mani
> 

Re: [PATCH v2 1/8] PCI: Add pcie_wait_after_link_train() helper

[Hans Zhang]

On 5/6/26 23:55, Manivannan Sadhasivam wrote:
> On Wed, May 06, 2026 at 11:23:39PM +0800, 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 pcie_wait_after_link_train() 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. The helper uses
>> the existing PCIE_RESET_CONFIG_WAIT_MS macro defined in pci.h.
>>
>> 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/pci.h | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>> index 4a14f88e543a..a8705a2a2d85 100644
>> --- a/drivers/pci/pci.h
>> +++ b/drivers/pci/pci.h
> 
> Since this helper is for host controller drivers, this needs to be defined in
> pci-host-common.h.
> 

Hi Mani,

Thank you very much for your reply and suggestions.

Will change.

>> @@ -60,6 +60,19 @@ struct pcie_tlp_log;
>>    */
>>   #define PCIE_RESET_CONFIG_WAIT_MS	100
>>   
>> +/**
>> + * pcie_wait_after_link_train - 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 pcie_wait_after_link_train(int max_link_speed)
> 
> pci_host_common_link_train_delay()?

I think it's really great. Thanks.

Best regards,
Hans

> 
> - Mani
> 

Re: [PATCH v2 2/8] PCI: cadence: LGA: Add max_link_speed field and 100 ms delay after link training

[Hans Zhang]

On 5/7/26 00:03, Manivannan Sadhasivam wrote:
> On Wed, May 06, 2026 at 11:23:40PM +0800, Hans Zhang wrote:
>> 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 to record the maximum
>> supported link speed (or the currently configured speed). In the common
>> host layer function cdns_pcie_host_start_link(), after the link has been
>> successfully established, call pcie_wait_after_link_train() to insert the
>> required delay if max_link_speed > 2.
>>
>> Glue drivers must set max_link_speed appropriately (e.g., from the device
>> tree property "max-link-speed") to enable the delay.
>>
> 
> You need to club those glue driver patches into this one. Otherwise, you'll end
> up breaking bisectability.

Hi Mani,

Will change.

Best regards,
Hans

> 
> - Mani
> 
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>>   drivers/pci/controller/cadence/pcie-cadence-host-common.c | 4 ++++
>>   drivers/pci/controller/cadence/pcie-cadence.h             | 2 ++
>>   2 files changed, 6 insertions(+)
>>
>> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-common.c b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>> index 2b0211870f02..51376f69d007 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.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)
>> +		pcie_wait_after_link_train(pcie->max_link_speed);
>> +
>>   	return ret;
>>   }
>>   EXPORT_SYMBOL_GPL(cdns_pcie_host_start_link);
>> diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h
>> index 574e9cf4d003..e222b095d2b6 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
>>
> 

Re: [PATCH v2 1/8] PCI: Add pcie_wait_after_link_train() helper

[Hans Zhang]

On 5/6/26 23:34, Biju Das wrote:
> 
> 
>> -----Original Message-----
>> From: Hans Zhang <18255117159@163.com>
>> Sent: 06 May 2026 16:24
>> Subject: [PATCH v2 1/8] PCI: Add pcie_wait_after_link_train() helper
>>
>> 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 pcie_wait_after_link_train() 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. The helper uses the existing
>> PCIE_RESET_CONFIG_WAIT_MS macro defined in pci.h.
>>
>> 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/pci.h | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 4a14f88e543a..a8705a2a2d85 100644
>> --- a/drivers/pci/pci.h
>> +++ b/drivers/pci/pci.h
>> @@ -60,6 +60,19 @@ struct pcie_tlp_log;
>>    */
>>   #define PCIE_RESET_CONFIG_WAIT_MS	100
>>
>> +/**
>> + * pcie_wait_after_link_train - 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 pcie_wait_after_link_train(int max_link_speed) {
>> +	if (max_link_speed > 2)
>> +		msleep(PCIE_RESET_CONFIG_WAIT_MS);
>> +}
> 
> Maybe move this patch to the user??

Hi Biju,

Just as Mani's reply stated, I am more inclined to place it in the 
common header file.

Best regards,
Hans

> 
> Cheers,
> Biju
> 
>> +
>>   /* Parameters for the waiting for link up routine */
>>   #define PCIE_LINK_WAIT_MAX_RETRIES	10
>>   #define PCIE_LINK_WAIT_SLEEP_MS		90
>> --
>> 2.34.1
>>

Re: [PATCH v2 2/8] PCI: cadence: LGA: Add max_link_speed field and 100 ms delay after link training

[Hans Zhang]

On 5/6/26 23:31, Biju Das wrote:
> 
> 
>> -----Original Message-----
>> From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org> On Behalf Of Hans Zhang
>> Sent: 06 May 2026 16:24
>> Subject: [PATCH v2 2/8] PCI: cadence: LGA: Add max_link_speed field and 100 ms delay after link
>> training
>>
>> 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 to record the maximum supported link speed (or the
>> currently configured speed). In the common host layer function cdns_pcie_host_start_link(), after the
>> link has been successfully established, call pcie_wait_after_link_train() to insert the required delay
>> if max_link_speed > 2.
>>
>> Glue drivers must set max_link_speed appropriately (e.g., from the device tree property "max-link-
>> speed") to enable the delay.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>>   drivers/pci/controller/cadence/pcie-cadence-host-common.c | 4 ++++
>>   drivers/pci/controller/cadence/pcie-cadence.h             | 2 ++
>>   2 files changed, 6 insertions(+)
>>
>> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>> b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>> index 2b0211870f02..51376f69d007 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.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)
>> +		pcie_wait_after_link_train(pcie->max_link_speed);
>> +
>>   	return ret;
>>   }
>>   EXPORT_SYMBOL_GPL(cdns_pcie_host_start_link);
>> diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-
>> cadence.h
>> index 574e9cf4d003..e222b095d2b6 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
> 
> Maximum to make consistent with other comments?

Hi Biju,

The reference I used is:

drivers/pci/controller/pcie-rzg3s-host.c
/**
  ......
  * @max_link_speed: maximum supported link speed
  */
struct rzg3s_pcie_host {
......


> 
> 
>>    */
>>   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;
> 
> unsigned int as speed cannot be negative??

The following file referred to:

drivers/pci/controller/dwc/pcie-designware.h
struct dw_pcie {
     ......
     int			max_link_speed;
     ......
};


Best regards,
Hans
> 
> Cheers,
> Biju
> 
>>   };
>>
>>   /**
>> --
>> 2.34.1
>>

RE: [PATCH v2 2/8] PCI: cadence: LGA: Add max_link_speed field and 100 ms delay after link training

[Biju Das]

Hi Hans,

> -----Original Message-----
> From: Hans Zhang <18255117159@163.com>
> Sent: 06 May 2026 17:21
> Subject: Re: [PATCH v2 2/8] PCI: cadence: LGA: Add max_link_speed field and 100 ms delay after link
> training
> 
> 
> 
> On 5/6/26 23:31, Biju Das wrote:
> >
> >
> >> -----Original Message-----
> >> From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org>
> >> On Behalf Of Hans Zhang
> >> Sent: 06 May 2026 16:24
> >> Subject: [PATCH v2 2/8] PCI: cadence: LGA: Add max_link_speed field
> >> and 100 ms delay after link training
> >>
> >> 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 to record the
> >> maximum supported link speed (or the currently configured speed). In
> >> the common host layer function cdns_pcie_host_start_link(), after the
> >> link has been successfully established, call pcie_wait_after_link_train() to insert the required
> delay if max_link_speed > 2.
> >>
> >> Glue drivers must set max_link_speed appropriately (e.g., from the
> >> device tree property "max-link-
> >> speed") to enable the delay.
> >>
> >> Signed-off-by: Hans Zhang <18255117159@163.com>
> >> ---
> >>   drivers/pci/controller/cadence/pcie-cadence-host-common.c | 4 ++++
> >>   drivers/pci/controller/cadence/pcie-cadence.h             | 2 ++
> >>   2 files changed, 6 insertions(+)
> >>
> >> diff --git
> >> a/drivers/pci/controller/cadence/pcie-cadence-host-common.c
> >> b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
> >> index 2b0211870f02..51376f69d007 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.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)
> >> +		pcie_wait_after_link_train(pcie->max_link_speed);
> >> +
> >>   	return ret;
> >>   }
> >>   EXPORT_SYMBOL_GPL(cdns_pcie_host_start_link);
> >> diff --git a/drivers/pci/controller/cadence/pcie-cadence.h
> >> b/drivers/pci/controller/cadence/pcie-
> >> cadence.h
> >> index 574e9cf4d003..e222b095d2b6 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
> >
> > Maximum to make consistent with other comments?
> 
> Hi Biju,
> 
> The reference I used is:
> 
> drivers/pci/controller/pcie-rzg3s-host.c

* @ops: Platform-specific ops to control various inputs from Cadence PCIe
*       wrapper
* @cdns_pcie_reg_offsets: Register bank offsets for different SoC

Bute here drivers/pci/controller/cadence/pcie-cadence.h, all start with Capital letter.


> 
> drivers/pci/controller/pcie-rzg3s-host.c
> /**
>   ......
>   * @max_link_speed: maximum supported link speed
>   */
> struct rzg3s_pcie_host {
> ......
> 
> 
> >
> >
> >>    */
> >>   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;
> >
> > unsigned int as speed cannot be negative??
> 
> The following file referred to:
> 
> drivers/pci/controller/dwc/pcie-designware.h
> struct dw_pcie {
>      ......
>      int			max_link_speed;

Maybe that driver is using negative values.
Is this driver using negative values for speed?

Cheers,
Biju

>      ......
> };
> 
> 
> Best regards,
> Hans
> >
> > Cheers,
> > Biju
> >
> >>   };
> >>
> >>   /**
> >> --
> >> 2.34.1
> >>

Re: [PATCH v2 2/8] PCI: cadence: LGA: Add max_link_speed field and 100 ms delay after link training

[Hans Zhang]

On 5/7/26 00:27, Biju Das wrote:
> Hi Hans,
> 
>> -----Original Message-----
>> From: Hans Zhang <18255117159@163.com>
>> Sent: 06 May 2026 17:21
>> Subject: Re: [PATCH v2 2/8] PCI: cadence: LGA: Add max_link_speed field and 100 ms delay after link
>> training
>>
>>
>>
>> On 5/6/26 23:31, Biju Das wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: linux-arm-kernel <linux-arm-kernel-bounces@lists.infradead.org>
>>>> On Behalf Of Hans Zhang
>>>> Sent: 06 May 2026 16:24
>>>> Subject: [PATCH v2 2/8] PCI: cadence: LGA: Add max_link_speed field
>>>> and 100 ms delay after link training
>>>>
>>>> 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 to record the
>>>> maximum supported link speed (or the currently configured speed). In
>>>> the common host layer function cdns_pcie_host_start_link(), after the
>>>> link has been successfully established, call pcie_wait_after_link_train() to insert the required
>> delay if max_link_speed > 2.
>>>>
>>>> Glue drivers must set max_link_speed appropriately (e.g., from the
>>>> device tree property "max-link-
>>>> speed") to enable the delay.
>>>>
>>>> Signed-off-by: Hans Zhang <18255117159@163.com>
>>>> ---
>>>>    drivers/pci/controller/cadence/pcie-cadence-host-common.c | 4 ++++
>>>>    drivers/pci/controller/cadence/pcie-cadence.h             | 2 ++
>>>>    2 files changed, 6 insertions(+)
>>>>
>>>> diff --git
>>>> a/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>>>> b/drivers/pci/controller/cadence/pcie-cadence-host-common.c
>>>> index 2b0211870f02..51376f69d007 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.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)
>>>> +		pcie_wait_after_link_train(pcie->max_link_speed);
>>>> +
>>>>    	return ret;
>>>>    }
>>>>    EXPORT_SYMBOL_GPL(cdns_pcie_host_start_link);
>>>> diff --git a/drivers/pci/controller/cadence/pcie-cadence.h
>>>> b/drivers/pci/controller/cadence/pcie-
>>>> cadence.h
>>>> index 574e9cf4d003..e222b095d2b6 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
>>>
>>> Maximum to make consistent with other comments?
>>
>> Hi Biju,
>>
>> The reference I used is:
>>
>> drivers/pci/controller/pcie-rzg3s-host.c
> 
> * @ops: Platform-specific ops to control various inputs from Cadence PCIe
> *       wrapper
> * @cdns_pcie_reg_offsets: Register bank offsets for different SoC
> 
> Bute here drivers/pci/controller/cadence/pcie-cadence.h, all start with Capital letter.
> 

Hi Biju,

Thanks, will change.

> 
>>
>> drivers/pci/controller/pcie-rzg3s-host.c
>> /**
>>    ......
>>    * @max_link_speed: maximum supported link speed
>>    */
>> struct rzg3s_pcie_host {
>> ......
>>
>>
>>>
>>>
>>>>     */
>>>>    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;
>>>
>>> unsigned int as speed cannot be negative??
>>
>> The following file referred to:
>>
>> drivers/pci/controller/dwc/pcie-designware.h
>> struct dw_pcie {
>>       ......
>>       int			max_link_speed;
> 
> Maybe that driver is using negative values.
> Is this driver using negative values for speed?

For speed, there are no negative values.

Best regards,
Hans

> 
> Cheers,
> Biju
> 
>>       ......
>> };
>>
>>
>> Best regards,
>> Hans
>>>
>>> Cheers,
>>> Biju
>>>
>>>>    };
>>>>
>>>>    /**
>>>> --
>>>> 2.34.1
>>>>
> 

Re: [PATCH v2 4/8] PCI: j721e: Set max_link_speed to enable 100 ms delay after link up

[Manivannan Sadhasivam]

On Thu, May 07, 2026 at 12:11:22AM +0800, Hans Zhang wrote:
> 
> 
> On 5/7/26 00:04, Manivannan Sadhasivam wrote:
> > On Wed, May 06, 2026 at 11:23:42PM +0800, Hans Zhang wrote:
> > > Set cdns_pcie.max_link_speed to the maximum supported link speed
> > > (obtained from the device tree property "max-link-speed") in
> > > j721e_pcie_set_link_speed(). This activates the post-link delay logic
> > > added in cdns_pcie_host_start_link() when the controller supports
> > > speeds greater than 5 GT/s.
> > > 
> > > As required by PCIe r6.0 sec 6.6.1, and following the same approach as
> > > commit 80dc18a0cba8d ("PCI: dwc: Ensure that dw_pcie_wait_for_link()
> > > waits 100 ms after link up"), this ensures a 100 ms delay after link
> > > training completes before any Configuration Request is sent.
> > > 
> > > Signed-off-by: Hans Zhang <18255117159@163.com>
> > > ---
> > >   drivers/pci/controller/cadence/pci-j721e.c | 1 +
> > >   1 file changed, 1 insertion(+)
> > > 
> > > 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;
> > 
> > What about other glue drivers?
> 
> Hi Mani,
> 
> pci-sky1.c:
> There is no time to handle the corresponding DTS yet, and the attribute
> "max-link-speed" has not been parsed either. There will be a plan for the
> subsequent upstream.
> 
> pcie-sg2042.c：
> I'm not familiar with this product.
> 

Since this helper gets called unconditionally for all glue drivers, not setting
'max_link_speed' would cause 100ms delay for all link speeds.

So either set 'max_link_speed' for all drivers, or call the helper directly from
glue drivers.

- Mani

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

Re: [PATCH v2 8/8] PCI: rzg3s-host: Add 100 ms delay after link training

[Claudiu Beznea]

Hi, Hans,

On 5/6/26 18:23, Hans Zhang wrote:
> The Renesas RZ/G3S PCIe host driver currently does not enforce the
> mandatory 100 ms delay after link training completes for speeds > 5.0 GT/s,
> required by PCIe r6.0 sec 6.6.1.
> 
> The driver already has a 'max_link_speed' field (derived from the device
> tree). Add a call to pcie_wait_after_link_train() in
> rzg3s_pcie_host_init() after reading the link status, ensuring that the
> delay is applied before any Configuration Request is sent downstream.
> 
> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---
>   drivers/pci/controller/pcie-rzg3s-host.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index d86e7516dcc2..6ab59c5464cf 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> @@ -1390,6 +1390,8 @@ static int rzg3s_pcie_host_init(struct rzg3s_pcie_host *host)
>   	val = readl_relaxed(host->axi + RZG3S_PCI_PCSTAT2);
>   	dev_info(host->dev, "PCIe link status [0x%x]\n", val);
>   
> +	pcie_wait_after_link_train(host->max_link_speed);

There is an msleep(PCIE_RESET_CONFIG_WAIT_MS) after 
rzg3s_pcie_set_max_link_speed() call. Shouldn't that msleep() call be replaced 
with your pcie_wait_after_link_train() ?

Thank you,
Claudiu

Re: [PATCH v2 8/8] PCI: rzg3s-host: Add 100 ms delay after link training

[Hans Zhang]

On 5/7/26 00:52, Claudiu Beznea wrote:
> Hi, Hans,
> 
> On 5/6/26 18:23, Hans Zhang wrote:
>> The Renesas RZ/G3S PCIe host driver currently does not enforce the
>> mandatory 100 ms delay after link training completes for speeds > 5.0 
>> GT/s,
>> required by PCIe r6.0 sec 6.6.1.
>>
>> The driver already has a 'max_link_speed' field (derived from the device
>> tree). Add a call to pcie_wait_after_link_train() in
>> rzg3s_pcie_host_init() after reading the link status, ensuring that the
>> delay is applied before any Configuration Request is sent downstream.
>>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>>   drivers/pci/controller/pcie-rzg3s-host.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/ 
>> controller/pcie-rzg3s-host.c
>> index d86e7516dcc2..6ab59c5464cf 100644
>> --- a/drivers/pci/controller/pcie-rzg3s-host.c
>> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
>> @@ -1390,6 +1390,8 @@ static int rzg3s_pcie_host_init(struct 
>> rzg3s_pcie_host *host)
>>       val = readl_relaxed(host->axi + RZG3S_PCI_PCSTAT2);
>>       dev_info(host->dev, "PCIe link status [0x%x]\n", val);
>> +    pcie_wait_after_link_train(host->max_link_speed);
> 
> There is an msleep(PCIE_RESET_CONFIG_WAIT_MS) after 
> rzg3s_pcie_set_max_link_speed() call. Shouldn't that msleep() call be 
> replaced with your pcie_wait_after_link_train() ?

Hi Claudiu,

Sorry for the late reply. Thank you for pointing it out. It will be 
replaced.

Best regards,
Hans


> 
> Thank you,
> Claudiu

Re: [PATCH v2 6/8] PCI: aardvark: Add 100 ms delay after link training

[Pali Rohár]

On Wednesday 06 May 2026 23:23:44 Hans Zhang wrote:
> 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 pcie_wait_after_link_train() 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 | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> index e34bea1ff0ac..526351c21c49 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
> @@ -350,8 +350,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)) {
> +			pcie_wait_after_link_train(pcie->link_gen);
>  			return 0;
> +		}
>  
>  		usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
>  	}
> -- 
> 2.34.1
> 

Are you sure that this is correct to do? Have you checked the A3720
Functional Specification which describes how to bring PCIe link up?

A3720 PCIe controller is buggy and needs more timing hacks to make it
behave. Playing with random sleeps can break its internal logic.
I'm not sure if it could be safe without proper testing.

And IIRC A3720 PCIe controller is just PCIe2.0 with 5 GT/s.

Re: [PATCH v2 6/8] PCI: aardvark: Add 100 ms delay after link training

[Hans Zhang]

On 5/13/26 05:25, Pali Rohár wrote:
> On Wednesday 06 May 2026 23:23:44 Hans Zhang wrote:
>> 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 pcie_wait_after_link_train() 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 | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
>> index e34bea1ff0ac..526351c21c49 100644
>> --- a/drivers/pci/controller/pci-aardvark.c
>> +++ b/drivers/pci/controller/pci-aardvark.c
>> @@ -350,8 +350,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)) {
>> +			pcie_wait_after_link_train(pcie->link_gen);
>>   			return 0;
>> +		}
>>   
>>   		usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
>>   	}
>> -- 
>> 2.34.1
>>
> 
> Are you sure that this is correct to do? Have you checked the A3720
> Functional Specification which describes how to bring PCIe link up?
> 
> A3720 PCIe controller is buggy and needs more timing hacks to make it
> behave. Playing with random sleeps can break its internal logic.
> I'm not sure if it could be safe without proper testing.
> 
> And IIRC A3720 PCIe controller is just PCIe2.0 with 5 GT/s.


Hi Pali,

1. This driver does not support A3720.

static const struct of_device_id advk_pcie_of_match_table[] = {
	{ .compatible = "marvell,armada-3700-pcie", },
	{},
};
MODULE_DEVICE_TABLE(of, advk_pcie_of_match_table);

If you need support for A3720, please submit the corresponding patch so 
that Bjorn and Mani can review it.


2. If A3720 only supports GEN2, you can configure "max-link-speed" to be 
2 in the DT. This will not affect the functionality of this patch.

3. This patch is a common delay requirement stipulated by the PCIe 
specification. If it is greater than GEN2, then msleep(100) will be 
added; otherwise, there will be no such delay.

4. For instance, we often come across the situation where some common 
APIs are modified, and in many cases, their functionality does not 
require the actual development board for verification. I believe that 
many other developers and maintainers have modified different parts of 
the code. For example, the recent submission:

commit 750277048afe7ce8ebfc0b120de7dfbc745058a7
Author: Nam Cao <namcao@linutronix.de>
Date:   Thu Jun 26 16:47:53 2025 +0200

     PCI: aardvark: Switch to msi_create_parent_irq_domain()

     Switch to msi_create_parent_irq_domain() from 
pci_msi_create_irq_domain()
     which was using legacy MSI domain setup.


And many controller drivers have been modified.


Best regards,
Hans

Re: [PATCH v2 6/8] PCI: aardvark: Add 100 ms delay after link training

[Pali Rohár]

On Wednesday 13 May 2026 15:00:04 Hans Zhang wrote:
> 
> 
> On 5/13/26 05:25, Pali Rohár wrote:
> > On Wednesday 06 May 2026 23:23:44 Hans Zhang wrote:
> > > 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 pcie_wait_after_link_train() 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 | 4 +++-
> > >   1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> > > index e34bea1ff0ac..526351c21c49 100644
> > > --- a/drivers/pci/controller/pci-aardvark.c
> > > +++ b/drivers/pci/controller/pci-aardvark.c
> > > @@ -350,8 +350,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)) {
> > > +			pcie_wait_after_link_train(pcie->link_gen);
> > >   			return 0;
> > > +		}
> > >   		usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
> > >   	}
> > > -- 
> > > 2.34.1
> > > 
> > 
> > Are you sure that this is correct to do? Have you checked the A3720
> > Functional Specification which describes how to bring PCIe link up?
> > 
> > A3720 PCIe controller is buggy and needs more timing hacks to make it
> > behave. Playing with random sleeps can break its internal logic.
> > I'm not sure if it could be safe without proper testing.
> > 
> > And IIRC A3720 PCIe controller is just PCIe2.0 with 5 GT/s.
> 
> 
> Hi Pali,
> 
> 1. This driver does not support A3720.
> 
> static const struct of_device_id advk_pcie_of_match_table[] = {
> 	{ .compatible = "marvell,armada-3700-pcie", },
> 	{},
> };
> MODULE_DEVICE_TABLE(of, advk_pcie_of_match_table);
> 
> If you need support for A3720, please submit the corresponding patch so that
> Bjorn and Mani can review it.

3700 (or 37xx) is family and covers both a3710 and a3720. In most cases is the
a3720 dominant and hence identifiers 3700 and 3720 are begin mixed.

> 
> 2. If A3720 only supports GEN2, you can configure "max-link-speed" to be 2
> in the DT. This will not affect the functionality of this patch.

Whole A37xx supports only GEN2. And in DT files for 37xx should be
already there max-link-speed.

Seems that in advk_pcie_of_match_table there is no GEN3 device
specified.

> 3. This patch is a common delay requirement stipulated by the PCIe
> specification. If it is greater than GEN2, then msleep(100) will be added;
> otherwise, there will be no such delay.
> 
> 4. For instance, we often come across the situation where some common APIs
> are modified, and in many cases, their functionality does not require the
> actual development board for verification. I believe that many other
> developers and maintainers have modified different parts of the code. For
> example, the recent submission:

Switching one API to another is one thing. But changing code which looks
to be critical, specially when it is known that hw has bugs, can cause
breaking of existing boards.

> commit 750277048afe7ce8ebfc0b120de7dfbc745058a7
> Author: Nam Cao <namcao@linutronix.de>
> Date:   Thu Jun 26 16:47:53 2025 +0200
> 
>     PCI: aardvark: Switch to msi_create_parent_irq_domain()
> 
>     Switch to msi_create_parent_irq_domain() from
> pci_msi_create_irq_domain()
>     which was using legacy MSI domain setup.
> 
> 
> And many controller drivers have been modified.
> 
> 
> Best regards,
> Hans
> 
> 

Re: [PATCH v2 6/8] PCI: aardvark: Add 100 ms delay after link training

[Hans Zhang]

On 5/13/26 15:20, Pali Rohár wrote:
> On Wednesday 13 May 2026 15:00:04 Hans Zhang wrote:
>>
>>
>> On 5/13/26 05:25, Pali Rohár wrote:
>>> On Wednesday 06 May 2026 23:23:44 Hans Zhang wrote:
>>>> 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 pcie_wait_after_link_train() 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 | 4 +++-
>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
>>>> index e34bea1ff0ac..526351c21c49 100644
>>>> --- a/drivers/pci/controller/pci-aardvark.c
>>>> +++ b/drivers/pci/controller/pci-aardvark.c
>>>> @@ -350,8 +350,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)) {
>>>> +			pcie_wait_after_link_train(pcie->link_gen);
>>>>    			return 0;
>>>> +		}
>>>>    		usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
>>>>    	}
>>>> -- 
>>>> 2.34.1
>>>>
>>>
>>> Are you sure that this is correct to do? Have you checked the A3720
>>> Functional Specification which describes how to bring PCIe link up?
>>>
>>> A3720 PCIe controller is buggy and needs more timing hacks to make it
>>> behave. Playing with random sleeps can break its internal logic.
>>> I'm not sure if it could be safe without proper testing.
>>>
>>> And IIRC A3720 PCIe controller is just PCIe2.0 with 5 GT/s.
>>
>>
>> Hi Pali,
>>
>> 1. This driver does not support A3720.
>>
>> static const struct of_device_id advk_pcie_of_match_table[] = {
>> 	{ .compatible = "marvell,armada-3700-pcie", },
>> 	{},
>> };
>> MODULE_DEVICE_TABLE(of, advk_pcie_of_match_table);
>>
>> If you need support for A3720, please submit the corresponding patch so that
>> Bjorn and Mani can review it.
> 
> 3700 (or 37xx) is family and covers both a3710 and a3720. In most cases is the
> a3720 dominant and hence identifiers 3700 and 3720 are begin mixed.
> 
>>
>> 2. If A3720 only supports GEN2, you can configure "max-link-speed" to be 2
>> in the DT. This will not affect the functionality of this patch.
> 
> Whole A37xx supports only GEN2. And in DT files for 37xx should be
> already there max-link-speed.
> 
> Seems that in advk_pcie_of_match_table there is no GEN3 device
> specified.
> 

Hi Pali,

However, I saw many GEN3 assignments and conditions in the code.

ret = of_pci_get_max_link_speed(dev->of_node);
if (ret <= 0 || ret > 3)
	pcie->link_gen = 3;
else
	pcie->link_gen = ret;


static void advk_pcie_train_link(struct advk_pcie *pcie)
{
	struct device *dev = &pcie->pdev->dev;
	u32 reg;
	int ret;

	/*
	 * Setup PCIe rev / gen compliance based on device tree property
	 * 'max-link-speed' which also forces maximal link speed.
	 */
	reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
	reg &= ~PCIE_GEN_SEL_MSK;
	if (pcie->link_gen == 3)
		reg |= SPEED_GEN_3;
	else if (pcie->link_gen == 2)
		reg |= SPEED_GEN_2;
	else
		reg |= SPEED_GEN_1;
	advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);

	/*
	 * Set maximal link speed value also into PCIe Link Control 2 register.
	 * Armada 3700 Functional Specification says that default value is based
	 * on SPEED_GEN but tests showed that default value is always 8.0 GT/s.
	 */
	reg = advk_readl(pcie, PCIE_CORE_PCIEXP_CAP + PCI_EXP_LNKCTL2);
	reg &= ~PCI_EXP_LNKCTL2_TLS;
	if (pcie->link_gen == 3)
		reg |= PCI_EXP_LNKCTL2_TLS_8_0GT;
	else if (pcie->link_gen == 2)
		reg |= PCI_EXP_LNKCTL2_TLS_5_0GT;
	else
		reg |= PCI_EXP_LNKCTL2_TLS_2_5GT;
	advk_writel(pcie, reg, PCIE_CORE_PCIEXP_CAP + PCI_EXP_LNKCTL2);

....


If you are certain about the relevant information. Is it understandable 
that we need to delete the code related to GEN3?


Best regards,
Hans


>> 3. This patch is a common delay requirement stipulated by the PCIe
>> specification. If it is greater than GEN2, then msleep(100) will be added;
>> otherwise, there will be no such delay.
>>
>> 4. For instance, we often come across the situation where some common APIs
>> are modified, and in many cases, their functionality does not require the
>> actual development board for verification. I believe that many other
>> developers and maintainers have modified different parts of the code. For
>> example, the recent submission:
> 
> Switching one API to another is one thing. But changing code which looks
> to be critical, specially when it is known that hw has bugs, can cause
> breaking of existing boards.
> 
>> commit 750277048afe7ce8ebfc0b120de7dfbc745058a7
>> Author: Nam Cao <namcao@linutronix.de>
>> Date:   Thu Jun 26 16:47:53 2025 +0200
>>
>>      PCI: aardvark: Switch to msi_create_parent_irq_domain()
>>
>>      Switch to msi_create_parent_irq_domain() from
>> pci_msi_create_irq_domain()
>>      which was using legacy MSI domain setup.
>>
>>
>> And many controller drivers have been modified.
>>
>>
>> Best regards,
>> Hans
>>
>>

Re: [PATCH v2 6/8] PCI: aardvark: Add 100 ms delay after link training

[Pali Rohár]

On Wednesday 13 May 2026 15:34:46 Hans Zhang wrote:
> On 5/13/26 15:20, Pali Rohár wrote:
> > On Wednesday 13 May 2026 15:00:04 Hans Zhang wrote:
> > > 
> > > 
> > > On 5/13/26 05:25, Pali Rohár wrote:
> > > > On Wednesday 06 May 2026 23:23:44 Hans Zhang wrote:
> > > > > 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 pcie_wait_after_link_train() 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 | 4 +++-
> > > > >    1 file changed, 3 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> > > > > index e34bea1ff0ac..526351c21c49 100644
> > > > > --- a/drivers/pci/controller/pci-aardvark.c
> > > > > +++ b/drivers/pci/controller/pci-aardvark.c
> > > > > @@ -350,8 +350,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)) {
> > > > > +			pcie_wait_after_link_train(pcie->link_gen);
> > > > >    			return 0;
> > > > > +		}
> > > > >    		usleep_range(LINK_WAIT_USLEEP_MIN, LINK_WAIT_USLEEP_MAX);
> > > > >    	}
> > > > > -- 
> > > > > 2.34.1
> > > > > 
> > > > 
> > > > Are you sure that this is correct to do? Have you checked the A3720
> > > > Functional Specification which describes how to bring PCIe link up?
> > > > 
> > > > A3720 PCIe controller is buggy and needs more timing hacks to make it
> > > > behave. Playing with random sleeps can break its internal logic.
> > > > I'm not sure if it could be safe without proper testing.
> > > > 
> > > > And IIRC A3720 PCIe controller is just PCIe2.0 with 5 GT/s.
> > > 
> > > 
> > > Hi Pali,
> > > 
> > > 1. This driver does not support A3720.
> > > 
> > > static const struct of_device_id advk_pcie_of_match_table[] = {
> > > 	{ .compatible = "marvell,armada-3700-pcie", },
> > > 	{},
> > > };
> > > MODULE_DEVICE_TABLE(of, advk_pcie_of_match_table);
> > > 
> > > If you need support for A3720, please submit the corresponding patch so that
> > > Bjorn and Mani can review it.
> > 
> > 3700 (or 37xx) is family and covers both a3710 and a3720. In most cases is the
> > a3720 dominant and hence identifiers 3700 and 3720 are begin mixed.
> > 
> > > 
> > > 2. If A3720 only supports GEN2, you can configure "max-link-speed" to be 2
> > > in the DT. This will not affect the functionality of this patch.
> > 
> > Whole A37xx supports only GEN2. And in DT files for 37xx should be
> > already there max-link-speed.
> > 
> > Seems that in advk_pcie_of_match_table there is no GEN3 device
> > specified.
> > 
> 
> Hi Pali,
> 
> However, I saw many GEN3 assignments and conditions in the code.
> 
> ret = of_pci_get_max_link_speed(dev->of_node);
> if (ret <= 0 || ret > 3)
> 	pcie->link_gen = 3;
> else
> 	pcie->link_gen = ret;
> 
> 
> static void advk_pcie_train_link(struct advk_pcie *pcie)
> {
> 	struct device *dev = &pcie->pdev->dev;
> 	u32 reg;
> 	int ret;
> 
> 	/*
> 	 * Setup PCIe rev / gen compliance based on device tree property
> 	 * 'max-link-speed' which also forces maximal link speed.
> 	 */
> 	reg = advk_readl(pcie, PCIE_CORE_CTRL0_REG);
> 	reg &= ~PCIE_GEN_SEL_MSK;
> 	if (pcie->link_gen == 3)
> 		reg |= SPEED_GEN_3;
> 	else if (pcie->link_gen == 2)
> 		reg |= SPEED_GEN_2;
> 	else
> 		reg |= SPEED_GEN_1;
> 	advk_writel(pcie, reg, PCIE_CORE_CTRL0_REG);
> 
> 	/*
> 	 * Set maximal link speed value also into PCIe Link Control 2 register.
> 	 * Armada 3700 Functional Specification says that default value is based
> 	 * on SPEED_GEN but tests showed that default value is always 8.0 GT/s.
> 	 */
> 	reg = advk_readl(pcie, PCIE_CORE_PCIEXP_CAP + PCI_EXP_LNKCTL2);
> 	reg &= ~PCI_EXP_LNKCTL2_TLS;
> 	if (pcie->link_gen == 3)
> 		reg |= PCI_EXP_LNKCTL2_TLS_8_0GT;
> 	else if (pcie->link_gen == 2)
> 		reg |= PCI_EXP_LNKCTL2_TLS_5_0GT;
> 	else
> 		reg |= PCI_EXP_LNKCTL2_TLS_2_5GT;
> 	advk_writel(pcie, reg, PCIE_CORE_PCIEXP_CAP + PCI_EXP_LNKCTL2);
> 
> ....
> 
> 
> If you are certain about the relevant information. Is it understandable that
> we need to delete the code related to GEN3?

Ok. So some explanation. pci-aardvark.c is implementing driver for PCIe
controller with codename aardvark. I have no idea from what this
codename comes and what is represents. What we know that the driver was
written for A37xx SoC platform according to A37xx functional specification.
As it is common in SoC world, vendors just buy some IP and integrate it
into SoC. In this case Marvell bought this PCIe controller IP and
integrated it into the A37xx. In past I tried to investigate what it
could be and IIRC my assumption was that it was PCIe IP from Denali.
Denali was acquired by Cadence, and when I compared Cadence PCIe
controller registers and PCIe controller registers in A37xx functional
specification there were large overlap. For me it looked like new
Cadence PCIe controller is an evolution (or new version) of what is in
A37xx. So this was some confirmation of my theory. Linux kernel has
separate driver for PCIe controller from Cadence and for refactoring
there were ideas to merge these two drivers... But there were more
important things, fix issues related to A37xx PCIe, lot of changes
which address these issues were sent to the list but they were not
taken. I do not think that it makes sense to do refactoring or doing any
other changes before addressing any existing issues with these
drivers (like PCIe card is not working correctly).

There are reported more HW erratas for this PCIe controller which needs
to be addressed in the software (meaning in Linux kernel) to make PCIe
card working properly. And there are more design HW decision which needs
does not conform to the PCIe specification and those deviations needs to
be "fixed" or "adjusted" in software (meaning in pci-aardvark.c driver)
to make PCI/PCIe compatible drivers to work correctly.

Now about GEN3. From register allocation it looks like that PCIe IP
supports GEN3. A37xx does not support it (or at least officially). This
does not mean that there cannot be some SoC with this "aardvark" PCIe IP
that is GEN3 capable. Just we see that such SoC is not supported by Linux.
Also as the comment in above code says, by default the speed is reported
as 8.0 GT/s, so changing it to 5.0 GT/s or 2.5 GT/s is needed as so code
some parts of GEN3 code in the driver is needed.

Does it makes sense to remove it? Does it makes sense to spend time on
such thing which does not address any existing issue? For me not.
Because it does not fix any _real_ issue with existing PCIe cards. And
for refactoring it is better to merge drivers as explained above and
IIRC cadence driver has HW on which is GEN3 used.

Now about your change. If you are sure that pcie_wait_after_link_train()
function is noop for pcie->link_gen == 2 || pcie->link_gen == 1 then go
ahead, I have no objects. I have not looked deeply at the change. I just
spotted some change which is touching timing critical code path which
was problematic in the past and broke many wifi cards. So I'm really
careful to prevent breaking Linux support again.

As maintainers decided to not take any new changes from me for this
driver, I have no motivation to prepare any new changes. I will rather
spend my free time on something which will make sense and not be wasting
of my free time.

> 
> Best regards,
> Hans
> 
> 
> > > 3. This patch is a common delay requirement stipulated by the PCIe
> > > specification. If it is greater than GEN2, then msleep(100) will be added;
> > > otherwise, there will be no such delay.
> > > 
> > > 4. For instance, we often come across the situation where some common APIs
> > > are modified, and in many cases, their functionality does not require the
> > > actual development board for verification. I believe that many other
> > > developers and maintainers have modified different parts of the code. For
> > > example, the recent submission:
> > 
> > Switching one API to another is one thing. But changing code which looks
> > to be critical, specially when it is known that hw has bugs, can cause
> > breaking of existing boards.
> > 
> > > commit 750277048afe7ce8ebfc0b120de7dfbc745058a7
> > > Author: Nam Cao <namcao@linutronix.de>
> > > Date:   Thu Jun 26 16:47:53 2025 +0200
> > > 
> > >      PCI: aardvark: Switch to msi_create_parent_irq_domain()
> > > 
> > >      Switch to msi_create_parent_irq_domain() from
> > > pci_msi_create_irq_domain()
> > >      which was using legacy MSI domain setup.
> > > 
> > > 
> > > And many controller drivers have been modified.
> > > 
> > > 
> > > Best regards,
> > > Hans
> > > 
> > > 
>