[RFC PATCH] PCI: pciehp: Allow more time for link activation after slot power-on

[RFC PATCH] PCI: pciehp: Allow more time for link activation after slot power-on

[Hongbo Yao]

The OCP NIC 3.0 Specification, version 1.6.0, Figure 132 [1], requires
an interval greater than one second between Main Power Valid and PERST#
deassertion.  The diagram labels this interval T_PVPERL.  PCIe link
activation requires additional time after PERST# is deasserted.

In the reported scenario, the adapter is already installed in a
powered-off slot.  Pressing the Attention Button generates the event
that causes pciehp to enable slot power and then check the link.
The one-second DLLLA polling timeout does not account for the above
reset hold interval when that interval overlaps the link wait, so it
can report failure before the adapter can activate the link.

This is observed with an OCP ConnectX-7 (MT2910 family) adapter.  On the
affected platform, power-on to PERST# deassertion takes about 1.2 seconds,
followed by a few hundred milliseconds for link activation, exceeding
the existing polling budget.

Add pcie_wait_for_link_timeout() to allow a caller-supplied initial
polling timeout.  Keep the existing pcie_wait_for_link() and
pcie_wait_for_link_delay() interfaces and their default timeout.
Use a three-second polling budget in pciehp for slots advertising
power control.  Other slots and link retraining retain one second.

Three seconds is a proposed allowance for the reported sequence, not a
timing requirement derived from the specification.

[1] OCP NIC 3.0 Specification, version 1.6.0, Figure 132,
    "Power-Up Sequencing - Normal Operation".

Link: https://www.opencompute.org/wiki/Server/NIC

Signed-off-by: Hongbo Yao <andy.xu@hj-micro.com>
---
 drivers/pci/hotplug/pciehp_hpc.c |  9 +++++++-
 drivers/pci/pci.c                | 38 ++++++++++++++++++++++----------
 drivers/pci/pci.h                |  2 ++
 3 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c
index 4c62140a3cb4..14b82b9b03a2 100644
--- a/drivers/pci/hotplug/pciehp_hpc.c
+++ b/drivers/pci/hotplug/pciehp_hpc.c
@@ -28,6 +28,8 @@
 #include "../pci.h"
 #include "pciehp.h"
 
+#define PCIEHP_LINK_UP_TIMEOUT_MS	3000
+
 static const struct dmi_system_id inband_presence_disabled_dmi_table[] = {
 	/*
 	 * Match all Dell systems, as some Dell systems have inband
@@ -291,10 +293,15 @@ static void pcie_wait_for_presence(struct pci_dev *pdev)
 int pciehp_check_link_status(struct controller *ctrl)
 {
 	struct pci_dev *pdev = ctrl_dev(ctrl);
+	unsigned int timeout_ms = PCIE_LINK_RETRAIN_TIMEOUT_MS;
 	bool found;
 	u16 lnk_status, linksta2;
 
-	if (!pcie_wait_for_link(pdev, true)) {
+	/* Account for power sequencing and reset hold time when enabling a slot. */
+	if (POWER_CTRL(ctrl))
+		timeout_ms = PCIEHP_LINK_UP_TIMEOUT_MS;
+
+	if (!pcie_wait_for_link_timeout(pdev, true, 100, timeout_ms)) {
 		ctrl_info(ctrl, "Slot(%s): No link\n", slot_name(ctrl));
 		return -1;
 	}
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index a73d1a3a8939..772d4c8abfcf 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4553,12 +4553,14 @@ static int pci_pm_reset(struct pci_dev *dev, bool probe)
  * @pdev: Device whose link to wait for.
  * @use_lt: Use the LT bit if TRUE, or the DLLLA bit if FALSE.
  * @active: Waiting for active or inactive?
+ * @timeout_ms: Link status polling timeout in milliseconds
  *
  * Return 0 if successful, or -ETIMEDOUT if status has not changed within
- * PCIE_LINK_RETRAIN_TIMEOUT_MS milliseconds.
+ * @timeout_ms milliseconds.
  */
 static int pcie_wait_for_link_status(struct pci_dev *pdev,
-				     bool use_lt, bool active)
+				     bool use_lt, bool active,
+				     unsigned int timeout_ms)
 {
 	u16 lnksta_mask, lnksta_match;
 	unsigned long end_jiffies;
@@ -4567,7 +4569,7 @@ static int pcie_wait_for_link_status(struct pci_dev *pdev,
 	lnksta_mask = use_lt ? PCI_EXP_LNKSTA_LT : PCI_EXP_LNKSTA_DLLLA;
 	lnksta_match = active ? lnksta_mask : 0;
 
-	end_jiffies = jiffies + msecs_to_jiffies(PCIE_LINK_RETRAIN_TIMEOUT_MS);
+	end_jiffies = jiffies + msecs_to_jiffies(timeout_ms);
 	do {
 		pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta);
 		if ((lnksta & lnksta_mask) == lnksta_match)
@@ -4606,7 +4608,8 @@ int pcie_retrain_link(struct pci_dev *pdev, bool use_lt)
 	 * avoid LTSSM race as recommended in Implementation Note at the end
 	 * of PCIe r6.1 sec 7.5.3.7.
 	 */
-	rc = pcie_wait_for_link_status(pdev, true, false);
+	rc = pcie_wait_for_link_status(pdev, true, false,
+				       PCIE_LINK_RETRAIN_TIMEOUT_MS);
 	if (rc)
 		return rc;
 
@@ -4620,7 +4623,8 @@ int pcie_retrain_link(struct pci_dev *pdev, bool use_lt)
 		pcie_capability_clear_word(pdev, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_RL);
 	}
 
-	rc = pcie_wait_for_link_status(pdev, use_lt, !use_lt);
+	rc = pcie_wait_for_link_status(pdev, use_lt, !use_lt,
+				       PCIE_LINK_RETRAIN_TIMEOUT_MS);
 
 	/*
 	 * Clear LBMS after a manual retrain so that the bit can be used
@@ -4644,24 +4648,27 @@ int pcie_retrain_link(struct pci_dev *pdev, bool use_lt)
 }
 
 /**
- * pcie_wait_for_link_delay - Wait until link is active or inactive
+ * pcie_wait_for_link_timeout - Wait for link with a specified polling timeout
  * @pdev: Bridge device
  * @active: waiting for active or inactive?
  * @delay: Delay to wait after link has become active (in ms)
+ * @timeout_ms: Link status polling timeout in milliseconds
  *
- * Use this to wait till link becomes active or inactive.
+ * The timeout covers initial link status polling, not the additional delays
+ * or failed-link recovery.  Without link active reporting, wait for timeout_ms
+ * plus delay instead of polling.
  */
-static bool pcie_wait_for_link_delay(struct pci_dev *pdev, bool active,
-				     int delay)
+bool pcie_wait_for_link_timeout(struct pci_dev *pdev, bool active, int delay,
+			       unsigned int timeout_ms)
 {
 	int rc;
 
 	/*
 	 * Some controllers might not implement link active reporting. In this
-	 * case, we wait for 1000 ms + any delay requested by the caller.
+	 * case, wait for timeout_ms plus any delay requested by the caller.
 	 */
 	if (!pdev->link_active_reporting) {
-		msleep(PCIE_LINK_RETRAIN_TIMEOUT_MS + delay);
+		msleep(timeout_ms + delay);
 		return true;
 	}
 
@@ -4676,7 +4683,7 @@ static bool pcie_wait_for_link_delay(struct pci_dev *pdev, bool active,
 	 */
 	if (active)
 		msleep(20);
-	rc = pcie_wait_for_link_status(pdev, false, active);
+	rc = pcie_wait_for_link_status(pdev, false, active, timeout_ms);
 	if (active) {
 		if (rc)
 			rc = pcie_failed_link_retrain(pdev);
@@ -4693,6 +4700,13 @@ static bool pcie_wait_for_link_delay(struct pci_dev *pdev, bool active,
 	return true;
 }
 
+static bool pcie_wait_for_link_delay(struct pci_dev *pdev, bool active,
+				     int delay)
+{
+	return pcie_wait_for_link_timeout(pdev, active, delay,
+					  PCIE_LINK_RETRAIN_TIMEOUT_MS);
+}
+
 /**
  * pcie_wait_for_link - Wait until link is active or inactive
  * @pdev: Bridge device
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 4469e1a77f3c..1984e4c1891c 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -1078,6 +1078,8 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
 		pci_ers_result_t (*reset_subordinates)(struct pci_dev *pdev));
 
 bool pcie_wait_for_link(struct pci_dev *pdev, bool active);
+bool pcie_wait_for_link_timeout(struct pci_dev *pdev, bool active, int delay,
+			       unsigned int timeout_ms);
 int pcie_retrain_link(struct pci_dev *pdev, bool use_lt);
 
 /* ASPM-related functionality we need even without CONFIG_PCIEASPM */
-- 
2.43.0

Re: [RFC PATCH] PCI: pciehp: Allow more time for link activation after slot power-on

[sashiko-bot]

> The OCP NIC 3.0 Specification, version 1.6.0, Figure 132 [1], requires
> an interval greater than one second between Main Power Valid and PERST#
> deassertion.  The diagram labels this interval T_PVPERL.  PCIe link
> activation requires additional time after PERST# is deasserted.
> 
> In the reported scenario, the adapter is already installed in a
> powered-off slot.  Pressing the Attention Button generates the event
> that causes pciehp to enable slot power and then check the link.
> The one-second DLLLA polling timeout does not account for the above
> reset hold interval when that interval overlaps the link wait, so it
> can report failure before the adapter can activate the link.
> 
> This is observed with an OCP ConnectX-7 (MT2910 family) adapter.  On the
> affected platform, power-on to PERST# deassertion takes about 1.2 seconds,
> followed by a few hundred milliseconds for link activation, exceeding
> [ ... ]
>     "Power-Up Sequencing - Normal Operation".
> 
> Link: https://www.opencompute.org/wiki/Server/NIC
> 
> Signed-off-by: Hongbo Yao <andy.xu@hj-micro.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910015151.1876630-1-andy.xu@hj-micro.com?part=1

Re: [RFC PATCH] PCI: pciehp: Allow more time for link activation after slot power-on

[Lukas Wunner]

On Thu, Sep 10, 2026 at 09:51:51AM +0800, Hongbo Yao wrote:
> The OCP NIC 3.0 Specification, version 1.6.0, Figure 132 [1], requires
> an interval greater than one second between Main Power Valid and PERST#
> deassertion.  The diagram labels this interval T_PVPERL.  PCIe link
> activation requires additional time after PERST# is deasserted.

The 1 sec delay implemented by:

  board_added()
    pciehp_power_on_slot()          # turn on the power
    pciehp_check_link_status()
      pcie_wait_for_link()
        pcie_wait_for_link_delay()  # wait 1 sec for DLLLA,
				    # then wait another 100 msec

... is specified by PCIe r7.0 sec 6.7.3.3, which is referenced by
sec 6.7.1.8 ("Power Controller").

In particular, sec 6.7.3.3 says:

 "The Data Link Layer State Changed event must occur within 1 second
  of the event that initiates the hot-insertion. If a power controller
  is supported, the time out interval is measured from when software
  initiated a write to the Slot Control register to turn on the power.
  [...] Software is allowed to time out on a hot add operation if the
  Data Link Layer State Changed event does not occur within 1 second."

So I think the delays observed by pciehp are conforming to the spec.

The PCIe CEM Spec r6.0.1 sec 2.11.2 specifies Tpvperl as "min 100 ms".

I don't know why the OCP NIC Spec says "> 1s" instead, but that number
isn't what the PCIe CEM Spec says and that's the authoritative document.

So I'm inclined to say the number in the OCP NIC Spec is wrong and
needs to be fixed.

> This is observed with an OCP ConnectX-7 (MT2910 family) adapter.  On the
> affected platform, power-on to PERST# deassertion takes about 1.2 seconds,
> followed by a few hundred milliseconds for link activation, exceeding
> the existing polling budget.

Which platform are we talking about?  A chassis from HJ Micro?
Has this already been shipped to customers or is it still in
internal validation?  If it's still in validation, please fix
the hardware to shorten the delays in accordance with the PCIe
Base and CEM Spec, ignoring the bogus value in the OCP NIC spec.

If it's already in customers' hands, can you fix this through
a firmware update or something like that?

I'd like to avoid lengthening the delays in pciehp beyond what
the spec prescribes, only to work around non-conforming platforms.

Thanks,

Lukas