Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34411 )
Change subject: soc/intel: Expand SA_DEV_ROOT for ramstage ......................................................................
soc/intel: Expand SA_DEV_ROOT for ramstage
We do not want to disguise somewhat complex function calls as simple macros.
Change-Id: I298f7f9a1c6a64cfba454e919eeaedc7bb2d4801 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/mainboard/google/poppy/variants/atlas/mainboard.c M src/mainboard/google/poppy/variants/nocturne/mainboard.c M src/soc/intel/apollolake/include/soc/pci_devs.h M src/soc/intel/broadwell/finalize.c M src/soc/intel/broadwell/include/soc/pci_devs.h 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/include/soc/pci_devs.h M src/soc/intel/cannonlake/smmrelocate.c M src/soc/intel/common/block/graphics/graphics.c M src/soc/intel/denverton_ns/include/soc/pci_devs.h M src/soc/intel/icelake/include/soc/pci_devs.h M src/soc/intel/icelake/smmrelocate.c M src/soc/intel/skylake/include/soc/pci_devs.h M src/soc/intel/skylake/romstage/romstage_fsp20.c M src/soc/intel/skylake/smmrelocate.c M src/soc/intel/skylake/systemagent.c M src/soc/intel/skylake/vr_config.c 19 files changed, 74 insertions(+), 79 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/34411/1
diff --git a/src/mainboard/google/poppy/variants/atlas/mainboard.c b/src/mainboard/google/poppy/variants/atlas/mainboard.c index d4db98e..9c4b2bc 100644 --- a/src/mainboard/google/poppy/variants/atlas/mainboard.c +++ b/src/mainboard/google/poppy/variants/atlas/mainboard.c @@ -25,8 +25,10 @@
static uint32_t get_pl2(void) { + struct device *igd_dev = pcidev_path_on_root(SA_DEVFN_IGD); uint16_t id; - id = pci_read_config16(SA_DEV_IGD, PCI_DEVICE_ID); + + id = pci_read_config16(igd_dev, PCI_DEVICE_ID); /* Assume we only have KLB-Y and AML-Y SKUs */ if (id == PCI_DEVICE_ID_INTEL_KBL_GT2_SULXM) return PL2_KBL; diff --git a/src/mainboard/google/poppy/variants/nocturne/mainboard.c b/src/mainboard/google/poppy/variants/nocturne/mainboard.c index f00394c..7b6b28b 100644 --- a/src/mainboard/google/poppy/variants/nocturne/mainboard.c +++ b/src/mainboard/google/poppy/variants/nocturne/mainboard.c @@ -26,8 +26,10 @@
static uint32_t get_pl2(void) { + struct device *igd_dev = pcidev_path_on_root(SA_DEVFN_IGD); uint16_t id; - id = pci_read_config16(SA_DEV_IGD, PCI_DEVICE_ID); + + id = pci_read_config16(igd_dev, PCI_DEVICE_ID); /* Assume we only have KLB-Y and AML-Y SKUs */ if (id == PCI_DEVICE_ID_INTEL_KBL_GT2_SULXM) return PL2_KBL; diff --git a/src/soc/intel/apollolake/include/soc/pci_devs.h b/src/soc/intel/apollolake/include/soc/pci_devs.h index c5eaf4c..acc282c 100644 --- a/src/soc/intel/apollolake/include/soc/pci_devs.h +++ b/src/soc/intel/apollolake/include/soc/pci_devs.h @@ -17,35 +17,32 @@
#include <device/pci_def.h>
-#define _SA_DEVFN(slot) PCI_DEVFN(SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEVFN(slot, func) PCI_DEVFN(PCH_DEV_SLOT_ ## slot, func)
#if !defined(__SIMPLE_DEVICE__) #include <device/device.h> -#define _SA_DEV(slot) pcidev_path_on_root_debug(_SA_DEVFN(slot), __func__) #define _PCH_DEV(slot, func) pcidev_path_on_root_debug(_PCH_DEVFN(slot, func), __func__) #else -#define _SA_DEV(slot) PCI_DEV(0, SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEV(slot, func) PCI_DEV(0, PCH_DEV_SLOT_ ## slot, func) #endif
/* System Agent Devices */
#define SA_DEV_SLOT_ROOT 0x00 -#define SA_DEVFN_ROOT _SA_DEVFN(ROOT) -#define SA_DEV_ROOT _SA_DEV(ROOT) +#define SA_DEVFN_ROOT PCI_DEVFN(SA_DEV_SLOT_ROOT, 0) +#define SA_DEV_ROOT PCI_DEV(0, SA_DEV_SLOT_ROOT, 0)
#define SA_DEV_SLOT_PUNIT 0x01 -#define SA_DEVFN_PUNIT _SA_DEVFN(PUNIT) -#define SA_DEV_PUNIT _SA_DEV(PUNIT) +#define SA_DEVFN_PUNIT PCI_DEVFN(SA_DEV_SLOT_PUNIT, 0) +#define SA_DEV_IPU PCI_DEV(0, SA_DEV_SLOT_PUNIT, 0)
#define SA_DEV_SLOT_IGD 0x02 -#define SA_DEVFN_IGD _SA_DEVFN(IGD) -#define SA_DEV_IGD _SA_DEV(IGD) +#define SA_DEVFN_IGD PCI_DEVFN(SA_DEV_SLOT_IGD, 0) +#define SA_DEV_IGD PCI_DEV(0, SA_DEV_SLOT_IGD, 0)
#define SA_DEV_SLOT_IPU 0x03 -#define SA_DEVFN_IPU _SA_DEVFN(IPU) -#define SA_DEV_IPU _SA_DEV(IPU) +#define SA_DEVFN_IPU PCI_DEVFN(SA_DEV_SLOT_IPU, 0) +#define SA_DEV_IPU PCI_DEV(0, SA_DEV_SLOT_IPU, 0)
/* PCH Devices */
diff --git a/src/soc/intel/broadwell/finalize.c b/src/soc/intel/broadwell/finalize.c index 1adbbc8..06cc18d 100644 --- a/src/soc/intel/broadwell/finalize.c +++ b/src/soc/intel/broadwell/finalize.c @@ -96,9 +96,11 @@
static void broadwell_finalize(void *unused) { + struct device *sa_dev = pcidev_path_on_root(SA_DEVFN_ROOT); + printk(BIOS_DEBUG, "Finalizing chipset.\n");
- reg_script_run_on_dev(SA_DEV_ROOT, system_agent_finalize_script); + reg_script_run_on_dev(sa_dev, system_agent_finalize_script); reg_script_run_on_dev(PCH_DEV_LPC, pch_finalize_script);
/* Lock */ diff --git a/src/soc/intel/broadwell/include/soc/pci_devs.h b/src/soc/intel/broadwell/include/soc/pci_devs.h index ae3e08f..ee522f7 100644 --- a/src/soc/intel/broadwell/include/soc/pci_devs.h +++ b/src/soc/intel/broadwell/include/soc/pci_devs.h @@ -16,32 +16,29 @@ #ifndef _BROADWELL_PCI_DEVS_H_ #define _BROADWELL_PCI_DEVS_H_
-#define _SA_DEVFN(slot) PCI_DEVFN(SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEVFN(slot, func) PCI_DEVFN(PCH_DEV_SLOT_ ## slot, func)
#if defined(__SIMPLE_DEVICE__) -#define _SA_DEV(slot) PCI_DEV(0, SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEV(slot, func) PCI_DEV(0, PCH_DEV_SLOT_ ## slot, func) #else #include <device/device.h> #include <device/pci_def.h> -#define _SA_DEV(slot) pcidev_path_on_root_debug(_SA_DEVFN(slot), __func__) #define _PCH_DEV(slot, func) pcidev_path_on_root_debug(_PCH_DEVFN(slot, func), __func__) #endif
/* System Agent Devices */
#define SA_DEV_SLOT_ROOT 0x00 -#define SA_DEVFN_ROOT _SA_DEVFN(ROOT) -#define SA_DEV_ROOT _SA_DEV(ROOT) +#define SA_DEVFN_ROOT PCI_DEVFN(SA_DEV_SLOT_ROOT, 0) +#define SA_DEV_ROOT PCI_DEV(0, SA_DEV_SLOT_ROOT, 0)
#define SA_DEV_SLOT_IGD 0x02 -#define SA_DEVFN_IGD _SA_DEVFN(IGD) -#define SA_DEV_IGD _SA_DEV(IGD) +#define SA_DEVFN_IGD PCI_DEVFN(SA_DEV_SLOT_IGD, 0) +#define SA_DEV_IGD PCI_DEV(0, SA_DEV_SLOT_IGD, 0)
#define SA_DEV_SLOT_MINIHD 0x03 -#define SA_DEVFN_MINIHD _SA_DEVFN(MINIHD) -#define SA_DEV_MINIHD _SA_DEV(MINIHD) +#define SA_DEVFN_MINIHD PCI_DEVFN(SA_DEV_SLOT_MINIHD, 0) +#define SA_DEV_IPU PCI_DEV(0, SA_DEV_SLOT_MINIHD, 0)
/* PCH Devices */
diff --git a/src/soc/intel/broadwell/lpc.c b/src/soc/intel/broadwell/lpc.c index 9be4aeb..b385d6b 100644 --- a/src/soc/intel/broadwell/lpc.c +++ b/src/soc/intel/broadwell/lpc.c @@ -365,6 +365,7 @@ { u32 reg32; u16 reg16; + struct device *igd_dev = pcidev_path_on_root(SA_DEVFN_IGD);
/* DMI */ RCBA32_OR(0x2234, 0xf); @@ -388,7 +389,7 @@ RCBA32_AND_OR(0x2614, ~0x64ff0000, 0x0a206500);
/* Check for 0:2.0@0x08 >= 0x0b */ - if (pch_is_wpt() || pci_read_config8(SA_DEV_IGD, 0x8) >= 0x0b) + if (pch_is_wpt() || pci_read_config8(igd_dev, 0x8) >= 0x0b) RCBA32_OR(0x2614, (1 << 26));
RCBA32_OR(0x900, 0x0000031f); diff --git a/src/soc/intel/broadwell/smmrelocate.c b/src/soc/intel/broadwell/smmrelocate.c index 98c3c4c..9ea73b2 100644 --- a/src/soc/intel/broadwell/smmrelocate.c +++ b/src/soc/intel/broadwell/smmrelocate.c @@ -319,10 +319,11 @@
void smm_lock(void) { + struct device *sa_dev = pcidev_path_on_root(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. */ printk(BIOS_DEBUG, "Locking SMM.\n"); - pci_write_config8(SA_DEV_ROOT, SMRAM, D_LCK | G_SMRAME | C_BASE_SEG); + pci_write_config8(sa_dev, SMRAM, D_LCK | G_SMRAME | C_BASE_SEG); } diff --git a/src/soc/intel/broadwell/systemagent.c b/src/soc/intel/broadwell/systemagent.c index c6444b1..b6b5608 100644 --- a/src/soc/intel/broadwell/systemagent.c +++ b/src/soc/intel/broadwell/systemagent.c @@ -32,19 +32,22 @@
u8 systemagent_revision(void) { - return pci_read_config8(SA_DEV_ROOT, PCI_REVISION_ID); + struct device *sa_dev = pcidev_path_on_root(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); /* Bit 0 is lock bit, not part of address */ - return pci_read_config32(SA_DEV_ROOT, TOLUD) & ~1; + 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); /* Bit 0 is lock bit, not part of address */ - return pci_read_config32(SA_DEV_ROOT, BGSM) & ~1; + return pci_read_config32(sa_dev, BGSM) & ~1; }
static int get_pcie_bar(struct device *dev, unsigned int index, u32 *base, @@ -291,6 +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);
/* Read in the MAP registers and report their values. */ mc_read_map_entries(dev, &mc_values[0]); @@ -302,7 +306,7 @@ * the DPR register reports the TOP of the region, which is the same * as TSEG base. The region size is reported in MiB in bits 11:4. */ - dpr_reg = pci_read_config32(SA_DEV_ROOT, DPR); + dpr_reg = pci_read_config32(sa_dev, DPR); if (dpr_reg & DPR_EPM) { dpr_size = (dpr_reg & DPR_SIZE_MASK) << 16; printk(BIOS_INFO, "DPR SIZE: 0x%lx\n", dpr_size); diff --git a/src/soc/intel/cannonlake/include/soc/pci_devs.h b/src/soc/intel/cannonlake/include/soc/pci_devs.h index 33814b0..8106447 100644 --- a/src/soc/intel/cannonlake/include/soc/pci_devs.h +++ b/src/soc/intel/cannonlake/include/soc/pci_devs.h @@ -19,35 +19,32 @@
#include <device/pci_def.h>
-#define _SA_DEVFN(slot) PCI_DEVFN(SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEVFN(slot, func) PCI_DEVFN(PCH_DEV_SLOT_ ## slot, func)
#if !defined(__SIMPLE_DEVICE__) #include <device/device.h> -#define _SA_DEV(slot) pcidev_path_on_root_debug(_SA_DEVFN(slot), __func__) #define _PCH_DEV(slot, func) pcidev_path_on_root_debug(_PCH_DEVFN(slot, func), __func__) #else -#define _SA_DEV(slot) PCI_DEV(0, SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEV(slot, func) PCI_DEV(0, PCH_DEV_SLOT_ ## slot, func) #endif
/* System Agent Devices */
#define SA_DEV_SLOT_ROOT 0x00 -#define SA_DEVFN_ROOT _SA_DEVFN(ROOT) -#define SA_DEV_ROOT _SA_DEV(ROOT) +#define SA_DEVFN_ROOT PCI_DEVFN(SA_DEV_SLOT_ROOT, 0) +#define SA_DEV_ROOT PCI_DEV(0, SA_DEV_SLOT_ROOT, 0)
#define SA_DEV_SLOT_IGD 0x02 -#define SA_DEVFN_IGD _SA_DEVFN(IGD) -#define SA_DEV_IGD _SA_DEV(IGD) +#define SA_DEVFN_IGD PCI_DEVFN(SA_DEV_SLOT_IGD, 0) +#define SA_DEV_IGD PCI_DEV(0, SA_DEV_SLOT_IGD, 0)
#define SA_DEV_SLOT_DSP 0x04 -#define SA_DEVFN_DSP _SA_DEVFN(DSP) -#define SA_DEV_DSP _SA_DEV(DSP) +#define SA_DEVFN_DSP PCI_DEVFN(SA_DEV_SLOT_DSP, 0) +#define SA_DEV_IPU PCI_DEV(0, SA_DEV_SLOT_DSP, 0)
#define SA_DEV_SLOT_IPU 0x05 -#define SA_DEVFN_IPU _SA_DEVFN(IPU) -#define SA_DEV_IPU _SA_DEV(IPU) +#define SA_DEVFN_IPU PCI_DEVFN(SA_DEV_SLOT_IPU, 0) +#define SA_DEV_IPU PCI_DEV(0, SA_DEV_SLOT_IPU, 0)
/* PCH Devices */ #define PCH_DEV_SLOT_THERMAL 0x12 diff --git a/src/soc/intel/cannonlake/smmrelocate.c b/src/soc/intel/cannonlake/smmrelocate.c index 2576c9c..980702f 100644 --- a/src/soc/intel/cannonlake/smmrelocate.c +++ b/src/soc/intel/cannonlake/smmrelocate.c @@ -301,11 +301,12 @@
void smm_lock(void) { + struct device *sa_dev = pcidev_path_on_root(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. */ printk(BIOS_DEBUG, "Locking SMM.\n"); - pci_write_config8(SA_DEV_ROOT, SMRAM, D_LCK | G_SMRAME | C_BASE_SEG); + pci_write_config8(sa_dev, SMRAM, D_LCK | G_SMRAME | C_BASE_SEG); } diff --git a/src/soc/intel/common/block/graphics/graphics.c b/src/soc/intel/common/block/graphics/graphics.c index 7885ad7..26e1cb8 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 = SA_DEV_IGD; + struct device *dev = pcidev_path_on_root(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 = SA_DEV_IGD; + struct device *dev = pcidev_path_on_root(SA_DEVFN_IGD);
if (is_graphics_disabled(dev)) die("IGD is disabled!"); diff --git a/src/soc/intel/denverton_ns/include/soc/pci_devs.h b/src/soc/intel/denverton_ns/include/soc/pci_devs.h index 27f9e35..2e510d9 100644 --- a/src/soc/intel/denverton_ns/include/soc/pci_devs.h +++ b/src/soc/intel/denverton_ns/include/soc/pci_devs.h @@ -21,16 +21,13 @@ /* All these devices live on bus 0 with the associated device and function */
-#define _SA_DEVFN(slot) PCI_DEVFN(SA_DEV_SLOT_##slot, 0) #define _PCH_DEVFN(slot, func) PCI_DEVFN(PCH_DEV_SLOT_##slot, func)
-#if ENV_RAMSTAGE +#if !defined(__SIMPLE_DEVICE__) #include <device/device.h> #include <device/pci_def.h> -#define _SA_DEV(slot) pcidev_path_on_root_debug(_SA_DEVFN(slot), __func__) #define _PCH_DEV(slot, func) pcidev_path_on_root_debug(_PCH_DEVFN(slot, func), __func__) #else -#define _SA_DEV(slot) PCI_DEV(0, SA_DEV_SLOT_##slot, 0) #define _PCH_DEV(slot, func) PCI_DEV(0, PCH_DEV_SLOT_##slot, func) #endif
@@ -191,8 +188,8 @@
/* TODO - New added */ #define SA_DEV_SLOT_ROOT 0x00 -#define SA_DEVFN_ROOT _SA_DEVFN(ROOT) -#define SA_DEV_ROOT _SA_DEV(ROOT) +#define SA_DEVFN_ROOT PCI_DEVFN(SA_DEV_SLOT_ROOT, 0) +#define SA_DEV_ROOT PCI_DEV(0, SA_DEV_SLOT_ROOT, 0)
#define PCH_DEV_SLOT_LPC 0x1f #define PCH_DEVFN_LPC _PCH_DEVFN(LPC, 0) diff --git a/src/soc/intel/icelake/include/soc/pci_devs.h b/src/soc/intel/icelake/include/soc/pci_devs.h index 0eddee2..62b91e9 100644 --- a/src/soc/intel/icelake/include/soc/pci_devs.h +++ b/src/soc/intel/icelake/include/soc/pci_devs.h @@ -18,31 +18,28 @@
#include <device/pci_def.h>
-#define _SA_DEVFN(slot) PCI_DEVFN(SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEVFN(slot, func) PCI_DEVFN(PCH_DEV_SLOT_ ## slot, func)
#if !defined(__SIMPLE_DEVICE__) #include <device/device.h> -#define _SA_DEV(slot) pcidev_path_on_root_debug(_SA_DEVFN(slot), __func__) #define _PCH_DEV(slot, func) pcidev_path_on_root_debug(_PCH_DEVFN(slot, func), __func__) #else -#define _SA_DEV(slot) PCI_DEV(0, SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEV(slot, func) PCI_DEV(0, PCH_DEV_SLOT_ ## slot, func) #endif
/* System Agent Devices */
#define SA_DEV_SLOT_ROOT 0x00 -#define SA_DEVFN_ROOT _SA_DEVFN(ROOT) -#define SA_DEV_ROOT _SA_DEV(ROOT) +#define SA_DEVFN_ROOT PCI_DEVFN(SA_DEV_SLOT_ROOT, 0) +#define SA_DEV_ROOT PCI_DEV(0, SA_DEV_SLOT_ROOT, 0)
#define SA_DEV_SLOT_IGD 0x02 -#define SA_DEVFN_IGD _SA_DEVFN(IGD) -#define SA_DEV_IGD _SA_DEV(IGD) +#define SA_DEVFN_IGD PCI_DEVFN(SA_DEV_SLOT_IGD, 0) +#define SA_DEV_IGD PCI_DEV(0, SA_DEV_SLOT_IGD, 0)
#define SA_DEV_SLOT_DSP 0x04 -#define SA_DEVFN_DSP _SA_DEVFN(DSP) -#define SA_DEV_DSP _SA_DEV(DSP) +#define SA_DEVFN_DSP PCI_DEVFN(SA_DEV_SLOT_DSP, 0) +#define SA_DEV_IPU PCI_DEV(0, SA_DEV_SLOT_DSP, 0)
/* PCH Devices */ #define PCH_DEV_SLOT_THERMAL 0x12 diff --git a/src/soc/intel/icelake/smmrelocate.c b/src/soc/intel/icelake/smmrelocate.c index 926ef63..63048eb 100644 --- a/src/soc/intel/icelake/smmrelocate.c +++ b/src/soc/intel/icelake/smmrelocate.c @@ -300,11 +300,12 @@
void smm_lock(void) { + struct device *sa_dev = pcidev_path_on_root(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. */ printk(BIOS_DEBUG, "Locking SMM.\n"); - pci_write_config8(SA_DEV_ROOT, SMRAM, D_LCK | G_SMRAME | C_BASE_SEG); + pci_write_config8(sa_dev, SMRAM, D_LCK | G_SMRAME | C_BASE_SEG); } diff --git a/src/soc/intel/skylake/include/soc/pci_devs.h b/src/soc/intel/skylake/include/soc/pci_devs.h index 0669ced..7147876 100644 --- a/src/soc/intel/skylake/include/soc/pci_devs.h +++ b/src/soc/intel/skylake/include/soc/pci_devs.h @@ -19,34 +19,26 @@
#include <device/pci_def.h>
-#define _SA_DEVFN(slot) PCI_DEVFN(SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEVFN(slot, func) PCI_DEVFN(PCH_DEV_SLOT_ ## slot, func)
#if !defined(__SIMPLE_DEVICE__) #include <device/device.h> -#define _SA_DEV(slot) pcidev_path_on_root_debug(_SA_DEVFN(slot), __func__) #define _PCH_DEV(slot, func) pcidev_path_on_root_debug(_PCH_DEVFN(slot, func), __func__) #else -#define _SA_DEV(slot) PCI_DEV(0, SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEV(slot, func) PCI_DEV(0, PCH_DEV_SLOT_ ## slot, func) #endif
/* System Agent Devices */
#define SA_DEV_SLOT_ROOT 0x00 -#define SA_DEVFN_ROOT _SA_DEVFN(ROOT) -#define SA_DEV_ROOT _SA_DEV(ROOT) +#define SA_DEVFN_ROOT PCI_DEVFN(SA_DEV_SLOT_ROOT, 0) +#define SA_DEV_ROOT PCI_DEV(0, SA_DEV_SLOT_ROOT, 0)
#define SA_DEV_SLOT_PEG 0x01 -#define SA_DEVFN_PEG(func) PCI_DEVFN(SA_DEV_SLOT_PEG, func) -#define SA_DEV_PEG(func) pcidev_path_on_root_debug(SA_DEVFN_PEG(func), __func__) -#define SA_DEV_PEG0 SA_DEV_PEG(0) -#define SA_DEV_PEG1 SA_DEV_PEG(1) -#define SA_DEV_PEG2 SA_DEV_PEG(2)
#define SA_DEV_SLOT_IGD 0x02 -#define SA_DEVFN_IGD _SA_DEVFN(IGD) -#define SA_DEV_IGD _SA_DEV(IGD) +#define SA_DEVFN_IGD PCI_DEVFN(SA_DEV_SLOT_IGD, 0) +#define SA_DEV_IGD PCI_DEV(0, SA_DEV_SLOT_IGD, 0)
/* PCH Devices */
diff --git a/src/soc/intel/skylake/romstage/romstage_fsp20.c b/src/soc/intel/skylake/romstage/romstage_fsp20.c index b15fa89..bb86c63 100644 --- a/src/soc/intel/skylake/romstage/romstage_fsp20.c +++ b/src/soc/intel/skylake/romstage/romstage_fsp20.c @@ -223,7 +223,7 @@ * If PEG port is not defined in the device tree, it will be disabled * in FSP */ - dev = SA_DEV_PEG0; /* PEG 0:1:0 */ + dev = pcidev_on_root(SA_DEV_SLOT_PEG, 0); /* PEG 0:1:0 */ if (!dev || !dev->enabled) m_cfg->Peg0Enable = 0; else if (dev->enabled) { @@ -238,7 +238,7 @@ m_t_cfg->Peg0Gen3EqPh3Method = 0; }
- dev = SA_DEV_PEG1; /* PEG 0:1:1 */ + dev = pcidev_on_root(SA_DEV_SLOT_PEG, 1); /* PEG 0:1:1 */ if (!dev || !dev->enabled) m_cfg->Peg1Enable = 0; else if (dev->enabled) { @@ -250,7 +250,7 @@ m_t_cfg->Peg1Gen3EqPh3Method = 0; }
- dev = SA_DEV_PEG2; /* PEG 0:1:2 */ + dev = pcidev_on_root(SA_DEV_SLOT_PEG, 2); /* PEG 0:1:2 */ if (!dev || !dev->enabled) m_cfg->Peg2Enable = 0; else if (dev->enabled) { diff --git a/src/soc/intel/skylake/smmrelocate.c b/src/soc/intel/skylake/smmrelocate.c index 7286187..816e1a8 100644 --- a/src/soc/intel/skylake/smmrelocate.c +++ b/src/soc/intel/skylake/smmrelocate.c @@ -310,11 +310,12 @@
void smm_lock(void) { + struct device *sa_dev = pcidev_path_on_root(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. */ printk(BIOS_DEBUG, "Locking SMM.\n"); - pci_write_config8(SA_DEV_ROOT, SMRAM, D_LCK | G_SMRAME | C_BASE_SEG); + pci_write_config8(sa_dev, SMRAM, D_LCK | G_SMRAME | C_BASE_SEG); } diff --git a/src/soc/intel/skylake/systemagent.c b/src/soc/intel/skylake/systemagent.c index ea55262..410265f 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 = SA_DEV_ROOT; + struct device *const root_dev = pcidev_path_on_root(SA_DEVFN_ROOT); return root_dev && !(pci_read_config32(root_dev, CAPID0_A) & VTD_DISABLE); } @@ -42,7 +42,8 @@ */ void soc_add_fixed_mmio_resources(struct device *dev, int *index) { - struct device *const igd_dev = SA_DEV_IGD; + struct device *const igd_dev = pcidev_path_on_root(SA_DEVFN_IGD); + static const struct sa_mmio_descriptor soc_fixed_resources[] = { { PCIEXBAR, CONFIG_MMCONF_BASE_ADDRESS, CONFIG_SA_PCIEX_LENGTH, "PCIEXBAR" }, diff --git a/src/soc/intel/skylake/vr_config.c b/src/soc/intel/skylake/vr_config.c index 905154e..c83e18d 100644 --- a/src/soc/intel/skylake/vr_config.c +++ b/src/soc/intel/skylake/vr_config.c @@ -174,17 +174,19 @@
static int get_kbl_sku(void) { + struct device *sa_dev = pcidev_path_on_root(SA_DEVFN_ROOT); static int sku = -1; uint16_t id;
if (sku != -1) return sku;
- id = get_dev_id(SA_DEV_ROOT); + id = get_dev_id(sa_dev); if (id == PCI_DEVICE_ID_INTEL_KBL_U_R) sku = KBL_R_SKU; else if (id == PCI_DEVICE_ID_INTEL_KBL_ID_Y) { - id = get_dev_id(SA_DEV_IGD); + struct device *igd_dev = pcidev_path_on_root(SA_DEVFN_IGD); + id = get_dev_id(igd_dev); if (id == PCI_DEVICE_ID_INTEL_AML_GT2_ULX) sku = AML_Y_SKU; else
Hello Patrick Rudolph, Vanny E, build bot (Jenkins), David Guckian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34411
to look at the new patch set (#2).
Change subject: soc/intel: Expand SA_DEV_ROOT for ramstage ......................................................................
soc/intel: Expand SA_DEV_ROOT for ramstage
We do not want to disguise somewhat complex function calls as simple macros.
Change-Id: I298f7f9a1c6a64cfba454e919eeaedc7bb2d4801 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/mainboard/google/poppy/variants/atlas/mainboard.c M src/mainboard/google/poppy/variants/nocturne/mainboard.c M src/soc/intel/apollolake/include/soc/pci_devs.h M src/soc/intel/broadwell/finalize.c M src/soc/intel/broadwell/include/soc/pci_devs.h 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/include/soc/pci_devs.h M src/soc/intel/cannonlake/smmrelocate.c M src/soc/intel/common/block/graphics/graphics.c M src/soc/intel/denverton_ns/include/soc/pci_devs.h M src/soc/intel/icelake/include/soc/pci_devs.h M src/soc/intel/icelake/smmrelocate.c M src/soc/intel/skylake/include/soc/pci_devs.h M src/soc/intel/skylake/romstage/romstage_fsp20.c M src/soc/intel/skylake/smmrelocate.c M src/soc/intel/skylake/systemagent.c M src/soc/intel/skylake/vr_config.c 19 files changed, 74 insertions(+), 79 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/34411/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34411 )
Change subject: soc/intel: Expand SA_DEV_ROOT for ramstage ......................................................................
Patch Set 2: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/34411/2/src/soc/intel/skylake/inclu... File src/soc/intel/skylake/include/soc/pci_devs.h:
https://review.coreboot.org/c/coreboot/+/34411/2/src/soc/intel/skylake/inclu... PS2, Line 35: SA_DEV_ROOT Are these still being used anywhere? Can we get rid of it from .h files?
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34411 )
Change subject: soc/intel: Expand SA_DEV_ROOT for ramstage ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34411/2/src/soc/intel/skylake/inclu... File src/soc/intel/skylake/include/soc/pci_devs.h:
https://review.coreboot.org/c/coreboot/+/34411/2/src/soc/intel/skylake/inclu... PS2, Line 35: SA_DEV_ROOT
Are these still being used anywhere? Can we get rid of it from . […]
Still used pre-ram, maybe smm.
I just realised we have smm-y += DEVICETREE_STATIC_C. How is that supposed to work? In ramstage, devicetree is mutable so smm only gets the outdated data.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34411 )
Change subject: soc/intel: Expand SA_DEV_ROOT for ramstage ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34411/2/src/soc/intel/skylake/inclu... File src/soc/intel/skylake/include/soc/pci_devs.h:
https://review.coreboot.org/c/coreboot/+/34411/2/src/soc/intel/skylake/inclu... PS2, Line 35: SA_DEV_ROOT
Still used pre-ram, maybe smm.
I see.
I just realised we have smm-y += DEVICETREE_STATIC_C. How is that supposed to work? In ramstage, devicetree is mutable so smm only gets the outdated data.
Yes, smm can only rely on the config data. Not anything that can dynamically change in ramstage.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34411 )
Change subject: soc/intel: Expand SA_DEV_ROOT for ramstage ......................................................................
Patch Set 2:
(1 comment)
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34411/2/src/soc/intel/skylake/inclu... File src/soc/intel/skylake/include/soc/pci_devs.h:
https://review.coreboot.org/c/coreboot/+/34411/2/src/soc/intel/skylake/inclu... PS2, Line 35: SA_DEV_ROOT
Still used pre-ram, maybe smm. […]
Yes, well config data is already dynamic e.g. google/nami variant_devtree_update().
Maybe config_of_XX() should always return const struct, and we require explicit casts where ramstage wants to modify config data? That way we would keep some track of the fields that would fail in smm.
TBH: I would rather remove devicetree from smm entirely but did not check why we have it.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34411 )
Change subject: soc/intel: Expand SA_DEV_ROOT for ramstage ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34411/2/src/soc/intel/skylake/inclu... File src/soc/intel/skylake/include/soc/pci_devs.h:
https://review.coreboot.org/c/coreboot/+/34411/2/src/soc/intel/skylake/inclu... PS2, Line 35: SA_DEV_ROOT
TBH: I would rather remove devicetree from smm entirely but did not check why we have it.
I know at least 1 place using it: CNL for reading heci disable config for finalize operation. Since we already have the devicetree in smm, I was planning on moving Chrome EC related configs from macros to device tree configs some day. Not sure about what other places are using it.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34411 )
Change subject: soc/intel: Expand SA_DEV_ROOT for ramstage ......................................................................
Patch Set 2:
(1 comment)
Let's discuss and document SMM expectations with CB:34257.
https://review.coreboot.org/c/coreboot/+/34411/2/src/soc/intel/skylake/inclu... File src/soc/intel/skylake/include/soc/pci_devs.h:
https://review.coreboot.org/c/coreboot/+/34411/2/src/soc/intel/skylake/inclu... PS2, Line 35: SA_DEV_ROOT
TBH: I would rather remove devicetree from smm entirely but did not check why we have it. […]
Done
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34411 )
Change subject: soc/intel: Expand SA_DEV_ROOT for ramstage ......................................................................
soc/intel: Expand SA_DEV_ROOT for ramstage
We do not want to disguise somewhat complex function calls as simple macros.
Change-Id: I298f7f9a1c6a64cfba454e919eeaedc7bb2d4801 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34411 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/mainboard/google/poppy/variants/atlas/mainboard.c M src/mainboard/google/poppy/variants/nocturne/mainboard.c M src/soc/intel/apollolake/include/soc/pci_devs.h M src/soc/intel/broadwell/finalize.c M src/soc/intel/broadwell/include/soc/pci_devs.h 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/include/soc/pci_devs.h M src/soc/intel/cannonlake/smmrelocate.c M src/soc/intel/common/block/graphics/graphics.c M src/soc/intel/denverton_ns/include/soc/pci_devs.h M src/soc/intel/icelake/include/soc/pci_devs.h M src/soc/intel/icelake/smmrelocate.c M src/soc/intel/skylake/include/soc/pci_devs.h M src/soc/intel/skylake/romstage/romstage_fsp20.c M src/soc/intel/skylake/smmrelocate.c M src/soc/intel/skylake/systemagent.c M src/soc/intel/skylake/vr_config.c 19 files changed, 74 insertions(+), 79 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/mainboard/google/poppy/variants/atlas/mainboard.c b/src/mainboard/google/poppy/variants/atlas/mainboard.c index d4db98e..9c4b2bc 100644 --- a/src/mainboard/google/poppy/variants/atlas/mainboard.c +++ b/src/mainboard/google/poppy/variants/atlas/mainboard.c @@ -25,8 +25,10 @@
static uint32_t get_pl2(void) { + struct device *igd_dev = pcidev_path_on_root(SA_DEVFN_IGD); uint16_t id; - id = pci_read_config16(SA_DEV_IGD, PCI_DEVICE_ID); + + id = pci_read_config16(igd_dev, PCI_DEVICE_ID); /* Assume we only have KLB-Y and AML-Y SKUs */ if (id == PCI_DEVICE_ID_INTEL_KBL_GT2_SULXM) return PL2_KBL; diff --git a/src/mainboard/google/poppy/variants/nocturne/mainboard.c b/src/mainboard/google/poppy/variants/nocturne/mainboard.c index f00394c..7b6b28b 100644 --- a/src/mainboard/google/poppy/variants/nocturne/mainboard.c +++ b/src/mainboard/google/poppy/variants/nocturne/mainboard.c @@ -26,8 +26,10 @@
static uint32_t get_pl2(void) { + struct device *igd_dev = pcidev_path_on_root(SA_DEVFN_IGD); uint16_t id; - id = pci_read_config16(SA_DEV_IGD, PCI_DEVICE_ID); + + id = pci_read_config16(igd_dev, PCI_DEVICE_ID); /* Assume we only have KLB-Y and AML-Y SKUs */ if (id == PCI_DEVICE_ID_INTEL_KBL_GT2_SULXM) return PL2_KBL; diff --git a/src/soc/intel/apollolake/include/soc/pci_devs.h b/src/soc/intel/apollolake/include/soc/pci_devs.h index c5eaf4c..583cc5f 100644 --- a/src/soc/intel/apollolake/include/soc/pci_devs.h +++ b/src/soc/intel/apollolake/include/soc/pci_devs.h @@ -17,35 +17,32 @@
#include <device/pci_def.h>
-#define _SA_DEVFN(slot) PCI_DEVFN(SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEVFN(slot, func) PCI_DEVFN(PCH_DEV_SLOT_ ## slot, func)
#if !defined(__SIMPLE_DEVICE__) #include <device/device.h> -#define _SA_DEV(slot) pcidev_path_on_root_debug(_SA_DEVFN(slot), __func__) #define _PCH_DEV(slot, func) pcidev_path_on_root_debug(_PCH_DEVFN(slot, func), __func__) #else -#define _SA_DEV(slot) PCI_DEV(0, SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEV(slot, func) PCI_DEV(0, PCH_DEV_SLOT_ ## slot, func) #endif
/* System Agent Devices */
#define SA_DEV_SLOT_ROOT 0x00 -#define SA_DEVFN_ROOT _SA_DEVFN(ROOT) -#define SA_DEV_ROOT _SA_DEV(ROOT) +#define SA_DEVFN_ROOT PCI_DEVFN(SA_DEV_SLOT_ROOT, 0) +#define SA_DEV_ROOT PCI_DEV(0, SA_DEV_SLOT_ROOT, 0)
#define SA_DEV_SLOT_PUNIT 0x01 -#define SA_DEVFN_PUNIT _SA_DEVFN(PUNIT) -#define SA_DEV_PUNIT _SA_DEV(PUNIT) +#define SA_DEVFN_PUNIT PCI_DEVFN(SA_DEV_SLOT_PUNIT, 0) +#define SA_DEV_PUNIT PCI_DEV(0, SA_DEV_SLOT_PUNIT, 0)
#define SA_DEV_SLOT_IGD 0x02 -#define SA_DEVFN_IGD _SA_DEVFN(IGD) -#define SA_DEV_IGD _SA_DEV(IGD) +#define SA_DEVFN_IGD PCI_DEVFN(SA_DEV_SLOT_IGD, 0) +#define SA_DEV_IGD PCI_DEV(0, SA_DEV_SLOT_IGD, 0)
#define SA_DEV_SLOT_IPU 0x03 -#define SA_DEVFN_IPU _SA_DEVFN(IPU) -#define SA_DEV_IPU _SA_DEV(IPU) +#define SA_DEVFN_IPU PCI_DEVFN(SA_DEV_SLOT_IPU, 0) +#define SA_DEV_IPU PCI_DEV(0, SA_DEV_SLOT_IPU, 0)
/* PCH Devices */
diff --git a/src/soc/intel/broadwell/finalize.c b/src/soc/intel/broadwell/finalize.c index 1adbbc8..06cc18d 100644 --- a/src/soc/intel/broadwell/finalize.c +++ b/src/soc/intel/broadwell/finalize.c @@ -96,9 +96,11 @@
static void broadwell_finalize(void *unused) { + struct device *sa_dev = pcidev_path_on_root(SA_DEVFN_ROOT); + printk(BIOS_DEBUG, "Finalizing chipset.\n");
- reg_script_run_on_dev(SA_DEV_ROOT, system_agent_finalize_script); + reg_script_run_on_dev(sa_dev, system_agent_finalize_script); reg_script_run_on_dev(PCH_DEV_LPC, pch_finalize_script);
/* Lock */ diff --git a/src/soc/intel/broadwell/include/soc/pci_devs.h b/src/soc/intel/broadwell/include/soc/pci_devs.h index ae3e08f..7ab5414 100644 --- a/src/soc/intel/broadwell/include/soc/pci_devs.h +++ b/src/soc/intel/broadwell/include/soc/pci_devs.h @@ -16,32 +16,29 @@ #ifndef _BROADWELL_PCI_DEVS_H_ #define _BROADWELL_PCI_DEVS_H_
-#define _SA_DEVFN(slot) PCI_DEVFN(SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEVFN(slot, func) PCI_DEVFN(PCH_DEV_SLOT_ ## slot, func)
#if defined(__SIMPLE_DEVICE__) -#define _SA_DEV(slot) PCI_DEV(0, SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEV(slot, func) PCI_DEV(0, PCH_DEV_SLOT_ ## slot, func) #else #include <device/device.h> #include <device/pci_def.h> -#define _SA_DEV(slot) pcidev_path_on_root_debug(_SA_DEVFN(slot), __func__) #define _PCH_DEV(slot, func) pcidev_path_on_root_debug(_PCH_DEVFN(slot, func), __func__) #endif
/* System Agent Devices */
#define SA_DEV_SLOT_ROOT 0x00 -#define SA_DEVFN_ROOT _SA_DEVFN(ROOT) -#define SA_DEV_ROOT _SA_DEV(ROOT) +#define SA_DEVFN_ROOT PCI_DEVFN(SA_DEV_SLOT_ROOT, 0) +#define SA_DEV_ROOT PCI_DEV(0, SA_DEV_SLOT_ROOT, 0)
#define SA_DEV_SLOT_IGD 0x02 -#define SA_DEVFN_IGD _SA_DEVFN(IGD) -#define SA_DEV_IGD _SA_DEV(IGD) +#define SA_DEVFN_IGD PCI_DEVFN(SA_DEV_SLOT_IGD, 0) +#define SA_DEV_IGD PCI_DEV(0, SA_DEV_SLOT_IGD, 0)
#define SA_DEV_SLOT_MINIHD 0x03 -#define SA_DEVFN_MINIHD _SA_DEVFN(MINIHD) -#define SA_DEV_MINIHD _SA_DEV(MINIHD) +#define SA_DEVFN_MINIHD PCI_DEVFN(SA_DEV_SLOT_MINIHD, 0) +#define SA_DEV_MINIHD PCI_DEV(0, SA_DEV_SLOT_MINIHD, 0)
/* PCH Devices */
diff --git a/src/soc/intel/broadwell/lpc.c b/src/soc/intel/broadwell/lpc.c index 9be4aeb..b385d6b 100644 --- a/src/soc/intel/broadwell/lpc.c +++ b/src/soc/intel/broadwell/lpc.c @@ -365,6 +365,7 @@ { u32 reg32; u16 reg16; + struct device *igd_dev = pcidev_path_on_root(SA_DEVFN_IGD);
/* DMI */ RCBA32_OR(0x2234, 0xf); @@ -388,7 +389,7 @@ RCBA32_AND_OR(0x2614, ~0x64ff0000, 0x0a206500);
/* Check for 0:2.0@0x08 >= 0x0b */ - if (pch_is_wpt() || pci_read_config8(SA_DEV_IGD, 0x8) >= 0x0b) + if (pch_is_wpt() || pci_read_config8(igd_dev, 0x8) >= 0x0b) RCBA32_OR(0x2614, (1 << 26));
RCBA32_OR(0x900, 0x0000031f); diff --git a/src/soc/intel/broadwell/smmrelocate.c b/src/soc/intel/broadwell/smmrelocate.c index 98c3c4c..9ea73b2 100644 --- a/src/soc/intel/broadwell/smmrelocate.c +++ b/src/soc/intel/broadwell/smmrelocate.c @@ -319,10 +319,11 @@
void smm_lock(void) { + struct device *sa_dev = pcidev_path_on_root(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. */ printk(BIOS_DEBUG, "Locking SMM.\n"); - pci_write_config8(SA_DEV_ROOT, SMRAM, D_LCK | G_SMRAME | C_BASE_SEG); + pci_write_config8(sa_dev, SMRAM, D_LCK | G_SMRAME | C_BASE_SEG); } diff --git a/src/soc/intel/broadwell/systemagent.c b/src/soc/intel/broadwell/systemagent.c index c6444b1..b6b5608 100644 --- a/src/soc/intel/broadwell/systemagent.c +++ b/src/soc/intel/broadwell/systemagent.c @@ -32,19 +32,22 @@
u8 systemagent_revision(void) { - return pci_read_config8(SA_DEV_ROOT, PCI_REVISION_ID); + struct device *sa_dev = pcidev_path_on_root(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); /* Bit 0 is lock bit, not part of address */ - return pci_read_config32(SA_DEV_ROOT, TOLUD) & ~1; + 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); /* Bit 0 is lock bit, not part of address */ - return pci_read_config32(SA_DEV_ROOT, BGSM) & ~1; + return pci_read_config32(sa_dev, BGSM) & ~1; }
static int get_pcie_bar(struct device *dev, unsigned int index, u32 *base, @@ -291,6 +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);
/* Read in the MAP registers and report their values. */ mc_read_map_entries(dev, &mc_values[0]); @@ -302,7 +306,7 @@ * the DPR register reports the TOP of the region, which is the same * as TSEG base. The region size is reported in MiB in bits 11:4. */ - dpr_reg = pci_read_config32(SA_DEV_ROOT, DPR); + dpr_reg = pci_read_config32(sa_dev, DPR); if (dpr_reg & DPR_EPM) { dpr_size = (dpr_reg & DPR_SIZE_MASK) << 16; printk(BIOS_INFO, "DPR SIZE: 0x%lx\n", dpr_size); diff --git a/src/soc/intel/cannonlake/include/soc/pci_devs.h b/src/soc/intel/cannonlake/include/soc/pci_devs.h index 33814b0..938b09a 100644 --- a/src/soc/intel/cannonlake/include/soc/pci_devs.h +++ b/src/soc/intel/cannonlake/include/soc/pci_devs.h @@ -19,35 +19,32 @@
#include <device/pci_def.h>
-#define _SA_DEVFN(slot) PCI_DEVFN(SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEVFN(slot, func) PCI_DEVFN(PCH_DEV_SLOT_ ## slot, func)
#if !defined(__SIMPLE_DEVICE__) #include <device/device.h> -#define _SA_DEV(slot) pcidev_path_on_root_debug(_SA_DEVFN(slot), __func__) #define _PCH_DEV(slot, func) pcidev_path_on_root_debug(_PCH_DEVFN(slot, func), __func__) #else -#define _SA_DEV(slot) PCI_DEV(0, SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEV(slot, func) PCI_DEV(0, PCH_DEV_SLOT_ ## slot, func) #endif
/* System Agent Devices */
#define SA_DEV_SLOT_ROOT 0x00 -#define SA_DEVFN_ROOT _SA_DEVFN(ROOT) -#define SA_DEV_ROOT _SA_DEV(ROOT) +#define SA_DEVFN_ROOT PCI_DEVFN(SA_DEV_SLOT_ROOT, 0) +#define SA_DEV_ROOT PCI_DEV(0, SA_DEV_SLOT_ROOT, 0)
#define SA_DEV_SLOT_IGD 0x02 -#define SA_DEVFN_IGD _SA_DEVFN(IGD) -#define SA_DEV_IGD _SA_DEV(IGD) +#define SA_DEVFN_IGD PCI_DEVFN(SA_DEV_SLOT_IGD, 0) +#define SA_DEV_IGD PCI_DEV(0, SA_DEV_SLOT_IGD, 0)
#define SA_DEV_SLOT_DSP 0x04 -#define SA_DEVFN_DSP _SA_DEVFN(DSP) -#define SA_DEV_DSP _SA_DEV(DSP) +#define SA_DEVFN_DSP PCI_DEVFN(SA_DEV_SLOT_DSP, 0) +#define SA_DEV_DSP PCI_DEV(0, SA_DEV_SLOT_DSP, 0)
#define SA_DEV_SLOT_IPU 0x05 -#define SA_DEVFN_IPU _SA_DEVFN(IPU) -#define SA_DEV_IPU _SA_DEV(IPU) +#define SA_DEVFN_IPU PCI_DEVFN(SA_DEV_SLOT_IPU, 0) +#define SA_DEV_IPU PCI_DEV(0, SA_DEV_SLOT_IPU, 0)
/* PCH Devices */ #define PCH_DEV_SLOT_THERMAL 0x12 diff --git a/src/soc/intel/cannonlake/smmrelocate.c b/src/soc/intel/cannonlake/smmrelocate.c index 2576c9c..980702f 100644 --- a/src/soc/intel/cannonlake/smmrelocate.c +++ b/src/soc/intel/cannonlake/smmrelocate.c @@ -301,11 +301,12 @@
void smm_lock(void) { + struct device *sa_dev = pcidev_path_on_root(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. */ printk(BIOS_DEBUG, "Locking SMM.\n"); - pci_write_config8(SA_DEV_ROOT, SMRAM, D_LCK | G_SMRAME | C_BASE_SEG); + pci_write_config8(sa_dev, SMRAM, D_LCK | G_SMRAME | C_BASE_SEG); } diff --git a/src/soc/intel/common/block/graphics/graphics.c b/src/soc/intel/common/block/graphics/graphics.c index 7885ad7..26e1cb8 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 = SA_DEV_IGD; + struct device *dev = pcidev_path_on_root(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 = SA_DEV_IGD; + struct device *dev = pcidev_path_on_root(SA_DEVFN_IGD);
if (is_graphics_disabled(dev)) die("IGD is disabled!"); diff --git a/src/soc/intel/denverton_ns/include/soc/pci_devs.h b/src/soc/intel/denverton_ns/include/soc/pci_devs.h index 27f9e35..2e510d9 100644 --- a/src/soc/intel/denverton_ns/include/soc/pci_devs.h +++ b/src/soc/intel/denverton_ns/include/soc/pci_devs.h @@ -21,16 +21,13 @@ /* All these devices live on bus 0 with the associated device and function */
-#define _SA_DEVFN(slot) PCI_DEVFN(SA_DEV_SLOT_##slot, 0) #define _PCH_DEVFN(slot, func) PCI_DEVFN(PCH_DEV_SLOT_##slot, func)
-#if ENV_RAMSTAGE +#if !defined(__SIMPLE_DEVICE__) #include <device/device.h> #include <device/pci_def.h> -#define _SA_DEV(slot) pcidev_path_on_root_debug(_SA_DEVFN(slot), __func__) #define _PCH_DEV(slot, func) pcidev_path_on_root_debug(_PCH_DEVFN(slot, func), __func__) #else -#define _SA_DEV(slot) PCI_DEV(0, SA_DEV_SLOT_##slot, 0) #define _PCH_DEV(slot, func) PCI_DEV(0, PCH_DEV_SLOT_##slot, func) #endif
@@ -191,8 +188,8 @@
/* TODO - New added */ #define SA_DEV_SLOT_ROOT 0x00 -#define SA_DEVFN_ROOT _SA_DEVFN(ROOT) -#define SA_DEV_ROOT _SA_DEV(ROOT) +#define SA_DEVFN_ROOT PCI_DEVFN(SA_DEV_SLOT_ROOT, 0) +#define SA_DEV_ROOT PCI_DEV(0, SA_DEV_SLOT_ROOT, 0)
#define PCH_DEV_SLOT_LPC 0x1f #define PCH_DEVFN_LPC _PCH_DEVFN(LPC, 0) diff --git a/src/soc/intel/icelake/include/soc/pci_devs.h b/src/soc/intel/icelake/include/soc/pci_devs.h index 0eddee2..a9fd4ad 100644 --- a/src/soc/intel/icelake/include/soc/pci_devs.h +++ b/src/soc/intel/icelake/include/soc/pci_devs.h @@ -18,31 +18,28 @@
#include <device/pci_def.h>
-#define _SA_DEVFN(slot) PCI_DEVFN(SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEVFN(slot, func) PCI_DEVFN(PCH_DEV_SLOT_ ## slot, func)
#if !defined(__SIMPLE_DEVICE__) #include <device/device.h> -#define _SA_DEV(slot) pcidev_path_on_root_debug(_SA_DEVFN(slot), __func__) #define _PCH_DEV(slot, func) pcidev_path_on_root_debug(_PCH_DEVFN(slot, func), __func__) #else -#define _SA_DEV(slot) PCI_DEV(0, SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEV(slot, func) PCI_DEV(0, PCH_DEV_SLOT_ ## slot, func) #endif
/* System Agent Devices */
#define SA_DEV_SLOT_ROOT 0x00 -#define SA_DEVFN_ROOT _SA_DEVFN(ROOT) -#define SA_DEV_ROOT _SA_DEV(ROOT) +#define SA_DEVFN_ROOT PCI_DEVFN(SA_DEV_SLOT_ROOT, 0) +#define SA_DEV_ROOT PCI_DEV(0, SA_DEV_SLOT_ROOT, 0)
#define SA_DEV_SLOT_IGD 0x02 -#define SA_DEVFN_IGD _SA_DEVFN(IGD) -#define SA_DEV_IGD _SA_DEV(IGD) +#define SA_DEVFN_IGD PCI_DEVFN(SA_DEV_SLOT_IGD, 0) +#define SA_DEV_IGD PCI_DEV(0, SA_DEV_SLOT_IGD, 0)
#define SA_DEV_SLOT_DSP 0x04 -#define SA_DEVFN_DSP _SA_DEVFN(DSP) -#define SA_DEV_DSP _SA_DEV(DSP) +#define SA_DEVFN_DSP PCI_DEVFN(SA_DEV_SLOT_DSP, 0) +#define SA_DEV_DSP PCI_DEV(0, SA_DEV_SLOT_DSP, 0)
/* PCH Devices */ #define PCH_DEV_SLOT_THERMAL 0x12 diff --git a/src/soc/intel/icelake/smmrelocate.c b/src/soc/intel/icelake/smmrelocate.c index 926ef63..63048eb 100644 --- a/src/soc/intel/icelake/smmrelocate.c +++ b/src/soc/intel/icelake/smmrelocate.c @@ -300,11 +300,12 @@
void smm_lock(void) { + struct device *sa_dev = pcidev_path_on_root(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. */ printk(BIOS_DEBUG, "Locking SMM.\n"); - pci_write_config8(SA_DEV_ROOT, SMRAM, D_LCK | G_SMRAME | C_BASE_SEG); + pci_write_config8(sa_dev, SMRAM, D_LCK | G_SMRAME | C_BASE_SEG); } diff --git a/src/soc/intel/skylake/include/soc/pci_devs.h b/src/soc/intel/skylake/include/soc/pci_devs.h index 0669ced..7147876 100644 --- a/src/soc/intel/skylake/include/soc/pci_devs.h +++ b/src/soc/intel/skylake/include/soc/pci_devs.h @@ -19,34 +19,26 @@
#include <device/pci_def.h>
-#define _SA_DEVFN(slot) PCI_DEVFN(SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEVFN(slot, func) PCI_DEVFN(PCH_DEV_SLOT_ ## slot, func)
#if !defined(__SIMPLE_DEVICE__) #include <device/device.h> -#define _SA_DEV(slot) pcidev_path_on_root_debug(_SA_DEVFN(slot), __func__) #define _PCH_DEV(slot, func) pcidev_path_on_root_debug(_PCH_DEVFN(slot, func), __func__) #else -#define _SA_DEV(slot) PCI_DEV(0, SA_DEV_SLOT_ ## slot, 0) #define _PCH_DEV(slot, func) PCI_DEV(0, PCH_DEV_SLOT_ ## slot, func) #endif
/* System Agent Devices */
#define SA_DEV_SLOT_ROOT 0x00 -#define SA_DEVFN_ROOT _SA_DEVFN(ROOT) -#define SA_DEV_ROOT _SA_DEV(ROOT) +#define SA_DEVFN_ROOT PCI_DEVFN(SA_DEV_SLOT_ROOT, 0) +#define SA_DEV_ROOT PCI_DEV(0, SA_DEV_SLOT_ROOT, 0)
#define SA_DEV_SLOT_PEG 0x01 -#define SA_DEVFN_PEG(func) PCI_DEVFN(SA_DEV_SLOT_PEG, func) -#define SA_DEV_PEG(func) pcidev_path_on_root_debug(SA_DEVFN_PEG(func), __func__) -#define SA_DEV_PEG0 SA_DEV_PEG(0) -#define SA_DEV_PEG1 SA_DEV_PEG(1) -#define SA_DEV_PEG2 SA_DEV_PEG(2)
#define SA_DEV_SLOT_IGD 0x02 -#define SA_DEVFN_IGD _SA_DEVFN(IGD) -#define SA_DEV_IGD _SA_DEV(IGD) +#define SA_DEVFN_IGD PCI_DEVFN(SA_DEV_SLOT_IGD, 0) +#define SA_DEV_IGD PCI_DEV(0, SA_DEV_SLOT_IGD, 0)
/* PCH Devices */
diff --git a/src/soc/intel/skylake/romstage/romstage_fsp20.c b/src/soc/intel/skylake/romstage/romstage_fsp20.c index b15fa89..bb86c63 100644 --- a/src/soc/intel/skylake/romstage/romstage_fsp20.c +++ b/src/soc/intel/skylake/romstage/romstage_fsp20.c @@ -223,7 +223,7 @@ * If PEG port is not defined in the device tree, it will be disabled * in FSP */ - dev = SA_DEV_PEG0; /* PEG 0:1:0 */ + dev = pcidev_on_root(SA_DEV_SLOT_PEG, 0); /* PEG 0:1:0 */ if (!dev || !dev->enabled) m_cfg->Peg0Enable = 0; else if (dev->enabled) { @@ -238,7 +238,7 @@ m_t_cfg->Peg0Gen3EqPh3Method = 0; }
- dev = SA_DEV_PEG1; /* PEG 0:1:1 */ + dev = pcidev_on_root(SA_DEV_SLOT_PEG, 1); /* PEG 0:1:1 */ if (!dev || !dev->enabled) m_cfg->Peg1Enable = 0; else if (dev->enabled) { @@ -250,7 +250,7 @@ m_t_cfg->Peg1Gen3EqPh3Method = 0; }
- dev = SA_DEV_PEG2; /* PEG 0:1:2 */ + dev = pcidev_on_root(SA_DEV_SLOT_PEG, 2); /* PEG 0:1:2 */ if (!dev || !dev->enabled) m_cfg->Peg2Enable = 0; else if (dev->enabled) { diff --git a/src/soc/intel/skylake/smmrelocate.c b/src/soc/intel/skylake/smmrelocate.c index 7286187..816e1a8 100644 --- a/src/soc/intel/skylake/smmrelocate.c +++ b/src/soc/intel/skylake/smmrelocate.c @@ -310,11 +310,12 @@
void smm_lock(void) { + struct device *sa_dev = pcidev_path_on_root(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. */ printk(BIOS_DEBUG, "Locking SMM.\n"); - pci_write_config8(SA_DEV_ROOT, SMRAM, D_LCK | G_SMRAME | C_BASE_SEG); + pci_write_config8(sa_dev, SMRAM, D_LCK | G_SMRAME | C_BASE_SEG); } diff --git a/src/soc/intel/skylake/systemagent.c b/src/soc/intel/skylake/systemagent.c index ea55262..410265f 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 = SA_DEV_ROOT; + struct device *const root_dev = pcidev_path_on_root(SA_DEVFN_ROOT); return root_dev && !(pci_read_config32(root_dev, CAPID0_A) & VTD_DISABLE); } @@ -42,7 +42,8 @@ */ void soc_add_fixed_mmio_resources(struct device *dev, int *index) { - struct device *const igd_dev = SA_DEV_IGD; + struct device *const igd_dev = pcidev_path_on_root(SA_DEVFN_IGD); + static const struct sa_mmio_descriptor soc_fixed_resources[] = { { PCIEXBAR, CONFIG_MMCONF_BASE_ADDRESS, CONFIG_SA_PCIEX_LENGTH, "PCIEXBAR" }, diff --git a/src/soc/intel/skylake/vr_config.c b/src/soc/intel/skylake/vr_config.c index 905154e..c83e18d 100644 --- a/src/soc/intel/skylake/vr_config.c +++ b/src/soc/intel/skylake/vr_config.c @@ -174,17 +174,19 @@
static int get_kbl_sku(void) { + struct device *sa_dev = pcidev_path_on_root(SA_DEVFN_ROOT); static int sku = -1; uint16_t id;
if (sku != -1) return sku;
- id = get_dev_id(SA_DEV_ROOT); + id = get_dev_id(sa_dev); if (id == PCI_DEVICE_ID_INTEL_KBL_U_R) sku = KBL_R_SKU; else if (id == PCI_DEVICE_ID_INTEL_KBL_ID_Y) { - id = get_dev_id(SA_DEV_IGD); + struct device *igd_dev = pcidev_path_on_root(SA_DEVFN_IGD); + id = get_dev_id(igd_dev); if (id == PCI_DEVICE_ID_INTEL_AML_GT2_ULX) sku = AML_Y_SKU; else
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34411 )
Change subject: soc/intel: Expand SA_DEV_ROOT for ramstage ......................................................................
Patch Set 3:
Missed a weird SA_DEV_ROOT in `mainboard/purism/librem_skl/hda_verb.c` maybe more? didn't check.
The bad thing: The compiler doesn't warn about the conversion from integer to pointer without a cast, because SA_DEV_ROOT is literally `0` and there seems to be an exception for that conversion.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34411 )
Change subject: soc/intel: Expand SA_DEV_ROOT for ramstage ......................................................................
Patch Set 3:
Patch Set 3:
Missed a weird SA_DEV_ROOT in `mainboard/purism/librem_skl/hda_verb.c` maybe more? didn't check.
The bad thing: The compiler doesn't warn about the conversion from integer to pointer without a cast, because SA_DEV_ROOT is literally `0` and there seems to be an exception for that conversion.
CB:34544
-
Furquan Shaikh (Code Review) -
Kyösti Mälkki (Code Review)
-
Martin Roth (Code Review)
-
Nico Huber (Code Review)