Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39136 )
Change subject: mb/google/dedede: Add memory initialization support for dedede ......................................................................
Patch Set 18:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39136/18/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/baseboard/memory.c:
https://review.coreboot.org/c/coreboot/+/39136/18/src/mainboard/google/deded... PS18, Line 23: ; Should we have a default config here instead of NULL. I think NULL will cause the memory initialization code to die. Below we are passing empty SPD. I am wondering if something similar for memcfg be done.
https://review.coreboot.org/c/coreboot/+/39136/18/src/mainboard/google/deded... File src/mainboard/google/dedede/variants/waddledoo/memory.c:
https://review.coreboot.org/c/coreboot/+/39136/18/src/mainboard/google/deded... PS18, Line 28: 0xff, 0x0 As per the available documentation, 0xff corresponds to CS signal used for all the 8 bytes in a package. But looking at the schematics
LPD4x_0_CS[0:1] is used for bytes 0 - 3. LPD4x_1_CS[0:1] is used for bytes 4 - 7. So shouldn't it also be {0xf, 0xf0} like how it is done for CLK and CMD signals.