[M] Change in coreboot[master]: device/pciexp_device.c: Do not enable common clock if already active

[Felix Held (Code Review)]

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/73310 )

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

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 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/73310
Reviewed-by: Subrata Banik 
Tested-by: build bot (Jenkins) 
---
M src/device/pciexp_device.c
1 file changed, 64 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Subrata Banik: Looks good to me, approved

diff --git a/src/device/pciexp_device.c b/src/device/pciexp_device.c
index c9441fb..4ec6ef0 100644
--- a/src/device/pciexp_device.c
+++ b/src/device/pciexp_device.c
@@ -188,6 +188,20 @@
    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 @@
 {
    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;