Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/33034
Change subject: [WIP]spi25.c: Implement fast read ......................................................................
[WIP]spi25.c: Implement fast read
Use the fast read OP by default and fall back to regular read if that failed.
Change-Id: I96393e1beea0164daa8e366a1d09a573f2c6d16f Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M ichspi.c M spi.h M spi25.c 3 files changed, 23 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/34/33034/1
diff --git a/ichspi.c b/ichspi.c index 0f1470d..54cb1f9 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1117,14 +1117,14 @@ opcode = &(curopcodes->opcode[opcode_index]);
/* The following valid writecnt/readcnt combinations exist: - * writecnt = 4, readcnt >= 0 + * writecnt = 4 or 5, readcnt >= 0 (The dummy bit on fast reads gets discarded) * writecnt = 1, readcnt >= 0 * writecnt >= 4, readcnt = 0 * writecnt >= 1, readcnt = 0 * writecnt >= 1 is guaranteed for all commands. */ if ((opcode->spi_type == SPI_OPCODE_TYPE_READ_WITH_ADDRESS) && - (writecnt != 4)) { + ((writecnt != 4) && (writecnt != 5))) { msg_perr("%s: Internal command size error for opcode " "0x%02x, got writecnt=%i, want =4\n", __func__, cmd, writecnt); diff --git a/spi.h b/spi.h index 0073c71..65dc033 100644 --- a/spi.h +++ b/spi.h @@ -149,6 +149,7 @@ /* Read the memory */ #define JEDEC_READ 0x03 #define JEDEC_READ_OUTSIZE 0x04 +#define JEDEC_FAST_READ 0x0b /* JEDEC_READ_INSIZE : any length */
/* Read the memory (with delay after sending address) */ @@ -168,6 +169,7 @@ /* Read the memory with 4-byte address From ANY mode (3-bytes or 4-bytes) it works with 4-byte address */ #define JEDEC_READ_4BA 0x13 +#define JEDEC_FAST_READ_4BA 0x0c
/* Read the memory with 4-byte address (and delay after sending address) From ANY mode (3-bytes or 4-bytes) it works with 4-byte address */ diff --git a/spi25.c b/spi25.c index 8b6c462..36d2436 100644 --- a/spi25.c +++ b/spi25.c @@ -645,12 +645,30 @@ unsigned int len) { const bool native_4ba = flash->chip->feature_bits & FEATURE_4BA_READ && spi_master_4ba(flash); - uint8_t cmd[1 + JEDEC_MAX_ADDR_LEN] = { native_4ba ? JEDEC_READ_4BA : JEDEC_READ, }; + bool fast_read = true; + uint8_t cmd[2 + JEDEC_MAX_ADDR_LEN]; + + if (native_4ba) + fast_read = flash->chip->feature_bits & FEATURE_4BA_FAST_READ; + + if (fast_read) + cmd[0] = native_4ba ? JEDEC_FAST_READ_4BA : JEDEC_FAST_READ; + else + cmd[0] = native_4ba ? JEDEC_READ_4BA : JEDEC_READ;
const int addr_len = spi_prepare_address(flash, cmd, native_4ba, address); if (addr_len < 0) return 1;
+ if (fast_read) { + cmd[addr_len + 1] = 0; + int ret = spi_send_command(flash, 2 + addr_len, len, cmd, bytes); + if (!ret) + return ret; + msg_cinfo("%s: Fast Read did not succeed, falling back to regular read", __func__); + cmd[0] = native_4ba ? JEDEC_READ_4BA : JEDEC_READ; + } + /* Send Read */ return spi_send_command(flash, 1 + addr_len, len, cmd, bytes); }
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33034 )
Change subject: [WIP]spi25.c: Implement fast read ......................................................................
Patch Set 1:
Do we need a separate flag for FAST_READ support in flashchips.c?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33034 )
Change subject: [WIP]spi25.c: Implement fast read ......................................................................
Patch Set 1:
(1 comment)
Do we need a separate flag for FAST_READ support in flashchips.c?
I'd say yes, it seems unlikely that all chips support it. And even if, who'd check all the datasheets?
https://review.coreboot.org/#/c/33034/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/33034/1//COMMIT_MSG@9 PS1, Line 9: by default Please don't. It's slower for most common flashrom use cases.
I guess a good heuristic would be if we are above the flash chip's threshold for the regular read (say >30MHz if the database doesn't state otherwise). Shouldn't be hard to implement as there are few supported programmers that can go that fast.
Make it a lowest common threshold of flash and programmer to cover the `ichspi` case?
-
Arthur Heymans (Code Review) -
Nico Huber (Code Review)