Hello Marco Chen,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/40302
to review the following change.
Change subject: soc/intel/jasperlake: Allow mainboard to override DRAM part number ......................................................................
soc/intel/jasperlake: Allow mainboard to override DRAM part number
In order to support mainboards that do not store DRAM part number in the traditional way i.e. within the CBFS SPD for soldered memory, this change provides a runtime callback to allow mainboards to provide DRAM part number from a custom location e.g. external EEPROM on dedede.
For other boards it should be a NOP since the weak implementation of mainboard_get_dram_part_num does nothing.
BUG=b:152019429
Change-Id: I7ba635f5504ba288308d7d7a4935f405f289aa8d Signed-off-by: Marco Chen marcochen@google.com --- M src/soc/intel/jasperlake/include/soc/romstage.h M src/soc/intel/jasperlake/romstage/romstage.c 2 files changed, 23 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/40302/1
diff --git a/src/soc/intel/jasperlake/include/soc/romstage.h b/src/soc/intel/jasperlake/include/soc/romstage.h index 4a4fbe6..e3c7969 100644 --- a/src/soc/intel/jasperlake/include/soc/romstage.h +++ b/src/soc/intel/jasperlake/include/soc/romstage.h @@ -6,6 +6,8 @@
#include <fsp/api.h>
+/* Provide a callback to allow mainboard to override the DRAM part number. */ +bool mainboard_get_dram_part_num(const char **part_num, size_t *len); void mainboard_memory_init_params(FSPM_UPD *mupd); void systemagent_early_init(void); void pch_init(void); diff --git a/src/soc/intel/jasperlake/romstage/romstage.c b/src/soc/intel/jasperlake/romstage/romstage.c index fa9db6e..b8e9032 100644 --- a/src/soc/intel/jasperlake/romstage/romstage.c +++ b/src/soc/intel/jasperlake/romstage/romstage.c @@ -22,6 +22,12 @@ 0x8d, 0x09, 0x11, 0xcf, 0x8b, 0x9f, 0x03, 0x23 \ }
+bool __weak mainboard_get_dram_part_num(const char **part_num, size_t *len) +{ + /* Default weak implementation, no need to override part number. */ + return false; +} + /* Save the DIMM information for SMBIOS table 17 */ static void save_dimm_info(void) { @@ -36,6 +42,9 @@ const uint8_t smbios_memory_info_guid[16] = FSP_SMBIOS_MEMORY_INFO_GUID; const uint8_t *serial_num; + const char *dram_part_num = NULL; + size_t dram_part_num_len; + bool is_dram_part_overridden = false;
/* Locate the memory info HOB, presence validated by raminit */ meminfo_hob = fsp_find_extension_hob_by_guid( @@ -57,6 +66,10 @@ } memset(mem_info, 0, sizeof(*mem_info));
+ /* Allow mainboard to override DRAM part number. */ + is_dram_part_overridden = mainboard_get_dram_part_num(&dram_part_num, + &dram_part_num_len); + /* Save available DIMM information */ index = 0; dimm_max = ARRAY_SIZE(mem_info->dimm); @@ -75,6 +88,14 @@ if (src_dimm->Status != DIMM_PRESENT) continue;
+ /* If there is no DRAM part number overridden by + * mainboard then use original one. */ + if (!is_dram_part_overridden) { + dram_part_num_len = sizeof(src_dimm->ModulePartNum); + dram_part_num = (const char *) + &src_dimm->ModulePartNum[0]; + } + u8 memProfNum = meminfo_hob->MemoryProfile; serial_num = src_dimm->SpdSave + SPD_SAVE_OFFSET_SERIAL;
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40302 )
Change subject: soc/intel/jasperlake: Allow mainboard to override DRAM part number ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40302 )
Change subject: soc/intel/jasperlake: Allow mainboard to override DRAM part number ......................................................................
soc/intel/jasperlake: Allow mainboard to override DRAM part number
In order to support mainboards that do not store DRAM part number in the traditional way i.e. within the CBFS SPD for soldered memory, this change provides a runtime callback to allow mainboards to provide DRAM part number from a custom location e.g. external EEPROM on dedede.
For other boards it should be a NOP since the weak implementation of mainboard_get_dram_part_num does nothing.
BUG=b:152019429
Change-Id: I7ba635f5504ba288308d7d7a4935f405f289aa8d Signed-off-by: Marco Chen marcochen@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/40302 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/soc/intel/jasperlake/include/soc/romstage.h M src/soc/intel/jasperlake/romstage/romstage.c 2 files changed, 23 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/intel/jasperlake/include/soc/romstage.h b/src/soc/intel/jasperlake/include/soc/romstage.h index 4a4fbe6..e3c7969 100644 --- a/src/soc/intel/jasperlake/include/soc/romstage.h +++ b/src/soc/intel/jasperlake/include/soc/romstage.h @@ -6,6 +6,8 @@
#include <fsp/api.h>
+/* Provide a callback to allow mainboard to override the DRAM part number. */ +bool mainboard_get_dram_part_num(const char **part_num, size_t *len); void mainboard_memory_init_params(FSPM_UPD *mupd); void systemagent_early_init(void); void pch_init(void); diff --git a/src/soc/intel/jasperlake/romstage/romstage.c b/src/soc/intel/jasperlake/romstage/romstage.c index fa9db6e..b8e9032 100644 --- a/src/soc/intel/jasperlake/romstage/romstage.c +++ b/src/soc/intel/jasperlake/romstage/romstage.c @@ -22,6 +22,12 @@ 0x8d, 0x09, 0x11, 0xcf, 0x8b, 0x9f, 0x03, 0x23 \ }
+bool __weak mainboard_get_dram_part_num(const char **part_num, size_t *len) +{ + /* Default weak implementation, no need to override part number. */ + return false; +} + /* Save the DIMM information for SMBIOS table 17 */ static void save_dimm_info(void) { @@ -36,6 +42,9 @@ const uint8_t smbios_memory_info_guid[16] = FSP_SMBIOS_MEMORY_INFO_GUID; const uint8_t *serial_num; + const char *dram_part_num = NULL; + size_t dram_part_num_len; + bool is_dram_part_overridden = false;
/* Locate the memory info HOB, presence validated by raminit */ meminfo_hob = fsp_find_extension_hob_by_guid( @@ -57,6 +66,10 @@ } memset(mem_info, 0, sizeof(*mem_info));
+ /* Allow mainboard to override DRAM part number. */ + is_dram_part_overridden = mainboard_get_dram_part_num(&dram_part_num, + &dram_part_num_len); + /* Save available DIMM information */ index = 0; dimm_max = ARRAY_SIZE(mem_info->dimm); @@ -75,6 +88,14 @@ if (src_dimm->Status != DIMM_PRESENT) continue;
+ /* If there is no DRAM part number overridden by + * mainboard then use original one. */ + if (!is_dram_part_overridden) { + dram_part_num_len = sizeof(src_dimm->ModulePartNum); + dram_part_num = (const char *) + &src_dimm->ModulePartNum[0]; + } + u8 memProfNum = meminfo_hob->MemoryProfile; serial_num = src_dimm->SpdSave + SPD_SAVE_OFFSET_SERIAL;