Nico Huber submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Edward O'Callaghan: Looks good to me, but someone else must approve
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(-)

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;
}


To view, visit change 52958. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I418bbfff15fb126b042fbc9be09dbf59f4d243b8
Gerrit-Change-Number: 52958
Gerrit-PatchSet: 6
Gerrit-Owner: Anastasia Klimchuk <aklm@chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter@mailbox.org>
Gerrit-MessageType: merged