device/pciexp_device.c: Do not enable common clock if already active

The Common Clock Configuration (CCC) is a PCIe feature for cases where
the upstream and downstream device of a link share the same reference
clock. After a change in this setting a link re-training is mandatory
to make it effective.

On recent Intel platforms (tested on Elkhart Lake) the FSP code which is
executed before coreboot performs the PCI scan already enumerates all
PCI buses for its internal uses. While this is done, all the PCI express
features of a link are configured, which includes CCC. If the link
supports common clock, FSP performs the link re-training already. When the
execution flow is returned to coreboot, the same link treatment is
applied again (coded in 'pciexp_tune_dev()') and CCC is enabled a second
time, just a few milliseconds after FSP did this already.

Because enabling CCC requires a link re-training, there are two link
re-trainings on the PCIe link within a few milliseconds (one from the FSP
code and one from coreboot) which can lead to issues with a connected
PCIe device on this link. In particular, link issues were discovered
with a Pericom PCIe switch (PI7C9X2G608) on mc_ehl1 where the link has
stalled for a while after the second re-training. This in turn leads to
non-initialized PCI devices on the bus after coreboot has finished.

This patch checks if CCC is already enabled on a link and does not
perform the steps to enable it again in coreboot which safes a link
re-training (and thus execution time) and a potential link stability
issue.

Test=Check log output on mc_ehl1 which shows the following lines:

[DEBUG]  PCI: pci_scan_bus for bus 09
[DEBUG]  PCI: 09:00.0 [8086/1533] enabled
[INFO ]  PCIe: Common Clock Configuration already enabled

Change-Id: I747fa406a120a215de189d7252f160c8ea2e3716
Signed-off-by: Werner Zeh <werner.zeh@siemens.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73310
Reviewed-by: Subrata Banik <subratabanik@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>

1 files changed, 18 insertions, 0 deletions

diff --git a/src/device/pciexp_device.c b/src/device/pciexp_device.c
index c9441fb22e..4ec6ef0f0f 100644
--- a/src/device/pciexp_device.c
+++ b/src/device/pciexp_device.c
@@ -188,6 +188,20 @@ static int pciexp_retrain_link(struct device *dev, unsigned int cap)
 	return -1;
 }
 
+static bool pciexp_is_ccc_active(struct device *root, unsigned int root_cap,
+				 struct device *endp, unsigned int endp_cap)
+{
+	u16 root_ccc, endp_ccc;
+
+	root_ccc = pci_read_config16(root, root_cap + PCI_EXP_LNKCTL) & PCI_EXP_LNKCTL_CCC;
+	endp_ccc = pci_read_config16(endp, endp_cap + PCI_EXP_LNKCTL) & PCI_EXP_LNKCTL_CCC;
+	if (root_ccc && endp_ccc) {
+		printk(BIOS_INFO, "PCIe: Common Clock Configuration already enabled\n");
+		return true;
+	}
+	return false;
+}
+
 /*
  * Check the Slot Clock Configuration for root port and endpoint
  * and enable Common Clock Configuration if possible.  If CCC is
@@ -198,6 +212,10 @@ static void pciexp_enable_common_clock(struct device *root, unsigned int root_ca
 {
 	u16 root_scc, endp_scc, lnkctl;
 
+	/* No need to enable common clock if it is already active. */
+	if (pciexp_is_ccc_active(root, root_cap, endp, endp_cap))
+		return;
+
 	/* Get Slot Clock Configuration for root port */
 	root_scc = pci_read_config16(root, root_cap + PCI_EXP_LNKSTA);
 	root_scc &= PCI_EXP_LNKSTA_SLC;