Change in flashrom[master]: wbsio_spi.c: Move singleton state into spi master state tracker

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/47854 )

Change subject: wbsio_spi.c: Move singleton state into spi master state tracker
......................................................................

wbsio_spi.c: Move singleton state into spi master state tracker

Make use of the reneterent framework by moving singleton static
state out of the global life-time and into a per-spi_master basis.
This allows for the it87spi master to be reneterent and its internal
state's life-time to be correctly handled by Flashrom's core dispatch
logic.

BUG=none
TEST=builds

Change-Id: Ic97fa41daf26f27b68ced11ddc2a4da91d18f68e
Signed-off-by: Edward O'Callaghan 
---
M wbsio_spi.c
1 file changed, 37 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/54/47854/1

diff --git a/wbsio_spi.c b/wbsio_spi.c
index 26e3bb9..632ff72 100644
--- a/wbsio_spi.c
+++ b/wbsio_spi.c
@@ -16,6 +16,8 @@

#if defined(__i386__) || defined(__x86_64__)

+#include <stdlib.h>
+
 #include "flash.h"
 #include "chipdrivers.h"
 #include "programmer.h"
@@ -25,7 +27,9 @@
 #define WBSIO_PORT1	0x2e
 #define WBSIO_PORT2	0x4e

-static uint16_t wbsio_spibase = 0;
+struct wbsio_spi_data {
+	uint16_t spibase;
+};

static uint16_t wbsio_get_spibase(uint16_t port)
 {
@@ -87,10 +91,13 @@

msg_pspew("%s:", __func__);

+	const struct wbsio_spi_data *data =
+		(const struct wbsio_spi_data *)flash->mst->spi.data;
+
    if (1 == writecnt && 0 == readcnt) {
    	mode = 0x10;
    } else if (2 == writecnt && 0 == readcnt) {
-		OUTB(writearr[1], wbsio_spibase + 4);
+		OUTB(writearr[1], data->spibase + 4);
    	msg_pspew(" data=0x%02x", writearr[1]);
    	mode = 0x20;
    } else if (1 == writecnt && 2 == readcnt) {
@@ -98,28 +105,28 @@
    } else if (4 == writecnt && 0 == readcnt) {
    	msg_pspew(" addr=0x%02x", (writearr[1] & 0x0f));
    	for (i = 2; i < writecnt; i++) {
-			OUTB(writearr[i], wbsio_spibase + i);
+			OUTB(writearr[i], data->spibase + i);
    		msg_pspew("%02x", writearr[i]);
    	}
    	mode = 0x40 | (writearr[1] & 0x0f);
    } else if (5 == writecnt && 0 == readcnt) {
    	msg_pspew(" addr=0x%02x", (writearr[1] & 0x0f));
    	for (i = 2; i < 4; i++) {
-			OUTB(writearr[i], wbsio_spibase + i);
+			OUTB(writearr[i], data->spibase + i);
    		msg_pspew("%02x", writearr[i]);
    	}
-		OUTB(writearr[i], wbsio_spibase + i);
+		OUTB(writearr[i], data->spibase + i);
    	msg_pspew(" data=0x%02x", writearr[i]);
    	mode = 0x50 | (writearr[1] & 0x0f);
    } else if (8 == writecnt && 0 == readcnt) {
    	msg_pspew(" addr=0x%02x", (writearr[1] & 0x0f));
    	for (i = 2; i < 4; i++) {
-			OUTB(writearr[i], wbsio_spibase + i);
+			OUTB(writearr[i], data->spibase + i);
    		msg_pspew("%02x", writearr[i]);
    	}
    	msg_pspew(" data=0x");
    	for (; i < writecnt; i++) {
-			OUTB(writearr[i], wbsio_spibase + i);
+			OUTB(writearr[i], data->spibase + i);
    		msg_pspew("%02x", writearr[i]);
    	}
    	mode = 0x60 | (writearr[1] & 0x0f);
@@ -133,7 +140,7 @@
    } else if (4 == writecnt && readcnt >= 1 && readcnt <= 4) {
    	msg_pspew(" addr=0x%02x", (writearr[1] & 0x0f));
    	for (i = 2; i < writecnt; i++) {
-			OUTB(writearr[i], wbsio_spibase + i);
+			OUTB(writearr[i], data->spibase + i);
    		msg_pspew("%02x", writearr[i]);
    	}
    	mode = ((7 + readcnt) << 4) | (writearr[1] & 0x0f);
@@ -147,8 +154,8 @@
    	return SPI_INVALID_LENGTH;
    }

-	OUTB(writearr[0], wbsio_spibase);
-	OUTB(mode, wbsio_spibase + 1);
+	OUTB(writearr[0], data->spibase);
+	OUTB(mode, data->spibase + 1);
    programmer_delay(10);

if (!readcnt)
@@ -156,7 +163,7 @@

msg_pspew("%s: returning data =", __func__);
    for (i = 0; i < readcnt; i++) {
-		readarr[i] = INB(wbsio_spibase + 4 + i);
+		readarr[i] = INB(data->spibase + 4 + i);
    	msg_pspew(" 0x%02x", readarr[i]);
    }
    msg_pspew("\n");
@@ -170,7 +177,7 @@
    return 0;
 }

-static const struct spi_master spi_master_wbsio = {
+static struct spi_master spi_master_wbsio = {
    .max_data_read = MAX_DATA_UNSPECIFIED,
    .max_data_write = MAX_DATA_UNSPECIFIED,
    .command = wbsio_spi_send_command,
@@ -180,8 +187,16 @@
    .write_aai = spi_chip_write_1,
 };

+static int wbsio_spi_shutdown(void *data)
+{
+	free(data);
+	return 0;
+}
+
 int wbsio_check_for_spi(void)
 {
+	uint16_t wbsio_spibase = 0;
+
    if (0 == (wbsio_spibase = wbsio_get_spibase(WBSIO_PORT1)))
    	if (0 == (wbsio_spibase = wbsio_get_spibase(WBSIO_PORT2)))
    		return 1;
@@ -191,6 +206,16 @@
    msg_pdbg("%s: Winbond saved on 4 register bits so max chip size is "
    	 "1024 kB!\n", __func__);
    max_rom_decode.spi = 1024 * 1024;
+
+	struct wbsio_spi_data * data = calloc(1, sizeof(struct wbsio_spi_data));
+	if (!data) {
+		msg_perr("Unable to allocate space for extra SPI master data.\n");
+		return SPI_GENERIC_ERROR;
+	}
+	data->spibase = wbsio_spibase;
+
+	register_shutdown(wbsio_spi_shutdown, data);
+	spi_master_wbsio.data = data;
    register_spi_master(&spi_master_wbsio);

return 0;

[Edward O'Callaghan (Code Review)]

Hello Sam McNally, build bot (Jenkins), Angel Pons,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/flashrom/+/47854

to look at the new patch set (#2).

Change subject: wbsio_spi.c: Move singleton state into spi master state tracker
......................................................................

wbsio_spi.c: Move singleton state into spi master state tracker

Make use of the reneterent framework by moving singleton static
state out of the global life-time and into a per-spi_master basis.
This allows for the wbsio_spi master to be reneterent and its internal
state's life-time to be correctly handled by Flashrom's core dispatch
logic.

BUG=none
TEST=builds

Change-Id: Ic97fa41daf26f27b68ced11ddc2a4da91d18f68e
Signed-off-by: Edward O'Callaghan 
---
M wbsio_spi.c
1 file changed, 37 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/54/47854/2

[Sam McNally (Code Review)]

Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/47854 )

Change subject: wbsio_spi.c: Move singleton state into spi master state tracker
......................................................................

Patch Set 2: Code-Review+2

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/47854 )

Change subject: wbsio_spi.c: Move singleton state into spi master state tracker
......................................................................

wbsio_spi.c: Move singleton state into spi master state tracker

Make use of the reneterent framework by moving singleton static
state out of the global life-time and into a per-spi_master basis.
This allows for the wbsio_spi master to be reneterent and its internal
state's life-time to be correctly handled by Flashrom's core dispatch
logic.

BUG=none
TEST=builds

Change-Id: Ic97fa41daf26f27b68ced11ddc2a4da91d18f68e
Signed-off-by: Edward O'Callaghan 
Reviewed-on: https://review.coreboot.org/c/flashrom/+/47854
Reviewed-by: Sam McNally 
Tested-by: build bot (Jenkins) 
---
M wbsio_spi.c
1 file changed, 37 insertions(+), 12 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Sam McNally: Looks good to me, approved

diff --git a/wbsio_spi.c b/wbsio_spi.c
index 26e3bb9..632ff72 100644
--- a/wbsio_spi.c
+++ b/wbsio_spi.c
@@ -16,6 +16,8 @@

#if defined(__i386__) || defined(__x86_64__)

+#include <stdlib.h>
+
 #include "flash.h"
 #include "chipdrivers.h"
 #include "programmer.h"
@@ -25,7 +27,9 @@
 #define WBSIO_PORT1	0x2e
 #define WBSIO_PORT2	0x4e

-static uint16_t wbsio_spibase = 0;
+struct wbsio_spi_data {
+	uint16_t spibase;
+};

static uint16_t wbsio_get_spibase(uint16_t port)
 {
@@ -87,10 +91,13 @@

msg_pspew("%s:", __func__);

+	const struct wbsio_spi_data *data =
+		(const struct wbsio_spi_data *)flash->mst->spi.data;
+
    if (1 == writecnt && 0 == readcnt) {
    	mode = 0x10;
    } else if (2 == writecnt && 0 == readcnt) {
-		OUTB(writearr[1], wbsio_spibase + 4);
+		OUTB(writearr[1], data->spibase + 4);
    	msg_pspew(" data=0x%02x", writearr[1]);
    	mode = 0x20;
    } else if (1 == writecnt && 2 == readcnt) {
@@ -98,28 +105,28 @@
    } else if (4 == writecnt && 0 == readcnt) {
    	msg_pspew(" addr=0x%02x", (writearr[1] & 0x0f));
    	for (i = 2; i < writecnt; i++) {
-			OUTB(writearr[i], wbsio_spibase + i);
+			OUTB(writearr[i], data->spibase + i);
    		msg_pspew("%02x", writearr[i]);
    	}
    	mode = 0x40 | (writearr[1] & 0x0f);
    } else if (5 == writecnt && 0 == readcnt) {
    	msg_pspew(" addr=0x%02x", (writearr[1] & 0x0f));
    	for (i = 2; i < 4; i++) {
-			OUTB(writearr[i], wbsio_spibase + i);
+			OUTB(writearr[i], data->spibase + i);
    		msg_pspew("%02x", writearr[i]);
    	}
-		OUTB(writearr[i], wbsio_spibase + i);
+		OUTB(writearr[i], data->spibase + i);
    	msg_pspew(" data=0x%02x", writearr[i]);
    	mode = 0x50 | (writearr[1] & 0x0f);
    } else if (8 == writecnt && 0 == readcnt) {
    	msg_pspew(" addr=0x%02x", (writearr[1] & 0x0f));
    	for (i = 2; i < 4; i++) {
-			OUTB(writearr[i], wbsio_spibase + i);
+			OUTB(writearr[i], data->spibase + i);
    		msg_pspew("%02x", writearr[i]);
    	}
    	msg_pspew(" data=0x");
    	for (; i < writecnt; i++) {
-			OUTB(writearr[i], wbsio_spibase + i);
+			OUTB(writearr[i], data->spibase + i);
    		msg_pspew("%02x", writearr[i]);
    	}
    	mode = 0x60 | (writearr[1] & 0x0f);
@@ -133,7 +140,7 @@
    } else if (4 == writecnt && readcnt >= 1 && readcnt <= 4) {
    	msg_pspew(" addr=0x%02x", (writearr[1] & 0x0f));
    	for (i = 2; i < writecnt; i++) {
-			OUTB(writearr[i], wbsio_spibase + i);
+			OUTB(writearr[i], data->spibase + i);
    		msg_pspew("%02x", writearr[i]);
    	}
    	mode = ((7 + readcnt) << 4) | (writearr[1] & 0x0f);
@@ -147,8 +154,8 @@
    	return SPI_INVALID_LENGTH;
    }

-	OUTB(writearr[0], wbsio_spibase);
-	OUTB(mode, wbsio_spibase + 1);
+	OUTB(writearr[0], data->spibase);
+	OUTB(mode, data->spibase + 1);
    programmer_delay(10);

if (!readcnt)
@@ -156,7 +163,7 @@

msg_pspew("%s: returning data =", __func__);
    for (i = 0; i < readcnt; i++) {
-		readarr[i] = INB(wbsio_spibase + 4 + i);
+		readarr[i] = INB(data->spibase + 4 + i);
    	msg_pspew(" 0x%02x", readarr[i]);
    }
    msg_pspew("\n");
@@ -170,7 +177,7 @@
    return 0;
 }

-static const struct spi_master spi_master_wbsio = {
+static struct spi_master spi_master_wbsio = {
    .max_data_read = MAX_DATA_UNSPECIFIED,
    .max_data_write = MAX_DATA_UNSPECIFIED,
    .command = wbsio_spi_send_command,
@@ -180,8 +187,16 @@
    .write_aai = spi_chip_write_1,
 };

+static int wbsio_spi_shutdown(void *data)
+{
+	free(data);
+	return 0;
+}
+
 int wbsio_check_for_spi(void)
 {
+	uint16_t wbsio_spibase = 0;
+
    if (0 == (wbsio_spibase = wbsio_get_spibase(WBSIO_PORT1)))
    	if (0 == (wbsio_spibase = wbsio_get_spibase(WBSIO_PORT2)))
    		return 1;
@@ -191,6 +206,16 @@
    msg_pdbg("%s: Winbond saved on 4 register bits so max chip size is "
    	 "1024 kB!\n", __func__);
    max_rom_decode.spi = 1024 * 1024;
+
+	struct wbsio_spi_data * data = calloc(1, sizeof(struct wbsio_spi_data));
+	if (!data) {
+		msg_perr("Unable to allocate space for extra SPI master data.\n");
+		return SPI_GENERIC_ERROR;
+	}
+	data->spibase = wbsio_spibase;
+
+	register_shutdown(wbsio_spi_shutdown, data);
+	spi_master_wbsio.data = data;
    register_spi_master(&spi_master_wbsio);

return 0;