Attention is currently required from: Simon Buhrow, Edward O'Callaghan, Samir Ibradžić. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 )
Change subject: ft2232_spi.c: Implement spi_send_multicommand() ......................................................................
Patch Set 33:
(1 comment)
Patchset:
PS32:
OK, I understand (I have/had got no idea of VM stuff).
So in case of not using VM, the speed we gain because of less USB communication is consumed by the increased processing time needed to pack the packages?
The processing time is negligible. Just prefix your flashrom command with `time`, you'll see that it actually does nothing but waiting in either case.
If you could provide a total number how long it takes to fully read/erase/write a chip, that could give us a clue (sorry if this was already given and I missed it). I suspect the VM case to be slower in general, so the speed increase of this patch would just be time re-gained that was lost due to the virtuali- zation in the first place. So far my theory.
I expect, but don't know, the individual packets to reach the FTDI with some delay. I have not looked into libftdi deeply enough. Maybe it waits for confirmation that the write reached the device before ftdi_write_data() returns. If this is the case, we'd get a lot syn- chronization overhead. Consider this:
send WREN packet --> send WP packet --> wait for acks <--
vs.
send WREN packet --> wait for ack <-- send WP packet --> wait for ack <--
Ignoring bandwidth, in the second example, it would take 2 round- trip times instead of one. And the round-trip is influenced by the virtualization. It depends a lot how the lower layers are implemen- ted. What flashrom leaves to libftdi.
Feel free to comment if you think this patch is worth to get merged.
I think it's nice work and even if it wouldn't ever increase the speed, it would still be a nice example how to implement spi_send_multicommand 😊 But that it helps your use case without affecting others would be enough reason to get it in anyway.
From my side, I'll just have to find the time for a final good look at it, then it could be merged.