Patrick Georgi merged this change.

View Change

Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved Subrata Banik: Looks good to me, approved
soc/intel/{cannonlake,icelake}: Do not define PCH_DEV_PMC in ramstage

This change intentionally removes the definition of PCH_DEV_PMC from
ramstage to avoid silent errors. This device gets hidden from PCI bus
in FSP-S and hence dropped from the root bus by the resource
allocator. In order to avoid incorrect references to the device, avoid
defining it in ramstage where it known to return NULL.

BUG=b:136861224

Change-Id: I4f69470ec80c7127a2b604ed2b1f794f5a63e126
Signed-off-by: Furquan Shaikh <furquan@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/34120
Reviewed-by: Subrata Banik <subrata.banik@intel.com>
Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
---
M src/soc/intel/cannonlake/include/soc/pci_devs.h
M src/soc/intel/icelake/include/soc/pci_devs.h
2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/src/soc/intel/cannonlake/include/soc/pci_devs.h b/src/soc/intel/cannonlake/include/soc/pci_devs.h
index 46bc1bf..1bc028f 100644
--- a/src/soc/intel/cannonlake/include/soc/pci_devs.h
+++ b/src/soc/intel/cannonlake/include/soc/pci_devs.h
@@ -187,7 +187,19 @@
#define PCH_DEVFN_TRACEHUB _PCH_DEVFN(LPC, 7)
#define PCH_DEV_LPC _PCH_DEV(LPC, 0)
#define PCH_DEV_P2SB _PCH_DEV(LPC, 1)
+
+#if !ENV_RAMSTAGE
+/*
+ * PCH_DEV_PMC is intentionally not defined in RAMSTAGE since PMC device gets
+ * hidden from PCI bus after call to FSP-S. This leads to resource allocator
+ * dropping it from the root bus as unused device. All references to PCH_DEV_PMC
+ * would then return NULL and can go unnoticed if not handled properly. Since,
+ * this device does not have any special chip config associated with it, it is
+ * okay to not provide the definition for it in ramstage.
+ */
#define PCH_DEV_PMC _PCH_DEV(LPC, 2)
+#endif
+
#define PCH_DEV_HDA _PCH_DEV(LPC, 3)
#define PCH_DEV_SMBUS _PCH_DEV(LPC, 4)
#define PCH_DEV_SPI _PCH_DEV(LPC, 5)
diff --git a/src/soc/intel/icelake/include/soc/pci_devs.h b/src/soc/intel/icelake/include/soc/pci_devs.h
index b242113..1348f23b 100644
--- a/src/soc/intel/icelake/include/soc/pci_devs.h
+++ b/src/soc/intel/icelake/include/soc/pci_devs.h
@@ -184,7 +184,19 @@
#define PCH_DEV_ESPI _PCH_DEV(ESPI, 0)
#define PCH_DEV_LPC PCH_DEV_ESPI
#define PCH_DEV_P2SB _PCH_DEV(ESPI, 1)
+
+#if !ENV_RAMSTAGE
+/*
+ * PCH_DEV_PMC is intentionally not defined in RAMSTAGE since PMC device gets
+ * hidden from PCI bus after call to FSP-S. This leads to resource allocator
+ * dropping it from the root bus as unused device. All references to PCH_DEV_PMC
+ * would then return NULL and can go unnoticed if not handled properly. Since,
+ * this device does not have any special chip config associated with it, it is
+ * okay to not provide the definition for it in ramstage.
+ */
#define PCH_DEV_PMC _PCH_DEV(ESPI, 2)
+#endif
+
#define PCH_DEV_HDA _PCH_DEV(ESPI, 3)
#define PCH_DEV_SMBUS _PCH_DEV(ESPI, 4)
#define PCH_DEV_SPI _PCH_DEV(ESPI, 5)

To view, visit change 34120. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I4f69470ec80c7127a2b604ed2b1f794f5a63e126
Gerrit-Change-Number: 34120
Gerrit-PatchSet: 2
Gerrit-Owner: Furquan Shaikh <furquan@google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org>
Gerrit-Reviewer: Duncan Laurie <dlaurie@chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan@google.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki@gmail.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: Paul Fagerburg <pfagerburg@chromium.org>
Gerrit-Reviewer: Rizwan Qureshi <rizwan.qureshi@intel.com>
Gerrit-Reviewer: Shelley Chen <shchen@google.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak@chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-MessageType: merged