From 048bbe0323793e382365815e499f4be0e8f05cf3 Mon Sep 17 00:00:00 2001 From: Anastasia Klimchuk Date: Thu, 6 May 2021 07:47:04 +1000 Subject: 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 Reviewed-on: https://review.coreboot.org/c/flashrom/+/52958 Reviewed-by: Edward O'Callaghan Reviewed-by: Nico Huber Tested-by: build bot (Jenkins) --- buspirate_spi.c | 61 +++++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 19 deletions(-) diff --git a/buspirate_spi.c b/buspirate_spi.c index 3d8193557..8c84261d5 100644 --- a/buspirate_spi.c +++ b/buspirate_spi.c @@ -50,26 +50,28 @@ static int buspirate_serialport_setup(char *dev) #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 const struct buspirate_speeds serialspeeds[] = { 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 @@ out_shutdown: /* 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 @@ static int buspirate_spi_send_command_v1(const struct flashctx *flash, unsigned { 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 @@ static int buspirate_spi_send_command_v2(const struct flashctx *flash, unsigned 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 @@ static int buspirate_spi_send_command_v2(const struct flashctx *flash, unsigned /* 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 @@ static int buspirate_spi_init(void) 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 @@ static int buspirate_spi_init(void) #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 @@ static int buspirate_spi_init(void) 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 @@ static int buspirate_spi_init(void) 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 @@ static int buspirate_spi_init(void) 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 @@ static int buspirate_spi_init(void) 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; } -- cgit v1.2.3