Attention is currently required from: Aaron Durbin, Furquan Shaikh, Marshall Dawson, Angel Pons, Julius Werner, Eric Peers. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56047 )
Change subject: commonlib/bsd/future: Introduce future API ......................................................................
Patch Set 2:
(7 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/56047/comment/72569df3_f2e5ad4d PS1, Line 32: protected. Auditing this is complicated anid error prone.
nit. […]
Done
File src/commonlib/bsd/include/commonlib/bsd/future.h:
https://review.coreboot.org/c/coreboot/+/56047/comment/d3d14d01_2133512e PS1, Line 1: /* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0-or-later */
Why not?
Since BSD is more permissive, it's able to be used in more code bases. I'm fine making it GPL only too. No real preference.
https://review.coreboot.org/c/coreboot/+/56047/comment/3df074fc_e46e7576 PS1, Line 37: * periodically poll until the second transaction completes. Once the
chaining here to start a decompress would also be useful. "AndThenFut". […]
We don't need an `AndThen` future. If a future has dependencies, it is responsible storing their futures and polling them. See the CBFS API for an example.
https://review.coreboot.org/c/coreboot/+/56047/comment/90633f29_5c5632f1 PS1, Line 40: * By only polling periodically, we allow the CPU is to continue
nit "is to"
Done
https://review.coreboot.org/c/coreboot/+/56047/comment/4106a503_970a01c1 PS1, Line 59: !!
nit: redundant, return value already does this conversion
Does the fact that this returns a bool cause it to only return 0 or 1? I would assume it would return the truncated pointer.
https://review.coreboot.org/c/coreboot/+/56047/comment/55648bc9_0e0044d7 PS1, Line 68: * those dependents should be polled by the containing future and not the
not quite following this. […]
Right, the future is no longer polled by the executor once it completes. So a parent future can only complete once all its child futures have completed.
https://review.coreboot.org/c/coreboot/+/56047/comment/ce2bdcf2_2caa8c6b PS1, Line 83:
nit: one space too many
Done
-
Raul Rangel (Code Review)