Namyoon Woo has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/46536 )
Change subject: dummyflasher: fixes on VARIABLE_SIZE chip support ......................................................................
dummyflasher: fixes on VARIABLE_SIZE chip support
This patches fixes a few bugs that two patches (https://review.coreboot.org/c/flashrom/+/44879 and https://review.coreboot.org/c/flashrom/+/45230) brought:
- dummy_init() used to fail for the absence of "size" argument, which broke the run on non VARIABLE_SIZE emulation target. Now the run fails without "size" only for VARIABLE_SIZE. - probe_variable_size() used to fail if the bus type is either PARALLEL, LPC or FWH because flash->mst->par.data is null. Now dummy_init() sets up par_master_dummy.data as well as spi_master_dummyflasher.data. - atoi() is replaced with strtol(). - man page is revised to describe how to use VARIABLE_SIZE emulation target.
TEST: $ flashrom -p dummy:image=dummy.bin,emulate=VARIABLE_SIZE,size=16777216 \ -w ${IMG} -V -f ... Verifying flash... VERIFIED. Writing dummy.bin
$ flashrom -p dummy:image=dummy.bin,emulate=VARIABLE_SIZE -w ${IMG} -V -f ... dummy_init: the size parameter is not given. Unhandled programmer parameters (possibly due to another failure): image=dummy.bin, Error: Programmer initialization failed
$ flashrom -p dummy:image=dummy.bin,emulate=SST25VF040.REMS -c SST25LF040A -w ${IMG} ... Erasing and writing flash chip... Erase/write done. Verifying flash... VERIFIED.
Signed-off-by: Namyoon Woo namyoon@google.com Change-Id: Ie6481943a831b946a91b643b4d79e684c27e48b8 --- M dummyflasher.c M flashrom.8.tmpl 2 files changed, 20 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/36/46536/1
diff --git a/dummyflasher.c b/dummyflasher.c index 6426ad2..93d0635 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -126,7 +126,7 @@ .write_aai = default_spi_write_aai, };
-static const struct par_master par_master_dummy = { +static struct par_master par_master_dummy = { .chip_readb = dummy_chip_readb, .chip_readw = dummy_chip_readw, .chip_readl = dummy_chip_readl, @@ -166,7 +166,7 @@ unsigned int i; #if EMULATE_SPI_CHIP char *status = NULL; - int size = -1; /* size for VARIOUS_SIZE chip device */ + int size = -1; /* size for VARIABLE_SIZE chip device */ #endif #if EMULATE_CHIP struct stat image_stat; @@ -180,6 +180,7 @@ data->emu_chip = EMULATE_NONE; data->delay_us = 0; spi_master_dummyflasher.data = data; + par_master_dummy.data = data;
msg_pspew("%s\n", __func__);
@@ -343,7 +344,7 @@ #if EMULATE_SPI_CHIP tmp = extract_programmer_param("size"); if (tmp) { - size = atoi(tmp); + size = strtol(tmp, NULL, 10); if (size <= 0 || (size % 1024 != 0)) { msg_perr("%s: Chip size is not a multipler of 1024: %s\n", __func__, tmp); @@ -351,9 +352,6 @@ return 1; } free(tmp); - } else { - msg_perr("%s: the size parameter is not given.\n", __func__); - return 1; } #endif
@@ -433,6 +431,11 @@ * flashrom -p dummy:emulate=VARIABLE_SIZE,size=4194304 */ if (!strcmp(tmp, "VARIABLE_SIZE")) { + if (size == -1) { + msg_perr("%s: the size parameter is not given.\n", __func__); + free(tmp); + return 1; + } data->emu_chip = EMULATE_VARIABLE_SIZE; data->emu_chip_size = size; data->emu_max_byteprogram_size = 256; diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl index db50d59..5df0aff 100644 --- a/flashrom.8.tmpl +++ b/flashrom.8.tmpl @@ -646,8 +646,19 @@ .sp .RB "* Macronix " MX25L6436 " SPI flash chip (8192 kB, RDID, SFDP)" .sp +.RB "* Dummy vendor " VARIABLE_SIZE " SPI flash chip (configurable size, page write)" +.sp Example: .B "flashrom -p dummy:emulate=SST25VF040.REMS" +.sp +To use +.B VARIABLE_SIZE +chip, +.B size +must be specified to configure the size of the flash chip, which is a power of two. +.sp +Example: +.B "flashrom -p dummy:emulate=VARIABLE_SIZE,size=16777216,image=dummy.bin" .TP .B Persistent images .sp
Namyoon Woo has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46536 )
Change subject: dummyflasher: fixes on VARIABLE_SIZE chip support ......................................................................
Patch Set 1: Code-Review+1
Namyoon Woo has removed a vote from this change. ( https://review.coreboot.org/c/flashrom/+/46536 )
Change subject: dummyflasher: fixes on VARIABLE_SIZE chip support ......................................................................
Removed Code-Review+1 by Namyoon Woo namyoon@google.com
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46536 )
Change subject: dummyflasher: fixes on VARIABLE_SIZE chip support ......................................................................
Patch Set 1:
(6 comments)
https://review.coreboot.org/c/flashrom/+/46536/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/flashrom/+/46536/1//COMMIT_MSG@7 PS1, Line 7: fixes on VARIABLE_SIZE chip support dummyflasher.c: Fix null par data and size param handling
https://review.coreboot.org/c/flashrom/+/46536/1//COMMIT_MSG@10 PS1, Line 10: (https://review.coreboot.org/c/flashrom/+/44879 Could you use the commit hashes instead of the links like
`3149822cd45cb2e5841e15d648783748ba1b2ec6` && `3b8fe0f8e907c0ba9f7c7935e950f3e1538d427f`
https://review.coreboot.org/c/flashrom/+/46536/1//COMMIT_MSG@13 PS1, Line 13: - dummy_init() used to fail for the absence of "size" argument, which broke : the run on non VARIABLE_SIZE emulation target. Now the run fails : without "size" only for VARIABLE_SIZE. Format these with an indent `*`.
* dummy_init() should not fail in the absence of the size param as this breaks non-VARIABLE_SIZE emulation functionality.
https://review.coreboot.org/c/flashrom/+/46536/1//COMMIT_MSG@16 PS1, Line 16: - probe_variable_size() used to fail if the bus type is either PARALLEL, LPC or FWH : because flash->mst->par.data is null. Now dummy_init() sets up par_master_dummy.data * probe_variable_size() should fail if bus type is 'PARALLEL, LPC or FWH' however 'flash->mst->par.data' is null. Fix init fn to initialise it.
https://review.coreboot.org/c/flashrom/+/46536/1//COMMIT_MSG@20 PS1, Line 20: - man page is revised to describe how to use VARIABLE_SIZE emulation : target. * Revise man page to describe how to use the VARIABLE_SIZE emulation target.
https://review.coreboot.org/c/flashrom/+/46536/1/flashrom.8.tmpl File flashrom.8.tmpl:
https://review.coreboot.org/c/flashrom/+/46536/1/flashrom.8.tmpl@658 PS1, Line 658: , which is a power of two. as a power of two.
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Lachlan Bishop,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/46536
to look at the new patch set (#2).
Change subject: dummyflasher.c: Fix null par data and size param handling ......................................................................
dummyflasher.c: Fix null par data and size param handling
This patches fixes a few bugs that two patches ( `3149822cd45cb2e5841e15d648783748ba1b2ec6` && `3b8fe0f8e907c0ba9f7c7935e950f3e1538d427f`) brought: * Check the presence of 'size' param only if the emulate is VARIABLE_SIZE. * Initialize 'flash->st->par.data' in dummy_init() so that it can probe the VARIABLE_SIZE emulator correct in probe_variable_size(). * Replace atoi() with strtol(). * Revise man page to describe how to use the VARIABLE_SIZE emulation target.
TEST: $ flashrom -p dummy:image=dummy.bin,emulate=VARIABLE_SIZE,size=16777216 \ -w ${IMG} -V -f ... Verifying flash... VERIFIED. Writing dummy.bin
$ flashrom -p dummy:image=dummy.bin,emulate=VARIABLE_SIZE -w ${IMG} -V -f ... dummy_init: the size parameter is not given. Unhandled programmer parameters (possibly due to another failure): image=dummy.bin, Error: Programmer initialization failed
$ flashrom -p dummy:image=dummy.bin,emulate=SST25VF040.REMS -c SST25LF040A -w ${IMG} ... Erasing and writing flash chip... Erase/write done. Verifying flash... VERIFIED.
$ man flashrom ... * Dummy vendor VARIABLE_SIZE SPI flash chip (configurable size, page write)
Example: flashrom -p dummy:emulate=SST25VF040.REMS
To use VARIABLE_SIZE chip, size must be specified to configure the size of the flash chip as a power of two.
Example: flashrom -p dummy:emulate=VARIABLE_SIZE,size=16777216,image=dummy.bin ...
Signed-off-by: Namyoon Woo namyoon@google.com Change-Id: Ie6481943a831b946a91b643b4d79e684c27e48b8 --- M dummyflasher.c M flashrom.8.tmpl 2 files changed, 20 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/36/46536/2
Namyoon Woo has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46536 )
Change subject: dummyflasher.c: Fix null par data and size param handling ......................................................................
Patch Set 2:
(6 comments)
https://review.coreboot.org/c/flashrom/+/46536/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/flashrom/+/46536/1//COMMIT_MSG@7 PS1, Line 7: fixes on VARIABLE_SIZE chip support
dummyflasher. […]
Done
https://review.coreboot.org/c/flashrom/+/46536/1//COMMIT_MSG@10 PS1, Line 10: (https://review.coreboot.org/c/flashrom/+/44879
Could you use the commit hashes instead of the links like […]
Done
https://review.coreboot.org/c/flashrom/+/46536/1//COMMIT_MSG@13 PS1, Line 13: - dummy_init() used to fail for the absence of "size" argument, which broke : the run on non VARIABLE_SIZE emulation target. Now the run fails : without "size" only for VARIABLE_SIZE.
Format these with an indent `*`. […]
Done
https://review.coreboot.org/c/flashrom/+/46536/1//COMMIT_MSG@16 PS1, Line 16: - probe_variable_size() used to fail if the bus type is either PARALLEL, LPC or FWH : because flash->mst->par.data is null. Now dummy_init() sets up par_master_dummy.data
- probe_variable_size() should fail if bus type is 'PARALLEL, LPC or FWH' however 'flash->mst->par. […]
probe_variable_size() should NOT fail whatever the bus type is. But the patch '3b8fe0f8e907c0ba9f7c7935e950f3e1538d427f' neglected to initialize 'flash->mst->par.data', and it broke the VARIABLE_SIZE dummy flash run.
https://review.coreboot.org/c/flashrom/+/46536/1//COMMIT_MSG@20 PS1, Line 20: - man page is revised to describe how to use VARIABLE_SIZE emulation : target.
- Revise man page to describe how to use the VARIABLE_SIZE emulation target.
Done
https://review.coreboot.org/c/flashrom/+/46536/1/flashrom.8.tmpl File flashrom.8.tmpl:
https://review.coreboot.org/c/flashrom/+/46536/1/flashrom.8.tmpl@658 PS1, Line 658: , which is a power of two.
as a power of two.
Done
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/46536 )
Change subject: dummyflasher.c: Fix null par data and size param handling ......................................................................
Patch Set 2: Code-Review+2
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Lachlan Bishop,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/46536
to look at the new patch set (#3).
Change subject: dummyflasher.c: Fix null par data and size param handling ......................................................................
dummyflasher.c: Fix null par data and size param handling
This patch fixes a few bugs that two patches ( `3149822cd45cb2e5841e15d648783748ba1b2ec6` && `3b8fe0f8e907c0ba9f7c7935e950f3e1538d427f`) brought: * Check the presence of 'size' param only if the emulate is VARIABLE_SIZE. * Initialize 'flash->st->par.data' in dummy_init() so that it can probe the VARIABLE_SIZE emulator correct in probe_variable_size(). * Replace atoi() with strtol(). * Revise man page to describe how to use the VARIABLE_SIZE emulation target.
TEST: $ flashrom -p dummy:image=dummy.bin,emulate=VARIABLE_SIZE,size=16777216 \ -w ${IMG} -V -f ... Verifying flash... VERIFIED. Writing dummy.bin
$ flashrom -p dummy:image=dummy.bin,emulate=VARIABLE_SIZE -w ${IMG} -V -f ... dummy_init: the size parameter is not given. Unhandled programmer parameters (possibly due to another failure): image=dummy.bin, Error: Programmer initialization failed
$ flashrom -p dummy:image=dummy.bin,emulate=SST25VF040.REMS -c SST25LF040A -w ${IMG} ... Erasing and writing flash chip... Erase/write done. Verifying flash... VERIFIED.
$ man flashrom ... * Dummy vendor VARIABLE_SIZE SPI flash chip (configurable size, page write)
Example: flashrom -p dummy:emulate=SST25VF040.REMS
To use VARIABLE_SIZE chip, size must be specified to configure the size of the flash chip as a power of two.
Example: flashrom -p dummy:emulate=VARIABLE_SIZE,size=16777216,image=dummy.bin ...
Signed-off-by: Namyoon Woo namyoon@google.com Change-Id: Ie6481943a831b946a91b643b4d79e684c27e48b8 --- M dummyflasher.c M flashrom.8.tmpl 2 files changed, 20 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/36/46536/3
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/46536 )
Change subject: dummyflasher.c: Fix null par data and size param handling ......................................................................
dummyflasher.c: Fix null par data and size param handling
This patch fixes a few bugs that two patches ( `3149822cd45cb2e5841e15d648783748ba1b2ec6` && `3b8fe0f8e907c0ba9f7c7935e950f3e1538d427f`) brought: * Check the presence of 'size' param only if the emulate is VARIABLE_SIZE. * Initialize 'flash->st->par.data' in dummy_init() so that it can probe the VARIABLE_SIZE emulator correct in probe_variable_size(). * Replace atoi() with strtol(). * Revise man page to describe how to use the VARIABLE_SIZE emulation target.
TEST: $ flashrom -p dummy:image=dummy.bin,emulate=VARIABLE_SIZE,size=16777216 \ -w ${IMG} -V -f ... Verifying flash... VERIFIED. Writing dummy.bin
$ flashrom -p dummy:image=dummy.bin,emulate=VARIABLE_SIZE -w ${IMG} -V -f ... dummy_init: the size parameter is not given. Unhandled programmer parameters (possibly due to another failure): image=dummy.bin, Error: Programmer initialization failed
$ flashrom -p dummy:image=dummy.bin,emulate=SST25VF040.REMS -c SST25LF040A -w ${IMG} ... Erasing and writing flash chip... Erase/write done. Verifying flash... VERIFIED.
$ man flashrom ... * Dummy vendor VARIABLE_SIZE SPI flash chip (configurable size, page write)
Example: flashrom -p dummy:emulate=SST25VF040.REMS
To use VARIABLE_SIZE chip, size must be specified to configure the size of the flash chip as a power of two.
Example: flashrom -p dummy:emulate=VARIABLE_SIZE,size=16777216,image=dummy.bin ...
Signed-off-by: Namyoon Woo namyoon@google.com Change-Id: Ie6481943a831b946a91b643b4d79e684c27e48b8 Reviewed-on: https://review.coreboot.org/c/flashrom/+/46536 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Edward O'Callaghan quasisec@chromium.org --- M dummyflasher.c M flashrom.8.tmpl 2 files changed, 20 insertions(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved
diff --git a/dummyflasher.c b/dummyflasher.c index 32a00ab..82aa3c8 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -126,7 +126,7 @@ .write_aai = default_spi_write_aai, };
-static const struct par_master par_master_dummy = { +static struct par_master par_master_dummy = { .chip_readb = dummy_chip_readb, .chip_readw = dummy_chip_readw, .chip_readl = dummy_chip_readl, @@ -166,7 +166,7 @@ unsigned int i; #if EMULATE_SPI_CHIP char *status = NULL; - int size = -1; /* size for VARIOUS_SIZE chip device */ + int size = -1; /* size for VARIABLE_SIZE chip device */ #endif #if EMULATE_CHIP struct stat image_stat; @@ -180,6 +180,7 @@ data->emu_chip = EMULATE_NONE; data->delay_us = 0; spi_master_dummyflasher.data = data; + par_master_dummy.data = data;
msg_pspew("%s\n", __func__);
@@ -343,7 +344,7 @@ #if EMULATE_SPI_CHIP tmp = extract_programmer_param("size"); if (tmp) { - size = atoi(tmp); + size = strtol(tmp, NULL, 10); if (size <= 0 || (size % 1024 != 0)) { msg_perr("%s: Chip size is not a multipler of 1024: %s\n", __func__, tmp); @@ -351,9 +352,6 @@ return 1; } free(tmp); - } else { - msg_perr("%s: the size parameter is not given.\n", __func__); - return 1; } #endif
@@ -433,6 +431,11 @@ * flashrom -p dummy:emulate=VARIABLE_SIZE,size=4194304 */ if (!strcmp(tmp, "VARIABLE_SIZE")) { + if (size == -1) { + msg_perr("%s: the size parameter is not given.\n", __func__); + free(tmp); + return 1; + } data->emu_chip = EMULATE_VARIABLE_SIZE; data->emu_chip_size = size; data->emu_max_byteprogram_size = 256; diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl index db50d59..02f60dc 100644 --- a/flashrom.8.tmpl +++ b/flashrom.8.tmpl @@ -646,8 +646,19 @@ .sp .RB "* Macronix " MX25L6436 " SPI flash chip (8192 kB, RDID, SFDP)" .sp +.RB "* Dummy vendor " VARIABLE_SIZE " SPI flash chip (configurable size, page write)" +.sp Example: .B "flashrom -p dummy:emulate=SST25VF040.REMS" +.sp +To use +.B VARIABLE_SIZE +chip, +.B size +must be specified to configure the size of the flash chip as a power of two. +.sp +Example: +.B "flashrom -p dummy:emulate=VARIABLE_SIZE,size=16777216,image=dummy.bin" .TP .B Persistent images .sp
-
Edward O'Callaghan (Code Review) -
Namyoon Woo (Code Review)