Hi Simon,
On Wed, Mar 22, 2017 at 4:06 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 16 March 2017 at 08:26, Bin Meng bmeng.cn@gmail.com wrote:
At the end of pre-relocation phase, save the new stack address to CMOS and use it as the stack on next S3 boot for fsp_init() continuation function.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/cpu.c | 8 ++++++++ arch/x86/include/asm/cmos_layout.h | 31 +++++++++++++++++++++++++++++++ arch/x86/include/asm/u-boot-x86.h | 1 + arch/x86/lib/fsp/fsp_common.c | 30 +++++++++++++++++++++++++++++- 4 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 arch/x86/include/asm/cmos_layout.h
Reviewed-by: Simon Glass sjg@chromium.org
nits below
diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c index afc8645..9e2aee2 100644 --- a/arch/x86/cpu/cpu.c +++ b/arch/x86/cpu/cpu.c @@ -278,6 +278,14 @@ int reserve_arch(void) high_table_reserve(); #endif
+#if defined(CONFIG_HAVE_ACPI_RESUME) && defined(CONFIG_HAVE_FSP)
/** Save stack address to CMOS so that at next S3 boot,* we can use it as the stack address for fsp_contiue()*/fsp_save_s3_stack();+#endif
return 0;} #endif diff --git a/arch/x86/include/asm/cmos_layout.h b/arch/x86/include/asm/cmos_layout.h new file mode 100644 index 0000000..0a0a51e --- /dev/null +++ b/arch/x86/include/asm/cmos_layout.h @@ -0,0 +1,31 @@ +/*
- Copyright (C) 2017, Bin Meng bmeng.cn@gmail.com
- SPDX-License-Identifier: GPL-2.0+
- */
+#ifndef __CMOS_LAYOUT_H +#define __CMOS_LAYOUT_H
+/*
- The RTC internal registers and RAM is organized as two banks of 128 bytes
- each, called the standard and extended banks. The first 14 bytes of the
- standard bank contain the RTC time and date information along with four
- registers, A - D, that are used for configuration of the RTC. The extended
- bank contains a full 128 bytes of battery backed SRAM.
- For simplicity in U-Boot we only support CMOS in the standard bank, and
- its base address starts from offset 0x10, which leaves us 112 bytes space.
- */
+#define CMOS_BASE 0x10
+/*
- The file records all offsets off CMOS_BASE that is currently used by
- U-Boot for various reasons. It is put in such a unified place in order
- to be consistent across platforms.
- */
+/* stack address for S3 boot in a FSP configuration, 4 bytes */ +#define CMOS_FSP_STACK_ADDR CMOS_BASE
+#endif /* __CMOS_LAYOUT_H */ diff --git a/arch/x86/include/asm/u-boot-x86.h b/arch/x86/include/asm/u-boot-x86.h index 4f901f9..024aaf4 100644 --- a/arch/x86/include/asm/u-boot-x86.h +++ b/arch/x86/include/asm/u-boot-x86.h @@ -56,6 +56,7 @@ u32 isa_map_rom(u32 bus_addr, int size); int video_bios_init(void);
/* arch/x86/lib/fsp/... */ +int fsp_save_s3_stack(void); int x86_fsp_init(void);
Function comment?
void board_init_f_r_trampoline(ulong) __attribute__ ((noreturn)); diff --git a/arch/x86/lib/fsp/fsp_common.c b/arch/x86/lib/fsp/fsp_common.c index 2b33fba..df73a2a 100644 --- a/arch/x86/lib/fsp/fsp_common.c +++ b/arch/x86/lib/fsp/fsp_common.c @@ -5,8 +5,12 @@ */
#include <common.h> +#include <dm.h> #include <errno.h> +#include <rtc.h> #include <asm/acpi_s3.h> +#include <asm/cmos_layout.h> +#include <asm/early_cmos.h> #include <asm/io.h> #include <asm/mrccache.h> #include <asm/post.h> @@ -71,9 +75,32 @@ static __maybe_unused void *fsp_prepare_mrc_cache(void) return cache->data; }
+#ifdef CONFIG_HAVE_ACPI_RESUME +int fsp_save_s3_stack(void) +{
struct udevice *dev;int ret;if (gd->arch.prev_sleep_state == ACPI_S3)return 0;ret = uclass_get_device(UCLASS_RTC, 0, &dev);You need to use uclass_get_device_err() to get an error I think.
There is no such API in current mainline.
if (ret) {debug("Cannot find RTC: err=%d\n", ret);return -ENODEV;}/* Save the stack address to CMOS */rtc_write32(dev, CMOS_FSP_STACK_ADDR, gd->start_addr_sp);Check error?
return 0;+} +#endif
int x86_fsp_init(void) { void *nvs;
int stack = CONFIG_FSP_TEMP_RAM_ADDR; int boot_mode = BOOT_FULL_CONFIG;#ifdef CONFIG_HAVE_ACPI_RESUME int prev_sleep_state = chipset_prev_sleep_state(); @@ -102,6 +129,7 @@ int x86_fsp_init(void) panic("Reboot System"); }
stack = cmos_read32(CMOS_FSP_STACK_ADDR);Perhaps you should comment why you are using this instead of rtc_...() ?
Regards, Bin