Nick Vaccaro has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45873 )
Change subject: lib, mb, soc: change mainboard_get_dram_part_num() prototype ......................................................................
lib, mb, soc: change mainboard_get_dram_part_num() prototype
Change mainboard_get_dram_part_num() to return a constant character pointer to a null-terminated C string and to take no input parameters. This also addresses the issue that different SOCs and motherboards were using different definitions for mainboard_get_dram_part_num by consolidating to a single definition.
BUG=b:168724473 TEST="emerge-volteer coreboot && emerge-dedede coreboot && emerge-hatch coreboot" and verify build completes successfully.
Change-Id: Ie7664eab65a2b9e25b7853bf68baf2525b040487 Signed-off-by: Nick Vaccaro nvaccaro@google.com --- M src/mainboard/google/dedede/romstage.c M src/mainboard/google/hatch/romstage_spd_cbfs.c M src/mainboard/google/volteer/romstage.c M src/soc/intel/alderlake/include/soc/romstage.h M src/soc/intel/alderlake/romstage/romstage.c M src/soc/intel/cannonlake/include/soc/romstage.h M src/soc/intel/cannonlake/romstage/romstage.c M src/soc/intel/elkhartlake/include/soc/romstage.h M src/soc/intel/elkhartlake/romstage/romstage.c M src/soc/intel/jasperlake/include/soc/romstage.h M src/soc/intel/jasperlake/romstage/romstage.c M src/soc/intel/tigerlake/include/soc/romstage.h M src/soc/intel/tigerlake/romstage/romstage.c 13 files changed, 64 insertions(+), 51 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/45873/1
diff --git a/src/mainboard/google/dedede/romstage.c b/src/mainboard/google/dedede/romstage.c index 8028db0..e143f70 100644 --- a/src/mainboard/google/dedede/romstage.c +++ b/src/mainboard/google/dedede/romstage.c @@ -22,17 +22,15 @@ memcfg_init(&memupd->FspmConfig, board_cfg, &spd_info, half_populated); }
-bool mainboard_get_dram_part_num(const char **part_num, size_t *len) +const char *mainboard_get_dram_part_num(void) { static char part_num_store[DIMM_INFO_PART_NUMBER_SIZE];
if (google_chromeec_cbi_get_dram_part_num(&part_num_store[0], sizeof(part_num_store)) < 0) { printk(BIOS_ERR, "No DRAM part number in CBI!\n"); - return false; + return NULL; }
- *part_num = &part_num_store[0]; - *len = strlen(part_num_store); - return true; + return part_num_store; } diff --git a/src/mainboard/google/hatch/romstage_spd_cbfs.c b/src/mainboard/google/hatch/romstage_spd_cbfs.c index a9a65e9..b8937de 100644 --- a/src/mainboard/google/hatch/romstage_spd_cbfs.c +++ b/src/mainboard/google/hatch/romstage_spd_cbfs.c @@ -57,7 +57,7 @@ cannonlake_memcfg_init(&memupd->FspmConfig, &memcfg); }
-void mainboard_get_dram_part_num(const char **part_num, size_t *len) +const char *mainboard_get_dram_part_num(void) { static char part_num_store[DIMM_INFO_PART_NUMBER_SIZE]; static enum { @@ -77,8 +77,7 @@ }
if (part_num_state == PART_NUM_NOT_IN_CBI) - return; + return NULL;
- *part_num = &part_num_store[0]; - *len = strlen(part_num_store) + 1; + return part_num_store; } diff --git a/src/mainboard/google/volteer/romstage.c b/src/mainboard/google/volteer/romstage.c index 552648b..51a4ebc 100644 --- a/src/mainboard/google/volteer/romstage.c +++ b/src/mainboard/google/volteer/romstage.c @@ -29,16 +29,14 @@ meminit_ddr(mem_cfg, board_cfg, &spd_info, half_populated); }
-bool mainboard_get_dram_part_num(const char **part_num, size_t *len) +const char *mainboard_get_dram_part_num(void) { static char part_num_store[DIMM_INFO_PART_NUMBER_SIZE];
if (google_chromeec_cbi_get_dram_part_num(part_num_store, sizeof(part_num_store)) < 0) { printk(BIOS_ERR, "ERROR: Couldn't obtain DRAM part number from CBI\n"); - return false; + return NULL; } - *part_num = part_num_store; - *len = strlen(part_num_store); - return true; + return part_num_store; } diff --git a/src/soc/intel/alderlake/include/soc/romstage.h b/src/soc/intel/alderlake/include/soc/romstage.h index 716602c..55469a3 100644 --- a/src/soc/intel/alderlake/include/soc/romstage.h +++ b/src/soc/intel/alderlake/include/soc/romstage.h @@ -7,7 +7,7 @@ #include <stddef.h>
/* Provide a callback to allow mainboard to override the DRAM part number. */ -const char *mainboard_get_dram_part_num(size_t *len); +const char *mainboard_get_dram_part_num(void); void mainboard_memory_init_params(FSPM_UPD *mupd); void systemagent_early_init(void); void romstage_pch_init(void); diff --git a/src/soc/intel/alderlake/romstage/romstage.c b/src/soc/intel/alderlake/romstage/romstage.c index 9f4fbb6..a62a726 100644 --- a/src/soc/intel/alderlake/romstage/romstage.c +++ b/src/soc/intel/alderlake/romstage/romstage.c @@ -21,7 +21,7 @@ 0x8d, 0x09, 0x11, 0xcf, 0x8b, 0x9f, 0x03, 0x23 \ }
-__weak const char *mainboard_get_dram_part_num(size_t *len) +const char * __weak mainboard_get_dram_part_num(void) { /* Default weak implementation, no need to override part number. */ return NULL; @@ -41,7 +41,8 @@ const uint8_t smbios_memory_info_guid[sizeof(EFI_GUID)] = FSP_SMBIOS_MEMORY_INFO_GUID; const uint8_t *serial_num; const char *dram_part_num = NULL; - size_t dram_part_num_len; + size_t dram_part_num_len = 0; + bool part_name_overridden = false;
/* Locate the memory info HOB, presence validated by raminit */ meminfo_hob = fsp_find_extension_hob_by_guid( @@ -64,7 +65,11 @@ memset(mem_info, 0, sizeof(*mem_info));
/* Allow mainboard to override DRAM part number. */ - dram_part_num = mainboard_get_dram_part_num(&dram_part_num_len); + dram_part_num = mainboard_get_dram_part_num(); + if (dram_part_num) { + dram_part_num_len = strlen(dram_part_num); + part_name_overridden = true; + }
/* Save available DIMM information */ index = 0; @@ -84,7 +89,7 @@
/* If there is no DRAM part number overridden by * mainboard then use original one. */ - if (!dram_part_num) { + if (!part_name_overridden) { dram_part_num_len = sizeof(src_dimm->ModulePartNum); dram_part_num = (const char *) &src_dimm->ModulePartNum[0]; diff --git a/src/soc/intel/cannonlake/include/soc/romstage.h b/src/soc/intel/cannonlake/include/soc/romstage.h index 4e513f0..f99175f 100644 --- a/src/soc/intel/cannonlake/include/soc/romstage.h +++ b/src/soc/intel/cannonlake/include/soc/romstage.h @@ -8,7 +8,7 @@ void mainboard_memory_init_params(FSPM_UPD *mupd);
/* Provide a callback to allow mainboard to override the DRAM part number. */ -void mainboard_get_dram_part_num(const char **part_num, size_t *len); +const char *mainboard_get_dram_part_num(void); void systemagent_early_init(void); void romstage_pch_init(void);
diff --git a/src/soc/intel/cannonlake/romstage/romstage.c b/src/soc/intel/cannonlake/romstage/romstage.c index c56add7..74dc20f 100644 --- a/src/soc/intel/cannonlake/romstage/romstage.c +++ b/src/soc/intel/cannonlake/romstage/romstage.c @@ -23,7 +23,7 @@ 0x8d, 0x09, 0x11, 0xcf, 0x8b, 0x9f, 0x03, 0x23 \ }
-void __weak mainboard_get_dram_part_num(const char **part_num, size_t *len) +const char * __weak mainboard_get_dram_part_num(void) { /* Default weak implementation, no need to override part number. */ } @@ -42,7 +42,7 @@ const uint8_t smbios_memory_info_guid[16] = FSP_SMBIOS_MEMORY_INFO_GUID; const char *dram_part_num; - size_t dram_part_num_len; + size_t dram_part_num_len = 0;
/* Locate the memory info HOB, presence validated by raminit */ memory_info_hob = fsp_find_extension_hob_by_guid( @@ -79,13 +79,16 @@ if (src_dimm->Status != DIMM_PRESENT) continue;
- dram_part_num_len = sizeof(src_dimm->ModulePartNum); - dram_part_num = (const char *) - &src_dimm->ModulePartNum[0]; - /* Allow mainboard to override DRAM part number. */ - mainboard_get_dram_part_num(&dram_part_num, - &dram_part_num_len); + dram_part_num = mainboard_get_dram_part_num(); + if (dram_part_num) { + dram_part_num_len = strlen(dram_part_num); + } else { + dram_part_num_len = + sizeof(src_dimm->ModulePartNum); + dram_part_num = (const char *) + &src_dimm->ModulePartNum[0]; + }
u8 memProfNum = memory_info_hob->MemoryProfile;
diff --git a/src/soc/intel/elkhartlake/include/soc/romstage.h b/src/soc/intel/elkhartlake/include/soc/romstage.h index baa35c5..1cffcb9 100644 --- a/src/soc/intel/elkhartlake/include/soc/romstage.h +++ b/src/soc/intel/elkhartlake/include/soc/romstage.h @@ -6,7 +6,7 @@ #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); +const char *mainboard_get_dram_part_num(void); void mainboard_memory_init_params(FSPM_UPD *mupd); void systemagent_early_init(void); void romstage_pch_init(void); diff --git a/src/soc/intel/elkhartlake/romstage/romstage.c b/src/soc/intel/elkhartlake/romstage/romstage.c index 09b1744..d7b2959 100644 --- a/src/soc/intel/elkhartlake/romstage/romstage.c +++ b/src/soc/intel/elkhartlake/romstage/romstage.c @@ -21,10 +21,10 @@ 0x8d, 0x09, 0x11, 0xcf, 0x8b, 0x9f, 0x03, 0x23 \ }
-bool __weak mainboard_get_dram_part_num(const char **part_num, size_t *len) +const char * __weak mainboard_get_dram_part_num(void) { /* Default implementation, no need to override part number. */ - return false; + return NULL; }
/* Save the DIMM information for SMBIOS table 17 */ @@ -42,8 +42,8 @@ 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; + size_t dram_part_num_len = 0; + bool part_name_overridden = false;
/* Locate the memory info HOB, presence validated by raminit */ meminfo_hob = fsp_find_extension_hob_by_guid( @@ -66,8 +66,11 @@ 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); + dram_part_num = mainboard_get_dram_part_num(); + if (dram_part_num) { + dram_part_num = strlen(dram_part_num); + part_name_overridden = true; + }
/* Save available DIMM information */ index = 0; @@ -89,7 +92,7 @@
/* If there is no DRAM part number overridden by * mainboard then use original one. */ - if (!is_dram_part_overridden) { + if (!part_name_overridden) { dram_part_num_len = sizeof(src_dimm->ModulePartNum); dram_part_num = (const char *) &src_dimm->ModulePartNum[0]; diff --git a/src/soc/intel/jasperlake/include/soc/romstage.h b/src/soc/intel/jasperlake/include/soc/romstage.h index baa35c5..1cffcb9 100644 --- a/src/soc/intel/jasperlake/include/soc/romstage.h +++ b/src/soc/intel/jasperlake/include/soc/romstage.h @@ -6,7 +6,7 @@ #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); +const char *mainboard_get_dram_part_num(void); void mainboard_memory_init_params(FSPM_UPD *mupd); void systemagent_early_init(void); void romstage_pch_init(void); diff --git a/src/soc/intel/jasperlake/romstage/romstage.c b/src/soc/intel/jasperlake/romstage/romstage.c index db014ea..6a3ebd6 100644 --- a/src/soc/intel/jasperlake/romstage/romstage.c +++ b/src/soc/intel/jasperlake/romstage/romstage.c @@ -21,10 +21,10 @@ 0x8d, 0x09, 0x11, 0xcf, 0x8b, 0x9f, 0x03, 0x23 \ }
-bool __weak mainboard_get_dram_part_num(const char **part_num, size_t *len) +const char * __weak mainboard_get_dram_part_num(void) { /* Default weak implementation, no need to override part number. */ - return false; + return NULL; }
/* Save the DIMM information for SMBIOS table 17 */ @@ -42,8 +42,8 @@ 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; + size_t dram_part_num_len = 0; + bool part_name_overridden = false;
/* Locate the memory info HOB, presence validated by raminit */ meminfo_hob = fsp_find_extension_hob_by_guid( @@ -66,8 +66,12 @@ 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 = mainboard_get_dram_part_num(&, &dram_part_num_len); + if (dram_part_num) { + dram_part_num_len = strlen(dram_part_num); + part_name_overridden = true; + }
/* Save available DIMM information */ index = 0; @@ -89,7 +93,7 @@
/* If there is no DRAM part number overridden by * mainboard then use original one. */ - if (!is_dram_part_overridden) { + if (!part_name_overridden) { dram_part_num_len = sizeof(src_dimm->ModulePartNum); dram_part_num = (const char *) &src_dimm->ModulePartNum[0]; diff --git a/src/soc/intel/tigerlake/include/soc/romstage.h b/src/soc/intel/tigerlake/include/soc/romstage.h index baa35c5..1cffcb9 100644 --- a/src/soc/intel/tigerlake/include/soc/romstage.h +++ b/src/soc/intel/tigerlake/include/soc/romstage.h @@ -6,7 +6,7 @@ #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); +const char *mainboard_get_dram_part_num(void); void mainboard_memory_init_params(FSPM_UPD *mupd); void systemagent_early_init(void); void romstage_pch_init(void); diff --git a/src/soc/intel/tigerlake/romstage/romstage.c b/src/soc/intel/tigerlake/romstage/romstage.c index db014ea..fd18fea 100644 --- a/src/soc/intel/tigerlake/romstage/romstage.c +++ b/src/soc/intel/tigerlake/romstage/romstage.c @@ -21,10 +21,10 @@ 0x8d, 0x09, 0x11, 0xcf, 0x8b, 0x9f, 0x03, 0x23 \ }
-bool __weak mainboard_get_dram_part_num(const char **part_num, size_t *len) +const char * __weak mainboard_get_dram_part_num(void) { /* Default weak implementation, no need to override part number. */ - return false; + return NULL; }
/* Save the DIMM information for SMBIOS table 17 */ @@ -42,8 +42,8 @@ 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; + size_t dram_part_num_len = 0; + bool part_name_overridden = false;
/* Locate the memory info HOB, presence validated by raminit */ meminfo_hob = fsp_find_extension_hob_by_guid( @@ -66,8 +66,11 @@ 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); + dram_part_num = mainboard_get_dram_part_num(); + if (dram_part_num) { + dram_part_num_len = strlen(dram_part_num); + part_name_overridden = true; + }
/* Save available DIMM information */ index = 0; @@ -89,7 +92,7 @@
/* If there is no DRAM part number overridden by * mainboard then use original one. */ - if (!is_dram_part_overridden) { + if (!part_name_overridden) { dram_part_num_len = sizeof(src_dimm->ModulePartNum); dram_part_num = (const char *) &src_dimm->ModulePartNum[0];