Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk.
Alexander Goncharov 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 6:
(6 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/66373/comment/355901bb_49fe0de3 PS4, Line 10: This is an important step to : remove the global state because it'll allow the programmer's data : to be used in delay functions.
Maybe just drop part of this sentence after 'because' as its not what this patch does specifically f […]
Sure!
https://review.coreboot.org/c/flashrom/+/66373/comment/3ebfc4cf_9d3e14e8 PS4, Line 13: : Programmers that use internal delay can provide NULL as a context.
Move this up to the last paragraph.
Done
https://review.coreboot.org/c/flashrom/+/66373/comment/fc2e439b_d1df8cba PS4, Line 16: This is one of the steps on the way to move master's data : memory management behind the initialisation API, for more : context see other patches under the same topic specified below.
Drop this paragraph.
Done
Patchset:
PS4:
Can we rebase this one and have it go in? […]
Done
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/66373/comment/2e4ad5f5_482cef5a PS5, Line 220: void programmer_delay(const struct flashctx *flash, unsigned int usecs) Is it OK that I've refactored this function in this patch? Because otherwise there would be lots of nested if-statements.
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/66373/comment/489874f0_f5fb7360 PS6, Line 226: return internal_delay(flash, usecs); Since we don't use `internal_delay` as a callback, we may not provide `flashctx`. It will be more meaningful, just `internal_delay(usecs)`.
What do you think?
-
Alexander Goncharov (Code Review)