Attention is currently required from: Felix Singer, Thomas Heijligen, Anastasia Klimchuk, Alexander Goncharov.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66373 )
Change subject: tree: provide flashrom context into programmer's delay ......................................................................
Patch Set 7: Code-Review+1
(3 comments)
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/66373/comment/c1d99fe9_c5be536f PS5, Line 220: void programmer_delay(const struct flashctx *flash, unsigned int usecs)
Currently the commit message doesn't give any hint why this is necessary. […]
The commit message to this patch is hard to have both ways, it is either very forward looking and mention things in the future or largely mention nothing at all.
I am happy with it as-is however if you have something specific in mind could you express the insight here Felix?
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/66373/comment/bddcb7bd_d7b1c8f9 PS6, Line 226: return internal_delay(flash, usecs);
Since we don't use `internal_delay` as a callback, we may not provide `flashctx`. […]
If we put CB:67393 on top of this patch then `programmer_delay()` has been largely dealt with from the prospective of getting things into a re-entrant state of affairs.
The follow on would be to flatten things out a bit and remove `programmer_delay()` from being a callback and have the drivers naively call their respective delay implementations internally or core just call the default implementation however that is less urgent. Overall with your patch and CB:67393 the fire of `programmer_delay()` subsides.
File ichspi.c:
https://review.coreboot.org/c/flashrom/+/66373/comment/f5d833c3_bdb57abf PS5, Line 860: truct flashctx *flas
Is it maybe sufficient to just use `NULL` in the case of ichspi as it is the internal timer anyway.
Ack
-
Edward O'Callaghan (Code Review)