Patchwork Make Bus Pirate init more robust

login
register
about
Submitter Carl-Daniel Hailfinger
Date 2010-07-23 20:52:12
Message ID <4C4A00FC.7000502@gmx.net>
Download mbox | patch
Permalink /patch/1673/
State Accepted
Commit r1178
Headers show

Comments

Carl-Daniel Hailfinger - 2010-07-23 20:52:12
Thanks to Johannes Sjölund for reporting that the Bus Pirate init could
not deal with a Bus Pirate which is already in binary Bitbang mode. This
is caused by a combination of the slowness of the Bus Pirate, the
slowness of USB and a fast serial port flush routine which just flushes
the buffer contents and does not wait until data arrival stops.

Make the Bus Pirate init more robust by running the flush command 10
times with 1.5 ms delay in between.

This code development was sponsored by Mattias Mattsson. Thanks!
Tested a few dozen times, should work reliably.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
Carl-Daniel Hailfinger - 2010-07-29 15:30:56
On 23.07.2010 22:52, Carl-Daniel Hailfinger wrote:
> Thanks to Johannes Sjölund for reporting that the Bus Pirate init could
> not deal with a Bus Pirate which is already in binary Bitbang mode. This
> is caused by a combination of the slowness of the Bus Pirate, the
> slowness of USB and a fast serial port flush routine which just flushes
> the buffer contents and does not wait until data arrival stops.
>
> Make the Bus Pirate init more robust by running the flush command 10
> times with 1.5 ms delay in between.
>
> This code development was sponsored by Mattias Mattsson. Thanks!
> Tested a few dozen times, should work reliably.
>
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
>   

Johannes, it would be nice if you could test and (if it works)
reply-to-all with
> Acked-by: Your Name <your@email>

Regards,
Carl-Daniel
Mattias Mattsson - 2010-09-16 22:27:18
On Fri, Jul 23, 2010 at 22:52, Carl-Daniel Hailfinger
<c-d.hailfinger.devel.2006@gmx.net> wrote:
> Thanks to Johannes Sjölund for reporting that the Bus Pirate init could
> not deal with a Bus Pirate which is already in binary Bitbang mode. This
> is caused by a combination of the slowness of the Bus Pirate, the
> slowness of USB and a fast serial port flush routine which just flushes
> the buffer contents and does not wait until data arrival stops.
>
> Make the Bus Pirate init more robust by running the flush command 10
> times with 1.5 ms delay in between.
>
> This code development was sponsored by Mattias Mattsson. Thanks!
> Tested a few dozen times, should work reliably.
>
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>

Acked-by: Mattias Mattsson <vitplister@gmail.com>


> Index: flashrom-buspirate_resilient_init/buspirate_spi.c
> ===================================================================
> --- flashrom-buspirate_resilient_init/buspirate_spi.c   (Revision 1099)
> +++ flashrom-buspirate_resilient_init/buspirate_spi.c   (Arbeitskopie)
> @@ -22,6 +22,7 @@
>  #include <string.h>
>  #include <stdlib.h>
>  #include <ctype.h>
> +#include <unistd.h>
>  #include "flash.h"
>  #include "chipdrivers.h"
>  #include "spi.h"
> @@ -140,6 +141,20 @@
>                /* Read any response and discard it. */
>                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
> +        * ocassional 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();
> +       }
>        /* Enter raw bitbang mode */
>        buf[0] = 0x00;
>        ret = buspirate_sendrecv(buf, 1, 5);
> @@ -147,6 +162,8 @@
>                return ret;
>        if (memcmp(buf, "BBIO", 4)) {
>                msg_perr("Entering raw bitbang mode failed!\n");
> +               msg_pdbg("Got %02x%02x%02x%02x%02x\n",
> +                        buf[0], buf[1], buf[2], buf[3], buf[4]);
>                return 1;
>        }
>        msg_pdbg("Raw bitbang mode version %c\n", buf[4]);
> @@ -158,8 +175,12 @@
>        /* Enter raw SPI mode */
>        buf[0] = 0x01;
>        ret = buspirate_sendrecv(buf, 1, 4);
> +       if (ret)
> +               return ret;
>        if (memcmp(buf, "SPI", 3)) {
>                msg_perr("Entering raw SPI mode failed!\n");
> +               msg_pdbg("Got %02x%02x%02x%02x\n",
> +                        buf[0], buf[1], buf[2], buf[3]);
>                return 1;
>        }
>        msg_pdbg("Raw SPI mode version %c\n", buf[3]);
> Index: flashrom-buspirate_resilient_init/serial.c
> ===================================================================
> --- flashrom-buspirate_resilient_init/serial.c  (Revision 1099)
> +++ flashrom-buspirate_resilient_init/serial.c  (Arbeitskopie)
> @@ -199,8 +199,10 @@
>  #else
>                tmp = write(sp_fd, buf, writecnt);
>  #endif
> -               if (tmp == -1)
> +               if (tmp == -1) {
> +                       msg_perr("Serial port write error!\n");
>                        return 1;
> +               }
>                if (!tmp)
>                        msg_pdbg("Empty write\n");
>                writecnt -= tmp;
> @@ -220,8 +222,10 @@
>  #else
>                tmp = read(sp_fd, buf, readcnt);
>  #endif
> -               if (tmp == -1)
> +               if (tmp == -1) {
> +                       msg_perr("Serial port read error!\n");
>                        return 1;
> +               }
>                if (!tmp)
>                        msg_pdbg("Empty read\n");
>                readcnt -= tmp;
>
>
> --
> http://www.hailfinger.org/
>
>
> _______________________________________________
> flashrom mailing list
> flashrom@flashrom.org
> http://www.flashrom.org/mailman/listinfo/flashrom
Carl-Daniel Hailfinger - 2010-09-16 22:35:52
On 17.09.2010 00:27, Mattias Mattsson wrote:
> On Fri, Jul 23, 2010 at 22:52, Carl-Daniel Hailfinger
> <c-d.hailfinger.devel.2006@gmx.net> wrote:
>   
>> Thanks to Johannes Sjölund for reporting that the Bus Pirate init could
>> not deal with a Bus Pirate which is already in binary Bitbang mode. This
>> is caused by a combination of the slowness of the Bus Pirate, the
>> slowness of USB and a fast serial port flush routine which just flushes
>> the buffer contents and does not wait until data arrival stops.
>>
>> Make the Bus Pirate init more robust by running the flush command 10
>> times with 1.5 ms delay in between.
>>
>> This code development was sponsored by Mattias Mattsson. Thanks!
>> Tested a few dozen times, should work reliably.
>>
>> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
>>     
>
> Acked-by: Mattias Mattsson <vitplister@gmail.com>
>   

Thanks, committed in r1178.

Regards,
Carl-Daniel

Patch

Index: flashrom-buspirate_resilient_init/buspirate_spi.c
===================================================================
--- flashrom-buspirate_resilient_init/buspirate_spi.c	(Revision 1099)
+++ flashrom-buspirate_resilient_init/buspirate_spi.c	(Arbeitskopie)
@@ -22,6 +22,7 @@ 
 #include <string.h>
 #include <stdlib.h>
 #include <ctype.h>
+#include <unistd.h>
 #include "flash.h"
 #include "chipdrivers.h"
 #include "spi.h"
@@ -140,6 +141,20 @@ 
 		/* Read any response and discard it. */
 		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
+	 * ocassional 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();
+	}
 	/* Enter raw bitbang mode */
 	buf[0] = 0x00;
 	ret = buspirate_sendrecv(buf, 1, 5);
@@ -147,6 +162,8 @@ 
 		return ret;
 	if (memcmp(buf, "BBIO", 4)) {
 		msg_perr("Entering raw bitbang mode failed!\n");
+		msg_pdbg("Got %02x%02x%02x%02x%02x\n",
+			 buf[0], buf[1], buf[2], buf[3], buf[4]);
 		return 1;
 	}
 	msg_pdbg("Raw bitbang mode version %c\n", buf[4]);
@@ -158,8 +175,12 @@ 
 	/* Enter raw SPI mode */
 	buf[0] = 0x01;
 	ret = buspirate_sendrecv(buf, 1, 4);
+	if (ret)
+		return ret;
 	if (memcmp(buf, "SPI", 3)) {
 		msg_perr("Entering raw SPI mode failed!\n");
+		msg_pdbg("Got %02x%02x%02x%02x\n",
+			 buf[0], buf[1], buf[2], buf[3]);
 		return 1;
 	}
 	msg_pdbg("Raw SPI mode version %c\n", buf[3]);
Index: flashrom-buspirate_resilient_init/serial.c
===================================================================
--- flashrom-buspirate_resilient_init/serial.c	(Revision 1099)
+++ flashrom-buspirate_resilient_init/serial.c	(Arbeitskopie)
@@ -199,8 +199,10 @@ 
 #else
 		tmp = write(sp_fd, buf, writecnt);
 #endif
-		if (tmp == -1)
+		if (tmp == -1) {
+			msg_perr("Serial port write error!\n");
 			return 1;
+		}
 		if (!tmp)
 			msg_pdbg("Empty write\n");
 		writecnt -= tmp; 
@@ -220,8 +222,10 @@ 
 #else
 		tmp = read(sp_fd, buf, readcnt);
 #endif
-		if (tmp == -1)
+		if (tmp == -1) {
+			msg_perr("Serial port read error!\n");
 			return 1;
+		}
 		if (!tmp)
 			msg_pdbg("Empty read\n");
 		readcnt -= tmp;