Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40370 )
Change subject: nb/i945: Rewrite peg_bits for MCHBAR16(UPMC1) ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40370/3/src/northbridge/intel/i945/... File src/northbridge/intel/i945/raminit.c:
https://review.coreboot.org/c/coreboot/+/40370/3/src/northbridge/intel/i945/... PS3, Line 2174: /* FIXME bits 5 and 0 only if PCIe graphics is disabled */ Why would you need to risk breaking things by changing this???
This is just wrong. The entire 16 bits of UPMC1 were updated before, and that is most likely intentional. Unless you have *reliable* documentation (not a mere datasheet where half the things are missing and the other half are reserved) that shows this is wrong, *and* that doing it this way fixes something without breaking anything else, then please don't change things.
In any case, you can still make things simpler without affecting the behavior:
u16 peg_bits = (1 << 4);
/* FIXME bits 5 and 0 only if PCIe graphics is disabled */ peg_bits |= (1 << 5) | (1 << 0);
if (i945_silicon_revision() > 1) peg_bits |= (1 << 12);
MCHBAR16(UPMC1) = peg_bits;