Hello Carl-Daniel Hailfinger,
I'd like you to do a code review. Please visit
https://review.coreboot.org/25114
to review the following change.
Change subject: Add support for generic SPI RES matching. ......................................................................
Add support for generic SPI RES matching.
Support generic 1-byte SPI RES matching (low match quality) and enable read support by guessing the chip size from the ID. Support generic 2-byte SPI RES matching (high match quality).
Change-Id: Ie82d57740ee620c03861d2ebe812490431f4fb34 Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net Signed-off-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at --- M chipdrivers.h M flashchips.c M flashrom.c M spi25.c 4 files changed, 84 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/14/25114/1
diff --git a/chipdrivers.h b/chipdrivers.h index 0c78b0d..5626db5 100644 --- a/chipdrivers.h +++ b/chipdrivers.h @@ -33,6 +33,7 @@ int probe_spi_rems(struct flashctx *flash); int probe_spi_res1(struct flashctx *flash); int probe_spi_res2(struct flashctx *flash); +int probe_spi_automagic_res1(struct flashctx *flash); int spi_write_enable(struct flashctx *flash); int spi_write_disable(struct flashctx *flash); int spi_block_erase_20(struct flashctx *flash, unsigned int addr, unsigned int blocklen); diff --git a/flashchips.c b/flashchips.c index 789f17f..2a38ee6 100644 --- a/flashchips.c +++ b/flashchips.c @@ -9662,6 +9662,24 @@
{ .vendor = "Generic", + .name = "unknown SPI chip (RES1)", + .bustype = BUS_SPI, + .manufacture_id = GENERIC_MANUF_ID, + .model_id = GENERIC_DEVICE_ID, + .total_size = 0, + .page_size = 256, + .tested = TEST_BAD_PREW, + .probe = probe_spi_automagic_res1, + .probe_timing = TIMING_ZERO, + .block_erasers = {}, + .unlock = spi_disable_blockprotect, + .write = NULL, + .read = spi_chip_read, + .voltage = {}, + }, + + { + .vendor = "Generic", .name = "unknown SPI chip (REMS)", .bustype = BUS_SPI, .manufacture_id = GENERIC_MANUF_ID, @@ -9673,5 +9691,18 @@ .write = NULL, },
+ { + .vendor = "Generic", + .name = "unknown SPI chip (RES2)", + .bustype = BUS_SPI, + .manufacture_id = GENERIC_MANUF_ID, + .model_id = GENERIC_DEVICE_ID, + .total_size = 0, + .page_size = 256, + .tested = TEST_BAD_PREW, + .probe = probe_spi_res2, + .write = NULL, + }, + { NULL } }; diff --git a/flashrom.c b/flashrom.c index 44a3eba..c39abf6 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1029,7 +1029,9 @@ if (startchip == 0) break; /* Not the first flash chip detected on this bus, but not a generic match either. */ - if ((flash->chip->model_id != GENERIC_DEVICE_ID) && (flash->chip->model_id != SFDP_DEVICE_ID)) + if ((flash->chip->manufacture_id != GENERIC_MANUF_ID) && + (flash->chip->model_id != GENERIC_DEVICE_ID) && + (flash->chip->model_id != SFDP_DEVICE_ID)) break; /* Not the first flash chip detected on this bus, and it's just a generic match. Ignore it. */ notfound: diff --git a/spi25.c b/spi25.c index 914b821..7d9e948 100644 --- a/spi25.c +++ b/spi25.c @@ -232,6 +232,7 @@
int probe_spi_res1(struct flashctx *flash) { + const struct flashchip *chip = flash->chip; static const unsigned char allff[] = {0xff, 0xff, 0xff}; static const unsigned char all00[] = {0x00, 0x00, 0x00}; unsigned char readarr[3]; @@ -265,18 +266,23 @@
msg_cdbg("%s: id 0x%x\n", __func__, id2);
- if (id2 != flash->chip->model_id) - return 0; + if (id2 == flash->chip->model_id) { + /* Print the status register to tell the user about possible write protection. */ + spi_prettyprint_status_register(flash);
- /* Print the status register to tell the - * user about possible write protection. - */ - spi_prettyprint_status_register(flash); - return 1; + return 1; + } + + /* Test if there is any device ID. */ + if (GENERIC_MANUF_ID == chip->manufacture_id && id2 == chip->model_id) + return 1; + + return 0; }
int probe_spi_res2(struct flashctx *flash) { + const struct flashchip *chip = flash->chip; unsigned char readarr[2]; uint32_t id1, id2;
@@ -289,13 +295,44 @@
msg_cdbg("%s: id1 0x%x, id2 0x%x\n", __func__, id1, id2);
- if (id1 != flash->chip->manufacture_id || id2 != flash->chip->model_id) + if (id1 == chip->manufacture_id && id2 == chip->model_id) { + /* Print the status register to tell the + * user about possible write protection. + */ + spi_prettyprint_status_register(flash); + + return 1; + } + + /* Test if this is a pure vendor match. */ + if (id1 == chip->manufacture_id && GENERIC_DEVICE_ID == chip->model_id) + return 1; + + /* Test if there is any vendor ID and if this is indeed a two-byte RES. */ + if (GENERIC_MANUF_ID == chip->manufacture_id && id1 != 0xff && id1 != id2) + return 1; + + return 0; +} + +int probe_spi_automagic_res1(struct flashctx *flash) +{ + unsigned char readarr[1]; + unsigned int total_size; + uint32_t id1; + + if (spi_res(flash, readarr, 1)) return 0;
- /* Print the status register to tell the - * user about possible write protection. - */ - spi_prettyprint_status_register(flash); + id1 = readarr[0]; + msg_cdbg("%s: id1 0x%02x\n", __func__, id1); + if (id1 < 0x10 || id1 > 0x17) { + msg_cdbg("RES ID out of bounds!\n"); + return 0; + } + total_size = (2 << id1) / 1024; + flash->chip->total_size = total_size; + flash->chip->tested = TEST_OK_PR | TEST_BAD_ERASE | TEST_BAD_WRITE; return 1; }