Attention is currently required from: Alan Green, Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, Samir Ibradžić. Simon Buhrow has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 )
Change subject: ft2232_spi.c: Implement spi_send_multicommand() ......................................................................
Patch Set 30:
(6 comments)
File ft2232_spi.c:
https://review.coreboot.org/c/flashrom/+/40477/comment/fbc132c6_cd7eafcc PS25, Line 317: if (!cmds->readcnt && ((cmds + 1)->writecnt || (cmds + 1)->readcnt)){
Checking if next command fits is a very good idea! I did as you proposed. […]
Done
File ft2232_spi.c:
https://review.coreboot.org/c/flashrom/+/40477/comment/024d0c77_50e16669 PS29, Line 171: return 12 + cmd->writecnt + cmd->readcnt <= buffer_size;
I just realized we could make the check more accurate, taking into account […]
I think it is a cleaner solution and, theoretically, it could save some send_buf() calls.
https://review.coreboot.org/c/flashrom/+/40477/comment/dc06cb71_b4454fd0 PS29, Line 306: /* Overflow check for buf : * 12 : up to 12 CMD-Bytes are added below : */
I think we can drop the comment here, the function name should be self-explanatory.
Done
https://review.coreboot.org/c/flashrom/+/40477/comment/6b61986f_4e76f3ee PS29, Line 344: ((cmds + 1)->writecnt || (cmds + 1)->readcnt) && : ft2232_spi_command_fits((cmds + 1), FTDI_HW_BUFFER_SIZE - i)) {
Please indent either with 4 spaces or two tabs (relative to the `if`). […]
Done
https://review.coreboot.org/c/flashrom/+/40477/comment/a2837648_91c616e5 PS29, Line 352: msg_perr("send_buf failed: %i\n", ret); : break;
Needs braces.
Doing too much python ;-)
https://review.coreboot.org/c/flashrom/+/40477/comment/a914ff65_d5842eab PS29, Line 358: msg_perr("get_buf failed: %i\n", ret); : break;
Same here.
Done