Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/47655 )
Change subject: it85spi.c: Refactor singleton states into reentrant pattern ......................................................................
Patch Set 4:
(10 comments)
https://review.coreboot.org/c/flashrom/+/47655/1/it85spi.c File it85spi.c:
https://review.coreboot.org/c/flashrom/+/47655/1/it85spi.c@a273 PS1, Line 273: static int it85xx_spi_send_command(const struct flashctx *flash, : unsigned int writecnt, unsigned int readcnt, : const unsigned char *writearr, : unsigned char *readarr);
this forward declaration can be deleted if in a patch before this one - you will need to move the it […]
Done
https://review.coreboot.org/c/flashrom/+/47655/1/it85spi.c@a278 PS1, Line 278: static const struct spi_master spi_master_it85xx = { : .max_data_read = 64, : .max_data_write = 64, : .command = it85xx_spi_send_command, : .multicommand = default_spi_send_multicommand, : .read = default_spi_read, : .write_256 = default_spi_write_256, : .write_aai = default_spi_write_aai, : }; :
keep this here.
Done
https://review.coreboot.org/c/flashrom/+/47655/1/it85spi.c@87 PS1, Line 87: static struct spi_master spi_master_it85xx = { : .max_data_read = 64, : .max_data_write = 64, : .command = it85xx_spi_send_command, : .multicommand = default_spi_send_multicommand, : .read = default_spi_read, : .write_256 = default_spi_write_256, : .write_aai = default_spi_write_aai, : };
I moved it as we discussed, it is now in the middle (no need to be on the very top).
Done
https://review.coreboot.org/c/flashrom/+/47655/1/it85spi.c@248 PS1, Line 248: if (!data) { : msg_perr("Out of memory!\n"); : return 1; : }
Done
https://review.coreboot.org/c/flashrom/+/47655/1/it85spi.c@258 PS1, Line 258: if (register_shutdown(it85xx_shutdown, data)) { : free(data); : return 1; : }
move to function eulogy.
Done
https://review.coreboot.org/c/flashrom/+/47655/1/it85spi.c@343 PS1, Line 343: if (!(flash->mst)) { : msg_perr("No data in flash context!\n"); : return 1; : }
I wrote get_data_from_context taking it87spi.c as an inspiration.
Done
https://review.coreboot.org/c/flashrom/+/47655/2/it85spi.c File it85spi.c:
https://review.coreboot.org/c/flashrom/+/47655/2/it85spi.c@76 PS2, Line 76: it85_data
call this `it85spi_data`
Done
https://review.coreboot.org/c/flashrom/+/47655/2/it85spi.c@222 PS2, Line 222: : return 0
you need to `free(data);` before the return since it is a heap allocation.
Done
https://review.coreboot.org/c/flashrom/+/47655/2/it85spi.c@312 PS2, Line 312: msg_perr("Out of memory!\n"); : return 1; : }
Done
https://review.coreboot.org/c/flashrom/+/47655/2/it85spi.c@315 PS2, Line 315: data->it85xx_scratch_rom_reenter = 0;
I know the original code doesn't do it but its probably worth initialising the rest as well.
Done