Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58333 )
Change subject: pci_mmio_cfg: Always use pci_s_* functions ......................................................................
pci_mmio_cfg: Always use pci_s_* functions
When MMIO functions are available, the pci_s_* functions do exactly the same thing. Drop the redundant pci_mmio_* versions.
Change-Id: I1043cbb9a1823ef94bcbb42169cb7edf282f560b Signed-off-by: Nico Huber nico.huber@secunet.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/58333 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Shelley Chen shchen@google.com Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/include/device/pci_mmio_cfg.h M src/mainboard/intel/cedarisland_crb/bootblock.c M src/mainboard/ocp/tiogapass/bootblock.c M src/soc/intel/xeon_sp/bootblock.c M src/soc/intel/xeon_sp/nb_acpi.c M src/soc/intel/xeon_sp/skx/include/soc/pci_devs.h M src/soc/intel/xeon_sp/skx/soc_util.c 7 files changed, 30 insertions(+), 65 deletions(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, but someone else must approve Shelley Chen: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve Tim Wawrzynczak: Looks good to me, but someone else must approve
diff --git a/src/include/device/pci_mmio_cfg.h b/src/include/device/pci_mmio_cfg.h index 557adcb..81faa9c 100644 --- a/src/include/device/pci_mmio_cfg.h +++ b/src/include/device/pci_mmio_cfg.h @@ -47,38 +47,44 @@
#endif
+/* + * Avoid name collisions as different stages have different signature + * for these functions. The _s_ stands for simple, fundamental IO or + * MMIO variant. + */ + static __always_inline -uint8_t pci_mmio_read_config8(pci_devfn_t dev, uint16_t reg) +uint8_t pci_s_read_config8(pci_devfn_t dev, uint16_t reg) { return pcicfg(dev)->reg8[reg]; }
static __always_inline -uint16_t pci_mmio_read_config16(pci_devfn_t dev, uint16_t reg) +uint16_t pci_s_read_config16(pci_devfn_t dev, uint16_t reg) { return pcicfg(dev)->reg16[reg / sizeof(uint16_t)]; }
static __always_inline -uint32_t pci_mmio_read_config32(pci_devfn_t dev, uint16_t reg) +uint32_t pci_s_read_config32(pci_devfn_t dev, uint16_t reg) { return pcicfg(dev)->reg32[reg / sizeof(uint32_t)]; }
static __always_inline -void pci_mmio_write_config8(pci_devfn_t dev, uint16_t reg, uint8_t value) +void pci_s_write_config8(pci_devfn_t dev, uint16_t reg, uint8_t value) { pcicfg(dev)->reg8[reg] = value; }
static __always_inline -void pci_mmio_write_config16(pci_devfn_t dev, uint16_t reg, uint16_t value) +void pci_s_write_config16(pci_devfn_t dev, uint16_t reg, uint16_t value) { pcicfg(dev)->reg16[reg / sizeof(uint16_t)] = value; }
static __always_inline -void pci_mmio_write_config32(pci_devfn_t dev, uint16_t reg, uint32_t value) +void pci_s_write_config32(pci_devfn_t dev, uint16_t reg, uint32_t value) { pcicfg(dev)->reg32[reg / sizeof(uint32_t)] = value; } @@ -107,45 +113,4 @@ return (uint32_t *)&pcicfg(dev)->reg32[reg / sizeof(uint32_t)]; }
-/* Avoid name collisions as different stages have different signature - * for these functions. The _s_ stands for simple, fundamental IO or - * MMIO variant. - */ - -static __always_inline -uint8_t pci_s_read_config8(pci_devfn_t dev, uint16_t reg) -{ - return pci_mmio_read_config8(dev, reg); -} - -static __always_inline -uint16_t pci_s_read_config16(pci_devfn_t dev, uint16_t reg) -{ - return pci_mmio_read_config16(dev, reg); -} - -static __always_inline -uint32_t pci_s_read_config32(pci_devfn_t dev, uint16_t reg) -{ - return pci_mmio_read_config32(dev, reg); -} - -static __always_inline -void pci_s_write_config8(pci_devfn_t dev, uint16_t reg, uint8_t value) -{ - pci_mmio_write_config8(dev, reg, value); -} - -static __always_inline -void pci_s_write_config16(pci_devfn_t dev, uint16_t reg, uint16_t value) -{ - pci_mmio_write_config16(dev, reg, value); -} - -static __always_inline -void pci_s_write_config32(pci_devfn_t dev, uint16_t reg, uint32_t value) -{ - pci_mmio_write_config32(dev, reg, value); -} - #endif /* _PCI_MMIO_CFG_H */ diff --git a/src/mainboard/intel/cedarisland_crb/bootblock.c b/src/mainboard/intel/cedarisland_crb/bootblock.c index e51752b..ae8212a 100644 --- a/src/mainboard/intel/cedarisland_crb/bootblock.c +++ b/src/mainboard/intel/cedarisland_crb/bootblock.c @@ -21,7 +21,7 @@ pcr_write32(PID_DMI, 0x2774, 1);
/* Decode for SuperIO (0x2e) and COM1 (0x3f8) */ - pci_mmio_write_config32(PCH_DEV_LPC, 0x80, (1 << 28) | (1 << 16)); + pci_s_write_config32(PCH_DEV_LPC, 0x80, (1 << 28) | (1 << 16));
const pnp_devfn_t serial_dev = PNP_DEV(0x2e, AST2400_SUART1); aspeed_enable_serial(serial_dev, CONFIG_TTYS0_BASE); diff --git a/src/mainboard/ocp/tiogapass/bootblock.c b/src/mainboard/ocp/tiogapass/bootblock.c index b5c2aa1..6bcbfcc 100644 --- a/src/mainboard/ocp/tiogapass/bootblock.c +++ b/src/mainboard/ocp/tiogapass/bootblock.c @@ -30,7 +30,7 @@ pcr_or32(PID_DMI, PCR_DMI_LPCIOE, (1 << 0) | (1 << 1));
/* Enable com1 (0x3f8), com2 (02f8) and superio (0x2e) */ - pci_mmio_write_config32(PCH_DEV_LPC, 0x80, + pci_s_write_config32(PCH_DEV_LPC, 0x80, (1 << 28) | (1 << 16) | (1 << 17) | (0 << 0) | (1 << 4)); }
diff --git a/src/soc/intel/xeon_sp/bootblock.c b/src/soc/intel/xeon_sp/bootblock.c index 1d706fe..5ea09ac 100644 --- a/src/soc/intel/xeon_sp/bootblock.c +++ b/src/soc/intel/xeon_sp/bootblock.c @@ -56,9 +56,9 @@ pch_enable_lpc();
/* Set up P2SB BAR. This is needed for PCR to work */ - uint8_t p2sb_cmd = pci_mmio_read_config8(PCH_DEV_P2SB, PCI_COMMAND); - pci_mmio_write_config8(PCH_DEV_P2SB, PCI_COMMAND, p2sb_cmd | PCI_COMMAND_MEMORY); - pci_mmio_write_config32(PCH_DEV_P2SB, PCI_BASE_ADDRESS_0, CONFIG_PCR_BASE_ADDRESS); + uint8_t p2sb_cmd = pci_s_read_config8(PCH_DEV_P2SB, PCI_COMMAND); + pci_s_write_config8(PCH_DEV_P2SB, PCI_COMMAND, p2sb_cmd | PCI_COMMAND_MEMORY); + pci_s_write_config32(PCH_DEV_P2SB, PCI_BASE_ADDRESS_0, CONFIG_PCR_BASE_ADDRESS); }
void bootblock_soc_init(void) diff --git a/src/soc/intel/xeon_sp/nb_acpi.c b/src/soc/intel/xeon_sp/nb_acpi.c index 0c1c5ab..9443966 100644 --- a/src/soc/intel/xeon_sp/nb_acpi.c +++ b/src/soc/intel/xeon_sp/nb_acpi.c @@ -160,7 +160,7 @@ const uint32_t dev = iio_resource->PcieInfo.PortInfo[port].Device; const uint32_t func = iio_resource->PcieInfo.PortInfo[port].Function;
- const uint32_t id = pci_mmio_read_config32(PCI_DEV(bus, dev, func), + const uint32_t id = pci_s_read_config32(PCI_DEV(bus, dev, func), PCI_VENDOR_ID); if (id == 0xffffffff) return 0; diff --git a/src/soc/intel/xeon_sp/skx/include/soc/pci_devs.h b/src/soc/intel/xeon_sp/skx/include/soc/pci_devs.h index 01e86e1..4b46ec8 100644 --- a/src/soc/intel/xeon_sp/skx/include/soc/pci_devs.h +++ b/src/soc/intel/xeon_sp/skx/include/soc/pci_devs.h @@ -13,13 +13,13 @@ printk(BIOS_SPEW, "%s%x:%x:%x reg: %s (0x%x), data: 0x%x\n", \ fmt, ((uint32_t)dev >> 20) & 0xfff, ((uint32_t)dev >> 15) & 0x1f, \ ((uint32_t)dev >> 12) & 0x07, \ - #reg, reg, pci_mmio_read_config32(dev, reg)) + #reg, reg, pci_s_read_config32(dev, reg))
#define dump_csr64(fmt, dev, reg) \ printk(BIOS_SPEW, "%s%x:%x:%x reg: %s (0x%x), data: 0x%x%x\n", \ fmt, ((uint32_t)dev >> 20) & 0xfff, ((uint32_t)dev >> 15) & 0x1f, \ ((uint32_t)dev >> 12) & 0x07, #reg, reg, \ - pci_mmio_read_config32(dev, reg+4), pci_mmio_read_config32(dev, reg)) + pci_s_read_config32(dev, reg+4), pci_s_read_config32(dev, reg))
#define SAD_ALL_DEV 29 #define SAD_ALL_FUNC 0 diff --git a/src/soc/intel/xeon_sp/skx/soc_util.c b/src/soc/intel/xeon_sp/skx/soc_util.c index b903249..cc8db64 100644 --- a/src/soc/intel/xeon_sp/skx/soc_util.c +++ b/src/soc/intel/xeon_sp/skx/soc_util.c @@ -79,11 +79,11 @@
/* configure PCU_CR0_FUN csrs */ pci_devfn_t cr0_dev = PCI_DEV(bus, PCU_DEV, PCU_CR0_FUN); - data = pci_mmio_read_config32(cr0_dev, PCU_CR0_P_STATE_LIMITS); + data = pci_s_read_config32(cr0_dev, PCU_CR0_P_STATE_LIMITS); data |= P_STATE_LIMITS_LOCK; - pci_mmio_write_config32(cr0_dev, PCU_CR0_P_STATE_LIMITS, data); + pci_s_write_config32(cr0_dev, PCU_CR0_P_STATE_LIMITS, data);
- plat_info = pci_mmio_read_config32(cr0_dev, PCU_CR0_PLATFORM_INFO); + plat_info = pci_s_read_config32(cr0_dev, PCU_CR0_PLATFORM_INFO); dump_csr64("", cr0_dev, PCU_CR0_PLATFORM_INFO); max_min_turbo_limit_ratio = (plat_info & MAX_NON_TURBO_LIM_RATIO_MASK) >> @@ -94,29 +94,29 @@ /* configure PCU_CR1_FUN csrs */ pci_devfn_t cr1_dev = PCI_DEV(bus, PCU_DEV, PCU_CR1_FUN);
- data = pci_mmio_read_config32(cr1_dev, PCU_CR1_SAPMCTL); + data = pci_s_read_config32(cr1_dev, PCU_CR1_SAPMCTL); /* clear bits 27:31 - FSP sets this with 0x7 which needs to be cleared */ data &= 0x0fffffff; data |= SAPMCTL_LOCK_MASK; - pci_mmio_write_config32(cr1_dev, PCU_CR1_SAPMCTL, data); + pci_s_write_config32(cr1_dev, PCU_CR1_SAPMCTL, data);
/* configure PCU_CR1_FUN csrs */ pci_devfn_t cr2_dev = PCI_DEV(bus, PCU_DEV, PCU_CR2_FUN);
data = PCIE_IN_PKGCSTATE_L1_MASK; - pci_mmio_write_config32(cr2_dev, PCU_CR2_PKG_CST_ENTRY_CRITERIA_MASK, data); + pci_s_write_config32(cr2_dev, PCU_CR2_PKG_CST_ENTRY_CRITERIA_MASK, data);
data = KTI_IN_PKGCSTATE_L1_MASK; - pci_mmio_write_config32(cr2_dev, PCU_CR2_PKG_CST_ENTRY_CRITERIA_MASK2, data); + pci_s_write_config32(cr2_dev, PCU_CR2_PKG_CST_ENTRY_CRITERIA_MASK2, data);
data = PROCHOT_RATIO; printk(BIOS_SPEW, "PCU_CR2_PROCHOT_RESPONSE_RATIO_REG data: 0x%x\n", data); - pci_mmio_write_config32(cr2_dev, PCU_CR2_PROCHOT_RESPONSE_RATIO_REG, data); + pci_s_write_config32(cr2_dev, PCU_CR2_PROCHOT_RESPONSE_RATIO_REG, data); dump_csr("", cr2_dev, PCU_CR2_PROCHOT_RESPONSE_RATIO_REG);
- data = pci_mmio_read_config32(cr2_dev, PCU_CR2_DYNAMIC_PERF_POWER_CTL); + data = pci_s_read_config32(cr2_dev, PCU_CR2_DYNAMIC_PERF_POWER_CTL); data |= UNOCRE_PLIMIT_OVERRIDE_SHIFT; - pci_mmio_write_config32(cr2_dev, PCU_CR2_DYNAMIC_PERF_POWER_CTL, data); + pci_s_write_config32(cr2_dev, PCU_CR2_DYNAMIC_PERF_POWER_CTL, data); } }
5 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one.
-
Nico Huber (Code Review)