Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/45461 )
Change subject: dummyflasher.c: Upstream ChromiumOS 'freq' param feature ......................................................................
dummyflasher.c: Upstream ChromiumOS 'freq' param feature
This allows the dummyflasher to emulate a bus/chip frequency by passing a delay parameter.
BUG=b:140394053 BRANCH=none TEST=builds and ran with freq passed, ``` └──╼ dd if=/dev/urandom of=/tmp/bar bs=2K count=1 1+0 records in 1+0 records out 2048 bytes (2.0 kB, 2.0 KiB) copied, 0.000583308 s, 3.5 MB/s
└──╼ ./flashrom -p dummy:image=/tmp/foo,bus=spi,freq=100Hz,size=2048,emulate=VARIABLE_SIZE -w /tmp/bar flashrom v1.2-105-g702c58a-dirty on Linux 5.7.10-1rodete2-amd64 (x86_64) flashrom is free software, get the source code at https://flashrom.org
Using clock_gettime for delay loops (clk_id: 1, resolution: 1ns). Found Generic flash chip "Variable Size SPI chip" (2 kB, SPI) on dummy. Reading old flash chip contents... done. Erasing and writing flash chip... Erase/write done. Verifying flash... VERIFIED. ```
Change-Id: I1c2702b9e0cae860f5f03114e307707d4d3219af Signed-off-by: Edward O'Callaghan quasisec@google.com --- M dummyflasher.c 1 file changed, 49 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/61/45461/1
diff --git a/dummyflasher.c b/dummyflasher.c index 89e8ba4..8af982d 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -54,6 +54,9 @@ unsigned int emu_chip_size; int emu_modified; /* is the image modified since reading it? */ uint8_t emu_status; + /* If "freq" parameter is passed in from command line, commands will delay + * for this period before returning. */ + unsigned long int delay_us; unsigned int emu_max_byteprogram_size; unsigned int emu_max_aai_size; unsigned int emu_jedec_se_size; @@ -175,6 +178,7 @@ return 1; } data->emu_chip = EMULATE_NONE; + data->delay_us = 0; spi_master_dummyflasher.data = data;
msg_pspew("%s\n", __func__); @@ -289,6 +293,49 @@ } free(tmp);
+ /* frequency to emulate in Hz (default), KHz, or MHz */ + tmp = extract_programmer_param("freq"); + if (tmp) { + unsigned long int freq; + char *units = tmp; + char *end = tmp + strlen(tmp); + + errno = 0; + freq = strtoul(tmp, &units, 0); + if (errno) { + msg_perr("Invalid frequency "%s", %s\n", + tmp, strerror(errno)); + goto dummy_init_out; + } + + if ((units > tmp) && (units < end)) { + int units_valid = 0; + + if (units < end - 3) { + ; + } else if (units == end - 2) { + if (!strcasecmp(units, "hz")) + units_valid = 1; + } else if (units == end - 3) { + if (!strcasecmp(units, "khz")) { + freq *= 1000; + units_valid = 1; + } else if (!strcasecmp(units, "mhz")) { + freq *= 1000000; + units_valid = 1; + } + } + + if (!units_valid) { + msg_perr("Invalid units: %s\n", units); + return 1; + } + } + + /* Assume we only work with bytes and transfer at 1 bit/Hz */ + data->delay_us = (1000000 * 8) / freq; + } + #if EMULATE_CHIP #if EMULATE_SPI_CHIP tmp = extract_programmer_param("size"); @@ -963,6 +1010,8 @@ for (i = 0; i < readcnt; i++) msg_pspew(" 0x%02x", readarr[i]); msg_pspew("\n"); + + programmer_delay((writecnt + readcnt) * emu_data->delay_us); return 0; }
Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/45461 )
Change subject: dummyflasher.c: Upstream ChromiumOS 'freq' param feature ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/flashrom/+/45461/1/dummyflasher.c File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/45461/1/dummyflasher.c@297 PS1, Line 297: tmp I don't see this use of tmp being freed anywhere.
https://review.coreboot.org/c/flashrom/+/45461/1/dummyflasher.c@308 PS1, Line 308: goto dummy_init_out; Should this be return 1 like the other error case?
Hello Sam McNally, build bot (Jenkins), Patrick Georgi, Namyoon Woo, Nikolai Artemiev,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/45461
to look at the new patch set (#2).
Change subject: dummyflasher.c: Upstream ChromiumOS 'freq' param feature ......................................................................
dummyflasher.c: Upstream ChromiumOS 'freq' param feature
This allows the dummyflasher to emulate a bus/chip frequency by passing a delay parameter.
BUG=b:140394053 BRANCH=none TEST=builds and ran with freq passed, ``` └──╼ dd if=/dev/urandom of=/tmp/bar bs=2K count=1 1+0 records in 1+0 records out 2048 bytes (2.0 kB, 2.0 KiB) copied, 0.000583308 s, 3.5 MB/s
└──╼ ./flashrom -p dummy:image=/tmp/foo,bus=spi,freq=100Hz,size=2048,emulate=VARIABLE_SIZE -w /tmp/bar flashrom v1.2-105-g702c58a-dirty on Linux 5.7.10-1rodete2-amd64 (x86_64) flashrom is free software, get the source code at https://flashrom.org
Using clock_gettime for delay loops (clk_id: 1, resolution: 1ns). Found Generic flash chip "Variable Size SPI chip" (2 kB, SPI) on dummy. Reading old flash chip contents... done. Erasing and writing flash chip... Erase/write done. Verifying flash... VERIFIED. ```
Change-Id: I1c2702b9e0cae860f5f03114e307707d4d3219af Signed-off-by: Edward O'Callaghan quasisec@google.com --- M dummyflasher.c 1 file changed, 52 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/61/45461/2
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/45461 )
Change subject: dummyflasher.c: Upstream ChromiumOS 'freq' param feature ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/flashrom/+/45461/1/dummyflasher.c File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/45461/1/dummyflasher.c@297 PS1, Line 297: tmp
I don't see this use of tmp being freed anywhere.
Agreed, typical cros fork seems buggy here so fixed those leaks now.
https://review.coreboot.org/c/flashrom/+/45461/1/dummyflasher.c@308 PS1, Line 308: goto dummy_init_out;
Should this be return 1 like the other error case?
yes.
Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/45461 )
Change subject: dummyflasher.c: Upstream ChromiumOS 'freq' param feature ......................................................................
Patch Set 2: Code-Review+2
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/45461 )
Change subject: dummyflasher.c: Upstream ChromiumOS 'freq' param feature ......................................................................
dummyflasher.c: Upstream ChromiumOS 'freq' param feature
This allows the dummyflasher to emulate a bus/chip frequency by passing a delay parameter.
BUG=b:140394053 BRANCH=none TEST=builds and ran with freq passed, ``` └──╼ dd if=/dev/urandom of=/tmp/bar bs=2K count=1 1+0 records in 1+0 records out 2048 bytes (2.0 kB, 2.0 KiB) copied, 0.000583308 s, 3.5 MB/s
└──╼ ./flashrom -p dummy:image=/tmp/foo,bus=spi,freq=100Hz,size=2048,emulate=VARIABLE_SIZE -w /tmp/bar flashrom v1.2-105-g702c58a-dirty on Linux 5.7.10-1rodete2-amd64 (x86_64) flashrom is free software, get the source code at https://flashrom.org
Using clock_gettime for delay loops (clk_id: 1, resolution: 1ns). Found Generic flash chip "Variable Size SPI chip" (2 kB, SPI) on dummy. Reading old flash chip contents... done. Erasing and writing flash chip... Erase/write done. Verifying flash... VERIFIED. ```
Change-Id: I1c2702b9e0cae860f5f03114e307707d4d3219af Signed-off-by: Edward O'Callaghan quasisec@google.com Reviewed-on: https://review.coreboot.org/c/flashrom/+/45461 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Sam McNally sammc@google.com --- M dummyflasher.c 1 file changed, 52 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Sam McNally: Looks good to me, approved
diff --git a/dummyflasher.c b/dummyflasher.c index 89e8ba4..6426ad2 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -54,6 +54,9 @@ unsigned int emu_chip_size; int emu_modified; /* is the image modified since reading it? */ uint8_t emu_status; + /* If "freq" parameter is passed in from command line, commands will delay + * for this period before returning. */ + unsigned long int delay_us; unsigned int emu_max_byteprogram_size; unsigned int emu_max_aai_size; unsigned int emu_jedec_se_size; @@ -175,6 +178,7 @@ return 1; } data->emu_chip = EMULATE_NONE; + data->delay_us = 0; spi_master_dummyflasher.data = data;
msg_pspew("%s\n", __func__); @@ -289,6 +293,52 @@ } free(tmp);
+ /* frequency to emulate in Hz (default), KHz, or MHz */ + tmp = extract_programmer_param("freq"); + if (tmp) { + unsigned long int freq; + char *units = tmp; + char *end = tmp + strlen(tmp); + + errno = 0; + freq = strtoul(tmp, &units, 0); + if (errno) { + msg_perr("Invalid frequency "%s", %s\n", + tmp, strerror(errno)); + free(tmp); + return 1; + } + + if ((units > tmp) && (units < end)) { + int units_valid = 0; + + if (units < end - 3) { + ; + } else if (units == end - 2) { + if (!strcasecmp(units, "hz")) + units_valid = 1; + } else if (units == end - 3) { + if (!strcasecmp(units, "khz")) { + freq *= 1000; + units_valid = 1; + } else if (!strcasecmp(units, "mhz")) { + freq *= 1000000; + units_valid = 1; + } + } + + if (!units_valid) { + msg_perr("Invalid units: %s\n", units); + free(tmp); + return 1; + } + } + + /* Assume we only work with bytes and transfer at 1 bit/Hz */ + data->delay_us = (1000000 * 8) / freq; + } + free(tmp); + #if EMULATE_CHIP #if EMULATE_SPI_CHIP tmp = extract_programmer_param("size"); @@ -963,6 +1013,8 @@ for (i = 0; i < readcnt; i++) msg_pspew(" 0x%02x", readarr[i]); msg_pspew("\n"); + + programmer_delay((writecnt + readcnt) * emu_data->delay_us); return 0; }
-
Edward O'Callaghan (Code Review) -
Sam McNally (Code Review)