Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45459 )
Change subject: soc/intel/tigerlake: log the memory part name ......................................................................
Patch Set 3:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45459/3/src/include/spd_bin.h File src/include/spd_bin.h:
https://review.coreboot.org/c/coreboot/+/45459/3/src/include/spd_bin.h@50 PS3, Line 50: void spd_set_name(uint8_t spd[], char part_name[]); nit: it's functionally the same, but you declare it here as `char part_name[]` but in the .c file it's `char *part_name`.
https://review.coreboot.org/c/coreboot/+/45459/3/src/lib/spd_bin.c File src/lib/spd_bin.c:
https://review.coreboot.org/c/coreboot/+/45459/3/src/lib/spd_bin.c@140 PS3, Line 140: static bool spd_part_name_overridden = false; all statics are cleared to 0 (bss section). Also this variable seems redundant; you could just check if` spd_dram_part_name != 0` to know if the override has been called;
https://review.coreboot.org/c/coreboot/+/45459/3/src/lib/spd_bin.c@141 PS3, Line 141: spd_set_name suggestion: `spd_override_name`