Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45717 )
Change subject: soc/intel/broadwell: Use get_{pmbase,gpiobase} ......................................................................
soc/intel/broadwell: Use get_{pmbase,gpiobase}
This is to align Broadwell and Lynx Point.
Change-Id: I9facaec2967616b07b537a8e79b915d6f04948a7 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/broadwell/pmutil.c M src/soc/intel/broadwell/smihandler.c 2 files changed, 47 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/45717/1
diff --git a/src/soc/intel/broadwell/pmutil.c b/src/soc/intel/broadwell/pmutil.c index c96ed18..e63a981 100644 --- a/src/soc/intel/broadwell/pmutil.c +++ b/src/soc/intel/broadwell/pmutil.c @@ -19,6 +19,12 @@ #include <soc/pm.h> #include <soc/gpio.h> #include <security/vboot/vbnv.h> +#include <stdint.h> + +static inline uint16_t get_gpiobase(void) +{ + return GPIO_BASE_ADDRESS; +}
/* Print status bits with descriptive names */ static void print_status_bits(u32 status, const char *bit_names[]) @@ -59,17 +65,17 @@ /* Enable events in PM1 control register */ void enable_pm1_control(u32 mask) { - u32 pm1_cnt = inl(ACPI_BASE_ADDRESS + PM1_CNT); + u32 pm1_cnt = inl(get_pmbase() + PM1_CNT); pm1_cnt |= mask; - outl(pm1_cnt, ACPI_BASE_ADDRESS + PM1_CNT); + outl(pm1_cnt, get_pmbase() + PM1_CNT); }
/* Disable events in PM1 control register */ void disable_pm1_control(u32 mask) { - u32 pm1_cnt = inl(ACPI_BASE_ADDRESS + PM1_CNT); + u32 pm1_cnt = inl(get_pmbase() + PM1_CNT); pm1_cnt &= ~mask; - outl(pm1_cnt, ACPI_BASE_ADDRESS + PM1_CNT); + outl(pm1_cnt, get_pmbase() + PM1_CNT); }
/* @@ -79,8 +85,8 @@ /* Clear and return PM1 status register */ static u16 reset_pm1_status(void) { - u16 pm1_sts = inw(ACPI_BASE_ADDRESS + PM1_STS); - outw(pm1_sts, ACPI_BASE_ADDRESS + PM1_STS); + u16 pm1_sts = inw(get_pmbase() + PM1_STS); + outw(pm1_sts, get_pmbase() + PM1_STS); return pm1_sts; }
@@ -117,7 +123,7 @@ /* Set the PM1 register to events */ void enable_pm1(u16 events) { - outw(events, ACPI_BASE_ADDRESS + PM1_EN); + outw(events, get_pmbase() + PM1_EN); }
/* @@ -127,8 +133,8 @@ /* Clear and return SMI status register */ static u32 reset_smi_status(void) { - u32 smi_sts = inl(ACPI_BASE_ADDRESS + SMI_STS); - outl(smi_sts, ACPI_BASE_ADDRESS + SMI_STS); + u32 smi_sts = inl(get_pmbase() + SMI_STS); + outl(smi_sts, get_pmbase() + SMI_STS); return smi_sts; }
@@ -177,17 +183,17 @@ /* Enable SMI event */ void enable_smi(u32 mask) { - u32 smi_en = inl(ACPI_BASE_ADDRESS + SMI_EN); + u32 smi_en = inl(get_pmbase() + SMI_EN); smi_en |= mask; - outl(smi_en, ACPI_BASE_ADDRESS + SMI_EN); + outl(smi_en, get_pmbase() + SMI_EN); }
/* Disable SMI event */ void disable_smi(u32 mask) { - u32 smi_en = inl(ACPI_BASE_ADDRESS + SMI_EN); + u32 smi_en = inl(get_pmbase() + SMI_EN); smi_en &= ~mask; - outl(smi_en, ACPI_BASE_ADDRESS + SMI_EN); + outl(smi_en, get_pmbase() + SMI_EN); }
/* @@ -200,9 +206,9 @@ u32 alt_sts, alt_en;
/* Low Power variant moves this to GPIO region as dword */ - alt_sts = inl(GPIO_BASE_ADDRESS + GPIO_ALT_GPI_SMI_STS); - outl(alt_sts, GPIO_BASE_ADDRESS + GPIO_ALT_GPI_SMI_STS); - alt_en = inl(GPIO_BASE_ADDRESS + GPIO_ALT_GPI_SMI_EN); + alt_sts = inl(get_gpiobase() + GPIO_ALT_GPI_SMI_STS); + outl(alt_sts, get_gpiobase() + GPIO_ALT_GPI_SMI_STS); + alt_en = inl(get_gpiobase() + GPIO_ALT_GPI_SMI_EN);
/* Only report enabled events */ return alt_sts & alt_en; @@ -235,9 +241,9 @@ { u32 alt_en;
- alt_en = inl(GPIO_BASE_ADDRESS + GPIO_ALT_GPI_SMI_EN); + alt_en = inl(get_gpiobase() + GPIO_ALT_GPI_SMI_EN); alt_en |= mask; - outl(alt_en, GPIO_BASE_ADDRESS + GPIO_ALT_GPI_SMI_EN); + outl(alt_en, get_gpiobase() + GPIO_ALT_GPI_SMI_EN); }
/* @@ -247,9 +253,9 @@ /* Clear TCO status and return events that are enabled and active */ static u32 reset_tco_status(void) { - u32 tcobase = ACPI_BASE_ADDRESS + 0x60; + u32 tcobase = get_pmbase() + 0x60; u32 tco_sts = inl(tcobase + 0x04); - u32 tco_en = inl(ACPI_BASE_ADDRESS + 0x68); + u32 tco_en = inl(get_pmbase() + 0x68);
/* Don't clear BOOT_STS before SECOND_TO_STS */ outl(tco_sts & ~(1 << 18), tcobase + 0x04); @@ -301,7 +307,7 @@ void enable_tco_sci(void) { /* Clear pending events */ - outl(ACPI_BASE_ADDRESS + GPE0_STS(3), TCOSCI_STS); + outl(get_pmbase() + GPE0_STS(3), TCOSCI_STS);
/* Enable TCO SCI events */ enable_gpe(TCOSCI_EN); @@ -314,10 +320,10 @@ /* Clear a GPE0 status and return events that are enabled and active */ static u32 reset_gpe_status(u16 sts_reg, u16 en_reg) { - u32 gpe0_sts = inl(ACPI_BASE_ADDRESS + sts_reg); - u32 gpe0_en = inl(ACPI_BASE_ADDRESS + en_reg); + u32 gpe0_sts = inl(get_pmbase() + sts_reg); + u32 gpe0_en = inl(get_pmbase() + en_reg);
- outl(gpe0_sts, ACPI_BASE_ADDRESS + sts_reg); + outl(gpe0_sts, get_pmbase() + sts_reg);
/* Only report enabled events */ return gpe0_sts & gpe0_en; @@ -376,10 +382,12 @@ /* Enable all requested GPE */ void enable_all_gpe(u32 set1, u32 set2, u32 set3, u32 set4) { - outl(set1, ACPI_BASE_ADDRESS + GPE0_EN(GPE_31_0)); - outl(set2, ACPI_BASE_ADDRESS + GPE0_EN(GPE_63_32)); - outl(set3, ACPI_BASE_ADDRESS + GPE0_EN(GPE_94_64)); - outl(set4, ACPI_BASE_ADDRESS + GPE0_EN(GPE_STD)); + u16 pmbase = get_pmbase(); + + outl(set1, pmbase + GPE0_EN(GPE_31_0)); + outl(set2, pmbase + GPE0_EN(GPE_63_32)); + outl(set3, pmbase + GPE0_EN(GPE_94_64)); + outl(set4, pmbase + GPE0_EN(GPE_STD)); }
/* Disable all GPE */ @@ -391,17 +399,17 @@ /* Enable a standard GPE */ void enable_gpe(u32 mask) { - u32 gpe0_en = inl(ACPI_BASE_ADDRESS + GPE0_EN(GPE_STD)); + u32 gpe0_en = inl(get_pmbase() + GPE0_EN(GPE_STD)); gpe0_en |= mask; - outl(gpe0_en, ACPI_BASE_ADDRESS + GPE0_EN(GPE_STD)); + outl(gpe0_en, get_pmbase() + GPE0_EN(GPE_STD)); }
/* Disable a standard GPE */ void disable_gpe(u32 mask) { - u32 gpe0_en = inl(ACPI_BASE_ADDRESS + GPE0_EN(GPE_STD)); + u32 gpe0_en = inl(get_pmbase() + GPE0_EN(GPE_STD)); gpe0_en &= ~mask; - outl(gpe0_en, ACPI_BASE_ADDRESS + GPE0_EN(GPE_STD)); + outl(gpe0_en, get_pmbase() + GPE0_EN(GPE_STD)); }
int acpi_sci_irq(void) @@ -434,10 +442,10 @@
int platform_is_resuming(void) { - if (!(inw(ACPI_BASE_ADDRESS + PM1_STS) & WAK_STS)) + if (!(inw(get_pmbase() + PM1_STS) & WAK_STS)) return 0;
- return acpi_sleep_from_pm1(inl(ACPI_BASE_ADDRESS + PM1_CNT)) == ACPI_S3; + return acpi_sleep_from_pm1(inl(get_pmbase() + PM1_CNT)) == ACPI_S3; }
/* STM Support */ diff --git a/src/soc/intel/broadwell/smihandler.c b/src/soc/intel/broadwell/smihandler.c index 4a12d78..fd5d452 100644 --- a/src/soc/intel/broadwell/smihandler.c +++ b/src/soc/intel/broadwell/smihandler.c @@ -133,6 +133,7 @@ u32 reg32; u8 slp_typ; u8 s5pwr = CONFIG_MAINBOARD_POWER_FAILURE_STATE; + u16 pmbase = get_pmbase();
/* save and recover RTC port values */ u8 tmp70, tmp72; @@ -146,7 +147,7 @@ disable_smi(SLP_SMI_EN);
/* Figure out SLP_TYP */ - reg32 = inl(ACPI_BASE_ADDRESS + PM1_CNT); + reg32 = inl(pmbase + PM1_CNT); printk(BIOS_SPEW, "SMI#: SLP = 0x%08x\n", reg32); slp_typ = acpi_sleep_from_pm1(reg32);
@@ -225,7 +226,7 @@ * the line above. However, if we entered sleep state S1 and wake * up again, we will continue to execute code in this function. */ - reg32 = inl(ACPI_BASE_ADDRESS + PM1_CNT); + reg32 = inl(pmbase + PM1_CNT); if (reg32 & SCI_EN) { /* The OS is not an ACPI OS, so we set the state to S0 */ disable_pm1_control(SLP_EN | SLP_TYP); @@ -393,7 +394,7 @@
static void southbridge_smi_mc(void) { - u32 reg32 = inl(ACPI_BASE_ADDRESS + SMI_EN); + u32 reg32 = inl(get_pmbase() + SMI_EN);
/* Are microcontroller SMIs enabled? */ if ((reg32 & MCSMI_EN) == 0) @@ -436,7 +437,7 @@
static void southbridge_smi_periodic(void) { - u32 reg32 = inl(ACPI_BASE_ADDRESS + SMI_EN); + u32 reg32 = inl(get_pmbase() + SMI_EN);
/* Are periodic SMIs enabled? */ if ((reg32 & PERIODIC_EN) == 0)
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45717 )
Change subject: soc/intel/broadwell: Use get_{pmbase,gpiobase} ......................................................................
Patch Set 2: Code-Review+2
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45717
to look at the new patch set (#5).
Change subject: soc/intel/broadwell: Use get_{pmbase,gpiobase} ......................................................................
soc/intel/broadwell: Use get_{pmbase,gpiobase}
This is to align Broadwell and Lynx Point.
Change-Id: I9facaec2967616b07b537a8e79b915d6f04948a7 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/broadwell/pmutil.c M src/soc/intel/broadwell/smihandler.c 2 files changed, 47 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/45717/5
Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45717 )
Change subject: soc/intel/broadwell: Use get_{pmbase,gpiobase} ......................................................................
soc/intel/broadwell: Use get_{pmbase,gpiobase}
This is to align Broadwell and Lynx Point.
Change-Id: I9facaec2967616b07b537a8e79b915d6f04948a7 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/45717 Reviewed-by: Patrick Rudolph siro@das-labor.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/broadwell/pmutil.c M src/soc/intel/broadwell/smihandler.c 2 files changed, 47 insertions(+), 38 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Rudolph: Looks good to me, approved
diff --git a/src/soc/intel/broadwell/pmutil.c b/src/soc/intel/broadwell/pmutil.c index c96ed18..e63a981 100644 --- a/src/soc/intel/broadwell/pmutil.c +++ b/src/soc/intel/broadwell/pmutil.c @@ -19,6 +19,12 @@ #include <soc/pm.h> #include <soc/gpio.h> #include <security/vboot/vbnv.h> +#include <stdint.h> + +static inline uint16_t get_gpiobase(void) +{ + return GPIO_BASE_ADDRESS; +}
/* Print status bits with descriptive names */ static void print_status_bits(u32 status, const char *bit_names[]) @@ -59,17 +65,17 @@ /* Enable events in PM1 control register */ void enable_pm1_control(u32 mask) { - u32 pm1_cnt = inl(ACPI_BASE_ADDRESS + PM1_CNT); + u32 pm1_cnt = inl(get_pmbase() + PM1_CNT); pm1_cnt |= mask; - outl(pm1_cnt, ACPI_BASE_ADDRESS + PM1_CNT); + outl(pm1_cnt, get_pmbase() + PM1_CNT); }
/* Disable events in PM1 control register */ void disable_pm1_control(u32 mask) { - u32 pm1_cnt = inl(ACPI_BASE_ADDRESS + PM1_CNT); + u32 pm1_cnt = inl(get_pmbase() + PM1_CNT); pm1_cnt &= ~mask; - outl(pm1_cnt, ACPI_BASE_ADDRESS + PM1_CNT); + outl(pm1_cnt, get_pmbase() + PM1_CNT); }
/* @@ -79,8 +85,8 @@ /* Clear and return PM1 status register */ static u16 reset_pm1_status(void) { - u16 pm1_sts = inw(ACPI_BASE_ADDRESS + PM1_STS); - outw(pm1_sts, ACPI_BASE_ADDRESS + PM1_STS); + u16 pm1_sts = inw(get_pmbase() + PM1_STS); + outw(pm1_sts, get_pmbase() + PM1_STS); return pm1_sts; }
@@ -117,7 +123,7 @@ /* Set the PM1 register to events */ void enable_pm1(u16 events) { - outw(events, ACPI_BASE_ADDRESS + PM1_EN); + outw(events, get_pmbase() + PM1_EN); }
/* @@ -127,8 +133,8 @@ /* Clear and return SMI status register */ static u32 reset_smi_status(void) { - u32 smi_sts = inl(ACPI_BASE_ADDRESS + SMI_STS); - outl(smi_sts, ACPI_BASE_ADDRESS + SMI_STS); + u32 smi_sts = inl(get_pmbase() + SMI_STS); + outl(smi_sts, get_pmbase() + SMI_STS); return smi_sts; }
@@ -177,17 +183,17 @@ /* Enable SMI event */ void enable_smi(u32 mask) { - u32 smi_en = inl(ACPI_BASE_ADDRESS + SMI_EN); + u32 smi_en = inl(get_pmbase() + SMI_EN); smi_en |= mask; - outl(smi_en, ACPI_BASE_ADDRESS + SMI_EN); + outl(smi_en, get_pmbase() + SMI_EN); }
/* Disable SMI event */ void disable_smi(u32 mask) { - u32 smi_en = inl(ACPI_BASE_ADDRESS + SMI_EN); + u32 smi_en = inl(get_pmbase() + SMI_EN); smi_en &= ~mask; - outl(smi_en, ACPI_BASE_ADDRESS + SMI_EN); + outl(smi_en, get_pmbase() + SMI_EN); }
/* @@ -200,9 +206,9 @@ u32 alt_sts, alt_en;
/* Low Power variant moves this to GPIO region as dword */ - alt_sts = inl(GPIO_BASE_ADDRESS + GPIO_ALT_GPI_SMI_STS); - outl(alt_sts, GPIO_BASE_ADDRESS + GPIO_ALT_GPI_SMI_STS); - alt_en = inl(GPIO_BASE_ADDRESS + GPIO_ALT_GPI_SMI_EN); + alt_sts = inl(get_gpiobase() + GPIO_ALT_GPI_SMI_STS); + outl(alt_sts, get_gpiobase() + GPIO_ALT_GPI_SMI_STS); + alt_en = inl(get_gpiobase() + GPIO_ALT_GPI_SMI_EN);
/* Only report enabled events */ return alt_sts & alt_en; @@ -235,9 +241,9 @@ { u32 alt_en;
- alt_en = inl(GPIO_BASE_ADDRESS + GPIO_ALT_GPI_SMI_EN); + alt_en = inl(get_gpiobase() + GPIO_ALT_GPI_SMI_EN); alt_en |= mask; - outl(alt_en, GPIO_BASE_ADDRESS + GPIO_ALT_GPI_SMI_EN); + outl(alt_en, get_gpiobase() + GPIO_ALT_GPI_SMI_EN); }
/* @@ -247,9 +253,9 @@ /* Clear TCO status and return events that are enabled and active */ static u32 reset_tco_status(void) { - u32 tcobase = ACPI_BASE_ADDRESS + 0x60; + u32 tcobase = get_pmbase() + 0x60; u32 tco_sts = inl(tcobase + 0x04); - u32 tco_en = inl(ACPI_BASE_ADDRESS + 0x68); + u32 tco_en = inl(get_pmbase() + 0x68);
/* Don't clear BOOT_STS before SECOND_TO_STS */ outl(tco_sts & ~(1 << 18), tcobase + 0x04); @@ -301,7 +307,7 @@ void enable_tco_sci(void) { /* Clear pending events */ - outl(ACPI_BASE_ADDRESS + GPE0_STS(3), TCOSCI_STS); + outl(get_pmbase() + GPE0_STS(3), TCOSCI_STS);
/* Enable TCO SCI events */ enable_gpe(TCOSCI_EN); @@ -314,10 +320,10 @@ /* Clear a GPE0 status and return events that are enabled and active */ static u32 reset_gpe_status(u16 sts_reg, u16 en_reg) { - u32 gpe0_sts = inl(ACPI_BASE_ADDRESS + sts_reg); - u32 gpe0_en = inl(ACPI_BASE_ADDRESS + en_reg); + u32 gpe0_sts = inl(get_pmbase() + sts_reg); + u32 gpe0_en = inl(get_pmbase() + en_reg);
- outl(gpe0_sts, ACPI_BASE_ADDRESS + sts_reg); + outl(gpe0_sts, get_pmbase() + sts_reg);
/* Only report enabled events */ return gpe0_sts & gpe0_en; @@ -376,10 +382,12 @@ /* Enable all requested GPE */ void enable_all_gpe(u32 set1, u32 set2, u32 set3, u32 set4) { - outl(set1, ACPI_BASE_ADDRESS + GPE0_EN(GPE_31_0)); - outl(set2, ACPI_BASE_ADDRESS + GPE0_EN(GPE_63_32)); - outl(set3, ACPI_BASE_ADDRESS + GPE0_EN(GPE_94_64)); - outl(set4, ACPI_BASE_ADDRESS + GPE0_EN(GPE_STD)); + u16 pmbase = get_pmbase(); + + outl(set1, pmbase + GPE0_EN(GPE_31_0)); + outl(set2, pmbase + GPE0_EN(GPE_63_32)); + outl(set3, pmbase + GPE0_EN(GPE_94_64)); + outl(set4, pmbase + GPE0_EN(GPE_STD)); }
/* Disable all GPE */ @@ -391,17 +399,17 @@ /* Enable a standard GPE */ void enable_gpe(u32 mask) { - u32 gpe0_en = inl(ACPI_BASE_ADDRESS + GPE0_EN(GPE_STD)); + u32 gpe0_en = inl(get_pmbase() + GPE0_EN(GPE_STD)); gpe0_en |= mask; - outl(gpe0_en, ACPI_BASE_ADDRESS + GPE0_EN(GPE_STD)); + outl(gpe0_en, get_pmbase() + GPE0_EN(GPE_STD)); }
/* Disable a standard GPE */ void disable_gpe(u32 mask) { - u32 gpe0_en = inl(ACPI_BASE_ADDRESS + GPE0_EN(GPE_STD)); + u32 gpe0_en = inl(get_pmbase() + GPE0_EN(GPE_STD)); gpe0_en &= ~mask; - outl(gpe0_en, ACPI_BASE_ADDRESS + GPE0_EN(GPE_STD)); + outl(gpe0_en, get_pmbase() + GPE0_EN(GPE_STD)); }
int acpi_sci_irq(void) @@ -434,10 +442,10 @@
int platform_is_resuming(void) { - if (!(inw(ACPI_BASE_ADDRESS + PM1_STS) & WAK_STS)) + if (!(inw(get_pmbase() + PM1_STS) & WAK_STS)) return 0;
- return acpi_sleep_from_pm1(inl(ACPI_BASE_ADDRESS + PM1_CNT)) == ACPI_S3; + return acpi_sleep_from_pm1(inl(get_pmbase() + PM1_CNT)) == ACPI_S3; }
/* STM Support */ diff --git a/src/soc/intel/broadwell/smihandler.c b/src/soc/intel/broadwell/smihandler.c index 4a12d78..fd5d452 100644 --- a/src/soc/intel/broadwell/smihandler.c +++ b/src/soc/intel/broadwell/smihandler.c @@ -133,6 +133,7 @@ u32 reg32; u8 slp_typ; u8 s5pwr = CONFIG_MAINBOARD_POWER_FAILURE_STATE; + u16 pmbase = get_pmbase();
/* save and recover RTC port values */ u8 tmp70, tmp72; @@ -146,7 +147,7 @@ disable_smi(SLP_SMI_EN);
/* Figure out SLP_TYP */ - reg32 = inl(ACPI_BASE_ADDRESS + PM1_CNT); + reg32 = inl(pmbase + PM1_CNT); printk(BIOS_SPEW, "SMI#: SLP = 0x%08x\n", reg32); slp_typ = acpi_sleep_from_pm1(reg32);
@@ -225,7 +226,7 @@ * the line above. However, if we entered sleep state S1 and wake * up again, we will continue to execute code in this function. */ - reg32 = inl(ACPI_BASE_ADDRESS + PM1_CNT); + reg32 = inl(pmbase + PM1_CNT); if (reg32 & SCI_EN) { /* The OS is not an ACPI OS, so we set the state to S0 */ disable_pm1_control(SLP_EN | SLP_TYP); @@ -393,7 +394,7 @@
static void southbridge_smi_mc(void) { - u32 reg32 = inl(ACPI_BASE_ADDRESS + SMI_EN); + u32 reg32 = inl(get_pmbase() + SMI_EN);
/* Are microcontroller SMIs enabled? */ if ((reg32 & MCSMI_EN) == 0) @@ -436,7 +437,7 @@
static void southbridge_smi_periodic(void) { - u32 reg32 = inl(ACPI_BASE_ADDRESS + SMI_EN); + u32 reg32 = inl(get_pmbase() + SMI_EN);
/* Are periodic SMIs enabled? */ if ((reg32 & PERIODIC_EN) == 0)
-
Angel Pons (Code Review) -
Patrick Rudolph (Code Review)