Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31007 )
Change subject: nb/intel/i945: Check if interleaved even if rank #4 size is zero ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/#/c/31007/4/src/northbridge/intel/i945/raminit.c File src/northbridge/intel/i945/raminit.c:
https://review.coreboot.org/#/c/31007/4/src/northbridge/intel/i945/raminit.c... PS4, Line 2572: (sysinfo->interleaved ? 26 : 25); You might hit this multiple times for zero banks. e.g. if you have 0, 1, 4 and 5 populated, it would spuriously increase `bankaddr` here for 2 and 3.
https://review.coreboot.org/#/c/31007/4/src/northbridge/intel/i945/raminit.c... PS4, Line 2580: continue; I would probably write everything from the start of the loop as follows:
if (sysinfo->banksize[i] == 0) continue;
if (nonzero != -1) { if (sysinfo->interleaved && nonzero < 4 && i >= 4) { bankaddr = 0x40; } else { bankaddr += ... } }
Reasoning: `bankaddr` is already initialized to `0`, we should never have to set `0` later. When we find another non-zero bank, we always add the size of the last non-zero bank, unless we do interleaving and found the first non-zero bank >= 4.