Change in ...coreboot[master]: soc/amd/stoneyridge: Relocate MMIO access of ACPI registers
Hello Richard Spiegel, Martin Roth, Furquan Shaikh,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/32655
to review the following change.
Change subject: soc/amd/stoneyridge: Relocate MMIO access of ACPI registers ......................................................................
soc/amd/stoneyridge: Relocate MMIO access of ACPI registers
The AcpiMmio block allowing direct access to the ACPI registers has remained consistent across AMD models. Move the support from soc//stoneyridge to soc//common.
Change-Id: I0e017a71f8efb4b614986cb327de398644599853 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/common/block/acpi/Makefile.inc M src/soc/amd/common/block/acpi/acpi.c A src/soc/amd/common/block/include/amdblocks/acpi.h M src/soc/amd/common/block/s3/Kconfig M src/soc/amd/common/block/s3/s3_resume.c M src/soc/amd/stoneyridge/acpi.c M src/soc/amd/stoneyridge/include/soc/southbridge.h M src/soc/amd/stoneyridge/pmutil.c M src/soc/amd/stoneyridge/smihandler.c M src/soc/amd/stoneyridge/southbridge.c 10 files changed, 200 insertions(+), 130 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/32655/1
diff --git a/src/soc/amd/common/block/acpi/Makefile.inc b/src/soc/amd/common/block/acpi/Makefile.inc index 1320849..708631a 100644 --- a/src/soc/amd/common/block/acpi/Makefile.inc +++ b/src/soc/amd/common/block/acpi/Makefile.inc @@ -1,2 +1,6 @@ +bootblock-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPI) += acpi.c +verstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPI) += acpi.c +romstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPI) += acpi.c ramstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPI) += acpi.c +postcar-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPI) += acpi.c smm-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPI) += acpi.c diff --git a/src/soc/amd/common/block/acpi/acpi.c b/src/soc/amd/common/block/acpi/acpi.c index 200b3c1..39e03e2 100644 --- a/src/soc/amd/common/block/acpi/acpi.c +++ b/src/soc/amd/common/block/acpi/acpi.c @@ -14,9 +14,14 @@ */
#include <arch/acpi.h> +#include <cbmem.h> +#include <elog.h> +#include <console/console.h> #include <soc/southbridge.h> #include <amdblocks/acpimmio.h> +#include <amdblocks/acpi.h> #include <halt.h> +#include <security/vboot/vboot_common.h>
void poweroff(void) { @@ -31,3 +36,142 @@ if (!ENV_SMM) halt(); } + +static uint16_t reset_pm1_status(void) +{ + uint16_t pm1_sts = acpi_read16(MMIO_ACPI_PM1_STS); + acpi_write16(MMIO_ACPI_PM1_STS, pm1_sts); + return pm1_sts; +} + +static void print_num_status_bits(int num_bits, uint32_t status, + const char *const bit_names[]) +{ + int i; + + if (!status) + return; + + for (i = num_bits - 1; i >= 0; i--) { + if (status & (1 << i)) { + if (bit_names[i]) + printk(BIOS_DEBUG, "%s ", bit_names[i]); + else + printk(BIOS_DEBUG, "BIT%d ", i); + } + } +} + +static uint16_t print_pm1_status(uint16_t pm1_sts) +{ + static const char *const pm1_sts_bits[16] = { + [0] = "TMROF", + [4] = "BMSTATUS", + [5] = "GBL", + [8] = "PWRBTN", + [10] = "RTC", + [14] = "PCIEXPWAK", + [15] = "WAK", + }; + + if (!pm1_sts) + return 0; + + printk(BIOS_DEBUG, "PM1_STS: "); + print_num_status_bits(ARRAY_SIZE(pm1_sts_bits), pm1_sts, pm1_sts_bits); + printk(BIOS_DEBUG, "\n"); + + return pm1_sts; +} + +static void log_pm1_status(uint16_t pm1_sts) +{ + if (!CONFIG(ELOG)) + return; + + if (pm1_sts & WAK_STS) + elog_add_event_byte(ELOG_TYPE_ACPI_WAKE, + acpi_is_wakeup_s3() ? ACPI_S3 : ACPI_S5); + + if (pm1_sts & PWRBTN_STS) + elog_add_event_wake(ELOG_WAKE_SOURCE_PWRBTN, 0); + + if (pm1_sts & RTC_STS) + elog_add_event_wake(ELOG_WAKE_SOURCE_RTC, 0); + + if (pm1_sts & PCIEXPWAK_STS) + elog_add_event_wake(ELOG_WAKE_SOURCE_PCIE, 0); +} + +static void save_sws(uint16_t pm1_status) +{ + struct soc_power_reg *sws; + uint32_t reg32; + uint16_t reg16; + + sws = cbmem_add(CBMEM_ID_POWER_STATE, sizeof(struct soc_power_reg)); + if (sws == NULL) + return; + sws->pm1_sts = pm1_status; + sws->pm1_en = acpi_read16(MMIO_ACPI_PM1_EN); + reg32 = acpi_read32(MMIO_ACPI_GPE0_STS); + acpi_write32(MMIO_ACPI_GPE0_STS, reg32); + sws->gpe0_sts = reg32; + sws->gpe0_en = acpi_read32(MMIO_ACPI_GPE0_EN); + reg16 = acpi_read16(MMIO_ACPI_PM1_CNT_BLK); + reg16 &= SLP_TYP; + sws->wake_from = reg16 >> SLP_TYP_SHIFT; +} + +void acpi_clear_pm1_status(void) +{ + uint16_t pm1_sts = reset_pm1_status(); + + save_sws(pm1_sts); + log_pm1_status(pm1_sts); + print_pm1_status(pm1_sts); +} + +int vboot_platform_is_resuming(void) +{ + if (!(acpi_read16(MMIO_ACPI_PM1_STS) & WAK_STS)) + return 0; + + uint16_t pm_cnt = acpi_read16(MMIO_ACPI_PM1_CNT_BLK); + return acpi_sleep_from_pm1(pm_cnt) == ACPI_S3; +} + +/* If a system reset is about to be requested, modify the PM1 register so it + * will never be misinterpreted as an S3 resume. */ +void set_pm1cnt_s5(void) +{ + uint16_t pm1; + + pm1 = acpi_read16(MMIO_ACPI_PM1_CNT_BLK); + pm1 &= ~SLP_TYP; + pm1 |= SLP_TYP_S5 << SLP_TYP_SHIFT; + acpi_write16(MMIO_ACPI_PM1_CNT_BLK, pm1); +} + +void vboot_platform_prepare_reboot(void) +{ + set_pm1cnt_s5(); +} + +void acpi_enable_sci(void) +{ + uint32_t pm1; + + pm1 = acpi_read32(MMIO_ACPI_PM1_CNT_BLK); + pm1 |= ACPI_PM1_CNT_SCIEN; + acpi_write32(MMIO_ACPI_PM1_CNT_BLK, pm1); +} + +void acpi_disable_sci(void) +{ + uint32_t pm1; + + pm1 = acpi_read32(MMIO_ACPI_PM1_CNT_BLK); + pm1 &= ~ACPI_PM1_CNT_SCIEN; + acpi_write32(MMIO_ACPI_PM1_CNT_BLK, pm1); +} diff --git a/src/soc/amd/common/block/include/amdblocks/acpi.h b/src/soc/amd/common/block/include/amdblocks/acpi.h new file mode 100644 index 0000000..cf266ed --- /dev/null +++ b/src/soc/amd/common/block/include/amdblocks/acpi.h @@ -0,0 +1,42 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2017 Google, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef __AMDBLOCKS_ACPI_H__ +#define __AMDBLOCKS_ACPI_H__ + +#include <stdint.h> + +/* ACPI MMIO registers 0xfed80800 */ +#define MMIO_ACPI_PM1_STS 0x00 +#define MMIO_ACPI_PM1_EN 0x02 +#define MMIO_ACPI_PM1_CNT_BLK 0x04 + /* sleep types defined in arch/x86/include/arch/acpi.h */ +#define ACPI_PM1_CNT_SCIEN BIT(0) +#define MMIO_ACPI_PM_TMR_BLK 0x08 +#define MMIO_ACPI_CPU_CONTROL 0x0c +#define MMIO_ACPI_GPE0_STS 0x14 +#define MMIO_ACPI_GPE0_EN 0x18 + +void acpi_clear_pm1_status(void); + +/* + * If a system reset is about to be requested, modify the PM1 register so it + * will never be misinterpreted as an S3 resume. + */ +void set_pm1cnt_s5(void); +void acpi_enable_sci(void); +void acpi_disable_sci(void); + +#endif /* __AMDBLOCKS_ACPI_H__ */ diff --git a/src/soc/amd/common/block/s3/Kconfig b/src/soc/amd/common/block/s3/Kconfig index 0880163..ebc1695 100644 --- a/src/soc/amd/common/block/s3/Kconfig +++ b/src/soc/amd/common/block/s3/Kconfig @@ -1,6 +1,7 @@ config SOC_AMD_COMMON_BLOCK_S3 bool default n + depends on SOC_AMD_COMMON_BLOCK_ACPI select CACHE_MRC_SETTINGS select MRC_WRITE_NV_LATE help diff --git a/src/soc/amd/common/block/s3/s3_resume.c b/src/soc/amd/common/block/s3/s3_resume.c index 0ba2f13..74aa79c 100644 --- a/src/soc/amd/common/block/s3/s3_resume.c +++ b/src/soc/amd/common/block/s3/s3_resume.c @@ -21,6 +21,7 @@ #include <console/console.h> #include <soc/southbridge.h> #include <amdblocks/s3_resume.h> +#include <amdblocks/acpi.h>
/* Training data versioning is not supported or tracked. */ #define DEFAULT_MRC_VERSION 0 diff --git a/src/soc/amd/stoneyridge/acpi.c b/src/soc/amd/stoneyridge/acpi.c index 2cd9632..d1ea24f 100644 --- a/src/soc/amd/stoneyridge/acpi.c +++ b/src/soc/amd/stoneyridge/acpi.c @@ -29,6 +29,7 @@ #include <device/device.h> #include <device/pci.h> #include <amdblocks/acpimmio.h> +#include <amdblocks/acpi.h> #include <soc/acpi.h> #include <soc/pci_devs.h> #include <soc/southbridge.h> @@ -99,7 +100,7 @@ fadt->s4bios_req = 0; /* Not supported */ fadt->pstate_cnt = 0; /* Not supported */ fadt->cst_cnt = 0; /* Not supported */ - acpi_write32(MMIO_ACPI_PM1_CNT_BLK, 0); /* clear SCI_EN */ + acpi_disable_sci(); } else { fadt->smi_cmd = 0; /* disable system management mode */ fadt->acpi_enable = 0; /* unused if SMI_CMD = 0 */ @@ -107,7 +108,7 @@ fadt->s4bios_req = 0; /* unused if SMI_CMD = 0 */ fadt->pstate_cnt = 0; /* unused if SMI_CMD = 0 */ fadt->cst_cnt = 0x00; /* unused if SMI_CMD = 0 */ - acpi_write32(MMIO_ACPI_PM1_CNT_BLK, 1); /* set SCI_EN */ + acpi_enable_sci(); }
fadt->pm1a_evt_blk = ACPI_PM_EVT_BLK; diff --git a/src/soc/amd/stoneyridge/include/soc/southbridge.h b/src/soc/amd/stoneyridge/include/soc/southbridge.h index aa0e783..40d6ec7 100644 --- a/src/soc/amd/stoneyridge/include/soc/southbridge.h +++ b/src/soc/amd/stoneyridge/include/soc/southbridge.h @@ -103,15 +103,6 @@ #define PM_USB_ENABLE 0xef #define PM_USB_ALL_CONTROLLERS 0x7f
-/* ACPI MMIO registers 0xfed80800 */ -#define MMIO_ACPI_PM1_STS 0x00 -#define MMIO_ACPI_PM1_EN 0x02 -#define MMIO_ACPI_PM1_CNT_BLK 0x04 -#define MMIO_ACPI_CPU_CONTROL 0x0c -#define MMIO_ACPI_GPE0_STS 0x14 -#define MMIO_ACPI_GPE0_EN 0x18 -#define MMIO_ACPI_PM_TMR_BLK 0x08 - /* SMBUS MMIO offsets 0xfed80a00 */ #define SMBHSTSTAT 0x0 #define SMBHST_STAT_FAILED 0x10 @@ -417,10 +408,4 @@ /* Initialize all the i2c buses that are not marked with early init. */ void i2c_soc_init(void);
-/* - * If a system reset is about to be requested, modify the PM1 register so it - * will never be misinterpreted as an S3 resume. - */ -void set_pm1cnt_s5(void); - #endif /* __STONEYRIDGE_H__ */ diff --git a/src/soc/amd/stoneyridge/pmutil.c b/src/soc/amd/stoneyridge/pmutil.c index 7367251..59de348 100644 --- a/src/soc/amd/stoneyridge/pmutil.c +++ b/src/soc/amd/stoneyridge/pmutil.c @@ -25,29 +25,3 @@ /* If CMOS power has failed, the century will be set to 0xff */ return cmos_read(RTC_CLK_ALTCENTURY) == 0xff; } - -int vboot_platform_is_resuming(void) -{ - if (!(acpi_read16(MMIO_ACPI_PM1_STS) & WAK_STS)) - return 0; - - uint16_t pm_cnt = acpi_read16(MMIO_ACPI_PM1_CNT_BLK); - return acpi_sleep_from_pm1(pm_cnt) == ACPI_S3; -} - -/* If a system reset is about to be requested, modify the PM1 register so it - * will never be misinterpreted as an S3 resume. */ -void set_pm1cnt_s5(void) -{ - uint16_t pm1; - - pm1 = acpi_read16(MMIO_ACPI_PM1_CNT_BLK); - pm1 &= ~SLP_TYP; - pm1 |= SLP_TYP_S5 << SLP_TYP_SHIFT; - acpi_write16(MMIO_ACPI_PM1_CNT_BLK, pm1); -} - -void vboot_platform_prepare_reboot(void) -{ - set_pm1cnt_s5(); -} diff --git a/src/soc/amd/stoneyridge/smihandler.c b/src/soc/amd/stoneyridge/smihandler.c index c3aed57..e08aee4 100644 --- a/src/soc/amd/stoneyridge/smihandler.c +++ b/src/soc/amd/stoneyridge/smihandler.c @@ -25,6 +25,7 @@ #include <soc/smi.h> #include <soc/southbridge.h> #include <amdblocks/acpimmio.h> +#include <amdblocks/acpi.h> #include <elog.h>
/* bits in smm_io_trap */ @@ -88,19 +89,14 @@
static void sb_apmc_smi_handler(void) { - u32 reg32; const uint8_t cmd = inb(pm_acpi_smi_cmd_port());
switch (cmd) { case APM_CNT_ACPI_ENABLE: - reg32 = acpi_read32(MMIO_ACPI_PM1_CNT_BLK); - reg32 |= (1 << 0); /* SCI_EN */ - acpi_write32(MMIO_ACPI_PM1_CNT_BLK, reg32); + acpi_enable_sci(); break; case APM_CNT_ACPI_DISABLE: - reg32 = acpi_read32(MMIO_ACPI_PM1_CNT_BLK); - reg32 &= ~(1 << 0); /* clear SCI_EN */ - acpi_write32(MMIO_ACPI_PM1_CNT_BLK, reg32); + acpi_disable_sci(); break; case APM_CNT_ELOG_GSMI: if (CONFIG(ELOG_GSMI)) diff --git a/src/soc/amd/stoneyridge/southbridge.c b/src/soc/amd/stoneyridge/southbridge.c index f86e4a9..a08d6b5 100644 --- a/src/soc/amd/stoneyridge/southbridge.c +++ b/src/soc/amd/stoneyridge/southbridge.c @@ -14,7 +14,6 @@ */
#include <console/console.h> - #include <arch/io.h> #include <device/mmio.h> #include <bootstate.h> @@ -24,12 +23,12 @@ #include <device/pci_ids.h> #include <device/pci_ops.h> #include <cbmem.h> -#include <elog.h> #include <amdblocks/amd_pci_util.h> #include <amdblocks/agesawrapper.h> #include <amdblocks/reset.h> #include <amdblocks/acpimmio.h> #include <amdblocks/lpc.h> +#include <amdblocks/acpi.h> #include <soc/southbridge.h> #include <soc/smbus.h> #include <soc/smi.h> @@ -523,83 +522,6 @@ PM_ACPI_TIMER_EN_EN); }
-static uint16_t reset_pm1_status(void) -{ - uint16_t pm1_sts = acpi_read16(MMIO_ACPI_PM1_STS); - acpi_write16(MMIO_ACPI_PM1_STS, pm1_sts); - return pm1_sts; -} - -static uint16_t print_pm1_status(uint16_t pm1_sts) -{ - static const char *const pm1_sts_bits[16] = { - [0] = "TMROF", - [4] = "BMSTATUS", - [5] = "GBL", - [8] = "PWRBTN", - [10] = "RTC", - [14] = "PCIEXPWAK", - [15] = "WAK", - }; - - if (!pm1_sts) - return 0; - - printk(BIOS_DEBUG, "PM1_STS: "); - print_num_status_bits(ARRAY_SIZE(pm1_sts_bits), pm1_sts, pm1_sts_bits); - printk(BIOS_DEBUG, "\n"); - - return pm1_sts; -} - -static void sb_log_pm1_status(uint16_t pm1_sts) -{ - if (!CONFIG(ELOG)) - return; - - if (pm1_sts & WAK_STS) - elog_add_event_byte(ELOG_TYPE_ACPI_WAKE, - acpi_is_wakeup_s3() ? ACPI_S3 : ACPI_S5); - - if (pm1_sts & PWRBTN_STS) - elog_add_event_wake(ELOG_WAKE_SOURCE_PWRBTN, 0); - - if (pm1_sts & RTC_STS) - elog_add_event_wake(ELOG_WAKE_SOURCE_RTC, 0); - - if (pm1_sts & PCIEXPWAK_STS) - elog_add_event_wake(ELOG_WAKE_SOURCE_PCIE, 0); -} - -static void sb_save_sws(uint16_t pm1_status) -{ - struct soc_power_reg *sws; - uint32_t reg32; - uint16_t reg16; - - sws = cbmem_add(CBMEM_ID_POWER_STATE, sizeof(struct soc_power_reg)); - if (sws == NULL) - return; - sws->pm1_sts = pm1_status; - sws->pm1_en = acpi_read16(MMIO_ACPI_PM1_EN); - reg32 = acpi_read32(MMIO_ACPI_GPE0_STS); - acpi_write32(MMIO_ACPI_GPE0_STS, reg32); - sws->gpe0_sts = reg32; - sws->gpe0_en = acpi_read32(MMIO_ACPI_GPE0_EN); - reg16 = acpi_read16(MMIO_ACPI_PM1_CNT_BLK); - reg16 &= SLP_TYP; - sws->wake_from = reg16 >> SLP_TYP_SHIFT; -} - -static void sb_clear_pm1_status(void) -{ - uint16_t pm1_sts = reset_pm1_status(); - - sb_save_sws(pm1_sts); - sb_log_pm1_status(pm1_sts); - print_pm1_status(pm1_sts); -} - static int get_index_bit(uint32_t value, uint16_t limit) { uint16_t i; @@ -652,7 +574,7 @@ void southbridge_init(void *chip_info) { sb_init_acpi_ports(); - sb_clear_pm1_status(); + acpi_clear_pm1_status(); }
static void set_sb_final_nvs(void)
Hello Richard Spiegel, build bot (Jenkins), Patrick Georgi, Martin Roth, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32655
to look at the new patch set (#2).
Change subject: soc/amd/stoneyridge: Relocate MMIO access of ACPI registers ......................................................................
soc/amd/stoneyridge: Relocate MMIO access of ACPI registers
The AcpiMmio block allowing direct access to the ACPI registers has remained consistent across AMD models. Move the support from soc//stoneyridge to soc//common.
BUG=b:131682806
Change-Id: I0e017a71f8efb4b614986cb327de398644599853 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/common/block/acpi/Makefile.inc M src/soc/amd/common/block/acpi/acpi.c A src/soc/amd/common/block/include/amdblocks/acpi.h M src/soc/amd/common/block/s3/Kconfig M src/soc/amd/common/block/s3/s3_resume.c M src/soc/amd/stoneyridge/acpi.c M src/soc/amd/stoneyridge/include/soc/southbridge.h M src/soc/amd/stoneyridge/pmutil.c M src/soc/amd/stoneyridge/smihandler.c M src/soc/amd/stoneyridge/southbridge.c 10 files changed, 200 insertions(+), 130 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/32655/2
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32655 )
Change subject: soc/amd/stoneyridge: Relocate MMIO access of ACPI registers ......................................................................
Patch Set 3: Code-Review+2
Not all changes are obvious, but examining the full context, they make sense.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32655 )
Change subject: soc/amd/stoneyridge: Relocate MMIO access of ACPI registers ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/#/c/32655/4/src/soc/amd/common/block/include/amd... File src/soc/amd/common/block/include/amdblocks/acpi.h:
https://review.coreboot.org/#/c/32655/4/src/soc/amd/common/block/include/amd... PS4, Line 38: set_pm1cnt_s5 acpi_set_pm1cnt_s5?
https://review.coreboot.org/#/c/32655/4/src/soc/amd/common/block/s3/s3_resum... File src/soc/amd/common/block/s3/s3_resume.c:
https://review.coreboot.org/#/c/32655/4/src/soc/amd/common/block/s3/s3_resum... PS4, Line 24: #include <amdblocks/acpi.h> Why is this required?
Hello Richard Spiegel, build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32655
to look at the new patch set (#5).
Change subject: soc/amd/stoneyridge: Relocate MMIO access of ACPI registers ......................................................................
soc/amd/stoneyridge: Relocate MMIO access of ACPI registers
The AcpiMmio block allowing direct access to the ACPI registers has remained consistent across AMD models. Move the support from soc//stoneyridge to soc//common.
BUG=b:131682806
Change-Id: I0e017a71f8efb4b614986cb327de398644599853 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com --- M src/soc/amd/common/block/acpi/Makefile.inc M src/soc/amd/common/block/acpi/acpi.c A src/soc/amd/common/block/include/amdblocks/acpi.h M src/soc/amd/common/block/s3/Kconfig M src/soc/amd/common/block/s3/s3_resume.c M src/soc/amd/stoneyridge/acpi.c M src/soc/amd/stoneyridge/include/soc/southbridge.h M src/soc/amd/stoneyridge/pmutil.c M src/soc/amd/stoneyridge/smihandler.c M src/soc/amd/stoneyridge/southbridge.c 10 files changed, 200 insertions(+), 129 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/32655/5
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32655 )
Change subject: soc/amd/stoneyridge: Relocate MMIO access of ACPI registers ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/#/c/32655/4/src/soc/amd/common/block/include/amd... File src/soc/amd/common/block/include/amdblocks/acpi.h:
https://review.coreboot.org/#/c/32655/4/src/soc/amd/common/block/include/amd... PS4, Line 38: set_pm1cnt_s5
acpi_set_pm1cnt_s5?
It's correct as-is, called from block/s3/s3_resume.c
https://review.coreboot.org/#/c/32655/4/src/soc/amd/common/block/s3/s3_resum... File src/soc/amd/common/block/s3/s3_resume.c:
https://review.coreboot.org/#/c/32655/4/src/soc/amd/common/block/s3/s3_resum... PS4, Line 24: #include <amdblocks/acpi.h>
Why is this required?
For set_pm1cnt_s5() at line 32
Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32655 )
Change subject: soc/amd/stoneyridge: Relocate MMIO access of ACPI registers ......................................................................
Patch Set 6: Code-Review+2
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32655 )
Change subject: soc/amd/stoneyridge: Relocate MMIO access of ACPI registers ......................................................................
soc/amd/stoneyridge: Relocate MMIO access of ACPI registers
The AcpiMmio block allowing direct access to the ACPI registers has remained consistent across AMD models. Move the support from soc//stoneyridge to soc//common.
BUG=b:131682806
Change-Id: I0e017a71f8efb4b614986cb327de398644599853 Signed-off-by: Marshall Dawson marshalldawson3rd@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32655 Reviewed-by: Richard Spiegel richard.spiegel@silverbackltd.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/amd/common/block/acpi/Makefile.inc M src/soc/amd/common/block/acpi/acpi.c A src/soc/amd/common/block/include/amdblocks/acpi.h M src/soc/amd/common/block/s3/Kconfig M src/soc/amd/common/block/s3/s3_resume.c M src/soc/amd/stoneyridge/acpi.c M src/soc/amd/stoneyridge/include/soc/southbridge.h M src/soc/amd/stoneyridge/pmutil.c M src/soc/amd/stoneyridge/smihandler.c M src/soc/amd/stoneyridge/southbridge.c 10 files changed, 200 insertions(+), 129 deletions(-)
Approvals: build bot (Jenkins): Verified Richard Spiegel: Looks good to me, approved
diff --git a/src/soc/amd/common/block/acpi/Makefile.inc b/src/soc/amd/common/block/acpi/Makefile.inc index 1320849..708631a 100644 --- a/src/soc/amd/common/block/acpi/Makefile.inc +++ b/src/soc/amd/common/block/acpi/Makefile.inc @@ -1,2 +1,6 @@ +bootblock-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPI) += acpi.c +verstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPI) += acpi.c +romstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPI) += acpi.c ramstage-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPI) += acpi.c +postcar-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPI) += acpi.c smm-$(CONFIG_SOC_AMD_COMMON_BLOCK_ACPI) += acpi.c diff --git a/src/soc/amd/common/block/acpi/acpi.c b/src/soc/amd/common/block/acpi/acpi.c index 200b3c1..39e03e2 100644 --- a/src/soc/amd/common/block/acpi/acpi.c +++ b/src/soc/amd/common/block/acpi/acpi.c @@ -14,9 +14,14 @@ */
#include <arch/acpi.h> +#include <cbmem.h> +#include <elog.h> +#include <console/console.h> #include <soc/southbridge.h> #include <amdblocks/acpimmio.h> +#include <amdblocks/acpi.h> #include <halt.h> +#include <security/vboot/vboot_common.h>
void poweroff(void) { @@ -31,3 +36,142 @@ if (!ENV_SMM) halt(); } + +static uint16_t reset_pm1_status(void) +{ + uint16_t pm1_sts = acpi_read16(MMIO_ACPI_PM1_STS); + acpi_write16(MMIO_ACPI_PM1_STS, pm1_sts); + return pm1_sts; +} + +static void print_num_status_bits(int num_bits, uint32_t status, + const char *const bit_names[]) +{ + int i; + + if (!status) + return; + + for (i = num_bits - 1; i >= 0; i--) { + if (status & (1 << i)) { + if (bit_names[i]) + printk(BIOS_DEBUG, "%s ", bit_names[i]); + else + printk(BIOS_DEBUG, "BIT%d ", i); + } + } +} + +static uint16_t print_pm1_status(uint16_t pm1_sts) +{ + static const char *const pm1_sts_bits[16] = { + [0] = "TMROF", + [4] = "BMSTATUS", + [5] = "GBL", + [8] = "PWRBTN", + [10] = "RTC", + [14] = "PCIEXPWAK", + [15] = "WAK", + }; + + if (!pm1_sts) + return 0; + + printk(BIOS_DEBUG, "PM1_STS: "); + print_num_status_bits(ARRAY_SIZE(pm1_sts_bits), pm1_sts, pm1_sts_bits); + printk(BIOS_DEBUG, "\n"); + + return pm1_sts; +} + +static void log_pm1_status(uint16_t pm1_sts) +{ + if (!CONFIG(ELOG)) + return; + + if (pm1_sts & WAK_STS) + elog_add_event_byte(ELOG_TYPE_ACPI_WAKE, + acpi_is_wakeup_s3() ? ACPI_S3 : ACPI_S5); + + if (pm1_sts & PWRBTN_STS) + elog_add_event_wake(ELOG_WAKE_SOURCE_PWRBTN, 0); + + if (pm1_sts & RTC_STS) + elog_add_event_wake(ELOG_WAKE_SOURCE_RTC, 0); + + if (pm1_sts & PCIEXPWAK_STS) + elog_add_event_wake(ELOG_WAKE_SOURCE_PCIE, 0); +} + +static void save_sws(uint16_t pm1_status) +{ + struct soc_power_reg *sws; + uint32_t reg32; + uint16_t reg16; + + sws = cbmem_add(CBMEM_ID_POWER_STATE, sizeof(struct soc_power_reg)); + if (sws == NULL) + return; + sws->pm1_sts = pm1_status; + sws->pm1_en = acpi_read16(MMIO_ACPI_PM1_EN); + reg32 = acpi_read32(MMIO_ACPI_GPE0_STS); + acpi_write32(MMIO_ACPI_GPE0_STS, reg32); + sws->gpe0_sts = reg32; + sws->gpe0_en = acpi_read32(MMIO_ACPI_GPE0_EN); + reg16 = acpi_read16(MMIO_ACPI_PM1_CNT_BLK); + reg16 &= SLP_TYP; + sws->wake_from = reg16 >> SLP_TYP_SHIFT; +} + +void acpi_clear_pm1_status(void) +{ + uint16_t pm1_sts = reset_pm1_status(); + + save_sws(pm1_sts); + log_pm1_status(pm1_sts); + print_pm1_status(pm1_sts); +} + +int vboot_platform_is_resuming(void) +{ + if (!(acpi_read16(MMIO_ACPI_PM1_STS) & WAK_STS)) + return 0; + + uint16_t pm_cnt = acpi_read16(MMIO_ACPI_PM1_CNT_BLK); + return acpi_sleep_from_pm1(pm_cnt) == ACPI_S3; +} + +/* If a system reset is about to be requested, modify the PM1 register so it + * will never be misinterpreted as an S3 resume. */ +void set_pm1cnt_s5(void) +{ + uint16_t pm1; + + pm1 = acpi_read16(MMIO_ACPI_PM1_CNT_BLK); + pm1 &= ~SLP_TYP; + pm1 |= SLP_TYP_S5 << SLP_TYP_SHIFT; + acpi_write16(MMIO_ACPI_PM1_CNT_BLK, pm1); +} + +void vboot_platform_prepare_reboot(void) +{ + set_pm1cnt_s5(); +} + +void acpi_enable_sci(void) +{ + uint32_t pm1; + + pm1 = acpi_read32(MMIO_ACPI_PM1_CNT_BLK); + pm1 |= ACPI_PM1_CNT_SCIEN; + acpi_write32(MMIO_ACPI_PM1_CNT_BLK, pm1); +} + +void acpi_disable_sci(void) +{ + uint32_t pm1; + + pm1 = acpi_read32(MMIO_ACPI_PM1_CNT_BLK); + pm1 &= ~ACPI_PM1_CNT_SCIEN; + acpi_write32(MMIO_ACPI_PM1_CNT_BLK, pm1); +} diff --git a/src/soc/amd/common/block/include/amdblocks/acpi.h b/src/soc/amd/common/block/include/amdblocks/acpi.h new file mode 100644 index 0000000..cf266ed --- /dev/null +++ b/src/soc/amd/common/block/include/amdblocks/acpi.h @@ -0,0 +1,42 @@ +/* + * This file is part of the coreboot project. + * + * Copyright 2017 Google, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef __AMDBLOCKS_ACPI_H__ +#define __AMDBLOCKS_ACPI_H__ + +#include <stdint.h> + +/* ACPI MMIO registers 0xfed80800 */ +#define MMIO_ACPI_PM1_STS 0x00 +#define MMIO_ACPI_PM1_EN 0x02 +#define MMIO_ACPI_PM1_CNT_BLK 0x04 + /* sleep types defined in arch/x86/include/arch/acpi.h */ +#define ACPI_PM1_CNT_SCIEN BIT(0) +#define MMIO_ACPI_PM_TMR_BLK 0x08 +#define MMIO_ACPI_CPU_CONTROL 0x0c +#define MMIO_ACPI_GPE0_STS 0x14 +#define MMIO_ACPI_GPE0_EN 0x18 + +void acpi_clear_pm1_status(void); + +/* + * If a system reset is about to be requested, modify the PM1 register so it + * will never be misinterpreted as an S3 resume. + */ +void set_pm1cnt_s5(void); +void acpi_enable_sci(void); +void acpi_disable_sci(void); + +#endif /* __AMDBLOCKS_ACPI_H__ */ diff --git a/src/soc/amd/common/block/s3/Kconfig b/src/soc/amd/common/block/s3/Kconfig index 0880163..ebc1695 100644 --- a/src/soc/amd/common/block/s3/Kconfig +++ b/src/soc/amd/common/block/s3/Kconfig @@ -1,6 +1,7 @@ config SOC_AMD_COMMON_BLOCK_S3 bool default n + depends on SOC_AMD_COMMON_BLOCK_ACPI select CACHE_MRC_SETTINGS select MRC_WRITE_NV_LATE help diff --git a/src/soc/amd/common/block/s3/s3_resume.c b/src/soc/amd/common/block/s3/s3_resume.c index 0ba2f13..74aa79c 100644 --- a/src/soc/amd/common/block/s3/s3_resume.c +++ b/src/soc/amd/common/block/s3/s3_resume.c @@ -21,6 +21,7 @@ #include <console/console.h> #include <soc/southbridge.h> #include <amdblocks/s3_resume.h> +#include <amdblocks/acpi.h>
/* Training data versioning is not supported or tracked. */ #define DEFAULT_MRC_VERSION 0 diff --git a/src/soc/amd/stoneyridge/acpi.c b/src/soc/amd/stoneyridge/acpi.c index 2cd9632..d1ea24f 100644 --- a/src/soc/amd/stoneyridge/acpi.c +++ b/src/soc/amd/stoneyridge/acpi.c @@ -29,6 +29,7 @@ #include <device/device.h> #include <device/pci.h> #include <amdblocks/acpimmio.h> +#include <amdblocks/acpi.h> #include <soc/acpi.h> #include <soc/pci_devs.h> #include <soc/southbridge.h> @@ -99,7 +100,7 @@ fadt->s4bios_req = 0; /* Not supported */ fadt->pstate_cnt = 0; /* Not supported */ fadt->cst_cnt = 0; /* Not supported */ - acpi_write32(MMIO_ACPI_PM1_CNT_BLK, 0); /* clear SCI_EN */ + acpi_disable_sci(); } else { fadt->smi_cmd = 0; /* disable system management mode */ fadt->acpi_enable = 0; /* unused if SMI_CMD = 0 */ @@ -107,7 +108,7 @@ fadt->s4bios_req = 0; /* unused if SMI_CMD = 0 */ fadt->pstate_cnt = 0; /* unused if SMI_CMD = 0 */ fadt->cst_cnt = 0x00; /* unused if SMI_CMD = 0 */ - acpi_write32(MMIO_ACPI_PM1_CNT_BLK, 1); /* set SCI_EN */ + acpi_enable_sci(); }
fadt->pm1a_evt_blk = ACPI_PM_EVT_BLK; diff --git a/src/soc/amd/stoneyridge/include/soc/southbridge.h b/src/soc/amd/stoneyridge/include/soc/southbridge.h index 6a7c58f..1e86ba6 100644 --- a/src/soc/amd/stoneyridge/include/soc/southbridge.h +++ b/src/soc/amd/stoneyridge/include/soc/southbridge.h @@ -103,15 +103,6 @@ #define PM_USB_ENABLE 0xef #define PM_USB_ALL_CONTROLLERS 0x7f
-/* ACPI MMIO registers 0xfed80800 */ -#define MMIO_ACPI_PM1_STS 0x00 -#define MMIO_ACPI_PM1_EN 0x02 -#define MMIO_ACPI_PM1_CNT_BLK 0x04 -#define MMIO_ACPI_CPU_CONTROL 0x0c -#define MMIO_ACPI_GPE0_STS 0x14 -#define MMIO_ACPI_GPE0_EN 0x18 -#define MMIO_ACPI_PM_TMR_BLK 0x08 - /* SMBUS MMIO offsets 0xfed80a00 */ #define SMBHSTSTAT 0x0 #define SMBHST_STAT_FAILED 0x10 @@ -416,10 +407,4 @@ /* Initialize all the i2c buses that are not marked with early init. */ void i2c_soc_init(void);
-/* - * If a system reset is about to be requested, modify the PM1 register so it - * will never be misinterpreted as an S3 resume. - */ -void set_pm1cnt_s5(void); - #endif /* __STONEYRIDGE_H__ */ diff --git a/src/soc/amd/stoneyridge/pmutil.c b/src/soc/amd/stoneyridge/pmutil.c index 7367251..59de348 100644 --- a/src/soc/amd/stoneyridge/pmutil.c +++ b/src/soc/amd/stoneyridge/pmutil.c @@ -25,29 +25,3 @@ /* If CMOS power has failed, the century will be set to 0xff */ return cmos_read(RTC_CLK_ALTCENTURY) == 0xff; } - -int vboot_platform_is_resuming(void) -{ - if (!(acpi_read16(MMIO_ACPI_PM1_STS) & WAK_STS)) - return 0; - - uint16_t pm_cnt = acpi_read16(MMIO_ACPI_PM1_CNT_BLK); - return acpi_sleep_from_pm1(pm_cnt) == ACPI_S3; -} - -/* If a system reset is about to be requested, modify the PM1 register so it - * will never be misinterpreted as an S3 resume. */ -void set_pm1cnt_s5(void) -{ - uint16_t pm1; - - pm1 = acpi_read16(MMIO_ACPI_PM1_CNT_BLK); - pm1 &= ~SLP_TYP; - pm1 |= SLP_TYP_S5 << SLP_TYP_SHIFT; - acpi_write16(MMIO_ACPI_PM1_CNT_BLK, pm1); -} - -void vboot_platform_prepare_reboot(void) -{ - set_pm1cnt_s5(); -} diff --git a/src/soc/amd/stoneyridge/smihandler.c b/src/soc/amd/stoneyridge/smihandler.c index c3aed57..e08aee4 100644 --- a/src/soc/amd/stoneyridge/smihandler.c +++ b/src/soc/amd/stoneyridge/smihandler.c @@ -25,6 +25,7 @@ #include <soc/smi.h> #include <soc/southbridge.h> #include <amdblocks/acpimmio.h> +#include <amdblocks/acpi.h> #include <elog.h>
/* bits in smm_io_trap */ @@ -88,19 +89,14 @@
static void sb_apmc_smi_handler(void) { - u32 reg32; const uint8_t cmd = inb(pm_acpi_smi_cmd_port());
switch (cmd) { case APM_CNT_ACPI_ENABLE: - reg32 = acpi_read32(MMIO_ACPI_PM1_CNT_BLK); - reg32 |= (1 << 0); /* SCI_EN */ - acpi_write32(MMIO_ACPI_PM1_CNT_BLK, reg32); + acpi_enable_sci(); break; case APM_CNT_ACPI_DISABLE: - reg32 = acpi_read32(MMIO_ACPI_PM1_CNT_BLK); - reg32 &= ~(1 << 0); /* clear SCI_EN */ - acpi_write32(MMIO_ACPI_PM1_CNT_BLK, reg32); + acpi_disable_sci(); break; case APM_CNT_ELOG_GSMI: if (CONFIG(ELOG_GSMI)) diff --git a/src/soc/amd/stoneyridge/southbridge.c b/src/soc/amd/stoneyridge/southbridge.c index cd91031..10a23f2 100644 --- a/src/soc/amd/stoneyridge/southbridge.c +++ b/src/soc/amd/stoneyridge/southbridge.c @@ -22,12 +22,12 @@ #include <device/pci_ids.h> #include <device/pci_ops.h> #include <cbmem.h> -#include <elog.h> #include <amdblocks/amd_pci_util.h> #include <amdblocks/agesawrapper.h> #include <amdblocks/reset.h> #include <amdblocks/acpimmio.h> #include <amdblocks/lpc.h> +#include <amdblocks/acpi.h> #include <soc/southbridge.h> #include <soc/smbus.h> #include <soc/smi.h> @@ -522,83 +522,6 @@ PM_ACPI_TIMER_EN_EN); }
-static uint16_t reset_pm1_status(void) -{ - uint16_t pm1_sts = acpi_read16(MMIO_ACPI_PM1_STS); - acpi_write16(MMIO_ACPI_PM1_STS, pm1_sts); - return pm1_sts; -} - -static uint16_t print_pm1_status(uint16_t pm1_sts) -{ - static const char *const pm1_sts_bits[16] = { - [0] = "TMROF", - [4] = "BMSTATUS", - [5] = "GBL", - [8] = "PWRBTN", - [10] = "RTC", - [14] = "PCIEXPWAK", - [15] = "WAK", - }; - - if (!pm1_sts) - return 0; - - printk(BIOS_DEBUG, "PM1_STS: "); - print_num_status_bits(ARRAY_SIZE(pm1_sts_bits), pm1_sts, pm1_sts_bits); - printk(BIOS_DEBUG, "\n"); - - return pm1_sts; -} - -static void sb_log_pm1_status(uint16_t pm1_sts) -{ - if (!CONFIG(ELOG)) - return; - - if (pm1_sts & WAK_STS) - elog_add_event_byte(ELOG_TYPE_ACPI_WAKE, - acpi_is_wakeup_s3() ? ACPI_S3 : ACPI_S5); - - if (pm1_sts & PWRBTN_STS) - elog_add_event_wake(ELOG_WAKE_SOURCE_PWRBTN, 0); - - if (pm1_sts & RTC_STS) - elog_add_event_wake(ELOG_WAKE_SOURCE_RTC, 0); - - if (pm1_sts & PCIEXPWAK_STS) - elog_add_event_wake(ELOG_WAKE_SOURCE_PCIE, 0); -} - -static void sb_save_sws(uint16_t pm1_status) -{ - struct soc_power_reg *sws; - uint32_t reg32; - uint16_t reg16; - - sws = cbmem_add(CBMEM_ID_POWER_STATE, sizeof(struct soc_power_reg)); - if (sws == NULL) - return; - sws->pm1_sts = pm1_status; - sws->pm1_en = acpi_read16(MMIO_ACPI_PM1_EN); - reg32 = acpi_read32(MMIO_ACPI_GPE0_STS); - acpi_write32(MMIO_ACPI_GPE0_STS, reg32); - sws->gpe0_sts = reg32; - sws->gpe0_en = acpi_read32(MMIO_ACPI_GPE0_EN); - reg16 = acpi_read16(MMIO_ACPI_PM1_CNT_BLK); - reg16 &= SLP_TYP; - sws->wake_from = reg16 >> SLP_TYP_SHIFT; -} - -static void sb_clear_pm1_status(void) -{ - uint16_t pm1_sts = reset_pm1_status(); - - sb_save_sws(pm1_sts); - sb_log_pm1_status(pm1_sts); - print_pm1_status(pm1_sts); -} - static int get_index_bit(uint32_t value, uint16_t limit) { uint16_t i; @@ -651,7 +574,7 @@ void southbridge_init(void *chip_info) { sb_init_acpi_ports(); - sb_clear_pm1_status(); + acpi_clear_pm1_status(); }
static void set_sb_final_nvs(void)
-
Furquan Shaikh (Code Review) -
Marshall Dawson (Code Review)
-
Martin Roth (Code Review)
-
Richard Spiegel (Code Review)