Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/18548 )
Change subject: nb/intel/i945: Programm CxODT value for each channel ......................................................................
Patch Set 26: Code-Review+2
(2 comments)
https://review.coreboot.org/c/coreboot/+/18548/22/src/northbridge/intel/i945... File src/northbridge/intel/i945/raminit.c:
https://review.coreboot.org/c/coreboot/+/18548/22/src/northbridge/intel/i945... PS22, Line 2463: (dimm_mask & 3) != 3
you have to look up what this means, while the previous statement was very clear.
It can be written even clearer by pulling the outer ! in:
if (sysinfo->dimm[0] == SYSINFO_DIMM_NOT_POPULATED || sysinfo->dimm[1] == SYSINFO_DIMM_NOT_POPULATED)
https://review.coreboot.org/c/coreboot/+/18548/22/src/northbridge/intel/i945... PS22, Line 2473: MCHBAR32(C1ODT)
There is no documentation on this one and only very limited documentation on C0ODT. […]
ODT is no magic, so I think it's fair to assume that the current code doesn't work if both channels are not equally populated. But the new code might work and can't break anything if they are.
-
Nico Huber (Code Review)