Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36208 )
Change subject: security/vboot: Add vboot callbacks to support EC software sync ......................................................................
Patch Set 8:
(7 comments)
https://review.coreboot.org/c/coreboot/+/36208/3/src/security/vboot/ec_sync.... File src/security/vboot/ec_sync.c:
https://review.coreboot.org/c/coreboot/+/36208/3/src/security/vboot/ec_sync.... PS3, Line 136: break;
-Wimplicit-fallthrough is enabled, I'll just add the mdelay here, too.
What that means is that you have to put a comment like /* fall-through */ there, it doesn't mean you're not allowed to do fall-throughs at all.
https://review.coreboot.org/c/coreboot/+/36208/8/src/security/vboot/ec_sync.... File src/security/vboot/ec_sync.c:
https://review.coreboot.org/c/coreboot/+/36208/8/src/security/vboot/ec_sync.... PS8, Line 14: #include <2api.h> vb2_api.h
https://review.coreboot.org/c/coreboot/+/36208/8/src/security/vboot/ec_sync.... PS8, Line 37: LIMIT_POWER_WAIT_TIMEOUT Please suffix this with a unit (e.g. _MS).
https://review.coreboot.org/c/coreboot/+/36208/8/src/security/vboot/ec_sync.... PS8, Line 39: #define LIMIT_POWER_POLL_SLEEP 10 This too
https://review.coreboot.org/c/coreboot/+/36208/8/src/security/vboot/ec_sync.... PS8, Line 236: MIN(1024 I think we may need to move this into google_chromeec_flash_write_burst_size(), because we still need to write a whole write block at a time. (Also, it currently doesn't take the struct ec_params_flash_write size into account, which I think it should.)
Maybe you'll need to move the whole burst size function into this file (and make it static) so the compiler can inline it and guarantee statically that the stack size is capped.
https://review.coreboot.org/c/coreboot/+/36208/8/src/security/vboot/ec_sync.... PS8, Line 398: "flag.\n"); No need to break?
https://review.coreboot.org/c/coreboot/+/36208/8/src/security/vboot/ec_sync.... PS8, Line 416: limit_power_wait_time += LIMIT_POWER_POLL_SLEEP; nit: Would be better to solve this with the stopwatch API