Attention is currently required from: Angel Pons, Arthur Heymans, Eric Lai, Felix Held, Felix Singer, Nico Huber, Patrick Rudolph.
Alicja Michalska has posted comments on this change by Angel Pons. ( https://review.coreboot.org/c/coreboot/+/64186?usp=email )
Change subject: haswell NRI: Configure initial MC settings ......................................................................
Patch Set 7:
(5 comments)
File src/northbridge/intel/haswell/native_raminit/configure_mc.c:
https://review.coreboot.org/c/coreboot/+/64186/comment/94fdb6b4_659eaac0?usp... : PS7, Line 16: if (!is_hsw_ult()) What's the purpose of this check? Does NRI currently support ULT CPUs?
https://review.coreboot.org/c/coreboot/+/64186/comment/80a68708_f02231b3?usp... : PS7, Line 641: if (!(reg32 & BIT(31))) { /** TODO: What to do if this is locked? **/ If it's locked, then reading the register will fail, right? I think it would be nice to put a debug message here
https://review.coreboot.org/c/coreboot/+/64186/comment/3d0f01f6_d738c7b3?usp... : PS7, Line 660: /** TODO: Remember why this is only done on cold boots **/ Stored in cache?
File src/northbridge/intel/haswell/native_raminit/raminit_main.c:
https://review.coreboot.org/c/coreboot/+/64186/comment/ee0600d7_740d010b?usp... : PS7, Line 58: ctrl->vdd_mv = is_hsw_ult() ? 1350 : 1500; /** FIXME: Hardcoded, does it matter? **/ Probably not, DDR3U isn't very common :)
File src/northbridge/intel/haswell/native_raminit/reg_structs.h:
https://review.coreboot.org/c/coreboot/+/64186/comment/fd7499bb_b09e415f?usp... : PS7, Line 12: int32_t vref : 6; // Bits 31:26 s/int/uint, or is there a good reason for it? There are few more occurrences in this file
-
Alicja Michalska (Code Review)