Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35088 )
Change subject: soc/intel: always use pcidev_path_on_root_debug() ......................................................................
soc/intel: always use pcidev_path_on_root_debug()
PCI devices love to play hide and seek on these chips for mysterious reasons. Therefore, always assume any device could just go missing, even the root bus.
BUG=cursed xyz
Change-Id: I2283554dfff4e9f2d89ec7bde9304a734a2546b5 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/apollolake/acpi.c M src/soc/intel/apollolake/chip.c M src/soc/intel/apollolake/romstage.c M src/soc/intel/apollolake/uart.c M src/soc/intel/broadwell/acpi.c M src/soc/intel/broadwell/finalize.c M src/soc/intel/broadwell/lpc.c M src/soc/intel/broadwell/smmrelocate.c M src/soc/intel/broadwell/systemagent.c M src/soc/intel/cannonlake/acpi.c M src/soc/intel/cannonlake/fsp_params.c M src/soc/intel/cannonlake/romstage/fsp_params.c M src/soc/intel/cannonlake/smmrelocate.c M src/soc/intel/cannonlake/uart.c M src/soc/intel/common/block/graphics/graphics.c M src/soc/intel/common/block/gspi/gspi.c M src/soc/intel/common/block/i2c/i2c.c M src/soc/intel/common/block/thermal/thermal.c M src/soc/intel/common/block/xhci/xhci.c M src/soc/intel/denverton_ns/memmap.c M src/soc/intel/fsp_baytrail/acpi.c M src/soc/intel/fsp_baytrail/fsp/chipset_fsp_util.c M src/soc/intel/fsp_broadwell_de/acpi.c M src/soc/intel/icelake/fsp_params.c M src/soc/intel/icelake/romstage/fsp_params.c M src/soc/intel/icelake/smmrelocate.c M src/soc/intel/icelake/uart.c M src/soc/intel/skylake/acpi.c M src/soc/intel/skylake/chip.c M src/soc/intel/skylake/chip_fsp20.c M src/soc/intel/skylake/irq.c M src/soc/intel/skylake/memmap.c M src/soc/intel/skylake/romstage/romstage_fsp20.c M src/soc/intel/skylake/romstage/systemagent.c M src/soc/intel/skylake/smmrelocate.c M src/soc/intel/skylake/systemagent.c M src/soc/intel/skylake/uart.c M src/soc/intel/skylake/vr_config.c 38 files changed, 87 insertions(+), 87 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/35088/1
diff --git a/src/soc/intel/apollolake/acpi.c b/src/soc/intel/apollolake/acpi.c index f729f31..785b253 100644 --- a/src/soc/intel/apollolake/acpi.c +++ b/src/soc/intel/apollolake/acpi.c @@ -175,7 +175,7 @@
static unsigned long soc_fill_dmar(unsigned long current) { - struct device *const igfx_dev = pcidev_path_on_root(SA_DEVFN_IGD); + struct device *const igfx_dev = pcidev_path_on_root_debug(SA_DEVFN_IGD); uint64_t gfxvtbar = MCHBAR64(GFXVTBAR) & VTBAR_MASK; uint64_t defvtbar = MCHBAR64(DEFVTBAR) & VTBAR_MASK; bool gfxvten = MCHBAR32(GFXVTBAR) & VTBAR_ENABLED; @@ -208,7 +208,7 @@ * get the info and hide it again when done. */ p2sb_unhide(); - struct device *p2sb_dev = pcidev_path_on_root(PCH_DEVFN_P2SB); + struct device *p2sb_dev = pcidev_path_on_root_debug(PCH_DEVFN_P2SB); uint16_t ibdf = pci_read_config16(p2sb_dev, PCH_P2SB_IBDF); uint16_t hbdf = pci_read_config16(p2sb_dev, PCH_P2SB_HBDF); p2sb_hide(); diff --git a/src/soc/intel/apollolake/chip.c b/src/soc/intel/apollolake/chip.c index b69f9ee..0440267 100644 --- a/src/soc/intel/apollolake/chip.c +++ b/src/soc/intel/apollolake/chip.c @@ -256,7 +256,7 @@ int i; unsigned int inc = PCI_DEVFN(0, 1);
- func0 = pcidev_path_on_root(devfn0); + func0 = pcidev_path_on_root_debug(devfn0); if (func0 == NULL) return;
@@ -272,7 +272,7 @@ * as that port was move to func0. */ for (i = 1; i < num_funcs; i++, devfn += inc) { - struct device *dev = pcidev_path_on_root(devfn); + struct device *dev = pcidev_path_on_root_debug(devfn); if (dev == NULL) continue;
@@ -538,7 +538,7 @@
static void parse_devicetree(FSP_S_CONFIG *silconfig) { - struct device *dev = pcidev_path_on_root(SA_DEVFN_ROOT); + struct device *dev = pcidev_path_on_root_debug(SA_DEVFN_ROOT);
if (!dev) { printk(BIOS_ERR, "Could not find root device\n"); @@ -672,7 +672,7 @@ /* Load VBT before devicetree-specific config. */ silconfig->GraphicsConfigPtr = (uintptr_t)vbt_get();
- dev = pcidev_path_on_root(SA_DEVFN_ROOT); + dev = pcidev_path_on_root_debug(SA_DEVFN_ROOT); cfg = config_of(dev);
mainboard_devtree_update(dev); @@ -743,7 +743,7 @@ apl_fsp_silicon_init_params_cb(cfg, silconfig);
/* Enable xDCI controller if enabled in devicetree and allowed */ - dev = pcidev_path_on_root(PCH_DEVFN_XDCI); + dev = pcidev_path_on_root_debug(PCH_DEVFN_XDCI); if (!xdci_can_enable()) dev->enabled = 0; silconfig->UsbOtg = dev->enabled; diff --git a/src/soc/intel/apollolake/romstage.c b/src/soc/intel/apollolake/romstage.c index 1464d2c..ae67582 100644 --- a/src/soc/intel/apollolake/romstage.c +++ b/src/soc/intel/apollolake/romstage.c @@ -332,7 +332,7 @@ static void parse_devicetree_setting(FSPM_UPD *m_upd) { #if CONFIG(SOC_INTEL_GLK) - DEVTREE_CONST struct device *dev = pcidev_path_on_root(PCH_DEVFN_NPK); + DEVTREE_CONST struct device *dev = pcidev_path_on_root_debug(PCH_DEVFN_NPK); if (!dev) return;
diff --git a/src/soc/intel/apollolake/uart.c b/src/soc/intel/apollolake/uart.c index f8c4aaf..da61a3f 100644 --- a/src/soc/intel/apollolake/uart.c +++ b/src/soc/intel/apollolake/uart.c @@ -82,13 +82,13 @@ */ switch (uart_console) { case 0: - return pcidev_path_on_root(PCH_DEVFN_UART0); + return pcidev_path_on_root_debug(PCH_DEVFN_UART0); case 1: - return pcidev_path_on_root(PCH_DEVFN_UART1); + return pcidev_path_on_root_debug(PCH_DEVFN_UART1); case 2: - return pcidev_path_on_root(PCH_DEVFN_UART2); + return pcidev_path_on_root_debug(PCH_DEVFN_UART2); case 3: - return pcidev_path_on_root(PCH_DEVFN_UART3); + return pcidev_path_on_root_debug(PCH_DEVFN_UART3); default: printk(BIOS_ERR, "Invalid UART console index\n"); return NULL; diff --git a/src/soc/intel/broadwell/acpi.c b/src/soc/intel/broadwell/acpi.c index 705bc00..5e5460c 100644 --- a/src/soc/intel/broadwell/acpi.c +++ b/src/soc/intel/broadwell/acpi.c @@ -580,7 +580,7 @@
static unsigned long acpi_fill_dmar(unsigned long current) { - struct device *const igfx_dev = pcidev_path_on_root(SA_DEVFN_IGD); + struct device *const igfx_dev = pcidev_path_on_root_debug(SA_DEVFN_IGD); const u32 gfxvtbar = MCHBAR32(GFXVTBAR) & ~0xfff; const u32 vtvc0bar = MCHBAR32(VTVC0BAR) & ~0xfff; const bool gfxvten = MCHBAR32(GFXVTBAR) & 0x1; diff --git a/src/soc/intel/broadwell/finalize.c b/src/soc/intel/broadwell/finalize.c index 06cc18d..85a0e74 100644 --- a/src/soc/intel/broadwell/finalize.c +++ b/src/soc/intel/broadwell/finalize.c @@ -96,7 +96,7 @@
static void broadwell_finalize(void *unused) { - struct device *sa_dev = pcidev_path_on_root(SA_DEVFN_ROOT); + struct device *sa_dev = pcidev_path_on_root_debug(SA_DEVFN_ROOT);
printk(BIOS_DEBUG, "Finalizing chipset.\n");
diff --git a/src/soc/intel/broadwell/lpc.c b/src/soc/intel/broadwell/lpc.c index b385d6b..1406c28 100644 --- a/src/soc/intel/broadwell/lpc.c +++ b/src/soc/intel/broadwell/lpc.c @@ -365,7 +365,7 @@ { u32 reg32; u16 reg16; - struct device *igd_dev = pcidev_path_on_root(SA_DEVFN_IGD); + struct device *igd_dev = pcidev_path_on_root_debug(SA_DEVFN_IGD);
/* DMI */ RCBA32_OR(0x2234, 0xf); diff --git a/src/soc/intel/broadwell/smmrelocate.c b/src/soc/intel/broadwell/smmrelocate.c index 61b0c4c..364d98e 100644 --- a/src/soc/intel/broadwell/smmrelocate.c +++ b/src/soc/intel/broadwell/smmrelocate.c @@ -272,7 +272,7 @@ void smm_info(uintptr_t *perm_smbase, size_t *perm_smsize, size_t *smm_save_state_size) { - struct device *dev = pcidev_path_on_root(SA_DEVFN_ROOT); + struct device *dev = pcidev_path_on_root_debug(SA_DEVFN_ROOT);
printk(BIOS_DEBUG, "Setting up SMI for CPU\n");
@@ -321,7 +321,7 @@
void smm_lock(void) { - struct device *sa_dev = pcidev_path_on_root(SA_DEVFN_ROOT); + struct device *sa_dev = pcidev_path_on_root_debug(SA_DEVFN_ROOT); /* LOCK the SMM memory window and enable normal SMM. * After running this function, only a full reset can * make the SMM registers writable again. diff --git a/src/soc/intel/broadwell/systemagent.c b/src/soc/intel/broadwell/systemagent.c index b6b5608..1e32c5b 100644 --- a/src/soc/intel/broadwell/systemagent.c +++ b/src/soc/intel/broadwell/systemagent.c @@ -32,20 +32,20 @@
u8 systemagent_revision(void) { - struct device *sa_dev = pcidev_path_on_root(SA_DEVFN_ROOT); + struct device *sa_dev = pcidev_path_on_root_debug(SA_DEVFN_ROOT); return pci_read_config8(sa_dev, PCI_REVISION_ID); }
uintptr_t sa_get_tolud_base(void) { - struct device *sa_dev = pcidev_path_on_root(SA_DEVFN_ROOT); + struct device *sa_dev = pcidev_path_on_root_debug(SA_DEVFN_ROOT); /* Bit 0 is lock bit, not part of address */ return pci_read_config32(sa_dev, TOLUD) & ~1; }
uintptr_t sa_get_gsm_base(void) { - struct device *sa_dev = pcidev_path_on_root(SA_DEVFN_ROOT); + struct device *sa_dev = pcidev_path_on_root_debug(SA_DEVFN_ROOT); /* Bit 0 is lock bit, not part of address */ return pci_read_config32(sa_dev, BGSM) & ~1; } @@ -294,7 +294,7 @@ uint64_t mc_values[NUM_MAP_ENTRIES]; unsigned long dpr_size = 0; u32 dpr_reg; - struct device *sa_dev = pcidev_path_on_root(SA_DEVFN_ROOT); + struct device *sa_dev = pcidev_path_on_root_debug(SA_DEVFN_ROOT);
/* Read in the MAP registers and report their values. */ mc_read_map_entries(dev, &mc_values[0]); diff --git a/src/soc/intel/cannonlake/acpi.c b/src/soc/intel/cannonlake/acpi.c index 89770c0..30c1361 100644 --- a/src/soc/intel/cannonlake/acpi.c +++ b/src/soc/intel/cannonlake/acpi.c @@ -294,7 +294,7 @@
static unsigned long soc_fill_dmar(unsigned long current) { - struct device *const igfx_dev = pcidev_path_on_root(SA_DEVFN_IGD); + struct device *const igfx_dev = pcidev_path_on_root_debug(SA_DEVFN_IGD); uint64_t gfxvtbar = MCHBAR64(GFXVTBAR) & VTBAR_MASK; bool gfxvten = MCHBAR32(GFXVTBAR) & VTBAR_ENABLED;
@@ -307,7 +307,7 @@ acpi_dmar_drhd_fixup(tmp, current); }
- struct device *const ipu_dev = pcidev_path_on_root(SA_DEVFN_IPU); + struct device *const ipu_dev = pcidev_path_on_root_debug(SA_DEVFN_IPU); uint64_t ipuvtbar = MCHBAR64(IPUVTBAR) & VTBAR_MASK; bool ipuvten = MCHBAR32(IPUVTBAR) & VTBAR_ENABLED;
diff --git a/src/soc/intel/cannonlake/fsp_params.c b/src/soc/intel/cannonlake/fsp_params.c index 494c1db..b6a2df5 100644 --- a/src/soc/intel/cannonlake/fsp_params.c +++ b/src/soc/intel/cannonlake/fsp_params.c @@ -53,7 +53,7 @@ { struct device *dev;
- dev = pcidev_path_on_root(serial_io_dev[dev_offset]); + dev = pcidev_path_on_root_debug(serial_io_dev[dev_offset]); if (!dev || !dev->enabled) return PCH_SERIAL_IO_INDEX(PchSerialIoDisabled);
@@ -175,7 +175,7 @@ params->PchLockDownRtcMemoryLock = 0;
/* SATA */ - dev = pcidev_path_on_root(PCH_DEVFN_SATA); + dev = pcidev_path_on_root_debug(PCH_DEVFN_SATA); if (!dev) params->SataEnable = 0; else { @@ -189,7 +189,7 @@ }
/* Lan */ - dev = pcidev_path_on_root(PCH_DEVFN_GBE); + dev = pcidev_path_on_root_debug(PCH_DEVFN_GBE); if (!dev) params->PchLanEnable = 0; else { @@ -272,7 +272,7 @@ }
/* Enable xDCI controller if enabled in devicetree and allowed */ - dev = pcidev_path_on_root(PCH_DEVFN_USBOTG); + dev = pcidev_path_on_root_debug(PCH_DEVFN_USBOTG); if (dev) { if (!xdci_can_enable()) dev->enabled = 0; @@ -287,7 +287,7 @@ #endif
/* Enable CNVi Wifi if enabled in device tree */ - dev = pcidev_path_on_root(PCH_DEVFN_CNViWIFI); + dev = pcidev_path_on_root_debug(PCH_DEVFN_CNViWIFI); #if CONFIG(SOC_INTEL_COMETLAKE) if (dev) params->CnviMode = dev->enabled; @@ -314,7 +314,7 @@ sizeof(config->PcieRpHotPlug));
/* eMMC and SD */ - dev = pcidev_path_on_root(PCH_DEVFN_EMMC); + dev = pcidev_path_on_root_debug(PCH_DEVFN_EMMC); if (!dev) params->ScsEmmcEnabled = 0; else { @@ -329,7 +329,7 @@ } }
- dev = pcidev_path_on_root(PCH_DEVFN_SDCARD); + dev = pcidev_path_on_root_debug(PCH_DEVFN_SDCARD); if (!dev) { params->ScsSdCardEnabled = 0; } else { @@ -341,7 +341,7 @@ #endif }
- dev = pcidev_path_on_root(PCH_DEVFN_UFS); + dev = pcidev_path_on_root_debug(PCH_DEVFN_UFS); if (!dev) params->ScsUfsEnabled = 0; else diff --git a/src/soc/intel/cannonlake/romstage/fsp_params.c b/src/soc/intel/cannonlake/romstage/fsp_params.c index 3ba997d..6ed4b3c 100644 --- a/src/soc/intel/cannonlake/romstage/fsp_params.c +++ b/src/soc/intel/cannonlake/romstage/fsp_params.c @@ -30,7 +30,7 @@ { unsigned int i; uint32_t mask = 0; - const struct device *dev = pcidev_path_on_root(PCH_DEVFN_ISH); + const struct device *dev = pcidev_path_on_root_debug(PCH_DEVFN_ISH);
/* Set IGD stolen size to 64MB. */ m_cfg->IgdDvmt50PreAlloc = 2; @@ -85,7 +85,7 @@ m_cfg->PchIshEnable = dev->enabled;
/* If HDA is enabled, enable HDA elements */ - dev = pcidev_path_on_root(PCH_DEVFN_HDA); + dev = pcidev_path_on_root_debug(PCH_DEVFN_HDA); if (!dev) m_cfg->PchHdaEnable = 0; else @@ -93,15 +93,15 @@
/* Enable IPU only if the device is enabled */ m_cfg->SaIpuEnable = 0; - dev = pcidev_path_on_root(SA_DEVFN_IPU); + dev = pcidev_path_on_root_debug(SA_DEVFN_IPU); if (dev) m_cfg->SaIpuEnable = dev->enabled; }
void platform_fsp_memory_init_params_cb(FSPM_UPD *mupd, uint32_t version) { - const struct device *dev = pcidev_path_on_root(PCH_DEVFN_LPC); - const struct device *smbus = pcidev_path_on_root(PCH_DEVFN_SMBUS); + const struct device *dev = pcidev_path_on_root_debug(PCH_DEVFN_LPC); + const struct device *smbus = pcidev_path_on_root_debug(PCH_DEVFN_SMBUS); assert(dev != NULL); const config_t *config = config_of(dev); FSP_M_CONFIG *m_cfg = &mupd->FspmConfig; diff --git a/src/soc/intel/cannonlake/smmrelocate.c b/src/soc/intel/cannonlake/smmrelocate.c index 493d003..0b8b7e2 100644 --- a/src/soc/intel/cannonlake/smmrelocate.c +++ b/src/soc/intel/cannonlake/smmrelocate.c @@ -257,7 +257,7 @@
void smm_lock(void) { - struct device *sa_dev = pcidev_path_on_root(SA_DEVFN_ROOT); + struct device *sa_dev = pcidev_path_on_root_debug(SA_DEVFN_ROOT); /* * LOCK the SMM memory window and enable normal SMM. * After running this function, only a full reset can diff --git a/src/soc/intel/cannonlake/uart.c b/src/soc/intel/cannonlake/uart.c index ae19acc..2d7e1d1 100644 --- a/src/soc/intel/cannonlake/uart.c +++ b/src/soc/intel/cannonlake/uart.c @@ -58,11 +58,11 @@ */ switch (uart_console) { case 0: - return pcidev_path_on_root(PCH_DEVFN_UART0); + return pcidev_path_on_root_debug(PCH_DEVFN_UART0); case 1: - return pcidev_path_on_root(PCH_DEVFN_UART1); + return pcidev_path_on_root_debug(PCH_DEVFN_UART1); case 2: - return pcidev_path_on_root(PCH_DEVFN_UART2); + return pcidev_path_on_root_debug(PCH_DEVFN_UART2); default: printk(BIOS_ERR, "Invalid UART console index\n"); return NULL; diff --git a/src/soc/intel/common/block/graphics/graphics.c b/src/soc/intel/common/block/graphics/graphics.c index 2b4c4a7..b050320 100644 --- a/src/soc/intel/common/block/graphics/graphics.c +++ b/src/soc/intel/common/block/graphics/graphics.c @@ -56,7 +56,7 @@ uintptr_t graphics_get_memory_base(void) { uintptr_t memory_base; - struct device *dev = pcidev_path_on_root(SA_DEVFN_IGD); + struct device *dev = pcidev_path_on_root_debug(SA_DEVFN_IGD);
if (is_graphics_disabled(dev)) return 0; @@ -75,7 +75,7 @@ static uintptr_t graphics_get_gtt_base(void) { static uintptr_t gtt_base; - struct device *dev = pcidev_path_on_root(SA_DEVFN_IGD); + struct device *dev = pcidev_path_on_root_debug(SA_DEVFN_IGD);
if (is_graphics_disabled(dev)) die("IGD is disabled!"); diff --git a/src/soc/intel/common/block/gspi/gspi.c b/src/soc/intel/common/block/gspi/gspi.c index f937bd6..4224b22 100644 --- a/src/soc/intel/common/block/gspi/gspi.c +++ b/src/soc/intel/common/block/gspi/gspi.c @@ -236,7 +236,7 @@ if (devfn < 0) return 0;
- dev = pcidev_path_on_root(devfn); + dev = pcidev_path_on_root_debug(devfn); if (!dev || !dev->enabled) return 0;
@@ -474,7 +474,7 @@ * devfn is already validated as part of gspi_ctrlr_params_init. * No need to revalidate it again. */ - device = pcidev_path_on_root(devfn); + device = pcidev_path_on_root_debug(devfn);
/* Ensure controller is in D0 state */ lpss_set_power_state(device, STATE_D0); diff --git a/src/soc/intel/common/block/i2c/i2c.c b/src/soc/intel/common/block/i2c/i2c.c index 854a61a..3dbe6dd 100644 --- a/src/soc/intel/common/block/i2c/i2c.c +++ b/src/soc/intel/common/block/i2c/i2c.c @@ -65,7 +65,7 @@
/* Look up the controller device in the devicetree */ dev = PCI_DEV(0, PCI_SLOT(devfn), PCI_FUNC(devfn)); - tree_dev = pcidev_path_on_root(devfn); + tree_dev = pcidev_path_on_root_debug(devfn); if (!tree_dev || !tree_dev->enabled) { printk(BIOS_ERR, "I2C%u device not enabled\n", bus); return -1; @@ -137,7 +137,7 @@ return (uintptr_t)NULL;
/* devfn -> dev */ - dev = pcidev_path_on_root(devfn); + dev = pcidev_path_on_root_debug(devfn); if (!dev || !dev->enabled) return (uintptr_t)NULL;
diff --git a/src/soc/intel/common/block/thermal/thermal.c b/src/soc/intel/common/block/thermal/thermal.c index 39a98a4..9fe19a5 100644 --- a/src/soc/intel/common/block/thermal/thermal.c +++ b/src/soc/intel/common/block/thermal/thermal.c @@ -62,7 +62,7 @@ struct device *dev; struct resource *res;
- dev = pcidev_path_on_root(PCH_DEVFN_THERMAL); + dev = pcidev_path_on_root_debug(PCH_DEVFN_THERMAL); if (!dev) { printk(BIOS_ERR, "ERROR: PCH_DEVFN_THERMAL device not found!\n"); return; diff --git a/src/soc/intel/common/block/xhci/xhci.c b/src/soc/intel/common/block/xhci/xhci.c index 0bdf1d9..d0c1494 100644 --- a/src/soc/intel/common/block/xhci/xhci.c +++ b/src/soc/intel/common/block/xhci/xhci.c @@ -75,7 +75,7 @@ struct drivers_usb_acpi_config *config; bool enable;
- xhci = pcidev_path_on_root(PCH_DEVFN_XHCI); + xhci = pcidev_path_on_root_debug(PCH_DEVFN_XHCI); if (!xhci) { printk(BIOS_ERR, "%s: Could not locate XHCI device in DT\n", __func__); return; diff --git a/src/soc/intel/denverton_ns/memmap.c b/src/soc/intel/denverton_ns/memmap.c index 0cca4b9..685affe 100644 --- a/src/soc/intel/denverton_ns/memmap.c +++ b/src/soc/intel/denverton_ns/memmap.c @@ -30,7 +30,7 @@ #if defined(__SIMPLE_DEVICE__) pci_devfn_t dev = SA_DEV_ROOT; #else - struct device *dev = pcidev_path_on_root(SA_DEVFN_ROOT); + struct device *dev = pcidev_path_on_root_debug(SA_DEVFN_ROOT); #endif /* All regions concerned for have 1 MiB alignment. */ return ALIGN_DOWN(pci_read_config32(dev, reg), 1 * MiB); diff --git a/src/soc/intel/fsp_baytrail/acpi.c b/src/soc/intel/fsp_baytrail/acpi.c index fb941ab..8bd3818 100644 --- a/src/soc/intel/fsp_baytrail/acpi.c +++ b/src/soc/intel/fsp_baytrail/acpi.c @@ -175,7 +175,7 @@ void acpi_fill_in_fadt(acpi_fadt_t * fadt, acpi_facs_t * facs, void *dsdt) { acpi_header_t *header = &(fadt->header); - struct device *lpcdev = pcidev_path_on_root(FADT_SOC_LPC_DEVFN); + struct device *lpcdev = pcidev_path_on_root_debug(FADT_SOC_LPC_DEVFN); u16 pmbase = pci_read_config16(lpcdev, ABASE) & 0xfff0; config_t *config = config_of(lpcdev);
diff --git a/src/soc/intel/fsp_baytrail/fsp/chipset_fsp_util.c b/src/soc/intel/fsp_baytrail/fsp/chipset_fsp_util.c index 48f0cdd..61604a8 100644 --- a/src/soc/intel/fsp_baytrail/fsp/chipset_fsp_util.c +++ b/src/soc/intel/fsp_baytrail/fsp/chipset_fsp_util.c @@ -80,7 +80,7 @@ DEVTREE_CONST config_t *config; printk(FSP_INFO_LEVEL, "Configure Default UPD Data\n");
- dev = pcidev_path_on_root(SOC_DEV_FUNC); + dev = pcidev_path_on_root_debug(SOC_DEV_FUNC); config = config_of(dev);
/* Set up default verb tables - Just HDMI audio */ diff --git a/src/soc/intel/fsp_broadwell_de/acpi.c b/src/soc/intel/fsp_broadwell_de/acpi.c index fef7773..692bf98 100644 --- a/src/soc/intel/fsp_broadwell_de/acpi.c +++ b/src/soc/intel/fsp_broadwell_de/acpi.c @@ -316,7 +316,7 @@ static unsigned long acpi_fill_dmar(unsigned long current) { uint32_t vtbar, tmp = current; - struct device *dev = pcidev_path_on_root(VTD_DEV_FUNC); + struct device *dev = pcidev_path_on_root_debug(VTD_DEV_FUNC); uint16_t bdf, hpet_bdf[8]; uint8_t i, j;
@@ -333,7 +333,7 @@ current += acpi_create_dmar_ds_ioapic(current, 9, 0, 5, 4); /* Get the PCI BDF for the PCH I/O APIC */ - dev = pcidev_path_on_root(LPC_DEV_FUNC); + dev = pcidev_path_on_root_debug(LPC_DEV_FUNC); bdf = pci_read_config16(dev, 0x6c); current += acpi_create_dmar_ds_ioapic(current, 8, (bdf >> 8), PCI_SLOT(bdf), PCI_FUNC(bdf)); diff --git a/src/soc/intel/icelake/fsp_params.c b/src/soc/intel/icelake/fsp_params.c index e31e47b..3506e25 100644 --- a/src/soc/intel/icelake/fsp_params.c +++ b/src/soc/intel/icelake/fsp_params.c @@ -93,7 +93,7 @@
mainboard_silicon_init_params(params);
- dev = pcidev_path_on_root(SA_DEVFN_IGD); + dev = pcidev_path_on_root_debug(SA_DEVFN_IGD);
if (!dev || !dev->enabled) { /* diff --git a/src/soc/intel/icelake/romstage/fsp_params.c b/src/soc/intel/icelake/romstage/fsp_params.c index a78c8a4..f63440a 100644 --- a/src/soc/intel/icelake/romstage/fsp_params.c +++ b/src/soc/intel/icelake/romstage/fsp_params.c @@ -25,7 +25,7 @@ const struct soc_intel_icelake_config *config) { unsigned int i; - const struct device *dev = pcidev_path_on_root(SA_DEVFN_IGD); + const struct device *dev = pcidev_path_on_root_debug(SA_DEVFN_IGD); uint32_t mask = 0;
if (!dev || !dev->enabled) { @@ -49,7 +49,7 @@ m_cfg->SkipMbpHob = 1;
/* If Audio Codec is enabled, enable FSP UPD */ - dev = pcidev_path_on_root(PCH_DEVFN_HDA); + dev = pcidev_path_on_root_debug(PCH_DEVFN_HDA); if (!dev) m_cfg->PchHdaEnable = 0; else diff --git a/src/soc/intel/icelake/smmrelocate.c b/src/soc/intel/icelake/smmrelocate.c index 65505c4..8093b7a 100644 --- a/src/soc/intel/icelake/smmrelocate.c +++ b/src/soc/intel/icelake/smmrelocate.c @@ -256,7 +256,7 @@
void smm_lock(void) { - struct device *sa_dev = pcidev_path_on_root(SA_DEVFN_ROOT); + struct device *sa_dev = pcidev_path_on_root_debug(SA_DEVFN_ROOT); /* * LOCK the SMM memory window and enable normal SMM. * After running this function, only a full reset can diff --git a/src/soc/intel/icelake/uart.c b/src/soc/intel/icelake/uart.c index ae19acc..2d7e1d1 100644 --- a/src/soc/intel/icelake/uart.c +++ b/src/soc/intel/icelake/uart.c @@ -58,11 +58,11 @@ */ switch (uart_console) { case 0: - return pcidev_path_on_root(PCH_DEVFN_UART0); + return pcidev_path_on_root_debug(PCH_DEVFN_UART0); case 1: - return pcidev_path_on_root(PCH_DEVFN_UART1); + return pcidev_path_on_root_debug(PCH_DEVFN_UART1); case 2: - return pcidev_path_on_root(PCH_DEVFN_UART2); + return pcidev_path_on_root_debug(PCH_DEVFN_UART2); default: printk(BIOS_ERR, "Invalid UART console index\n"); return NULL; diff --git a/src/soc/intel/skylake/acpi.c b/src/soc/intel/skylake/acpi.c index aa51cbe..aafaa9f 100644 --- a/src/soc/intel/skylake/acpi.c +++ b/src/soc/intel/skylake/acpi.c @@ -552,7 +552,7 @@
static unsigned long acpi_fill_dmar(unsigned long current) { - struct device *const igfx_dev = pcidev_path_on_root(SA_DEVFN_IGD); + struct device *const igfx_dev = pcidev_path_on_root_debug(SA_DEVFN_IGD); const u32 gfx_vtbar = MCHBAR32(GFXVTBAR) & ~0xfff; const bool gfxvten = MCHBAR32(GFXVTBAR) & 1;
@@ -575,7 +575,7 @@ acpi_dmar_rmrr_fixup(tmp, current); }
- struct device *const p2sb_dev = pcidev_path_on_root(PCH_DEVFN_P2SB); + struct device *const p2sb_dev = pcidev_path_on_root_debug(PCH_DEVFN_P2SB); const u32 vtvc0bar = MCHBAR32(VTVC0BAR) & ~0xfff; const bool vtvc0en = MCHBAR32(VTVC0BAR) & 1;
diff --git a/src/soc/intel/skylake/chip.c b/src/soc/intel/skylake/chip.c index a7d5872..fd283fc 100644 --- a/src/soc/intel/skylake/chip.c +++ b/src/soc/intel/skylake/chip.c @@ -92,7 +92,7 @@ /* UPD parameters to be initialized before SiliconInit */ void soc_silicon_init_params(SILICON_INIT_UPD *params) { - struct device *dev = pcidev_path_on_root(PCH_DEVFN_LPC); + struct device *dev = pcidev_path_on_root_debug(PCH_DEVFN_LPC); const struct soc_intel_skylake_config *config = config_of(dev); int i;
@@ -152,7 +152,7 @@ params->ScsSdCardEnabled = config->ScsSdCardEnabled;
/* Enable ISH if device is on */ - dev = pcidev_path_on_root(PCH_DEVFN_ISH); + dev = pcidev_path_on_root_debug(PCH_DEVFN_ISH); if (dev) params->IshEnable = dev->enabled; else @@ -219,11 +219,11 @@ fill_vr_domain_config(params, i, &config->domain_vr_config[i]);
/* Show SPI controller if enabled in devicetree.cb */ - dev = pcidev_path_on_root(PCH_DEVFN_SPI); + dev = pcidev_path_on_root_debug(PCH_DEVFN_SPI); params->ShowSpiController = dev->enabled;
/* Enable xDCI controller if enabled in devicetree and allowed */ - dev = pcidev_path_on_root(PCH_DEVFN_USBOTG); + dev = pcidev_path_on_root_debug(PCH_DEVFN_USBOTG); if (!xdci_can_enable()) dev->enabled = 0; params->XdciEnable = dev->enabled; diff --git a/src/soc/intel/skylake/chip_fsp20.c b/src/soc/intel/skylake/chip_fsp20.c index 064f71e..c61937a 100644 --- a/src/soc/intel/skylake/chip_fsp20.c +++ b/src/soc/intel/skylake/chip_fsp20.c @@ -102,7 +102,7 @@
for (group = 0; group < pci_groups; group++) { devfn0 = pcie_rp_group[group].devfn; - func0 = pcidev_path_on_root(devfn0); + func0 = pcidev_path_on_root_debug(devfn0); if (func0 == NULL) continue;
@@ -119,7 +119,7 @@ */ for (i = 1; i < pcie_rp_group[group].func_count; i++, devfn += inc) { - struct device *dev = pcidev_path_on_root(devfn); + struct device *dev = pcidev_path_on_root_debug(devfn); if (dev == NULL || !dev->enabled) continue;
@@ -353,7 +353,7 @@ }
/* If ISH is enabled, enable ISH elements */ - dev = pcidev_path_on_root(PCH_DEVFN_ISH); + dev = pcidev_path_on_root_debug(PCH_DEVFN_ISH); if (dev) params->PchIshEnable = dev->enabled; else @@ -432,11 +432,11 @@ fill_vr_domain_config(params, i, &config->domain_vr_config[i]);
/* Show SPI controller if enabled in devicetree.cb */ - dev = pcidev_path_on_root(PCH_DEVFN_SPI); + dev = pcidev_path_on_root_debug(PCH_DEVFN_SPI); params->ShowSpiController = dev->enabled;
/* Enable xDCI controller if enabled in devicetree and allowed */ - dev = pcidev_path_on_root(PCH_DEVFN_USBOTG); + dev = pcidev_path_on_root_debug(PCH_DEVFN_USBOTG); if (!xdci_can_enable()) dev->enabled = 0; params->XdciEnable = dev->enabled; diff --git a/src/soc/intel/skylake/irq.c b/src/soc/intel/skylake/irq.c index ddaffda..2f02959 100644 --- a/src/soc/intel/skylake/irq.c +++ b/src/soc/intel/skylake/irq.c @@ -223,7 +223,7 @@
uint32_t i, intdeventry; u8 irq_config[PCH_MAX_IRQ_CONFIG]; - const struct device *dev = pcidev_path_on_root(PCH_DEVFN_LPC); + const struct device *dev = pcidev_path_on_root_debug(PCH_DEVFN_LPC); const struct soc_intel_skylake_config *config = config_of(dev);
/* Get Device Int Count */ diff --git a/src/soc/intel/skylake/memmap.c b/src/soc/intel/skylake/memmap.c index 9d3f377..c9f0f9d 100644 --- a/src/soc/intel/skylake/memmap.c +++ b/src/soc/intel/skylake/memmap.c @@ -113,7 +113,7 @@ /* Calculate Intel Traditional Memory size based on GSM, DSM, TSEG and DPR. */ static size_t calculate_traditional_mem_size(uintptr_t dram_base) { - const struct device *igd_dev = pcidev_path_on_root(SA_DEVFN_IGD); + const struct device *igd_dev = pcidev_path_on_root_debug(SA_DEVFN_IGD); uintptr_t traditional_mem_base = dram_base; size_t traditional_mem_size;
@@ -143,7 +143,7 @@ */ static size_t calculate_reserved_mem_size(uintptr_t dram_base) { - const struct device *dev = pcidev_path_on_root(SA_DEVFN_ROOT); + const struct device *dev = pcidev_path_on_root_debug(SA_DEVFN_ROOT); uintptr_t reserve_mem_base = dram_base; size_t reserve_mem_size; const struct soc_intel_skylake_config *config; diff --git a/src/soc/intel/skylake/romstage/romstage_fsp20.c b/src/soc/intel/skylake/romstage/romstage_fsp20.c index 8b5cd18..985c861 100644 --- a/src/soc/intel/skylake/romstage/romstage_fsp20.c +++ b/src/soc/intel/skylake/romstage/romstage_fsp20.c @@ -300,7 +300,7 @@ { const struct device *dev;
- dev = pcidev_path_on_root(SA_DEVFN_IGD); + dev = pcidev_path_on_root_debug(SA_DEVFN_IGD); if (!dev || !dev->enabled) { /* * If iGPU is disabled or not defined in the devicetree.cb, diff --git a/src/soc/intel/skylake/romstage/systemagent.c b/src/soc/intel/skylake/romstage/systemagent.c index 9b7ea24..e444698 100644 --- a/src/soc/intel/skylake/romstage/systemagent.c +++ b/src/soc/intel/skylake/romstage/systemagent.c @@ -26,7 +26,7 @@
static void systemagent_vtd_init(void) { - const struct device *const igd_dev = pcidev_path_on_root(SA_DEVFN_IGD); + const struct device *const igd_dev = pcidev_path_on_root_debug(SA_DEVFN_IGD); const struct soc_intel_skylake_config *config = NULL;
config = config_of_path(SA_DEVFN_ROOT); diff --git a/src/soc/intel/skylake/smmrelocate.c b/src/soc/intel/skylake/smmrelocate.c index e1779d1..f3117c2 100644 --- a/src/soc/intel/skylake/smmrelocate.c +++ b/src/soc/intel/skylake/smmrelocate.c @@ -256,7 +256,7 @@
void smm_lock(void) { - struct device *sa_dev = pcidev_path_on_root(SA_DEVFN_ROOT); + struct device *sa_dev = pcidev_path_on_root_debug(SA_DEVFN_ROOT); /* * LOCK the SMM memory window and enable normal SMM. * After running this function, only a full reset can diff --git a/src/soc/intel/skylake/systemagent.c b/src/soc/intel/skylake/systemagent.c index 410265f..2f56cf9 100644 --- a/src/soc/intel/skylake/systemagent.c +++ b/src/soc/intel/skylake/systemagent.c @@ -29,7 +29,7 @@
bool soc_is_vtd_capable(void) { - struct device *const root_dev = pcidev_path_on_root(SA_DEVFN_ROOT); + struct device *const root_dev = pcidev_path_on_root_debug(SA_DEVFN_ROOT); return root_dev && !(pci_read_config32(root_dev, CAPID0_A) & VTD_DISABLE); } @@ -42,7 +42,7 @@ */ void soc_add_fixed_mmio_resources(struct device *dev, int *index) { - struct device *const igd_dev = pcidev_path_on_root(SA_DEVFN_IGD); + struct device *const igd_dev = pcidev_path_on_root_debug(SA_DEVFN_IGD);
static const struct sa_mmio_descriptor soc_fixed_resources[] = { { PCIEXBAR, CONFIG_MMCONF_BASE_ADDRESS, CONFIG_SA_PCIEX_LENGTH, diff --git a/src/soc/intel/skylake/uart.c b/src/soc/intel/skylake/uart.c index 18fcf1b..d049c60 100644 --- a/src/soc/intel/skylake/uart.c +++ b/src/soc/intel/skylake/uart.c @@ -59,11 +59,11 @@ */ switch (uart_console) { case 0: - return pcidev_path_on_root(PCH_DEVFN_UART0); + return pcidev_path_on_root_debug(PCH_DEVFN_UART0); case 1: - return pcidev_path_on_root(PCH_DEVFN_UART1); + return pcidev_path_on_root_debug(PCH_DEVFN_UART1); case 2: - return pcidev_path_on_root(PCH_DEVFN_UART2); + return pcidev_path_on_root_debug(PCH_DEVFN_UART2); default: printk(BIOS_ERR, "Invalid UART console index\n"); return NULL; diff --git a/src/soc/intel/skylake/vr_config.c b/src/soc/intel/skylake/vr_config.c index 089dd5d..5c92fb4 100644 --- a/src/soc/intel/skylake/vr_config.c +++ b/src/soc/intel/skylake/vr_config.c @@ -94,18 +94,18 @@
static uint16_t mch_id = 0, igd_id = 0, lpc_id = 0; if (!mch_id) { - struct device *dev = pcidev_path_on_root(SA_DEVFN_ROOT); + struct device *dev = pcidev_path_on_root_debug(SA_DEVFN_ROOT); mch_id = pci_read_config16(dev, PCI_DEVICE_ID); } if (!igd_id) { - struct device *dev = pcidev_path_on_root(SA_DEVFN_IGD); + struct device *dev = pcidev_path_on_root_debug(SA_DEVFN_IGD); if (dev) igd_id = pci_read_config16(dev, PCI_DEVICE_ID); else igd_id = 0xffff; } if (!lpc_id) { - struct device *dev = pcidev_path_on_root(PCH_DEVFN_LPC); + struct device *dev = pcidev_path_on_root_debug(PCH_DEVFN_LPC); lpc_id = pci_read_config16(dev, PCI_DEVICE_ID); }
@@ -225,11 +225,11 @@ { static uint16_t mch_id = 0, igd_id = 0; if (!mch_id) { - struct device *dev = pcidev_path_on_root(SA_DEVFN_ROOT); + struct device *dev = pcidev_path_on_root_debug(SA_DEVFN_ROOT); mch_id = pci_read_config16(dev, PCI_DEVICE_ID); } if (!igd_id) { - struct device *dev = pcidev_path_on_root(SA_DEVFN_IGD); + struct device *dev = pcidev_path_on_root_debug(SA_DEVFN_IGD); if (dev) igd_id = pci_read_config16(dev, PCI_DEVICE_ID); else
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35088 )
Change subject: soc/intel: always use pcidev_path_on_root_debug() ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35088/1//COMMIT_MSG Commit Message:
PS1: Blocking submit due to lack of testing
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35088 )
Change subject: soc/intel: always use pcidev_path_on_root_debug() ......................................................................
Patch Set 2: Code-Review-1
Patch Set 2: Code-Review-2
The trouble is the entire PCI subsystem in ramstage is based on matching the vendor/device ID register with a PCI driver and to the source we want to control that device with. To allow this hiding of PCI devices will ultimately force us to write the control somewhere in the scope of SOC instead. Oh, but wait, perhaps the intention is for us to __not__ write that control anymore but let the FSP blob do all that too!
AFAICS, hardware that does not respond to vendor/device ID register reads does not meet PCI compliance. I am willing to hit +2 on removing support for platforms that do not meet PCI compliance, specially when in the cases here, it is a matter of broken FSP blobs and not broken silicon per-se.
Also, I should not be accepting new callers for dev_find_slot() due the ill semantics it has. Prior to device enumeration, it can return false positives because all devices appear on bus 0. So please look for alternative solution if you want to support Intel's initiative of more blob less FOSS.
I perfectly understand your concerns, hence the sarcastic commit message. This patch is meant to be a "less bad" CB:35087 variant, as it allows printing where errors happen.
If this patch were to be merged in as-is, it would mean the platforms it affects would now be deprecated and marked for removal unless platforms don't cause any BUGs to appear in logs (so this patch shouldn't be necessary for them anyway).
Unfortunately, I do not have any hardware for many of these platforms, so I cannot fix coreboot for them. To clean up the blob mess in here in a reasonable timeframe, one would have to perform more than a dozen tests per hour. Help is appreciated.
Angel Pons has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/35088 )
Change subject: soc/intel: always use pcidev_path_on_root_debug() ......................................................................
Abandoned
This patch is not meant to be merged, and already did a good job to locate where things went wrong.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35088 )
Change subject: soc/intel: always use pcidev_path_on_root_debug() ......................................................................
Patch Set 2:
ok, we've identified the issue, now what? Skylake/Kabylake are still partially broken in master since CB:33996 was merged, and no mitigation has been proposed
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35088 )
Change subject: soc/intel: always use pcidev_path_on_root_debug() ......................................................................
Patch Set 2:
Patch Set 2:
ok, we've identified the issue, now what? Skylake/Kabylake are still partially broken in master since CB:33996 was merged, and no mitigation has been proposed
I suggest you post on the mailing list. That active PCI devices no longer respond to Vendor ID / Device ID queries does not meet PCI compliance, as I understand the specs. Unfortunately, someone with enough authority inside the org will likely decide it's just fine that ramstage will no longer be designed using PCI drivers and allow use of shim calling massive FSP blob.
-
Angel Pons (Code Review) -
Kyösti Mälkki (Code Review)
-
Matt DeVillier (Code Review)