Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32262 )
Change subject: vboot: refactor OPROM code ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/#/c/32262/3/src/security/vboot/common.c File src/security/vboot/common.c:
https://review.coreboot.org/#/c/32262/3/src/security/vboot/common.c@125 PS3, Line 125: int vboot_declares_display(void) Sorry, I'm still not really happy with the name "declares display". What's wrong with "display needed" or "display required"? That feels way more natural as to what this is to me. (Or just call it init_display or something that would probably be good enough too.)
Also, I'm not sure if we really need full getter and setter accessors here, we're not in Java 101. ;) I think for a simple flags field, it's okay if code manipulates it directly.
https://review.coreboot.org/#/c/32262/3/src/security/vboot/misc.h File src/security/vboot/misc.h:
https://review.coreboot.org/#/c/32262/3/src/security/vboot/misc.h@37 PS3, Line 37: uint8_t declares_display; Let's make it a uint32_t flags in case we come up with other one-bit stuff we need to track here later. (You could also make it a bit-field for ease of access.)