Simon Buhrow 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 to save programming time ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/flashrom/+/40477/2/ft2232_spi.c File ft2232_spi.c:
https://review.coreboot.org/c/flashrom/+/40477/2/ft2232_spi.c@549 PS2, Line 549: return 0;
I missed that buf is static, so this will work out fine.
Well, in the usual use case with maximum buf size equal to normal page program operations, I agree.
But if writecnt gets bigger than page program (plus cmd and addr bytes), realloc is called again. And then the WREN which was saved at the old buf memory location will get lost, right? (although buf is static)
I think at the moment there does not happen bigger writecnt than 256Byte page program?! Nevertheless it might be better to make this somewhat cleaner. I see the following options: A) to catch bigger writecnt in line 477 OR B) to filter them in line 481 OR C) to set bufsize fix to 4096. This is the size of the FIFO buffer of the ftdi2232 [1]. So this is the biggest size which ever can be send to the ftdi chip. Sure, this would lead to some overhead in current case of use only page program size, but it might be OK and lead to a simpler code.
I would prefer option C) and add it to this commit. Or are there any other preferences?
[1] https://www.ftdichip.com/Support/Documents/DataSheets/ICs/DS_FT2232H.pdf -> chapter 4.2