Attention is currently required from: Simon Buhrow, Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, Samir Ibradžić. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 )
Change subject: ft2232_spi.c: Implement spi_send_multicommand() ......................................................................
Patch Set 34: Code-Review+1
(6 comments)
File ft2232_spi.c:
https://review.coreboot.org/c/flashrom/+/40477/comment/f4bd210b_3c545e91 PS34, Line 283: const size_t cmd_len = 3; /* same length for any ft2232 command */
If it is always the same, maybe it can be #define above?
I don't think it's necessary, as the literal value is never used outside of this function. It's implicitly used when filling in the commands: statements that assign a value to `buf[i++]` always appear in groups of three.
https://review.coreboot.org/c/flashrom/+/40477/comment/c73746fa_5c8df5f1 PS34, Line 300: int i = 0, ret = 0; Shouldn't `i` be `unsigned int` or `size_t`?
size_t i = 0; int ret = 0;
https://review.coreboot.org/c/flashrom/+/40477/comment/2af1b50a_dbf15566 PS34, Line 307: 65536 hmmm, max_data_write is 256
https://review.coreboot.org/c/flashrom/+/40477/comment/a93a71b9_df4d9274 PS34, Line 311: Out of memory! `Out of memory!` is usually printed when a malloc() or similar function fails. To avoid confusion, I'd use a different message, e.g. `FTDI command does not fit` (suggestions welcome).
https://review.coreboot.org/c/flashrom/+/40477/comment/051d8f8a_9397ab80 PS34, Line 317: nit: no space between unary operators:
buf[i++] = ~0x08 & spi_data->pinlvl; /* assert CS (3rd) bit only */
https://review.coreboot.org/c/flashrom/+/40477/comment/90cf9385_3ac1509d PS34, Line 349: /* send_buf */ I'd say this comment is redundant, and would remove it