[flashrom] [PATCH] ft2232_spi: refine error handling in init() and add shutdown function

Stefan Tauner stefan.tauner at alumni.tuwien.ac.at
Wed Mar 16 00:39:11 CET 2016


Signed-off-by: Stefan Tauner <stefan.tauner at alumni.tuwien.ac.at>
---
 ft2232_spi.c | 84 ++++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 47 insertions(+), 37 deletions(-)

Build-tested on the buildbot but not runtime-tested (i still do not own
an FT*232 device yet... I can't believe that either :P).

diff --git a/ft2232_spi.c b/ft2232_spi.c
index be60837..6d388e3 100644
--- a/ft2232_spi.c
+++ b/ft2232_spi.c
@@ -101,7 +101,7 @@ const struct dev_entry devs_ft2232spi[] = {
  */
 static uint8_t cs_bits = 0x08;
 static uint8_t pindir = 0x0b;
-static struct ftdi_context ftdic_context;
+static struct ftdi_context ftdic_context = {0};
 
 static const char *get_ft2232_devicename(int ft2232_vid, int ft2232_type)
 {
@@ -168,10 +168,19 @@ static const struct spi_master spi_master_ft2232 = {
 	.write_aai	= default_spi_write_aai,
 };
 
+static int ft2232_spi_shutdown(void *ftdic) {
+	int ret = ftdi_usb_close(ftdic);
+	if (ret < 0) {
+		msg_perr("Unable to close FTDI device: %s (%d)\n", ftdi_get_error_string(ftdic), ret);
+	}
+	/* The following will deinit the context and set various of its fields to initial values. */
+	ftdi_free(ftdic);
+	return ret;
+}
+
 /* Returns 0 upon success, a negative number upon errors. */
 int ft2232_spi_init(void)
 {
-	int ret = 0;
 	struct ftdi_context *ftdic = &ftdic_context;
 	unsigned char buf[512];
 	int ft2232_vid = FTDI_VID;
@@ -193,11 +202,8 @@ int ft2232_spi_init(void)
 	 * 92 Hz for 12 MHz inputs.
 	 */
 	uint32_t divisor = DEFAULT_DIVISOR;
-	int f;
-	char *arg;
-	double mpsse_clk;
 
-	arg = extract_programmer_param("type");
+	char *arg = extract_programmer_param("type");
 	if (arg) {
 		if (!strcasecmp(arg, "2232H")) {
 			ft2232_type = FTDI_FT2232H_PID;
@@ -338,22 +344,31 @@ int ft2232_spi_init(void)
 		 (ft2232_interface == INTERFACE_B) ? "B" :
 		 (ft2232_interface == INTERFACE_C) ? "C" : "D");
 
-	if (ftdi_init(ftdic) < 0) {
-		msg_perr("ftdi_init failed.\n");
-		return -3;
+	int ret = ftdi_init(ftdic);
+	if (ret < 0) {
+		msg_perr("Failed to initialize libftdi driver (%d).\n", ret);
+		return 1;
 	}
 
-	if (ftdi_set_interface(ftdic, ft2232_interface) < 0) {
-		msg_perr("Unable to select channel (%s).\n", ftdi_get_error_string(ftdic));
+	if (register_shutdown(ft2232_spi_shutdown, NULL) != 0) {
+		return 1;
+	}
+
+	ret = ftdi_set_interface(ftdic, ft2232_interface);
+	if (ret < 0) {
+		msg_perr("Unable to select channel: %s (%d).\n", ftdi_get_error_string(ftdic), ret);
+		return 1;
 	}
 
 	arg = extract_programmer_param("serial");
-	f = ftdi_usb_open_desc(ftdic, ft2232_vid, ft2232_type, NULL, arg);
+	int f = ftdi_usb_open_desc(ftdic, ft2232_vid, ft2232_type, NULL, arg);
 	free(arg);
 
+	/* -5 is reported in case we cannot claim the respective device. If that's really the case we will abort
+	 * later anywhere. Till then pretend we did not notice... rationale: we always have beed doing it. */
 	if (f < 0 && f != -5) {
-		msg_perr("Unable to open FTDI device: %d (%s).\n", f, ftdi_get_error_string(ftdic));
-		return -4;
+		msg_perr("Unable to open FTDI device: %s (%d).\n", ftdi_get_error_string(ftdic), f);
+		return 1;
 	}
 
 	if (ftdic->type != TYPE_2232H && ftdic->type != TYPE_4232H && ftdic->type != TYPE_232H) {
@@ -362,27 +377,32 @@ int ft2232_spi_init(void)
 	}
 
 	if (ftdi_usb_reset(ftdic) < 0) {
-		msg_perr("Unable to reset FTDI device (%s).\n", ftdi_get_error_string(ftdic));
+		msg_perr("Unable to reset FTDI device: %s.\n", ftdi_get_error_string(ftdic));
+		return 1;
 	}
 
 	if (ftdi_set_latency_timer(ftdic, 2) < 0) {
-		msg_perr("Unable to set latency timer (%s).\n", ftdi_get_error_string(ftdic));
+		msg_perr("Unable to set latency timer: %s.\n", ftdi_get_error_string(ftdic));
+		return 1;
 	}
 
 	if (ftdi_write_data_set_chunksize(ftdic, 256)) {
-		msg_perr("Unable to set chunk size (%s).\n", ftdi_get_error_string(ftdic));
+		msg_perr("Unable to set chunk size: %s.\n", ftdi_get_error_string(ftdic));
+		return 1;
 	}
 
 	if (ftdi_set_bitmode(ftdic, 0x00, BITMODE_BITBANG_SPI) < 0) {
-		msg_perr("Unable to set bitmode to SPI (%s).\n", ftdi_get_error_string(ftdic));
+		msg_perr("Unable to set bitmode to SPI: %s.\n", ftdi_get_error_string(ftdic));
+		return 1;
 	}
 
+	double mpsse_clk;
 	if (clock_5x) {
 		msg_pdbg("Disable divide-by-5 front stage\n");
 		buf[0] = 0x8a; /* Disable divide-by-5. DIS_DIV_5 in newer libftdi */
 		if (send_buf(ftdic, buf, 1)) {
-			ret = -5;
-			goto ftdi_err;
+			msg_perr("Unable to disable front stage divider: %s.\n", ftdi_get_error_string(ftdic));
+			return 1;
 		}
 		mpsse_clk = 60.0;
 	} else {
@@ -394,39 +414,29 @@ int ft2232_spi_init(void)
 	buf[1] = (divisor / 2 - 1) & 0xff;
 	buf[2] = ((divisor / 2 - 1) >> 8) & 0xff;
 	if (send_buf(ftdic, buf, 3)) {
-		ret = -6;
-		goto ftdi_err;
+		msg_perr("Unable to set divider: %s.\n", ftdi_get_error_string(ftdic));
+		return 1;
 	}
 
 	msg_pdbg("MPSSE clock: %f MHz, divisor: %u, SPI clock: %f MHz\n",
 		 mpsse_clk, divisor, (double)(mpsse_clk / divisor));
 
 	/* Disconnect TDI/DO to TDO/DI for loopback. */
-	msg_pdbg("No loopback of TDI/DO TDO/DI\n");
 	buf[0] = LOOPBACK_END;
 	if (send_buf(ftdic, buf, 1)) {
-		ret = -7;
-		goto ftdi_err;
+		msg_perr("Unable to disable TDI/DO TDO/DI loopback: %s.\n", ftdi_get_error_string(ftdic));
+		return 1;
 	}
 
-	msg_pdbg("Set data bits\n");
 	buf[0] = SET_BITS_LOW;
 	buf[1] = cs_bits;
 	buf[2] = pindir;
 	if (send_buf(ftdic, buf, 3)) {
-		ret = -8;
-		goto ftdi_err;
+		msg_perr("Unable to set initial pin states: %s.\n", ftdi_get_error_string(ftdic));
+		return 1;
 	}
 
-	register_spi_master(&spi_master_ft2232);
-
-	return 0;
-
-ftdi_err:
-	if ((f = ftdi_usb_close(ftdic)) < 0) {
-		msg_perr("Unable to close FTDI device: %d (%s)\n", f, ftdi_get_error_string(ftdic));
-	}
-	return ret;
+	return register_spi_master(&spi_master_ft2232);
 }
 
 /* Returns 0 upon success, a negative number upon errors. */
-- 
Kind regards, Stefan Tauner





More information about the flashrom mailing list