Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34964 )
Change subject: google/rambi,intel/baytrail: Simplified romstage flow ......................................................................
google/rambi,intel/baytrail: Simplified romstage flow
Change-Id: I99440539d7b7586df66395776dcd0b4f72f66818 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/mainboard/google/rambi/romstage.c M src/soc/intel/baytrail/include/soc/romstage.h M src/soc/intel/baytrail/romstage/Makefile.inc M src/soc/intel/baytrail/romstage/romstage.c 4 files changed, 19 insertions(+), 71 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/34964/1
diff --git a/src/mainboard/google/rambi/romstage.c b/src/mainboard/google/rambi/romstage.c index 9fbe1ca..f74d77d 100644 --- a/src/mainboard/google/rambi/romstage.c +++ b/src/mainboard/google/rambi/romstage.c @@ -55,21 +55,13 @@ return &spd_file_content[SPD_SIZE * ram_id]; }
-void mainboard_romstage_entry_rp(struct romstage_params *rp) +void mainboard_fill_mrc_params(struct mrc_params *mp) { void *spd_content; int dual_channel = 0; void *spd_file; size_t spd_fsize;
- struct mrc_params mp = { - .mainboard = { - .dram_type = DRAM_DDR3L, - .dram_info_location = DRAM_INFO_SPD_MEM, - .weaker_odt_settings = 1, - }, - }; - spd_file = cbfs_boot_map_with_leak("spd.bin", CBFS_TYPE_SPD, &spd_fsize); if (!spd_file) @@ -77,10 +69,12 @@
spd_content = get_spd_pointer(spd_file, spd_fsize / SPD_SIZE, &dual_channel); - mp.mainboard.dram_data[0] = spd_content; - if (dual_channel) - mp.mainboard.dram_data[1] = spd_content;
- rp->mrc_params = ∓ - romstage_common(rp); + mp->mainboard.dram_type = DRAM_DDR3L; + mp->mainboard.dram_info_location = DRAM_INFO_SPD_MEM, + mp->mainboard.weaker_odt_settings = 1, + + mp->mainboard.dram_data[0] = spd_content; + if (dual_channel) + mp->mainboard.dram_data[1] = spd_content; } diff --git a/src/soc/intel/baytrail/include/soc/romstage.h b/src/soc/intel/baytrail/include/soc/romstage.h index 305cf27..ea7a2a5 100644 --- a/src/soc/intel/baytrail/include/soc/romstage.h +++ b/src/soc/intel/baytrail/include/soc/romstage.h @@ -24,12 +24,7 @@ #include <arch/cpu.h> #include <soc/mrc_wrapper.h>
-struct romstage_params { - struct mrc_params *mrc_params; -}; - -void mainboard_romstage_entry_rp(struct romstage_params *params); -void romstage_common(struct romstage_params *params); +void mainboard_fill_mrc_params(struct mrc_params *mp);
void raminit(struct mrc_params *mp, int prev_sleep_state); void gfx_init(void); diff --git a/src/soc/intel/baytrail/romstage/Makefile.inc b/src/soc/intel/baytrail/romstage/Makefile.inc index d43a6fb..2effbb0 100644 --- a/src/soc/intel/baytrail/romstage/Makefile.inc +++ b/src/soc/intel/baytrail/romstage/Makefile.inc @@ -1,5 +1,7 @@ cpu_incs-y += $(src)/cpu/intel/car/non-evict/cache_as_ram.S cpu_incs-y += $(obj)/fmap_config.h + +romstage-y += ../../../../cpu/intel/car/romstage.c romstage-y += romstage.c romstage-y += raminit.c romstage-$(CONFIG_ENABLE_BUILTIN_COM1) += uart.c diff --git a/src/soc/intel/baytrail/romstage/romstage.c b/src/soc/intel/baytrail/romstage/romstage.c index 151956e..27ed58f 100644 --- a/src/soc/intel/baytrail/romstage/romstage.c +++ b/src/soc/intel/baytrail/romstage/romstage.c @@ -14,21 +14,16 @@ */
#include <stddef.h> -#include <arch/cpu.h> #include <arch/io.h> #include <arch/romstage.h> #include <device/mmio.h> #include <device/pci_ops.h> -#include <bootblock_common.h> #include <console/console.h> #include <cbmem.h> -#include <cpu/x86/mtrr.h> -#include <cpu/x86/smm.h> #if CONFIG(EC_GOOGLE_CHROMEEC) #include <ec/google/chromeec/ec.h> #endif #include <elog.h> -#include <program_loading.h> #include <romstage_handoff.h> #include <string.h> #include <timestamp.h> @@ -155,28 +150,12 @@ return prev_sleep_state; }
-/* The cache-as-ram assembly file calls romstage_main() after setting up - * cache-as-ram. romstage_main() will then call the mainboards's - * mainboard_romstage_entry() function. That function then calls - * romstage_common() below. The reason for the back and forth is to provide - * common entry point from cache-as-ram while still allowing for code sharing. - * Because we can't use global variables the stack is used for allocations -- - * thus the need to call back and forth. */ - -static struct postcar_frame early_mtrrs; - -/* Entry from cache-as-ram.inc. */ -static void romstage_main(uint64_t tsc) +/* Entry from cpu/intel/car/romstage.c */ +void mainboard_romstage_entry(void) { - struct romstage_params rp = { - .mrc_params = NULL, - }; - - /* Save initial timestamp from bootblock. */ - timestamp_init(tsc); - - /* Save romstage begin */ - timestamp_add_now(TS_START_ROMSTAGE); + struct chipset_power_state *ps; + int prev_sleep_state; + struct mrc_params mp;
program_base_addresses();
@@ -194,21 +173,8 @@
gfx_init();
- /* Call into mainboard. */ - mainboard_romstage_entry_rp(&rp); - - if (CONFIG(SMM_TSEG)) - smm_list_regions(); - - prepare_and_run_postcar(&early_mtrrs); - /* We do not return here. */ -} - -/* Entry from the mainboard. */ -void romstage_common(struct romstage_params *params) -{ - struct chipset_power_state *ps; - int prev_sleep_state; + memset(&mp, 0, sizeof(mp)); + mainboard_fill_mrc_params(&mp);
timestamp_add_now(TS_BEFORE_INITRAM);
@@ -222,19 +188,10 @@ boot_count_increment(); #endif
- /* Initialize RAM */ - raminit(params->mrc_params, prev_sleep_state); + raminit(&mp, prev_sleep_state);
timestamp_add_now(TS_AFTER_INITRAM);
romstage_handoff_init(prev_sleep_state == ACPI_S3); } - -/* This wrapper enables easy transition towards C_ENVIRONMENT_BOOTBLOCK, - * keeping changes in cache_as_ram.S easy to manage. - */ -asmlinkage void bootblock_c_entry_bist(uint64_t base_timestamp, uint32_t bist) -{ - romstage_main(base_timestamp); -}
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34964 )
Change subject: google/rambi,intel/baytrail: Simplified romstage flow ......................................................................
Patch Set 3:
Arthur, sorry I did not notice your work on same topic.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34964 )
Change subject: google/rambi,intel/baytrail: Simplified romstage flow ......................................................................
Patch Set 8: Code-Review+2
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34964 )
Change subject: google/rambi,intel/baytrail: Simplified romstage flow ......................................................................
Patch Set 8: Code-Review+1
tested on google/clapper, doesn't appear to have broken anything
Kyösti Mälkki has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34964 )
Change subject: google/rambi,intel/baytrail: Simplified romstage flow ......................................................................
google/rambi,intel/baytrail: Simplified romstage flow
Change-Id: I99440539d7b7586df66395776dcd0b4f72f66818 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34964 Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Matt DeVillier matt.devillier@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/rambi/romstage.c M src/soc/intel/baytrail/include/soc/romstage.h M src/soc/intel/baytrail/romstage/Makefile.inc M src/soc/intel/baytrail/romstage/romstage.c 4 files changed, 19 insertions(+), 71 deletions(-)
Approvals: build bot (Jenkins): Verified Matt DeVillier: Looks good to me, but someone else must approve Angel Pons: Looks good to me, approved
diff --git a/src/mainboard/google/rambi/romstage.c b/src/mainboard/google/rambi/romstage.c index 9fbe1ca..f74d77d 100644 --- a/src/mainboard/google/rambi/romstage.c +++ b/src/mainboard/google/rambi/romstage.c @@ -55,21 +55,13 @@ return &spd_file_content[SPD_SIZE * ram_id]; }
-void mainboard_romstage_entry_rp(struct romstage_params *rp) +void mainboard_fill_mrc_params(struct mrc_params *mp) { void *spd_content; int dual_channel = 0; void *spd_file; size_t spd_fsize;
- struct mrc_params mp = { - .mainboard = { - .dram_type = DRAM_DDR3L, - .dram_info_location = DRAM_INFO_SPD_MEM, - .weaker_odt_settings = 1, - }, - }; - spd_file = cbfs_boot_map_with_leak("spd.bin", CBFS_TYPE_SPD, &spd_fsize); if (!spd_file) @@ -77,10 +69,12 @@
spd_content = get_spd_pointer(spd_file, spd_fsize / SPD_SIZE, &dual_channel); - mp.mainboard.dram_data[0] = spd_content; - if (dual_channel) - mp.mainboard.dram_data[1] = spd_content;
- rp->mrc_params = ∓ - romstage_common(rp); + mp->mainboard.dram_type = DRAM_DDR3L; + mp->mainboard.dram_info_location = DRAM_INFO_SPD_MEM, + mp->mainboard.weaker_odt_settings = 1, + + mp->mainboard.dram_data[0] = spd_content; + if (dual_channel) + mp->mainboard.dram_data[1] = spd_content; } diff --git a/src/soc/intel/baytrail/include/soc/romstage.h b/src/soc/intel/baytrail/include/soc/romstage.h index ae38b70..93669f2 100644 --- a/src/soc/intel/baytrail/include/soc/romstage.h +++ b/src/soc/intel/baytrail/include/soc/romstage.h @@ -24,12 +24,7 @@ #include <arch/cpu.h> #include <soc/mrc_wrapper.h>
-struct romstage_params { - struct mrc_params *mrc_params; -}; - -void mainboard_romstage_entry_rp(struct romstage_params *params); -void romstage_common(struct romstage_params *params); +void mainboard_fill_mrc_params(struct mrc_params *mp);
void raminit(struct mrc_params *mp, int prev_sleep_state); void gfx_init(void); diff --git a/src/soc/intel/baytrail/romstage/Makefile.inc b/src/soc/intel/baytrail/romstage/Makefile.inc index d43a6fb..2effbb0 100644 --- a/src/soc/intel/baytrail/romstage/Makefile.inc +++ b/src/soc/intel/baytrail/romstage/Makefile.inc @@ -1,5 +1,7 @@ cpu_incs-y += $(src)/cpu/intel/car/non-evict/cache_as_ram.S cpu_incs-y += $(obj)/fmap_config.h + +romstage-y += ../../../../cpu/intel/car/romstage.c romstage-y += romstage.c romstage-y += raminit.c romstage-$(CONFIG_ENABLE_BUILTIN_COM1) += uart.c diff --git a/src/soc/intel/baytrail/romstage/romstage.c b/src/soc/intel/baytrail/romstage/romstage.c index 80915fd..7348d2b 100644 --- a/src/soc/intel/baytrail/romstage/romstage.c +++ b/src/soc/intel/baytrail/romstage/romstage.c @@ -14,21 +14,16 @@ */
#include <stddef.h> -#include <arch/cpu.h> #include <arch/io.h> #include <arch/romstage.h> #include <device/mmio.h> #include <device/pci_ops.h> -#include <bootblock_common.h> #include <console/console.h> #include <cbmem.h> -#include <cpu/x86/mtrr.h> -#include <cpu/x86/smm.h> #if CONFIG(EC_GOOGLE_CHROMEEC) #include <ec/google/chromeec/ec.h> #endif #include <elog.h> -#include <program_loading.h> #include <romstage_handoff.h> #include <string.h> #include <timestamp.h> @@ -156,28 +151,12 @@ return prev_sleep_state; }
-/* The cache-as-ram assembly file calls romstage_main() after setting up - * cache-as-ram. romstage_main() will then call the mainboards's - * mainboard_romstage_entry() function. That function then calls - * romstage_common() below. The reason for the back and forth is to provide - * common entry point from cache-as-ram while still allowing for code sharing. - * Because we can't use global variables the stack is used for allocations -- - * thus the need to call back and forth. */ - -static struct postcar_frame early_mtrrs; - -/* Entry from cache-as-ram.inc. */ -static void romstage_main(uint64_t tsc) +/* Entry from cpu/intel/car/romstage.c */ +void mainboard_romstage_entry(void) { - struct romstage_params rp = { - .mrc_params = NULL, - }; - - /* Save initial timestamp from bootblock. */ - timestamp_init(tsc); - - /* Save romstage begin */ - timestamp_add_now(TS_START_ROMSTAGE); + struct chipset_power_state *ps; + int prev_sleep_state; + struct mrc_params mp;
program_base_addresses();
@@ -196,21 +175,8 @@
gfx_init();
- /* Call into mainboard. */ - mainboard_romstage_entry_rp(&rp); - - if (CONFIG(SMM_TSEG)) - smm_list_regions(); - - prepare_and_run_postcar(&early_mtrrs); - /* We do not return here. */ -} - -/* Entry from the mainboard. */ -void romstage_common(struct romstage_params *params) -{ - struct chipset_power_state *ps; - int prev_sleep_state; + memset(&mp, 0, sizeof(mp)); + mainboard_fill_mrc_params(&mp);
timestamp_add_now(TS_BEFORE_INITRAM);
@@ -224,19 +190,10 @@ boot_count_increment(); #endif
- /* Initialize RAM */ - raminit(params->mrc_params, prev_sleep_state); + raminit(&mp, prev_sleep_state);
timestamp_add_now(TS_AFTER_INITRAM);
romstage_handoff_init(prev_sleep_state == ACPI_S3); } - -/* This wrapper enables easy transition towards C_ENVIRONMENT_BOOTBLOCK, - * keeping changes in cache_as_ram.S easy to manage. - */ -asmlinkage void bootblock_c_entry_bist(uint64_t base_timestamp, uint32_t bist) -{ - romstage_main(base_timestamp); -}
-
Angel Pons (Code Review) -
Kyösti Mälkki (Code Review)
-
Matt DeVillier (Code Review)