Hello Jamie Ryu,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/39359
to review the following change.
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
soc/intel/tigerlake: Save DIMM info by available nodes
TEST= make build image returns correct dimm info by mosys and dmidecode with TGL-U and TGL-Y + MICA + LPDDR4
Signed-off-by: Jamie Ryu jamie.m.ryu@intel.com Change-Id: I951ea94c280b7dd5b67f320a264d13fca82a4596 --- M src/soc/intel/tigerlake/romstage/romstage.c 1 file changed, 40 insertions(+), 33 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/39359/1
diff --git a/src/soc/intel/tigerlake/romstage/romstage.c b/src/soc/intel/tigerlake/romstage/romstage.c index 17efc98..f592bb0 100644 --- a/src/soc/intel/tigerlake/romstage/romstage.c +++ b/src/soc/intel/tigerlake/romstage/romstage.c @@ -37,22 +37,23 @@ /* Save the DIMM information for SMBIOS table 17 */ static void save_dimm_info(void) { - int channel, dimm, dimm_max, index; + int node, channel, dimm, dimm_max, index; size_t hob_size; const CONTROLLER_INFO *ctrlr_info; const CHANNEL_INFO *channel_info; const DIMM_INFO *src_dimm; struct dimm_info *dest_dimm; struct memory_info *mem_info; - const MEMORY_INFO_DATA_HOB *memory_info_hob; + const MEMORY_INFO_DATA_HOB *meminfo_hob; const uint8_t smbios_memory_info_guid[16] = FSP_SMBIOS_MEMORY_INFO_GUID; + const uint8_t *serial_num;
/* Locate the memory info HOB, presence validated by raminit */ - memory_info_hob = fsp_find_extension_hob_by_guid( + meminfo_hob = fsp_find_extension_hob_by_guid( smbios_memory_info_guid, &hob_size); - if (memory_info_hob == NULL || hob_size == 0) { + if (meminfo_hob == NULL || hob_size == 0) { printk(BIOS_ERR, "SMBIOS MEMORY_INFO_DATA_HOB not found\n"); return; } @@ -68,40 +69,46 @@ } memset(mem_info, 0, sizeof(*mem_info));
- /* Describe the first N DIMMs in the system */ + /* Save available DIMM information */ index = 0; dimm_max = ARRAY_SIZE(mem_info->dimm); - ctrlr_info = &memory_info_hob->Controller[0]; - for (channel = 0; channel < MAX_CH && index < dimm_max; channel++) { - channel_info = &ctrlr_info->ChannelInfo[channel]; - if (channel_info->Status != CHANNEL_PRESENT) - continue; - for (dimm = 0; dimm < MAX_DIMM && index < dimm_max; dimm++) { - src_dimm = &channel_info->DimmInfo[dimm]; - dest_dimm = &mem_info->dimm[index]; - - if (src_dimm->Status != DIMM_PRESENT) + for (node = 0; node < MAX_NODE; node++) { + ctrlr_info = &meminfo_hob->Controller[node]; + for (channel = 0; channel < MAX_CH && index < dimm_max; + channel++) { + channel_info = &ctrlr_info->ChannelInfo[channel]; + if (channel_info->Status != CHANNEL_PRESENT) continue;
- u8 memProfNum = memory_info_hob->MemoryProfile; + for (dimm = 0; dimm < MAX_DIMM && index < dimm_max; + dimm++) { + src_dimm = &channel_info->DimmInfo[dimm]; + dest_dimm = &mem_info->dimm[index]; + if (src_dimm->Status != DIMM_PRESENT) + continue;
- /* Populate the DIMM information */ - dimm_info_fill(dest_dimm, - src_dimm->DimmCapacity, - memory_info_hob->MemoryType, - memory_info_hob->ConfiguredMemoryClockSpeed, - src_dimm->RankInDimm, - channel_info->ChannelId, - src_dimm->DimmId, - (const char *)src_dimm->ModulePartNum, - sizeof(src_dimm->ModulePartNum), - src_dimm->SpdSave + SPD_SAVE_OFFSET_SERIAL, - memory_info_hob->DataWidth, - memory_info_hob->VddVoltage[memProfNum], - memory_info_hob->EccSupport, - src_dimm->MfgId, - src_dimm->SpdModuleType); - index++; + u8 memProfNum = meminfo_hob->MemoryProfile; + serial_num = src_dimm->SpdSave + + SPD_SAVE_OFFSET_SERIAL; + + /* Populate the DIMM information */ + dimm_info_fill(dest_dimm, + src_dimm->DimmCapacity, + meminfo_hob->MemoryType, + meminfo_hob->ConfiguredMemoryClockSpeed, + src_dimm->RankInDimm, + channel_info->ChannelId, + src_dimm->DimmId, + (const char *)src_dimm->ModulePartNum, + sizeof(src_dimm->ModulePartNum), + serial_num, + meminfo_hob->DataWidth, + meminfo_hob->VddVoltage[memProfNum], + meminfo_hob->EccSupport, + src_dimm->MfgId, + src_dimm->SpdModuleType); + index++; + } } } mem_info->dimm_cnt = index;
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39359 )
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
Patch Set 1:
The code of this patch does not seem to have been later removed from the private branch.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39359 )
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
Patch Set 1:
(2 comments)
+Karthik - Most likely this is not going to work for JSL.
https://review.coreboot.org/c/coreboot/+/39359/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39359/1//COMMIT_MSG@9 PS1, Line 9: TEST= make build image returns correct dimm info by mosys and dmidecode : with TGL-U and TGL-Y + MICA + LPDDR4 Can you verify if this works for volteer as well?
https://review.coreboot.org/c/coreboot/+/39359/1/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/romstage.c:
https://review.coreboot.org/c/coreboot/+/39359/1/src/soc/intel/tigerlake/rom... PS1, Line 75: MAX_NODE Where is this defined?
Jamie Ryu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39359 )
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/39359/1/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/romstage.c:
https://review.coreboot.org/c/coreboot/+/39359/1/src/soc/intel/tigerlake/rom... PS1, Line 75: MAX_NODE
Where is this defined?
MAX_NODE is defined @ src/vendorcode/intel/fsp/fsp2_0/tigerlake/MemInfoHob.h
#define MAX_NODE 2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39359 )
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39359/1/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/romstage.c:
https://review.coreboot.org/c/coreboot/+/39359/1/src/soc/intel/tigerlake/rom... PS1, Line 75: MAX_NODE
MAX_NODE is defined @ src/vendorcode/intel/fsp/fsp2_0/tigerlake/MemInfoHob.h […]
Ah Thanks!
Hello build bot (Jenkins), Jamie Ryu, Caveh Jalali, Nick Vaccaro, Subrata Banik, Patrick Rudolph, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/39359
to look at the new patch set (#2).
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
soc/intel/tigerlake: Save DIMM info by available nodes
TEST=Verified that cherry-picked code compiled, by running "make" after selecting "Volteer" board.
Signed-off-by: Jamie Ryu jamie.m.ryu@intel.com Change-Id: I951ea94c280b7dd5b67f320a264d13fca82a4596 --- M src/soc/intel/tigerlake/romstage/romstage.c 1 file changed, 40 insertions(+), 33 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/39359/2
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39359 )
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
Patch Set 2:
(2 comments)
+Karthik - Most likely this is not going to work for JSL.
Is that a problem? This code change is in a "tigerlake" directory. Of course, we can talk about whether the code can or cannot be shared with other platforms in the future, but I do not take this comment as challenging whether it is OK to commit as is.
https://review.coreboot.org/c/coreboot/+/39359/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39359/1//COMMIT_MSG@9 PS1, Line 9: TEST= make build image returns correct dimm info by mosys and dmidecode : with TGL-U and TGL-Y + MICA + LPDDR4
Can you verify if this works for volteer as well?
This TEST= line is copied from the original CL. I have tried compiling on the coreboot.org top of tree with this cherry-pick, but I have no idea what the "correct dimm info" would be. I cannot run the code on a Volteer device, because the coreboot.org repo is not yet in a state where it can boot on Volteer, that is what we are trying to get to.
https://review.coreboot.org/c/coreboot/+/39359/1/src/soc/intel/tigerlake/rom... File src/soc/intel/tigerlake/romstage/romstage.c:
https://review.coreboot.org/c/coreboot/+/39359/1/src/soc/intel/tigerlake/rom... PS1, Line 75: MAX_NODE
Ah Thanks!
Ack
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39359 )
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
Patch Set 2:
Patch Set 2:
(2 comments)
+Karthik - Most likely this is not going to work for JSL.
Is that a problem? This code change is in a "tigerlake" directory. Of course, we can talk about whether the code can or cannot be shared with other platforms in the future, but I do not take this comment as challenging whether it is OK to commit as is.
Right now, jasperlake is actually using "tigerlake" directory. So, it does impact jasperlake boards. But I think this is most likely already broken for jasperlake, so your change probably doesn't break it more. I have added Karthik as FYI here.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39359 )
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39359/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39359/1//COMMIT_MSG@9 PS1, Line 9: TEST= make build image returns correct dimm info by mosys and dmidecode : with TGL-U and TGL-Y + MICA + LPDDR4
This TEST= line is copied from the original CL. I have tried compiling on the coreboot. […]
I think Nick mentioned that chromium coreboot boots up on volteer. Can you please check with him?
You can use dmidecode to ensure that it displays the right memory information.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39359 )
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
(2 comments)
+Karthik - Most likely this is not going to work for JSL.
Is that a problem? This code change is in a "tigerlake" directory. Of course, we can talk about whether the code can or cannot be shared with other platforms in the future, but I do not take this comment as challenging whether it is OK to commit as is.
Right now, jasperlake is actually using "tigerlake" directory. So, it does impact jasperlake boards. But I think this is most likely already broken for jasperlake, so your change probably doesn't break it more. I have added Karthik as FYI here.
In Jasperlake MAX_NODE is 1 as per the FSP. So all the DIMM info would have been filled earlier.
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39359 )
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
Patch Set 2:
please set topic to TGL_UPSTREAM.
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39359 )
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39359/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39359/1//COMMIT_MSG@9 PS1, Line 9: TEST= make build image returns correct dimm info by mosys and dmidecode : with TGL-U and TGL-Y + MICA + LPDDR4
I think Nick mentioned that chromium coreboot boots up on volteer. Can you please check with him? […]
mosys now reports 8 devices @ x16 (instead of 4 devices):
localhost ~ #dmidecodemosys memory spd print geometryall 0 | LPDDR4 1 | LPDDR4 2 | LPDDR4 3 | LPDDR4 4 | LPDDR4 5 | LPDDR4 6 | LPDDR4 7 | LPDDR4 0 | 1-0 | 00000000 | None 1 | 1-0 | 00000000 | None 2 | 1-0 | 00000000 | None 3 | 1-0 | 00000000 | None 4 | 1-0 | 00000000 | None 5 | 1-0 | 00000000 | None 6 | 1-0 | 00000000 | None 7 | 1-0 | 00000000 | None 0 | 1024 | 1 | 16 1 | 1024 | 1 | 16 2 | 1024 | 1 | 16 3 | 1024 | 1 | 16 4 | 1024 | 1 | 16 5 | 1024 | 1 | 16 6 | 1024 | 1 | 16 7 | 1024 | 1 | 16 0 | LPDDR4-2133 1 | LPDDR4-2133 2 | LPDDR4-2133 3 | LPDDR4-2133 4 | LPDDR4-2133 5 | LPDDR4-2133 6 | LPDDR4-2133 7 | LPDDR4-2133 localhost~ #
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39359 )
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39359/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39359/1//COMMIT_MSG@9 PS1, Line 9: TEST= make build image returns correct dimm info by mosys and dmidecode : with TGL-U and TGL-Y + MICA + LPDDR4
mosys now reports 8 devices @ x16 (instead of 4 devices): […]
that command should read mosys, not dmidecodemosys.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39359 )
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39359/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39359/1//COMMIT_MSG@9 PS1, Line 9: TEST= make build image returns correct dimm info by mosys and dmidecode : with TGL-U and TGL-Y + MICA + LPDDR4
that command should read mosys, not dmidecodemosys.
Based on MAX_NODE, MAX_CH and ARRAY_SIZE(mem_info->dimm), 8 entries seem correct.
For JSL, MAX_NODE is 1 and hence the old code itself looks fine. Since I don't have physical access to the mainboard, I could not verify it on the board. I can re-visit this CL if things are broken for JSL.
A question out of curiosity: for TGL 16 entries can be added(MAX_NODE: 2, MAX_CH: 4 and MAX_DIMM: 2). But only 8 entries are added because of ARRAY_SIZE(mem_info->dimm). Do we need to increase that array size in the future? Is there a reason for that limit?
Jes Klinke has uploaded a new patch set (#3) to the change originally created by Jes Klinke. ( https://review.coreboot.org/c/coreboot/+/39359 )
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
soc/intel/tigerlake: Save DIMM info by available nodes
TEST=Verified that dmidecode produces output identical to private repo
Signed-off-by: Jamie Ryu jamie.m.ryu@intel.com Change-Id: I951ea94c280b7dd5b67f320a264d13fca82a4596 --- M src/soc/intel/tigerlake/romstage/romstage.c 1 file changed, 40 insertions(+), 33 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/39359/3
Jes Klinke has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39359 )
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39359/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39359/1//COMMIT_MSG@9 PS1, Line 9: TEST= make build image returns correct dimm info by mosys and dmidecode : with TGL-U and TGL-Y + MICA + LPDDR4
Based on MAX_NODE, MAX_CH and ARRAY_SIZE(mem_info->dimm), 8 entries seem correct. […]
I have used dmidecode to verify that public coreboot with this patch displays the same information in the Memory Device section, as do the private intel repo.
Assuming we want the public coreboot to be "on par" with Intels private repo, in order to stop using the latter, I guess that is sufficient.
I cannot speak to whether the displayed information is correct, nor whether array sizes should be adjusted. Perhaps we can have a followup CL.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39359 )
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
Patch Set 3: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39359 )
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39359/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39359/1//COMMIT_MSG@9 PS1, Line 9: TEST= make build image returns correct dimm info by mosys and dmidecode : with TGL-U and TGL-Y + MICA + LPDDR4
I have used dmidecode to verify that public coreboot with this patch displays the same information i […]
SGTM. Thanks Jes!
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39359 )
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
Patch Set 3: Code-Review+1
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39359 )
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
Patch Set 3: Code-Review+1
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39359 )
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/39359/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39359/1//COMMIT_MSG@9 PS1, Line 9: TEST= make build image returns correct dimm info by mosys and dmidecode : with TGL-U and TGL-Y + MICA + LPDDR4
SGTM. […]
Done
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39359 )
Change subject: soc/intel/tigerlake: Save DIMM info by available nodes ......................................................................
soc/intel/tigerlake: Save DIMM info by available nodes
TEST=Verified that dmidecode produces output identical to private repo
Signed-off-by: Jamie Ryu jamie.m.ryu@intel.com Change-Id: I951ea94c280b7dd5b67f320a264d13fca82a4596 Reviewed-on: https://review.coreboot.org/c/coreboot/+/39359 Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Caveh Jalali caveh@chromium.org Reviewed-by: Wonkyu Kim wonkyu.kim@intel.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/tigerlake/romstage/romstage.c 1 file changed, 40 insertions(+), 33 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Caveh Jalali: Looks good to me, but someone else must approve Wonkyu Kim: Looks good to me, but someone else must approve Jamie Ryu: Looks good to me, but someone else must approve
diff --git a/src/soc/intel/tigerlake/romstage/romstage.c b/src/soc/intel/tigerlake/romstage/romstage.c index 17efc98..f592bb0 100644 --- a/src/soc/intel/tigerlake/romstage/romstage.c +++ b/src/soc/intel/tigerlake/romstage/romstage.c @@ -37,22 +37,23 @@ /* Save the DIMM information for SMBIOS table 17 */ static void save_dimm_info(void) { - int channel, dimm, dimm_max, index; + int node, channel, dimm, dimm_max, index; size_t hob_size; const CONTROLLER_INFO *ctrlr_info; const CHANNEL_INFO *channel_info; const DIMM_INFO *src_dimm; struct dimm_info *dest_dimm; struct memory_info *mem_info; - const MEMORY_INFO_DATA_HOB *memory_info_hob; + const MEMORY_INFO_DATA_HOB *meminfo_hob; const uint8_t smbios_memory_info_guid[16] = FSP_SMBIOS_MEMORY_INFO_GUID; + const uint8_t *serial_num;
/* Locate the memory info HOB, presence validated by raminit */ - memory_info_hob = fsp_find_extension_hob_by_guid( + meminfo_hob = fsp_find_extension_hob_by_guid( smbios_memory_info_guid, &hob_size); - if (memory_info_hob == NULL || hob_size == 0) { + if (meminfo_hob == NULL || hob_size == 0) { printk(BIOS_ERR, "SMBIOS MEMORY_INFO_DATA_HOB not found\n"); return; } @@ -68,40 +69,46 @@ } memset(mem_info, 0, sizeof(*mem_info));
- /* Describe the first N DIMMs in the system */ + /* Save available DIMM information */ index = 0; dimm_max = ARRAY_SIZE(mem_info->dimm); - ctrlr_info = &memory_info_hob->Controller[0]; - for (channel = 0; channel < MAX_CH && index < dimm_max; channel++) { - channel_info = &ctrlr_info->ChannelInfo[channel]; - if (channel_info->Status != CHANNEL_PRESENT) - continue; - for (dimm = 0; dimm < MAX_DIMM && index < dimm_max; dimm++) { - src_dimm = &channel_info->DimmInfo[dimm]; - dest_dimm = &mem_info->dimm[index]; - - if (src_dimm->Status != DIMM_PRESENT) + for (node = 0; node < MAX_NODE; node++) { + ctrlr_info = &meminfo_hob->Controller[node]; + for (channel = 0; channel < MAX_CH && index < dimm_max; + channel++) { + channel_info = &ctrlr_info->ChannelInfo[channel]; + if (channel_info->Status != CHANNEL_PRESENT) continue;
- u8 memProfNum = memory_info_hob->MemoryProfile; + for (dimm = 0; dimm < MAX_DIMM && index < dimm_max; + dimm++) { + src_dimm = &channel_info->DimmInfo[dimm]; + dest_dimm = &mem_info->dimm[index]; + if (src_dimm->Status != DIMM_PRESENT) + continue;
- /* Populate the DIMM information */ - dimm_info_fill(dest_dimm, - src_dimm->DimmCapacity, - memory_info_hob->MemoryType, - memory_info_hob->ConfiguredMemoryClockSpeed, - src_dimm->RankInDimm, - channel_info->ChannelId, - src_dimm->DimmId, - (const char *)src_dimm->ModulePartNum, - sizeof(src_dimm->ModulePartNum), - src_dimm->SpdSave + SPD_SAVE_OFFSET_SERIAL, - memory_info_hob->DataWidth, - memory_info_hob->VddVoltage[memProfNum], - memory_info_hob->EccSupport, - src_dimm->MfgId, - src_dimm->SpdModuleType); - index++; + u8 memProfNum = meminfo_hob->MemoryProfile; + serial_num = src_dimm->SpdSave + + SPD_SAVE_OFFSET_SERIAL; + + /* Populate the DIMM information */ + dimm_info_fill(dest_dimm, + src_dimm->DimmCapacity, + meminfo_hob->MemoryType, + meminfo_hob->ConfiguredMemoryClockSpeed, + src_dimm->RankInDimm, + channel_info->ChannelId, + src_dimm->DimmId, + (const char *)src_dimm->ModulePartNum, + sizeof(src_dimm->ModulePartNum), + serial_num, + meminfo_hob->DataWidth, + meminfo_hob->VddVoltage[memProfNum], + meminfo_hob->EccSupport, + src_dimm->MfgId, + src_dimm->SpdModuleType); + index++; + } } } mem_info->dimm_cnt = index;
-
Caveh Jalali (Code Review) -
Furquan Shaikh (Code Review)
-
Jamie Ryu (Code Review)
-
Jes Klinke (Code Review)
-
Karthik Ramasubramanian (Code Review)
-
Patrick Georgi (Code Review)
-
Wonkyu Kim (Code Review)