[flashrom-gerrit] Change in flashrom[master]: jlink_spi.c: Refactor singleton states into reentrant pattern

[Angel Pons (Code Review)]

Angel Pons has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52560 )

Change subject: jlink_spi.c: Refactor singleton states into reentrant pattern
......................................................................

jlink_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".

TEST=builds
BUG=b:185191942

Change-Id: If13ad0c979ccf62ca4ccbd479d471c657895ef59
Signed-off-by: Anastasia Klimchuk 
Reviewed-on: https://review.coreboot.org/c/flashrom/+/52560
Tested-by: build bot (Jenkins) 
Reviewed-by: Edward O'Callaghan 
Reviewed-by: Angel Pons 
Reviewed-by: Miklós Márton 
---
M jlink_spi.c
1 file changed, 47 insertions(+), 20 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Miklós Márton: Looks good to me, approved
  Angel Pons: Looks good to me, approved
  Edward O'Callaghan: Looks good to me, approved

diff --git a/jlink_spi.c b/jlink_spi.c
index 3d86764..097b0fb 100644
--- a/jlink_spi.c
+++ b/jlink_spi.c
@@ -51,23 +51,25 @@
 /* Minimum target voltage required for operation in mV. */
 #define MIN_TARGET_VOLTAGE	1200

-static struct jaylink_context *jaylink_ctx;
-static struct jaylink_device_handle *jaylink_devh;
-static bool reset_cs;
+struct jlink_spi_data {
+	struct jaylink_context *jaylink_ctx;
+	struct jaylink_device_handle *jaylink_devh;
+	bool reset_cs;
+};

-static bool assert_cs(void)
+static bool assert_cs(struct jlink_spi_data *spi_data)
 {
    int ret;

-	if (reset_cs) {
-		ret = jaylink_clear_reset(jaylink_devh);
+	if (spi_data->reset_cs) {
+		ret = jaylink_clear_reset(spi_data->jaylink_devh);

if (ret != JAYLINK_OK) {
    		msg_perr("jaylink_clear_reset() failed: %s.\n", jaylink_strerror(ret));
    		return false;
    	}
    } else {
-		ret = jaylink_jtag_clear_trst(jaylink_devh);
+		ret = jaylink_jtag_clear_trst(spi_data->jaylink_devh);

if (ret != JAYLINK_OK) {
    		msg_perr("jaylink_jtag_clear_trst() failed: %s.\n", jaylink_strerror(ret));
@@ -78,19 +80,19 @@
    return true;
 }

-static bool deassert_cs(void)
+static bool deassert_cs(struct jlink_spi_data *spi_data)
 {
    int ret;

-	if (reset_cs) {
-		ret = jaylink_set_reset(jaylink_devh);
+	if (spi_data->reset_cs) {
+		ret = jaylink_set_reset(spi_data->jaylink_devh);

if (ret != JAYLINK_OK) {
    		msg_perr("jaylink_set_reset() failed: %s.\n", jaylink_strerror(ret));
    		return false;
    	}
    } else {
-		ret = jaylink_jtag_set_trst(jaylink_devh);
+		ret = jaylink_jtag_set_trst(spi_data->jaylink_devh);

if (ret != JAYLINK_OK) {
    		msg_perr("jaylink_jtag_set_trst() failed: %s.\n", jaylink_strerror(ret));
@@ -106,6 +108,7 @@
 {
    uint32_t length;
    uint8_t *buffer;
+	struct jlink_spi_data *spi_data = flash->mst->spi.data;

length = writecnt + readcnt;

@@ -124,14 +127,15 @@

memset(buffer + writecnt, 0x00, readcnt);

-	if (!assert_cs()) {
+	if (!assert_cs(spi_data)) {
    	free(buffer);
    	return SPI_PROGRAMMER_ERROR;
    }

int ret;

-	ret = jaylink_jtag_io(jaylink_devh, buffer, buffer, buffer, length * 8, JAYLINK_JTAG_VERSION_2);
+	ret = jaylink_jtag_io(spi_data->jaylink_devh,
+				buffer, buffer, buffer, length * 8, JAYLINK_JTAG_VERSION_2);

if (ret != JAYLINK_OK) {
    	msg_perr("jaylink_jtag_io() failed: %s.\n", jaylink_strerror(ret));
@@ -139,7 +143,7 @@
    	return SPI_PROGRAMMER_ERROR;
    }

-	if (!deassert_cs()) {
+	if (!deassert_cs(spi_data)) {
    	free(buffer);
    	return SPI_PROGRAMMER_ERROR;
    }
@@ -151,7 +155,7 @@
    return 0;
 }

-static const struct spi_master spi_master_jlink_spi = {
+static struct spi_master spi_master_jlink_spi = {
    /* Maximum data read size in one go (excluding opcode+address). */
    .max_data_read	= JTAG_MAX_TRANSFER_SIZE - 5,
    /* Maximum data write size in one go (excluding opcode+address). */
@@ -166,11 +170,14 @@

static int jlink_spi_shutdown(void *data)
 {
-	if (jaylink_devh)
-		jaylink_close(jaylink_devh);
+	struct jlink_spi_data *spi_data = data;
+	if (spi_data->jaylink_devh)
+		jaylink_close(spi_data->jaylink_devh);

-	jaylink_exit(jaylink_ctx);
+	jaylink_exit(spi_data->jaylink_ctx);

+	/* jaylink_ctx, jaylink_devh are freed by jaylink_close and jaylink_exit */
+	free(spi_data);
    return 0;
 }

@@ -178,6 +185,10 @@
 {
    char *arg;
    unsigned long speed = 0;
+	struct jaylink_context *jaylink_ctx = NULL;
+	struct jaylink_device_handle *jaylink_devh = NULL;
+	bool reset_cs;
+	struct jlink_spi_data *spi_data = NULL;

arg = extract_programmer_param("spispeed");

@@ -443,11 +454,23 @@

msg_pdbg("SPI speed: %lu kHz\n", speed);

+	spi_data = calloc(1, sizeof(*spi_data));
+	if (!spi_data) {
+		msg_perr("Unable to allocate space for SPI master data\n");
+		goto init_err;
+	}
+
+	/* jaylink_ctx, jaylink_devh are allocated by jaylink_init and jaylink_open */
+	spi_data->jaylink_ctx = jaylink_ctx;
+	spi_data->jaylink_devh = jaylink_devh;
+	spi_data->reset_cs = reset_cs;
+	spi_master_jlink_spi.data = spi_data;
+
    /* Ensure that the CS signal is not active initially. */
-	if (!deassert_cs())
+	if (!deassert_cs(spi_data))
    	goto init_err;

-	if (register_shutdown(jlink_spi_shutdown, NULL))
+	if (register_shutdown(jlink_spi_shutdown, spi_data))
    	goto init_err;
    register_spi_master(&spi_master_jlink_spi);

@@ -459,5 +482,9 @@

jaylink_exit(jaylink_ctx);

+	/* jaylink_ctx, jaylink_devh are freed by jaylink_close and jaylink_exit */
+	if (spi_data)
+		free(spi_data);
+
    return 1;
 }