Angel Pons 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 8:
(6 comments)
https://review.coreboot.org/c/flashrom/+/40477/8/ft2232_spi.c File ft2232_spi.c:
https://review.coreboot.org/c/flashrom/+/40477/8/ft2232_spi.c@402 PS8, Line 402: /* 280 Byte = (9 Byte CMD + 1 Byte WREN) + (9 Byte CMD + 1 Byte op) + 4 Byte Addr This comment would need an extra tab and could be tidied up. Maybe use:
/* * 270 bytes = * + 9 B (CMD) * + 1 B (WREN) * + 9 B (CMD) * + 1 B (op) * + 4 B (addr) * + 256 B (page data) * * With op: PageProgram or Erase; CMD: FTDI-Chip commands */
https://review.coreboot.org/c/flashrom/+/40477/8/ft2232_spi.c@471 PS8, Line 471: static unsigned char *buf = NULL; Just make this an array:
static unsigned char buf[FTDI_HW_BUFFER_SIZE];
https://review.coreboot.org/c/flashrom/+/40477/8/ft2232_spi.c@473 PS8, Line 473: static Why is `i` static now?
https://review.coreboot.org/c/flashrom/+/40477/8/ft2232_spi.c@480 PS8, Line 480: ftdi FTDI (and missing a space before the comment closes)
https://review.coreboot.org/c/flashrom/+/40477/8/ft2232_spi.c@481 PS8, Line 481: 4096 Please #define this magic number somewhere
https://review.coreboot.org/c/flashrom/+/40477/8/ft2232_spi.c@542 PS8, Line 542: /* Return to get second op (Program or Erase) without resetting buf nor i*/ Why?