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

Change in flashrom[master]: layout: Add -i <region>[:<file>] support
by Nico Huber (Code Review) 18 Apr '21

18 Apr '21

Attention is currently required from: Sam McNally, Nicola Corna, Jack Rosenthal, Paul Menzel, Stefan Reinauer, David Hendricks, Daniel Campello, Arthur Heymans, Carl-Daniel Hailfinger. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/23021 ) Change subject: layout: Add -i <region>[:<file>] support ...................................................................... Patch Set 33: (1 comment) Patchset: PS27: > (which is the existing message for missing -r/-v/-w on check_filename()) Currently, check_filename() is not even called in this case. -- To view, visit https://review.coreboot.org/c/flashrom/+/23021 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic5465659605d8431d931053967b40290195cfd99 Gerrit-Change-Number: 23021 Gerrit-PatchSet: 33 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: David Hendricks <david.hendricks(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: Nicola Corna <nicola(a)corna.info> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Nicola Corna <nicola(a)corna.info> Gerrit-Attention: Jack Rosenthal <jrosenth(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Comment-Date: Sun, 18 Apr 2021 17:29:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Daniel Campello <campello(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: i2c_helper_linux.c: Use a fixed-size buffer
by Nico Huber (Code Review) 18 Apr '21

18 Apr '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52415 ) Change subject: i2c_helper_linux.c: Use a fixed-size buffer ...................................................................... i2c_helper_linux.c: Use a fixed-size buffer Given that the buffer size is known in advance, using malloc() is not necessary. To avoid open-coding buffer sizes, add some trailing null characters to `I2C_DEV_PREFIX`, as placeholders for bus number digits. Finally, replace the now-unnecessary `goto` statements with `return`. Change-Id: I6060749c6ded10949344caee3cc3943306e74a1c Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/52415 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M i2c_helper_linux.c 1 file changed, 9 insertions(+), 20 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved diff --git a/i2c_helper_linux.c b/i2c_helper_linux.c index 5c191f9..9383c85 100644 --- a/i2c_helper_linux.c +++ b/i2c_helper_linux.c @@ -28,7 +28,8 @@ #include "flash.h" #include "i2c_helper.h" -#define I2C_DEV_PREFIX "/dev/i2c-" +/* Null characters are placeholders for bus number digits */ +#define I2C_DEV_PREFIX "/dev/i2c-\0\0\0" #define I2C_MAX_BUS 255 int i2c_close(int fd) @@ -38,11 +39,11 @@ int i2c_open(int bus, uint16_t addr, int force) { + char dev[sizeof(I2C_DEV_PREFIX)] = {0}; + int ret = -1; int fd = -1; - /* Maximum i2c bus number is 255(3 char), +1 for null terminated string. */ - int path_len = strlen(I2C_DEV_PREFIX) + 4; int request = force ? I2C_SLAVE_FORCE : I2C_SLAVE; if (bus < 0 || bus > I2C_MAX_BUS) { @@ -50,38 +51,26 @@ return ret; } - char *dev = calloc(1, path_len); - if (!dev) { - msg_perr("Unable to allocate space for device name of len %d: %s.\n", - path_len, strerror(errno)); - goto linux_i2c_open_err; - } - - ret = snprintf(dev, path_len, "%s%d", I2C_DEV_PREFIX, bus); + ret = snprintf(dev, sizeof(dev), "%s%d", I2C_DEV_PREFIX, bus); if (ret < 0) { msg_perr("Unable to join bus number to device name: %s.\n", strerror(errno)); - goto linux_i2c_open_err; + return ret; } fd = open(dev, O_RDWR); if (fd < 0) { msg_perr("Unable to open I2C device %s: %s.\n", dev, strerror(errno)); - ret = fd; - goto linux_i2c_open_err; + return fd; } ret = ioctl(fd, request, addr); if (ret < 0) { msg_perr("Unable to set I2C slave address to 0x%02x: %s.\n", addr, strerror(errno)); i2c_close(fd); - goto linux_i2c_open_err; + return ret; } -linux_i2c_open_err: - if (dev) - free(dev); - - return ret ? ret : fd; + return fd; } int i2c_read(int fd, uint16_t addr, i2c_buffer_t *buf) -- To view, visit https://review.coreboot.org/c/flashrom/+/52415 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6060749c6ded10949344caee3cc3943306e74a1c Gerrit-Change-Number: 52415 Gerrit-PatchSet: 2 Gerrit-Owner: 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: Peter Marheine <pmarheine(a)chromium.org> 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]: ene_lpc.c: Clean up cosmetics
by Nico Huber (Code Review) 18 Apr '21

18 Apr '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52413 ) Change subject: ene_lpc.c: Clean up cosmetics ...................................................................... ene_lpc.c: Clean up cosmetics Reflow long lines, drop unnecessary parentheses, add spaces after `if` and `while` keywords and add braces to a single-statement `else if` block that follows a multi-statement `if` block. TEST=Build with `make distclean && make VERSION=none -j` with and without this patch, the flashrom executable does not change. Change-Id: Iaa5277b12fca192c46c11f5e0f375dc43d06bf5c Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/52413 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M ene_lpc.c 1 file changed, 18 insertions(+), 32 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved diff --git a/ene_lpc.c b/ene_lpc.c index 56d6580..993ef4d 100644 --- a/ene_lpc.c +++ b/ene_lpc.c @@ -166,7 +166,7 @@ gettimeofday(&begin, NULL); while (INB(chip->port_ec_command) & MASK_INPUT_BUFFER_FULL) { gettimeofday(&now, NULL); - if ((now.tv_sec - begin.tv_sec) >= EC_COMMAND_TIMEOUT) { + if (now.tv_sec - begin.tv_sec >= EC_COMMAND_TIMEOUT) { msg_pdbg("%s: buf not empty\n", __func__); return; } @@ -178,11 +178,9 @@ if (chip->chip_id == ENE_KB932) { /* Spin wait for EC input buffer empty */ gettimeofday(&begin, NULL); - while (INB(chip->port_ec_command) & - MASK_INPUT_BUFFER_FULL) { + while (INB(chip->port_ec_command) & MASK_INPUT_BUFFER_FULL) { gettimeofday(&now, NULL); - if ((now.tv_sec - begin.tv_sec) >= - EC_COMMAND_TIMEOUT) { + if (now.tv_sec - begin.tv_sec >= EC_COMMAND_TIMEOUT) { msg_pdbg("%s: buf not empty\n", __func__); return; } @@ -295,9 +293,9 @@ struct timeval begin, now; gettimeofday(&begin, NULL); - while(ene_read(chip, REG_SPI_CONFIG) & CFG_STATUS) { + while (ene_read(chip, REG_SPI_CONFIG) & CFG_STATUS) { gettimeofday(&now, NULL); - if ((now.tv_sec - begin.tv_sec) >= EC_COMMAND_TIMEOUT) { + if (now.tv_sec - begin.tv_sec >= EC_COMMAND_TIMEOUT) { msg_pdbg("%s: spi busy\n", __func__); return 1; } @@ -318,17 +316,14 @@ gettimeofday(&begin, NULL); /* Spin wait for EC ready */ - while (ene_read(chip, chip->ec_status_buf) != - chip->ec_is_pausing) { + while (ene_read(chip, chip->ec_status_buf) != chip->ec_is_pausing) { gettimeofday(&now, NULL); - if ((now.tv_sec - begin.tv_sec) >= - EC_COMMAND_TIMEOUT) { + if (now.tv_sec - begin.tv_sec >= EC_COMMAND_TIMEOUT) { msg_pdbg("%s: unable to pause ec\n", __func__); return -1; } } - gettimeofday(&ctx_data->pause_begin, NULL); ctx_data->ec_state = EC_STATE_IDLE; return 0; @@ -346,11 +341,9 @@ ene_write(chip, REG_EC_EXTCMD, 0xff); gettimeofday(&begin, NULL); - while (ene_read(chip, chip->ec_status_buf) != - chip->ec_is_running) { + while (ene_read(chip, chip->ec_status_buf) != chip->ec_is_running) { gettimeofday(&now, NULL); - if ((now.tv_sec - begin.tv_sec) >= - EC_COMMAND_TIMEOUT) { + if (now.tv_sec - begin.tv_sec >= EC_COMMAND_TIMEOUT) { msg_pdbg("%s: unable to resume ec\n", __func__); return -1; } @@ -364,11 +357,9 @@ { struct timeval pause_now; gettimeofday(&pause_now, NULL); - if ((pause_now.tv_sec - ctx_data->pause_begin.tv_sec) >= - EC_PAUSE_TIMEOUT) { - if(ene_resume_ec(ctx_data) == 0) + if (pause_now.tv_sec - ctx_data->pause_begin.tv_sec >= EC_PAUSE_TIMEOUT) { + if (ene_resume_ec(ctx_data) == 0) ene_pause_ec(ctx_data); - } return 0; } @@ -385,11 +376,9 @@ ec_command(chip, chip->ec_reset_cmd, chip->ec_reset_data); /* Spin wait for EC ready */ - while (ene_read(chip, chip->ec_status_buf) != - chip->ec_is_stopping) { + while (ene_read(chip, chip->ec_status_buf) != chip->ec_is_stopping) { gettimeofday(&now, NULL); - if ((now.tv_sec - begin.tv_sec) >= - EC_COMMAND_TIMEOUT) { + if (now.tv_sec - begin.tv_sec >= EC_COMMAND_TIMEOUT) { msg_pdbg("%s: unable to reset ec\n", __func__); return -1; } @@ -440,9 +429,9 @@ ctx_data->ec_state = EC_STATE_IDLE; return 1; } - } - else if(chip->chip_id == ENE_KB94X && ctx_data->ec_state == EC_STATE_IDLE) + } else if (chip->chip_id == ENE_KB94X && ctx_data->ec_state == EC_STATE_IDLE) { ene_pause_timeout_check(ctx_data); + } ene_spi_start(chip); @@ -487,19 +476,16 @@ gettimeofday(&begin, NULL); /* EC restart */ - while (ene_read(chip, chip->ec_status_buf) != - chip->ec_is_running) { + while (ene_read(chip, chip->ec_status_buf) != chip->ec_is_running) { gettimeofday(&now, NULL); - if ((now.tv_sec - begin.tv_sec) >= - EC_RESTART_TIMEOUT) { + if (now.tv_sec - begin.tv_sec >= EC_RESTART_TIMEOUT) { msg_pdbg("%s: ec restart busy\n", __func__); rv = 1; goto exit; } } msg_pdbg("%s: send ec restart\n", __func__); - ec_command(chip, chip->ec_restart_cmd, - chip->ec_restart_data); + ec_command(chip, chip->ec_restart_cmd, chip->ec_restart_data); ctx_data->ec_state = EC_STATE_NORMAL; rv = 0; -- To view, visit https://review.coreboot.org/c/flashrom/+/52413 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iaa5277b12fca192c46c11f5e0f375dc43d06bf5c Gerrit-Change-Number: 52413 Gerrit-PatchSet: 2 Gerrit-Owner: 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: Victor Ding <victording(a)chromium.org> 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]: jlink_spi.c: Correct some log messages
by Nico Huber (Code Review) 18 Apr '21

18 Apr '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52412 ) Change subject: jlink_spi.c: Correct some log messages ...................................................................... jlink_spi.c: Correct some log messages Fix a few typos and a copy-paste error in log messages. Change-Id: Ic69503f60a59aa0f4b991eaa2a7be40a7d9c1301 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/52412 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Miklós Márton <martonmiklosqdev(a)gmail.com> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M jlink_spi.c 1 file changed, 4 insertions(+), 4 deletions(-) Approvals: build bot (Jenkins): Verified Miklós Márton: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved diff --git a/jlink_spi.c b/jlink_spi.c index efd4782..3d86764 100644 --- a/jlink_spi.c +++ b/jlink_spi.c @@ -134,7 +134,7 @@ ret = jaylink_jtag_io(jaylink_devh, buffer, buffer, buffer, length * 8, JAYLINK_JTAG_VERSION_2); if (ret != JAYLINK_OK) { - msg_perr("jaylink_jag_io() failed: %s.\n", jaylink_strerror(ret)); + msg_perr("jaylink_jtag_io() failed: %s.\n", jaylink_strerror(ret)); free(buffer); return SPI_PROGRAMMER_ERROR; } @@ -210,7 +210,7 @@ if (arg) { if (!strlen(arg)) { - msg_perr("Emptpy serial number specified.\n"); + msg_perr("Empty serial number specified.\n"); free(arg); return 1; } @@ -266,7 +266,7 @@ ret = jaylink_discovery_scan(jaylink_ctx, 0); if (ret != JAYLINK_OK) { - msg_perr("jaylink_discover_scan() failed: %s.\n", jaylink_strerror(ret)); + msg_perr("jaylink_discovery_scan() failed: %s.\n", jaylink_strerror(ret)); goto init_err; } @@ -361,7 +361,7 @@ ret = jaylink_get_extended_caps(jaylink_devh, caps); if (ret != JAYLINK_OK) { - msg_perr("jaylink_get_available_interfaces() failed: %s.\n", jaylink_strerror(ret)); + msg_perr("jaylink_get_extended_caps() failed: %s.\n", jaylink_strerror(ret)); goto init_err; } } -- To view, visit https://review.coreboot.org/c/flashrom/+/52412 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic69503f60a59aa0f4b991eaa2a7be40a7d9c1301 Gerrit-Change-Number: 52412 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(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-MessageType: merged

1 0

Change in flashrom[master]: ene_lpc.c: Moving register_shutdown at the end of initialisation
by Nico Huber (Code Review) 18 Apr '21

18 Apr '21

Attention is currently required from: Victor Ding, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52406 ) Change subject: ene_lpc.c: Moving register_shutdown at the end of initialisation ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/52406/comment/96aa8fbc_56b0cab9 PS1, Line 7: Moving Should be present tense "Move". Actually, "Move ... to" I guess. -- To view, visit https://review.coreboot.org/c/flashrom/+/52406 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If89a758c91c77486adbac2779449bcd71ab8fc78 Gerrit-Change-Number: 52406 Gerrit-PatchSet: 1 Gerrit-Owner: 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: Victor Ding <victording(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-Attention: Victor Ding <victording(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 18 Apr 2021 17:00:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer.h,chipset_enable.c: Make penable cb more descript
by Nico Huber (Code Review) 18 Apr '21

18 Apr '21

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52364 ) Change subject: programmer.h,chipset_enable.c: Make penable cb more descript ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/52364/comment/a49d605d_d403fe88 PS1, Line 7: Make penable cb more descript > Maybe it's just me, but `cb` resolves to `coreboot`. How about: […] It's not a callback anyway. It's a function pointer used for a dispatch mechanism. As `penable` binds the pointer to data one could also call it method (as in object-oriented programming). A callback is when a() is calling b() passing a1() and b is calling back into the context of a via a1. Sometimes the definition is a little stretched to include cases where the passed function is not directly related to a. But the common pattern is that the original caller decides, by specifying the callback, what will be called eventually. File programmer.h: https://review.coreboot.org/c/flashrom/+/52364/comment/2819bc18_475d66bc PS1, Line 228: enable_flash_xxx > I'd prefer `enable_flash` too. How about `run` or anything else that doesn't repeat `enable`. -- 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: 1 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-Comment-Date: Sun, 18 Apr 2021 16:47:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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]: linux_spi.c: Separate shutdown from failed init cleanup
by Nico Huber (Code Review) 18 Apr '21

18 Apr '21

Attention is currently required from: Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52284 ) Change subject: linux_spi.c: Separate shutdown from failed init cleanup ...................................................................... Patch Set 2: (2 comments) File linux_spi.c: https://review.coreboot.org/c/flashrom/+/52284/comment/95698951_d6d0f0a4 PS2, Line 237: if (fd != -1) { This is always true, AFAICS. https://review.coreboot.org/c/flashrom/+/52284/comment/d80da4db_b060ccd4 PS2, Line 241: ret AFAICS, this is always 1. -- To view, visit https://review.coreboot.org/c/flashrom/+/52284 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1c8da2878cd0e85a1e43ba9b4b8e6f3d9f38ae5c Gerrit-Change-Number: 52284 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 18 Apr 2021 16:07:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: linux_spi.c: Refactor singleton states into reentrant pattern
by Nico Huber (Code Review) 18 Apr '21

18 Apr '21

Attention is currently required from: Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52285 ) Change subject: linux_spi.c: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 3: (6 comments) File linux_spi.c: https://review.coreboot.org/c/flashrom/+/52285/comment/c7e44348_638a1c05 PS3, Line 111: const Why drop it? Ah, I've seen how it's used below now. Seems to counteract the idea of reentrant, though, doesn't it? I know you have plans to change the API, so it doesn't really matter now. https://review.coreboot.org/c/flashrom/+/52285/comment/626751e4_b00e0fa0 PS3, Line 71: (struct linux_spi_data *) Nit, generally no need to cast `void *` to anything. https://review.coreboot.org/c/flashrom/+/52285/comment/b86dc368_a298271f PS3, Line 72: if (spi_data->fd != -1) Seems impossible to fail? https://review.coreboot.org/c/flashrom/+/52285/comment/96a9c39b_2df9ba92 PS3, Line 177: -1 Where would this be consumed? https://review.coreboot.org/c/flashrom/+/52285/comment/b1ab9d52_e66e36f0 PS3, Line 242: ret = SPI_GENERIC_ERROR; FWIW, this and the related definitions are part of the SPI command API and not for initialization. https://review.coreboot.org/c/flashrom/+/52285/comment/575aacfc_035fc7a5 PS3, Line 260: fd = -1; It's a local variable now, making this dead code. -- To view, visit https://review.coreboot.org/c/flashrom/+/52285 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I93408c2ca846fca6a1c7eda7180862c51bd48078 Gerrit-Change-Number: 52285 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 18 Apr 2021 16:06:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: cli_classic: prevent corruption of flash when stdout/stderr is closed
by Jack Rosenthal (Code Review) 18 Apr '21

18 Apr '21

Jack Rosenthal has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52215 ) Change subject: cli_classic: prevent corruption of flash when stdout/stderr is closed ...................................................................... Patch Set 3: (1 comment) Patchset: PS2: > There is only a small group with submit rights. I also like to give […] thx -- To view, visit https://review.coreboot.org/c/flashrom/+/52215 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I751c9dd88ad1d30283b94bd2185b4f8f25569c8f Gerrit-Change-Number: 52215 Gerrit-PatchSet: 3 Gerrit-Owner: Jack Rosenthal <jrosenth(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-Comment-Date: Sun, 18 Apr 2021 15:52:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Jack Rosenthal <jrosenth(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call
by Nico Huber (Code Review) 18 Apr '21

18 Apr '21

Attention is currently required from: Simon Buhrow, Alan Green, Edward O'Callaghan, Anastasia Klimchuk, Samir Ibradžić. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/40477 ) Change subject: ft2232_spi.c: Pack WREN and op in one ftdi_write_data() call ...................................................................... Patch Set 25: (1 comment) File ft2232_spi.c: https://review.coreboot.org/c/flashrom/+/40477/comment/35b3d07b_ce357286 PS25, Line 356: msg_perr("get_buf failed: %i\n", ret); Sorry, forgot this one, we'd have to `break;` here too on error. Also the comment above seems stale. I guess it referred to the situation that CS# would still be asserted, which is not the case here. -- 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: 25 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: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Alan Green <avg(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Samir Ibradžić <sibradzic(a)gmail.com> Gerrit-Comment-Date: Sun, 18 Apr 2021 15:22:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2021