Francois Toguo Fotso has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31235 )
Change subject: soc/intel: Fix for missing mem_rank info in SMBIOS ......................................................................
Patch Set 10:
(8 comments)
Patch Set 9:
(3 comments)
The blob have already been deleted and is marked as "D" Other comments now implemented
https://review.coreboot.org/#/c/31235/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31235/5//COMMIT_MSG@8 PS5, Line 8:
Can you please add 2 or 3 line description regarding what the problem currently is without this chan […]
Done
https://review.coreboot.org/#/c/31235/5//COMMIT_MSG@9 PS5, Line 9: Found-by: Google
I am not sure if this is common to add a "Found-by:" line. […]
Done
https://review.coreboot.org/#/c/31235/5//COMMIT_MSG@10 PS5, Line 10: BUG=Wrong memory rank info in SMBIOS
Please update this line to "BUG=b:122329046" as Furquan mentioned in one of his previous comments
Done
https://review.coreboot.org/#/c/31235/5//COMMIT_MSG@11 PS5, Line 11: TEST=Boot to OS
Seems this change was done to improve the performance. […]
No this change was done to correct the memory information being repported by MOSYS.
https://review.coreboot.org/#/c/31235/5/3rdparty/blobs File 3rdparty/blobs:
https://review.coreboot.org/#/c/31235/5/3rdparty/blobs@a1 PS5, Line 1:
Is this Delete intentional? Is it related to the change?
The delete was intentional, it should not be in the patch as mentioned by one reviewer
https://review.coreboot.org/#/c/31235/9/3rdparty/blobs File 3rdparty/blobs:
PS9:
This should be removed.
It has already been removed. It is marked as "D". What elese should be done?
https://review.coreboot.org/#/c/31235/9/src/soc/intel/common/smbios.h File src/soc/intel/common/smbios.h:
https://review.coreboot.org/#/c/31235/9/src/soc/intel/common/smbios.h@24 PS9, Line 24: dim
dimm?
Done
https://review.coreboot.org/#/c/31235/9/src/soc/intel/common/smbios.c File src/soc/intel/common/smbios.c:
https://review.coreboot.org/#/c/31235/9/src/soc/intel/common/smbios.c@23 PS9, Line 23: rankInDimm
keep this the same as the param name in declaration in smbios. […]
Done