Attention is currently required from: Jason Glenesk, Jakub Czapiga, Marshall Dawson, Tim Wawrzynczak, Angel Pons, Rob Barnes, Patrick Rudolph, Felix Held. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56628 )
Change subject: arch/x86: Refactor the SMBIOS type 17 write function ......................................................................
Patch Set 21:
(4 comments)
File src/include/device/dram/spd.h:
https://review.coreboot.org/c/coreboot/+/56628/comment/f777ec8b_0cdb5b5f PS21, Line 14: __packed
Sure, and i think changing the order would make more sense rather __packed here
Ack
File tests/lib/dimm_info_util-test.c:
https://review.coreboot.org/c/coreboot/+/56628/comment/1f3b7edb_de933516 PS21, Line 189: const LargestIntegralType udimm_allowed[] = { : DDR5_SPD_UDIMM, DDR5_SPD_MINI_UDIMM, : }; : : const LargestIntegralType rdimm_allowed[] = { DDR5_SPD_RDIMM, DDR5_SPD_MINI_RDIMM }; :
could you merge all of these tests into one, which would get `udimm_allowed`, `rdimm_allowed` and `e […]
Ack
https://review.coreboot.org/c/coreboot/+/56628/comment/82193848_04898c50 PS21, Line 215: struct memory_info
How about: `struct smbios_form_factor_test_info`? Would it be better?
Ack
https://review.coreboot.org/c/coreboot/+/56628/comment/797f7cbf_b4a08596 PS21, Line 219: { MEMORY_TYPE_DDR2, test_smbios_form_factor_to_spd_mod_type_on_ddr2 }, : { MEMORY_TYPE_DDR3, test_smbios_form_factor_to_spd_mod_type_on_ddr3 }, : { MEMORY_TYPE_DDR4, test_smbios_form_factor_to_spd_mod_type_on_ddr4 }, : { MEMORY_TYPE_DDR5, test_smbios_form_factor_to_spd_mod_type_on_ddr5 },
I think, that ddr tests can be in another test case, than lpddrx. […]
Ack
-
Subrata Banik (Code Review)