Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34120 )
Change subject: soc/intel/{cannonlake,icelake}: Do not define PCH_DEV_PMC in ramstage ......................................................................
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(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved Subrata Banik: Looks good to me, approved
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)