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

27 Apr 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