Joel Kitching has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36210 )
Change subject: security/vboot: Add new Kconfig option VBOOT_EARLY_EC_SYNC ......................................................................
Patch Set 2:
(5 comments)
https://review.coreboot.org/c/coreboot/+/36210/2/src/security/vboot/Kconfig File src/security/vboot/Kconfig:
https://review.coreboot.org/c/coreboot/+/36210/2/src/security/vboot/Kconfig@... PS2, Line 232: primary EC I think we want to stop referring to the EC as "primary" or "secondary" and just say EC. Going forward, there should only ever be one.
https://review.coreboot.org/c/coreboot/+/36210/2/src/security/vboot/Kconfig@... PS2, Line 232: When selected, this will perform Maybe just "Enables EC software sync..."
https://review.coreboot.org/c/coreboot/+/36210/2/src/security/vboot/Kconfig@... PS2, Line 233: . May want to add some more explanation as to why this option might be useful (i.e. perform USB-PD power negotiation earlier on). Also add any kind of drawbacks (may add some extra boot time? I'm not sure about this).
https://review.coreboot.org/c/coreboot/+/36210/2/src/security/vboot/Makefile... File src/security/vboot/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/36210/2/src/security/vboot/Makefile... PS2, Line 40: sync_ec.c Might want to call this file ec_sync.c to be more consistent with vboot's naming...
https://review.coreboot.org/c/coreboot/+/36210/2/src/security/vboot/Makefile... PS2, Line 134: CFLAGS_common += -I3rdparty/vboot/firmware/2lib/include : CFLAGS_common += -I3rdparty/vboot/firmware/lib/include Rather than adding vboot1 dependencies in coreboot (which I already worked really hard to get rid of!), can we work to migrate firmware/lib/ec_sync.c to firmware/2lib/2ec_sync.c and firmware/2lib/2auxfw_sync.c as I mentioned?