[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 <
c-d.hailfinger.devel.2006 at gmx.net> 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 <c-d.hailfinger.devel.2006 at gmx.net>
>
> 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)
>
>
> --
> Developer quote of the week:
> "We are juggling too many chainsaws and flaming arrows and tigers."
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.flashrom.org/pipermail/flashrom/attachments/20091110/09c97492/attachment.html>
-------------- next part --------------
A binary attachment has been removed from this mail.
The name of the attachment was "Patched_Reads.tar.gz".