Attention is currently required from: Sean Rhodes, Andy Pont, Stefan Reinauer. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61550 )
Change subject: payloads/tianocore: Rework Makefile ......................................................................
Patch Set 8:
(6 comments)
File payloads/external/tianocore/Kconfig:
https://review.coreboot.org/c/coreboot/+/61550/comment/f490f9d2_4c2a5df2 PS8, Line 9: config TIANOCORE_REPOSITORY : string "URL to git repository for edk2" : default "https://github.com/tianocore/edk2" if TIANOCORE_UPSTREAM : default "https://github.com/mrchromebox/edk2" I think this should default to "https://review.coreboot.org/edk2", which should in theory be a superset of the mrchromebox repo.
https://review.coreboot.org/c/coreboot/+/61550/comment/9c8ea935_2c60fd8f PS8, Line 14: TIANOCORE_TAG since this doesn't need to be a tag, maybe TIANOCORE_TAG_OR_REV ?
https://review.coreboot.org/c/coreboot/+/61550/comment/d8119693_a6b23b06 PS8, Line 82: Disabling this option, which will reserve memory above 4G, I think this could be phrased better - Though it's probable that nobody will misunderstand, It seems to be saying that disabling the option will enable the memory above 4G.
maybe "Disabling memory above 4G is useful..."
https://review.coreboot.org/c/coreboot/+/61550/comment/4976f313_48eb7d36 PS8, Line 128: without an internal with an external display?
https://review.coreboot.org/c/coreboot/+/61550/comment/cafd8529_a9edcc83 PS8, Line 136: by ~220KB. This will probably change in the future, so maybe say something like "as of Jan 2022, this increases..."
File payloads/external/tianocore/Makefile:
https://review.coreboot.org/c/coreboot/+/61550/comment/f86e5aee_ebe8a030 PS8, Line 26: -D A Nit: I think we usually prefer to have -D options without the space. I don't think it's particularly important, and I see that it wasn't that way in the old makefile, but I thought I'd mention it.