From c83c9587752a46d2ffdf5eb188cd98df346eaa42 Mon Sep 17 00:00:00 2001 From: Werner Zeh Date: Mon, 27 Feb 2023 07:08:59 +0100 Subject: [PATCH] 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) --- src/device/pciexp_device.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) 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;