Attention is currently required from: Julius Werner, Eric Peers, Felix Held. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56230 )
Change subject: lib/thread: Add mutex ......................................................................
Patch Set 6:
(4 comments)
File src/include/thread.h:
https://review.coreboot.org/c/coreboot/+/56230/comment/7d856fce_190386ea PS5, Line 88: mutex_lock
nit: I think it's better to keep everything here prefixed thread_ (e.g. […]
Done
File src/lib/thread.c:
https://review.coreboot.org/c/coreboot/+/56230/comment/e220c9ac_9038adf3 PS5, Line 391: thread_yield_microseconds(0);
Should assert that this didn't fail (e.g. because can_yield is 0, because then you'll wait forever).
Good point.
https://review.coreboot.org/c/coreboot/+/56230/comment/a7fc040d_ecb1d6fc PS5, Line 392: 1
Please only use `true` and `false` with bools, and only numbers with integers.
Good catch.
https://review.coreboot.org/c/coreboot/+/56230/comment/5eddd6a7_4e26dc94 PS5, Line 394: printk(BIOS_DEBUG, " took %lu us to acquire mutex\n", stopwatch_duration_usecs(&sw)); Made it SPEW. I found it useful to figure out how long things were blocked. It is usually a sign that nothing has yielded in a while:
APOB thread running spi_dma_readat_dma: start: dest: 0xcb7aa640, source: 0x0, size: 65536 took 0 us to acquire mutex
took 209446 us to acquire mutex start_spi_dma_transaction: dest: 0x021c0000, source: 0x7f540, remaining: 174994
As for the space, I've removed it.
-
Raul Rangel (Code Review)