[PATCH v2] pci: Do not enable PCIe GEN3 link retrain workaround by default
Pali Rohár
pali at kernel.org
Sat Aug 27 14:30:05 CEST 2022
PCIe GEN3 link retrain workaround, specially designed for system with PCIe
ASMedia ASM2824 Switch and other Endpoint devices, unconditionally increase
size of all SPL binaries with PCIe support, even those which do not require it.
Moreover this workaround is enabled for all existing hardware and also all
future PCIe hardware, which opens a hole that other PCIe vendors may
introduce same HW issue as on systems where this workaround is required and
nobody would notice it because U-Boot automatically apply workaround for it.
So do not unconditionally enable this workaround on all hardware. Instead
introduce a new config option which is disabled by default. This decrease
SPL size and also ensure that workaround is not blindly or inexactly
enabled.
To make workaround code local, move it into separate file, so pci_auto.c
contains only generic auto configuration code.
Signed-off-by: Pali Rohár <pali at kernel.org>
---
Changes in v2:
* Rename config option to CONFIG_PCIE_GEN3_RETRAIN_WORKAROUND as it is
generic workaround and does not have to be needed only for ASMedia switch
(moreover it looks like issue is triggered by combination of ASMedia
switch with other endpoint PCIe device)
* Rephrase Kconfig help text as suggected by Stefan
---
drivers/pci/Kconfig | 9 +
drivers/pci/Makefile | 1 +
drivers/pci/pci_auto.c | 171 +------------------
drivers/pci/pcie_gen3_retrain_workaround.c | 183 +++++++++++++++++++++
4 files changed, 197 insertions(+), 167 deletions(-)
create mode 100644 drivers/pci/pcie_gen3_retrain_workaround.c
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 22f4995453ed..f56c8e89141c 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -365,4 +365,13 @@ config PCIE_UNIPHIER
Say Y here if you want to enable PCIe controller support on
UniPhier SoCs.
+config PCIE_GEN3_RETRAIN_WORKAROUND
+ bool "PCIe GEN3 retrain workaround"
+ help
+ Say Y here if you want to enable link re-training upon failures
+ for all PCIe Root Ports, PCIe Switch Downstream Ports and also
+ for PCIe to PCI/PCI-X Bridges. This seems to be helpful on the
+ ASMedia ASM2824 PCIe Switch.
+ Compliant PCIe hierarchy does not need this workaround.
+
endif
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index cfcd6fd6c52f..57587205d198 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -49,3 +49,4 @@ obj-$(CONFIG_PCI_OCTEONTX) += pci_octeontx.o
obj-$(CONFIG_PCIE_OCTEON) += pcie_octeon.o
obj-$(CONFIG_PCIE_DW_SIFIVE) += pcie_dw_sifive.o
obj-$(CONFIG_PCIE_UNIPHIER) += pcie_uniphier.o
+obj-$(CONFIG_PCIE_GEN3_RETRAIN_WORKAROUND) += pcie_gen3_retrain_workaround.o
diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
index c7968926a17f..4583ff8b384e 100644
--- a/drivers/pci/pci_auto.c
+++ b/drivers/pci/pci_auto.c
@@ -13,7 +13,6 @@
#include <errno.h>
#include <log.h>
#include <pci.h>
-#include <time.h>
#include "pci_internal.h"
/* the user can define CONFIG_SYS_PCI_CACHE_LINE_SIZE to avoid problems */
@@ -182,167 +181,7 @@ static void dm_pciauto_setup_device(struct udevice *dev,
dm_pci_write_config8(dev, PCI_LATENCY_TIMER, 0x80);
}
-/*
- * Check if the link of a downstream PCIe port operates correctly.
- *
- * For that check if the optional Data Link Layer Link Active status gets
- * on within a 200ms period or failing that wait until the completion of
- * that period and check if link training has shown the completed status
- * continuously throughout the second half of that period.
- *
- * Observation with the ASMedia ASM2824 Gen 3 switch indicates it takes
- * 11-44ms to indicate the Data Link Layer Link Active status at 2.5GT/s,
- * though it may take a couple of link training iterations.
- */
-static bool dm_pciauto_exp_link_stable(struct udevice *dev, int pcie_off)
-{
- u64 loops = 0, trcount = 0, ntrcount = 0, flips = 0;
- bool dllla, lnktr, plnktr;
- u16 exp_lnksta;
- pci_dev_t bdf;
- u64 end;
-
- dm_pci_read_config16(dev, pcie_off + PCI_EXP_LNKSTA, &exp_lnksta);
- plnktr = !!(exp_lnksta & PCI_EXP_LNKSTA_LT);
-
- end = get_ticks() + usec_to_tick(200000);
- do {
- dm_pci_read_config16(dev, pcie_off + PCI_EXP_LNKSTA,
- &exp_lnksta);
- dllla = !!(exp_lnksta & PCI_EXP_LNKSTA_DLLLA);
- lnktr = !!(exp_lnksta & PCI_EXP_LNKSTA_LT);
-
- flips += plnktr ^ lnktr;
- if (lnktr) {
- ntrcount = 0;
- trcount++;
- } else {
- ntrcount++;
- }
- loops++;
-
- plnktr = lnktr;
- } while (!dllla && get_ticks() < end);
-
- bdf = dm_pci_get_bdf(dev);
- debug("PCI Autoconfig: %02x.%02x.%02x: Fixup link: DL active: %u; "
- "%3llu flips, %6llu loops of which %6llu while training, "
- "final %6llu stable\n",
- PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf),
- (unsigned int)dllla,
- (unsigned long long)flips, (unsigned long long)loops,
- (unsigned long long)trcount, (unsigned long long)ntrcount);
-
- return dllla || ntrcount >= loops / 2;
-}
-
-/*
- * Retrain the link of a downstream PCIe port by hand if necessary.
- *
- * This is needed at least where a downstream port of the ASMedia ASM2824
- * Gen 3 switch is wired to the upstream port of the Pericom PI7C9X2G304
- * Gen 2 switch, and observed with the Delock Riser Card PCI Express x1 >
- * 2 x PCIe x1 device, P/N 41433, plugged into the SiFive HiFive Unmatched
- * board.
- *
- * In such a configuration the switches are supposed to negotiate the link
- * speed of preferably 5.0GT/s, falling back to 2.5GT/s. However the link
- * continues switching between the two speeds indefinitely and the data
- * link layer never reaches the active state, with link training reported
- * repeatedly active ~84% of the time. Forcing the target link speed to
- * 2.5GT/s with the upstream ASM2824 device makes the two switches talk to
- * each other correctly however. And more interestingly retraining with a
- * higher target link speed afterwards lets the two successfully negotiate
- * 5.0GT/s.
- *
- * As this can potentially happen with any device and is cheap in the case
- * of correctly operating hardware, let's do it for all downstream ports,
- * for root complexes, PCIe switches and PCI/PCI-X to PCIe bridges.
- *
- * First check if automatic link training may have failed to complete, as
- * indicated by the optional Data Link Layer Link Active status being off
- * and the Link Bandwidth Management Status indicating that hardware has
- * changed the link speed or width in an attempt to correct unreliable
- * link operation. If this is the case, then check if the link operates
- * correctly by seeing whether it is being trained excessively. If it is,
- * then conclude the link is broken.
- *
- * In that case restrict the speed to 2.5GT/s, observing that the Target
- * Link Speed field is sticky and therefore the link will stay restricted
- * even after a device reset is later made by an OS that is unaware of the
- * problem. With the speed restricted request that the link be retrained
- * and check again if the link operates correctly. If not, then set the
- * Target Link Speed back to the original value.
- *
- * This requires the presence of the Link Control 2 register, so make sure
- * the PCI Express Capability Version is at least 2. Also don't try, for
- * obvious reasons, to limit the speed if 2.5GT/s is the only link speed
- * supported.
- */
-static void dm_pciauto_exp_fixup_link(struct udevice *dev, int pcie_off)
-{
- u16 exp_lnksta, exp_lnkctl, exp_lnkctl2;
- u16 exp_flags, exp_type, exp_version;
- u32 exp_lnkcap;
- pci_dev_t bdf;
-
- dm_pci_read_config16(dev, pcie_off + PCI_EXP_FLAGS, &exp_flags);
- exp_version = exp_flags & PCI_EXP_FLAGS_VERS;
- if (exp_version < 2)
- return;
-
- exp_type = (exp_flags & PCI_EXP_FLAGS_TYPE) >> 4;
- switch (exp_type) {
- case PCI_EXP_TYPE_ROOT_PORT:
- case PCI_EXP_TYPE_DOWNSTREAM:
- case PCI_EXP_TYPE_PCIE_BRIDGE:
- break;
- default:
- return;
- }
-
- dm_pci_read_config32(dev, pcie_off + PCI_EXP_LNKCAP, &exp_lnkcap);
- if ((exp_lnkcap & PCI_EXP_LNKCAP_SLS) <= PCI_EXP_LNKCAP_SLS_2_5GB)
- return;
-
- dm_pci_read_config16(dev, pcie_off + PCI_EXP_LNKSTA, &exp_lnksta);
- if ((exp_lnksta & (PCI_EXP_LNKSTA_LBMS | PCI_EXP_LNKSTA_DLLLA)) !=
- PCI_EXP_LNKSTA_LBMS)
- return;
-
- if (dm_pciauto_exp_link_stable(dev, pcie_off))
- return;
-
- bdf = dm_pci_get_bdf(dev);
- printf("PCI Autoconfig: %02x.%02x.%02x: "
- "Downstream link non-functional\n",
- PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
- printf("PCI Autoconfig: %02x.%02x.%02x: "
- "Retrying with speed restricted to 2.5GT/s...\n",
- PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
-
- dm_pci_read_config16(dev, pcie_off + PCI_EXP_LNKCTL, &exp_lnkctl);
- dm_pci_read_config16(dev, pcie_off + PCI_EXP_LNKCTL2, &exp_lnkctl2);
-
- dm_pci_write_config16(dev, pcie_off + PCI_EXP_LNKCTL2,
- (exp_lnkctl2 & ~PCI_EXP_LNKCTL2_TLS) |
- PCI_EXP_LNKCTL2_TLS_2_5GT);
- dm_pci_write_config16(dev, pcie_off + PCI_EXP_LNKCTL,
- exp_lnkctl | PCI_EXP_LNKCTL_RL);
-
- if (dm_pciauto_exp_link_stable(dev, pcie_off)) {
- printf("PCI Autoconfig: %02x.%02x.%02x: Succeeded!\n",
- PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
- } else {
- printf("PCI Autoconfig: %02x.%02x.%02x: Failed!\n",
- PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
-
- dm_pci_write_config16(dev, pcie_off + PCI_EXP_LNKCTL2,
- exp_lnkctl2);
- dm_pci_write_config16(dev, pcie_off + PCI_EXP_LNKCTL,
- exp_lnkctl | PCI_EXP_LNKCTL_RL);
- }
-}
+void pcie_gen3_retrain_workaround(struct udevice *dev);
void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus)
{
@@ -353,7 +192,6 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus)
u8 io_32;
struct udevice *ctlr = pci_get_controller(dev);
struct pci_controller *ctlr_hose = dev_get_uclass_priv(ctlr);
- int pcie_off;
pci_mem = ctlr_hose->pci_mem;
pci_prefetch = ctlr_hose->pci_prefetch;
@@ -440,10 +278,9 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, int sub_bus)
}
}
- /* For PCIe devices see if we need to retrain the link by hand */
- pcie_off = dm_pci_find_capability(dev, PCI_CAP_ID_EXP);
- if (pcie_off)
- dm_pciauto_exp_fixup_link(dev, pcie_off);
+#ifdef CONFIG_PCIE_GEN3_RETRAIN_WORKAROUND
+ pcie_gen3_retrain_workaround(dev);
+#endif
/* Enable memory and I/O accesses, enable bus master */
dm_pci_write_config16(dev, PCI_COMMAND, cmdstat | PCI_COMMAND_MASTER);
diff --git a/drivers/pci/pcie_gen3_retrain_workaround.c b/drivers/pci/pcie_gen3_retrain_workaround.c
new file mode 100644
index 000000000000..e4fe3368652d
--- /dev/null
+++ b/drivers/pci/pcie_gen3_retrain_workaround.c
@@ -0,0 +1,183 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * PCIe GEN3 retrain workaround
+ *
+ * Copyright (c) 2021 Maciej W. Rozycki <macro at orcam.me.uk>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <pci.h>
+#include <time.h>
+
+/*
+ * Check if the link of a downstream PCIe port operates correctly.
+ *
+ * For that check if the optional Data Link Layer Link Active status gets
+ * on within a 200ms period or failing that wait until the completion of
+ * that period and check if link training has shown the completed status
+ * continuously throughout the second half of that period.
+ *
+ * Observation with the ASMedia ASM2824 Gen 3 switch indicates it takes
+ * 11-44ms to indicate the Data Link Layer Link Active status at 2.5GT/s,
+ * though it may take a couple of link training iterations.
+ */
+static bool dm_pciauto_exp_link_stable(struct udevice *dev, int pcie_off)
+{
+ u64 loops = 0, trcount = 0, ntrcount = 0, flips = 0;
+ bool dllla, lnktr, plnktr;
+ u16 exp_lnksta;
+ pci_dev_t bdf;
+ u64 end;
+
+ dm_pci_read_config16(dev, pcie_off + PCI_EXP_LNKSTA, &exp_lnksta);
+ plnktr = !!(exp_lnksta & PCI_EXP_LNKSTA_LT);
+
+ end = get_ticks() + usec_to_tick(200000);
+ do {
+ dm_pci_read_config16(dev, pcie_off + PCI_EXP_LNKSTA,
+ &exp_lnksta);
+ dllla = !!(exp_lnksta & PCI_EXP_LNKSTA_DLLLA);
+ lnktr = !!(exp_lnksta & PCI_EXP_LNKSTA_LT);
+
+ flips += plnktr ^ lnktr;
+ if (lnktr) {
+ ntrcount = 0;
+ trcount++;
+ } else {
+ ntrcount++;
+ }
+ loops++;
+
+ plnktr = lnktr;
+ } while (!dllla && get_ticks() < end);
+
+ bdf = dm_pci_get_bdf(dev);
+ debug("PCI Autoconfig: %02x.%02x.%02x: Fixup link: DL active: %u; "
+ "%3llu flips, %6llu loops of which %6llu while training, "
+ "final %6llu stable\n",
+ PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf),
+ (unsigned int)dllla,
+ (unsigned long long)flips, (unsigned long long)loops,
+ (unsigned long long)trcount, (unsigned long long)ntrcount);
+
+ return dllla || ntrcount >= loops / 2;
+}
+
+/*
+ * Retrain the link of a downstream PCIe port by hand if necessary.
+ *
+ * This is needed at least where a downstream port of the ASMedia ASM2824
+ * Gen 3 switch is wired to the upstream port of the Pericom PI7C9X2G304
+ * Gen 2 switch, and observed with the Delock Riser Card PCI Express x1 >
+ * 2 x PCIe x1 device, P/N 41433, plugged into the SiFive HiFive Unmatched
+ * board.
+ *
+ * In such a configuration the switches are supposed to negotiate the link
+ * speed of preferably 5.0GT/s, falling back to 2.5GT/s. However the link
+ * continues switching between the two speeds indefinitely and the data
+ * link layer never reaches the active state, with link training reported
+ * repeatedly active ~84% of the time. Forcing the target link speed to
+ * 2.5GT/s with the upstream ASM2824 device makes the two switches talk to
+ * each other correctly however. And more interestingly retraining with a
+ * higher target link speed afterwards lets the two successfully negotiate
+ * 5.0GT/s.
+ *
+ * As this can potentially happen with any device and is cheap in the case
+ * of correctly operating hardware, let's do it for all downstream ports,
+ * for root complexes, PCIe switches and PCI/PCI-X to PCIe bridges.
+ *
+ * First check if automatic link training may have failed to complete, as
+ * indicated by the optional Data Link Layer Link Active status being off
+ * and the Link Bandwidth Management Status indicating that hardware has
+ * changed the link speed or width in an attempt to correct unreliable
+ * link operation. If this is the case, then check if the link operates
+ * correctly by seeing whether it is being trained excessively. If it is,
+ * then conclude the link is broken.
+ *
+ * In that case restrict the speed to 2.5GT/s, observing that the Target
+ * Link Speed field is sticky and therefore the link will stay restricted
+ * even after a device reset is later made by an OS that is unaware of the
+ * problem. With the speed restricted request that the link be retrained
+ * and check again if the link operates correctly. If not, then set the
+ * Target Link Speed back to the original value.
+ *
+ * This requires the presence of the Link Control 2 register, so make sure
+ * the PCI Express Capability Version is at least 2. Also don't try, for
+ * obvious reasons, to limit the speed if 2.5GT/s is the only link speed
+ * supported.
+ */
+static void dm_pciauto_exp_fixup_link(struct udevice *dev, int pcie_off)
+{
+ u16 exp_lnksta, exp_lnkctl, exp_lnkctl2;
+ u16 exp_flags, exp_type, exp_version;
+ u32 exp_lnkcap;
+ pci_dev_t bdf;
+
+ dm_pci_read_config16(dev, pcie_off + PCI_EXP_FLAGS, &exp_flags);
+ exp_version = exp_flags & PCI_EXP_FLAGS_VERS;
+ if (exp_version < 2)
+ return;
+
+ exp_type = (exp_flags & PCI_EXP_FLAGS_TYPE) >> 4;
+ switch (exp_type) {
+ case PCI_EXP_TYPE_ROOT_PORT:
+ case PCI_EXP_TYPE_DOWNSTREAM:
+ case PCI_EXP_TYPE_PCIE_BRIDGE:
+ break;
+ default:
+ return;
+ }
+
+ dm_pci_read_config32(dev, pcie_off + PCI_EXP_LNKCAP, &exp_lnkcap);
+ if ((exp_lnkcap & PCI_EXP_LNKCAP_SLS) <= PCI_EXP_LNKCAP_SLS_2_5GB)
+ return;
+
+ dm_pci_read_config16(dev, pcie_off + PCI_EXP_LNKSTA, &exp_lnksta);
+ if ((exp_lnksta & (PCI_EXP_LNKSTA_LBMS | PCI_EXP_LNKSTA_DLLLA)) !=
+ PCI_EXP_LNKSTA_LBMS)
+ return;
+
+ if (dm_pciauto_exp_link_stable(dev, pcie_off))
+ return;
+
+ bdf = dm_pci_get_bdf(dev);
+ printf("PCI Autoconfig: %02x.%02x.%02x: "
+ "Downstream link non-functional\n",
+ PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
+ printf("PCI Autoconfig: %02x.%02x.%02x: "
+ "Retrying with speed restricted to 2.5GT/s...\n",
+ PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
+
+ dm_pci_read_config16(dev, pcie_off + PCI_EXP_LNKCTL, &exp_lnkctl);
+ dm_pci_read_config16(dev, pcie_off + PCI_EXP_LNKCTL2, &exp_lnkctl2);
+
+ dm_pci_write_config16(dev, pcie_off + PCI_EXP_LNKCTL2,
+ (exp_lnkctl2 & ~PCI_EXP_LNKCTL2_TLS) |
+ PCI_EXP_LNKCTL2_TLS_2_5GT);
+ dm_pci_write_config16(dev, pcie_off + PCI_EXP_LNKCTL,
+ exp_lnkctl | PCI_EXP_LNKCTL_RL);
+
+ if (dm_pciauto_exp_link_stable(dev, pcie_off)) {
+ printf("PCI Autoconfig: %02x.%02x.%02x: Succeeded!\n",
+ PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
+ } else {
+ printf("PCI Autoconfig: %02x.%02x.%02x: Failed!\n",
+ PCI_BUS(bdf), PCI_DEV(bdf), PCI_FUNC(bdf));
+
+ dm_pci_write_config16(dev, pcie_off + PCI_EXP_LNKCTL2,
+ exp_lnkctl2);
+ dm_pci_write_config16(dev, pcie_off + PCI_EXP_LNKCTL,
+ exp_lnkctl | PCI_EXP_LNKCTL_RL);
+ }
+}
+
+void pcie_gen3_retrain_workaround(struct udevice *dev)
+{
+ int pcie_off;
+
+ /* For PCIe devices see if we need to retrain the link by hand */
+ pcie_off = dm_pci_find_capability(dev, PCI_CAP_ID_EXP);
+ if (pcie_off)
+ dm_pciauto_exp_fixup_link(dev, pcie_off);
+}
--
2.20.1
More information about the U-Boot
mailing list