Change in ...coreboot[master]: soc/intel/broadwell: Use the common cpu/intel/car romstage entry

[Arthur Heymans (Code Review)]

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32761

Change subject: soc/intel/broadwell: Use the common cpu/intel/car romstage entry
......................................................................

soc/intel/broadwell: Use the common cpu/intel/car romstage entry

The only functional difference is the use of stack guards.

Change-Id: I95645271e0d93a97f544a1cc4e9a4320738e6a20
Signed-off-by: Arthur Heymans 
---
M src/soc/intel/broadwell/romstage/Makefile.inc
M src/soc/intel/broadwell/romstage/romstage.c
2 files changed, 4 insertions(+), 18 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/32761/1

diff --git a/src/soc/intel/broadwell/romstage/Makefile.inc b/src/soc/intel/broadwell/romstage/Makefile.inc
index 2d562d9..cc0a051 100644
--- a/src/soc/intel/broadwell/romstage/Makefile.inc
+++ b/src/soc/intel/broadwell/romstage/Makefile.inc
@@ -1,5 +1,6 @@
 cpu_incs-y += $(src)/cpu/intel/car/non-evict/cache_as_ram.S

+romstage-y += ../../../../cpu/intel/car/romstage.c
 romstage-y += cpu.c
 romstage-y += pch.c
 romstage-y += power_state.c
diff --git a/src/soc/intel/broadwell/romstage/romstage.c b/src/soc/intel/broadwell/romstage/romstage.c
index 8ad8513..104e826 100644
--- a/src/soc/intel/broadwell/romstage/romstage.c
+++ b/src/soc/intel/broadwell/romstage/romstage.c
@@ -21,6 +21,7 @@
 #include <bootmode.h>
 #include <cbmem.h>
 #include <console/console.h>
+#include <cpu/intel/romstage.h>
 #include <cpu/x86/mtrr.h>
 #include <elog.h>
 #include <program_loading.h>
@@ -39,7 +40,7 @@
 /* platform_enter_postcar() determines the stack to use after
  * cache-as-ram is torn down as well as the MTRR settings to use,
  * and continues execution in postcar stage. */
-static void platform_enter_postcar(void)
+void platform_enter_postcar(void)
 {
    struct postcar_frame pcf;
    uintptr_t top_of_ram;
@@ -64,7 +65,7 @@
 }

/* Entry from cache-as-ram.inc. */
-static void romstage_main(uint64_t tsc, uint32_t bist)
+void mainboard_romstage_entry(unsigned long bist)
 {
    struct romstage_params rp = {
    	.bist = bist,
@@ -73,12 +74,6 @@

post_code(0x30);

-	/* Save initial timestamp from bootblock. */
-	timestamp_init(tsc);
-
-	/* Save romstage begin */
-	timestamp_add_now(TS_START_ROMSTAGE);
-
    /* System Agent Early Initialization */
    systemagent_early_init();

@@ -110,16 +105,6 @@
    romstage_common(&rp);

mainboard_post_raminit(&rp);
-
-	platform_enter_postcar();
-}
-
-/* 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, bist);
 }

/* Entry from the mainboard. */

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32761 )

Change subject: soc/intel/broadwell: Use the common cpu/intel/car romstage entry
......................................................................

Patch Set 3: Code-Review+1

(1 comment)

https://review.coreboot.org/#/c/32761/3/src/soc/intel/broadwell/romstage/rom... 
File src/soc/intel/broadwell/romstage/romstage.c:

https://review.coreboot.org/#/c/32761/3/src/soc/intel/broadwell/romstage/rom... 
PS3, Line 67: /* Entry from cache-as-ram.inc. */
update, please

[Arthur Heymans (Code Review)]

Hello Patrick Rudolph, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/32761

to look at the new patch set (#5).

Change subject: soc/intel/broadwell: Use the common cpu/intel/car romstage entry
......................................................................

soc/intel/broadwell: Use the common cpu/intel/car romstage entry

The only functional difference is the use of stack guards.

Change-Id: I95645271e0d93a97f544a1cc4e9a4320738e6a20
Signed-off-by: Arthur Heymans 
---
M src/soc/intel/broadwell/romstage/Makefile.inc
M src/soc/intel/broadwell/romstage/romstage.c
2 files changed, 5 insertions(+), 19 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/32761/5

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32761 )

Change subject: soc/intel/broadwell: Use the common cpu/intel/car romstage entry
......................................................................

Patch Set 6:

(1 comment)

https://review.coreboot.org/#/c/32761/6/src/soc/intel/broadwell/romstage/rom... 
File src/soc/intel/broadwell/romstage/romstage.c:

https://review.coreboot.org/#/c/32761/6/src/soc/intel/broadwell/romstage/rom... 
PS6, Line 67: cu
*cpu*

[Arthur Heymans (Code Review)]

Hello Patrick Rudolph, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/32761

to look at the new patch set (#8).

Change subject: soc/intel/broadwell: Use the common cpu/intel/car romstage entry
......................................................................

soc/intel/broadwell: Use the common cpu/intel/car romstage entry

The only functional difference is the use of stack guards.

Change-Id: I95645271e0d93a97f544a1cc4e9a4320738e6a20
Signed-off-by: Arthur Heymans 
---
M src/soc/intel/broadwell/romstage/Makefile.inc
M src/soc/intel/broadwell/romstage/romstage.c
2 files changed, 5 insertions(+), 19 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/32761/8

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32761 )

Change subject: soc/intel/broadwell: Use the common cpu/intel/car romstage entry
......................................................................

Patch Set 10: Code-Review+2

[Matt DeVillier (Code Review)]

Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32761 )

Change subject: soc/intel/broadwell: Use the common cpu/intel/car romstage entry
......................................................................

Patch Set 10:

(1 comment)

https://review.coreboot.org/#/c/32761/10/src/soc/intel/broadwell/romstage/Ma... 
File src/soc/intel/broadwell/romstage/Makefile.inc:

https://review.coreboot.org/#/c/32761/10/src/soc/intel/broadwell/romstage/Ma... 
PS10, Line 3: ../../../..
any reason to not use $(src) here as above?

[Matt DeVillier (Code Review)]

Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32761 )

Change subject: soc/intel/broadwell: Use the common cpu/intel/car romstage entry
......................................................................

Patch Set 10: Code-Review+1

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32761 )

Change subject: soc/intel/broadwell: Use the common cpu/intel/car romstage entry
......................................................................

Patch Set 10: Code-Review+1

(1 comment)

https://review.coreboot.org/#/c/32761/10/src/soc/intel/broadwell/romstage/Ma... 
File src/soc/intel/broadwell/romstage/Makefile.inc:

https://review.coreboot.org/#/c/32761/10/src/soc/intel/broadwell/romstage/Ma... 
PS10, Line 3: ../../../..
> any reason to not use $(src) here as above?

I guess, and it seems a good opportunity to retrigger Jenkins :)

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32761 )

Change subject: soc/intel/broadwell: Use the common cpu/intel/car romstage entry
......................................................................

Patch Set 10:

(1 comment)

https://review.coreboot.org/#/c/32761/10/src/soc/intel/broadwell/romstage/Ma... 
File src/soc/intel/broadwell/romstage/Makefile.inc:

https://review.coreboot.org/#/c/32761/10/src/soc/intel/broadwell/romstage/Ma... 
PS10, Line 3: ../../../..
> I guess, and it seems a good opportunity to retrigger Jenkins :)

For some reason that get's interpreted as src/soc/intel/broadwell/romstage/src/cpu/intel/car/romstage.c

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32761 )

Change subject: soc/intel/broadwell: Use the common cpu/intel/car romstage entry
......................................................................

Patch Set 10: Code-Review+2

(1 comment)

Does somebody want to hit "Rebase"? If I do, my +2
doesn't count anymore.

https://review.coreboot.org/#/c/32761/10/src/soc/intel/broadwell/romstage/Ma... 
File src/soc/intel/broadwell/romstage/Makefile.inc:

https://review.coreboot.org/#/c/32761/10/src/soc/intel/broadwell/romstage/Ma... 
PS10, Line 3: ../../../..
> For some reason that get's interpreted as src/soc/intel/broadwell/romstage/src/cpu/intel/car/romstag […]

Oh, $(src) is a relative path, missed that (`cpu_incs` doesn't have our
compilation class semantics, so it works there).

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32761 )

Change subject: soc/intel/broadwell: Use the common cpu/intel/car romstage entry
......................................................................

soc/intel/broadwell: Use the common cpu/intel/car romstage entry

The only functional difference is the use of stack guards.

Change-Id: I95645271e0d93a97f544a1cc4e9a4320738e6a20
Signed-off-by: Arthur Heymans 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32761
Tested-by: build bot (Jenkins) 
Reviewed-by: Matt DeVillier 
Reviewed-by: Nico Huber 
---
M src/soc/intel/broadwell/romstage/Makefile.inc
M src/soc/intel/broadwell/romstage/romstage.c
2 files changed, 5 insertions(+), 19 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/soc/intel/broadwell/romstage/Makefile.inc b/src/soc/intel/broadwell/romstage/Makefile.inc
index 2d562d9..cc0a051 100644
--- a/src/soc/intel/broadwell/romstage/Makefile.inc
+++ b/src/soc/intel/broadwell/romstage/Makefile.inc
@@ -1,5 +1,6 @@
 cpu_incs-y += $(src)/cpu/intel/car/non-evict/cache_as_ram.S

+romstage-y += ../../../../cpu/intel/car/romstage.c
 romstage-y += cpu.c
 romstage-y += pch.c
 romstage-y += power_state.c
diff --git a/src/soc/intel/broadwell/romstage/romstage.c b/src/soc/intel/broadwell/romstage/romstage.c
index acbca14..25c47c6 100644
--- a/src/soc/intel/broadwell/romstage/romstage.c
+++ b/src/soc/intel/broadwell/romstage/romstage.c
@@ -21,6 +21,7 @@
 #include <bootmode.h>
 #include <cbmem.h>
 #include <console/console.h>
+#include <cpu/intel/romstage.h>
 #include <cpu/x86/mtrr.h>
 #include <elog.h>
 #include <program_loading.h>
@@ -39,7 +40,7 @@
 /* platform_enter_postcar() determines the stack to use after
  * cache-as-ram is torn down as well as the MTRR settings to use,
  * and continues execution in postcar stage. */
-static void platform_enter_postcar(void)
+void platform_enter_postcar(void)
 {
    struct postcar_frame pcf;
    uintptr_t top_of_ram;
@@ -63,8 +64,8 @@
    run_postcar_phase(&pcf);
 }

-/* Entry from cache-as-ram.inc. */
-static void romstage_main(uint64_t tsc, uint32_t bist)
+/* Entry from cpu/intel/car/romstage.c. */
+void mainboard_romstage_entry(unsigned long bist)
 {
    struct romstage_params rp = {
    	.bist = bist,
@@ -72,12 +73,6 @@

post_code(0x30);

-	/* Save initial timestamp from bootblock. */
-	timestamp_init(tsc);
-
-	/* Save romstage begin */
-	timestamp_add_now(TS_START_ROMSTAGE);
-
    /* System Agent Early Initialization */
    systemagent_early_init();

@@ -131,16 +126,6 @@
    romstage_handoff_init(rp.power_state->prev_sleep_state == ACPI_S3);

mainboard_post_raminit(&rp);
-
-	platform_enter_postcar();
-}
-
-/* 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, bist);
 }

void __weak mainboard_pre_console_init(void) {}