Change in coreboot[master]: mainboard/google: Split Hatch mainboard_memory_init_params() on spd path

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36495 )

Change subject: mainboard/google: Split Hatch mainboard_memory_init_params() on spd path
......................................................................

mainboard/google: Split Hatch mainboard_memory_init_params() on spd path

The follow bifurcates mainboard_memory_init_params() in Hatch's romstage
depending on if the SPD data path is either derived from CBFS or over
SMBus. This restructure allows for variant boards that need to configure
the SMBus path to do it via their own custom variant_memory_params()
that implements the SMBus path.

BRANCH=none
BUG=b:143134702
TEST=./util/abuild/abuild -p none -t google/hatch -x -a

Change-Id: I430853270bdc2a3d8916d23fb6f085a28890518a
Signed-off-by: Edward O'Callaghan 
---
M src/mainboard/google/hatch/Kconfig
M src/mainboard/google/hatch/romstage.c
2 files changed, 28 insertions(+), 17 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/36495/1

diff --git a/src/mainboard/google/hatch/Kconfig b/src/mainboard/google/hatch/Kconfig
index 004cc28..fbc41df 100644
--- a/src/mainboard/google/hatch/Kconfig
+++ b/src/mainboard/google/hatch/Kconfig
@@ -58,6 +58,10 @@
    int
    default 512

+config ROMSTAGE_SPD_SMBUS
+	bool
+	default n
+
 config DRIVER_TPM_SPI_BUS
    default 0x1

diff --git a/src/mainboard/google/hatch/romstage.c b/src/mainboard/google/hatch/romstage.c
index 07864e6..b264d66d 100644
--- a/src/mainboard/google/hatch/romstage.c
+++ b/src/mainboard/google/hatch/romstage.c
@@ -32,26 +32,33 @@
 void mainboard_memory_init_params(FSPM_UPD *memupd)
 {
    struct cnl_mb_cfg memcfg;
-	int is_single_ch_mem;

variant_memory_params(&memcfg);
-	/*
-	 * GPP_F2 is the MEM_CH_SEL gpio, which is set to 1 for single
-	 * channel skus and 0 for dual channel skus.
-	 */
-	is_single_ch_mem = gpio_get(GPIO_MEM_CH_SEL);

-	/*
-	 * spd[0]-spd[3] map to CH0D0, CH0D1, CH1D0, CH1D1 respectively.
-	 * Dual-DIMM memory is not used in hatch family, so we only
-	 * fill in spd_info for CH0D0 and CH1D0 here.
-	 */
-	int mem_sku = variant_memory_sku();
-	memcfg.spd[0].read_type = READ_SPD_CBFS;
-	memcfg.spd[0].spd_spec.spd_index = mem_sku;
-	if (!is_single_ch_mem) {
-		memcfg.spd[2].read_type = READ_SPD_CBFS;
-		memcfg.spd[2].spd_spec.spd_index = mem_sku;
+	if (CONFIG(ROMSTAGE_SPD_SMBUS)) {
+		const FSPM_ARCH_UPD *arch_upd = &memupd->FspmArchUpd;
+		/* SPD was saved in S0/S5 path, skips it when resumes from S3 */
+		if (arch_upd->BootMode == FSP_BOOT_ON_S3_RESUME)
+			return;
+	} else {
+		/*
+		 * GPP_F2 is the MEM_CH_SEL gpio, which is set to 1 for single
+		 * channel skus and 0 for dual channel skus.
+		 */
+		int is_single_ch_mem = gpio_get(GPIO_MEM_CH_SEL);
+
+		/*
+		 * spd[0]-spd[3] map to CH0D0, CH0D1, CH1D0, CH1D1 respectively.
+		 * Dual-DIMM memory is not used in hatch family, so we only
+		 * fill in spd_info for CH0D0 and CH1D0 here.
+		 */
+		int mem_sku = variant_memory_sku();
+		memcfg.spd[0].read_type = READ_SPD_CBFS;
+		memcfg.spd[0].spd_spec.spd_index = mem_sku;
+		if (!is_single_ch_mem) {
+			memcfg.spd[2].read_type = READ_SPD_CBFS;
+			memcfg.spd[2].spd_spec.spd_index = mem_sku;
+		}
    }

cannonlake_memcfg_init(&memupd->FspmConfig, &memcfg);

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36495 )

Change subject: mainboard/google: Split Hatch mainboard_memory_init_params() on spd path
......................................................................

Patch Set 1:

Edward, I see you are building on the other patch I just commented on. I'll take a look tomorrow unless Furquan beats me. My guess w/o getting into this and the previous patch's details that this is likely good. Thanks for putting in the time and digging into stuff.

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/36495 )

Change subject: mainboard/google: Split Hatch mainboard_memory_init_params() on spd path
......................................................................

Abandoned

resolved now.