Attention is currently required from: Maulik V Vaghela. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51027 )
Change subject: soc/intel/adl: Allow mainboard to fill CmdMirror and DqDqsRetraining ......................................................................
Patch Set 6:
(5 comments)
Patchset:
PS6: Sorry about the late comments. I was out end of last week and just got to this CL. Can you please help address these comments in a follow-up? Thanks!
File src/soc/intel/alderlake/include/soc/meminit.h:
https://review.coreboot.org/c/coreboot/+/51027/comment/97c86f60_49eedce5 PS6, Line 107: /* Command Mirror */ This comment is not very helpful. Under what conditions would this be set? Also, what are the expected values for this - 0/1? I think it would be better to use bool if this is supposed to be 0/1.
https://review.coreboot.org/c/coreboot/+/51027/comment/89afedd4_6ea5c6f0 PS6, Line 110: TxDqDqs Retraining Under what conditions is this supposed to be set? Is this applicable to both memory down and dimms?
https://review.coreboot.org/c/coreboot/+/51027/comment/35acf4bc_40c4b65f PS6, Line 111: LpDdr I think it might be better to rename this to just `tx_dq_dqs_retrain`. "LpDdr" as the prefix made me think that this is applicable only to LP memory types.
File src/soc/intel/alderlake/meminit.c:
https://review.coreboot.org/c/coreboot/+/51027/comment/51dc3699_00da7d7b PS6, Line 229: /* Fill command mirror for memory */ Comments for CmdMirror and LpDdrDqDqsReTraining are not really helpful. I think these can be dropped as they are basically just describing what the code does.