Aaron Durbin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37959 )
Change subject: drivers/spi/sst: remove unnecessary byte programming ......................................................................
drivers/spi/sst: remove unnecessary byte programming
The SST25VF064C supports page programming mode like other spi flash parts in that there isn't an offset requirement. Remove this check and single byte program because CMD_SST_BP (0x2) is the same as page programming command. Lastly, for clariy purposes provide a CMD_SST_PP to explicitly indicate page programming despite the values (0x2) being the same as byte programming for the other parts.
Change-Id: I84eea0b044ccac6c6f26ea4cb42f4c13cf8f5173 Signed-off-by: Aaron Durbin adurbin@chromium.org --- M src/drivers/spi/sst.c 1 file changed, 2 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/37959/1
diff --git a/src/drivers/spi/sst.c b/src/drivers/spi/sst.c index 6223cf9..631170b 100644 --- a/src/drivers/spi/sst.c +++ b/src/drivers/spi/sst.c @@ -32,6 +32,7 @@ #define CMD_SST_READ 0x03 /* Read Data Bytes */ #define CMD_SST_FAST_READ 0x0b /* Read Data Bytes at Higher Speed */ #define CMD_SST_BP 0x02 /* Byte Program */ +#define CMD_SST_PP 0x02 /* Page Program */ #define CMD_SST_AAI_WP 0xAD /* Auto Address Increment Word Program */ #define CMD_SST_SE 0x20 /* Sector Erase */
@@ -197,30 +198,12 @@
page_size = 256;
- /* If the data is not word aligned, write out leading single byte */ - actual = offset % 2; - if (actual) { - ret = sst_byte_write(flash, offset, buf); - if (ret) - goto done; - } - offset += actual; - - ret = sst_enable_writing(flash); - if (ret) - goto done; - - cmd[0] = CMD_SST_AAI_WP; - cmd[1] = offset >> 16; - cmd[2] = offset >> 8; - cmd[3] = offset; - for (actual = 0; actual < len; actual += chunk_len) { byte_addr = offset % page_size; chunk_len = MIN(len - actual, page_size - byte_addr); chunk_len = spi_crop_chunk(&flash->spi, sizeof(cmd), chunk_len);
- cmd[0] = CMD_SST_BP; + cmd[0] = CMD_SST_PP; cmd[1] = (offset >> 16) & 0xff; cmd[2] = (offset >> 8) & 0xff; cmd[3] = offset & 0xff;
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37959
to look at the new patch set (#2).
Change subject: drivers/spi/sst: remove unnecessary byte programming ......................................................................
drivers/spi/sst: remove unnecessary byte programming
The SST25VF064C supports page programming mode like other spi flash parts in that there isn't an offset requirement. Remove this check and single byte program because CMD_SST_BP (0x2) is the same as page programming command. Lastly, for clariy purposes provide a CMD_SST_PP to explicitly indicate page programming despite the values (0x2) being the same as byte programming for the other parts.
Change-Id: I84eea0b044ccac6c6f26ea4cb42f4c13cf8f5173 Signed-off-by: Aaron Durbin adurbin@chromium.org --- M src/drivers/spi/sst.c 1 file changed, 2 insertions(+), 20 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/37959/2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37959 )
Change subject: drivers/spi/sst: remove unnecessary byte programming ......................................................................
Patch Set 3: Code-Review+1
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37959 )
Change subject: drivers/spi/sst: remove unnecessary byte programming ......................................................................
Patch Set 3: Code-Review+2
Hello Marshall Dawson, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37959
to look at the new patch set (#4).
Change subject: drivers/spi/sst: remove unnecessary byte programming ......................................................................
drivers/spi/sst: remove unnecessary byte programming
The SST25VF064C supports page programming mode like other spi flash parts in that there isn't an offset requirement. Remove this check and single byte program because CMD_SST_BP (0x2) is the same as page programming command. Lastly, for clariy purposes provide a CMD_SST_PP to explicitly indicate page programming despite the values (0x2) being the same as byte programming for the other parts.
Change-Id: I84eea0b044ccac6c6f26ea4cb42f4c13cf8f5173 Signed-off-by: Aaron Durbin adurbin@chromium.org --- M src/drivers/spi/sst.c 1 file changed, 2 insertions(+), 20 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/37959/4
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37959 )
Change subject: drivers/spi/sst: remove unnecessary byte programming ......................................................................
drivers/spi/sst: remove unnecessary byte programming
The SST25VF064C supports page programming mode like other spi flash parts in that there isn't an offset requirement. Remove this check and single byte program because CMD_SST_BP (0x2) is the same as page programming command. Lastly, for clariy purposes provide a CMD_SST_PP to explicitly indicate page programming despite the values (0x2) being the same as byte programming for the other parts.
Change-Id: I84eea0b044ccac6c6f26ea4cb42f4c13cf8f5173 Signed-off-by: Aaron Durbin adurbin@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/37959 Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Marshall Dawson marshalldawson3rd@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/spi/sst.c 1 file changed, 2 insertions(+), 20 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Marshall Dawson: Looks good to me, approved
diff --git a/src/drivers/spi/sst.c b/src/drivers/spi/sst.c index 6223cf9..499db35 100644 --- a/src/drivers/spi/sst.c +++ b/src/drivers/spi/sst.c @@ -32,6 +32,7 @@ #define CMD_SST_READ 0x03 /* Read Data Bytes */ #define CMD_SST_FAST_READ 0x0b /* Read Data Bytes at Higher Speed */ #define CMD_SST_BP 0x02 /* Byte Program */ +#define CMD_SST_PP 0x02 /* Page Program */ #define CMD_SST_AAI_WP 0xAD /* Auto Address Increment Word Program */ #define CMD_SST_SE 0x20 /* Sector Erase */
@@ -197,30 +198,12 @@
page_size = 256;
- /* If the data is not word aligned, write out leading single byte */ - actual = offset % 2; - if (actual) { - ret = sst_byte_write(flash, offset, buf); - if (ret) - goto done; - } - offset += actual; - - ret = sst_enable_writing(flash); - if (ret) - goto done; - - cmd[0] = CMD_SST_AAI_WP; - cmd[1] = offset >> 16; - cmd[2] = offset >> 8; - cmd[3] = offset; - for (actual = 0; actual < len; actual += chunk_len) { byte_addr = offset % page_size; chunk_len = MIN(len - actual, page_size - byte_addr); chunk_len = spi_crop_chunk(&flash->spi, sizeof(cmd), chunk_len);
- cmd[0] = CMD_SST_BP; + cmd[0] = CMD_SST_PP; cmd[1] = (offset >> 16) & 0xff; cmd[2] = (offset >> 8) & 0xff; cmd[3] = offset & 0xff; @@ -251,7 +234,6 @@ offset += chunk_len; }
-done: #if CONFIG(DEBUG_SPI_FLASH) printk(BIOS_SPEW, "SF: SST: program %s %zu bytes @ 0x%lx\n", ret ? "failure" : "success", len, (unsigned long)offset - actual);
-
Aaron Durbin (Code Review) -
Marshall Dawson (Code Review)
-
Paul Menzel (Code Review)