Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52958 )
Change subject: buspirate_spi.c: Refactor singleton states into reentrant pattern ......................................................................
buspirate_spi.c: Refactor singleton states into reentrant pattern
Move global singleton states into a struct and store within the spi_master data field for the life-time of the driver.
This is one of the steps on the way to move spi_master data memory management behind the initialisation API, for more context see other patches under the same topic "register_master_api".
BUG=b:185191942 TEST=builds
Change-Id: I418bbfff15fb126b042fbc9be09dbf59f4d243b8 Signed-off-by: Anastasia Klimchuk aklm@chromium.org Reviewed-on: https://review.coreboot.org/c/flashrom/+/52958 Reviewed-by: Edward O'Callaghan quasisec@chromium.org Reviewed-by: Nico Huber nico.h@gmx.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M buspirate_spi.c 1 file changed, 42 insertions(+), 19 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Edward O'Callaghan: Looks good to me, but someone else must approve
diff --git a/buspirate_spi.c b/buspirate_spi.c index 3d81935..8c84261 100644 --- a/buspirate_spi.c +++ b/buspirate_spi.c @@ -50,26 +50,28 @@ #define sp_flush_incoming(...) 0 #endif
-static unsigned char *bp_commbuf = NULL; -static int bp_commbufsize = 0; +struct bp_spi_data { + unsigned char *bp_commbuf; + int bp_commbufsize; +};
-static int buspirate_commbuf_grow(int bufsize) +static int buspirate_commbuf_grow(int bufsize, unsigned char **bp_commbuf, int *bp_commbufsize) { unsigned char *tmpbuf;
/* Never shrink. realloc() calls are expensive. */ - if (bufsize <= bp_commbufsize) + if (bufsize <= *bp_commbufsize) return 0;
- tmpbuf = realloc(bp_commbuf, bufsize); + tmpbuf = realloc(*bp_commbuf, bufsize); if (!tmpbuf) { /* Keep the existing buffer because memory is already tight. */ msg_perr("Out of memory!\n"); return ERROR_OOM; }
- bp_commbuf = tmpbuf; - bp_commbufsize = bufsize; + *bp_commbuf = tmpbuf; + *bp_commbufsize = bufsize; return 0; }
@@ -162,6 +164,8 @@
static int buspirate_spi_shutdown(void *data) { + struct bp_spi_data *bp_data = data; + unsigned char *const bp_commbuf = bp_data->bp_commbuf; int ret = 0, ret2 = 0; /* No need to allocate a buffer here, we know that bp_commbuf is at least DEFAULT_BUFSIZE big. */
@@ -189,14 +193,14 @@ /* Keep the oldest error, it is probably the best indicator. */ if (ret2 && !ret) ret = ret2; - bp_commbufsize = 0; + free(bp_commbuf); - bp_commbuf = NULL; if (ret) msg_pdbg("Bus Pirate shutdown failed.\n"); else msg_pdbg("Bus Pirate shutdown completed.\n");
+ free(data); return ret; }
@@ -205,14 +209,17 @@ { unsigned int i = 0; int ret = 0; + struct bp_spi_data *bp_data = flash->mst->spi.data;
if (writecnt > 16 || readcnt > 16 || (readcnt + writecnt) > 16) return SPI_INVALID_LENGTH;
/* 3 bytes extra for CS#, len, CS#. */ - if (buspirate_commbuf_grow(writecnt + readcnt + 3)) + if (buspirate_commbuf_grow(writecnt + readcnt + 3, &bp_data->bp_commbuf, &bp_data->bp_commbufsize)) return ERROR_OOM;
+ unsigned char *const bp_commbuf = bp_data->bp_commbuf; + /* Assert CS# */ bp_commbuf[i++] = 0x02;
@@ -257,6 +264,7 @@ const unsigned char *writearr, unsigned char *readarr) { int i = 0, ret = 0; + struct bp_spi_data *bp_data = flash->mst->spi.data;
if (writecnt > 4096 || readcnt > 4096 || (readcnt + writecnt) > 4096) return SPI_INVALID_LENGTH; @@ -264,9 +272,11 @@ /* 5 bytes extra for command, writelen, readlen. * 1 byte extra for Ack/Nack. */ - if (buspirate_commbuf_grow(max(writecnt + 5, readcnt + 1))) + if (buspirate_commbuf_grow(max(writecnt + 5, readcnt + 1), &bp_data->bp_commbuf, &bp_data->bp_commbufsize)) return ERROR_OOM;
+ unsigned char *const bp_commbuf = bp_data->bp_commbuf; + /* Combined SPI write/read. */ bp_commbuf[i++] = 0x04; bp_commbuf[i++] = (writecnt >> 8) & 0xff; @@ -316,6 +326,8 @@ int serialspeed_index = -1; int ret = 0; int pullup = 0; + unsigned char *bp_commbuf; + int bp_commbufsize;
dev = extract_programmer_param("dev"); if (dev && !strlen(dev)) { @@ -369,7 +381,6 @@ #define DEFAULT_BUFSIZE (16 + 3) bp_commbuf = malloc(DEFAULT_BUFSIZE); if (!bp_commbuf) { - bp_commbufsize = 0; msg_perr("Out of memory!\n"); free(dev); return ERROR_OOM; @@ -379,12 +390,20 @@ ret = buspirate_serialport_setup(dev); free(dev); if (ret) { - bp_commbufsize = 0; free(bp_commbuf); - bp_commbuf = NULL; return ret; }
+ + struct bp_spi_data *bp_data = calloc(1, sizeof(*bp_data)); + if (!bp_data) { + msg_perr("Unable to allocate space for SPI master data\n"); + free(bp_commbuf); + return 1; + } + bp_data->bp_commbuf = bp_commbuf; + bp_data->bp_commbufsize = bp_commbufsize; + /* This is the brute force version, but it should work. * It is likely to fail if a previous flashrom run was aborted during a write with the new SPI commands * in firmware v5.5 because that firmware may wait for up to 4096 bytes of input before responding to @@ -481,10 +500,12 @@ if (BP_FWVERSION(fw_version_major, fw_version_minor) >= BP_FWVERSION(5, 5)) { msg_pdbg("Using SPI command set v2.\n"); /* Sensible default buffer size. */ - if (buspirate_commbuf_grow(260 + 5)) { + if (buspirate_commbuf_grow(260 + 5, &bp_commbuf, &bp_commbufsize)) { ret = ERROR_OOM; goto init_err_cleanup_exit; } + bp_data->bp_commbuf = bp_commbuf; + bp_data->bp_commbufsize = bp_commbufsize; spi_master_buspirate.max_data_read = 2048; spi_master_buspirate.max_data_write = 256; spi_master_buspirate.command = buspirate_spi_send_command_v2; @@ -493,10 +514,12 @@ msg_pinfo("Reading/writing a flash chip may take hours.\n"); msg_pinfo("It is recommended to upgrade to firmware 5.5 or newer.\n"); /* Sensible default buffer size. */ - if (buspirate_commbuf_grow(16 + 3)) { + if (buspirate_commbuf_grow(16 + 3, &bp_commbuf, &bp_commbufsize)) { ret = ERROR_OOM; goto init_err_cleanup_exit; } + bp_data->bp_commbuf = bp_commbuf; + bp_data->bp_commbufsize = bp_commbufsize; spi_master_buspirate.max_data_read = 12; spi_master_buspirate.max_data_write = 12; spi_master_buspirate.command = buspirate_spi_send_command_v1; @@ -669,16 +692,16 @@ goto init_err_cleanup_exit; }
- if (register_shutdown(buspirate_spi_shutdown, NULL) != 0) { + if (register_shutdown(buspirate_spi_shutdown, bp_data) != 0) { ret = 1; goto init_err_cleanup_exit; } - register_spi_master(&spi_master_buspirate, NULL); + register_spi_master(&spi_master_buspirate, bp_data);
return 0;
init_err_cleanup_exit: - buspirate_spi_shutdown(NULL); + buspirate_spi_shutdown(bp_data); return ret; }