Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk, Alexander Goncharov.
Felix Singer 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:
(11 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/66373/comment/d3c54984_e2a12db3 PS7, Line 7: tree: provide flashrom context into programmer's delay It's not passed to the programmers anymore
https://review.coreboot.org/c/flashrom/+/66373/comment/59ed5ac8_f6f8543f PS7, Line 9: Add the flashrom context to the delay function declaration, which : located in `struct programmer_entry`. Doesn't apply anymore. Also, there is not *that* delay function. So mention the name `programmer_delay`.
https://review.coreboot.org/c/flashrom/+/66373/comment/b794405e_f55a9860 PS7, Line 14: TOPIC=register_master_api How is this patch related to this topic?
File amd_imc.c:
https://review.coreboot.org/c/flashrom/+/66373/comment/097acf57_cab11ed9 PS7, Line 63: /* flashctx is not completely ready during init, so it's not : * needed (NULL). */ Same here
File atavia.c:
https://review.coreboot.org/c/flashrom/+/66373/comment/9d979fa2_25b0e610 PS7, Line 93: /* flashctx is not needed (NULL) because atavia does : * not use it in its delay function. */ Same here
File dediprog.c:
https://review.coreboot.org/c/flashrom/+/66373/comment/977c3f41_2e24d75e PS7, Line 319: /* flashctx not needed (NULL) because dediprog does not use it in its : * delay function. */ Same here
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/66373/comment/3918bf2d_ce7644c6 PS5, Line 220: void programmer_delay(const struct flashctx *flash, unsigned int usecs)
The commit message to this patch is hard to have both ways, it is either very forward looking and me […]
My comment is related to an older patch set in which the function was reworked while the commit message just says "pass flashctx to functions", which was wrong.
However, the commit message still doesn't fit to the recent patch set. See my other comments.
File ichspi.c:
https://review.coreboot.org/c/flashrom/+/66373/comment/6e2f4beb_e9d388d1 PS7, Line 878: flashctx is not needed (NULL) because ichspi does not use : * it in its delay function. */ Same here
File pony_spi.c:
https://review.coreboot.org/c/flashrom/+/66373/comment/8969ae7d_7a5b0211 PS7, Line 247: flashctx not needed (NULL) because pony_spi does not : * use it in its delay function. The parameter should be documented at the function declaration, not in the programmers code.
https://review.coreboot.org/c/flashrom/+/66373/comment/23c1da22_109faf12 PS7, Line 248: Also flashctx is not : * completely ready during init anyway. This doesn't add much value. So please remove.
File raiden_debug_spi.c:
https://review.coreboot.org/c/flashrom/+/66373/comment/ea070601_95c81183 PS7, Line 1015: * flashctx not needed (NULL) because raiden_debug_spi does not : * use it in its delay function. Also flashctx is not : * completely ready during init anyway. */ Same here