Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/41398 )
Change subject: spi25.c: Factor out rdid_get_ids() and compare_id() ......................................................................
spi25.c: Factor out rdid_get_ids() and compare_id()
This is in preparation for implementing a cache for the probe results of RDID and REMS (3&4-byte variant) commands. The intention is to make probing of SPI rom's slightly faster, a few 10's of ms dependant upon the spi master used.
BUG=b:15656443 BRANCH=none TEST=builds
Change-Id: I1556e97a7c70425069e3d1dc0d5daf0aeec4e7bf Signed-off-by: Edward O'Callaghan quasisec@google.com --- M spi25.c 1 file changed, 30 insertions(+), 32 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/98/41398/1
diff --git a/spi25.c b/spi25.c index 0a939e7..fb91903 100644 --- a/spi25.c +++ b/spi25.c @@ -93,19 +93,9 @@ return spi_send_command(flash, sizeof(cmd), 0, cmd, NULL); }
-static int probe_spi_rdid_generic(struct flashctx *flash, int bytes) +static void rdid_get_ids(unsigned char *readarr, int bytes, + uint32_t *id1, uint32_t *id2) { - const struct flashchip *chip = flash->chip; - unsigned char readarr[4]; - uint32_t id1; - uint32_t id2; - - 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 (!oddparity(readarr[0])) msg_cdbg("RDID byte 0 parity violation. ");
@@ -115,17 +105,20 @@ if (readarr[0] == 0x7f) { if (!oddparity(readarr[1])) msg_cdbg("RDID byte 1 parity violation. "); - id1 = (readarr[0] << 8) | readarr[1]; - id2 = readarr[2]; + *id1 = (readarr[0] << 8) | readarr[1]; + *id2 = readarr[2]; if (bytes > 3) { - id2 <<= 8; - id2 |= readarr[3]; + *id2 <<= 8; + *id2 |= readarr[3]; } } else { - id1 = readarr[0]; - id2 = (readarr[1] << 8) | readarr[2]; + *id1 = readarr[0]; + *id2 = (readarr[1] << 8) | readarr[2]; } +}
+static int compare_id(const struct flashchip *chip, uint32_t id1, uint32_t id2) +{ msg_cdbg("%s: id1 0x%02x, id2 0x%02x\n", __func__, id1, id2);
if (id1 == chip->manufacture_id && id2 == chip->model_id) @@ -142,6 +135,24 @@ return 0; }
+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; + + 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; + + rdid_get_ids(readarr, bytes, &id1, &id2); + + return compare_id(chip, id1, id2); +} + int probe_spi_rdid(struct flashctx *flash) { return probe_spi_rdid_generic(flash, 3); @@ -165,20 +176,7 @@ id1 = readarr[0]; id2 = readarr[1];
- msg_cdbg("%s: id1 0x%x, id2 0x%x\n", __func__, id1, id2); - - if (id1 == chip->manufacture_id && id2 == chip->model_id) - 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. */ - if (GENERIC_MANUF_ID == chip->manufacture_id && id1 != 0xff && id1 != 0x00) - return 1; - - return 0; + return compare_id(chip, id1, id2); }
int probe_spi_res1(struct flashctx *flash)
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/41398 )
Change subject: spi25.c: Factor out rdid_get_ids() and compare_id() ......................................................................
Patch Set 1: Code-Review+1
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/41398 )
Change subject: spi25.c: Factor out rdid_get_ids() and compare_id() ......................................................................
Patch Set 1:
Hey Angel, anything holding this back from a +2 ?
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/41398 )
Change subject: spi25.c: Factor out rdid_get_ids() and compare_id() ......................................................................
Patch Set 1: Code-Review+2
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/41398 )
Change subject: spi25.c: Factor out rdid_get_ids() and compare_id() ......................................................................
spi25.c: Factor out rdid_get_ids() and compare_id()
This is in preparation for implementing a cache for the probe results of RDID and REMS (3&4-byte variant) commands. The intention is to make probing of SPI rom's slightly faster, a few 10's of ms dependant upon the spi master used.
BUG=b:15656443 BRANCH=none TEST=builds
Change-Id: I1556e97a7c70425069e3d1dc0d5daf0aeec4e7bf Signed-off-by: Edward O'Callaghan quasisec@google.com Reviewed-on: https://review.coreboot.org/c/flashrom/+/41398 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Stefan Reinauer stefan.reinauer@coreboot.org --- M spi25.c 1 file changed, 30 insertions(+), 32 deletions(-)
Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve
diff --git a/spi25.c b/spi25.c index 0a939e7..fb91903 100644 --- a/spi25.c +++ b/spi25.c @@ -93,19 +93,9 @@ return spi_send_command(flash, sizeof(cmd), 0, cmd, NULL); }
-static int probe_spi_rdid_generic(struct flashctx *flash, int bytes) +static void rdid_get_ids(unsigned char *readarr, int bytes, + uint32_t *id1, uint32_t *id2) { - const struct flashchip *chip = flash->chip; - unsigned char readarr[4]; - uint32_t id1; - uint32_t id2; - - 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 (!oddparity(readarr[0])) msg_cdbg("RDID byte 0 parity violation. ");
@@ -115,17 +105,20 @@ if (readarr[0] == 0x7f) { if (!oddparity(readarr[1])) msg_cdbg("RDID byte 1 parity violation. "); - id1 = (readarr[0] << 8) | readarr[1]; - id2 = readarr[2]; + *id1 = (readarr[0] << 8) | readarr[1]; + *id2 = readarr[2]; if (bytes > 3) { - id2 <<= 8; - id2 |= readarr[3]; + *id2 <<= 8; + *id2 |= readarr[3]; } } else { - id1 = readarr[0]; - id2 = (readarr[1] << 8) | readarr[2]; + *id1 = readarr[0]; + *id2 = (readarr[1] << 8) | readarr[2]; } +}
+static int compare_id(const struct flashchip *chip, uint32_t id1, uint32_t id2) +{ msg_cdbg("%s: id1 0x%02x, id2 0x%02x\n", __func__, id1, id2);
if (id1 == chip->manufacture_id && id2 == chip->model_id) @@ -142,6 +135,24 @@ return 0; }
+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; + + 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; + + rdid_get_ids(readarr, bytes, &id1, &id2); + + return compare_id(chip, id1, id2); +} + int probe_spi_rdid(struct flashctx *flash) { return probe_spi_rdid_generic(flash, 3); @@ -165,20 +176,7 @@ id1 = readarr[0]; id2 = readarr[1];
- msg_cdbg("%s: id1 0x%x, id2 0x%x\n", __func__, id1, id2); - - if (id1 == chip->manufacture_id && id2 == chip->model_id) - 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. */ - if (GENERIC_MANUF_ID == chip->manufacture_id && id1 != 0xff && id1 != 0x00) - return 1; - - return 0; + return compare_id(chip, id1, id2); }
int probe_spi_res1(struct flashctx *flash)
-
Angel Pons (Code Review) -
Edward O'Callaghan (Code Review)
-
Stefan Reinauer (Code Review)