Change in flashrom[master]: support variable-size SPI chip for dummy programmer
Namyoon Woo has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/44879 ) Change subject: support variable-size SPI chip for dummy programmer ...................................................................... support variable-size SPI chip for dummy programmer This is designed for firmware updater to pack firmware image preserving some specific partitions in any size. BUG=none TEST=ran the command line below: $ flashrom -p dummy:image=${TMP_FILE},size=16777216, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f $ flashrom -p dummy:image=${TMP_FILE},size=auto, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f Signed-off-by: Namyoon Woo <namyoon@google.com> Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 --- M chipdrivers.h M dummyflasher.c M flashchips.c M flashchips.h 4 files changed, 159 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/44879/1 diff --git a/chipdrivers.h b/chipdrivers.h index 3c7d146..cf03811 100644 --- a/chipdrivers.h +++ b/chipdrivers.h @@ -196,6 +196,9 @@ int probe_en29lv640b(struct flashctx *flash); int write_en29lv640b(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); +/* dummyflasher.c */ +int probe_variable_size(struct flashctx *flash); + /* edi.c */ int edi_chip_block_erase(struct flashctx *flash, unsigned int page, unsigned int size); int edi_chip_write(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); diff --git a/dummyflasher.c b/dummyflasher.c index e1a1d80..4024f2a 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -21,6 +21,7 @@ #include "flash.h" #include "chipdrivers.h" #include "programmer.h" +#include "flashchips.h" /* Remove the #define below if you don't want SPI flash chip emulation. */ #define EMULATE_SPI_CHIP 1 @@ -33,6 +34,13 @@ #if EMULATE_CHIP #include <sys/types.h> #include <sys/stat.h> + +#if EMULATE_SPI_CHIP +/* The name of variable-size virtual chip. A 4MB flash example: + * flashrom -p dummy:emulate=VARIABLE_SIZE,size=4194304 + */ +#define VARIABLE_SIZE_CHIP_NAME "VARIABLE_SIZE" +#endif #endif #if EMULATE_CHIP @@ -44,6 +52,7 @@ EMULATE_SST_SST25VF032B, EMULATE_MACRONIX_MX25L6436, EMULATE_WINBOND_W25Q128FV, + EMULATE_VARIABLE_SIZE, }; static enum emu_chip emu_chip = EMULATE_NONE; static char *emu_persistent_image = NULL; @@ -147,6 +156,12 @@ return 0; } +/* Values for the 'size' parameter */ +enum { + SIZE_UNKNOWN = -1, + SIZE_AUTO = -2, +}; + int dummy_init(void) { char *bustext = NULL; @@ -154,6 +169,7 @@ unsigned int i; #if EMULATE_SPI_CHIP char *status = NULL; + int size = SIZE_UNKNOWN; /* size for VARIOUS_SIZE chip device */ #endif #if EMULATE_CHIP struct stat image_stat; @@ -272,6 +288,31 @@ free(tmp); #if EMULATE_CHIP +#if EMULATE_SPI_CHIP + tmp = extract_programmer_param("size"); + if (tmp) { + int multiplier = 1; + if (!strcmp(tmp, "auto")) + size = SIZE_AUTO; + else if (strlen(tmp)) { + int remove_last_char = 1; + switch (tmp[strlen(tmp) - 1]) { + case 'k': case 'K': + multiplier = 1024; + break; + case 'm': case 'M': + multiplier = 1024 * 1024; + break; + default: + remove_last_char = 0; + break; + } + if (remove_last_char) tmp[strlen(tmp) - 1] = '\0'; + size = atoi(tmp) * multiplier; + } + } +#endif + tmp = extract_programmer_param("emulate"); if (!tmp) { msg_pdbg("Not emulating any flash chip.\n"); @@ -343,6 +384,42 @@ emu_jedec_ce_c7_size = emu_chip_size; msg_pdbg("Emulating Winbond W25Q128FV SPI flash chip (RDID)\n"); } + + emu_persistent_image = extract_programmer_param("image"); + /* Will be freed by shutdown function if necessary. */ + if (!emu_persistent_image) { + /* Nothing else to do. */ + goto dummy_init_out; + } + + if (!strncmp(tmp, VARIABLE_SIZE_CHIP_NAME, + strlen(VARIABLE_SIZE_CHIP_NAME))) { + if (size == SIZE_UNKNOWN) { + msg_perr("%s: the size parameter is not given.\n", + __func__); + free(tmp); + return 1; + } else if (size == SIZE_AUTO) { + if (stat(emu_persistent_image, &image_stat)) { + msg_perr("%s: no image so cannot use automatic size.\n", + __func__); + free(tmp); + return 1; + } + size = image_stat.st_size; + } + emu_chip = EMULATE_VARIABLE_SIZE; + emu_chip_size = size; + emu_max_byteprogram_size = 256; + emu_max_aai_size = 0; + emu_jedec_se_size = 4 * 1024; + emu_jedec_be_52_size = 32 * 1024; + emu_jedec_be_d8_size = 64 * 1024; + emu_jedec_ce_60_size = emu_chip_size; + emu_jedec_ce_c7_size = emu_chip_size; + msg_pdbg("Emulating generic SPI flash chip (size=%d bytes)\n", + emu_chip_size); + } #endif if (emu_chip == EMULATE_NONE) { msg_perr("Invalid chip specified for emulation: %s\n", tmp); @@ -376,12 +453,6 @@ msg_pdbg("Filling fake flash chip with 0xff, size %i\n", emu_chip_size); memset(flashchip_contents, 0xff, emu_chip_size); - /* Will be freed by shutdown function if necessary. */ - emu_persistent_image = extract_programmer_param("image"); - if (!emu_persistent_image) { - /* Nothing else to do. */ - goto dummy_init_out; - } /* We will silently (in default verbosity) ignore the file if it does not exist (yet) or the size does * not match the emulated chip. */ if (!stat(emu_persistent_image, &image_stat)) { @@ -610,6 +681,16 @@ if (readcnt > 2) readarr[2] = 0x18; break; + case EMULATE_VARIABLE_SIZE: + if (readcnt > 0) + readarr[0] = (VARIABLE_SIZE_MANUF_ID >> 8) & 0xff; + if (readcnt > 1) + readarr[1] = VARIABLE_SIZE_MANUF_ID & 0xff; + if (readcnt > 2) + readarr[2] = (VARIABLE_SIZE_DEVICE_ID >> 8) & 0xff; + if (readcnt > 3) + readarr[3] = VARIABLE_SIZE_DEVICE_ID & 0xff; + break; default: /* ignore */ break; } @@ -840,6 +921,7 @@ case EMULATE_SST_SST25VF032B: case EMULATE_MACRONIX_MX25L6436: case EMULATE_WINBOND_W25Q128FV: + case EMULATE_VARIABLE_SIZE: if (emulate_spi_chip_response(writecnt, readcnt, writearr, readarr)) { msg_pdbg("Invalid command sent to flash chip!\n"); @@ -862,3 +944,47 @@ return spi_write_chunked(flash, buf, start, len, spi_write_256_chunksize); } + +#if EMULATE_CHIP && EMULATE_SPI_CHIP +int probe_variable_size(struct flashctx *flash) +{ + int i; + + /* Skip the probing if we don't emulate this chip. */ + if (emu_chip != EMULATE_VARIABLE_SIZE) + return 0; + + /* + * This will break if one day flashctx becomes read-only. + * Once that happens, we need to have special hacks in functions: + * + * erase_and_write_flash() in flashrom.c + * read_flash_to_file() + * handle_romentries() + * ... + * + * Search "total_size * 1024" in code. + */ + if (emu_chip_size % 1024) + msg_perr("%s: emu_chip_size is not multipler of 1024.\n", + __func__); + flash->chip->total_size = emu_chip_size / 1024; + msg_cdbg("%s: set flash->total_size to %dK bytes.\n", __func__, + flash->chip->total_size); + + /* Update eraser count */ + for (i = 0; i < NUM_ERASEFUNCTIONS; i++) { + struct block_eraser *eraser = &flash->chip->block_erasers[i]; + if (eraser->block_erase == NULL) + break; + + eraser->eraseblocks[0].count = emu_chip_size / + eraser->eraseblocks[0].size; + msg_cdbg("%s: eraser.size=%d, .count=%d\n", + __func__, eraser->eraseblocks[0].size, + eraser->eraseblocks[0].count); + } + + return 1; +} +#endif diff --git a/flashchips.c b/flashchips.c index 8b5b5cc..e99073b 100644 --- a/flashchips.c +++ b/flashchips.c @@ -18759,6 +18759,27 @@ { .vendor = "Generic", + .name = "Variable Size SPI chip", + .bustype = BUS_SPI, + .manufacture_id = VARIABLE_SIZE_MANUF_ID, + .model_id = VARIABLE_SIZE_DEVICE_ID, + .total_size = 64, /* This size is set temporarily */ + .page_size = 256, + .tested = TEST_OK_PREW, + .probe = probe_variable_size, + .block_erasers = + { + { + .eraseblocks = { {64 * 1024, 1} }, + .block_erase = spi_block_erase_d8, + } + }, + .write = spi_chip_write_256, + .read = spi_chip_read, + }, + + { + .vendor = "Generic", .name = "unknown SPI chip (RDID)", .bustype = BUS_SPI, .manufacture_id = GENERIC_MANUF_ID, diff --git a/flashchips.h b/flashchips.h index 5b7937f..85010ea 100644 --- a/flashchips.h +++ b/flashchips.h @@ -36,6 +36,9 @@ #define PROGMANUF_ID 0xFFFE /* dummy ID for opaque chips behind a programmer */ #define PROGDEV_ID 0x01 /* dummy ID for opaque chips behind a programmer */ +#define VARIABLE_SIZE_MANUF_ID 0x3eaf +#define VARIABLE_SIZE_DEVICE_ID 0x10af + #define ALLIANCE_ID 0x52 /* Alliance Semiconductor */ #define ALLIANCE_AS29F002B 0x34 #define ALLIANCE_AS29F002T 0xB0 -- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 1 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-MessageType: newchange
Namyoon Woo has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 ) Change subject: support variable-size SPI chip for dummy programmer ...................................................................... Patch Set 1: Code-Review+1 This is to sync with the one in chromiumos repo. This patch is quoted from two patches: - https://codereview.chromium.org/6791015 - https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/+... -- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 1 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Thu, 27 Aug 2020 23:22:27 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Namyoon Woo has removed a vote from this change. ( https://review.coreboot.org/c/flashrom/+/44879 ) Change subject: support variable-size SPI chip for dummy programmer ...................................................................... Removed Code-Review+1 by Namyoon Woo <namyoon@google.com> -- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 1 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: deleteVote
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 ) Change subject: support variable-size SPI chip for dummy programmer ...................................................................... Patch Set 1: Code-Review+2 (1 comment) https://review.coreboot.org/c/flashrom/+/44879/1/dummyflasher.c File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/44879/1/dummyflasher.c@395 PS1, Line 395: if (!strncmp(tmp, VARIABLE_SIZE_CHIP_NAME, : strlen(VARIABLE_SIZE_CHIP_NAME))) { I think this is fine to put on the same line for readability. -- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 1 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Thu, 27 Aug 2020 23:33:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/44879 to look at the new patch set (#2). Change subject: support variable-size SPI chip for dummy programmer ...................................................................... support variable-size SPI chip for dummy programmer This is designed for firmware updater to pack firmware image preserving some specific partitions in any size. BUG=none TEST=ran the command line below: $ flashrom -p dummy:image=${TMP_FILE},size=16777216, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f $ flashrom -p dummy:image=${TMP_FILE},size=auto, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f Signed-off-by: Namyoon Woo <namyoon@google.com> Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 --- M chipdrivers.h M dummyflasher.c M flashchips.c M flashchips.h 4 files changed, 154 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/44879/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 2 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-MessageType: newpatchset
Namyoon Woo has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 ) Change subject: support variable-size SPI chip for dummy programmer ...................................................................... Patch Set 2: (1 comment) https://review.coreboot.org/c/flashrom/+/44879/1/dummyflasher.c File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/44879/1/dummyflasher.c@395 PS1, Line 395: if (!strncmp(tmp, VARIABLE_SIZE_CHIP_NAME, : strlen(VARIABLE_SIZE_CHIP_NAME))) {
I think this is fine to put on the same line for readability. Like other cases above, I converted it to use strcmp(), without a name definition.
-- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 2 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Thu, 27 Aug 2020 23:59:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec@chromium.org> Gerrit-MessageType: comment
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 ) Change subject: support variable-size SPI chip for dummy programmer ...................................................................... Patch Set 2: Code-Review+2 (1 comment) https://review.coreboot.org/c/flashrom/+/44879/1/dummyflasher.c File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/44879/1/dummyflasher.c@395 PS1, Line 395: if (!strncmp(tmp, VARIABLE_SIZE_CHIP_NAME, : strlen(VARIABLE_SIZE_CHIP_NAME))) {
Like other cases above, I converted it to use strcmp(), without a name definition. Sorry, what I mean is that you can remove the line wrap here.
-- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 2 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Fri, 28 Aug 2020 00:44:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Namyoon Woo <namyoon@google.com> Comment-In-Reply-To: Edward O'Callaghan <quasisec@chromium.org> Gerrit-MessageType: comment
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 ) Change subject: support variable-size SPI chip for dummy programmer ...................................................................... Patch Set 2: Code-Review-1 (8 comments) I don't like the parsing of the command-line parameters at all. https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@a379 PS2, Line 379: /* Will be freed by shutdown function if necessary. */ Why is this gone? https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@288 PS2, Line 288: if (!strcmp(tmp, "auto")) : size = SIZE_AUTO; : This branch of the if should have braces https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@293 PS2, Line 293: case 'k': case 'K': I'd put these two case statements in separate lines: case 'k': case 'K': https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@303 PS2, Line 303: tmp[strlen(tmp) - 1] = '\0'; This should be on a separate line https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@304 PS2, Line 304: atoi(tmp) What will happen if I specify the following sizes as a parameter? K 1G 87654321 -512m 1 big_enough 0 https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@388 PS2, Line 388: 4MB that's 4 MiB (4 MB would be size=4000000) https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@963 PS2, Line 963: if (emu_chip_size % 1024) : msg_perr("%s: emu_chip_size is not multipler of 1024.\n", : __func__); Please move this to the parsing of the commmandline arguments https://review.coreboot.org/c/flashrom/+/44879/2/flashchips.h File flashchips.h: https://review.coreboot.org/c/flashrom/+/44879/2/flashchips.h@39 PS2, Line 39: #define VARIABLE_SIZE_MANUF_ID 0x3eaf : #define VARIABLE_SIZE_DEVICE_ID 0x10af Where do these IDs come from? Why not use PROGMANUF_ID and PROGDEV_ID instead? -- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 2 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-Comment-Date: Sat, 29 Aug 2020 10:20:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/44879 to look at the new patch set (#3). Change subject: support variable-size SPI chip for dummy programmer ...................................................................... support variable-size SPI chip for dummy programmer This is designed for firmware updater to pack firmware image preserving some specific partitions in any size. BUG=none TEST=ran the command line below: $ flashrom -p dummy:image=${TMP_FILE},size=16777216, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f $ flashrom -p dummy:image=${TMP_FILE},size=16M, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f $ flashrom -p dummy:image=${TMP_FILE},size=auto, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f Signed-off-by: Namyoon Woo <namyoon@google.com> Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 --- M chipdrivers.h M dummyflasher.c M flashchips.c M flashchips.h 4 files changed, 177 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/44879/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 3 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Namyoon Woo has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 ) Change subject: support variable-size SPI chip for dummy programmer ...................................................................... Patch Set 3: (9 comments) https://review.coreboot.org/c/flashrom/+/44879/1/dummyflasher.c File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/44879/1/dummyflasher.c@395 PS1, Line 395: if (!strncmp(tmp, VARIABLE_SIZE_CHIP_NAME, : strlen(VARIABLE_SIZE_CHIP_NAME))) {
Sorry, what I mean is that you can remove the line wrap here. Done
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@a379 PS2, Line 379: /* Will be freed by shutdown function if necessary. */
Why is this gone? This is moved to above line 347, because it needed to stat emu_persistent_image in case that SIZE_AUTO was given in parameter.
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@288 PS2, Line 288: if (!strcmp(tmp, "auto")) : size = SIZE_AUTO; :
This branch of the if should have braces Done
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@293 PS2, Line 293: case 'k': case 'K':
I'd put these two case statements in separate lines: […] Done
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@303 PS2, Line 303: tmp[strlen(tmp) - 1] = '\0';
This should be on a separate line Done
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@304 PS2, Line 304: atoi(tmp)
What will happen if I specify the following sizes as a parameter? […] Added more parse error handling codes.
K => size shall be 0, and processed as an error at the end. 1G => atoi('1G') shall be 0, and processed as an error at the end. 87654321 => shall be processed as an error since it is not a multiple of 1024. -512m => shall be processed as an error since it is negative. 1 => shall be processed as an error since it is not a multiple of 1024. big_enough => size shall be 0, and processed as an error at the end. 0 => size shall be 0, and processed as an error at the end. https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@388 PS2, Line 388: 4MB
that's 4 MiB (4 MB would be size=4000000) Done
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@963 PS2, Line 963: if (emu_chip_size % 1024) : msg_perr("%s: emu_chip_size is not multipler of 1024.\n", : __func__);
Please move this to the parsing of the commmandline arguments Done. Moved to the line 427 in patchset#3.
https://review.coreboot.org/c/flashrom/+/44879/2/flashchips.h File flashchips.h: https://review.coreboot.org/c/flashrom/+/44879/2/flashchips.h@39 PS2, Line 39: #define VARIABLE_SIZE_MANUF_ID 0x3eaf : #define VARIABLE_SIZE_DEVICE_ID 0x10af
Where do these IDs come from? Why not use PROGMANUF_ID and PROGDEV_ID instead? Done
-- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 3 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sun, 30 Aug 2020 22:17:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Namyoon Woo <namyoon@google.com> Comment-In-Reply-To: Edward O'Callaghan <quasisec@chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/44879 to look at the new patch set (#4). Change subject: support variable-size SPI chip for dummy programmer ...................................................................... support variable-size SPI chip for dummy programmer This is designed for firmware updater to pack firmware image preserving some specific partitions in any size. BUG=none TEST=ran the command line below: $ flashrom -p dummy:image=${TMP_FILE},size=16777216, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f $ flashrom -p dummy:image=${TMP_FILE},size=16M, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f $ flashrom -p dummy:image=${TMP_FILE},size=auto, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f Signed-off-by: Namyoon Woo <namyoon@google.com> Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 --- M chipdrivers.h M dummyflasher.c M flashchips.c 3 files changed, 174 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/44879/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 4 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/44879 to look at the new patch set (#5). Change subject: support variable-size SPI chip for dummy programmer ...................................................................... support variable-size SPI chip for dummy programmer This is designed for firmware updater to pack firmware image preserving some specific partitions in any size. BUG=none TEST=ran the command line below: $ flashrom -p dummy:image=${TMP_FILE},size=16777216, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f $ flashrom -p dummy:image=${TMP_FILE},size=auto, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f Signed-off-by: Namyoon Woo <namyoon@google.com> Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 --- M chipdrivers.h M dummyflasher.c M flashchips.c 3 files changed, 114 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/44879/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 5 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/44879 to look at the new patch set (#6). Change subject: support variable-size SPI chip for dummy programmer ...................................................................... support variable-size SPI chip for dummy programmer This is designed for firmware updater to pack firmware image preserving some specific partitions in any size. BUG=none TEST=ran the command line below: $ flashrom -p dummy:image=${TMP_FILE},size=16777216, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f $ flashrom -p dummy:image=${TMP_FILE},size=auto, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f Signed-off-by: Namyoon Woo <namyoon@google.com> Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 --- M chipdrivers.h M dummyflasher.c M flashchips.c 3 files changed, 113 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/44879/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 6 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Namyoon Woo has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 ) Change subject: support variable-size SPI chip for dummy programmer ...................................................................... Patch Set 6: (1 comment)
Patch Set 2: Code-Review-1
(8 comments)
I don't like the parsing of the command-line parameters at all.
In patchset#6, I removed SIZE_AUTO and 'k|K|m|M' prefix handling in size parameter for simplicity. https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@a379 PS2, Line 379: /* Will be freed by shutdown function if necessary. */
This is moved to above line 347, because it needed to stat emu_persistent_image in case that SIZE_AU […] I am bring this back here.
-- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 6 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 01 Sep 2020 00:47:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Namyoon Woo <namyoon@google.com> Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Gerrit-MessageType: comment
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 ) Change subject: support variable-size SPI chip for dummy programmer ...................................................................... Patch Set 6: Code-Review+2 (3 comments) https://review.coreboot.org/c/flashrom/+/44879/6/dummyflasher.c File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/44879/6/dummyflasher.c@917 PS6, Line 917: int unsigned i; https://review.coreboot.org/c/flashrom/+/44879/6/dummyflasher.c@938 PS6, Line 938: pdate eraser count * "Update the first count of each of the block_erasers" or something more detailed why here. https://review.coreboot.org/c/flashrom/+/44879/6/dummyflasher.c@941 PS6, Line 941: eraser->block_erase == NULL `if (!eraser->block_erase)` is canonical -- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 6 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 01 Sep 2020 02:36:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/44879 to look at the new patch set (#7). Change subject: support variable-size SPI chip for dummy programmer ...................................................................... support variable-size SPI chip for dummy programmer This is designed for firmware updater to pack firmware image preserving some specific partitions in any size. BUG=none TEST=ran the command line below: $ flashrom -p dummy:image=${TMP_FILE},size=16777216, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f $ flashrom -p dummy:image=${TMP_FILE},size=auto, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f Signed-off-by: Namyoon Woo <namyoon@google.com> Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 --- M chipdrivers.h M dummyflasher.c M flashchips.c 3 files changed, 113 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/44879/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 7 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset
Namyoon Woo has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 ) Change subject: support variable-size SPI chip for dummy programmer ...................................................................... Patch Set 7: (3 comments) https://review.coreboot.org/c/flashrom/+/44879/6/dummyflasher.c File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/44879/6/dummyflasher.c@917 PS6, Line 917: int
unsigned i; Done
https://review.coreboot.org/c/flashrom/+/44879/6/dummyflasher.c@938 PS6, Line 938: pdate eraser count *
"Update the first count of each of the block_erasers" or something more detailed why here. Done
https://review.coreboot.org/c/flashrom/+/44879/6/dummyflasher.c@941 PS6, Line 941: eraser->block_erase == NULL
`if (!eraser->block_erase)` is canonical Done
-- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 7 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 01 Sep 2020 02:47:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec@chromium.org> Gerrit-MessageType: comment
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 ) Change subject: support variable-size SPI chip for dummy programmer ...................................................................... Patch Set 7: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 7 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Tue, 01 Sep 2020 03:39:21 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
Namyoon Woo has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 ) Change subject: support variable-size SPI chip for dummy programmer ...................................................................... Patch Set 7: ping. -- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 7 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 03 Sep 2020 16:39:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/44879 ) Change subject: support variable-size SPI chip for dummy programmer ...................................................................... support variable-size SPI chip for dummy programmer This is designed for firmware updater to pack firmware image preserving some specific partitions in any size. BUG=none TEST=ran the command line below: $ flashrom -p dummy:image=${TMP_FILE},size=16777216, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f $ flashrom -p dummy:image=${TMP_FILE},size=auto, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f Signed-off-by: Namyoon Woo <namyoon@google.com> Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Reviewed-on: https://review.coreboot.org/c/flashrom/+/44879 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec@chromium.org> --- M chipdrivers.h M dummyflasher.c M flashchips.c 3 files changed, 113 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved diff --git a/chipdrivers.h b/chipdrivers.h index 3c7d146..cf03811 100644 --- a/chipdrivers.h +++ b/chipdrivers.h @@ -196,6 +196,9 @@ int probe_en29lv640b(struct flashctx *flash); int write_en29lv640b(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); +/* dummyflasher.c */ +int probe_variable_size(struct flashctx *flash); + /* edi.c */ int edi_chip_block_erase(struct flashctx *flash, unsigned int page, unsigned int size); int edi_chip_write(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); diff --git a/dummyflasher.c b/dummyflasher.c index e1a1d80..b2b3b54 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -21,6 +21,7 @@ #include "flash.h" #include "chipdrivers.h" #include "programmer.h" +#include "flashchips.h" /* Remove the #define below if you don't want SPI flash chip emulation. */ #define EMULATE_SPI_CHIP 1 @@ -44,6 +45,7 @@ EMULATE_SST_SST25VF032B, EMULATE_MACRONIX_MX25L6436, EMULATE_WINBOND_W25Q128FV, + EMULATE_VARIABLE_SIZE, }; static enum emu_chip emu_chip = EMULATE_NONE; static char *emu_persistent_image = NULL; @@ -154,6 +156,7 @@ unsigned int i; #if EMULATE_SPI_CHIP char *status = NULL; + int size = -1; /* size for VARIOUS_SIZE chip device */ #endif #if EMULATE_CHIP struct stat image_stat; @@ -272,6 +275,23 @@ free(tmp); #if EMULATE_CHIP +#if EMULATE_SPI_CHIP + tmp = extract_programmer_param("size"); + if (tmp) { + size = atoi(tmp); + if (size <= 0 || (size % 1024 != 0)) { + msg_perr("%s: Chip size is not a multipler of 1024: %s\n", + __func__, tmp); + free(tmp); + return 1; + } + free(tmp); + } else { + msg_perr("%s: the size parameter is not given.\n", __func__); + return 1; + } +#endif + tmp = extract_programmer_param("emulate"); if (!tmp) { msg_pdbg("Not emulating any flash chip.\n"); @@ -343,6 +363,23 @@ emu_jedec_ce_c7_size = emu_chip_size; msg_pdbg("Emulating Winbond W25Q128FV SPI flash chip (RDID)\n"); } + + /* The name of variable-size virtual chip. A 4 MiB flash example: + * flashrom -p dummy:emulate=VARIABLE_SIZE,size=4194304 + */ + if (!strcmp(tmp, "VARIABLE_SIZE")) { + emu_chip = EMULATE_VARIABLE_SIZE; + emu_chip_size = size; + emu_max_byteprogram_size = 256; + emu_max_aai_size = 0; + emu_jedec_se_size = 4 * 1024; + emu_jedec_be_52_size = 32 * 1024; + emu_jedec_be_d8_size = 64 * 1024; + emu_jedec_ce_60_size = emu_chip_size; + emu_jedec_ce_c7_size = emu_chip_size; + msg_pdbg("Emulating generic SPI flash chip (size=%d bytes)\n", + emu_chip_size); + } #endif if (emu_chip == EMULATE_NONE) { msg_perr("Invalid chip specified for emulation: %s\n", tmp); @@ -610,6 +647,16 @@ if (readcnt > 2) readarr[2] = 0x18; break; + case EMULATE_VARIABLE_SIZE: + if (readcnt > 0) + readarr[0] = (PROGMANUF_ID >> 8) & 0xff; + if (readcnt > 1) + readarr[1] = PROGMANUF_ID & 0xff; + if (readcnt > 2) + readarr[2] = (PROGDEV_ID >> 8) & 0xff; + if (readcnt > 3) + readarr[3] = PROGDEV_ID & 0xff; + break; default: /* ignore */ break; } @@ -840,6 +887,7 @@ case EMULATE_SST_SST25VF032B: case EMULATE_MACRONIX_MX25L6436: case EMULATE_WINBOND_W25Q128FV: + case EMULATE_VARIABLE_SIZE: if (emulate_spi_chip_response(writecnt, readcnt, writearr, readarr)) { msg_pdbg("Invalid command sent to flash chip!\n"); @@ -862,3 +910,44 @@ return spi_write_chunked(flash, buf, start, len, spi_write_256_chunksize); } + +#if EMULATE_CHIP && EMULATE_SPI_CHIP +int probe_variable_size(struct flashctx *flash) +{ + unsigned int i; + + /* Skip the probing if we don't emulate this chip. */ + if (emu_chip != EMULATE_VARIABLE_SIZE) + return 0; + + /* + * This will break if one day flashctx becomes read-only. + * Once that happens, we need to have special hacks in functions: + * + * erase_and_write_flash() in flashrom.c + * read_flash_to_file() + * handle_romentries() + * ... + * + * Search "total_size * 1024" in code. + */ + flash->chip->total_size = emu_chip_size / 1024; + msg_cdbg("%s: set flash->total_size to %dK bytes.\n", __func__, + flash->chip->total_size); + + /* Update the first count of each of the block_erasers. */ + for (i = 0; i < NUM_ERASEFUNCTIONS; i++) { + struct block_eraser *eraser = &flash->chip->block_erasers[i]; + if (!eraser->block_erase) + break; + + eraser->eraseblocks[0].count = emu_chip_size / + eraser->eraseblocks[0].size; + msg_cdbg("%s: eraser.size=%d, .count=%d\n", + __func__, eraser->eraseblocks[0].size, + eraser->eraseblocks[0].count); + } + + return 1; +} +#endif diff --git a/flashchips.c b/flashchips.c index 8b5b5cc..6fced9e 100644 --- a/flashchips.c +++ b/flashchips.c @@ -18759,6 +18759,27 @@ { .vendor = "Generic", + .name = "Variable Size SPI chip", + .bustype = BUS_SPI, + .manufacture_id = PROGMANUF_ID, + .model_id = PROGDEV_ID, + .total_size = 64, /* This size is set temporarily */ + .page_size = 256, + .tested = TEST_OK_PREW, + .probe = probe_variable_size, + .block_erasers = + { + { + .eraseblocks = { {64 * 1024, 1} }, + .block_erase = spi_block_erase_d8, + } + }, + .write = spi_chip_write_256, + .read = spi_chip_read, + }, + + { + .vendor = "Generic", .name = "unknown SPI chip (RDID)", .bustype = BUS_SPI, .manufacture_id = GENERIC_MANUF_ID, -- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 8 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: merged
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 ) Change subject: support variable-size SPI chip for dummy programmer ...................................................................... Patch Set 8: (2 comments) This doesn't seem like anybody has read this. It breaks the dummyflasher, uses discouraged functions (like atoi()), didn't update the manpage... https://review.coreboot.org/c/flashrom/+/44879/8/dummyflasher.c File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/44879/8/dummyflasher.c@159 PS8, Line 159: VARIOUS VARIABLE_SIZE? https://review.coreboot.org/c/flashrom/+/44879/8/dummyflasher.c@291 PS8, Line 291: return 1; What about all the other emulation targets? I'll stop reading here. -- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 8 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sat, 17 Oct 2020 21:48:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
Namyoon Woo has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 ) Change subject: support variable-size SPI chip for dummy programmer ...................................................................... Patch Set 8:
Patch Set 8:
(2 comments)
This doesn't seem like anybody has read this. It breaks the dummyflasher, uses discouraged functions (like atoi()), didn't update the manpage...
Uploaded https://review.coreboot.org/c/flashrom/+/46536 for fixes. -- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 8 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Sun, 18 Oct 2020 04:08:02 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 ) Change subject: support variable-size SPI chip for dummy programmer ...................................................................... Patch Set 8:
This doesn't seem like anybody has read this. It breaks the dummyflasher, uses discouraged functions (like atoi()), didn't update the manpage...
Uploaded https://review.coreboot.org/c/flashrom/+/46536 for fixes.
Thanks for the immediate reaction! And sorry for the harsh words. I'm sometimes too tense because I see the community cleaning up behind such things. Really, much appreciate that you care about it after merge :) -- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 8 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Mon, 19 Oct 2020 14:16:12 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Namyoon Woo has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 ) Change subject: support variable-size SPI chip for dummy programmer ...................................................................... Patch Set 8:
Patch Set 8:
This doesn't seem like anybody has read this. It breaks the dummyflasher, uses discouraged functions (like atoi()), didn't update the manpage...
Uploaded https://review.coreboot.org/c/flashrom/+/46536 for fixes.
Thanks for the immediate reaction! And sorry for the harsh words. I'm sometimes too tense because I see the community cleaning up behind such things. Really, much appreciate that you care about it after merge :)
No problem. Thanks for your feedback! -- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 8 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Mon, 19 Oct 2020 15:52:40 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 ) Change subject: support variable-size SPI chip for dummy programmer ...................................................................... Patch Set 8: (1 comment) Patchset: PS8: Broke building with CONFIG_DUMMY=no; probe_variable_size() would have to move to a chip driver. It needs access to programmer internals, though. That's what the opaque master is there for... Seems all borked. It also casts objects of all programmers to the dummy one. Only bails out by chance. Another 3 hours wasted on a Saturday night, please somebody remove this code. We've been patching this for almost a year now and it's still broken. -- To view, visit https://review.coreboot.org/c/flashrom/+/44879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Gerrit-Change-Number: 44879 Gerrit-PatchSet: 8 Gerrit-Owner: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org> Gerrit-Reviewer: Namyoon Woo <namyoon@google.com> Gerrit-Reviewer: Stefan Reinauer <reinauer@chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-Comment-Date: Sat, 26 Jun 2021 22:55:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
participants (4)
-
Angel Pons (Code Review) -
Edward O'Callaghan (Code Review)
-
Namyoon Woo (Code Review)
-
Nico Huber (Code Review)