Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39599 )
Change subject: nb/intel/sandybridge: Tidy up code and comments ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39599/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/39599/2//COMMIT_MSG@16 PS2, Line 16: - Use HOST_BRIDGE instead of PCI_DEV(0, 0, 0)
Would have been nice as a separate commit on top.
I would instead prefer to spend my time making many tiny follow-ups atop this commit for things that result in changes to the binary (actual fixes, for instance). Splitting it now means that I have to undo my work, then redo it :/
And no, doing it in advance would have been annoying as well: I have done replacements on multiple occasions, as I have discovered various mutated-but-equivalent forms of HOST_BRIDGE:
PCI_DEV_SNB PCI_DEV(0,0,0) PCI_DEV(0, 0, 0) PCI_DEV(0, 0x00, 0)
https://review.coreboot.org/c/coreboot/+/39599/2/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/mchbar_regs.h:
https://review.coreboot.org/c/coreboot/+/39599/2/src/northbridge/intel/sandy... PS2, Line 16:
Mention the data sheet name and revision?
None.
Yes, literally none: some of these registers do not appear on any datasheets. Pretty much everyone who does things with raminit knows that the MCHBAR is sorely undocumented.
I mean, we even have documentation pages about this topic, if you would like to read:
https://doc.coreboot.org/northbridge/intel/sandybridge/nri.html