April 2021 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: cli_classic: Make -r/-w/-v argument optional
by Daniel Campello (Code Review) 30 Apr '21

30 Apr '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/52362 to look at the new patch set (#14). Change subject: cli_classic: Make -r/-w/-v argument optional ...................................................................... cli_classic: Make -r/-w/-v argument optional Makes filename optional from -r/-w/-v since the -i parameter allows building the image to be written from multiple files, and regions to be read from flash and written to separate image files, Signed-off-by: Daniel Campello <campello(a)chromium.org> Change-Id: I15384b92cb987a6ea1bb4369ef415488b9cf7f41 --- M cli_classic.c M flashrom.c 2 files changed, 86 insertions(+), 18 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/52362/14 -- To view, visit https://review.coreboot.org/c/flashrom/+/52362 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I15384b92cb987a6ea1bb4369ef415488b9cf7f41 Gerrit-Change-Number: 52362 Gerrit-PatchSet: 14 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: 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-CC: Nikolai Artemiev <nartemiev(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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c
by Anastasia Klimchuk (Code Review) 30 Apr '21

30 Apr '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51487 ) Change subject: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c ...................................................................... Patch Set 10: (5 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/51487/comment/895ea4b0_adbc9c8d PS6, Line 9: hwaccess_io.h : because those are needed to test lifecycle of mec1308.c > Do you mean you would like to see more tests that are using mocked i/o, to ensure the approach works […] I added test for ene_lpc.c, to verify the io mocking framework is working for different drivers, and yes it is! Moreover, ene's state is more complicated, it needs to track two values. I picked ene almost at random, but it is actually a good example that framework can deal with different states. Patchset: PS1: > flashbuses_to_text_test_success() was only introduced to get the ball rolling on starting to think a […] I would keep them until I have a better replacement, what do you think? Patchset: PS10: And now we have very cool io mocking framework, and two tests that are using it! This definitely needs to be split into 3 patches (framework, mec test, ene test) but I thought it would be good to have a first look at the whole picture. Also thank you to everyone! File tests/tests.c: https://review.coreboot.org/c/flashrom/+/51487/comment/d2e996c9_8f09c4c2 PS6, Line 88: /* : * Uncomment LOG_ME below for verbose logging. : * It is commented by default because of too many calls. : */ : /* LOG_ME; */ > So far I was able to print everything or nothing, but I will look more into this. […] I haven't done anything with verbosity yet, a general question: is it possible to leave this for the next patch? I fully understand this comment is not a final solution! what do you think? https://review.coreboot.org/c/flashrom/+/51487/comment/1f7c7fef_d1b58928 PS6, Line 102: return ((port == 0x2e /* MEC1308_SIO_PORT1 */ : || port == 0x4e /* MEC1308_SIO_PORT2 */) : ? 0x20 /* MEC1308_DEVICE_ID_REG */ : : ((outb_val == 0x84 /* MEC1308_MBX_DATA_START */) : ? 0xaa /* MEC1308_CMD_PASSTHRU_SUCCESS */ : : 0)); > So, CB:52604 is what I had in mind. Feel free to take it over if […] This is awesome, and it works! Thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/51487 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc Gerrit-Change-Number: 51487 Gerrit-PatchSet: 10 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 30 Apr 2021 00:23:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c
by Anastasia Klimchuk (Code Review) 30 Apr '21

30 Apr '21

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/51487 to look at the new patch set (#10). Change subject: tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c ...................................................................... tests: Add unit test to run init/shutdown for mec1308.c, ene_lpc.c This patch includes mocks for io operations in hwaccess_x86_io.h because those are needed to test lifecycle of mec1308.c and ene_lpc.c BUG=b:181803212 TEST=builds and ninja test Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- A hwaccess_x86_io_unittest.h M meson.build M tests/init_shutdown.c A tests/io_mock.h M tests/meson.build M tests/tests.c M tests/tests.h 7 files changed, 246 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/87/51487/10 -- To view, visit https://review.coreboot.org/c/flashrom/+/51487 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3af612defe1af3850dfc1626a208d873e3a3eddc Gerrit-Change-Number: 51487 Gerrit-PatchSet: 10 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: flashrom.c: allow - as filename for stdin
by Edward O'Callaghan (Code Review) 30 Apr '21

30 Apr '21

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

1 0

Change in flashrom[master]: ft2232_spi.c: Implement spi_send_multicommand()
by Simon Buhrow (Code Review) 29 Apr '21

29 Apr '21

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

1 0

Change in flashrom[master]: ft2232_spi.c: Implement spi_send_multicommand()
by Simon Buhrow (Code Review) 29 Apr '21

29 Apr '21

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

1 0

Change in flashrom[master]: ft2232_spi.c: Implement spi_send_multicommand()
by Simon Buhrow (Code Review) 29 Apr '21

29 Apr '21

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

1 0

Change in flashrom[master]: ene_lpc.c: Untangle successful vs failed init paths
by Nico Huber (Code Review) 29 Apr '21

29 Apr '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52685 ) Change subject: ene_lpc.c: Untangle successful vs failed init paths ...................................................................... ene_lpc.c: Untangle successful vs failed init paths Exit label now serves as failed init path, it does cleanup and returns 1, so it is renamed into init_err_exit. Since all error paths return 1, and successful init is separated from failure, there is no need to have ret variable anymore. BUG=b:185191942 TEST=builds Change-Id: Iac295f1353785cd73d7cb2f19e4a8cbb69beb576 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/52685 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Nico Huber <nico.h(a)gmx.de> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M ene_lpc.c 1 file changed, 14 insertions(+), 15 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve diff --git a/ene_lpc.c b/ene_lpc.c index c05fef8..b66e96e 100644 --- a/ene_lpc.c +++ b/ene_lpc.c @@ -529,7 +529,6 @@ int ene_lpc_init() { uint8_t hwver, ediid, i; - int ret = 0; ene_lpc_data_t *ctx_data = NULL; msg_pdbg("%s\n", __func__); @@ -541,10 +540,8 @@ } ctx_data->ec_state = EC_STATE_NORMAL; - if (check_params()) { - ret = 1; - goto ene_probe_spi_flash_exit; - } + if (check_params()) + goto init_err_exit; for (i = 0; i < ENE_LAST; ++i) { ctx_data->chip = &ene_chips[i]; @@ -560,8 +557,7 @@ if (i == ENE_LAST) { msg_pdbg("ENE EC not found (probe failed)\n"); - ret = 1; - goto ene_probe_spi_flash_exit; + goto init_err_exit; } /* TODO: probe the EC stop protocol @@ -574,17 +570,20 @@ internal_buses_supported |= BUS_LPC; spi_master_ene.data = ctx_data; - if (register_shutdown(ene_leave_flash_mode, ctx_data)) { - ret = 1; - goto ene_probe_spi_flash_exit; - } + if (register_shutdown(ene_leave_flash_mode, ctx_data)) + goto init_err_cleanup_exit; register_spi_master(&spi_master_ene); msg_pdbg("%s: successfully initialized ene\n", __func__); -ene_probe_spi_flash_exit: - if (ret) - free(ctx_data); - return ret; + return 0; + +init_err_cleanup_exit: + ene_leave_flash_mode(ctx_data); + return 1; + +init_err_exit: + free(ctx_data); + return 1; } #endif /* __i386__ || __x86_64__ */ -- 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: 4 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-MessageType: merged

1 0

Change in flashrom[master]: ene_lpc.c: Untangle successful vs failed init paths
by Nico Huber (Code Review) 29 Apr '21

29 Apr '21

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

1 0

Change in flashrom[master]: lspcon, realtek_mst: move i2c bus resolution from params to i2c_helper
by Peter Marheine (Code Review) 29 Apr '21

29 Apr '21

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

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2021