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 33:
(1 comment)
Patchset:
PS32:
> > * I´m running flashrom on a virutal machine (don´t think this makes any difference) […]
OK, I understand (I have/had got no idea of VM stuff).
So in case of not using VM, the speed we gain because of less USB communication is consumed by the increased processing time needed to pack the packages?
Feel free to comment if you think this patch is worth to get merged.
--
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: 33
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: Tue, 04 May 2021 08:50:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Simon Buhrow
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 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 (#33).
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 when using flashrom
inside a virtual machine. This leads to what the comment already says:
Minimize USB transfers by packing as many commands as possible together.
This saves about 1 minute when programming a 128MBit Flash with my
VM 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/33
--
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: 33
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: Richard Hughes, Angel Pons, Anastasia Klimchuk, Patrick Rudolph.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49643 )
Change subject: libflashrom: Return progress state to the library user
......................................................................
Patch Set 4:
(1 comment)
File libflashrom.c:
https://review.coreboot.org/c/flashrom/+/49643/comment/ae954e24_dea0062a
PS4, Line 113: private
> This function doesn't look to be private, it is called outside of this file.
private from the libflashrom API, as in "internal" to the flashrom implementation. Not to mean static local. private is a bit of a overloaded term [sic. java/c++].
--
To view, visit https://review.coreboot.org/c/flashrom/+/49643
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c
Gerrit-Change-Number: 49643
Gerrit-PatchSet: 4
Gerrit-Owner: Richard Hughes <richard(a)hughsie.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Richard Hughes <hughsient(a)gmail.com>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Richard Hughes <richard(a)hughsie.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Tue, 04 May 2021 08:15:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Richard Hughes, Angel Pons, Patrick Rudolph.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49643 )
Change subject: libflashrom: Return progress state to the library user
......................................................................
Patch Set 4:
(4 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/49643/comment/19007be0_a919d018
PS4, Line 9: Include test for the dummy spi25 device.
If you could elaborate more on what this commit is doing, it would be great. Maybe add a paragraph explaining the change in more details?
File libflashrom.h:
https://review.coreboot.org/c/flashrom/+/49643/comment/4b938cc4_968242e6
PS4, Line 106: void *
Is there a reason why last argument doesn't have a name here? all other arguments have names.
File libflashrom.c:
https://review.coreboot.org/c/flashrom/+/49643/comment/6676a19e_d98d9810
PS4, Line 113: private
This function doesn't look to be private, it is called outside of this file.
File tests/spi25.c:
https://review.coreboot.org/c/flashrom/+/49643/comment/c5032b8f_f4aa73cb
PS4, Line 54: size_t current,
Thank you for the test! Is there any way to assert what current value is? it should change (unlike total) right?
--
To view, visit https://review.coreboot.org/c/flashrom/+/49643
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c
Gerrit-Change-Number: 49643
Gerrit-PatchSet: 4
Gerrit-Owner: Richard Hughes <richard(a)hughsie.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Richard Hughes <hughsient(a)gmail.com>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Richard Hughes <richard(a)hughsie.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Tue, 04 May 2021 06:43:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
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/+/52877 )
Change subject: buspirate_spi.c: Separate shutdown from failed init cleanup
......................................................................
Patch Set 1:
(1 comment)
File buspirate_spi.c:
https://review.coreboot.org/c/flashrom/+/52877/comment/9ef3d665_9008d396
PS1, Line 672: if (register_shutdown(buspirate_spi_shutdown, NULL) != 0) {
: ret = 1;
: goto init_err_cleanup_exit;
: }
After moving register_shutdown below this has the same situation as CB:52773 , specifically if register_shutdown fails than full cleanup is performed. I use the same approach as we agreed on CB:52773
--
To view, visit https://review.coreboot.org/c/flashrom/+/52877
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I04d57e2552d23a1a4a906c68539f0ccefc13e8a0
Gerrit-Change-Number: 52877
Gerrit-PatchSet: 1
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-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: Tue, 04 May 2021 04:58:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/52877 )
Change subject: buspirate_spi.c: Separate shutdown from failed init cleanup
......................................................................
buspirate_spi.c: Separate shutdown from failed init cleanup
Shutdown function was covering two different jobs here: 1) the actual
shutdown which is run at the end of the driver's lifecycle and
2) cleanup in cases when initialisation failed. Now, shutdown is only
doing its main job (#1), and the driver itself is doing cleanup
when init fails (#2).
The good thing is that now resources are released/closed immediately
in cases when init fails (vs shutdown function which was run at some
point later), and the driver leaves clean space after itself if init
fails.
And very importantly this unlocks API change which plans to move
register_shutdown inside register master API, see
https://review.coreboot.org/c/flashrom/+/51761
BUG=b:185191942
TEST=builds
Change-Id: I04d57e2552d23a1a4a906c68539f0ccefc13e8a0
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M buspirate_spi.c
1 file changed, 71 insertions(+), 49 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/52877/1
diff --git a/buspirate_spi.c b/buspirate_spi.c
index eee8daa..31d36cd 100644
--- a/buspirate_spi.c
+++ b/buspirate_spi.c
@@ -385,13 +385,6 @@
return ret;
}
- if (register_shutdown(buspirate_spi_shutdown, NULL) != 0) {
- bp_commbufsize = 0;
- free(bp_commbuf);
- bp_commbuf = NULL;
- return 1;
- }
-
/* This is the brute force version, but it should work.
* It is likely to fail if a previous flashrom run was aborted during a write with the new SPI commands
* in firmware v5.5 because that firmware may wait for up to 4096 bytes of input before responding to
@@ -403,7 +396,7 @@
/* Send the command, don't read the response. */
ret = buspirate_sendrecv(bp_commbuf, 1, 0);
if (ret)
- return ret;
+ goto init_err_cleanup_exit;
/* The old way to handle responses from a Bus Pirate already in BBIO mode was to flush any
* response which came in over serial. Unfortunately that does not work reliably on Linux
* with FTDI USB-serial.
@@ -416,19 +409,19 @@
}
/* We know that 20 commands of \0 should elicit at least one BBIO1 response. */
if ((ret = buspirate_wait_for_string(bp_commbuf, "BBIO")))
- return ret;
+ goto init_err_cleanup_exit;
/* Reset the Bus Pirate. */
bp_commbuf[0] = 0x0f;
/* Send the command, don't read the response. */
if ((ret = buspirate_sendrecv(bp_commbuf, 1, 0)))
- return ret;
+ goto init_err_cleanup_exit;
if ((ret = buspirate_wait_for_string(bp_commbuf, "irate ")))
- return ret;
+ goto init_err_cleanup_exit;
/* Read the hardware version string. Last byte of the buffer is reserved for \0. */
for (i = 0; i < DEFAULT_BUFSIZE - 1; i++) {
if ((ret = buspirate_sendrecv(bp_commbuf + i, 0, 1)))
- return ret;
+ goto init_err_cleanup_exit;
if (strchr("\r\n\t ", bp_commbuf[i]))
break;
}
@@ -449,11 +442,11 @@
msg_pdbg("\n");
if ((ret = buspirate_wait_for_string(bp_commbuf, "irmware ")))
- return ret;
+ goto init_err_cleanup_exit;
/* Read the firmware version string. Last byte of the buffer is reserved for \0. */
for (i = 0; i < DEFAULT_BUFSIZE - 1; i++) {
if ((ret = buspirate_sendrecv(bp_commbuf + i, 0, 1)))
- return ret;
+ goto init_err_cleanup_exit;
if (strchr("\r\n\t ", bp_commbuf[i]))
break;
}
@@ -474,21 +467,24 @@
msg_pdbg("\n");
if ((ret = buspirate_wait_for_string(bp_commbuf, "HiZ>")))
- return ret;
+ goto init_err_cleanup_exit;
/* Tell the user about missing SPI binary mode in firmware 2.3 and older. */
if (BP_FWVERSION(fw_version_major, fw_version_minor) < BP_FWVERSION(2, 4)) {
msg_pinfo("Bus Pirate firmware 2.3 and older does not support binary SPI access.\n");
msg_pinfo("Please upgrade to the latest firmware (at least 2.4).\n");
- return SPI_PROGRAMMER_ERROR;
+ ret = SPI_PROGRAMMER_ERROR;
+ goto init_err_cleanup_exit;
}
/* Use fast SPI mode in firmware 5.5 and newer. */
if (BP_FWVERSION(fw_version_major, fw_version_minor) >= BP_FWVERSION(5, 5)) {
msg_pdbg("Using SPI command set v2.\n");
/* Sensible default buffer size. */
- if (buspirate_commbuf_grow(260 + 5))
- return ERROR_OOM;
+ if (buspirate_commbuf_grow(260 + 5)) {
+ ret = ERROR_OOM;
+ goto init_err_cleanup_exit;
+ }
spi_master_buspirate.max_data_read = 2048;
spi_master_buspirate.max_data_write = 256;
spi_master_buspirate.command = buspirate_spi_send_command_v2;
@@ -497,8 +493,10 @@
msg_pinfo("Reading/writing a flash chip may take hours.\n");
msg_pinfo("It is recommended to upgrade to firmware 5.5 or newer.\n");
/* Sensible default buffer size. */
- if (buspirate_commbuf_grow(16 + 3))
- return ERROR_OOM;
+ if (buspirate_commbuf_grow(16 + 3)) {
+ ret = ERROR_OOM;
+ goto init_err_cleanup_exit;
+ }
spi_master_buspirate.max_data_read = 12;
spi_master_buspirate.max_data_write = 12;
spi_master_buspirate.command = buspirate_spi_send_command_v1;
@@ -538,37 +536,37 @@
/* Enter baud rate configuration mode */
cnt = snprintf((char *)bp_commbuf, DEFAULT_BUFSIZE, "b\n");
if ((ret = buspirate_sendrecv(bp_commbuf, cnt, 0)))
- return ret;
+ goto init_err_cleanup_exit;
if ((ret = buspirate_wait_for_string(bp_commbuf, ">")))
- return ret;
+ goto init_err_cleanup_exit;
/* Enter manual clock divisor entry mode */
cnt = snprintf((char *)bp_commbuf, DEFAULT_BUFSIZE, "10\n");
if ((ret = buspirate_sendrecv(bp_commbuf, cnt, 0)))
- return ret;
+ goto init_err_cleanup_exit;
if ((ret = buspirate_wait_for_string(bp_commbuf, ">")))
- return ret;
+ goto init_err_cleanup_exit;
/* Set the clock divisor to the value calculated from the user's input */
cnt = snprintf((char *)bp_commbuf, DEFAULT_BUFSIZE, "%d\n",
BP_DIVISOR(serialspeeds[serialspeed_index].speed));
if ((ret = buspirate_sendrecv(bp_commbuf, cnt, 0)))
- return ret;
+ goto init_err_cleanup_exit;
sleep(1);
/* Reconfigure the host's serial baud rate to the new value */
if ((ret = serialport_config(sp_fd, serialspeeds[serialspeed_index].speed))) {
msg_perr("Unable to configure system baud rate to specified value.");
- return ret;
+ goto init_err_cleanup_exit;
}
/* Return to the main prompt */
bp_commbuf[0] = ' ';
if ((ret = buspirate_sendrecv(bp_commbuf, 1, 0)))
- return ret;
+ goto init_err_cleanup_exit;
if ((ret = buspirate_wait_for_string(bp_commbuf, "HiZ>")))
- return ret;
+ goto init_err_cleanup_exit;
msg_pdbg("Serial speed is %d baud\n", serialspeeds[serialspeed_index].speed);
}
@@ -581,30 +579,34 @@
for (i = 0; i < 20; i++) {
bp_commbuf[0] = 0x00;
if ((ret = buspirate_sendrecv(bp_commbuf, 1, 0)))
- return ret;
+ goto init_err_cleanup_exit;
}
if ((ret = buspirate_wait_for_string(bp_commbuf, "BBIO")))
- return ret;
+ goto init_err_cleanup_exit;
if ((ret = buspirate_sendrecv(bp_commbuf, 0, 1)))
- return ret;
+ goto init_err_cleanup_exit;
msg_pdbg("Raw bitbang mode version %c\n", bp_commbuf[0]);
if (bp_commbuf[0] != '1') {
msg_perr("Can't handle raw bitbang mode version %c!\n", bp_commbuf[0]);
- return 1;
+ ret = 1;
+ goto init_err_cleanup_exit;
}
/* Enter raw SPI mode */
bp_commbuf[0] = 0x01;
ret = buspirate_sendrecv(bp_commbuf, 1, 0);
- if (ret)
- return 1;
+ if (ret) {
+ ret = 1;
+ goto init_err_cleanup_exit;
+ }
if ((ret = buspirate_wait_for_string(bp_commbuf, "SPI")))
- return ret;
+ goto init_err_cleanup_exit;
if ((ret = buspirate_sendrecv(bp_commbuf, 0, 1)))
- return ret;
+ goto init_err_cleanup_exit;
msg_pdbg("Raw SPI mode version %c\n", bp_commbuf[0]);
if (bp_commbuf[0] != '1') {
msg_perr("Can't handle raw SPI mode version %c!\n", bp_commbuf[0]);
- return 1;
+ ret = 1;
+ goto init_err_cleanup_exit;
}
/* Initial setup (SPI peripherals config): Enable power, CS high, AUX */
@@ -614,21 +616,27 @@
msg_pdbg("Enabling pull-up resistors.\n");
}
ret = buspirate_sendrecv(bp_commbuf, 1, 1);
- if (ret)
- return 1;
+ if (ret) {
+ ret = 1;
+ goto init_err_cleanup_exit;
+ }
if (bp_commbuf[0] != 0x01) {
msg_perr("Protocol error while setting power/CS/AUX(/Pull-up resistors)!\n");
- return 1;
+ ret = 1;
+ goto init_err_cleanup_exit;
}
/* Set SPI speed */
bp_commbuf[0] = 0x60 | spispeed;
ret = buspirate_sendrecv(bp_commbuf, 1, 1);
- if (ret)
- return 1;
+ if (ret) {
+ ret = 1;
+ goto init_err_cleanup_exit;
+ }
if (bp_commbuf[0] != 0x01) {
msg_perr("Protocol error while setting SPI speed!\n");
- return 1;
+ ret = 1;
+ goto init_err_cleanup_exit;
}
/* Set SPI config: output type, idle, clock edge, sample */
@@ -638,24 +646,38 @@
msg_pdbg("Pull-ups enabled, so using HiZ pin output! (Open-Drain mode)\n");
}
ret = buspirate_sendrecv(bp_commbuf, 1, 1);
- if (ret)
- return 1;
+ if (ret) {
+ ret = 1;
+ goto init_err_cleanup_exit;
+ }
if (bp_commbuf[0] != 0x01) {
msg_perr("Protocol error while setting SPI config!\n");
- return 1;
+ ret = 1;
+ goto init_err_cleanup_exit;
}
/* De-assert CS# */
bp_commbuf[0] = 0x03;
ret = buspirate_sendrecv(bp_commbuf, 1, 1);
- if (ret)
- return 1;
+ if (ret) {
+ ret = 1;
+ goto init_err_cleanup_exit;
+ }
if (bp_commbuf[0] != 0x01) {
msg_perr("Protocol error while raising CS#!\n");
- return 1;
+ ret = 1;
+ goto init_err_cleanup_exit;
}
+ if (register_shutdown(buspirate_spi_shutdown, NULL) != 0) {
+ ret = 1;
+ goto init_err_cleanup_exit;
+ }
register_spi_master(&spi_master_buspirate);
return 0;
+
+init_err_cleanup_exit:
+ buspirate_spi_shutdown(NULL);
+ return ret;
}
--
To view, visit https://review.coreboot.org/c/flashrom/+/52877
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I04d57e2552d23a1a4a906c68539f0ccefc13e8a0
Gerrit-Change-Number: 52877
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newchange
Attention is currently required from: Richard Hughes, Angel Pons, Anastasia Klimchuk, Patrick Rudolph.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49643 )
Change subject: libflashrom: Return progress state to the library user
......................................................................
Patch Set 4: Code-Review+1
(1 comment)
Patchset:
PS4:
I added some reviewers Richard to help try and make progress with libflashrom improvements.
It maybe helpful though to split this into two portions. The first is the framework and test (thanks for writing that!) and the second which hooks up the various programmers.
A little more detail in the commit messages for such a large diff may also invoke shorter review time? Although I'll defer to the communities views on precisely the things they would like to see to help things get reviewed?
--
To view, visit https://review.coreboot.org/c/flashrom/+/49643
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7197572bb7f19e3bdb2bde855d70a0f50fd3854c
Gerrit-Change-Number: 49643
Gerrit-PatchSet: 4
Gerrit-Owner: Richard Hughes <richard(a)hughsie.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Richard Hughes <hughsient(a)gmail.com>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Richard Hughes <richard(a)hughsie.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Comment-Date: Tue, 04 May 2021 03:32:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Daniel Campello.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52876 )
Change subject: flashrom_tester: update syntax for --wp-range
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://review.coreboot.org/c/flashrom/+/52876
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: If17a40fba1f7d41e09e0163b353d1602c215c8db
Gerrit-Change-Number: 52876
Gerrit-PatchSet: 1
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Comment-Date: Tue, 04 May 2021 03:12:13 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk.
Hello build bot (Jenkins), Nico Huber, Angel Pons, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/52364
to look at the new patch set (#3).
Change subject: programmer.h,chipset_enable.c: Make penable cb more descript
......................................................................
programmer.h,chipset_enable.c: Make penable cb more descript
The enable_flash callback function pointer embedded within
the penable struct is used by the chipset_enable entry-point
to dispatch to the correct concrete implementation. The handles
have the canonical form `flash_enable_<chipset-name>()` and
therefore `doit()` is hard to comprehend while groking code
and seeing to what it refers to. It is also a little more
confusing as there use to be another doit() in flashrom
once upon a time.
BUG=none
TEST=builds
Change-Id: I8b3a4ec51ea488c40c20ccafea883f8dea93577d
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M chipset_enable.c
M programmer.h
2 files changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/64/52364/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/52364
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8b3a4ec51ea488c40c20ccafea883f8dea93577d
Gerrit-Change-Number: 52364
Gerrit-PatchSet: 3
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
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