Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32760
Change subject: soc/intel/broadwell: Clean up the bootflow ......................................................................
soc/intel/broadwell: Clean up the bootflow
Call the raminit from a common location instead of from the mainboard specific code.
Change-Id: I65d522237a0bb7b2c032536ede10e2cf93c134d8 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/mainboard/google/auron/romstage.c M src/mainboard/google/jecht/romstage.c M src/mainboard/google/slippy/romstage.c M src/mainboard/intel/wtm2/romstage.c M src/mainboard/purism/librem_bdw/romstage.c M src/soc/intel/broadwell/include/soc/romstage.h M src/soc/intel/broadwell/romstage/romstage.c 7 files changed, 18 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/32760/1
diff --git a/src/mainboard/google/auron/romstage.c b/src/mainboard/google/auron/romstage.c index 5e1a66a..d56f357 100644 --- a/src/mainboard/google/auron/romstage.c +++ b/src/mainboard/google/auron/romstage.c @@ -27,7 +27,7 @@ { }
-void mainboard_romstage_entry(struct romstage_params *rp) +void mainboard_pre_raminit(struct romstage_params *rp) { struct pei_data pei_data;
@@ -38,10 +38,10 @@ mainboard_fill_pei_data(&pei_data); mainboard_fill_spd_data(&pei_data); rp->pei_data = &pei_data; +}
- /* Call into the real romstage main with this board's attributes. */ - romstage_common(rp); - +void mainboard_post_raminit(struct romstage_params *rp) +{ /* Do variant-specific init */ variant_romstage_entry(rp); } diff --git a/src/mainboard/google/jecht/romstage.c b/src/mainboard/google/jecht/romstage.c index de0ed30..e906666 100644 --- a/src/mainboard/google/jecht/romstage.c +++ b/src/mainboard/google/jecht/romstage.c @@ -27,7 +27,7 @@ #include "onboard.h"
-void mainboard_romstage_entry(struct romstage_params *rp) +void mainboard_pre_raminit(struct romstage_params *rp) { struct pei_data pei_data;
@@ -38,10 +38,10 @@ mainboard_fill_pei_data(&pei_data); mainboard_fill_spd_data(&pei_data); rp->pei_data = &pei_data; +}
- /* Call into the real romstage main with this board's attributes. */ - romstage_common(rp); - +void mainboard_post_raminit(struct romstage_params *rp) +{ if (CONFIG(CHROMEOS)) init_bootmode_straps(); } diff --git a/src/mainboard/google/slippy/romstage.c b/src/mainboard/google/slippy/romstage.c index ec5d5ea..efa3a07 100644 --- a/src/mainboard/google/slippy/romstage.c +++ b/src/mainboard/google/slippy/romstage.c @@ -20,5 +20,5 @@
void mainboard_romstage_entry(unsigned long bist) { - variant_romstage_entry(bist); + variant_romstage_entry(bist); } diff --git a/src/mainboard/intel/wtm2/romstage.c b/src/mainboard/intel/wtm2/romstage.c index de4237d..223621a 100644 --- a/src/mainboard/intel/wtm2/romstage.c +++ b/src/mainboard/intel/wtm2/romstage.c @@ -22,7 +22,7 @@ #include <soc/pei_wrapper.h> #include <soc/romstage.h>
-void mainboard_romstage_entry(struct romstage_params *rp) +void mainboard_pre_raminit(struct romstage_params *rp) { struct pei_data pei_data;
@@ -32,6 +32,4 @@ memset(&pei_data, 0, sizeof(pei_data)); mainboard_fill_pei_data(&pei_data); rp->pei_data = &pei_data; - - romstage_common(rp); } diff --git a/src/mainboard/purism/librem_bdw/romstage.c b/src/mainboard/purism/librem_bdw/romstage.c index 6591229..9aae83f 100644 --- a/src/mainboard/purism/librem_bdw/romstage.c +++ b/src/mainboard/purism/librem_bdw/romstage.c @@ -26,7 +26,4 @@ memset(&pei_data, 0, sizeof(pei_data)); mainboard_fill_pei_data(&pei_data); rp->pei_data = &pei_data; - - /* Initialize memory */ - romstage_common(rp); } diff --git a/src/soc/intel/broadwell/include/soc/romstage.h b/src/soc/intel/broadwell/include/soc/romstage.h index 31184f9..fd95e92 100644 --- a/src/soc/intel/broadwell/include/soc/romstage.h +++ b/src/soc/intel/broadwell/include/soc/romstage.h @@ -27,7 +27,8 @@ struct pei_data *pei_data; };
-void mainboard_romstage_entry(struct romstage_params *params); +void mainboard_pre_raminit(struct romstage_params *params); +void mainboard_post_raminit(struct romstage_params *params); void romstage_common(struct romstage_params *params);
void raminit(struct pei_data *pei_data); diff --git a/src/soc/intel/broadwell/romstage/romstage.c b/src/soc/intel/broadwell/romstage/romstage.c index 7847829..8ad8513 100644 --- a/src/soc/intel/broadwell/romstage/romstage.c +++ b/src/soc/intel/broadwell/romstage/romstage.c @@ -104,8 +104,12 @@ /* Initialize GPIOs */ init_gpios(mainboard_gpio_config);
- /* Call into mainboard. */ - mainboard_romstage_entry(&rp); + /* Fill in mainboard pei_date. */ + mainboard_pre_raminit(&rp); + + romstage_common(&rp); + + mainboard_post_raminit(&rp);
platform_enter_postcar(); }
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32760 )
Change subject: soc/intel/broadwell: Clean up the bootflow ......................................................................
Patch Set 2:
(5 comments)
https://review.coreboot.org/#/c/32760/1/src/mainboard/google/auron/romstage.... File src/mainboard/google/auron/romstage.c:
https://review.coreboot.org/#/c/32760/1/src/mainboard/google/auron/romstage.... PS1, Line 41: } oops, `pei_data` is gone :-/
https://review.coreboot.org/#/c/32760/1/src/mainboard/google/auron/romstage.... PS1, Line 46: _entry Change makes the weird naming obvious ;)
https://review.coreboot.org/#/c/32760/1/src/soc/intel/broadwell/romstage/rom... File src/soc/intel/broadwell/romstage/romstage.c:
https://review.coreboot.org/#/c/32760/1/src/soc/intel/broadwell/romstage/rom... PS1, Line 125: /* Entry from the mainboard. */ Not true anymore.
https://review.coreboot.org/#/c/32760/1/src/soc/intel/broadwell/romstage/rom... PS1, Line 126: void romstage_common(struct romstage_params *params) Maybe merge it into romstage_main()?
https://review.coreboot.org/#/c/32760/1/src/soc/intel/broadwell/romstage/rom... PS1, Line 128: post_code(0x32); Redundant with some mainboards.
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32760
to look at the new patch set (#4).
Change subject: soc/intel/broadwell: Clean up the bootflow ......................................................................
soc/intel/broadwell: Clean up the bootflow
Call the raminit from a common location instead of from the mainboard specific code.
Change-Id: I65d522237a0bb7b2c032536ede10e2cf93c134d8 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/mainboard/google/auron/romstage.c M src/mainboard/google/jecht/romstage.c M src/mainboard/intel/wtm2/romstage.c M src/mainboard/purism/librem_bdw/romstage.c M src/soc/intel/broadwell/include/soc/romstage.h M src/soc/intel/broadwell/romstage/romstage.c 6 files changed, 46 insertions(+), 50 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/32760/4
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32760
to look at the new patch set (#5).
Change subject: soc/intel/broadwell: Clean up the bootflow ......................................................................
soc/intel/broadwell: Clean up the bootflow
Call the raminit from a common location instead of from the mainboard specific code.
Change-Id: I65d522237a0bb7b2c032536ede10e2cf93c134d8 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/mainboard/google/auron/romstage.c M src/mainboard/google/jecht/romstage.c M src/mainboard/intel/wtm2/romstage.c M src/mainboard/purism/librem_bdw/romstage.c M src/soc/intel/broadwell/include/soc/romstage.h M src/soc/intel/broadwell/romstage/romstage.c 6 files changed, 46 insertions(+), 50 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/32760/5
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32760 )
Change subject: soc/intel/broadwell: Clean up the bootflow ......................................................................
Patch Set 5: Code-Review+2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32760 )
Change subject: soc/intel/broadwell: Clean up the bootflow ......................................................................
Patch Set 6:
(1 comment)
Looking for something to bikeshed so we can retrigger Jenkins ;)
https://review.coreboot.org/#/c/32760/6/src/soc/intel/broadwell/romstage/rom... File src/soc/intel/broadwell/romstage/romstage.c:
https://review.coreboot.org/#/c/32760/6/src/soc/intel/broadwell/romstage/rom... PS6, Line 115: #if CONFIG(ELOG_BOOT_COUNT) Use C `if ()`.
Hello Patrick Rudolph, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32760
to look at the new patch set (#7).
Change subject: soc/intel/broadwell: Clean up the bootflow ......................................................................
soc/intel/broadwell: Clean up the bootflow
Call the raminit from a common location instead of from the mainboard specific code.
Change-Id: I65d522237a0bb7b2c032536ede10e2cf93c134d8 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/mainboard/google/auron/romstage.c M src/mainboard/google/jecht/romstage.c M src/mainboard/intel/wtm2/romstage.c M src/mainboard/purism/librem_bdw/romstage.c M src/soc/intel/broadwell/include/soc/romstage.h M src/soc/intel/broadwell/romstage/romstage.c 6 files changed, 44 insertions(+), 50 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/32760/7
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32760 )
Change subject: soc/intel/broadwell: Clean up the bootflow ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/#/c/32760/7/src/soc/intel/broadwell/romstage/rom... File src/soc/intel/broadwell/romstage/romstage.c:
https://review.coreboot.org/#/c/32760/7/src/soc/intel/broadwell/romstage/rom... PS7, Line 115: if (CONFIG(ELOG_BOOT_COUNT && rp.power_state->prev_sleep_state != ACPI_S3) line over 80 characters
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32760 )
Change subject: soc/intel/broadwell: Clean up the bootflow ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/#/c/32760/6/src/mainboard/purism/librem_bdw/roms... File src/mainboard/purism/librem_bdw/romstage.c:
https://review.coreboot.org/#/c/32760/6/src/mainboard/purism/librem_bdw/roms... PS6, Line 21: mainboard_romstage_entry Oh, this is still wrong anyway...
Hello Patrick Rudolph, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32760
to look at the new patch set (#8).
Change subject: soc/intel/broadwell: Clean up the bootflow ......................................................................
soc/intel/broadwell: Clean up the bootflow
Call the raminit from a common location instead of from the mainboard specific code.
Change-Id: I65d522237a0bb7b2c032536ede10e2cf93c134d8 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/mainboard/google/auron/romstage.c M src/mainboard/google/jecht/romstage.c M src/mainboard/intel/wtm2/romstage.c M src/mainboard/purism/librem_bdw/romstage.c M src/soc/intel/broadwell/include/soc/romstage.h M src/soc/intel/broadwell/romstage/romstage.c 6 files changed, 45 insertions(+), 51 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/32760/8
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32760 )
Change subject: soc/intel/broadwell: Clean up the bootflow ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/#/c/32760/8/src/soc/intel/broadwell/romstage/rom... File src/soc/intel/broadwell/romstage/romstage.c:
https://review.coreboot.org/#/c/32760/8/src/soc/intel/broadwell/romstage/rom... PS8, Line 115: if (CONFIG(ELOG_BOOT_COUNT) && rp.power_state->prev_sleep_state != ACPI_S3) line over 80 characters
Hello Patrick Rudolph, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32760
to look at the new patch set (#9).
Change subject: soc/intel/broadwell: Clean up the bootflow ......................................................................
soc/intel/broadwell: Clean up the bootflow
Call the raminit from a common location instead of from the mainboard specific code.
Change-Id: I65d522237a0bb7b2c032536ede10e2cf93c134d8 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/mainboard/google/auron/romstage.c M src/mainboard/google/jecht/romstage.c M src/mainboard/intel/wtm2/romstage.c M src/mainboard/purism/librem_bdw/romstage.c M src/soc/intel/broadwell/include/soc/romstage.h M src/soc/intel/broadwell/romstage/romstage.c 6 files changed, 46 insertions(+), 51 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/32760/9
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32760 )
Change subject: soc/intel/broadwell: Clean up the bootflow ......................................................................
Patch Set 8: Code-Review+2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32760 )
Change subject: soc/intel/broadwell: Clean up the bootflow ......................................................................
Patch Set 9: Code-Review+2
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32760 )
Change subject: soc/intel/broadwell: Clean up the bootflow ......................................................................
Patch Set 9: Code-Review+1
Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32760 )
Change subject: soc/intel/broadwell: Clean up the bootflow ......................................................................
soc/intel/broadwell: Clean up the bootflow
Call the raminit from a common location instead of from the mainboard specific code.
Change-Id: I65d522237a0bb7b2c032536ede10e2cf93c134d8 Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/32760 Reviewed-by: Nico Huber nico.h@gmx.de Reviewed-by: Matt DeVillier matt.devillier@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/auron/romstage.c M src/mainboard/google/jecht/romstage.c M src/mainboard/intel/wtm2/romstage.c M src/mainboard/purism/librem_bdw/romstage.c M src/soc/intel/broadwell/include/soc/romstage.h M src/soc/intel/broadwell/romstage/romstage.c 6 files changed, 46 insertions(+), 51 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Matt DeVillier: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/auron/romstage.c b/src/mainboard/google/auron/romstage.c index 4974899..568c4c8 100644 --- a/src/mainboard/google/auron/romstage.c +++ b/src/mainboard/google/auron/romstage.c @@ -27,17 +27,16 @@ { }
-void mainboard_romstage_entry(struct romstage_params *rp) +void mainboard_pre_raminit(struct romstage_params *rp) { - post_code(0x32); - /* Fill out PEI DATA */ mainboard_fill_pei_data(&rp->pei_data); mainboard_fill_spd_data(&rp->pei_data);
- /* Call into the real romstage main with this board's attributes. */ - romstage_common(rp); +}
+void mainboard_post_raminit(struct romstage_params *rp) +{ /* Do variant-specific init */ variant_romstage_entry(rp); } diff --git a/src/mainboard/google/jecht/romstage.c b/src/mainboard/google/jecht/romstage.c index 8d1ae8a..86888c8 100644 --- a/src/mainboard/google/jecht/romstage.c +++ b/src/mainboard/google/jecht/romstage.c @@ -27,17 +27,15 @@ #include "onboard.h"
-void mainboard_romstage_entry(struct romstage_params *rp) +void mainboard_pre_raminit(struct romstage_params *rp) { - post_code(0x32); - /* Fill out PEI DATA */ mainboard_fill_pei_data(&rp->pei_data); mainboard_fill_spd_data(&rp->pei_data); +}
- /* Call into the real romstage main with this board's attributes. */ - romstage_common(rp); - +void mainboard_post_raminit(struct romstage_params *rp) +{ if (CONFIG(CHROMEOS)) init_bootmode_straps(); } diff --git a/src/mainboard/intel/wtm2/romstage.c b/src/mainboard/intel/wtm2/romstage.c index 5b8df27..f4e3366 100644 --- a/src/mainboard/intel/wtm2/romstage.c +++ b/src/mainboard/intel/wtm2/romstage.c @@ -22,12 +22,12 @@ #include <soc/pei_wrapper.h> #include <soc/romstage.h>
-void mainboard_romstage_entry(struct romstage_params *rp) +void mainboard_pre_raminit(struct romstage_params *rp) { - post_code(0x32); - /* Fill out PEI DATA */ mainboard_fill_pei_data(&rp->pei_data); +}
- romstage_common(rp); +void mainboard_post_raminit(struct romstage_params *rp) +{ } diff --git a/src/mainboard/purism/librem_bdw/romstage.c b/src/mainboard/purism/librem_bdw/romstage.c index 5330d191..0e1ad88 100644 --- a/src/mainboard/purism/librem_bdw/romstage.c +++ b/src/mainboard/purism/librem_bdw/romstage.c @@ -18,11 +18,12 @@ #include <soc/pei_wrapper.h> #include <soc/romstage.h>
-void mainboard_romstage_entry(struct romstage_params *rp) +void mainboard_pre_raminit(struct romstage_params *rp) { /* Fill out PEI DATA */ mainboard_fill_pei_data(&rp->pei_data); +}
- /* Initialize memory */ - romstage_common(rp); +void mainboard_post_raminit(struct romstage_params *rp) +{ } diff --git a/src/soc/intel/broadwell/include/soc/romstage.h b/src/soc/intel/broadwell/include/soc/romstage.h index 46f29d6..d65692a 100644 --- a/src/soc/intel/broadwell/include/soc/romstage.h +++ b/src/soc/intel/broadwell/include/soc/romstage.h @@ -27,8 +27,8 @@ struct pei_data pei_data; };
-void mainboard_romstage_entry(struct romstage_params *params); -void romstage_common(struct romstage_params *params); +void mainboard_pre_raminit(struct romstage_params *params); +void mainboard_post_raminit(struct romstage_params *params);
void raminit(struct pei_data *pei_data);
diff --git a/src/soc/intel/broadwell/romstage/romstage.c b/src/soc/intel/broadwell/romstage/romstage.c index 2a3ac8b..acbca14 100644 --- a/src/soc/intel/broadwell/romstage/romstage.c +++ b/src/soc/intel/broadwell/romstage/romstage.c @@ -103,8 +103,34 @@ /* Initialize GPIOs */ init_gpios(mainboard_gpio_config);
- /* Call into mainboard. */ - mainboard_romstage_entry(&rp); + /* Fill in mainboard pei_date. */ + mainboard_pre_raminit(&rp); + + post_code(0x32); + + timestamp_add_now(TS_BEFORE_INITRAM); + + rp.pei_data.boot_mode = rp.power_state->prev_sleep_state; + + if (CONFIG(ELOG_BOOT_COUNT) + && rp.power_state->prev_sleep_state != ACPI_S3) + boot_count_increment(); + + /* Print ME state before MRC */ + intel_me_status(); + + /* Save ME HSIO version */ + intel_me_hsio_version(&rp.power_state->hsio_version, + &rp.power_state->hsio_checksum); + + /* Initialize RAM */ + raminit(&rp.pei_data); + + timestamp_add_now(TS_AFTER_INITRAM); + + romstage_handoff_init(rp.power_state->prev_sleep_state == ACPI_S3); + + mainboard_post_raminit(&rp);
platform_enter_postcar(); } @@ -117,33 +143,4 @@ romstage_main(base_timestamp, bist); }
-/* Entry from the mainboard. */ -void romstage_common(struct romstage_params *params) -{ - post_code(0x32); - - timestamp_add_now(TS_BEFORE_INITRAM); - - params->pei_data.boot_mode = params->power_state->prev_sleep_state; - -#if CONFIG(ELOG_BOOT_COUNT) - if (params->power_state->prev_sleep_state != ACPI_S3) - boot_count_increment(); -#endif - - /* Print ME state before MRC */ - intel_me_status(); - - /* Save ME HSIO version */ - intel_me_hsio_version(¶ms->power_state->hsio_version, - ¶ms->power_state->hsio_checksum); - - /* Initialize RAM */ - raminit(¶ms->pei_data); - - timestamp_add_now(TS_AFTER_INITRAM); - - romstage_handoff_init(params->power_state->prev_sleep_state == ACPI_S3); -} - void __weak mainboard_pre_console_init(void) {}
-
Arthur Heymans (Code Review) -
build bot (Jenkins) (Code Review)
-
Matt DeVillier (Code Review)
-
Nico Huber (Code Review)