Edward O'Callaghan has uploaded this change for review.

View Change

spi: Allow cached ID bytes to be cleared

This does two things:
1. Removes the static variables from various ID functions and moves
them to a single struct.
2. Adds a function to clear the struct.

The idea of the original caching mechanism introduced years ago was
simply to speed up probe_flash() by not issuing the same read ID
commands dozens or hundreds of times for a single chip.

That implementation assumed that we would only call probe_flash()
once. However, there are cases when we want to call probe_flash()
multiple times, for example, if using an external programmer and
using different voltages (for probing the chip).

This patch is extremely similar to the original works of:
`commit 57b7524b1448189d3630d6c4735e60dbbdf14d51`.
Author: David Hendricks <dhendrix@chromium.org>
&&
`commit 7f7c711ee1a41649607f81f1533e5135fc1361fc`.
Author: David Hendricks <dhendrix@chromium.org>

BUG=b:15656443
BRANCH=none
TEST=none

Change-Id: I879cb08dbe66db9ab0c3b8a7f93b04fe1c5980f4
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
---
M spi.h
M spi25.c
2 files changed, 50 insertions(+), 26 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/99/41399/1
diff --git a/spi.h b/spi.h
index 3f45038..c1ca46a 100644
--- a/spi.h
+++ b/spi.h
@@ -192,4 +192,6 @@
#define SPI_FLASHROM_BUG -5
#define SPI_PROGRAMMER_ERROR -6

+void clear_spi_id_cache(void);
+
#endif /* !__SPI_H__ */
diff --git a/spi25.c b/spi25.c
index fb91903..b4fbe6d 100644
--- a/spi25.c
+++ b/spi25.c
@@ -27,6 +27,26 @@
#include "programmer.h"
#include "spi.h"

+enum id_type {
+ RDID,
+ RDID4,
+ REMS,
+ RES2,
+ RES3,
+ NUM_ID_TYPES,
+};
+
+static struct {
+ int is_cached;
+ unsigned char bytes[4]; /* enough to hold largest ID type */
+} id_cache[NUM_ID_TYPES];
+
+void clear_spi_id_cache(void)
+{
+ memset(id_cache, 0, sizeof(id_cache));
+ return;
+}
+
static int spi_rdid(struct flashctx *flash, unsigned char *readarr, int bytes)
{
static const unsigned char cmd[JEDEC_RDID_OUTSIZE] = { JEDEC_RDID };
@@ -138,18 +158,20 @@
static int probe_spi_rdid_generic(struct flashctx *flash, int bytes)
{
const struct flashchip *chip = flash->chip;
- unsigned char readarr[4];
uint32_t id1;
uint32_t id2;
+ enum id_type idty = bytes == 3 ? RDID : RDID4;

- const int ret = spi_rdid(flash, readarr, bytes);
- if (ret == SPI_INVALID_LENGTH)
- msg_cinfo("%d byte RDID not supported on this SPI controller\n", bytes);
- if (ret)
- return 0;
+ if (!id_cache[idty].is_cached) {
+ const int ret = spi_rdid(flash, id_cache[idty].bytes, bytes);
+ if (ret == SPI_INVALID_LENGTH)
+ msg_cinfo("%d byte RDID not supported on this SPI controller\n", bytes);
+ if (ret)
+ return 0;
+ id_cache[idty].is_cached = 1;
+ }

- rdid_get_ids(readarr, bytes, &id1, &id2);
-
+ rdid_get_ids(id_cache[idty].bytes, bytes, &id1, &id2);
return compare_id(chip, id1, id2);
}

@@ -166,16 +188,16 @@
int probe_spi_rems(struct flashctx *flash)
{
const struct flashchip *chip = flash->chip;
- unsigned char readarr[JEDEC_REMS_INSIZE];
uint32_t id1, id2;

- if (spi_rems(flash, readarr)) {
- return 0;
+ if (!id_cache[REMS].is_cached) {
+ if (spi_rems(flash, id_cache[REMS].bytes))
+ return 0;
+ id_cache[REMS].is_cached = 1;
}

- id1 = readarr[0];
- id2 = readarr[1];
-
+ id1 = id_cache[REMS].bytes[0];
+ id2 = id_cache[REMS].bytes[1];
return compare_id(chip, id1, id2);
}

@@ -222,16 +244,16 @@

int probe_spi_res2(struct flashctx *flash)
{
- unsigned char readarr[2];
uint32_t id1, id2;

- if (spi_res(flash, readarr, 2)) {
- return 0;
+ if (!id_cache[RES2].is_cached) {
+ if (spi_res(flash, id_cache[RES2].bytes, 2))
+ return 0;
+ id_cache[RES2].is_cached = 1;
}

- id1 = readarr[0];
- id2 = readarr[1];
-
+ id1 = id_cache[RES2].bytes[0];
+ id2 = id_cache[RES2].bytes[1];
msg_cdbg("%s: id1 0x%x, id2 0x%x\n", __func__, id1, id2);

if (id1 != flash->chip->manufacture_id || id2 != flash->chip->model_id)
@@ -242,16 +264,16 @@

int probe_spi_res3(struct flashctx *flash)
{
- unsigned char readarr[3];
uint32_t id1, id2;

- if (spi_res(flash, readarr, 3)) {
- return 0;
+ if (!id_cache[RES3].is_cached) {
+ if (spi_res(flash, id_cache[RES3].bytes, 3))
+ return 0;
+ id_cache[RES3].is_cached = 1;
}

- id1 = (readarr[0] << 8) | readarr[1];
- id2 = readarr[2];
-
+ id1 = (id_cache[RES3].bytes[0] << 8) | id_cache[RES3].bytes[1];
+ id2 = id_cache[RES3].bytes[3];
msg_cdbg("%s: id1 0x%x, id2 0x%x\n", __func__, id1, id2);

if (id1 != flash->chip->manufacture_id || id2 != flash->chip->model_id)

To view, visit change 41399. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I879cb08dbe66db9ab0c3b8a7f93b04fe1c5980f4
Gerrit-Change-Number: 41399
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec@chromium.org>
Gerrit-MessageType: newchange