Attention is currently required from: Bora Guvendik, Maulik V Vaghela, Selma Bensaid, Tim Wawrzynczak, Patrick Rudolph. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54036 )
Change subject: [WIP]soc/intel/alderlake: Update meminit code due to upd changes FSP 2147 onwards ......................................................................
Patch Set 1:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/54036/comment/9508690c_d07c0d70 PS1, Line 7: WIP Understand that this is still WIP. Provided some early comments.
File src/soc/intel/alderlake/meminit.c:
https://review.coreboot.org/c/coreboot/+/54036/comment/e5983c30_2f6d581d PS1, Line 135: disable_dimm_upds This should be renamed to `disable_channel_upds` and the pointers need update too.
https://review.coreboot.org/c/coreboot/+/54036/comment/3f977e96_d0ebcbd8 PS1, Line 164: nit: Instead of disabling all channels individually above, you can use a flag here: bool enable_channel = 0;
and do this:
if (*spd_ptr) { enable_channel = 1; }
and after the `for (dimm = 0...)` loop, you can set:
*disable_channel_ptr = !enable_channel;
https://review.coreboot.org/c/coreboot/+/54036/comment/ec197a15_7c81041f PS1, Line 168: break; This is wrong. It will not set the SPD pointer for second DIMM in case of DIMM modules where there are multiple DIMMs per channel.