Attention is currently required from: Raul Rangel, Aaron Durbin, Furquan Shaikh, Marshall Dawson, Julius Werner. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56048 )
Change subject: lib/future: Implement Futures API ......................................................................
Patch Set 2:
(5 comments)
File src/lib/Kconfig:
https://review.coreboot.org/c/coreboot/+/56048/comment/ac95dea3_4469fbae PS2, Line 103: bool Add a prompt so it can be disabled for debug?
File src/lib/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/56048/comment/fa25aa74_3707234e PS2, Line 274: -y Make dependent on the config option being enabled?
File src/lib/future.c:
https://review.coreboot.org/c/coreboot/+/56048/comment/49b4facb_abb65014 PS2, Line 34: die Maybe assert? This shouldn't be a die in my opinion - it should be the same as not having the config enabled.
Changing to an assert would allow it to halt if fatal asserts is enabled, but would continue otherwise. Presumably if futures are disabled in config, these routines would happen at some point in the code flow anyway, so if a future failed to be registered, it might impact performance slightly, but shouldn't harm anything, right?
https://review.coreboot.org/c/coreboot/+/56048/comment/ceed4a85_ba6c274f PS2, Line 54: die Similar to the last die - Maybe assert and return?
https://review.coreboot.org/c/coreboot/+/56048/comment/06ab437d_48968f8f PS2, Line 68: state = futures[i]->poll(futures[i], false); For debug, maybe add an optional print before and after and include the amount of time it took?