Change in ...coreboot[master]: drivers/intel/fsp1.1: Simplify bootflow and clean up

[Arthur Heymans (Code Review)]

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

Change subject: drivers/intel/fsp1.1: Simplify bootflow and clean up
......................................................................

drivers/intel/fsp1.1: Simplify bootflow and clean up

This gets rid of the boilerplate back and forward calls between the
SOC/FSP-driver code and mainboard code.

Change-Id: I5d4a10d1da6b3ac5e65efd7f82607b56b80e08d4
Signed-off-by: Arthur Heymans 
---
M src/drivers/intel/fsp1_1/include/fsp/romstage.h
M src/drivers/intel/fsp1_1/romstage.c
M src/mainboard/google/cyan/romstage.c
M src/mainboard/google/glados/romstage.c
M src/mainboard/intel/kunimitsu/romstage.c
M src/mainboard/intel/saddlebrook/romstage.c
M src/mainboard/intel/strago/romstage.c
7 files changed, 45 insertions(+), 75 deletions(-)

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

diff --git a/src/drivers/intel/fsp1_1/include/fsp/romstage.h b/src/drivers/intel/fsp1_1/include/fsp/romstage.h
index 7efbb10..2caceeb 100644
--- a/src/drivers/intel/fsp1_1/include/fsp/romstage.h
+++ b/src/drivers/intel/fsp1_1/include/fsp/romstage.h
@@ -44,15 +44,13 @@

void mainboard_memory_init_params(struct romstage_params *params,
    MEMORY_INIT_UPD *memory_params);
-void mainboard_romstage_entry(struct romstage_params *params);
+void mainboard_pre_raminit(struct romstage_params *params);
 void mainboard_save_dimm_info(struct romstage_params *params);
 void mainboard_add_dimm_info(struct romstage_params *params,
    		     struct memory_info *mem_info,
    		     int channel, int dimm, int index);
 void raminit(struct romstage_params *params);
 void report_memory_config(void);
-void romstage_common(struct romstage_params *params);
-asmlinkage void romstage_main(FSP_INFO_HEADER *fih);
 /* Initialize memory margin analysis settings. */
 void setup_mma(MEMORY_INIT_UPD *memory_upd);
 void soc_after_ram_init(struct romstage_params *params);
diff --git a/src/drivers/intel/fsp1_1/romstage.c b/src/drivers/intel/fsp1_1/romstage.c
index 85badce..b15fa92 100644
--- a/src/drivers/intel/fsp1_1/romstage.c
+++ b/src/drivers/intel/fsp1_1/romstage.c
@@ -38,48 +38,7 @@
 #include <timestamp.h>
 #include <vendorcode/google/chromeos/chromeos.h>

-asmlinkage void romstage_main(FSP_INFO_HEADER *fih)
-{
-	struct romstage_params params = {
-		.chipset_context = fih,
-	};
-
-	post_code(0x30);
-
-	timestamp_add_now(TS_START_ROMSTAGE);
-
-	/* Load microcode before RAM init */
-	if (CONFIG(SUPPORT_CPU_UCODE_IN_CBFS))
-		intel_update_microcode_from_cbfs();
-
-	/* Display parameters */
-	if (!CONFIG(NO_MMCONF_SUPPORT))
-		printk(BIOS_SPEW, "CONFIG_MMCONF_BASE_ADDRESS: 0x%08x\n",
-			CONFIG_MMCONF_BASE_ADDRESS);
-	printk(BIOS_INFO, "Using FSP 1.1\n");
-
-	/* Display FSP banner */
-	print_fsp_info(fih);
-
-	/* Stash FSP version. */
-	params.fsp_version = fsp_version(fih);
-
-	/* Get power state */
-	params.power_state = fill_power_state();
-
-	/* Call into mainboard. */
-	mainboard_romstage_entry(&params);
-	soc_after_ram_init(&params);
-	post_code(0x38);
-}
-
-void cache_as_ram_stage_main(FSP_INFO_HEADER *fih)
-{
-	romstage_main(fih);
-}
-
-/* Entry from the mainboard. */
-void romstage_common(struct romstage_params *params)
+static void raminit_common(struct romstage_params *params)
 {
    bool s3wake;
    struct region_device rdev;
@@ -154,6 +113,47 @@
    	full_reset();
 }

+void cache_as_ram_stage_main(FSP_INFO_HEADER *fih)
+{
+	struct romstage_params params = {
+		.chipset_context = fih,
+	};
+
+	post_code(0x30);
+
+	timestamp_add_now(TS_START_ROMSTAGE);
+
+	/* Load microcode before RAM init */
+	if (CONFIG(SUPPORT_CPU_UCODE_IN_CBFS))
+		intel_update_microcode_from_cbfs();
+
+	/* Display parameters */
+	if (!CONFIG(NO_MMCONF_SUPPORT))
+		printk(BIOS_SPEW, "CONFIG_MMCONF_BASE_ADDRESS: 0x%08x\n",
+			CONFIG_MMCONF_BASE_ADDRESS);
+	printk(BIOS_INFO, "Using FSP 1.1\n");
+
+	/* Display FSP banner */
+	print_fsp_info(fih);
+
+	/* Stash FSP version. */
+	params.fsp_version = fsp_version(fih);
+
+	/* Get power state */
+	params.power_state = fill_power_state();
+
+	/* Board initialization before and after RAM is enabled */
+	mainboard_pre_raminit(&params);
+
+	post_code(0x31);
+
+	/* Initialize memory */
+	raminit_common(&params);
+
+	soc_after_ram_init(&params);
+	post_code(0x38);
+}
+
 /* Initialize the power state */
 __weak struct chipset_power_state *fill_power_state(void)
 {
@@ -161,13 +161,8 @@
 }

/* Board initialization before and after RAM is enabled */
-__weak void mainboard_romstage_entry(
-	struct romstage_params *params)
+__weak void mainboard_pre_raminit(struct romstage_params *params)
 {
-	post_code(0x31);
-
-	/* Initialize memory */
-	romstage_common(params);
 }

/* Save the DIMM information for SMBIOS table 17 */
diff --git a/src/mainboard/google/cyan/romstage.c b/src/mainboard/google/cyan/romstage.c
index c877e42..dea73e9 100644
--- a/src/mainboard/google/cyan/romstage.c
+++ b/src/mainboard/google/cyan/romstage.c
@@ -19,13 +19,6 @@

#include "spd/spd_util.h"

-/* All FSP specific code goes in this block */
-void mainboard_romstage_entry(struct romstage_params *rp)
-{
-	/* Call back into chipset code with platform values updated. */
-	romstage_common(rp);
-}
-
 void mainboard_memory_init_params(struct romstage_params *params,
    MEMORY_INIT_UPD *memory_params)
 {
diff --git a/src/mainboard/google/glados/romstage.c b/src/mainboard/google/glados/romstage.c
index 47524c2..f2daa38 100644
--- a/src/mainboard/google/glados/romstage.c
+++ b/src/mainboard/google/glados/romstage.c
@@ -25,15 +25,13 @@
 #include "spd/spd_util.h"
 #include "spd/spd.h"

-void mainboard_romstage_entry(struct romstage_params *params)
+void mainboard_pre_raminit(struct romstage_params *params)
 {
 #ifdef EC_ENABLE_KEYBOARD_BACKLIGHT
    /* Turn on keyboard backlight to indicate we are booting */
    if (params->power_state->prev_sleep_state != ACPI_S3)
    	google_chromeec_kbbacklight(25);
 #endif
-	/* Initialize memory */
-	romstage_common(params);
 }

void mainboard_memory_init_params(struct romstage_params *params,
diff --git a/src/mainboard/intel/kunimitsu/romstage.c b/src/mainboard/intel/kunimitsu/romstage.c
index f25f88b..f900ca3 100644
--- a/src/mainboard/intel/kunimitsu/romstage.c
+++ b/src/mainboard/intel/kunimitsu/romstage.c
@@ -20,12 +20,6 @@
 #include "gpio.h"
 #include "spd/spd.h"

-void mainboard_romstage_entry(struct romstage_params *params)
-{
-	/* Initialize memory */
-	romstage_common(params);
-}
-
 void mainboard_memory_init_params(struct romstage_params *params,
    			  MEMORY_INIT_UPD *memory_params)
 {
diff --git a/src/mainboard/intel/saddlebrook/romstage.c b/src/mainboard/intel/saddlebrook/romstage.c
index 48d39db..82b85fd 100644
--- a/src/mainboard/intel/saddlebrook/romstage.c
+++ b/src/mainboard/intel/saddlebrook/romstage.c
@@ -33,12 +33,6 @@
    nuvoton_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE);
 }

-void mainboard_romstage_entry(struct romstage_params *params)
-{
-	post_code(0x31);
-	romstage_common(params);
-}
-
 void mainboard_memory_init_params(
    struct romstage_params *params,
    MEMORY_INIT_UPD *memory_params)
diff --git a/src/mainboard/intel/strago/romstage.c b/src/mainboard/intel/strago/romstage.c
index ba0ff7b..1dc7ba3 100644
--- a/src/mainboard/intel/strago/romstage.c
+++ b/src/mainboard/intel/strago/romstage.c
@@ -14,8 +14,6 @@
  * GNU General Public License for more details.
  */

-#include <soc/gpio.h>
-#include <soc/pci_devs.h>
 #include <soc/romstage.h>
 #include "onboard.h"
 #include <boardid.h>

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32961 )

Change subject: drivers/intel/fsp1.1: Simplify bootflow and clean up
......................................................................

Patch Set 2: Code-Review+1

[Frans Hendriks (Code Review)]

Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32961 )

Change subject: drivers/intel/fsp1.1: Simplify bootflow and clean up
......................................................................

Patch Set 3: Code-Review+1

[Arthur Heymans (Code Review)]

Hello Kyösti Mälkki, Patrick Rudolph, Huang Jin, Frans Hendriks, Lee Leahy, build bot (Jenkins),

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

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

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

Change subject: drivers/intel/fsp1.1: Simplify bootflow and clean up
......................................................................

drivers/intel/fsp1.1: Simplify bootflow and clean up

This gets rid of the boilerplate back and forward calls between the
SOC/FSP-driver code and mainboard code.

Change-Id: I5d4a10d1da6b3ac5e65efd7f82607b56b80e08d4
Signed-off-by: Arthur Heymans 
---
M src/drivers/intel/fsp1_1/include/fsp/romstage.h
M src/drivers/intel/fsp1_1/romstage.c
M src/mainboard/google/cyan/romstage.c
M src/mainboard/google/glados/romstage.c
M src/mainboard/intel/kunimitsu/romstage.c
M src/mainboard/intel/saddlebrook/romstage.c
M src/mainboard/intel/strago/romstage.c
7 files changed, 45 insertions(+), 75 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/32961/4

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32961 )

Change subject: drivers/intel/fsp1.1: Simplify bootflow and clean up
......................................................................

Patch Set 4: Code-Review+2

[Patrick Georgi (Code Review)]

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

Change subject: drivers/intel/fsp1.1: Simplify bootflow and clean up
......................................................................

drivers/intel/fsp1.1: Simplify bootflow and clean up

This gets rid of the boilerplate back and forward calls between the
SOC/FSP-driver code and mainboard code.

Change-Id: I5d4a10d1da6b3ac5e65efd7f82607b56b80e08d4
Signed-off-by: Arthur Heymans 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32961
Reviewed-by: Patrick Georgi 
Reviewed-by: Frans Hendriks 
Reviewed-by: Kyösti Mälkki 
Tested-by: build bot (Jenkins) 
---
M src/drivers/intel/fsp1_1/include/fsp/romstage.h
M src/drivers/intel/fsp1_1/romstage.c
M src/mainboard/google/cyan/romstage.c
M src/mainboard/google/glados/romstage.c
M src/mainboard/intel/kunimitsu/romstage.c
M src/mainboard/intel/saddlebrook/romstage.c
M src/mainboard/intel/strago/romstage.c
7 files changed, 45 insertions(+), 75 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Georgi: Looks good to me, approved
  Kyösti Mälkki: Looks good to me, but someone else must approve
  Frans Hendriks: Looks good to me, but someone else must approve

diff --git a/src/drivers/intel/fsp1_1/include/fsp/romstage.h b/src/drivers/intel/fsp1_1/include/fsp/romstage.h
index 7efbb10..2caceeb 100644
--- a/src/drivers/intel/fsp1_1/include/fsp/romstage.h
+++ b/src/drivers/intel/fsp1_1/include/fsp/romstage.h
@@ -44,15 +44,13 @@

void mainboard_memory_init_params(struct romstage_params *params,
    MEMORY_INIT_UPD *memory_params);
-void mainboard_romstage_entry(struct romstage_params *params);
+void mainboard_pre_raminit(struct romstage_params *params);
 void mainboard_save_dimm_info(struct romstage_params *params);
 void mainboard_add_dimm_info(struct romstage_params *params,
    		     struct memory_info *mem_info,
    		     int channel, int dimm, int index);
 void raminit(struct romstage_params *params);
 void report_memory_config(void);
-void romstage_common(struct romstage_params *params);
-asmlinkage void romstage_main(FSP_INFO_HEADER *fih);
 /* Initialize memory margin analysis settings. */
 void setup_mma(MEMORY_INIT_UPD *memory_upd);
 void soc_after_ram_init(struct romstage_params *params);
diff --git a/src/drivers/intel/fsp1_1/romstage.c b/src/drivers/intel/fsp1_1/romstage.c
index 45ca87a..d86d3ab 100644
--- a/src/drivers/intel/fsp1_1/romstage.c
+++ b/src/drivers/intel/fsp1_1/romstage.c
@@ -37,48 +37,7 @@
 #include <timestamp.h>
 #include <vendorcode/google/chromeos/chromeos.h>

-asmlinkage void romstage_main(FSP_INFO_HEADER *fih)
-{
-	struct romstage_params params = {
-		.chipset_context = fih,
-	};
-
-	post_code(0x30);
-
-	timestamp_add_now(TS_START_ROMSTAGE);
-
-	/* Load microcode before RAM init */
-	if (CONFIG(SUPPORT_CPU_UCODE_IN_CBFS))
-		intel_update_microcode_from_cbfs();
-
-	/* Display parameters */
-	if (!CONFIG(NO_MMCONF_SUPPORT))
-		printk(BIOS_SPEW, "CONFIG_MMCONF_BASE_ADDRESS: 0x%08x\n",
-			CONFIG_MMCONF_BASE_ADDRESS);
-	printk(BIOS_INFO, "Using FSP 1.1\n");
-
-	/* Display FSP banner */
-	print_fsp_info(fih);
-
-	/* Stash FSP version. */
-	params.fsp_version = fsp_version(fih);
-
-	/* Get power state */
-	params.power_state = fill_power_state();
-
-	/* Call into mainboard. */
-	mainboard_romstage_entry(&params);
-	soc_after_ram_init(&params);
-	post_code(0x38);
-}
-
-void cache_as_ram_stage_main(FSP_INFO_HEADER *fih)
-{
-	romstage_main(fih);
-}
-
-/* Entry from the mainboard. */
-void romstage_common(struct romstage_params *params)
+static void raminit_common(struct romstage_params *params)
 {
    bool s3wake;
    struct region_device rdev;
@@ -153,6 +112,47 @@
    	full_reset();
 }

+void cache_as_ram_stage_main(FSP_INFO_HEADER *fih)
+{
+	struct romstage_params params = {
+		.chipset_context = fih,
+	};
+
+	post_code(0x30);
+
+	timestamp_add_now(TS_START_ROMSTAGE);
+
+	/* Load microcode before RAM init */
+	if (CONFIG(SUPPORT_CPU_UCODE_IN_CBFS))
+		intel_update_microcode_from_cbfs();
+
+	/* Display parameters */
+	if (!CONFIG(NO_MMCONF_SUPPORT))
+		printk(BIOS_SPEW, "CONFIG_MMCONF_BASE_ADDRESS: 0x%08x\n",
+			CONFIG_MMCONF_BASE_ADDRESS);
+	printk(BIOS_INFO, "Using FSP 1.1\n");
+
+	/* Display FSP banner */
+	print_fsp_info(fih);
+
+	/* Stash FSP version. */
+	params.fsp_version = fsp_version(fih);
+
+	/* Get power state */
+	params.power_state = fill_power_state();
+
+	/* Board initialization before and after RAM is enabled */
+	mainboard_pre_raminit(&params);
+
+	post_code(0x31);
+
+	/* Initialize memory */
+	raminit_common(&params);
+
+	soc_after_ram_init(&params);
+	post_code(0x38);
+}
+
 /* Initialize the power state */
 __weak struct chipset_power_state *fill_power_state(void)
 {
@@ -160,13 +160,8 @@
 }

/* Board initialization before and after RAM is enabled */
-__weak void mainboard_romstage_entry(
-	struct romstage_params *params)
+__weak void mainboard_pre_raminit(struct romstage_params *params)
 {
-	post_code(0x31);
-
-	/* Initialize memory */
-	romstage_common(params);
 }

/* Save the DIMM information for SMBIOS table 17 */
diff --git a/src/mainboard/google/cyan/romstage.c b/src/mainboard/google/cyan/romstage.c
index c877e42..dea73e9 100644
--- a/src/mainboard/google/cyan/romstage.c
+++ b/src/mainboard/google/cyan/romstage.c
@@ -19,13 +19,6 @@

#include "spd/spd_util.h"

-/* All FSP specific code goes in this block */
-void mainboard_romstage_entry(struct romstage_params *rp)
-{
-	/* Call back into chipset code with platform values updated. */
-	romstage_common(rp);
-}
-
 void mainboard_memory_init_params(struct romstage_params *params,
    MEMORY_INIT_UPD *memory_params)
 {
diff --git a/src/mainboard/google/glados/romstage.c b/src/mainboard/google/glados/romstage.c
index 47524c2..f2daa38 100644
--- a/src/mainboard/google/glados/romstage.c
+++ b/src/mainboard/google/glados/romstage.c
@@ -25,15 +25,13 @@
 #include "spd/spd_util.h"
 #include "spd/spd.h"

-void mainboard_romstage_entry(struct romstage_params *params)
+void mainboard_pre_raminit(struct romstage_params *params)
 {
 #ifdef EC_ENABLE_KEYBOARD_BACKLIGHT
    /* Turn on keyboard backlight to indicate we are booting */
    if (params->power_state->prev_sleep_state != ACPI_S3)
    	google_chromeec_kbbacklight(25);
 #endif
-	/* Initialize memory */
-	romstage_common(params);
 }

void mainboard_memory_init_params(struct romstage_params *params,
diff --git a/src/mainboard/intel/kunimitsu/romstage.c b/src/mainboard/intel/kunimitsu/romstage.c
index f25f88b..f900ca3 100644
--- a/src/mainboard/intel/kunimitsu/romstage.c
+++ b/src/mainboard/intel/kunimitsu/romstage.c
@@ -20,12 +20,6 @@
 #include "gpio.h"
 #include "spd/spd.h"

-void mainboard_romstage_entry(struct romstage_params *params)
-{
-	/* Initialize memory */
-	romstage_common(params);
-}
-
 void mainboard_memory_init_params(struct romstage_params *params,
    			  MEMORY_INIT_UPD *memory_params)
 {
diff --git a/src/mainboard/intel/saddlebrook/romstage.c b/src/mainboard/intel/saddlebrook/romstage.c
index 48d39db..82b85fd 100644
--- a/src/mainboard/intel/saddlebrook/romstage.c
+++ b/src/mainboard/intel/saddlebrook/romstage.c
@@ -33,12 +33,6 @@
    nuvoton_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE);
 }

-void mainboard_romstage_entry(struct romstage_params *params)
-{
-	post_code(0x31);
-	romstage_common(params);
-}
-
 void mainboard_memory_init_params(
    struct romstage_params *params,
    MEMORY_INIT_UPD *memory_params)
diff --git a/src/mainboard/intel/strago/romstage.c b/src/mainboard/intel/strago/romstage.c
index ba0ff7b..1dc7ba3 100644
--- a/src/mainboard/intel/strago/romstage.c
+++ b/src/mainboard/intel/strago/romstage.c
@@ -14,8 +14,6 @@
  * GNU General Public License for more details.
  */

-#include <soc/gpio.h>
-#include <soc/pci_devs.h>
 #include <soc/romstage.h>
 #include "onboard.h"
 #include <boardid.h>