November 2021 - flashrom-gerrit

Change in flashrom[master]: spi25_statusreg: make register read/write functions generic
by Nikolai Artemiev (Code Review) 22 Nov '21

22 Nov '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58475 ) Change subject: spi25_statusreg: make register read/write functions generic ...................................................................... Patch Set 11: (3 comments) File spi25_statusreg.c: https://review.coreboot.org/c/flashrom/+/58475/comment/2601bdea_b2de0314 PS9, Line 120: msg_cerr("Cannot read register: not supported by chip\n"); > Maybe I am missing something, but how nested ifs prevent using an else statement? You wrap condition […] There are multiple ways of writing SR2, so there's a nested if/else chain inside the `if (reg == STATUS2)` block. But someone may try to try to write SR2 on a chip that doesn't support it, in which case we should generate an error, even though the `if (reg == STATUS2)` block will run. File spi25_statusreg.c: https://review.coreboot.org/c/flashrom/+/58475/comment/1c5b4d1d_cccacf79 PS10, Line 119: } : else { : > I am sorry I should have noticed this earlier, but `else` (and `else if`) needs to be on the same li […] Done https://review.coreboot.org/c/flashrom/+/58475/comment/f1688d70_3528f68a PS10, Line 130: flash, sizeof(read_cmd), sizeof(readarr), : &read_cmd, readarr); > This can fit into 112 chars right? It helps to git grep when all function arguments are on one line. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/58475 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0a3951bbf993f2d8d830143b29d3ce16cc6901d7 Gerrit-Change-Number: 58475 Gerrit-PatchSet: 11 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 22 Nov 2021 02:42:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: spi25_statusreg: delete read_status_register()
by Nikolai Artemiev (Code Review) 22 Nov '21

22 Nov '21

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/59529 to look at the new patch set (#2). Change subject: spi25_statusreg: delete read_status_register() ...................................................................... spi25_statusreg: delete read_status_register() BUG=b:195381327,b:153800563 TEST=builds BRANCH=none Change-Id: I146b4b5439872e66c5d33e156451a729d248c7da Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M chipdrivers.h M it87spi.c M s25f.c M spi25.c M spi25_statusreg.c 5 files changed, 119 insertions(+), 55 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/29/59529/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/59529 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I146b4b5439872e66c5d33e156451a729d248c7da Gerrit-Change-Number: 59529 Gerrit-PatchSet: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: spi25_statusreg,flashchips: add SR2 read/write support
by Nikolai Artemiev (Code Review) 22 Nov '21

22 Nov '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/58570 to look at the new patch set (#10). Change subject: spi25_statusreg,flashchips: add SR2 read/write support ...................................................................... spi25_statusreg,flashchips: add SR2 read/write support This patch adds support for reading and writing the second status register and enables it on a limited set of flash chips. Chip support for RDSR2/WRSR2/extended WRSR is represented using feature flags to be consistent with how other SPI capabilities are represented. BUG=b:195381327,b:153800563 TEST=flashrom --wp-{enable,disable,range,list,status} at end of patch series TEST=logged SR2 read/write values during wp commands BRANCH=none Change-Id: I34a503b0958e8f2f22a2a993a6ea529eb46b41db Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flash.h M flashchips.c M spi.h M spi25_statusreg.c 4 files changed, 57 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/70/58570/10 -- To view, visit https://review.coreboot.org/c/flashrom/+/58570 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I34a503b0958e8f2f22a2a993a6ea529eb46b41db Gerrit-Change-Number: 58570 Gerrit-PatchSet: 10 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> 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]: spi25_statusreg: delete read_status_register()
by Nikolai Artemiev (Code Review) 22 Nov '21

22 Nov '21

Nikolai Artemiev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/59529 ) Change subject: spi25_statusreg: delete read_status_register() ...................................................................... spi25_statusreg: delete read_status_register() BUG=b:195381327,b:153800563 TEST=builds BRANCH=none Change-Id: I146b4b5439872e66c5d33e156451a729d248c7da Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M chipdrivers.h M it87spi.c M s25f.c M spi25.c M spi25_statusreg.c 5 files changed, 119 insertions(+), 55 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/29/59529/1 diff --git a/chipdrivers.h b/chipdrivers.h index 41b99e7..145abeb 100644 --- a/chipdrivers.h +++ b/chipdrivers.h @@ -62,8 +62,6 @@ /* spi25_statusreg.c */ -/* FIXME: replace spi_read_status_register() calls with spi_read_register() */ -uint8_t spi_read_status_register(const struct flashctx *flash); int spi_read_register(const struct flashctx *flash, enum flash_reg reg, uint8_t *value); int spi_write_register(const struct flashctx *flash, enum flash_reg reg, uint8_t value); void spi_prettyprint_status_register_bit(uint8_t status, int bit); diff --git a/it87spi.c b/it87spi.c index 5f6fb65..ae021e0 100644 --- a/it87spi.c +++ b/it87spi.c @@ -133,8 +133,16 @@ /* Wait until the Write-In-Progress bit is cleared. * This usually takes 1-10 ms, so wait in 1 ms steps. */ - while (spi_read_status_register(flash) & SPI_SR_WIP) + while (true) { + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; + + if((status & SPI_SR_WIP) == 0) + return 0; + programmer_delay(1000); + } return 0; } @@ -276,7 +284,8 @@ } while (len >= chip->page_size) { - it8716f_spi_page_program(flash, buf, start); + if (it8716f_spi_page_program(flash, buf, start)) + return -1; start += chip->page_size; len -= chip->page_size; buf += chip->page_size; diff --git a/s25f.c b/s25f.c index 0bd4a7c..82ca610 100644 --- a/s25f.c +++ b/s25f.c @@ -133,9 +133,14 @@ static int s25f_poll_status(const struct flashctx *flash) { - uint8_t tmp = spi_read_status_register(flash); + while (true) { + uint8_t tmp; + if (spi_read_register(flash, STATUS1, &tmp)) + return -1; - while (tmp & SPI_SR_WIP) { + if ((tmp & SPI_SR_WIP) == 0) + break; + /* * The WIP bit on S25F chips remains set to 1 if erase or * programming errors occur, so we must check for those @@ -156,7 +161,6 @@ } programmer_delay(1000 * 10); - tmp = spi_read_status_register(flash); } return 0; diff --git a/spi25.c b/spi25.c index 213273f..a8d1343 100644 --- a/spi25.c +++ b/spi25.c @@ -304,12 +304,16 @@ static int spi_poll_wip(struct flashctx *const flash, const unsigned int poll_delay) { - /* FIXME: We can't tell if spi_read_status_register() failed. */ - /* FIXME: We don't time out. */ - while (spi_read_status_register(flash) & SPI_SR_WIP) + for (int i = 0; i < 500; i++) { + uint8_t status; + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) return ret; + if (!(status & SPI_SR_WIP)) return 0; + programmer_delay(poll_delay); - /* FIXME: Check the status register for errors. */ - return 0; + } + + return TIMEOUT_ERROR; } /** diff --git a/spi25_statusreg.c b/spi25_statusreg.c index 07d9d5a..1204b66 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -110,17 +110,21 @@ * allow running RDSR only once. Therefore pick an initial delay of * 100 ms, then wait in 10 ms steps until a total of 5 s have elapsed. */ - int i = 0; programmer_delay(100 * 1000); - while (spi_read_status_register(flash) & SPI_SR_WIP) { - if (++i > 490) { - msg_cerr("Error: WIP bit after WRSR never cleared\n"); - return TIMEOUT_ERROR; - } + + uint8_t status; + for (int i = 0; i < 490; i++) { + result = spi_read_register(flash, STATUS1, &status); + if (result) + return result; + if ((status & SPI_SR_WIP) == 0) + return 0; + programmer_delay(10 * 1000); } - return 0; -} + + msg_cerr("Error: WIP bit after WRSR never cleared\n"); + return TIMEOUT_ERROR; } int spi_read_register(const struct flashctx *flash, enum flash_reg reg, uint8_t *value) @@ -151,15 +155,6 @@ return 0; } - -uint8_t spi_read_status_register(const struct flashctx *flash) -{ - uint8_t status = 0; - /* FIXME: We should propagate the error. */ - spi_read_register(flash, STATUS1, &status); - return status; -} - static int spi_restore_status(struct flashctx *flash, uint8_t status) { msg_cdbg("restoring chip status (0x%02x)\n", status); @@ -192,7 +187,10 @@ uint8_t status; int result; - status = spi_read_status_register(flash); + int ret = spi_read_register(flash, STATUS1, &status); + if (ret) + return ret; + if ((status & bp_mask) == 0) { msg_cdbg2("Block protection is disabled.\n"); return 0; @@ -214,7 +212,10 @@ msg_cerr("spi_write_register failed.\n"); return result; } - status = spi_read_status_register(flash); + + if (spi_read_register(flash, STATUS1, &status)) + return -1; + if ((status & lock_mask) != 0) { msg_cerr("Unsetting lock bit(s) failed.\n"); return 1; @@ -227,7 +228,10 @@ msg_cerr("spi_write_register failed.\n"); return result; } - status = spi_read_status_register(flash); + + if (spi_read_register(flash, STATUS1, &status)) + return -1; + if ((status & bp_mask) != 0) { msg_cerr("Block protection could not be disabled!\n"); if (flash->chip->printlock) @@ -348,7 +352,9 @@ int spi_prettyprint_status_register_plain(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; spi_prettyprint_status_register_hex(status); return 0; } @@ -356,7 +362,9 @@ /* Print the plain hex value and the welwip bits only. */ int spi_prettyprint_status_register_default_welwip(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_welwip(status); @@ -369,7 +377,9 @@ */ int spi_prettyprint_status_register_bp1_srwd(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_srwd(status); @@ -387,7 +397,9 @@ */ int spi_prettyprint_status_register_bp2_srwd(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_srwd(status); @@ -404,7 +416,9 @@ */ int spi_prettyprint_status_register_bp3_srwd(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_srwd(status); @@ -416,7 +430,9 @@ int spi_prettyprint_status_register_bp4_srwd(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_srwd(status); @@ -427,7 +443,9 @@ int spi_prettyprint_status_register_bp2_bpl(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_bpl(status); @@ -440,7 +458,9 @@ int spi_prettyprint_status_register_bp2_tb_bpl(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_bpl(status); @@ -462,7 +482,9 @@ int spi_prettyprint_status_register_amic_a25l032(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_srwd(status); @@ -518,7 +540,10 @@ int spi_prettyprint_status_register_at25df(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; + spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_atmel_at25_srpl(status); @@ -541,8 +566,9 @@ int spi_prettyprint_status_register_at25f(struct flashctx *flash) { uint8_t status; + if (spi_read_register(flash, STATUS1, &status)) + return -1; - status = spi_read_status_register(flash); spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_atmel_at25_wpen(status); @@ -557,8 +583,9 @@ int spi_prettyprint_status_register_at25f512a(struct flashctx *flash) { uint8_t status; + if (spi_read_register(flash, STATUS1, &status)) + return -1; - status = spi_read_status_register(flash); spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_atmel_at25_wpen(status); @@ -573,7 +600,9 @@ int spi_prettyprint_status_register_at25f512b(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_atmel_at25_srpl(status); @@ -589,7 +618,9 @@ { uint8_t status; - status = spi_read_status_register(flash); + if(spi_read_register(flash, STATUS1, &status)) + return -1; + spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_atmel_at25_wpen(status); @@ -602,7 +633,9 @@ int spi_prettyprint_status_register_at25fs010(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_atmel_at25_wpen(status); @@ -622,7 +655,9 @@ int spi_prettyprint_status_register_at25fs040(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_atmel_at25_wpen(status); @@ -634,7 +669,9 @@ int spi_prettyprint_status_register_at26df081a(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_atmel_at25_srpl(status); @@ -692,7 +729,9 @@ int spi_prettyprint_status_register_en25s_wp(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_srwd(status); @@ -711,7 +750,9 @@ int spi_prettyprint_status_register_n25q(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_srwd(status); @@ -736,7 +777,9 @@ /* Used by Intel/Numonyx S33 and Spansion S25FL-S chips */ int spi_prettyprint_status_register_bp2_ep_srwd(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; spi_prettyprint_status_register_hex(status); spi_prettyprint_status_register_srwd(status); @@ -764,7 +807,9 @@ int spi_prettyprint_status_register_sst25(struct flashctx *flash) { - uint8_t status = spi_read_status_register(flash); + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; spi_prettyprint_status_register_sst25_common(status); return 0; } @@ -780,7 +825,9 @@ "100000H-1FFFFFH", "all", "all" }; - uint8_t status = spi_read_status_register(flash); + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; spi_prettyprint_status_register_sst25_common(status); msg_cdbg("Resulting block protection : %s\n", bpt[(status & 0x1c) >> 2]); return 0; @@ -795,7 +842,9 @@ "0x40000-0x7ffff", "all blocks", "all blocks", "all blocks", "all blocks" }; - uint8_t status = spi_read_status_register(flash); + uint8_t status; + if(spi_read_register(flash, STATUS1, &status)) + return -1; spi_prettyprint_status_register_sst25_common(status); msg_cdbg("Resulting block protection : %s\n", bpt[(status & 0x1c) >> 2]); return 0; -- To view, visit https://review.coreboot.org/c/flashrom/+/59529 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I146b4b5439872e66c5d33e156451a729d248c7da Gerrit-Change-Number: 59529 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: spi25_statusreg: inline spi_write_register_flag()
by Nikolai Artemiev (Code Review) 22 Nov '21

22 Nov '21

Nikolai Artemiev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/59528 ) Change subject: spi25_statusreg: inline spi_write_register_flag() ...................................................................... spi25_statusreg: inline spi_write_register_flag() BUG=b:195381327,b:153800563 TEST=builds BRANCH=none Change-Id: I4996b0848d0ed09032bad2ab13ab1f40bbfc0304 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M spi25_statusreg.c 1 file changed, 47 insertions(+), 61 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/28/59528/1 diff --git a/spi25_statusreg.c b/spi25_statusreg.c index 61910ad..07d9d5a 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -21,58 +21,6 @@ #include "chipdrivers.h" #include "spi.h" -/* === Generic functions === */ -static int spi_write_register_flag(const struct flashctx *flash, uint8_t enable_opcode, uint8_t *write_cmd, size_t write_cmd_len) -{ - /* - * Enabling register writes requires either EWSR or WREN depending on - * chip type. The code below relies on the fact hat EWSR and WREN have - * the same INSIZE and OUTSIZE. - */ - - struct spi_command cmds[] = { - { - .writecnt = JEDEC_WREN_OUTSIZE, - .writearr = &enable_opcode, - .readcnt = 0, - .readarr = NULL, - }, { - .writecnt = write_cmd_len, - .writearr = write_cmd, - .readcnt = 0, - .readarr = NULL, - }, { - .writecnt = 0, - .writearr = NULL, - .readcnt = 0, - .readarr = NULL, - }}; - - int result = spi_send_multicommand(flash, cmds); - if (result) { - msg_cerr("%s failed during command execution\n", __func__); - /* No point in waiting for the command to complete if execution - * failed. - */ - return result; - } - /* WRSR performs a self-timed erase before the changes take effect. - * This may take 50-85 ms in most cases, and some chips apparently - * allow running RDSR only once. Therefore pick an initial delay of - * 100 ms, then wait in 10 ms steps until a total of 5 s have elapsed. - */ - int i = 0; - programmer_delay(100 * 1000); - while (spi_read_status_register(flash) & SPI_SR_WIP) { - if (++i > 490) { - msg_cerr("Error: WIP bit after WRSR never cleared\n"); - return TIMEOUT_ERROR; - } - programmer_delay(10 * 1000); - } - return 0; -} - static const char *reg_name(enum flash_reg reg) { switch (reg) { @@ -89,7 +37,6 @@ uint8_t write_cmd[3]; size_t write_cmd_len = 0; - bool chip_supports_write = false; /* Create SPI write command sequence based on the destination register * and the chip's supported command set. */ @@ -123,18 +70,57 @@ return 1; } - if (!(feature_bits & (FEATURE_WRSR_WREN | FEATURE_WRSR_EWSR))) { + uint8_t enable_cmd; + if (feature_bits & FEATURE_WRSR_WREN) + enable_cmd = JEDEC_WREN; + else if (feature_bits & FEATURE_WRSR_EWSR) + enable_cmd = JEDEC_EWSR; + else { msg_cdbg("Missing status register write definition, assuming " "EWSR is needed\n"); - feature_bits |= FEATURE_WRSR_EWSR; + enable_cmd = JEDEC_EWSR; } - int ret = 1; - if (feature_bits & FEATURE_WRSR_WREN) - ret = spi_write_register_flag(flash, JEDEC_WREN, write_cmd, write_cmd_len); - if (ret && (feature_bits & FEATURE_WRSR_EWSR)) - ret = spi_write_register_flag(flash, JEDEC_EWSR, write_cmd, write_cmd_len); - return ret; + struct spi_command cmds[] = { + { + .writecnt = JEDEC_WREN_OUTSIZE, + .writearr = &enable_cmd, + .readcnt = 0, + .readarr = NULL, + }, { + .writecnt = write_cmd_len, + .writearr = write_cmd, + .readcnt = 0, + .readarr = NULL, + }, { + .writecnt = 0, + .writearr = NULL, + .readcnt = 0, + .readarr = NULL, + }}; + + int result = spi_send_multicommand(flash, cmds); + if (result) { + msg_cerr("%s failed during command execution\n", __func__); + return result; + } + + /* WRSR performs a self-timed erase before the changes take effect. + * This may take 50-85 ms in most cases, and some chips apparently + * allow running RDSR only once. Therefore pick an initial delay of + * 100 ms, then wait in 10 ms steps until a total of 5 s have elapsed. + */ + int i = 0; + programmer_delay(100 * 1000); + while (spi_read_status_register(flash) & SPI_SR_WIP) { + if (++i > 490) { + msg_cerr("Error: WIP bit after WRSR never cleared\n"); + return TIMEOUT_ERROR; + } + programmer_delay(10 * 1000); + } + return 0; +} } int spi_read_register(const struct flashctx *flash, enum flash_reg reg, uint8_t *value) -- To view, visit https://review.coreboot.org/c/flashrom/+/59528 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4996b0848d0ed09032bad2ab13ab1f40bbfc0304 Gerrit-Change-Number: 59528 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: libflashrom,writeprotect: add flashrom_wp_{config,range,mode}_to_str()
by Nikolai Artemiev (Code Review) 22 Nov '21

22 Nov '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/58484 to look at the new patch set (#14). Change subject: libflashrom,writeprotect: add flashrom_wp_{config,range,mode}_to_str() ...................................................................... libflashrom,writeprotect: add flashrom_wp_{config,range,mode}_to_str() BUG=b:195381327,b:153800563 TEST=flashrom --wp-{enable,disable,range,list,status} at end of patch series BRANCH=none Change-Id: I82441c60c239a5b947b5a2a87b61e7bdf60bee89 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M libflashrom.h M writeprotect.c 2 files changed, 102 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/84/58484/14 -- To view, visit https://review.coreboot.org/c/flashrom/+/58484 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I82441c60c239a5b947b5a2a87b61e7bdf60bee89 Gerrit-Change-Number: 58484 Gerrit-PatchSet: 14 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: libflashrom,writeprotect: add flashrom_wp_get_available_ranges()
by Nikolai Artemiev (Code Review) 22 Nov '21

22 Nov '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/58481 to look at the new patch set (#13). Change subject: libflashrom,writeprotect: add flashrom_wp_get_available_ranges() ...................................................................... libflashrom,writeprotect: add flashrom_wp_get_available_ranges() Generate list of available ranges by enumerating all possible values that range bits (BPx, TB, ...) can take and using the chip's range decoding function to get the range associated with each one. BUG=b:195381327,b:153800563 TEST=flashrom --wp-list BRANCH=none Change-Id: Id51f038f03305c8536d80313e52f77d27835f34d Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M libflashrom.h M writeprotect.c 2 files changed, 146 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/81/58481/13 -- To view, visit https://review.coreboot.org/c/flashrom/+/58481 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id51f038f03305c8536d80313e52f77d27835f34d Gerrit-Change-Number: 58481 Gerrit-PatchSet: 13 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: flashchips,writeprotect_ranges: add range decoding function
by Nikolai Artemiev (Code Review) 22 Nov '21

22 Nov '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/58480 to look at the new patch set (#13). Change subject: flashchips,writeprotect_ranges: add range decoding function ...................................................................... flashchips,writeprotect_ranges: add range decoding function Allow chips to specify functions that map status register bits to protection ranges. These are used to enumerate available ranges and determine the protection state of chips. The patch also adds a range decoding function for the example chips. Many other chips can also be handled by it, though some will require different functions (e.g. MX25L6406 and related chips). Another approach that has been tried in cros flashrom is maintaining tables of range data, but it quickly becomes error prone and hard to validate. Using a function to interpret the ranges allows compact encoding with most chips and is flexible enough to allow chips with less predictable ranges to be handled as well. BUG=b:195381327,b:153800563 TEST=dumped range tables, checked against datasheets BRANCH=none Change-Id: Id163ed80938a946a502ed116e48e8236e36eb203 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M Makefile M chipdrivers.h M flash.h M flashchips.c M libflashrom.h M meson.build A writeprotect_ranges.c 7 files changed, 115 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/80/58480/13 -- To view, visit https://review.coreboot.org/c/flashrom/+/58480 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id163ed80938a946a502ed116e48e8236e36eb203 Gerrit-Change-Number: 58480 Gerrit-PatchSet: 13 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: libflashrom,writeprotect: add flashrom_wp_{read,write}_chip_config()
by Nikolai Artemiev (Code Review) 22 Nov '21

22 Nov '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/58479 to look at the new patch set (#12). Change subject: libflashrom,writeprotect: add flashrom_wp_{read,write}_chip_config() ...................................................................... libflashrom,writeprotect: add flashrom_wp_{read,write}_chip_config() The read/write functions map between register values and `struct wp_chip_state` using the chip's register bit map. All currently supported chips can be handled with a single pair of read/write functions, though an option for chips to override them might be necessary in the future. BUG=b:195381327,b:153800563 TEST=flashrom --wp-{enable,disable,range,list,status} at end of patch series BRANCH=none Change-Id: I3ad25708c3321b8fb0216c3eaf6ffc07616537ad Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M libflashrom.h M writeprotect.c 2 files changed, 168 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/58479/12 -- To view, visit https://review.coreboot.org/c/flashrom/+/58479 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3ad25708c3321b8fb0216c3eaf6ffc07616537ad Gerrit-Change-Number: 58479 Gerrit-PatchSet: 12 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: spi25_statusreg,flashchips: add SR2 read/write support
by Nikolai Artemiev (Code Review) 22 Nov '21

22 Nov '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/58570 to look at the new patch set (#9). Change subject: spi25_statusreg,flashchips: add SR2 read/write support ...................................................................... spi25_statusreg,flashchips: add SR2 read/write support This patch adds support for reading and writing the second status register and enables it on a limited set of flash chips. Chip support for RDSR2/WRSR2/extended WRSR is represented using feature flags to be consistent with how other SPI capabilities are represented. BUG=b:195381327,b:153800563 TEST=flashrom --wp-{enable,disable,range,list,status} at end of patch series TEST=logged SR2 read/write values during wp commands BRANCH=none Change-Id: I34a503b0958e8f2f22a2a993a6ea529eb46b41db Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flash.h M flashchips.c M spi.h M spi25_statusreg.c 4 files changed, 58 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/70/58570/9 -- To view, visit https://review.coreboot.org/c/flashrom/+/58570 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I34a503b0958e8f2f22a2a993a6ea529eb46b41db Gerrit-Change-Number: 58570 Gerrit-PatchSet: 9 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2021