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

Change in flashrom[master]: flashchips: Add Spansion/Cypress S25FL256L
by Angel Pons (Code Review) 16 Apr '21

16 Apr '21

Attention is currently required from: Nico Huber, Thomas Walker. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52310 ) Change subject: flashchips: Add Spansion/Cypress S25FL256L ...................................................................... Patch Set 4: Code-Review+1 (3 comments) File spi.h: https://review.coreboot.org/c/flashrom/+/52310/comment/9d1acb92_35211bdf PS4, Line 93: Block Erase 0x53 is only supported by Spansion/Cypress S25FL-L chips. I don't think it's a JEDEC standard, so I wouldn't add these macros. File spi25.c: https://review.coreboot.org/c/flashrom/+/52310/comment/569069f9_c3374b21 PS4, Line 491: /* This usually takes 100-4000ms, so wait in 100ms steps. */ Datasheet for S25FL064L says typical is 300 ms, and maximum is 600 ms. Datasheet for S25FL256L says typical is 190 ms, and maximum is 363 ms. Without checking any other datasheets, I'd say the 4000 ms in the comment could be lowered to 1000 ms. https://review.coreboot.org/c/flashrom/+/52310/comment/0aa668bc_2ec59718 PS4, Line 621: return &spi_block_erase_52; Add an entry here too? -- To view, visit https://review.coreboot.org/c/flashrom/+/52310 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4e8cba554d0590c94dac92aa91e9ab400efca281 Gerrit-Change-Number: 52310 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Walker <thh.walker(a)gmail.com> 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Walker <thh.walker(a)gmail.com> Gerrit-Comment-Date: Fri, 16 Apr 2021 09:44:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ene_lpc.c: Clean up cosmetics
by Angel Pons (Code Review) 16 Apr '21

16 Apr '21

Angel Pons has uploaded this change for review. ( 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> --- M ene_lpc.c 1 file changed, 18 insertions(+), 32 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/13/52413/1 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: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

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

16 Apr '21

Attention is currently required from: Victor Ding, Anastasia Klimchuk. Angel Pons 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 -- 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: 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: Nico Huber <nico.h(a)gmx.de> 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: Fri, 16 Apr 2021 09:20:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: jlink_spi.c: Correct some log messages
by Angel Pons (Code Review) 16 Apr '21

16 Apr '21

Angel Pons has uploaded this change for review. ( 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> --- M jlink_spi.c 1 file changed, 4 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/12/52412/1 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: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: jlink_spi.c: Separate shutdown from failed init cleanup
by Angel Pons (Code Review) 16 Apr '21

16 Apr '21

Angel Pons has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52308 ) Change subject: jlink_spi.c: Separate shutdown from failed init cleanup ...................................................................... jlink_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 this https://review.coreboot.org/c/flashrom/+/51761 TEST=builds BUG=b:185191942 Change-Id: I71f64ed38154af670d4d28b8c7914d87fbc75679 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/52308 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M jlink_spi.c 1 file changed, 26 insertions(+), 18 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved diff --git a/jlink_spi.c b/jlink_spi.c index 3a73c01..efd4782 100644 --- a/jlink_spi.c +++ b/jlink_spi.c @@ -179,8 +179,6 @@ char *arg; unsigned long speed = 0; - register_shutdown(jlink_spi_shutdown, NULL); - arg = extract_programmer_param("spispeed"); if (arg) { @@ -269,7 +267,7 @@ if (ret != JAYLINK_OK) { msg_perr("jaylink_discover_scan() failed: %s.\n", jaylink_strerror(ret)); - return 1; + goto init_err; } struct jaylink_device **devs; @@ -278,7 +276,7 @@ if (ret != JAYLINK_OK) { msg_perr("jaylink_get_devices() failed: %s.\n", jaylink_strerror(ret)); - return 1; + goto init_err; } if (!use_serial_number) @@ -321,7 +319,7 @@ if (!device_found) { msg_perr("No J-Link device found.\n"); - return 1; + goto init_err; } size_t length; @@ -332,7 +330,7 @@ if (ret != JAYLINK_OK) { msg_perr("jaylink_get_firmware_version() failed: %s.\n", jaylink_strerror(ret)); - return 1; + goto init_err; } else if (length > 0) { msg_pdbg("Firmware: %s\n", firmware_version); free(firmware_version); @@ -346,7 +344,7 @@ msg_pdbg("S/N: N/A\n"); } else { msg_perr("jaylink_device_get_serial_number() failed: %s.\n", jaylink_strerror(ret)); - return 1; + goto init_err; } uint8_t caps[JAYLINK_DEV_EXT_CAPS_SIZE]; @@ -356,7 +354,7 @@ if (ret != JAYLINK_OK) { msg_perr("jaylink_get_caps() failed: %s.\n", jaylink_strerror(ret)); - return 1; + goto init_err; } if (jaylink_has_cap(caps, JAYLINK_DEV_CAP_GET_EXT_CAPS)) { @@ -364,7 +362,7 @@ if (ret != JAYLINK_OK) { msg_perr("jaylink_get_available_interfaces() failed: %s.\n", jaylink_strerror(ret)); - return 1; + goto init_err; } } @@ -374,19 +372,19 @@ if (ret != JAYLINK_OK) { msg_perr("jaylink_get_available_interfaces() failed: %s.\n", jaylink_strerror(ret)); - return 1; + goto init_err; } if (!(ifaces & (1 << JAYLINK_TIF_JTAG))) { msg_perr("Device does not support JTAG interface.\n"); - return 1; + goto init_err; } ret = jaylink_select_interface(jaylink_devh, JAYLINK_TIF_JTAG, NULL); if (ret != JAYLINK_OK) { msg_perr("jaylink_select_interface() failed: %s.\n", jaylink_strerror(ret)); - return 1; + goto init_err; } struct jaylink_hardware_status hwstat; @@ -395,7 +393,7 @@ if (ret != JAYLINK_OK) { msg_perr("jaylink_get_hardware_status() failed: %s.\n", jaylink_strerror(ret)); - return 1; + goto init_err; } msg_pdbg("VTarget: %u.%03u V\n", hwstat.target_voltage / 1000, @@ -404,7 +402,7 @@ if (hwstat.target_voltage < MIN_TARGET_VOLTAGE) { msg_perr("Target voltage is below %u.%03u V. You need to attach VTref to the I/O voltage of " "the chip.\n", MIN_TARGET_VOLTAGE / 1000, MIN_TARGET_VOLTAGE % 1000); - return 1; + goto init_err; } struct jaylink_speed device_speeds; @@ -417,7 +415,7 @@ if (ret != JAYLINK_OK) { msg_perr("jaylink_get_speeds() failed: %s.\n", jaylink_strerror(ret)); - return 1; + goto init_err; } } @@ -433,23 +431,33 @@ if (speed > (device_speeds.freq / device_speeds.div)) { msg_perr("Specified SPI speed of %lu kHz is too high. Maximum is %" PRIu32 " kHz.\n", speed, device_speeds.freq / device_speeds.div); - return 1; + goto init_err; } ret = jaylink_set_speed(jaylink_devh, speed); if (ret != JAYLINK_OK) { msg_perr("jaylink_set_speed() failed: %s.\n", jaylink_strerror(ret)); - return 1; + goto init_err; } msg_pdbg("SPI speed: %lu kHz\n", speed); /* Ensure that the CS signal is not active initially. */ if (!deassert_cs()) - return 1; + goto init_err; + if (register_shutdown(jlink_spi_shutdown, NULL)) + goto init_err; register_spi_master(&spi_master_jlink_spi); return 0; + +init_err: + if (jaylink_devh) + jaylink_close(jaylink_devh); + + jaylink_exit(jaylink_ctx); + + return 1; } -- To view, visit https://review.coreboot.org/c/flashrom/+/52308 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I71f64ed38154af670d4d28b8c7914d87fbc75679 Gerrit-Change-Number: 52308 Gerrit-PatchSet: 5 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: Miklós Márton <martonmiklosqdev(a)gmail.com> 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-MessageType: merged

1 0

Change in flashrom[master]: jlink_spi.c: Separate shutdown from failed init cleanup
by Angel Pons (Code Review) 16 Apr '21

16 Apr '21

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52308 ) Change subject: jlink_spi.c: Separate shutdown from failed init cleanup ...................................................................... Patch Set 4: (1 comment) File jlink_spi.c: https://review.coreboot.org/c/flashrom/+/52308/comment/f33f75ca_c3af7d6f PS4, Line 263: return 1; Note: We do not change this return on purpose. If `jaylink_init` failed, `jaylink_ctx` may not be initialised properly, so we skip calling `jaylink_exit`. (No action required here) -- To view, visit https://review.coreboot.org/c/flashrom/+/52308 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I71f64ed38154af670d4d28b8c7914d87fbc75679 Gerrit-Change-Number: 52308 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: Miklós Márton <martonmiklosqdev(a)gmail.com> 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 16 Apr 2021 08:51:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: jlink_spi.c: Separate shutdown from failed init cleanup
by Angel Pons (Code Review) 16 Apr '21

16 Apr '21

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52308 ) Change subject: jlink_spi.c: Separate shutdown from failed init cleanup ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52308 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I71f64ed38154af670d4d28b8c7914d87fbc75679 Gerrit-Change-Number: 52308 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: Miklós Márton <martonmiklosqdev(a)gmail.com> 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 16 Apr 2021 08:25:11 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

16 Apr '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/52406 ) Change subject: ene_lpc.c: Moving register_shutdown at the end of initialisation ...................................................................... ene_lpc.c: Moving register_shutdown at the end of initialisation A bit more details: since register_shutdown has moved a few lines below, now ene_enter_flash_mode is called before the shutdown function is registered. ene_enter_flash_mode can fail (at least in theory, it has some return 1s), but its return value is not analyzed, so even if it returns non-0, execution goes further and register_shutdown will happen anyway. It is a separate question whether the return value of ene_enter_flash_mode needs to be analyzed or is it ok to ignore it, but in this patch I plan to keep the same behaviour, and probably I will get back to error handling later. This unlocks API change which plans to move register_shutdown inside register master API, see https://review.coreboot.org/c/flashrom/+/51761 TEST=builds BUG=b:185191942 Change-Id: If89a758c91c77486adbac2779449bcd71ab8fc78 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M ene_lpc.c 1 file changed, 4 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/52406/1 diff --git a/ene_lpc.c b/ene_lpc.c index 56d6580..61f27b4 100644 --- a/ene_lpc.c +++ b/ene_lpc.c @@ -573,16 +573,15 @@ * Compal - ec_command(0x41, 0xa1) returns 43 4f 4d 50 41 4c 9c */ + ene_enter_flash_mode(ctx_data); + + 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; } - - ene_enter_flash_mode(ctx_data); - - internal_buses_supported |= BUS_LPC; - spi_master_ene.data = ctx_data; register_spi_master(&spi_master_ene); msg_pdbg("%s: successfully initialized ene\n", __func__); -- 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-MessageType: newchange

1 0

Change in flashrom[master]: realtek_mst: add support for a devpath option
by Peter Marheine (Code Review) 16 Apr '21

16 Apr '21

Attention is currently required from: Peter Marheine. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/52405 to look at the new patch set (#4). Change subject: realtek_mst: add support for a devpath option ...................................................................... realtek_mst: add support for a devpath option As with the lspcon_i2c_spi programmer, it may be more convenient for callers to specify the I2C bus by device path rather than number. Add support for a 'devpath' option that is mutually exclusive with the 'bus' option and specifies how to communicate with the target. TEST=-p realtek_mst_i2c_spi:devpath=/dev/i2c-8 --flash-size works, bus=8 continues to work. If both or neither are given, fails gracefully. Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> Change-Id: Ieea57b2445f3aca16c87d8f839dff131009531a0 --- M realtek_mst_i2c_spi.c 1 file changed, 49 insertions(+), 26 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/05/52405/4 -- 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: 4 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: realtek_mst: add support for a devpath option
by Peter Marheine (Code Review) 16 Apr '21

16 Apr '21

Attention is currently required from: Peter Marheine. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/52405 to look at the new patch set (#3). Change subject: realtek_mst: add support for a devpath option ...................................................................... realtek_mst: add support for a devpath option As with the lspcon_i2c_spi programmer, it may be more convenient for callers to specify the I2C bus by device path rather than number. Add support for a 'devpath' option that is mutually exclusive with the 'bus' option and specifies how to communicate with the target. TEST=-p realtek_mst_i2c_spi:devpath=/dev/i2c-8 --flash-size works, bus=8 continues to work. If both or neither are given, fails gracefully. Change-Id: Ieea57b2445f3aca16c87d8f839dff131009531a0 --- M realtek_mst_i2c_spi.c 1 file changed, 49 insertions(+), 26 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/05/52405/3 -- 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: 3 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2021