Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52383 )
Change subject: flashrom.c: allow - as filename for stdin
......................................................................
flashrom.c: allow - as filename for stdin
Allows - as filename for -w/-v options. It is sometimes useful to
script flashrom and allowing it to work with pipes allows for more
flexibility in this specific use-case.
Signed-off-by: Daniel Campello <campello(a)chromium.org>
Change-Id: I97889cfdf7ba9a257e182c4ee2b20075cfa58d4d
Reviewed-on: https://review.coreboot.org/c/flashrom/+/52383
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org>
---
M cli_classic.c
M flashrom.c
2 files changed, 11 insertions(+), 5 deletions(-)
Approvals:
build bot (Jenkins): Verified
Edward O'Callaghan: Looks good to me, approved
diff --git a/cli_classic.c b/cli_classic.c
index a22d642..7d4dee0 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -48,8 +48,10 @@
printf(" -h | --help print this help text\n"
" -R | --version print version (release)\n"
" -r | --read <file> read flash and save to <file>\n"
- " -w | --write <file> write <file> to flash\n"
- " -v | --verify <file> verify flash against <file>\n"
+ " -w | --write <file|-> write <file> or the content provided\n"
+ " on the standard input to flash\n"
+ " -v | --verify <file|-> verify flash against <file>\n"
+ " or the content provided on the standard input\n"
" -E | --erase erase flash memory\n"
" -V | --verbose more verbose output\n"
" -c | --chip <chipname> probe only for specified flash chip\n"
@@ -107,7 +109,7 @@
return 1;
}
/* Not an error, but maybe the user intended to specify a CLI option instead of a file name. */
- if (filename[0] == '-')
+ if (filename[0] == '-' && filename[1] != '\0')
fprintf(stderr, "Warning: Supplied %s file name starts with -\n", type);
return 0;
}
diff --git a/flashrom.c b/flashrom.c
index 3a733df..e1121d8 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -1348,7 +1348,11 @@
int ret = 0;
FILE *image;
- if ((image = fopen(filename, "rb")) == NULL) {
+ if (!strcmp(filename, "-"))
+ image = fdopen(fileno(stdin), "rb");
+ else
+ image = fopen(filename, "rb");
+ if (image == NULL) {
msg_gerr("Error: opening file \"%s\" failed: %s\n", filename, strerror(errno));
return 1;
}
@@ -1359,7 +1363,7 @@
ret = 1;
goto out;
}
- if (image_stat.st_size != (intmax_t)size) {
+ if ((image_stat.st_size != (intmax_t)size) && strcmp(filename, "-")) {
msg_gerr("Error: Image size (%jd B) doesn't match the expected size (%lu B)!\n",
(intmax_t)image_stat.st_size, size);
ret = 1;
--
To view, visit https://review.coreboot.org/c/flashrom/+/52383
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I97889cfdf7ba9a257e182c4ee2b20075cfa58d4d
Gerrit-Change-Number: 52383
Gerrit-PatchSet: 15
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Jack Rosenthal <jrosenth(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: merged
Attention is currently required from: Nico Huber, Edward O'Callaghan, Samir Ibradžić.
Hello build bot (Jenkins), Alan Green, Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Samir Ibradžić,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/40477
to look at the new patch set (#32).
Change subject: ft2232_spi.c: Implement spi_send_multicommand()
......................................................................
ft2232_spi.c: Implement spi_send_multicommand()
Every ftdi_write_data() call is quite time consuming as the ftdi-chips
seems to take always 2-3ms to respond. This leads to what the comment
already says: Minimize USB transfers by packing as many commands as
possible together. So I packed the WREN command together with the
following operation which can be program or erase operation.
This saves about 1 minute when programming a 128MBit Flash with my
config!
Signed-off-by: Simon Buhrow <simon.buhrow(a)posteo.de>
Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a
---
M ft2232_spi.c
1 file changed, 90 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/40477/32
--
To view, visit https://review.coreboot.org/c/flashrom/+/40477
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a
Gerrit-Change-Number: 40477
Gerrit-PatchSet: 32
Gerrit-Owner: Simon Buhrow
Gerrit-Reviewer: Alan Green <avg(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: Samir Ibradžić <sibradzic(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com>
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: Samir Ibradžić <sibradzic(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Edward O'Callaghan, Samir Ibradžić.
Simon Buhrow has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 )
Change subject: ft2232_spi.c: Implement spi_send_multicommand()
......................................................................
Patch Set 31:
(4 comments)
Patchset:
PS31:
I made some tests with my oscilloscope on the SPI lines. Looks good to me. E.g. the WREN is send in one burst with the page program data.
As chunksize of 270 was set to make page program in one burst but without having WREN in mind the chunksize must be adapted to really benefit of this change. Fot this see review.coreboot.org/c/flashrom/+/43282
File ft2232_spi.c:
https://review.coreboot.org/c/flashrom/+/40477/comment/060915f0_2608122c
PS30, Line 291: <= buffer_size;
> Sorry, looks like you copy-pasted from my comment? Needs tabs.
Done
Yes I did, sorry for forgetting the tabs and all the other indent stuff.
https://review.coreboot.org/c/flashrom/+/40477/comment/0206c0a3_8fd8dc6a
PS30, Line 303: * Minimize FTDI-calls by packing as many commands as possible
> Nit, the line break just seems odd.
Done
https://review.coreboot.org/c/flashrom/+/40477/comment/c74d3031_148a343f
PS30, Line 311: ){
> Missing space before opening brace.
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/40477
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a
Gerrit-Change-Number: 40477
Gerrit-PatchSet: 31
Gerrit-Owner: Simon Buhrow
Gerrit-Reviewer: Alan Green <avg(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: Samir Ibradžić <sibradzic(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com>
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: Samir Ibradžić <sibradzic(a)gmail.com>
Gerrit-Comment-Date: Thu, 29 Apr 2021 11:31:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Simon Buhrow, Edward O'Callaghan, Samir Ibradžić.
Hello Alan Green, build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Samir Ibradžić,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/40477
to look at the new patch set (#31).
Change subject: ft2232_spi.c: Implement spi_send_multicommand()
......................................................................
ft2232_spi.c: Implement spi_send_multicommand()
Every ftdi_write_data() call is quite time consuming as the ftdi-chips
seems to take always 2-3ms to respond. This leads to what the comment
already says: Minimize USB transfers by packing as many commands as
possible together. So I packed the WREN command together with the
following operation which can be program or erase operation.
This saves about 1 minute when programming a 128MBit Flash with my
config!
Signed-off-by: Simon Buhrow <simon.buhrow(a)posteo.de>
Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a
---
M ft2232_spi.c
1 file changed, 90 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/40477/31
--
To view, visit https://review.coreboot.org/c/flashrom/+/40477
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie4a07499ec5ef0af23818593f45dc427285a9e8a
Gerrit-Change-Number: 40477
Gerrit-PatchSet: 31
Gerrit-Owner: Simon Buhrow
Gerrit-Reviewer: Alan Green <avg(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: Samir Ibradžić <sibradzic(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Simon Buhrow
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Samir Ibradžić <sibradzic(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Anastasia Klimchuk.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52685 )
Change subject: ene_lpc.c: Untangle successful vs failed init paths
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
File ene_lpc.c:
https://review.coreboot.org/c/flashrom/+/52685/comment/c57f0f63_d8057e63
PS3, Line 582: return 1;
> No because ene_leave_flash_mode frees ctx_data and there's nothing more to do after it. […]
Oooooh, thanks for correcting me. A bit ugly situation, I trusted
the function name which implies for me that it's symmetrical to
ene_enter_flash_mode().
--
To view, visit https://review.coreboot.org/c/flashrom/+/52685
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Iac295f1353785cd73d7cb2f19e4a8cbb69beb576
Gerrit-Change-Number: 52685
Gerrit-PatchSet: 3
Gerrit-Owner: 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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Victor Ding <victording(a)google.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Thu, 29 Apr 2021 07:09:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Sam McNally, Angel Pons.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52405 )
Change subject: lspcon, realtek_mst: move i2c bus resolution from params to i2c_helper
......................................................................
Patch Set 9:
(1 comment)
File i2c_helper_linux.c:
https://review.coreboot.org/c/flashrom/+/52405/comment/0bf45914_ea2dba8c
PS6, Line 42: char *dev = malloc(sizeof(I2C_DEV_PREFIX));
> I see. Let me try another approach...
Have you had any interesting ideas?
It seem like the only alternative is to have the caller provide a buffer, which either needs to be large enough that we assume it's always large enough (PATH_MAX?), or provide a function that returns how long the string we want to return is and let the caller allocate enough space. In the latter case we're just moving who does the allocation so it doesn't seem worth adding the complexity of another function.
--
To view, visit https://review.coreboot.org/c/flashrom/+/52405
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ieea57b2445f3aca16c87d8f839dff131009531a0
Gerrit-Change-Number: 52405
Gerrit-PatchSet: 9
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Thu, 29 Apr 2021 06:17:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52604 )
Change subject: [RFC] tests: Abstract port I/O and factor mec1308 specifics out
......................................................................
Patch Set 1:
(1 comment)
File tests/init_shutdown.c:
https://review.coreboot.org/c/flashrom/+/52604/comment/ef8c6b0f_3a4173bc
PS1, Line 63: .outb = mec1308_outb,
> I'm actually not familiar with our port-i/o using programmer drivers. Without […]
I just wanted to tell you that you are wise person! I am now adding another test with io mocking (for ene_lpc) and yes I need a additional variable in test data struct, a flag which is set in outb and checked/unset in inb.
New test is already working! But I need to make the code look nice and then upload to 51487.
(and then I will split the whole thing into smaller patches, but that's another story).
--
To view, visit https://review.coreboot.org/c/flashrom/+/52604
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ia3a424478456a7c968b3c736ba371d755c494f30
Gerrit-Change-Number: 52604
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Thu, 29 Apr 2021 05:26:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52685 )
Change subject: ene_lpc.c: Untangle successful vs failed init paths
......................................................................
Patch Set 3:
(1 comment)
File ene_lpc.c:
https://review.coreboot.org/c/flashrom/+/52685/comment/99dbdc13_fd7eef24
PS3, Line 582: return 1;
> It should fall through instead, right?
No because ene_leave_flash_mode frees ctx_data and there's nothing more to do after it. The same flow is in mec1308.
One thing ene and mec have in common is that ctx_data is allocated at the very beginning of init function, means all error paths need to free it. Ideally, I want data to be allocated at the very end of init function, just before register_shutdown, and use local variables in most of init function body. I did it in some places where it was trivial, but for ene and mec that would be more diffs and I think to have a dedicated patch for this change. (from my memory dummy has the same issue: data allocated in the beginning of init, and some error paths forget to free it).
--
To view, visit https://review.coreboot.org/c/flashrom/+/52685
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Iac295f1353785cd73d7cb2f19e4a8cbb69beb576
Gerrit-Change-Number: 52685
Gerrit-PatchSet: 3
Gerrit-Owner: 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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-CC: Victor Ding <victording(a)google.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Wed, 28 Apr 2021 22:59:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52719 )
Change subject: mec1308.c: Ensure programmer param variable is always initialised
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
Oops, sorry for the early submission, I missed that this was rather new.
--
To view, visit https://review.coreboot.org/c/flashrom/+/52719
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2d2b3039da2ef185cb31509b3901d56b428688b7
Gerrit-Change-Number: 52719
Gerrit-PatchSet: 2
Gerrit-Owner: 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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Victor Ding <victording(a)google.com>
Gerrit-Comment-Date: Wed, 28 Apr 2021 15:15:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment