Attention is currently required from: Arthur Heymans, Martin L Roth, Martin Roth.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76511?usp=email )
Change subject: vendorcode/amd: Hook up opensil ......................................................................
Patch Set 13:
(7 comments)
Patchset:
PS13: still need to look more into how the meson build system works, but already adding some comments
File src/vendorcode/amd/opensil/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/76511/comment/abd303e5_b9337426 : PS13, Line 15: else it would make the code more readable to add a comment that this os the else branch for the if CONFIG_ARCH_RAMSTAGE_X86_32. or, which might be the better solution, don't use else here and change that to and endif for the last block and a ifeq ($(CONFIG_ARCH_RAMSTAGE_X86_64),y) as condition for the 64 bit support. might be good to make sure that exactly one of those two is set
https://review.coreboot.org/c/coreboot/+/76511/comment/7f4efcf9_d541969f : PS13, Line 24: ifeq ($(CONFIG_OPENSIL_DEBUG_OUTPUT),y) add an empty line before this one, to have this if block that's unrelated to the ones before it separated for better readability
https://review.coreboot.org/c/coreboot/+/76511/comment/be656096_11fac0ef : PS13, Line 30: ifeq ($(shell printf %.1s "$(obj)"),/) still need to figure out what this is doing exactly. adding a comment would certainly help
https://review.coreboot.org/c/coreboot/+/76511/comment/8c40699c_b004432b : PS13, Line 46: $(CONFIG_C_ENV_BOOTBLOCK_SIZE) this should either be $(bios_size) or the bios_size definition in line 41 should be dropped
https://review.coreboot.org/c/coreboot/+/76511/comment/7b5466ef_33b7f1c0 : PS13, Line 69: # Don't set a meson buildtype as opensil is broken when compiler optimizations are enabled ouch :(
File src/vendorcode/amd/opensil/genoa_poc/meson_cross.template:
https://review.coreboot.org/c/coreboot/+/76511/comment/7a2df6f6_8a711234 : PS13, Line 18: '-D_PORTING_H_=1', is this needed for opensil to build?