Makefile.inc: Add support for building AMD Onyx coreboot. #8
Conversation
Makefile.inc
Outdated
| sed -i "s|CONFIG_IFD_BIN_PATH=.*|CONFIG_IFD_BIN_PATH=\"$(COREBOOT_BLOBS_DIR)/flashregion_0_flashdescriptor.bin\"|" $@; \ | ||
| sed -i "s|CONFIG_ME_BIN_PATH=.*|CONFIG_ME_BIN_PATH=\"$(COREBOOT_BLOBS_DIR)/flashregion_2_intel_me.bin\"|" $@; \ | ||
| fi | ||
| ifndef ONYX_BUILD |
There was a problem hiding this comment.
Instead of calling the environment variable ONYX_BUILD, suggest calling it "NO_INTEL_BLOBS".
AMD Onyx CRB just happened to be the first mainboard that does not need Intel Blobs.
If NO_INTEL_BLOBS is defined, could we unset COREBOOT_BLOBS_DIR? In such case, we do not need to touch this piece of code.
There was a problem hiding this comment.
The title may be "Makefile.inc: build coreboot for non-Intel platform"
There was a problem hiding this comment.
The previous change had a mistake. I need to define control more content than the one just under the control of COREBOOT_BLOBS_DIR define, so the define approach may not be the right one. Changed the names and other suggestions.
|
The changes look good, but I think the commit message needs to be updated. It appears that we're really just telling the Makefile not to worry about Intel blobs. That doesn't necessarily have anything to do with AMD. (fwiw, I'm also looking at builds for other chips: #5) |
This change uses a projects defined Makefile variable to avoid using the Intel specific coreboot config changes implemented in the osf-builder Makefile.inc Signed-off-by: Eddie Vas <aeddiesharma@fb.com>
jonzhang-fb
left a comment
jonzhang-fb
left a comment
There was a problem hiding this comment.
LGTM. @dhendrix @ArthurHeymans I will merge this PR if no additional comment till Friday.
Looks good now. LGTM |
4 participants
This change uses an AMD coreboot defined variable from their projects Makefile, to avoid using the Intel specific coreboot config changes implemented in the osf-builder Makefile.inc
Signed-off-by: Eddie Vas aeddiesharma@meta.com