[coreboot-gerrit] Change in coreboot[master]: Define BL31_SOURCE in Makefile.inc and include it for headers

[Paul Kocialkowski (Code Review)]

Paul Kocialkowski has posted comments on this change. ( https://review.coreboot.org/19136 )

Change subject: Define BL31_SOURCE in Makefile.inc and include it for headers
......................................................................


Patch Set 1:

> BL31_SOURCE was really just meant as a shorthand to save space, not
 > as something you're explicitly allowed to override. I won't
 > guarantee that there aren't other places this would break.
 
Generally speaking, I think it's important that we're able to override it, just like VBOOT_SOURCE, to specify another location for the code. There's no problem with that at the moment, as specifying another BL31_SOURCE already works perfectly.

 > If you really think we need such a functionality (we never had a
 > reason yet, and coreboot doesn't allow that for vboot either, after
 > all), it should probably be changed to a Kconfig option. It should
 > still be very explicit from the #include line that a given header
 > is an ARM TF header. If it's just "plat/vendor/file.h" it is very
 > confusing where it comes from. (But really, I'd rather we don't add
 > something like this at all and treat ARM TF just like all the other
 > submodules. You can always check it out to a different HEAD if you
 > want, or even delete the whole directory and copy or symlink
 > something else in there.)

I must admit having a arm-trusted-firmware prefix to the headers makes things a lot less confusing than including directly from <plat/...>, which doesn't clearly indicate that the header is from ATF instead of coreboot.

Overall, creating a symlink in 3rdparty/ works for me, even if it's probably not the most elegant solution. I'll go with that and abandon this change.

-- 
To view, visit https://review.coreboot.org/19136
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I148fa21a77cbb21f2b3b434e3d047a4577c991bb
Gerrit-PatchSet: 1
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Paul Kocialkowski <contact at paulk.fr>
Gerrit-Reviewer: Julius Werner <jwerner at chromium.org>
Gerrit-Reviewer: Martin Roth <martinroth at google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi at google.com>
Gerrit-Reviewer: Paul Kocialkowski <contact at paulk.fr>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: No