May 2022 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: ichspi.c: Implement read_write_status for wp
by Edward O'Callaghan (Code Review) 29 May '22

29 May '22

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev. Hello build bot (Jenkins), Subrata Banik, Nikolai Artemiev, Sergii Dmytruk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/64540 to look at the new patch set (#5). Change subject: ichspi.c: Implement read_write_status for wp ...................................................................... ichspi.c: Implement read_write_status for wp The ichspi hwseq path has a opaque master specialisation that allows for reading and writing STATUS1 registers. Hook the callbacks with a implementation to allow for this so that writeprotect maybe supported though this path. Change-Id: I7ecbe8491ecea3697922c91af26ca62276e86317 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M ichspi.c 1 file changed, 75 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/64540/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/64540 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7ecbe8491ecea3697922c91af26ca62276e86317 Gerrit-Change-Number: 64540 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: writeprotect: Add line-break after each `spew` message
by Thomas Heijligen (Code Review) 28 May '22

28 May '22

Attention is currently required from: Nico Huber, Nikolai Artemiev, Sergii Dmytruk. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64745 ) Change subject: writeprotect: Add line-break after each `spew` message ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/64745 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3131ff0e3fa4f9e949ce2e8d2d0a9c862a15e1cd Gerrit-Change-Number: 64745 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Sat, 28 May 2022 19:58:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: spi25_statusreg: Allow WRSR_EXT for Status Register 3
by Nico Huber (Code Review) 28 May '22

28 May '22

Attention is currently required from: Nico Huber, Thomas Heijligen, Nikolai Artemiev, Sergii Dmytruk. Hello build bot (Jenkins), Thomas Heijligen, Nikolai Artemiev, Sergii Dmytruk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/64746 to look at the new patch set (#2). Change subject: spi25_statusreg: Allow WRSR_EXT for Status Register 3 ...................................................................... spi25_statusreg: Allow WRSR_EXT for Status Register 3 Spansion flash chips S25FL128L and S25FL256L use the WRSR instruction to write more than 2 registers. So align SR2 and SR3 support: The current FEATURE_WRSR_EXT is renamed to FEATURE_WRSR_EXT2 and FEATURE_WRSR_EXT3 is added. Also, WRSR3 needs a separate flag now. Verified that FEATURE_WRSR_EXT2 still works using the `dummy_flasher`. Signed-off-by: Nico Huber <nico.h(a)gmx.de> Change-Id: Ibdfc6eb3d2cfecbf8da0493d067031ddb079a094 --- M dummyflasher.c M flashchips.c M include/flash.h M include/spi.h M spi25_statusreg.c M tests/chip_wp.c 6 files changed, 83 insertions(+), 38 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/46/64746/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/64746 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibdfc6eb3d2cfecbf8da0493d067031ddb079a094 Gerrit-Change-Number: 64746 Gerrit-PatchSet: 2 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: dummyflasher: Add emulation for S25FL128L
by Sergii Dmytruk (Code Review) 28 May '22

28 May '22

Attention is currently required from: Nico Huber, Thomas Heijligen, Anastasia Klimchuk, Nikolai Artemiev. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64748 ) Change subject: dummyflasher: Add emulation for S25FL128L ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: The new chip wasn't added to the list of emulated chips in flashrom.8.tmpl -- To view, visit https://review.coreboot.org/c/flashrom/+/64748 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic3cbea87218c973331b9b83e809e7d438407bc13 Gerrit-Change-Number: 64748 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sat, 28 May 2022 16:17:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: dummyflasher: Add emulation for S25FL128L
by Nico Huber (Code Review) 28 May '22

28 May '22

Attention is currently required from: Thomas Heijligen, Nikolai Artemiev, Sergii Dmytruk. Hello Thomas Heijligen, Nikolai Artemiev, Sergii Dmytruk, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/64748 to review the following change. Change subject: dummyflasher: Add emulation for S25FL128L ...................................................................... dummyflasher: Add emulation for S25FL128L Used to test WRSR_EXT2/3 support. Signed-off-by: Nico Huber <nico.h(a)gmx.de> Change-Id: Ic3cbea87218c973331b9b83e809e7d438407bc13 --- M dummyflasher.c 1 file changed, 56 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/48/64748/1 diff --git a/dummyflasher.c b/dummyflasher.c index 22b46b1..c345d5d 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -35,6 +35,7 @@ EMULATE_SST_SST25VF032B, EMULATE_MACRONIX_MX25L6436, EMULATE_WINBOND_W25Q128FV, + EMULATE_SPANSION_S25FL128L, EMULATE_VARIABLE_SIZE, }; @@ -201,12 +202,36 @@ } } + if (data->emu_chip == EMULATE_SPANSION_S25FL128L) { + const bool srp0 = (data->emu_status[0] >> 7); + const bool srp1 = (data->emu_status[1] & 1); + + const bool wp_active = (srp1 || (srp0 && data->hwwp)); + + if (wp_active) { + ro_bits = 0xff; + } else if (reg == STATUS2) { + /* SUS (bit_7) */ + ro_bits = 0x80; + /* Once any of the lock bits (LB[0..3]) are set, they + can't be unset. */ + ro_bits |= data->emu_status[1] & (1 << 2); + ro_bits |= data->emu_status[1] & (1 << 3); + ro_bits |= data->emu_status[1] & (1 << 4); + ro_bits |= data->emu_status[1] & (1 << 5); + } else if (reg == STATUS3) { + /* Two reserved bits. */ + ro_bits = 0x11; + } + } + return ro_bits; } static void update_write_protection(struct emu_data *data) { - if (data->emu_chip != EMULATE_WINBOND_W25Q128FV) + if (data->emu_chip != EMULATE_WINBOND_W25Q128FV && + data->emu_chip != EMULATE_SPANSION_S25FL128L) return; const struct wp_bits bits = { @@ -353,6 +378,12 @@ if (readcnt > 0) memset(readarr, 0x17, readcnt); break; + case EMULATE_SPANSION_S25FL128L: + if (readcnt > 0) + readarr[0] = 0x60; + if (readcnt > 1) + readarr[1] = 0x18; + break; default: /* ignore */ break; } @@ -410,6 +441,14 @@ if (readcnt > 2) readarr[2] = 0x18; break; + case EMULATE_SPANSION_S25FL128L: + if (readcnt > 0) + readarr[0] = 0x01; + if (readcnt > 1) + readarr[1] = 0x60; + if (readcnt > 2) + readarr[2] = 0x18; + break; case EMULATE_VARIABLE_SIZE: if (readcnt > 0) readarr[0] = (PROGMANUF_ID >> 8) & 0xff; @@ -764,6 +803,7 @@ case EMULATE_SST_SST25VF032B: case EMULATE_MACRONIX_MX25L6436: case EMULATE_WINBOND_W25Q128FV: + case EMULATE_SPANSION_S25FL128L: case EMULATE_VARIABLE_SIZE: if (emulate_spi_chip_response(writecnt, readcnt, writearr, readarr, emu_data)) { @@ -1094,6 +1134,21 @@ data->emu_jedec_ce_c7_size = data->emu_chip_size; msg_pdbg("Emulating Winbond W25Q128FV SPI flash chip (RDID)\n"); } + if (!strcmp(tmp, "S25FL128L")) { + data->emu_chip = EMULATE_SPANSION_S25FL128L; + data->emu_wrsr_ext2 = true; + data->emu_wrsr_ext3 = true; + data->emu_chip_size = 16 * 1024 * 1024; + data->emu_max_byteprogram_size = 256; + data->emu_max_aai_size = 0; + data->emu_status_len = 3; + data->emu_jedec_se_size = 4 * 1024; + data->emu_jedec_be_52_size = 32 * 1024; + data->emu_jedec_be_d8_size = 64 * 1024; + data->emu_jedec_ce_60_size = data->emu_chip_size; + data->emu_jedec_ce_c7_size = data->emu_chip_size; + msg_pdbg("Emulating Spansion S25FL128L SPI flash chip (RES, RDID, WP)\n"); + } /* The name of variable-size virtual chip. A 4 MiB flash example: * flashrom -p dummy:emulate=VARIABLE_SIZE,size=4194304 -- To view, visit https://review.coreboot.org/c/flashrom/+/64748 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic3cbea87218c973331b9b83e809e7d438407bc13 Gerrit-Change-Number: 64748 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: flashchips: Add Spansion (today Infineon) S25FL128L and S25FL256L
by Nico Huber (Code Review) 28 May '22

28 May '22

Attention is currently required from: Thomas Heijligen, Nikolai Artemiev, Sergii Dmytruk. Hello Thomas Heijligen, Nikolai Artemiev, Sergii Dmytruk, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/64747 to review the following change. Change subject: flashchips: Add Spansion (today Infineon) S25FL128L and S25FL256L ...................................................................... flashchips: Add Spansion (today Infineon) S25FL128L and S25FL256L These chips seem to be rather regular, supporting 2.7V..3.6V, the common erase block sizes 4KiB, 32KiB, 64KiB and the usual block- protection bits. Status/configuration register naming differs from other vendors, though. These chips have 2 status registers plus 3 configuration registers. Configuration registers 1 & 2 match status registers 2 & 3 of what we are used from other vendors. Read opcodes match too, however writes are always done through the WRSR instruction which can write up to 4 bytes (SR1, CR1, CR2, CR3). S25FL256L supports native 4BA commands and entering a 4BA mode. However, it uses an unusual opcode (0x53) for the 32KiB 4BA block erase. Signed-off-by: Nico Huber <nico.h(a)gmx.de> Change-Id: I356df6649f29e50879a4da4183f1164a81cb0a09 --- M flashchips.c M include/chipdrivers.h M include/flashchips.h M spi25.c 4 files changed, 133 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/64747/1 diff --git a/flashchips.c b/flashchips.c index 363a51e..3d85135 100644 --- a/flashchips.c +++ b/flashchips.c @@ -16184,6 +16184,63 @@ { .vendor = "Spansion", + .name = "S25FL128L", + .bustype = BUS_SPI, + .manufacture_id = SPANSION_ID, + .model_id = SPANSION_S25FL128L, + .total_size = 16384, + .page_size = 256, + /* 4 x 256B Security Region (OTP) */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_WRSR_EXT3 | FEATURE_OTP, + .tested = TEST_UNTESTED, + .probe = probe_spi_rdid, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 4096} }, + .block_erase = spi_block_erase_20, + }, { + .eraseblocks = { {32 * 1024, 512} }, + .block_erase = spi_block_erase_52, + }, { + .eraseblocks = { {64 * 1024, 256} }, + .block_erase = spi_block_erase_d8, + }, { + .eraseblocks = { {16384 * 1024, 1} }, + .block_erase = spi_block_erase_60, + }, { + .eraseblocks = { {16384 * 1024, 1} }, + .block_erase = spi_block_erase_c7, + } + }, + .printlock = spi_prettyprint_status_register_bp2_srwd, + .unlock = spi_disable_blockprotect_bp2_srwd, + .write = spi_chip_write_256, + .read = spi_chip_read, /* Fast read (0x0B) supported */ + .voltage = {2700, 3600}, + .reg_bits = + { + /* + * Note: This chip has a read-only Status Register 2 that is not + * counted here. Registers are mapped as follows: + * STATUS1 ... Status Register 1 + * STATUS2 ... Configuration Register 1 + * STATUS3 ... Configuration Register 2 + */ + .srp = {STATUS1, 7, RW}, + .srl = {STATUS2, 0, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}}, + .tb = {STATUS1, 5, RW}, + .sec = {STATUS1, 6, RW}, + .cmp = {STATUS2, 6, RW}, + .wps = {STATUS3, 2, RW}, + }, + .decode_range = decode_range_spi25, + }, + + { + .vendor = "Spansion", .name = "S25FL128P......0", /* uniform 64 kB sectors */ .bustype = BUS_SPI, .manufacture_id = SPANSION_ID, @@ -16611,6 +16668,72 @@ { .vendor = "Spansion", + .name = "S25FL256L", + .bustype = BUS_SPI, + .manufacture_id = SPANSION_ID, + .model_id = SPANSION_S25FL256L, + .total_size = 32768, + .page_size = 256, + /* 4 x 256B Security Region (OTP) */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_WRSR_EXT3 | FEATURE_OTP | + FEATURE_4BA_ENTER | FEATURE_4BA_NATIVE, + .tested = TEST_UNTESTED, + .probe = probe_spi_rdid, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 8192} }, + .block_erase = spi_block_erase_21, + }, { + .eraseblocks = { {4 * 1024, 8192} }, + .block_erase = spi_block_erase_20, + }, { + .eraseblocks = { {32 * 1024, 1024} }, + .block_erase = spi_block_erase_53, + }, { + .eraseblocks = { {32 * 1024, 1024} }, + .block_erase = spi_block_erase_52, + }, { + .eraseblocks = { {64 * 1024, 512} }, + .block_erase = spi_block_erase_dc, + }, { + .eraseblocks = { {64 * 1024, 512} }, + .block_erase = spi_block_erase_d8, + }, { + .eraseblocks = { {32768 * 1024, 1} }, + .block_erase = spi_block_erase_60, + }, { + .eraseblocks = { {32768 * 1024, 1} }, + .block_erase = spi_block_erase_c7, + } + }, + .printlock = spi_prettyprint_status_register_bp3_srwd, + .unlock = spi_disable_blockprotect_bp3_srwd, + .write = spi_chip_write_256, + .read = spi_chip_read, /* Fast read (0x0B) supported */ + .voltage = {2700, 3600}, + .reg_bits = + { + /* + * Note: This chip has a read-only Status Register 2 that is not + * counted here. Registers are mapped as follows: + * STATUS1 ... Status Register 1 + * STATUS2 ... Configuration Register 1 + * STATUS3 ... Configuration Register 2 + */ + .srp = {STATUS1, 7, RW}, + .srl = {STATUS2, 0, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}, {STATUS1, 5, RW}}, + .tb = {STATUS1, 6, RW}, + .cmp = {STATUS2, 6, RW}, + .wps = {STATUS3, 2, RW}, + }, + .decode_range = decode_range_spi25, + }, + + { + .vendor = "Spansion", .name = "S25FL256S Large Sectors", .bustype = BUS_SPI, .manufacture_id = SPANSION_ID, diff --git a/include/chipdrivers.h b/include/chipdrivers.h index 0695993..f6724b8 100644 --- a/include/chipdrivers.h +++ b/include/chipdrivers.h @@ -41,6 +41,7 @@ int spi_block_erase_21(struct flashctx *flash, unsigned int addr, unsigned int blocklen); int spi_block_erase_50(struct flashctx *flash, unsigned int addr, unsigned int blocklen); int spi_block_erase_52(struct flashctx *flash, unsigned int addr, unsigned int blocklen); +int spi_block_erase_53(struct flashctx *flash, unsigned int addr, unsigned int blocklen); int spi_block_erase_5c(struct flashctx *flash, unsigned int addr, unsigned int blocklen); int spi_block_erase_60(struct flashctx *flash, unsigned int addr, unsigned int blocklen); int spi_block_erase_62(struct flashctx *flash, unsigned int addr, unsigned int blocklen); diff --git a/include/flashchips.h b/include/flashchips.h index ba97719..71e8cee 100644 --- a/include/flashchips.h +++ b/include/flashchips.h @@ -668,6 +668,8 @@ #define SPANSION_S25FL116K 0x4015 #define SPANSION_S25FL132K 0x4016 #define SPANSION_S25FL164K 0x4017 +#define SPANSION_S25FL128L 0x6018 +#define SPANSION_S25FL256L 0x6019 #define SPANSION_S25FS128S_L 0x20180081 /* Large sectors. */ #define SPANSION_S25FS128S_S 0x20180181 /* Small sectors. */ #define SPANSION_S25FS256S_L 0x02190081 /* Large sectors. */ diff --git a/spi25.c b/spi25.c index 39a8876..0e348b2 100644 --- a/spi25.c +++ b/spi25.c @@ -589,6 +589,13 @@ } /* Erase 32 KB of flash with 4-bytes address from ANY mode (3-bytes or 4-bytes) */ +int spi_block_erase_53(struct flashctx *flash, unsigned int addr, unsigned int blocklen) +{ + /* This usually takes 100-4000ms, so wait in 100ms steps. */ + return spi_write_cmd(flash, 0x53, true, addr, NULL, 0, 100 * 1000); +} + +/* Erase 32 KB of flash with 4-bytes address from ANY mode (3-bytes or 4-bytes) */ int spi_block_erase_5c(struct flashctx *flash, unsigned int addr, unsigned int blocklen) { /* This usually takes 100-4000ms, so wait in 100ms steps. */ -- To view, visit https://review.coreboot.org/c/flashrom/+/64747 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I356df6649f29e50879a4da4183f1164a81cb0a09 Gerrit-Change-Number: 64747 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: spi25_statusreg: Allow WRSR_EXT for Status Register 3
by Nico Huber (Code Review) 28 May '22

28 May '22

Attention is currently required from: Thomas Heijligen, Nikolai Artemiev, Sergii Dmytruk. Hello Thomas Heijligen, Nikolai Artemiev, Sergii Dmytruk, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/64746 to review the following change. Change subject: spi25_statusreg: Allow WRSR_EXT for Status Register 3 ...................................................................... spi25_statusreg: Allow WRSR_EXT for Status Register 3 Spansion flash chips S25FL128L and S25FL256L use the WRSR instruction to write more than 2 registers. So align SR2 and SR3 support: The current FEATURE_WRSR_EXT is renamed to FEATURE_WRSR_EXT2 and FEATURE_WRSR_EXT3 is added. Also, WRSR3 needs a separate flag now. Verified that FEATURE_WRSR_EXT2 still works using the `dummy_flasher`. Signed-off-by: Nico Huber <nico.h(a)gmx.de> Change-Id: Ibdfc6eb3d2cfecbf8da0493d067031ddb079a094 --- M dummyflasher.c M flashchips.c M include/flash.h M include/spi.h M spi25_statusreg.c 5 files changed, 82 insertions(+), 37 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/46/64746/1 diff --git a/dummyflasher.c b/dummyflasher.c index 50a84d4..22b46b1 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -44,8 +44,9 @@ unsigned int emu_chip_size; /* Note: W25Q128FV doesn't change value of SR2 if it's not provided, but * even its previous generations do, so don't forget to update - * WRSR code on enabling WRSR_EXT for more chips. */ - bool emu_wrsr_ext; + * WRSR code on enabling WRSR_EXT2 for more chips. */ + bool emu_wrsr_ext2; + bool emu_wrsr_ext3; int erase_to_zero; int emu_modified; /* is the image modified since reading it? */ uint8_t emu_status[3]; @@ -284,7 +285,7 @@ { unsigned int offs, i, toread; uint8_t ro_bits; - bool wrsr_ext; + bool wrsr_ext2, wrsr_ext3; static int unsigned aai_offs; const unsigned char sst25vf040_rems_response[2] = {0xbf, 0x44}; const unsigned char sst25vf032b_rems_response[2] = {0xbf, 0x4a}; @@ -445,20 +446,28 @@ break; } - wrsr_ext = (writecnt == 3 && data->emu_wrsr_ext); + wrsr_ext2 = (writecnt == 3 && data->emu_wrsr_ext2); + wrsr_ext3 = (writecnt == 4 && data->emu_wrsr_ext3); /* FIXME: add some reasonable simulation of the busy flag */ ro_bits = get_reg_ro_bit_mask(data, STATUS1); data->emu_status[0] &= ro_bits; data->emu_status[0] |= writearr[1] & ~ro_bits; - if (wrsr_ext) { + if (wrsr_ext2 || wrsr_ext3) { ro_bits = get_reg_ro_bit_mask(data, STATUS2); data->emu_status[1] &= ro_bits; data->emu_status[1] |= writearr[2] & ~ro_bits; } + if (wrsr_ext3) { + ro_bits = get_reg_ro_bit_mask(data, STATUS3); + data->emu_status[2] &= ro_bits; + data->emu_status[2] |= writearr[3] & ~ro_bits; + } - if (wrsr_ext) + if (wrsr_ext3) + msg_pdbg2("WRSR wrote 0x%02x%02x%02x.\n", data->emu_status[2], data->emu_status[1], data->emu_status[0]); + else if (wrsr_ext2) msg_pdbg2("WRSR wrote 0x%02x%02x.\n", data->emu_status[1], data->emu_status[0]); else msg_pdbg2("WRSR wrote 0x%02x.\n", data->emu_status[0]); @@ -1073,7 +1082,7 @@ } if (!strcmp(tmp, "W25Q128FV")) { data->emu_chip = EMULATE_WINBOND_W25Q128FV; - data->emu_wrsr_ext = true; + data->emu_wrsr_ext2 = true; data->emu_chip_size = 16 * 1024 * 1024; data->emu_max_byteprogram_size = 256; data->emu_max_aai_size = 0; diff --git a/flashchips.c b/flashchips.c index a9fd1bf..363a51e 100644 --- a/flashchips.c +++ b/flashchips.c @@ -6334,7 +6334,7 @@ .total_size = 16384, .page_size = 256, /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44 */ - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_WRSR_EXT, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_WRSR_EXT2, .tested = TEST_OK_PREW, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, @@ -6500,7 +6500,7 @@ .total_size = 8192, .page_size = 256, /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44 */ - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_WRSR_EXT, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_WRSR_EXT2, .tested = TEST_OK_PREW, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, @@ -6754,7 +6754,7 @@ .total_size = 32768, .page_size = 256, .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_WREN | - FEATURE_WRSR_EXT | FEATURE_WRSR2, + FEATURE_WRSR_EXT2 | FEATURE_WRSR2 | FEATURE_WRSR3, .tested = TEST_OK_PREW, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, @@ -17181,7 +17181,8 @@ .page_size = 256, /* supports SFDP */ /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44, read ID 0x4B */ - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_WRSR2, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | + FEATURE_WRSR_EXT2 | FEATURE_WRSR2 | FEATURE_WRSR3, .tested = TEST_OK_PREW, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, @@ -18028,7 +18029,8 @@ .page_size = 256, /* supports SFDP */ /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44, read ID 0x4B */ - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_WRSR2, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | + FEATURE_WRSR_EXT2 | FEATURE_WRSR2 | FEATURE_WRSR3, .tested = TEST_OK_PREW, .probe = probe_spi_rdid, .probe_timing = TIMING_ZERO, diff --git a/include/flash.h b/include/flash.h index 45533ba..d6766f8 100644 --- a/include/flash.h +++ b/include/flash.h @@ -141,8 +141,10 @@ #define FEATURE_ERASED_ZERO (1 << 17) #define FEATURE_NO_ERASE (1 << 18) -#define FEATURE_WRSR_EXT (1 << 19) +#define FEATURE_WRSR_EXT2 (1 << 19) #define FEATURE_WRSR2 (1 << 20) +#define FEATURE_WRSR_EXT3 ((1 << 21) | FEATURE_WRSR_EXT2) +#define FEATURE_WRSR3 (1 << 22) #define ERASED_VALUE(flash) (((flash)->chip->feature_bits & FEATURE_ERASED_ZERO) ? 0x00 : 0xff) diff --git a/include/spi.h b/include/spi.h index 14f71aa..05d2239 100644 --- a/include/spi.h +++ b/include/spi.h @@ -156,7 +156,6 @@ #define JEDEC_WRSR 0x01 #define JEDEC_WRSR_OUTSIZE 0x02 #define JEDEC_WRSR_INSIZE 0x00 -#define JEDEC_WRSR_EXT_OUTSIZE 0x03 /* Write Status Register 2 */ #define JEDEC_WRSR2 0x31 diff --git a/spi25_statusreg.c b/spi25_statusreg.c index 0f43056..4cd19ab 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -22,11 +22,44 @@ #include "spi.h" /* === Generic functions === */ + +/* + * Writing SR2 or higher with an extended WRSR command requires + * writing all lower SRx along with it, so just read the lower + * SRx and write them back. + */ +static int spi_prepare_wrsr_ext( + uint8_t write_cmd[4], size_t *const write_cmd_len, + const struct flashctx *const flash, + const enum flash_reg reg, const uint8_t value) +{ + enum flash_reg reg_it; + size_t i = 0; + + write_cmd[i++] = JEDEC_WRSR; + + for (reg_it = STATUS1; reg_it < reg; ++reg_it) { + uint8_t sr; + + if (spi_read_register(flash, reg_it, &sr)) { + msg_cerr("Writing SR%d failed: failed to read SR%d for writeback.\n", + reg - STATUS1 + 1, reg_it - STATUS1 + 1); + return 1; + } + write_cmd[i++] = sr; + } + + write_cmd[i++] = value; + *write_cmd_len = i; + + return 0; +} + int spi_write_register(const struct flashctx *flash, enum flash_reg reg, uint8_t value) { int feature_bits = flash->chip->feature_bits; - uint8_t write_cmd[3]; + uint8_t write_cmd[4]; size_t write_cmd_len = 0; /* @@ -46,31 +79,27 @@ write_cmd_len = JEDEC_WRSR2_OUTSIZE; break; } - if (feature_bits & FEATURE_WRSR_EXT) { - /* - * Writing SR2 with an extended WRSR command requires - * writing SR1 along with SR2, so just read SR1 and - * write it back - */ - uint8_t sr1; - - if (spi_read_register(flash, STATUS1, &sr1)) { - msg_cerr("Writing SR2 failed: failed to read SR1 for writeback.\n"); + if (feature_bits & FEATURE_WRSR_EXT2) { + if (spi_prepare_wrsr_ext(write_cmd, &write_cmd_len, flash, reg, value)) return 1; - } - write_cmd[0] = JEDEC_WRSR; - write_cmd[1] = sr1; - write_cmd[2] = value; - write_cmd_len = JEDEC_WRSR_EXT_OUTSIZE; break; } msg_cerr("Cannot write SR2: unsupported by chip\n"); return 1; case STATUS3: - write_cmd[0] = JEDEC_WRSR3; - write_cmd[1] = value; - write_cmd_len = JEDEC_WRSR3_OUTSIZE; - break; + if (feature_bits & FEATURE_WRSR3) { + write_cmd[0] = JEDEC_WRSR3; + write_cmd[1] = value; + write_cmd_len = JEDEC_WRSR3_OUTSIZE; + break; + } + if (feature_bits & FEATURE_WRSR_EXT3) { + if (spi_prepare_wrsr_ext(write_cmd, &write_cmd_len, flash, reg, value)) + return 1; + break; + } + msg_cerr("Cannot write SR3: unsupported by chip\n"); + return 1; default: msg_cerr("Cannot write register: unknown register\n"); return 1; @@ -152,15 +181,19 @@ read_cmd = JEDEC_RDSR; break; case STATUS2: - if (feature_bits & (FEATURE_WRSR_EXT | FEATURE_WRSR2)) { + if (feature_bits & (FEATURE_WRSR_EXT2 | FEATURE_WRSR2)) { read_cmd = JEDEC_RDSR2; break; } msg_cerr("Cannot read SR2: unsupported by chip\n"); return 1; case STATUS3: - read_cmd = JEDEC_RDSR3; - break; + if (feature_bits & (FEATURE_WRSR_EXT3 | FEATURE_WRSR3)) { + read_cmd = JEDEC_RDSR3; + break; + } + msg_cerr("Cannot read SR3: unsupported by chip\n"); + return 1; default: msg_cerr("Cannot read register: unknown register\n"); return 1; -- To view, visit https://review.coreboot.org/c/flashrom/+/64746 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibdfc6eb3d2cfecbf8da0493d067031ddb079a094 Gerrit-Change-Number: 64746 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: writeprotect: Add line-break after each `spew` message
by Nico Huber (Code Review) 28 May '22

28 May '22

Attention is currently required from: Thomas Heijligen, Nikolai Artemiev, Sergii Dmytruk. Hello Thomas Heijligen, Nikolai Artemiev, Sergii Dmytruk, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/64745 to review the following change. Change subject: writeprotect: Add line-break after each `spew` message ...................................................................... writeprotect: Add line-break after each `spew` message Signed-off-by: Nico Huber <nico.h(a)gmx.de> Change-Id: I3131ff0e3fa4f9e949ce2e8d2d0a9c862a15e1cd --- M writeprotect.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/45/64745/1 diff --git a/writeprotect.c b/writeprotect.c index cfdd15b..60d8ead 100644 --- a/writeprotect.c +++ b/writeprotect.c @@ -289,7 +289,7 @@ size_t j = bits.bp_bit_count - i - 1; msg_gspew("BP%zu=%u ", j, bits.bp[j]); } - msg_gspew(" start=0x%08zx length=0x%08zx ", + msg_gspew(" start=0x%08zx length=0x%08zx\n", output->range.start, output->range.len); } -- To view, visit https://review.coreboot.org/c/flashrom/+/64745 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3131ff0e3fa4f9e949ce2e8d2d0a9c862a15e1cd Gerrit-Change-Number: 64745 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: flashchips,spi25: Replace `.wrea_override` with FEATURE_4BA_EAR_1716
by Nico Huber (Code Review) 28 May '22

28 May '22

Attention is currently required from: Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64636 ) Change subject: flashchips,spi25: Replace `.wrea_override` with FEATURE_4BA_EAR_1716 ...................................................................... Patch Set 1: (1 comment) File include/flash.h: https://review.coreboot.org/c/flashrom/+/64636/comment/98b35de9_7869416f PS1, Line 144: #define FEATURE_ERASED_ZERO (1 << 17) : #define FEATURE_NO_ERASE (1 << 18) : : #define FEATURE_WRSR_EXT (1 << 19) : #define FEATURE_WRSR2 (1 << 20) Need update... -- To view, visit https://review.coreboot.org/c/flashrom/+/64636 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6d82f24898acd0789203516a7456fd785907bc10 Gerrit-Change-Number: 64636 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sat, 28 May 2022 13:27:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom: Allow getting the progress_state from the flashctx
by Nico Huber (Code Review) 27 May '22

27 May '22

Attention is currently required from: Felix Singer, Richard Hughes. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64663 ) Change subject: libflashrom: Allow getting the progress_state from the flashctx ...................................................................... Patch Set 2: (3 comments) Patchset: PS2: Felix, thanks for the invitation. Not sure what to do, though, as I don't understand the API. Richard, have you tested if the current implementation works? I'm a bit confused about when update_progress() should get called. Does it do any- thing on a modern Intel system at all? There are odd paths like spi_chip_read() calls into the master, which may call spi_read_chunked() which calls update_progress(). But spi_chip_read() calls it as well? I wonder what was the reason to do it at the chip and programmer levels. File include/libflashrom.h: https://review.coreboot.org/c/flashrom/+/64663/comment/d62c7ca9_49c76e5b PS2, Line 85: typedef void(flashrom_progress_callback)(struct flashrom_flashctx *flashctx); Why doesn't it get the progress passed directly? https://review.coreboot.org/c/flashrom/+/64663/comment/f1838035_d834604b PS2, Line 95: * callback. The library user passes the progress state? -- To view, visit https://review.coreboot.org/c/flashrom/+/64663 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I322bf56ff92f7b4d0ffc92768e9f0cdf7cb82010 Gerrit-Change-Number: 64663 Gerrit-PatchSet: 2 Gerrit-Owner: Richard Hughes <richard(a)hughsie.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Richard Hughes <richard(a)hughsie.com> Gerrit-Comment-Date: Fri, 27 May 2022 12:26:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2022