[M] Change in flashrom[master]: ni845x_spi: refactor singleton states into reentrant pattern

[Anastasia Klimchuk (Code Review)]

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/72160?usp=email )

 ( 

4 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
 )Change subject: ni845x_spi: refactor singleton states into reentrant pattern
......................................................................

ni845x_spi: 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.

TOPIC=register_master_api

Change-Id: I45fcb8e20582cb0c532c4a9f0c78543a25f8d484
Signed-off-by: Alexander Goncharov <[e-mail]>
Ticket: https://ticket.coreboot.org/issues/391
Reviewed-on: https://review.coreboot.org/c/flashrom/+/72160
Reviewed-by: Anastasia Klimchuk <[e-mail]>
Tested-by: build bot (Jenkins) <[e-mail]>
---
M ni845x_spi.c
1 file changed, 62 insertions(+), 39 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Anastasia Klimchuk: Looks good to me, approved




diff --git a/ni845x_spi.c b/ni845x_spi.c
index 58a8ef4..1948387 100644
--- a/ni845x_spi.c
+++ b/ni845x_spi.c
@@ -47,13 +47,14 @@
 	USE_HIGHER
 };
 
-static unsigned char CS_number;	// use chip select 0 as default
-static enum USB845x_type device_pid = Unknown_NI845X_Device;
-
-static uInt32 device_handle;
-static NiHandle configuration_handle;
-static uint16_t io_voltage_in_mV;
-static bool ignore_io_voltage_limits;
+struct ni845x_spi_data {
+	unsigned char CS_number; // use chip select 0 as default
+	enum USB845x_type device_pid;
+	uInt32 device_handle;
+	NiHandle configuration_handle;
+	uint16_t io_voltage_in_mV;
+	bool ignore_io_voltage_limits;
+};
 
 // USB-8452 supported voltages, keep this array in ascending order!
 static const uint8_t usb8452_io_voltages_in_100mV[5] = {
@@ -394,98 +395,104 @@
 
 static int ni845x_spi_shutdown(void *data)
 {
+	struct ni845x_spi_data *ni_data = data;
 	int32 ret = 0;
 
-	if (configuration_handle != 0) {
-		ret = ni845xSpiConfigurationClose(configuration_handle);
+	if (ni_data->configuration_handle != 0) {
+		ret = ni845xSpiConfigurationClose(ni_data->configuration_handle);
 		if (ret)
 			ni845x_report_error("ni845xSpiConfigurationClose", ret);
 	}
 
-	if (device_handle != 0) {
-		ret = ni845xClose(device_handle);
+	if (ni_data->device_handle != 0) {
+		ret = ni845xClose(ni_data->device_handle);
 		if (ret)
 			ni845x_report_error("ni845xClose", ret);
 	}
-	return 0;
+
+	free(data);
+	return ret;
 }
 
 static void ni845x_warn_over_max_voltage(const struct flashctx *flash)
 {
-	if (device_pid == USB8451) {
+	const struct ni845x_spi_data *data = flash->mst->spi.data;
+
+	if (data->device_pid == USB8451) {
 		msg_pwarn("The %s chip maximum voltage is %.1fV, while the USB-8451 "
 			  "IO voltage levels are 3.3V.\n"
 			  "Ignoring this because ignore_io_voltage_limits parameter is set.\n",
 			 flash->chip->name,
 			 flash->chip->voltage.max / 1000.0f);
-	} else if (device_pid == USB8452) {
+	} else if (data->device_pid == USB8452) {
 		msg_pwarn("The %s chip maximum voltage is %.1fV, while the USB-8452 "
 			  "IO voltage is set to %.1fV.\n"
 			  "Ignoring this because ignore_io_voltage_limits parameter is set.\n",
 			  flash->chip->name,
 			  flash->chip->voltage.max / 1000.0f,
-			  io_voltage_in_mV / 1000.0f);
+			  data->io_voltage_in_mV / 1000.0f);
 	}
 }
 
 static int ni845x_spi_io_voltage_check(const struct flashctx *flash)
 {
+	struct ni845x_spi_data *data = flash->mst->spi.data;
 	static bool first_transmit = true;
 
 	if (first_transmit && flash->chip) {
 		first_transmit = false;
-		if (io_voltage_in_mV > flash->chip->voltage.max) {
-			if (ignore_io_voltage_limits) {
+		if (data->io_voltage_in_mV > flash->chip->voltage.max) {
+			if (data->ignore_io_voltage_limits) {
 				ni845x_warn_over_max_voltage(flash);
 				return 0;
 			}
 
-			if (device_pid == USB8451) {
+			if (data->device_pid == USB8451) {
 				msg_perr("The %s chip maximum voltage is %.1fV, while the USB-8451 "
 					 "IO voltage levels are 3.3V.\nAborting operations\n",
 					 flash->chip->name,
 					 flash->chip->voltage.max / 1000.0f);
 				return -1;
-			} else if (device_pid == USB8452) {
+			} else if (data->device_pid == USB8452) {
 				msg_perr("Lowering IO voltage because the %s chip maximum voltage is %.1fV, "
 					 "(%.1fV was set)\n",
 					 flash->chip->name,
 					 flash->chip->voltage.max / 1000.0f,
-					 io_voltage_in_mV / 1000.0f);
+					 data->io_voltage_in_mV / 1000.0f);
 				if (usb8452_spi_set_io_voltage(flash->chip->voltage.max,
-							       &io_voltage_in_mV,
+							       &data->io_voltage_in_mV,
 							       USE_LOWER,
-							       device_pid,
-							       device_handle)) {
+							       data->device_pid,
+							       data->device_handle)) {
 					msg_perr("Unable to lower the IO voltage below "
 						 "the chip's maximum voltage\n");
 					return -1;
 				}
 			}
-		} else if (io_voltage_in_mV < flash->chip->voltage.min) {
-			if (device_pid == USB8451) {
+		} else if (data->io_voltage_in_mV < flash->chip->voltage.min) {
+			if (data->device_pid == USB8451) {
 				msg_pwarn("Flash operations might be unreliable, because the %s chip's "
 					  "minimum voltage is %.1fV, while the USB-8451's "
 					  "IO voltage levels are 3.3V.\n",
 					  flash->chip->name,
 					  flash->chip->voltage.min / 1000.0f);
-				return ignore_io_voltage_limits ? 0 : -1;
-			} else if (device_pid == USB8452) {
+				return data->ignore_io_voltage_limits ? 0 : -1;
+			} else if (data->device_pid == USB8452) {
 				msg_pwarn("Raising the IO voltage because the %s chip's "
 					  "minimum voltage is %.1fV, "
 					  "(%.1fV was set)\n",
 					  flash->chip->name,
 					  flash->chip->voltage.min / 1000.0f,
-					  io_voltage_in_mV / 1000.0f);
+					  data->io_voltage_in_mV / 1000.0f);
 				if (usb8452_spi_set_io_voltage(flash->chip->voltage.min,
-							       &io_voltage_in_mV,
+							       &data->io_voltage_in_mV,
 							       USE_HIGHER,
-							       device_pid,
-							       device_handle)) {
+							       data->device_pid,
+							       data->device_handle)) {
 					msg_pwarn("Unable to raise the IO voltage above the chip's "
 						  "minimum voltage\n"
 						  "Flash operations might be unreliable.\n");
-					return ignore_io_voltage_limits ? 0 : -1;
+					return data->ignore_io_voltage_limits ? 0 : -1;
 				}
 			}
 		}
@@ -499,6 +506,7 @@
 			       const unsigned char *write_arr,
 			       unsigned char *read_arr)
 {
+	const struct ni845x_spi_data *data = flash->mst->spi.data;
 	uInt32 read_size = 0;
 	uInt8 *transfer_buffer = NULL;
 	int32 ret = 0;
@@ -514,8 +522,8 @@
 
 	memcpy(transfer_buffer, write_arr, write_cnt);
 
-	ret = ni845xSpiWriteRead(device_handle,
-				 configuration_handle,
+	ret = ni845xSpiWriteRead(data->device_handle,
+				 data->configuration_handle,
 				 (write_cnt + read_cnt), transfer_buffer, &read_size, transfer_buffer);
 	if (ret < 0) {
 		// Negative specifies an error, meaning the function did not perform the expected behavior.
@@ -560,6 +568,10 @@
 	int spi_speed_KHz = 1000;		// selecting 1 MHz SCK is a good bet
 	char *serial_number = NULL;		// by default open the first connected device
 	char *ignore_io_voltage_limits_str = NULL;
+	bool ignore_io_voltage_limits;
+	unsigned char CS_number = 0;
+	enum USB845x_type device_pid = Unknown_NI845X_Device;
+	uInt32 device_handle;
 	int32 tmp = 0;
 
 	// read the cs parameter (which Chip select should we use)
@@ -618,27 +630,38 @@
 	}
 	free(serial_number);
 
+	struct ni845x_spi_data *data = calloc(1, sizeof(*data));
+	if (!data) {
+		msg_perr("Unable to allocate space for SPI master data\n");
+		return 1;
+	}
+	data->CS_number = CS_number;
+	data->device_pid = device_pid;
+	data->device_handle = device_handle;
+	data->ignore_io_voltage_limits = ignore_io_voltage_limits;
+
 	// open the SPI config handle
-	tmp = ni845xSpiConfigurationOpen(&configuration_handle);
+	tmp = ni845xSpiConfigurationOpen(&data->configuration_handle);
 	if (tmp != 0) {
 		ni845x_report_error("ni845xSpiConfigurationOpen", tmp);
 		goto err;
 	}
 
-	if (usb8452_spi_set_io_voltage(requested_io_voltage_mV, &io_voltage_in_mV, USE_LOWER, device_pid, device_handle) < 0) {
+	if (usb8452_spi_set_io_voltage(requested_io_voltage_mV, &data->io_voltage_in_mV,
+				       USE_LOWER, data->device_pid, data->device_handle) < 0) {
 		// no alert here usb8452_spi_set_io_voltage already printed that
 		goto err;
 	}
 
-	if (ni845x_spi_set_speed(configuration_handle, spi_speed_KHz)) {
+	if (ni845x_spi_set_speed(data->configuration_handle, spi_speed_KHz)) {
 		msg_perr("Unable to set SPI speed\n");
 		goto err;
 	}
 
-	return register_spi_master(&spi_programmer_ni845x, NULL);
+	return register_spi_master(&spi_programmer_ni845x, data);
 
 err:
-	ni845x_spi_shutdown(NULL);
+	ni845x_spi_shutdown(data);
 	return 1;
 }
 

-- 
To view, visit https://review.coreboot.org/c/flashrom/+/72160?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I45fcb8e20582cb0c532c4a9f0c78543a25f8d484
Gerrit-Change-Number: 72160
Gerrit-PatchSet: 6
Gerrit-Owner: Alexander Goncharov <[e-mail]>
Gerrit-Reviewer: Anastasia Klimchuk <[e-mail]>
Gerrit-Reviewer: Miklós Márton <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Thomas Heijligen <[e-mail]>
Gerrit-MessageType: merged