Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/29352 )
Change subject: amd: [test] Fix IORR MTRR ......................................................................
Patch Set 4:
(3 comments)
You are having problems with include order, where some places AGESA.h is included firs, some places mtrr.h is included first. Options: 1) Remove the old and new definitions of IORR from mtrr.h and include AGESA.h where the definition is needed. 2) Rename the definitions slightly to avoid collision with AGESA.h. 3) Add protection to all AGESA.h files under src/vendorcode/amd/pi. 4) make sure all includes are in AGESA.h first in every file where mtrr.h and AGESA.h are included.
https://review.coreboot.org/#/c/29352/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/29352/4//COMMIT_MSG@7 PS4, Line 7: Fi amd/mtrr: Fix IORR MTRR
https://review.coreboot.org/#/c/29352/4//COMMIT_MSG@8 PS4, Line 8: Add explanation... why was it wrong, problems with include order.
https://review.coreboot.org/#/c/29352/4/src/include/cpu/amd/mtrr.h File src/include/cpu/amd/mtrr.h:
https://review.coreboot.org/#/c/29352/4/src/include/cpu/amd/mtrr.h@4 PS4, Line 4: #ifndef _AGESA_H_ I would love to have it removed from AGESA.h... however, because you're doing this for family 10 through family 16, non-stoney vendor code do use the definition. So for the time being this is an acceptable solution.