Re: [PATCH v3] PCI: Skip Target Speed quirk on clamped ports with no link

[Andreas Wild <andiwild@gmail.com>]

On Sat, 1 Aug 2026, Aoxtj wrote:

> - 7.0.14-8 + revert 72780f796468: boots reliably.
> - 7.0.14-8 + this v3 patch: only boots sometimes; failed boots lose the
> drive the same way.

Thanks for testing it.  v3 only covers ports with no link at all, so it
does not address your case.

72780f796468 removed two guards at once: the Data Link Layer Link Active
check, and the device ID match against the ASMedia ASM2824.  My
regression comes from losing the first; yours looks like it comes from
losing the second.

v3 only bails out where DLLLA is clear.  Your link is up:

>      LnkSta:  Speed 2.5GT/s, Width x2, DLActive+

so the quirk runs past the v3 early return, lifts the restriction and
retrains at 8GT/s, exactly as it did before.  Reverting works for you
because your AMD Renoir Root Port didn't match the ASM2824 ID list that
used to gate this.

A possible workaround:

If your BIOS lets you pin that slot to Gen1, you could try that.  With
the Root Port advertising only 2.5GT/s the quirk never reaches the
retrain at all: on current mainline it returns at

	speed_cap = pcie_get_speed_cap(dev);
	if (speed_cap <= PCIE_SPEED_2_5GT)
		return ret;

and on the older code your 7.0.14 backport is based on, the
(lnkcap & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_2_5GB condition is
false, so the restriction is never lifted.

Since the link already runs at 2.5GT/s x2, that should cost you nothing
- it just makes the Root Port advertise what the link can actually do.

Something that is different in your case:

Your Root Port reports

>      LnkCtl2: Target Link Speed: 2.5GT/s, SpeedDis+

where mine reports the same clamp with SpeedDis-.  SpeedDis is
PCI_EXP_LNKCTL2_HASD, and as far as I can tell the quirk never looks at
it; the only user in the tree is pcie-designware.c.  Firmware setting
both the clamp and Hardware Autonomous Speed Disable looks like a
deliberate pin rather than an incidental one.

I am aware that HASD is specified as disabling *hardware autonomous*
speed changes and so does not literally forbid a software-initiated
retrain, so this is just a heuristic about firmware intent.

About the drive disappearing:

The error path is

	err:
		pci_info(dev, "retraining failed\n");
		pcie_set_target_speed(dev, PCIE_LNKCTL2_TLS2SPEED(oldlnkctl2), true);

which reaches pcie_retrain_link(pdev, use_lt=true), whose last wait is

	rc = pcie_wait_for_link_status(pdev, use_lt, !use_lt);

With use_lt set that waits for the Link Training bit to clear (for
training to stop) and not for DLLLA to come back.  So the restore can
return having never confirmed the link recovered.  Since the quirk runs
from pci_device_add(), before pci_scan_bridge_extend() creates the
subordinate bus, a link that comes up shortly afterwards is never
scanned and the device behind it is simply not there.  That would fit
only booting sometimes better than anything about speed policy does.

I could easily be wrong about this - I am just reading the code, and I
came to the PCI core a few days ago via this one bug.

Since you offered to test a diagnostic, here is one.  It only touches the
err: block, which is identical in mainline and in the 7.0.14 code your
backport is based on, so it should apply to your tree (it applied to
v7.1.5 here with a -2 line offset):

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index b09f27f..e4fcdfb 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -126,7 +126,32 @@ int pcie_failed_link_retrain(struct pci_dev *dev)
 	return ret;
 err:
 	pci_info(dev, "retraining failed\n");
+	{
+		/* DIAGNOSTIC ONLY -- not for merging */
+		u16 sta = 0, ctl2 = 0;
+
+		pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &sta);
+		pcie_capability_read_word(dev, PCI_EXP_LNKCTL2, &ctl2);
+		pci_info(dev, "diag: pre ret=%d DLLLA=%d sta=%#06x ctl2=%#06x\n",
+			 ret, !!(sta & PCI_EXP_LNKSTA_DLLLA), sta, ctl2);
+	}
 	pcie_set_target_speed(dev, PCIE_LNKCTL2_TLS2SPEED(oldlnkctl2), true);
+	{
+		/* DIAGNOSTIC ONLY -- not for merging */
+		u16 sta = 0, ctl2 = 0;
+		int i;
+
+		for (i = 0; i <= 100; i++) {
+			pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &sta);
+			if (sta & PCI_EXP_LNKSTA_DLLLA)
+				break;
+			msleep(10);
+		}
+		pcie_capability_read_word(dev, PCI_EXP_LNKCTL2, &ctl2);
+		pci_info(dev, "diag: post DLLLA=%d after %dms sta=%#06x\n",
+			 !!(sta & PCI_EXP_LNKSTA_DLLLA), i * 10, sta);
+		pci_info(dev, "diag: post ctl2=%#06x\n", ctl2);
+	}
 	return ret;
 }


Expected output:

- "diag: pre" gives the error the retrain returned and whether the
  link was already down at that point.
- "diag: post DLLLA=1 after <n>ms" would mean the link does come back,
  just not before pci_device_add() returns and the bus below is
  scanned.  That would point at the error path needing to wait for
  DLLLA rather than only for the Link Training bit to clear.
- "diag: post DLLLA=0 after 1000ms" would mean the link is genuinely
  down and staying down, which is a different problem and
  probably a worse one.

One caveat: the poll loop waits up to a second in the error path, so it
changes timing.  If the diagnostic build happens to boot reliably where
the plain v3 build did not, that is also worth reporting.

Best regards,
Andreas Wild