Change in flashrom[master]: digilent_spi: add a driver for the iCEblink40 development board

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/23338 )

Change subject: digilent_spi: add a driver for the iCEblink40 development board
......................................................................

Patch Set 4:

(5 comments)

I'm pretty much confused by the padding, did you try what it replies
without a padded write?

https://review.coreboot.org/#/c/23338/4/digilent_spi.c
File digilent_spi.c:

https://review.coreboot.org/#/c/23338/4/digilent_spi.c@221
PS4, Line 221: 	uint8_t res[read_follows ? 10 : 6];
Looking at iceBurn, the meaning of the bytes seems known? 2-byte
status, received write length, returned read length. Why not check
them?

https://review.coreboot.org/#/c/23338/4/digilent_spi.c@230
PS4, Line 230: 	int len = writecnt + readcnt;
This seems flawed or at least confuses me too much. If I read iceBurn
correctly, it pads up to `readcnt` so the length of the write is `MAX(
writecnt, readcnt)` but not `writecnt + readcnt`. The length of the
read in iceBurn seems to always be `readcnt` without anything to drop
from the read data.

https://review.coreboot.org/#/c/23338/4/digilent_spi.c@236
PS4, Line 236: 	memset(buf + writecnt, 0xff, readcnt);
iceBurn pads with zeros, aiui. And not by `readcnt` bytes but only
up to a total length of `readcnt`.

https://review.coreboot.org/#/c/23338/4/digilent_spi.c@246
PS4, Line 246: 	ret = libusb_bulk_transfer(handle, DATA_WRITE_EP, buf, len, &tx_len, USB_TIMEOUT);
Maybe we should check `tx_len` as well.

https://review.coreboot.org/#/c/23338/4/digilent_spi.c@276
PS4, Line 276: 	.max_data_read	= 64,
             : 	.max_data_write	= 64,
These are about the payload size, i.e. data actually read / written
from / to the flash storage. iceBurn seems to use 64 as a limit for
the bulk transfers, though, which will include the SPI command
and address. So to limit the bulk transfer to 64 bytes, we should
set 59 here, I guess. OTOH, if you'd implement a loop around the
64 byte bulk transfers like iceBurn, we wouldn't need a limit here
(could speed things a little up due to less protocol overhead).