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

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 (#2). 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/2 -- 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: 2 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

Peter Marheine has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/52405 ) 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/1 diff --git a/realtek_mst_i2c_spi.c b/realtek_mst_i2c_spi.c index ae79fdd..1c35495 100644 --- a/realtek_mst_i2c_spi.c +++ b/realtek_mst_i2c_spi.c @@ -442,36 +442,51 @@ return ret; } -static int get_params(int *i2c_bus, int *reset, int *enter_isp) +static int get_params(int *i2c_bus, char **i2c_devpath, int *reset, int *enter_isp) { - char *bus_str = NULL, *reset_str = NULL, *isp_str = NULL; - int ret = SPI_GENERIC_ERROR; + char *bus_str = NULL, *bus_path = NULL, *reset_str = NULL, *isp_str = NULL; + int ret = 0; bus_str = extract_programmer_param("bus"); - if (bus_str) { - char *bus_suffix; - errno = 0; - int bus = (int)strtol(bus_str, &bus_suffix, 10); - if (errno != 0 || bus_str == bus_suffix) { - msg_perr("%s: Could not convert 'bus'.\n", __func__); - goto _get_params_failed; - } + bus_path = extract_programmer_param("devpath"); + if (bus_str == NULL && bus_path == NULL) { + msg_perr("%s: one of 'bus' or 'devpath' must be specified\n", __func__); + return SPI_GENERIC_ERROR; + } + if (bus_str != NULL && bus_path != NULL) { + msg_perr("%s: only one of 'bus' or 'devpath' may be specified\n", __func__); + free(bus_str); + free(bus_path); + return SPI_GENERIC_ERROR; + } + if (bus_str) { + // Up to three characters for '255', plus one byte to detect trailing garbage + // and a null terminator. Use a local buffer to avoid needing to remember to + // free bus_str later. + char bus_str_buf[5] = {0}; + strncpy(bus_str_buf, bus_str, sizeof(bus_str_buf) - 1); + free(bus_str); + + char *bus_suffix; + int bus = (int)strtol(bus_str_buf, &bus_suffix, 10); + if (bus_str_buf == bus_suffix) { + msg_perr("%s: Could not convert 'bus'.\n", __func__); + return SPI_GENERIC_ERROR; + } if (bus < 0 || bus > 255) { msg_perr("%s: Value for 'bus' is out of range(0-255).\n", __func__); - goto _get_params_failed; + return SPI_GENERIC_ERROR; } if (strlen(bus_suffix) > 0) { msg_perr("%s: Garbage following 'bus' value.\n", __func__); - goto _get_params_failed; + return SPI_GENERIC_ERROR; } - msg_pinfo("Using i2c bus %i.\n", bus); *i2c_bus = bus; - ret = 0; } else { - msg_perr("%s: Bus number not specified.\n", __func__); + *i2c_devpath = bus_path; } reset_str = extract_programmer_param("reset-mcu"); @@ -502,32 +517,40 @@ *enter_isp = 1; /* Default behaviour is enter ISP on setup. */ free(isp_str); -_get_params_failed: - if (bus_str) - free(bus_str); - + // If failing, free a devpath that was being returned + if (ret && *i2c_devpath != NULL) + free(*i2c_devpath); return ret; } int realtek_mst_i2c_spi_init(void) { int ret = 0; - int i2c_bus = 0, reset = 0, enter_isp = 0; + int i2c_bus = -1, reset = 0, enter_isp = 0; + char *i2c_devpath = NULL; - if (get_params(&i2c_bus, &reset, &enter_isp)) + if (get_params(&i2c_bus, &i2c_devpath, &reset, &enter_isp)) return SPI_GENERIC_ERROR; - int fd = i2c_open(i2c_bus, REGISTER_ADDRESS, 0); + int fd; + // bus= and devpath= should be mutually exclusive, but defensively handle both here + if (i2c_bus != -1) { + fd = i2c_open(i2c_bus, REGISTER_ADDRESS, 0); + } + if (i2c_devpath != NULL) { + fd = i2c_open_path(i2c_devpath, REGISTER_ADDRESS, 0); + free(i2c_devpath); + } if (fd < 0) return fd; if (enter_isp) { - ret |= realtek_mst_i2c_spi_enter_isp_mode(fd); + ret = realtek_mst_i2c_spi_enter_isp_mode(fd); if (ret) return ret; } - ret |= realtek_mst_i2c_spi_toggle_gpio_88_strap(fd, true); + ret = realtek_mst_i2c_spi_toggle_gpio_88_strap(fd, true); if (ret) { msg_perr("Unable to toggle gpio 88 strap to True.\n"); return ret; @@ -541,7 +564,7 @@ data->fd = fd; data->reset = reset; - ret |= register_shutdown(realtek_mst_i2c_spi_shutdown, data); + ret = register_shutdown(realtek_mst_i2c_spi_shutdown, data); spi_master_i2c_realtek_mst.data = data; ret |= register_spi_master(&spi_master_i2c_realtek_mst); -- 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: 1 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: newchange

1 0

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

16 Apr '21

Attention is currently required from: Nico Huber, Daniel Campello, Angel Pons. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52362 ) Change subject: cli_classic: Make -r/-w/-v argument optional ...................................................................... Patch Set 5: (1 comment) File cli_classic.c: https://review.coreboot.org/c/flashrom/+/52362/comment/239783e0_e44497bf PS1, Line 46: > > I'm not too familiar with what the design considerations are here, so take this with a grain of sa […] Ack -- 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: 5 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: Daniel Campello <campello(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 16 Apr 2021 03:01:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Daniel Campello <campello(a)chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

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

16 Apr '21

Attention is currently required from: Nico Huber, Angel Pons. Thomas Walker has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52310 ) Change subject: flashchips: Add Spansion/Cypress S25FL256L ...................................................................... Patch Set 4: (2 comments) Patchset: PS4: Fixed my lack of tabs! File flashchips.c: https://review.coreboot.org/c/flashrom/+/52310/comment/cea352fc_2c8235c1 PS3, Line 16380: > nit: tabs here too (see other chips) Done -- 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 16 Apr 2021 01:15:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

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

16 Apr '21

Attention is currently required from: Nico Huber, Thomas Walker. Hello build bot (Jenkins), Nico Huber, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/52310 to look at the new patch set (#4). Change subject: flashchips: Add Spansion/Cypress S25FL256L ...................................................................... flashchips: Add Spansion/Cypress S25FL256L Conducted random write test using Adafruit FT232H programmer. Confirmed read/write/erase/probe and VERIFIED on completion. Signed-off-by: Thomas Walker <thh.walker(a)gmail.com> Change-Id: I4e8cba554d0590c94dac92aa91e9ab400efca281 --- M chipdrivers.h M flashchips.c M flashchips.h M spi.h M spi25.c 5 files changed, 60 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/10/52310/4 -- 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-MessageType: newpatchset

1 0

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

15 Apr '21

Attention is currently required from: Edward O'Callaghan, Angel Pons. Anastasia Klimchuk 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/d1e999c8_6efa090f PS2, Line 465: return init_ret; > Drop `init_ret`? […] oh yes, it's not needed anymore! -- 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 15 Apr 2021 23:45:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

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

15 Apr '21

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Hello build bot (Jenkins), Miklós Márton, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/52308 to look at the new patch set (#4). 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> --- M jlink_spi.c 1 file changed, 26 insertions(+), 18 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/52308/4 -- 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-MessageType: newpatchset

1 0

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

15 Apr '21

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Hello build bot (Jenkins), Miklós Márton, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/52308 to look at the new patch set (#3). 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> --- M jlink_spi.c 1 file changed, 29 insertions(+), 19 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/52308/3 -- 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: 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: 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-MessageType: newpatchset

1 0

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

15 Apr '21

Attention is currently required from: Sam McNally, Nico Huber, Nicola Corna, Paul Menzel, Stefan Reinauer, David Hendricks, Daniel Campello, Arthur Heymans, Carl-Daniel Hailfinger. Daniel Campello has uploaded a new patch set (#29) to the change originally created by David Hendricks. ( https://review.coreboot.org/c/flashrom/+/23021 ) Change subject: layout: Add -i <region>[:<file>] support ...................................................................... layout: Add -i <region>[:<file>] support Add an optional sub-parameter to the -i parameter to allow building the image to be written from multiple files. This will also allow regions to be read from flash and written to separate image files. This is a rebase of a patch that was ported from chromiumos. A lot of things have changed, but the idea is the same. Original patch by Louis Yung-Chieh Lo <yjlou(a)chromium.org>: Summary: Support -i partition:file feature for both read and write. Commit: 9c7525f Review URL: http://codereview.chromium.org/6611015 Ported version by Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> and Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net>: Summary: [PATCH 2/6] layout: Add -i <region>[:<file>] support. Review URL: https://mail.coreboot.org/pipermail/flashrom/2013-October/011729.html Change-Id: Ic5465659605d8431d931053967b40290195cfd99 Signed-off-by: David Hendricks <dhendrix(a)chromium.org> Signed-off-by: Stefan Tauner <stefan.tauner(a)student.tuwien.ac.at> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Signed-off-by: Nico Huber <nico.huber(a)secunet.com> Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Signed-off-by: Daniel Campello <campello(a)chromium.org> Co-Authored-by: Edward O'Callaghan <quasisec(a)google.com> Co-Authored-by: Daniel Campello <campello(a)chromium.org> --- M cli_classic.c M flashrom.8.tmpl M flashrom.c M layout.c M layout.h 5 files changed, 234 insertions(+), 27 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/23021/29 -- 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: 29 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: 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nicola Corna <nicola(a)corna.info> 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-MessageType: newpatchset

1 0

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

15 Apr '21

Attention is currently required from: Sam McNally, Nico Huber, Nicola Corna, Paul Menzel, Stefan Reinauer, David Hendricks, Arthur Heymans, Carl-Daniel Hailfinger. Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/23021 ) Change subject: layout: Add -i <region>[:<file>] support ...................................................................... Patch Set 27: (7 comments) Patchset: PS27: > Thanks for taking this over. I have to say, however, that it was […] In the follow up CB:52362 I implement the chromiumos syntax in https://flashrom.org/Per_region_file_arguments I have been working on converge chromium and upstream flashrom and this change plus the two followups would achieve that in regards of syntax. I realize now that CB:23022 was attempting the same 😊 Convergence has advanced quite a bit (crrev.com/c/2823343) that I think striving for the same CLI syntax is the right approach. Also, all of these changes are backward compatible with existent interface. File cli_classic.c: https://review.coreboot.org/c/flashrom/+/23021/comment/be786e23_6f12a404 PS27, Line 319: tempstr = strdup(optarg); > If register_include_arg() is not going to own it, we should pass it `const` […] strdup() here since strtok() modifies the string: layout.c: In function 'register_include_arg': layout.c:123:16: error: passing argument 1 of 'strtok' discards 'const' qualifier from pointer target type [-Werror=discarded-qualifiers] 123 | name = strtok(include, ":"); File flashrom.c: https://review.coreboot.org/c/flashrom/+/23021/comment/9e166f18_9b5fcc93 PS16, Line 1367: when writing > Any other case when this is used? or, why remove this? I have been working with the intention of reduce as much as possible the differences between upstream and chromiumos flashrom. This is one case where the string was like that, but I'll add it back and send a patch in chromium. File layout.c: https://review.coreboot.org/c/flashrom/+/23021/comment/a0165551_beef69c6 PS21, Line 253: goto out; > The intention obviously was to report all overlapping regions. […] I overlooked this. Will revert to have the reporting as I don't expect the loop over all regions to be expensive... File layout.c: https://review.coreboot.org/c/flashrom/+/23021/comment/f578513c_8177e092 PS27, Line 147: tmp->file = strdup(file); > else? Done https://review.coreboot.org/c/flashrom/+/23021/comment/ec603e74_bd313e10 PS27, Line 155: include_region > Could be exported as part of the libflashrom API, e.g. […] Agree! This could come in a follow up CL! https://review.coreboot.org/c/flashrom/+/23021/comment/906e2106_3a649916 PS27, Line 221: } > Looks like somewhere in the rebasing reporting of the file names got lost. Done -- 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: 27 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: 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nicola Corna <nicola(a)corna.info> 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: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Gerrit-Comment-Date: Thu, 15 Apr 2021 22:38:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2021