ron minnich has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32667 )
Change subject: {arch, console, drivers, include, lib, soc}: Add new features in postcar ......................................................................
Patch Set 8:
(3 comments)
can you rethink this a bit. First off, consider ENV_PAYLOAD_LOADER as the variable, as it's a bit more indicative of why we're doing this. That can replace every single one of your # ? : instances. Also fix that up in rules.h Also shouldn't the guard be systemagent_early.c be ENV_BOOTBLOCK, since you only call that in the bootblock?
https://review.coreboot.org/#/c/32667/8/src/lib/timestamp.c File src/lib/timestamp.c:
https://review.coreboot.org/#/c/32667/8/src/lib/timestamp.c@50 PS8, Line 50: #if CONFIG(SKIP_RAMSTAGE) I think this is not workable. I think you could have a variable called ENV_PAYLOADLOADER that is more indicative of what we're after, namely, this is the stage that loads the payload, whatever stage it is, and then all this complexity would be much reduced.
https://review.coreboot.org/#/c/32667/8/src/lib/timestamp.c@307 PS8, Line 307: #if CONFIG(SKIP_RAMSTAGE) we could then define ENV_RAMPAYLOAD, maybe, which is really what this is about: a RAMPAYLOAD which skips the RAMSTAGE.
https://review.coreboot.org/#/c/32667/8/src/soc/intel/common/block/systemage... File src/soc/intel/common/block/systemagent/systemagent_early.c:
https://review.coreboot.org/#/c/32667/8/src/soc/intel/common/block/systemage... PS8, Line 27: #if (CONFIG(SKIP_RAMSTAGE) ? (!ENV_RAMSTAGE && !ENV_POSTCAR) : !ENV_RAMSTAGE) so is this run in EVERY stage before ramstage or should it be #if ENV_BOOTBLOCK?