Attention is currently required from: Simon Buhrow, Alan Green, Edward O'Callaghan, Anastasia Klimchuk, Samir Ibradžić. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 )
Change subject: ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call ......................................................................
Patch Set 25:
(1 comment)
File ft2232_spi.c:
https://review.coreboot.org/c/flashrom/+/40477/comment/c39cfe7e_a4ce3711 PS25, Line 295: 9
I counted 15 (lines 303, 309, 320, 328, 335). […]
I see 6 potential paths, due to the 3 if's but the second may end the path prematurely with the `continue;`:
* false, false, false: 303, 335 (empty command, actually impossible due to the loop condition) * false, false, true: 303, 328, 335 (read) * false, true, *: 303, 320 (empty command with continuation, impossible due to the loop condition) * true, false, false: 303, 309, 335 (write without continuation) * true, false, true: 303, 309, 328, 335 (write+read) * true, true, *: 303, 309, 320 (write with continuation)
Turns out there's actually only 4 paths :) The simple write+read wins with 4 * 3 == 12 (assert CS#, write, read, de-assert CS#). Overhead for any continuation would be accounted for in the next iteration.
Now that it's decoded (admittedly, it doesn't look like it), I think it could get more obvious if the continuation check would be moved after the regular CS# de-assertion. i.e.
assert cs# if (write) write command if (read) read command de-assert cs# if (!read && possible continuation) continue send_buf()
Simon, what do you think? That should make it clear that we always do
* assert - foo - de-assert, * eventually either continue with the next iteration or call send_buf().