Attention is currently required from: Hung-Te Lin, Nico Huber, Subrata Banik. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62213 )
Change subject: flashrom.c: Implement file-based locking semantics ......................................................................
Patch Set 1:
(5 comments)
File big_lock.c:
https://review.coreboot.org/c/flashrom/+/62213/comment/8d54b14c_fa792ee1 PS1, Line 39: return acquire_lock(&big_lock, timeout_secs * 1000);
should we check #if BIG_LOCK and decide if lock is required?
Seems unnecessary complexity to make it conditional since we would always want it to be the case that two flashrom instances never run concurrently?
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/62213/comment/dc7b4f24_14b85557 PS1, Line 51: #ifndef USE_BIG_LOCK : #define USE_BIG_LOCK 0 : #endif
Not really used?
Done
https://review.coreboot.org/c/flashrom/+/62213/comment/587389ae_175cce24 PS1, Line 142: programmer_entry
Is it possible to add an attribute to programmer_entry so we can know if big_lock is required? Then […]
afaik, this is issue is specifically related to futility tests not yet running under platform2_wrapper so that is just a tmp workaround until we fix that. Therefore upstream shouldn't be burden with the issue that is our own. WDYT?
https://review.coreboot.org/c/flashrom/+/62213/comment/7b97253e_de6983ed PS1, Line 151: /* Get big lock before doing any work that touches hardware. */ : msg_gdbg("Acquiring lock (timeout=%d sec)...\n", LOCK_TIMEOUT_SECS); : if (acquire_big_lock(LOCK_TIMEOUT_SECS) < 0) { : msg_gerr("Could not acquire lock.\n"); : return -1; : } : big_lock_acquired = true; : msg_gdbg("Lock acquired.\n"); :
If we're going to have a big_lock. […]
Sounds good, Done.
https://review.coreboot.org/c/flashrom/+/62213/comment/77b6a8f3_a57b7df2 PS1, Line 217: if (big_lock_acquired) { : release_big_lock(); : big_lock_acquired = false; : }
we should just call release_big_lock() and let it check/handle big_lock_acquired internally
Done