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

Idwer Vollering vidwer at gmail.com
Sun Jul 15 21:28:36 CEST 2012


2012/6/13 Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>:
> 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.

Tested with reading/erasing/writing a MX25L1605 on a Thinkpad X60
mainboard, using buspirate hardware v3.a running firmware v6.2-test
that can be found here (bp-spifix.zip):
http://dangerousprototypes.com/forum/viewtopic.php?f=40&t=3864&sid=426d32139bbbd6fa5abf86ef084cfe8c&start=15#p41505
.

>
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
>
> 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");

or:
while (writecnt)
instead?

>         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");

or:
while (readcnt)
?


>         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;

This could use some explanation of the bits that are set in bit[0]: 0x40 and 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;

explanation of 0x60

> +       ret = buspirate_sendrecv(buf, 1, 1);
> +       if (ret)
> +               return 1;
> +       if (buf[0] != 0x01) {

#define BP_PROTO_ERR_SPEED 0x1?

> +               msg_perr("Protocol error while setting SPI speed!\n");
> +               return 1;
> +       }
> +
> +       /* Set SPI config: output type, idle, clock edge, sample */
> +       buf[0] = 0x80 | 0xa;

explanation of 0x80 and 0xa

> +       ret = buspirate_sendrecv(buf, 1, 1);
> +       if (ret)
> +               return 1;
> +       if (buf[0] != 0x01) {

#define BP_PROTO_ERR_CONFIG 0x1?

> +               msg_perr("Protocol error while setting SPI config!\n");
> +               return 1;
> +       }
> +
> +       /* De-assert CS# */
> +       buf[0] = 0x03;

#define ... 0x3 ?

> +       ret = buspirate_sendrecv(buf, 1, 1);
> +       if (ret)
> +               return 1;
> +       if (buf[0] != 0x01) {

#define ... 0x1 ?

> +               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,160 @@
>         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.
> +        */

Resetting the buspirate can be done with entering a pound sign on its
console: "HiZ>#" (if that is what you want to here, to make it respond
again?).

> +       for (i = 0; i < 20; i++) {
>                 /* Enter raw bitbang mode */
>                 bp_commbuf[0] = 0x00;

#define?

>                 /* 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;

#define BP_COMMBUF_RESET 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;
> +
> +       /* 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) {

#define BP_SPEED_MHZ_FOR_FW_6.2_AND_NEWER 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;

#define BP_RAW_BITBANG_MODE 0x00 ?

> -       ret = buspirate_sendrecv(bp_commbuf, 1, 5);
> -       if (ret)
> +       for (i = 0; i < 20; i++) {
> +               bp_commbuf[0] = 0x00;

#define ?

> +               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;

0x04 and 0: #defines?

> +       ret = buspirate_sendrecv(bp_commbuf, 5, 1);
>         if (ret)
>                 return 1;
>         if (bp_commbuf[0] != 0x01) {

0x01 => BP_PROTO_SPEED_ERR?

> -               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;

Change 12 to MAX_DATA_UNSPECIFIED (twice)?

> +
> +               /* 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 +458,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 +510,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;

You are using [0]..[4] instead of i++ earlier in this patch, let's
stick with [0] for consistency.

> +       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);
> +       }
> +}
>
>
> --
> http://www.hailfinger.org/
>
>
> _______________________________________________
> flashrom mailing list
> flashrom at flashrom.org
> http://www.flashrom.org/mailman/listinfo/flashrom




More information about the flashrom mailing list