Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32725 )
Change subject: Replace ENV_RAMSTAGE with ENV_PAYLOAD_LOADER ......................................................................
Patch Set 2:
(9 comments)
Is there no need to update these:
https://review.coreboot.org/cgit/coreboot.git/tree/src/arch/x86/cbmem.c?id=r...
https://review.coreboot.org/cgit/coreboot.git/tree/src/drivers/elog/elog.c?i...
https://review.coreboot.org/#/c/32725/2/src/arch/x86/include/arch/acpi.h File src/arch/x86/include/arch/acpi.h:
https://review.coreboot.org/#/c/32725/2/src/arch/x86/include/arch/acpi.h@841 PS2, Line 841: #if ENV_PAYLOAD_LOADER && !defined(__SIMPLE_DEVICE__) I don't think there is a need to guard the declarations here.
https://review.coreboot.org/#/c/32725/2/src/include/rules.h File src/include/rules.h:
https://review.coreboot.org/#/c/32725/2/src/include/rules.h@265 PS2, Line 265: post-CAR always build with simple device model This should be updated to reflect the change being done.
https://review.coreboot.org/#/c/32725/2/src/include/rules.h@267 PS2, Line 267: Currently there's : * no known requirement that devicetree would be needed during that stage. This is not accurate anymore.
https://review.coreboot.org/#/c/32725/2/src/include/rules.h@275 PS2, Line 275: defined(__PRE_RAM__) || ENV_SMM Aren't checks for __PRE_RAM__ and ENV_SMM redundant now since !ENV_PAYLOAD_LOADER would always be true for those stages?
https://review.coreboot.org/#/c/32725/2/src/lib/bootmode.c File src/lib/bootmode.c:
https://review.coreboot.org/#/c/32725/2/src/lib/bootmode.c@20 PS2, Line 20: #if ENV_PAYLOAD_LOADER Same here. It should be possible to avoid this and just let the linker get rid of it for stages that don't use it.
https://review.coreboot.org/#/c/32725/2/src/lib/timestamp.c File src/lib/timestamp.c:
https://review.coreboot.org/#/c/32725/2/src/lib/timestamp.c@50 PS2, Line 50: ramstage Need to update the comment.
https://review.coreboot.org/#/c/32725/2/src/lib/timestamp.c@55 PS2, Line 55: ramstage this too.
https://review.coreboot.org/#/c/32725/2/src/lib/timestamp.c@226 PS2, Line 226: ENV_RAMSTAGE Shouldn't this be updated as well?
https://review.coreboot.org/#/c/32725/2/src/lib/timestamp.c@305 PS2, Line 305: ramstage Update comment too.