Change in flashrom[master]: ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call

[Nico Huber (Code Review)]

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 16:

(5 comments)
> hm... anyone having time to review/merge this? I´d like to get this done...

Sorry to let you know after you went through many cycles already,
but I think this needs a re-write :-/

There is already everything in place in flashrom to avoid such
a layering violation, namely the `.multicommand` member of struct
spi_master. The biggest problem with your current implementation
is that you assume that there is always a JEDEC flash chip, but
that's not generally true.

So, the correct way to do this is to implement `.multicommand`
instead of `.command`. I would roughly do it as follows:

Loop
        Put write part of command into buffer.
        If not last command and read part is empty,
            Continue with next loop iteration.
        Execute buffer.
        Execute read part if any.
    While not last command.

https://review.coreboot.org/c/flashrom/+/40477/16//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/flashrom/+/40477/16//COMMIT_MSG@25 
PS16, Line 25: in a similar way.
The infrastructure is already in place. You just have to implement
`.multicommand` instead of `.command` for the programmer.

https://review.coreboot.org/c/flashrom/+/40477/16/ft2232_spi.c 
File ft2232_spi.c:

https://review.coreboot.org/c/flashrom/+/40477/16/ft2232_spi.c@172 
PS16, Line 172: 	.multicommand	= default_spi_send_multicommand,
You just need to implement this and will get both commands passed at once.

https://review.coreboot.org/c/flashrom/+/40477/16/ft2232_spi.c@497 
PS16, Line 497: 	bufsize = max(writecnt + 19, 261 + 19);
This assumes `i` is at most 10. But there is really no guarantee because
we could have been called with WREN multiple times (by accident). Easy
to fix though, replace the assumed 10 in `writecnt + 19` with `i`:
`i + writecnt + 9`.

Btw. this `max(..., 261 + 19)` optimization seems insignificant in the
presence of USB transfers (which probably have 100 times more overhead
than realloc()). I would probably drop it.

https://review.coreboot.org/c/flashrom/+/40477/16/ft2232_spi.c@562 
PS16, Line 562: 	if (writearr[0] == JEDEC_WREN) {
What if the chip isn't JEDEC? 0x06 (WREN) could mean something very different.

This is pretty much a blocker. Especially because the proper infrastructure
(.multicommand) is already in place.

https://review.coreboot.org/c/flashrom/+/40477/16/ft2232_spi.c@568 
PS16, Line 568: 	} else {
To "quote" checkpatch from memory: `else` is not needed after a return.