Re: [flashrom] [PATCH] FT2232 error checking

[Jeremy Buseman]

Carl,
I downloaded a clean copy from SVN and patched it.  I pulled 4 reads without
verbose and 4 with, they are attached.  The reads came back different each
time just as before, but I did not get any error messages so this must be a
libftdi issue.  Let me know if there's more I can do.
Jeremy

On Mon, Nov 9, 2009 at 10:07 PM, Carl-Daniel Hailfinger <
[e-mail]> wrote:
> Pretty much everybody who used the FT2232 SPI driver in flashrom had
> problems with incorrect reads from time to time.
> One reason was that the hardware is pretty timing sensitive even for reads.
>
> The other reason was that the code silently ignored errors. This patch
> doesn't add any error recovery, but it will emit error messages if
> FT2232 communication goes wrong. That allows us to track down errors
> without investing hours in driver debugging.
>
> Jeremy, I'd be very interested in the results of an unmodified flashrom
> with only this patch applied (read is sufficient). In theory, you should
> either get a working read or loads of error messages about
> send_buf/read_buf. If you get no error messages and the image read is
> still wrong, libftdi doesn't tell us about the problem. Oh, and please
> try in verbose and normal mode. Maybe there's a difference.
>
> This patch will show up at
> http://patchwork.coreboot.org/project/flashrom/list/ in a few minutes.
>
> Signed-off-by: Carl-Daniel Hailfinger
> Index: flashrom-ft2232_errorcheck/ft2232_spi.c
>
> --- flashrom-ft2232_errorcheck/ft2232_spi.c     (Revision 756)
> +++ flashrom-ft2232_errorcheck/ft2232_spi.c     (Arbeitskopie)
> @@ -198,7 +198,8 @@
>  {
>        struct ftdi_context *ftdic = &ftdic_context;
>        static unsigned char *buf = NULL;
>   int i = 0, ret = 0;
>   /* failed is special. We use bitwise ops, but it is essentially
>
> bool. */
>   int i = 0, ret = 0, failed = 0;
>
>  if (writecnt > 65536 || readcnt > 65536)
>          return SPI_INVALID_LENGTH;
>
> @@ -237,6 +238,11 @@
>                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)
>                   fprintf(stderr, "send_buf failed before read:
>
> %i\n",
>                           ret);
>          i = 0;
>          if (ret == 0) {
>                  /*
>
> @@ -245,6 +251,10 @@
>                         * command. We may be scheduled out etc.
>                         */
>                        ret = get_buf(ftdic, readarr, readcnt);
>                   failed |= ret;
>                   /* We can't abort here either. */
>                   if (ret)
>                           fprintf(stderr, "get_buf failed: %i\n",
>
> ret);
>                }
>        }
>
> @@ -252,10 +262,12 @@
>        buf[i++] = SET_BITS_LOW;
>        buf[i++] = CS_BIT;
>        buf[i++] = 0x0b;
>   if (send_buf(ftdic, buf, i))
>           return -1;
>   ret = send_buf(ftdic, buf, i);
>   failed |= ret;
>   if (ret)
>           fprintf(stderr, "send_buf failed at end: %i\n", ret);
>
>   return ret;
>   return failed ? -1 : 0;
>
> }
>
> int ft2232_spi_read(struct flashchip *flash, uint8_t *buf, int start, int
> len)

A binary attachment has been removed from this mail.
The name of the attachment was "Patched_Reads.tar.gz".