Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40952 )
Change subject: nb/intel/sandybridge: Correct IOSAV register notes ......................................................................
Patch Set 1:
(9 comments)
https://review.coreboot.org/c/coreboot/+/40952/1/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/mchbar_regs.h:
https://review.coreboot.org/c/coreboot/+/40952/1/src/northbridge/intel/sandy... PS1, Line 14: every how many this seems a bit weird to me. not sure how i'd write that instead
https://review.coreboot.org/c/coreboot/+/40952/1/src/northbridge/intel/sandy... PS1, Line 15: Every subseq can be programmed into several MCHBAR registers "There are 4 sets of subseq registers in MCHBAR" maybe? the into several mchbar registers is rather unspecific
https://review.coreboot.org/c/coreboot/+/40952/1/src/northbridge/intel/sandy... PS1, Line 23: 'closed' "inclusive"
https://review.coreboot.org/c/coreboot/+/40952/1/src/northbridge/intel/sandy... PS1, Line 23: endpoints wouldn't call this endpoint, but not sure what i'd rather call it
https://review.coreboot.org/c/coreboot/+/40952/1/src/northbridge/intel/sandy... PS1, Line 30: Defines "configures"?
https://review.coreboot.org/c/coreboot/+/40952/1/src/northbridge/intel/sandy... PS1, Line 32: as per the rules defined in the "as configured in the" instead?
https://review.coreboot.org/c/coreboot/+/40952/1/src/northbridge/intel/sandy... PS1, Line 33: loaded "loaded"? "stored" would be what i'd expect here
https://review.coreboot.org/c/coreboot/+/40952/1/src/northbridge/intel/sandy... PS1, Line 69: shall "will"?
https://review.coreboot.org/c/coreboot/+/40952/1/src/northbridge/intel/sandy... PS1, Line 69: Defines "configures"