[PATCH] Cache SPI RDID response

[Carl-Daniel Hailfinger]

Not for merge... yet.
It works for dummy, nothing else was tested.
Limitations/bugs mentioned in the patch.

Signed-off-by: Carl-Daniel Hailfinger

Index: flashrom-spi_cache_rdid/spi.c
===================================================================
--- flashrom-spi_cache_rdid/spi.c	(Revision 1611)
+++ flashrom-spi_cache_rdid/spi.c	(Arbeitskopie)
@@ -24,18 +24,46 @@

#include <strings.h>
 #include <string.h>
+#include <stdlib.h>
 #include "flash.h"
 #include "flashchips.h"
 #include "chipdrivers.h"
 #include "programmer.h"
 #include "spi.h"

+/* FIXME: We want a per-command cache, not just a RDID cache.
+ * FIXME: We should cache this for spi_send_multicommand programmers as well.
+ */
+struct rdidcache {
+	int available;
+	unsigned char *readarr;
+} rdidcache = {0};
+
 int spi_send_command(struct flashctx *flash, unsigned int writecnt,
    	     unsigned int readcnt, const unsigned char *writearr,
    	     unsigned char *readarr)
 {
-	return flash->pgm->spi.command(flash, writecnt, readcnt, writearr,
+	int ret;
+	unsigned char *tmp;
+
+	if ((writearr[0] == JEDEC_RDID) && (rdidcache.available >= readcnt)) {
+		memcpy(readarr, rdidcache.readarr, readcnt);
+		return 0;
+	}
+	ret = flash->pgm->spi.command(flash, writecnt, readcnt, writearr,
    			       readarr);
+	if (!ret && (writearr[0] == JEDEC_RDID) && (rdidcache.available < readcnt)) {
+		tmp = realloc(rdidcache.readarr, readcnt);
+		if (!tmp) {
+			/* Oh well. Don't cache stuff, then. No problem. */
+			msg_perr("Doom due to OOM! Brace for impact!\n");
+			return ret;
+		}
+		rdidcache.readarr = tmp;
+		rdidcache.available = readcnt;
+		memcpy(rdidcache.readarr, readarr, readcnt);
+	}
+	return ret;
 }

int spi_send_multicommand(struct flashctx *flash, struct spi_command *cmds)

[David Hendricks]

On Wed, Oct 3, 2012 at 3:59 PM, Carl-Daniel Hailfinger <
[e-mail]> wrote:
> Not for merge... yet.
> It works for dummy, nothing else was tested.
> Limitations/bugs mentioned in the patch.
>
> Signed-off-by: Carl-Daniel Hailfinger
> Index: flashrom-spi_cache_rdid/spi.c
>
> --- flashrom-spi_cache_rdid/spi.c       (Revision 1611)
> +++ flashrom-spi_cache_rdid/spi.c       (Arbeitskopie)
> @@ -24,18 +24,46 @@
>
> #include <strings.h>
>  #include <string.h>
> +#include <stdlib.h>
>  #include "flash.h"
>  #include "flashchips.h"
>  #include "chipdrivers.h"
>  #include "programmer.h"
>  #include "spi.h"
>
> +/* FIXME: We want a per-command cache, not just a RDID cache.
> FIXME: We should cache this for spi_send_multicommand programmers as
>
> well.
> */
>
> +struct rdidcache {
>   int available;
>   unsigned char *readarr;
>
> +} rdidcache = {0};
>
> int spi_send_command(struct flashctx *flash, unsigned int writecnt,
>                      unsigned int readcnt, const unsigned char *writearr,
>                      unsigned char *readarr)
>  {
>   return flash->pgm->spi.command(flash, writecnt, readcnt, writearr,
>   int ret;
>   unsigned char *tmp;
>   if ((writearr[0] == JEDEC_RDID) && (rdidcache.available >=
>
> readcnt)) {
>           memcpy(readarr, rdidcache.readarr, readcnt);
>           return 0;
>   }
>   ret = flash->pgm->spi.command(flash, writecnt, readcnt, writearr,
>                                  readarr);
>   if (!ret && (writearr[0] == JEDEC_RDID) && (rdidcache.available <
>
> readcnt)) {
>           tmp = realloc(rdidcache.readarr, readcnt);
>           if (!tmp) {
>                   /* Oh well. Don't cache stuff, then. No problem. */
>                   msg_perr("Doom due to OOM! Brace for impact!\n");
>                   return ret;
>           }
>           rdidcache.readarr = tmp;
>           rdidcache.available = readcnt;
>           memcpy(rdidcache.readarr, readarr, readcnt);
>   }
>   return ret;
>
> }
>
> int spi_send_multicommand(struct flashctx *flash, struct spi_command
> *cmds)

Interesting approach. However, I think 3-byte vs. 4-byte RDID commands and
caching REMS might make patching spi_send_command rather messy.

I made a patch that takes a different route by changing
probe_spi_{rdid,rdid4,rems} functions instead. My patch can be viewed via
Gerrit on Chromium.org @
https://gerrit.chromium.org/gerrit/#/c/35376/2 (doesn't
apply cleanly against upsteam currently).