Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/23851 )
Change subject: soc/intel/denverton_ns: Fill dimm info for SMBIOS table 17 ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/#/c/23851/1/src/soc/intel/denverton_ns/hob_mem.c File src/soc/intel/denverton_ns/hob_mem.c:
https://review.coreboot.org/#/c/23851/1/src/soc/intel/denverton_ns/hob_mem.c... PS1, Line 56: #if IS_ENABLED(CONFIG_DISPLAY_HOBS) please don't use that many preprocessor directives. It would be much easier to understand if you'd split it into multiple files.
https://review.coreboot.org/#/c/23851/1/src/soc/intel/denverton_ns/hob_mem.c... PS1, Line 58: const FSP_SMBIOS_MEMORY_INFO *memory_info_hob) align with opening bracket
https://review.coreboot.org/#/c/23851/1/src/soc/intel/denverton_ns/hob_mem.c... PS1, Line 165: if (dimm_info->SizeInMb) { if (!dimm_info->SizeInMb) continue;
to reduce indentation level
https://review.coreboot.org/#/c/23851/1/src/soc/intel/denverton_ns/hob_mem.c... PS1, Line 223: struct memory_info *mem_info, align with opening bracket
-
Patrick Rudolph (Code Review)