Change in coreboot[master]: mb/intel/tglrvp: add Tiger Lake memory initialization support
Srinidhi N Kaushik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38998 )
Change subject: mb/intel/tglrvp: add Tiger Lake memory initialization support ......................................................................
Patch Set 15: Code-Review+1
(7 comments)
https://review.coreboot.org/c/coreboot/+/38998/13//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/38998/13//COMMIT_MSG@13 PS13, Line 13: Upds
UPDs
Done
https://review.coreboot.org/c/coreboot/+/38998/13/src/mainboard/intel/tglrvp... File src/mainboard/intel/tglrvp/romstage_fsp_params.c:
https://review.coreboot.org/c/coreboot/+/38998/13/src/mainboard/intel/tglrvp... PS13, Line 48: spd_index = SPD_ID_MICRON;
Why this assignment?
default case to chose a more generic/stable spd if we get a board_id not in list.
https://review.coreboot.org/c/coreboot/+/38998/13/src/mainboard/intel/tglrvp... PS13, Line 49: printk(BIOS_INFO, "Invalid board_id 0x%x\n", board_id);
Sounds more like an error (or at least warning).
Done
https://review.coreboot.org/c/coreboot/+/38998/13/src/mainboard/intel/tglrvp... PS13, Line 51:
Can we add Windows EC case which doesn't report DDR4? […]
Will do in the other patch for DDR4 config
https://review.coreboot.org/c/coreboot/+/38998/13/src/mainboard/intel/tglrvp... PS13, Line 61: if (spd_index == SPD_ADDR_TABLE) {
Variable not needed as only used once?
Done
https://review.coreboot.org/c/coreboot/+/38998/13/src/mainboard/intel/tglrvp... PS13, Line 73: 0
false
Done
https://review.coreboot.org/c/coreboot/+/38998/13/src/mainboard/intel/tglrvp... File src/mainboard/intel/tglrvp/spd/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/38998/13/src/mainboard/intel/tglrvp... PS13, Line 22: ifeq ($(SPD_SOURCES),) : SPD_DEPS := $(error SPD_SOURCES is not set. Variant must provide this) : else
Given that the SPD_SOURCES are set just couple of lines above, this check looks redundant.
Done
-
Srinidhi N Kaushik (Code Review)