Attention is currently required from: Martin Roth, Matt DeVillier. Sean Rhodes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/61620 )
Change subject: payloads/tianocore: Rework Makefile ......................................................................
Patch Set 21:
(7 comments)
Patchset:
PS21: b
File payloads/external/tianocore/Kconfig:
https://review.coreboot.org/c/coreboot/+/61620/comment/70a653d6_6e7793d4 PS20, Line 5: $(obj)
how does this resolve to payloads/external/tianocore/output ?
payloads/external/Makefile moves it from output - the double copy will be tidied up in a separate commit.
https://review.coreboot.org/c/coreboot/+/61620/comment/67b5a446_9e47cdc4 PS20, Line 9: TIANOCORE_REPOSITORY
worth adding 9elements and/or System76's repos (possibly in a follow-up commit)?
Maybe leave it up them if they want to? All it's doing is setting REPOSITORY and TAG_OR_REV accordingly, so it's not hard to do.
https://review.coreboot.org/c/coreboot/+/61620/comment/24f82ad6_5e099c93 PS20, Line 12: TIANOCORE_UPSTREAM
should be TIANOCORE_COREBOOTPAYLOAD
Done
File payloads/external/tianocore/Makefile:
https://review.coreboot.org/c/coreboot/+/61620/comment/6c265184_4be48b6a PS20, Line 9: BUILD_STR = -q : BUILD_STR += -a IA32 -a X64 -t COREBOOT
why set these on separate lines? Also, worth adding a toggle for the -q so compilation errors can be […]
To make it easy to remove - how about we tie it to TIANOCORE_DEBUG, save another Kconfig option?
https://review.coreboot.org/c/coreboot/+/61620/comment/84b40a01_0514c777 PS20, Line 22: # OPTION = DEFAULT_VALUE : # : # ABOVE_4G_MEMORY = TRUE : ifneq ($(CONFIG_TIANOCORE_ABOVE_4G_MEMORY),y) : BUILD_STR += -D ABOVE_4G_MEMORY=FALSE : endif : # BOOTSPLASH_IMAGE = FALSE : ifneq ($(CONFIG_TIANOCORE_BOOTSPLASH_FILE),) : BUILD_STR += -D BOOTSPLASH_IMAGE=TRUE : endif : # BOOT_MANAGER_ESCAPE = FALSE : ifeq ($(CONFIG_TIANOCORE_BOOT_MANAGER_ESCAPE),y) : BUILD_STR += -D BOOT_MANAGER_ESCAPE=TRUE : endif : # BUILD_TARGETS = RELEASE : ifeq ($(CONFIG_TIANOCORE_DEBUG),y) : BUILD_STR += -b DEBUG : endif : # FOLLOW_BGRT_SPEC = FALSE : ifeq ($(CONFIG_TIANOCORE_FOLLOW_BGRT_SPEC),y) : BUILD_STR += -D FOLLOW_BGRT_SPEC=TRUE : endif : # PS2_KEYBOARD_ENABLE = FALSE : ifeq ($(CONFIG_TIANOCORE_PS2_SUPPORT),y) : BUILD_STR += -D PS2_KEYBOARD_ENABLE=TRUE : endif : # PLATFORM_BOOT_TIMEOUT = 3 : ifneq ($(TIANOCORE_BOOT_TIMEOUT),) : BUILD_STR += -D PLATFORM_BOOT_TIMEOUT=$(CONFIG_TIANOCORE_BOOT_TIMEOUT) : endif : # SIO_BUS_ENABLE = FALSE : ifeq ($(CONFIG_TIANOCORE_PS2_KEYBOARD),y) : BUILD_STR += -D SIO_BUS_ENABLE=TRUE : endif : # SHELL_TYPE = BUILD_SHELL : ifneq ($(CONFIG_TIANOCORE_HAVE_EFI_SHELL),y) : BUILD_STR += -D SHELL_TYPE=NONE : endif : # USE_CBMEM_FOR_CONSOLE = FALSE : ifeq ($(CONFIG_TIANOCORE_CBMEM_LOGGING),y) : BUILD_STR += -D USE_CBMEM_FOR_CONSOLE=TRUE : endif
I'm not sure leaving these unset for the default is ideal, since if the repo/branch being built from […]
I'd lean towards crossing that bridge if we ever need to - the build options "should" be identical for all repositories as hard coding stuff should really only be last resort or debugging.
File src/mainboard/starlabs/labtop/Kconfig:
https://review.coreboot.org/c/coreboot/+/61620/comment/a4b13334_58732925 PS20, Line 84: depends on TIANOCORE_BOOTSPLASH_IMAGE
separate commit since outside the scope of payloads/ ?
Jenkins doesn't like it being left.