Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31850
Change subject: soc/intel/cannonlake: Allow mainboard to override DRAM part number ......................................................................
soc/intel/cannonlake: 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 hatch.
For other boards it should be a NOP since the weak implementation of mainboard_get_dram_part_num does nothing.
BUG=b:127609572
Change-Id: I9b2d4c33fc378b9a24b111971ec2bfdb5f8d57d0 Signed-off-by: Furquan Shaikh furquan@google.com --- M src/soc/intel/cannonlake/include/soc/romstage.h M src/soc/intel/cannonlake/romstage/romstage.c 2 files changed, 20 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/31850/1
diff --git a/src/soc/intel/cannonlake/include/soc/romstage.h b/src/soc/intel/cannonlake/include/soc/romstage.h index a58ace5..643105a 100644 --- a/src/soc/intel/cannonlake/include/soc/romstage.h +++ b/src/soc/intel/cannonlake/include/soc/romstage.h @@ -20,6 +20,9 @@ #include <fsp/api.h>
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); void systemagent_early_init(void);
/* Board type */ diff --git a/src/soc/intel/cannonlake/romstage/romstage.c b/src/soc/intel/cannonlake/romstage/romstage.c index 6abeb3d..ebbd213 100644 --- a/src/soc/intel/cannonlake/romstage/romstage.c +++ b/src/soc/intel/cannonlake/romstage/romstage.c @@ -37,6 +37,12 @@ 0x8d, 0x09, 0x11, 0xcf, 0x8b, 0x9f, 0x03, 0x23 \ }
+void __weak mainboard_get_dram_part_num(const char **part_num, size_t *len) +{ + /* Default weak implementation, no need to override part number. */ + return; +} + /* Save the DIMM information for SMBIOS table 17 */ static void save_dimm_info(void) { @@ -50,6 +56,8 @@ const MEMORY_INFO_DATA_HOB *memory_info_hob; const uint8_t smbios_memory_info_guid[16] = FSP_SMBIOS_MEMORY_INFO_GUID; + const char *dram_part_num; + size_t dram_part_num_len;
/* Locate the memory info HOB, presence validated by raminit */ memory_info_hob = fsp_find_extension_hob_by_guid( @@ -86,6 +94,13 @@ 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); + /* Populate the DIMM information */ dimm_info_fill(dest_dimm, src_dimm->DimmCapacity, @@ -94,8 +109,8 @@ src_dimm->RankInDimm, channel_info->ChannelId, src_dimm->DimmId, - (const char *)src_dimm->ModulePartNum, - sizeof(src_dimm->ModulePartNum), + dram_part_num, + dram_part_num_len, memory_info_hob->DataWidth); index++; }
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31850 )
Change subject: soc/intel/cannonlake: Allow mainboard to override DRAM part number ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/31850/1/src/soc/intel/cannonlake/romstage/ro... File src/soc/intel/cannonlake/romstage/romstage.c:
https://review.coreboot.org/#/c/31850/1/src/soc/intel/cannonlake/romstage/ro... PS1, Line 44: } void function return statements are not generally useful
Hello Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31850
to look at the new patch set (#2).
Change subject: soc/intel/cannonlake: Allow mainboard to override DRAM part number ......................................................................
soc/intel/cannonlake: 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 hatch.
For other boards it should be a NOP since the weak implementation of mainboard_get_dram_part_num does nothing.
BUG=b:127609572
Change-Id: I9b2d4c33fc378b9a24b111971ec2bfdb5f8d57d0 Signed-off-by: Furquan Shaikh furquan@google.com --- M src/soc/intel/cannonlake/include/soc/romstage.h M src/soc/intel/cannonlake/romstage/romstage.c 2 files changed, 19 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/31850/2
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31850
to look at the new patch set (#3).
Change subject: soc/intel/cannonlake: Allow mainboard to override DRAM part number ......................................................................
soc/intel/cannonlake: 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 hatch.
For other boards it should be a NOP since the weak implementation of mainboard_get_dram_part_num does nothing.
BUG=b:127609572
Change-Id: I9b2d4c33fc378b9a24b111971ec2bfdb5f8d57d0 Signed-off-by: Furquan Shaikh furquan@google.com --- M src/soc/intel/cannonlake/include/soc/romstage.h M src/soc/intel/cannonlake/romstage/romstage.c 2 files changed, 20 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/31850/3
Hello Aaron Durbin, Patrick Rudolph, Philip Chen, Duncan Laurie, Shelley Chen, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31850
to look at the new patch set (#4).
Change subject: soc/intel/cannonlake: Allow mainboard to override DRAM part number ......................................................................
soc/intel/cannonlake: 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 hatch.
For other boards it should be a NOP since the weak implementation of mainboard_get_dram_part_num does nothing.
BUG=b:127609572
Change-Id: I9b2d4c33fc378b9a24b111971ec2bfdb5f8d57d0 Signed-off-by: Furquan Shaikh furquan@google.com --- M src/soc/intel/cannonlake/include/soc/romstage.h M src/soc/intel/cannonlake/romstage/romstage.c 2 files changed, 20 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/50/31850/4
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31850 )
Change subject: soc/intel/cannonlake: Allow mainboard to override DRAM part number ......................................................................
Patch Set 4: Code-Review+2
Furquan Shaikh has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31850 )
Change subject: soc/intel/cannonlake: Allow mainboard to override DRAM part number ......................................................................
soc/intel/cannonlake: 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 hatch.
For other boards it should be a NOP since the weak implementation of mainboard_get_dram_part_num does nothing.
BUG=b:127609572
Change-Id: I9b2d4c33fc378b9a24b111971ec2bfdb5f8d57d0 Signed-off-by: Furquan Shaikh furquan@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/31850 Reviewed-by: Aaron Durbin adurbin@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/cannonlake/include/soc/romstage.h M src/soc/intel/cannonlake/romstage/romstage.c 2 files changed, 20 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved
diff --git a/src/soc/intel/cannonlake/include/soc/romstage.h b/src/soc/intel/cannonlake/include/soc/romstage.h index a58ace5..643105a 100644 --- a/src/soc/intel/cannonlake/include/soc/romstage.h +++ b/src/soc/intel/cannonlake/include/soc/romstage.h @@ -20,6 +20,9 @@ #include <fsp/api.h>
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); void systemagent_early_init(void);
/* Board type */ diff --git a/src/soc/intel/cannonlake/romstage/romstage.c b/src/soc/intel/cannonlake/romstage/romstage.c index 6abeb3d..893f37d 100644 --- a/src/soc/intel/cannonlake/romstage/romstage.c +++ b/src/soc/intel/cannonlake/romstage/romstage.c @@ -37,6 +37,11 @@ 0x8d, 0x09, 0x11, 0xcf, 0x8b, 0x9f, 0x03, 0x23 \ }
+void __weak mainboard_get_dram_part_num(const char **part_num, size_t *len) +{ + /* Default weak implementation, no need to override part number. */ +} + /* Save the DIMM information for SMBIOS table 17 */ static void save_dimm_info(void) { @@ -50,6 +55,8 @@ const MEMORY_INFO_DATA_HOB *memory_info_hob; const uint8_t smbios_memory_info_guid[16] = FSP_SMBIOS_MEMORY_INFO_GUID; + const char *dram_part_num; + size_t dram_part_num_len;
/* Locate the memory info HOB, presence validated by raminit */ memory_info_hob = fsp_find_extension_hob_by_guid( @@ -86,6 +93,14 @@ 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); + /* Populate the DIMM information */ dimm_info_fill(dest_dimm, src_dimm->DimmCapacity, @@ -94,8 +109,8 @@ src_dimm->RankInDimm, channel_info->ChannelId, src_dimm->DimmId, - (const char *)src_dimm->ModulePartNum, - sizeof(src_dimm->ModulePartNum), + dram_part_num, + dram_part_num_len, memory_info_hob->DataWidth); index++; }