New subject: Change in flashrom[master]: ft2232_spi.c: Pack read cmd and nCS in one ftdi_write_data() call

April 27, 2020

Simon Buhrow has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/40748 )


Change subject: ft2232_spi.c - Pack read cmd and nCS in one ftdi_write_data() call
......................................................................

ft2232_spi.c - Pack read cmd and nCS in one ftdi_write_data() call

Every ftdi_write_data() call is quite time consuming as the ftdi-chip takes 2-3ms to respond.
As the comment already says: Minimize USB transfers by packing as many commands as possible together.

As I cannot see any reason to put read cmd and deassertion of nCS into seperated ftdi calls,
I packed them together to save programming time.

Signed-off-by: Simon Buhrow <simon.buhrow@posteo.de>
Change-Id: I3f76da3dd0124a0deec7f937dbc4b7dc488c8004
---
M ft2232_spi.c
1 file changed, 38 insertions(+), 40 deletions(-)



  git pull ssh://review.coreboot.org:29418/flashrom refs/changes/48/40748/1

diff --git a/ft2232_spi.c b/ft2232_spi.c
index 9f4c7f0..b2f5e40 100644
--- a/ft2232_spi.c
+++ b/ft2232_spi.c
@@ -398,7 +398,10 @@
 		msg_perr("Unable to set latency timer (%s).\n", ftdi_get_error_string(ftdic));
 	}
 
-	if (ftdi_write_data_set_chunksize(ftdic, 270)) {
+	if (ftdi_write_data_set_chunksize(ftdic, 280)) {
+	/* 280 Byte = (9 Byte CMD + 1 Byte WREN) + (9 Byte CMD + 1 Byte op) + 4 Byte Addr
+	 * 				+ 256 Byte PageWrite-data
+		with op: PageProgram or Erase; CMD: FTDI-Chip commands*/
 		msg_perr("Unable to set chunk size (%s).\n", ftdi_get_error_string(ftdic));
 	}
 
@@ -467,31 +470,26 @@
 	struct ftdi_context *ftdic = &ftdic_context;
 	static unsigned char *buf = NULL;
 	/* failed is special. We use bitwise ops, but it is essentially bool. */
-	int i = 0, ret = 0, failed = 0;
-	size_t bufsize;
-	static size_t oldbufsize = 0;
+	static int i = 0;
+	int ret = 0, failed = 0;
 
 	if (writecnt > 65536 || readcnt > 65536)
 		return SPI_INVALID_LENGTH;
 
-	/* buf is not used for the response from the chip. */
-	bufsize = max(writecnt + 9, 260 + 9);
-	/* Never shrink. realloc() calls are expensive. */
-	if (!buf || bufsize > oldbufsize) {
-		buf = realloc(buf, bufsize);
+	if (!buf) {
+		/* set buf size to hardware buffer size of ftdi*/
+		buf = realloc(buf, 4096);
 		if (!buf) {
 			msg_perr("Out of memory!\n");
 			/* TODO: What to do with buf? */
 			return SPI_GENERIC_ERROR;
 		}
-		oldbufsize = bufsize;
 	}
 
 	/*
 	 * Minimize USB transfers by packing as many commands as possible
-	 * together. If we're not expecting to read, we can assert CS#, write,
-	 * and deassert CS# all in one shot. If reading, we do three separate
-	 * operations.
+	 * together. We can assert CS#, write, (read,)
+	 * and deassert CS# all in one shot.
 	 */
 	msg_pspew("Assert CS#\n");
 	buf[i++] = SET_BITS_LOW;
@@ -506,44 +504,44 @@
 		i += writecnt;
 	}
 
-	/*
-	 * Optionally terminate this batch of commands with a
-	 * read command, then do the fetch of the results.
-	 */
+	/* A optionally read command */
 	if (readcnt) {
 		buf[i++] = MPSSE_DO_READ;
 		buf[i++] = (readcnt - 1) & 0xff;
 		buf[i++] = ((readcnt - 1) >> 8) & 0xff;
-		ret = send_buf(ftdic, buf, i);
-		failed = ret;
-		/* We can't abort here, we still have to deassert CS#. */
-		if (ret)
-			msg_perr("send_buf failed before read: %i\n", ret);
-		i = 0;
-		if (ret == 0) {
-			/*
-			 * FIXME: This is unreliable. There's no guarantee that
-			 * we read the response directly after sending the read
-			 * command. We may be scheduled out etc.
-			 */
-			ret = get_buf(ftdic, readarr, readcnt);
-			failed |= ret;
-			/* We can't abort here either. */
-			if (ret)
-				msg_perr("get_buf failed: %i\n", ret);
-		}
 	}
 
 	msg_pspew("De-assert CS#\n");
 	buf[i++] = SET_BITS_LOW;
 	buf[i++] = cs_bits;
 	buf[i++] = pindir;
-	ret = send_buf(ftdic, buf, i);
-	failed |= ret;
-	if (ret)
-		msg_perr("send_buf failed at end: %i\n", ret);
 
-	return failed ? -1 : 0;
+	if (writearr[0] == JEDEC_WREN) {
+		/* Return to get second op (Program or Erase) without resetting buf nor i*/
+		return 0;
+	} else {
+		ret = send_buf(ftdic, buf, i);
+		failed |= ret;
+		if (ret)
+			msg_perr("send_buf failed: %i\n", ret);
+		i = 0;
+
+		if (readcnt) {
+			if (ret == 0) {
+				/*
+				* FIXME: This is unreliable. There's no guarantee that
+				* we read the response directly after sending the read
+				* command. We may be scheduled out etc.
+				*/
+				ret = get_buf(ftdic, readarr, readcnt);
+				failed |= ret;
+				/* We can't abort here either. */
+				if (ret)
+					msg_perr("get_buf failed: %i\n", ret);
+			}
+		}
+		return failed ? -1 : 0;
+	}
 }
 
 #endif

-- 
To view, visit https://review.coreboot.org/c/flashrom/+/40748
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3f76da3dd0124a0deec7f937dbc4b7dc488c8004
Gerrit-Change-Number: 40748
Gerrit-PatchSet: 1
Gerrit-Owner: Simon Buhrow
Gerrit-MessageType: newchange

    

Change in flashrom[master]: ft2232_spi.c - Pack read cmd and nCS in one ftdi_write_data() call

Simon Buhrow (Code Review)

Simon Buhrow (Code Review)

Paul Menzel (Code Review)

Simon Buhrow (Code Review)

Simon Buhrow (Code Review)

Simon Buhrow (Code Review)

Angel Pons (Code Review)

Simon Buhrow (Code Review)

Simon Buhrow (Code Review)

Simon Buhrow (Code Review)

Simon Buhrow (Code Review)

tags

participants (3)