Re: [flashrom] [PATCH] Bus Pirate: Speedup and firmware workarounds

[Carl-Daniel Hailfinger]

Am 13.06.2012 03:40 schrieb Carl-Daniel Hailfinger:
> Bus Pirate Firmware v5.5 and newer support a new SPI binary mode. Use it
> if available.
> Bus Pirate Firmware v6.1 and older have broken (too slow) SPI clock
> divisor for any requested speed above 2 MHz. Force a downgrade to 2 MHz
> for affected firmware versions.
> flashrom will recommend to upgrade the Bus Pirate Firmware if it is
> older than v6.2.
>
> Somewhat tested, code still needs to be cleaned up in a few spots.
>
> Signed-off-by: Carl-Daniel Hailfinger

With this patch, you get a 10x speedup (if you use latest firmware).

WARNING: This patch breaks Windows compilation and has other nasty side
effects, but it's OK if you want to test with Linux.

Index: flashrom-buspirate_newcommands/spi25.c
===================================================================
--- flashrom-buspirate_newcommands/spi25.c	(Revision 1541)
+++ flashrom-buspirate_newcommands/spi25.c	(Arbeitskopie)
@@ -968,7 +968,7 @@
 {
    int rc = 0;
    unsigned int i, j, starthere, lenhere, toread;
-	unsigned int page_size = flash->page_size;
+	unsigned int page_size = 2048;

/* Warning: This loop has a very unusual condition and body.
     * The loop needs to go through each page with at least one affected
Index: flashrom-buspirate_newcommands/buspirate_spi.c
===================================================================
--- flashrom-buspirate_newcommands/buspirate_spi.c	(Revision 1541)
+++ flashrom-buspirate_newcommands/buspirate_spi.c	(Arbeitskopie)
@@ -50,6 +50,7 @@
 #define sp_flush_incoming(...) 0
 #endif

+static int buspirate_interface_version;
 static unsigned char *bp_commbuf = NULL;
 static int bp_commbufsize = 0;

@@ -83,7 +84,8 @@
    	msg_perr("Zero length command!\n");
    	return 1;
    }
-	msg_pspew("Sending");
+	if (writecnt)
+		msg_pspew("Sending");
    for (i = 0; i < writecnt; i++)
    	msg_pspew(" 0x%02x", buf[i]);
 #ifdef FAKE_COMMUNICATION
@@ -103,23 +105,36 @@
    if (ret)
    	return ret;
 #endif
-	msg_pspew(", receiving");
+	if (readcnt)
+		msg_pspew(", receiving");
    for (i = 0; i < readcnt; i++)
    	msg_pspew(" 0x%02x", buf[i]);
    msg_pspew("\n");
    return 0;
 }

-static int buspirate_spi_send_command(struct flashctx *flash,
-				      unsigned int writecnt,
-				      unsigned int readcnt,
-				      const unsigned char *writearr,
-				      unsigned char *readarr);
+static int buspirate_wait_for_string(unsigned char *buf, char *key)
+{
+	unsigned int keylen = strlen(key);
+	int ret;

-static const struct spi_programmer spi_programmer_buspirate = {
+	ret = buspirate_sendrecv(buf, 0, keylen);
+	while (!ret) {
+		if (!memcmp(buf, key, keylen))
+			return 0;
+		memmove(buf, buf + 1, keylen - 1);
+		ret = buspirate_sendrecv(buf + keylen - 1, 0, 1);
+	}
+	return ret;
+}
+
+static int buspirate_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt,
+				      const unsigned char *writearr, unsigned char *readarr);
+
+static struct spi_programmer spi_programmer_buspirate = {
    .type		= SPI_CONTROLLER_BUSPIRATE,
-	.max_data_read	= 12,
-	.max_data_write	= 12,
+	.max_data_read	= MAX_DATA_UNSPECIFIED,
+	.max_data_write	= MAX_DATA_UNSPECIFIED,
    .command	= buspirate_spi_send_command,
    .multicommand	= default_spi_send_multicommand,
    .read		= default_spi_read,
@@ -138,6 +153,53 @@
    {NULL,		0x0},
 };

+int buspirate_spi_set_config(unsigned char *buf, int spispeed)
+{
+	int ret;
+
+	/* Initial setup (SPI peripherals config): Enable power, CS high, AUX */
+	buf[0] = 0x40 | 0xb;
+	ret = buspirate_sendrecv(buf, 1, 1);
+	if (ret)
+		return 1;
+	if (buf[0] != 0x01) {
+		msg_perr("Protocol error while setting power/CS/AUX!\n");
+		return 1;
+	}
+
+	/* Set SPI speed */
+	buf[0] = 0x60 | spispeed;
+	ret = buspirate_sendrecv(buf, 1, 1);
+	if (ret)
+		return 1;
+	if (buf[0] != 0x01) {
+		msg_perr("Protocol error while setting SPI speed!\n");
+		return 1;
+	}
+	
+	/* Set SPI config: output type, idle, clock edge, sample */
+	buf[0] = 0x80 | 0xa;
+	ret = buspirate_sendrecv(buf, 1, 1);
+	if (ret)
+		return 1;
+	if (buf[0] != 0x01) {
+		msg_perr("Protocol error while setting SPI config!\n");
+		return 1;
+	}
+
+	/* De-assert CS# */
+	buf[0] = 0x03;
+	ret = buspirate_sendrecv(buf, 1, 1);
+	if (ret)
+		return 1;
+	if (buf[0] != 0x01) {
+		msg_perr("Protocol error while raising CS#!\n");
+		return 1;
+	}
+
+	return 0;
+}
+
 static int buspirate_spi_shutdown(void *data)
 {
    int ret = 0, ret2 = 0;
@@ -180,10 +242,15 @@
    return ret;
 }

+#define BP_VERSION(a,b)	((a) << 8 | (b))
+
 int buspirate_spi_init(void)
 {
    char *dev = NULL;
    char *speed = NULL;
+	char *tmp;
+	unsigned int fw_version_major = 0;
+	unsigned int fw_version_minor = 0;
    int spispeed = 0x7;
    int ret = 0;
    int i;
@@ -208,9 +275,6 @@
    }
    free(speed);

-	/* This works because speeds numbering starts at 0 and is contiguous. */
-	msg_pdbg("SPI speed is %sHz\n", spispeeds[spispeed].name);
-
    /* Default buffer size is 19: 16 bytes data, 3 bytes control. */
 #define DEFAULT_BUFSIZE (16 + 3)
    bp_commbuf = malloc(DEFAULT_BUFSIZE);
@@ -233,106 +297,188 @@
    if (register_shutdown(buspirate_spi_shutdown, NULL))
    	return 1;

-	/* This is the brute force version, but it should work. */
-	for (i = 0; i < 19; i++) {
+	/* This is the brute force version, but it should work.
+	 * It is likely to fail if a previous flashrom run was aborted during a write with the new SPI commands
+	 * in firmware v5.4 because that firmware may wait for up to 4096 bytes of input before responding to
+	 * 0x00 again. The obvious workaround (sending 4096 bytes of \0) may cause significant startup delays.
+	 */
+	for (i = 0; i < 20; i++) {
    	/* Enter raw bitbang mode */
    	bp_commbuf[0] = 0x00;
    	/* Send the command, don't read the response. */
    	ret = buspirate_sendrecv(bp_commbuf, 1, 0);
    	if (ret)
    		return ret;
-		/* Read any response and discard it. */
-		sp_flush_incoming();
+		/* The old way to handle responses from a Bus Pirate already in BBIO mode was to flush any
+		 * response which came in over serial. Unfortunately that does not work reliably on Linux
+		 * with FTDI USB-serial.
+		 */
+		//sp_flush_incoming();
    }
-	/* USB is slow. The Bus Pirate is even slower. Apparently the flush
-	 * action above is too fast or too early. Some stuff still remains in
-	 * the pipe after the flush above, and one additional flush is not
-	 * sufficient either. Use a 1.5 ms delay inside the loop to make
-	 * mostly sure that at least one USB frame had time to arrive.
-	 * Looping only 5 times is not sufficient and causes the
-	 * occasional failure.
-	 * Folding the delay into the loop above is not reliable either.
-	 */
-	for (i = 0; i < 10; i++) {
-		usleep(1500);
-		/* Read any response and discard it. */
-		sp_flush_incoming();
+	/* We know that 20 commands of \0 should elicit at least one BBIO1 response. */
+	if ((ret = buspirate_wait_for_string(bp_commbuf, "BBIO")))
+		return ret;
+
+	/* Reset the Bus Pirate. */
+	bp_commbuf[0] = 0x0f;
+	/* Send the command, don't read the response. */
+	if ((ret = buspirate_sendrecv(bp_commbuf, 1, 0)))
+		return ret;
+	if ((ret = buspirate_wait_for_string(bp_commbuf, "irate ")))
+		return ret;
+	/* Read the hardware version string. Last byte of the buffer is reserved for \0. */
+	for (i = 0; i < DEFAULT_BUFSIZE - 1; i++) {
+		if ((ret = buspirate_sendrecv(bp_commbuf + i, 0, 1)))
+			return ret;
+		if (strchr("\r\n\t ", bp_commbuf[i]))
+			break;
    }
+	bp_commbuf[i] = '\0';
+	msg_pdbg("Detected Bus Pirate hardware %s\n", bp_commbuf);
+
+	if ((ret = buspirate_wait_for_string(bp_commbuf, "irmware ")))
+		return ret;
+	/* Read the firmware version string. Last byte of the buffer is reserved for \0. */
+	for (i = 0; i < DEFAULT_BUFSIZE - 1; i++) {
+		if ((ret = buspirate_sendrecv(bp_commbuf + i, 0, 1)))
+			return ret;
+		if (strchr("\r\n\t ", bp_commbuf[i]))
+			break;
+	}
+	bp_commbuf[i] = '\0';
+	msg_pdbg("Detected Bus Pirate firmware %s ", bp_commbuf);
+	if (bp_commbuf[0] != 'v')
+		msg_pdbg("(unknown version number format)\n");
+	else if (!strchr("0123456789", bp_commbuf[1]))
+		msg_pdbg("(unknown version number format)\n");
+	else {
+		fw_version_major = strtoul((char *)bp_commbuf + 1, &tmp, 10);
+		while ((*tmp != '\0') && !strchr("0123456789", *tmp))
+			tmp++;
+		fw_version_minor = strtoul(tmp, NULL, 10);
+		msg_pdbg("(%u.%u)\n", fw_version_major, fw_version_minor);
+	}
+
+	if ((ret = buspirate_wait_for_string(bp_commbuf, "HiZ>")))
+		return ret;
+	
+	/* Increase UART speed settings in firmware 6.2 and newer. */
+	if (BP_VERSION(fw_version_major, fw_version_minor) >= BP_VERSION(6, 2)) {
+		int cmdlen;
+		/* Request setting the UART baud rate. */
+		cmdlen = snprintf((char *)bp_commbuf, DEFAULT_BUFSIZE, "b\n");
+		if ((ret = buspirate_sendrecv(bp_commbuf, cmdlen, 0)))
+			return ret;
+		if ((ret = buspirate_wait_for_string(bp_commbuf, ">")))
+			return ret;
+		/* Request setting the UART clock divisor manually. */
+		cmdlen = snprintf((char *)bp_commbuf, DEFAULT_BUFSIZE, "10\n");
+		if ((ret = buspirate_sendrecv(bp_commbuf, cmdlen, 0)))
+			return ret;
+		if ((ret = buspirate_wait_for_string(bp_commbuf, ">")))
+			return ret;
+		/* Set the UART clock divisor. New clock is 4000000/(divisor+1). */
+		cmdlen = snprintf((char *)bp_commbuf, DEFAULT_BUFSIZE, "%i\n", 1);
+		if ((ret = buspirate_sendrecv(bp_commbuf, cmdlen, 0)))
+			return ret;
+		sleep(1);
+		sp_serport_changebaud(2000000);
+		bp_commbuf[0] = ' ';
+		if ((ret = buspirate_sendrecv(bp_commbuf, 1, 0)))
+			return ret;
+		if ((ret = buspirate_wait_for_string(bp_commbuf, "HiZ>")))
+			return ret;
+	}
+
+	/* Workaround for broken speed settings in firmware 6.1 and older. */
+	if (BP_VERSION(fw_version_major, fw_version_minor) < BP_VERSION(6, 2))
+		if (spispeed > 0x4) {
+			msg_perr("Bus Pirate firmware 6.1 and older does not support SPI speeds above 2 MHz. "
+				 "Limiting speed to 2 MHz.\n");
+			msg_pinfo("It is recommended to upgrade to firmware 6.2 or newer.\n");
+			spispeed = 0x4;
+		}
+		
+	/* Tell the user about missing fast mode in firmware 5.4 and older. */
+	if (BP_VERSION(fw_version_major, fw_version_minor) < BP_VERSION(5, 5)) {
+		msg_pinfo("Bus Pirate firmware 5.4 and older does not support fast SPI access.\n");
+		msg_pinfo("It is recommended to upgrade to firmware 6.2 or newer.\n");
+	}
+
+	/* This works because speeds numbering starts at 0 and is contiguous. */
+	msg_pdbg("SPI speed is %sHz\n", spispeeds[spispeed].name);
+
    /* Enter raw bitbang mode */
-	bp_commbuf[0] = 0x00;
-	ret = buspirate_sendrecv(bp_commbuf, 1, 5);
-	if (ret)
+	for (i = 0; i < 20; i++) {
+		bp_commbuf[0] = 0x00;
+		if ((ret = buspirate_sendrecv(bp_commbuf, 1, 0)))
+			return ret;
+	}
+	if ((ret = buspirate_wait_for_string(bp_commbuf, "BBIO")))
    	return ret;
-	if (memcmp(bp_commbuf, "BBIO", 4)) {
-		msg_perr("Entering raw bitbang mode failed!\n");
-		msg_pdbg("Got %02x%02x%02x%02x%02x\n",
-			 bp_commbuf[0], bp_commbuf[1], bp_commbuf[2],
-			 bp_commbuf[3], bp_commbuf[4]);
+	if ((ret = buspirate_sendrecv(bp_commbuf, 0, 1)))
+		return ret;
+	msg_pdbg("Raw bitbang mode version %c\n", bp_commbuf[0]);
+	if (bp_commbuf[0] != '1') {
+		msg_perr("Can't handle raw bitbang mode version %c!\n", bp_commbuf[0]);
    	return 1;
    }
-	msg_pdbg("Raw bitbang mode version %c\n", bp_commbuf[4]);
-	if (bp_commbuf[4] != '1') {
-		msg_perr("Can't handle raw bitbang mode version %c!\n",
-			bp_commbuf[4]);
-		return 1;
-	}
    /* Enter raw SPI mode */
    bp_commbuf[0] = 0x01;
-	ret = buspirate_sendrecv(bp_commbuf, 1, 4);
-	if (ret)
+	ret = buspirate_sendrecv(bp_commbuf, 1, 0);
+	if ((ret = buspirate_wait_for_string(bp_commbuf, "SPI")))
    	return ret;
-	if (memcmp(bp_commbuf, "SPI", 3)) {
-		msg_perr("Entering raw SPI mode failed!\n");
-		msg_pdbg("Got %02x%02x%02x%02x\n",
-			 bp_commbuf[0], bp_commbuf[1], bp_commbuf[2],
-			 bp_commbuf[3]);
+	if ((ret = buspirate_sendrecv(bp_commbuf, 0, 1)))
+		return ret;
+	msg_pdbg("Raw SPI mode version %c\n", bp_commbuf[0]);
+	if (bp_commbuf[0] != '1') {
+		msg_perr("Can't handle raw SPI mode version %c!\n", bp_commbuf[0]);
    	return 1;
    }
-	msg_pdbg("Raw SPI mode version %c\n", bp_commbuf[3]);
-	if (bp_commbuf[3] != '1') {
-		msg_perr("Can't handle raw SPI mode version %c!\n",
-			bp_commbuf[3]);
-		return 1;
-	}

-	/* Initial setup (SPI peripherals config): Enable power, CS high, AUX */
-	bp_commbuf[0] = 0x40 | 0xb;
-	ret = buspirate_sendrecv(bp_commbuf, 1, 1);
-	if (ret)
+	if (buspirate_spi_set_config(bp_commbuf, spispeed))
    	return 1;
-	if (bp_commbuf[0] != 0x01) {
-		msg_perr("Protocol error while setting power/CS/AUX!\n");
-		return 1;
-	}

-	/* Set SPI speed */
-	bp_commbuf[0] = 0x60 | spispeed;
-	ret = buspirate_sendrecv(bp_commbuf, 1, 1);
+	/* Test combined SPI write/read, length 0. */
+	bp_commbuf[0] = 0x04;
+	bp_commbuf[1] = 0;
+	bp_commbuf[2] = 0;
+	bp_commbuf[3] = 0;
+	bp_commbuf[4] = 0;
+	ret = buspirate_sendrecv(bp_commbuf, 5, 1);
    if (ret)
    	return 1;
    if (bp_commbuf[0] != 0x01) {
-		msg_perr("Protocol error while setting SPI speed!\n");
-		return 1;
-	}
-	
-	/* Set SPI config: output type, idle, clock edge, sample */
-	bp_commbuf[0] = 0x80 | 0xa;
-	ret = buspirate_sendrecv(bp_commbuf, 1, 1);
-	if (ret)
-		return 1;
-	if (bp_commbuf[0] != 0x01) {
-		msg_perr("Protocol error while setting SPI config!\n");
-		return 1;
-	}
+		msg_pdbg("SPI command set v2 not available, using old commands "
+			 "present in firmware <v5.5.\n");

-	/* De-assert CS# */
-	bp_commbuf[0] = 0x03;
-	ret = buspirate_sendrecv(bp_commbuf, 1, 1);
-	if (ret)
-		return 1;
-	if (bp_commbuf[0] != 0x01) {
-		msg_perr("Protocol error while raising CS#!\n");
-		return 1;
+		/* FIXME: Check the error code? */
+		/* We sent 4 bytes of 0x00, so we expect 4 BBIO1 responses. */
+		buspirate_sendrecv(bp_commbuf, 0, 4 * 5);
+
+		/* Enter raw SPI mode again. */
+		bp_commbuf[0] = 0x01;
+		/* FIXME: Check the error code? */
+		buspirate_sendrecv(bp_commbuf, 1, 4);
+
+		buspirate_interface_version = 1;
+		/* Sensible default buffer size. */
+		if (buspirate_commbuf_grow(16 + 3))
+			return ERROR_OOM;
+		spi_programmer_buspirate.max_data_read = 12;
+		spi_programmer_buspirate.max_data_write = 12;
+
+		/* Reinit the whole shebang. */
+		if (buspirate_spi_set_config(bp_commbuf, spispeed))
+			return 1;
+	} else {
+		msg_pdbg("Using SPI command set v2.\n"); 
+		buspirate_interface_version = 2;
+		/* Sensible default buffer size. */
+		if (buspirate_commbuf_grow(260 + 5))
+			return ERROR_OOM;
+		spi_programmer_buspirate.max_data_read = 2048;
+		spi_programmer_buspirate.max_data_write = 256;
    }

register_spi_programmer(&spi_programmer_buspirate);
@@ -340,11 +486,8 @@
    return 0;
 }

-static int buspirate_spi_send_command(struct flashctx *flash,
-				      unsigned int writecnt,
-				      unsigned int readcnt,
-				      const unsigned char *writearr,
-				      unsigned char *readarr)
+static int buspirate_spi_send_command_v1(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt,
+					 const unsigned char *writearr, unsigned char *readarr)
 {
    unsigned int i = 0;
    int ret = 0;
@@ -395,3 +538,54 @@

return ret;
 }
+
+static int buspirate_spi_send_command_v2(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt,
+					 const unsigned char *writearr, unsigned char *readarr)
+{
+	int i = 0, ret = 0;
+
+	if (writecnt > 4096 || readcnt > 4096 || (readcnt + writecnt) > 4096)
+		return SPI_INVALID_LENGTH;
+
+	/* 5 bytes extra for command, writelen, readlen.
+	 * 1 byte extra for Ack/Nack.
+	 */
+	if (buspirate_commbuf_grow(max(writecnt + 5, readcnt + 1)))
+		return ERROR_OOM;
+
+	/* Combined SPI write/read. */
+	bp_commbuf[i++] = 0x04;
+	bp_commbuf[i++] = (writecnt >> 8) & 0xff;
+	bp_commbuf[i++] = writecnt & 0xff;
+	bp_commbuf[i++] = (readcnt >> 8) & 0xff;
+	bp_commbuf[i++] = readcnt & 0xff;
+	memcpy(bp_commbuf + i, writearr, writecnt);
+	
+	ret = buspirate_sendrecv(bp_commbuf, i + writecnt, 1 + readcnt);
+
+	if (ret) {
+		msg_perr("Bus Pirate communication error!\n");
+		return SPI_GENERIC_ERROR;
+	}
+
+	if (bp_commbuf[0] != 0x01) {
+		msg_perr("Protocol error while sending SPI write/read!\n");
+		return SPI_GENERIC_ERROR;
+	}
+
+	/* Skip Ack. */
+	memcpy(readarr, bp_commbuf + 1, readcnt);
+
+	return ret;
+}
+
+static int buspirate_spi_send_command(struct flashctx *flash, unsigned int writecnt, unsigned int readcnt,
+				      const unsigned char *writearr, unsigned char *readarr)
+{
+	switch (buspirate_interface_version) {
+	case 2:
+		return buspirate_spi_send_command_v2(flash, writecnt, readcnt, writearr, readarr);
+	default:
+		return buspirate_spi_send_command_v1(flash, writecnt, readcnt, writearr, readarr);
+	}
+}
Index: flashrom-buspirate_newcommands/serial.c
===================================================================
--- flashrom-buspirate_newcommands/serial.c	(Revision 1541)
+++ flashrom-buspirate_newcommands/serial.c	(Arbeitskopie)
@@ -104,6 +104,28 @@
 };
 #endif

+int sp_serport_changebaud(unsigned int baud)
+{
+	struct termios options;
+	int i;
+
+	tcgetattr(sp_fd, &options);
+	for (i = 0;; i++) {
+		if (sp_baudtable[i].baud == 0) {
+			close(sp_fd);
+			msg_perr("Error: cannot configure for baudrate %d\n", baud);
+			exit(1);
+		}
+		if (sp_baudtable[i].baud == baud) {
+			cfsetispeed(&options, sp_baudtable[i].flag);
+			cfsetospeed(&options, sp_baudtable[i].flag);
+			break;
+		}
+	}
+	tcsetattr(sp_fd, TCSANOW, &options);
+	return 0;
+}
+
 fdtype sp_openserport(char *dev, unsigned int baud)
 {
 #ifdef _WIN32
Index: flashrom-buspirate_newcommands/flashchips.c
===================================================================
--- flashrom-buspirate_newcommands/flashchips.c	(Revision 1541)
+++ flashrom-buspirate_newcommands/flashchips.c	(Arbeitskopie)
@@ -1133,7 +1133,7 @@
    	.page_size	= 256,
    	/* OTP: 64B total; read 0x4B, 0x48; write 0x42 */
    	.feature_bits	= FEATURE_WRSR_WREN | FEATURE_OTP,
-		.tested		= TEST_UNTESTED,
+		.tested		= TEST_OK_PREW,
    	.probe		= probe_spi_rdid,
    	.probe_timing	= TIMING_ZERO,
    	.block_erasers	=
Index: flashrom-buspirate_newcommands/programmer.h
===================================================================
--- flashrom-buspirate_newcommands/programmer.h	(Revision 1541)
+++ flashrom-buspirate_newcommands/programmer.h	(Arbeitskopie)
@@ -642,6 +642,7 @@

void sp_flush_incoming(void);
 fdtype sp_openserport(char *dev, unsigned int baud);
+int sp_serport_changebaud(unsigned int baud);
 void __attribute__((noreturn)) sp_die(char *msg);
 extern fdtype sp_fd;
 /* expose serialport_shutdown as it's currently used by buspirate */