Simon Glass has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31887 )
Change subject: vboot: make vboot workbuf available to payload ......................................................................
Patch Set 10:
(4 comments)
https://review.coreboot.org/#/c/31887/9/payloads/libpayload/include/coreboot... File payloads/libpayload/include/coreboot_tables.h:
https://review.coreboot.org/#/c/31887/9/payloads/libpayload/include/coreboot... PS9, Line 205: 0x0034
Doesn't really seem to be in order of memory layout... […]
Ack
https://review.coreboot.org/#/c/31887/9/payloads/libpayload/include/sysinfo.... File payloads/libpayload/include/sysinfo.h:
https://review.coreboot.org/#/c/31887/9/payloads/libpayload/include/sysinfo.... PS9, Line 100: void *vboot_workbuf;
As it writing a comment for each member? Doesn't look like it... […]
As you say, it doesn't look like it, so don't worry.
https://review.coreboot.org/#/c/31887/9/payloads/libpayload/libc/coreboot.c File payloads/libpayload/libc/coreboot.c:
https://review.coreboot.org/#/c/31887/9/payloads/libpayload/libc/coreboot.c@... PS9, Line 91: struct
If it makes sense to use const here, perhaps we can do that for all functions in coreboot. […]
I think const should be included whenever possible :-)
https://review.coreboot.org/#/c/31887/9/src/lib/coreboot_table.c File src/lib/coreboot_table.c:
https://review.coreboot.org/#/c/31887/9/src/lib/coreboot_table.c@549 PS9, Line 549: CONFIG(VBOOT)
Yeah, seems like both Aaron and Julius are in heavy favour of using C conditionals wherever possible […]
Yes if() is better than #ifdef for several reasons.
For search, if I want to find all uses of CONFIG_VBOOT it's a bit hard. If we consistently use CONFIG(VBOOT) (with if() or #if) then I suppose it is easier. But just searching for VBOOT is not that useful.