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

Change in flashrom[master]: mec1308.c: Extract params check into a function
by Nico Huber (Code Review) 27 Apr '21

27 Apr '21

Attention is currently required from: Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52595 ) Change subject: mec1308.c: Extract params check into a function ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/52595/comment/03744c05_fc2789d5 PS1, Line 9: This allows char *p to become a local variable in > I can explain why I thought it's a good idea to make it wider: so that commit message spans less lin […] Hmm, probably a good point. Personally, I don't notice such things much. Maybe because I use rather large screens, usually I feel limited by the amount of windows I can have side-by-side. Also, I rarely use laptops with degraded 16:9 screens. Anyway, if you wanted to change it, I fully agree. Just wanted to make sure nobody feels forced to make their lines longer :) -- To view, visit https://review.coreboot.org/c/flashrom/+/52595 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If5be7709e93233a2e7ea9133de50382d2524a55f Gerrit-Change-Number: 52595 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-CC: Victor Ding <victording(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 27 Apr 2021 10:27:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> 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]: jlink_spi.c: Drop jaylink_ prefix for spi data struct members
by Nico Huber (Code Review) 27 Apr '21

27 Apr '21

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52661 ) Change subject: jlink_spi.c: Drop jaylink_ prefix for spi data struct members ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Patchset: PS2: uh, wrong button (didn't verify anything) -- To view, visit https://review.coreboot.org/c/flashrom/+/52661 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If6e68e0dabb6bfad1088ff975445961294bbc03d Gerrit-Change-Number: 52661 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: 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-Comment-Date: Tue, 27 Apr 2021 10:09:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: jlink_spi.c: Drop jaylink_ prefix for spi data struct members
by Nico Huber (Code Review) 27 Apr '21

27 Apr '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52661 ) Change subject: jlink_spi.c: Drop jaylink_ prefix for spi data struct members ...................................................................... jlink_spi.c: Drop jaylink_ prefix for spi data struct members Since jaylink context and jaylink device handle are not global variables anymore, but members of the struct jlink_spi_data, jaylink_ prefix can be dropped from members names and moved to struct variable name. Follow up on 52560. BUG=b:185191942 TEST=builds Change-Id: If6e68e0dabb6bfad1088ff975445961294bbc03d Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/52661 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Tested-by: Nico Huber <nico.h(a)gmx.de> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Miklós Márton <martonmiklosqdev(a)gmail.com> --- M jlink_spi.c 1 file changed, 31 insertions(+), 31 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Verified Miklós Márton: Looks good to me, but someone else must approve Edward O'Callaghan: Looks good to me, approved diff --git a/jlink_spi.c b/jlink_spi.c index 097b0fb..8b5cd2c 100644 --- a/jlink_spi.c +++ b/jlink_spi.c @@ -52,24 +52,24 @@ #define MIN_TARGET_VOLTAGE 1200 struct jlink_spi_data { - struct jaylink_context *jaylink_ctx; - struct jaylink_device_handle *jaylink_devh; + struct jaylink_context *ctx; + struct jaylink_device_handle *devh; bool reset_cs; }; -static bool assert_cs(struct jlink_spi_data *spi_data) +static bool assert_cs(struct jlink_spi_data *jlink_data) { int ret; - if (spi_data->reset_cs) { - ret = jaylink_clear_reset(spi_data->jaylink_devh); + if (jlink_data->reset_cs) { + ret = jaylink_clear_reset(jlink_data->devh); if (ret != JAYLINK_OK) { msg_perr("jaylink_clear_reset() failed: %s.\n", jaylink_strerror(ret)); return false; } } else { - ret = jaylink_jtag_clear_trst(spi_data->jaylink_devh); + ret = jaylink_jtag_clear_trst(jlink_data->devh); if (ret != JAYLINK_OK) { msg_perr("jaylink_jtag_clear_trst() failed: %s.\n", jaylink_strerror(ret)); @@ -80,19 +80,19 @@ return true; } -static bool deassert_cs(struct jlink_spi_data *spi_data) +static bool deassert_cs(struct jlink_spi_data *jlink_data) { int ret; - if (spi_data->reset_cs) { - ret = jaylink_set_reset(spi_data->jaylink_devh); + if (jlink_data->reset_cs) { + ret = jaylink_set_reset(jlink_data->devh); if (ret != JAYLINK_OK) { msg_perr("jaylink_set_reset() failed: %s.\n", jaylink_strerror(ret)); return false; } } else { - ret = jaylink_jtag_set_trst(spi_data->jaylink_devh); + ret = jaylink_jtag_set_trst(jlink_data->devh); if (ret != JAYLINK_OK) { msg_perr("jaylink_jtag_set_trst() failed: %s.\n", jaylink_strerror(ret)); @@ -108,7 +108,7 @@ { uint32_t length; uint8_t *buffer; - struct jlink_spi_data *spi_data = flash->mst->spi.data; + struct jlink_spi_data *jlink_data = flash->mst->spi.data; length = writecnt + readcnt; @@ -127,14 +127,14 @@ memset(buffer + writecnt, 0x00, readcnt); - if (!assert_cs(spi_data)) { + if (!assert_cs(jlink_data)) { free(buffer); return SPI_PROGRAMMER_ERROR; } int ret; - ret = jaylink_jtag_io(spi_data->jaylink_devh, + ret = jaylink_jtag_io(jlink_data->devh, buffer, buffer, buffer, length * 8, JAYLINK_JTAG_VERSION_2); if (ret != JAYLINK_OK) { @@ -143,7 +143,7 @@ return SPI_PROGRAMMER_ERROR; } - if (!deassert_cs(spi_data)) { + if (!deassert_cs(jlink_data)) { free(buffer); return SPI_PROGRAMMER_ERROR; } @@ -170,14 +170,14 @@ static int jlink_spi_shutdown(void *data) { - struct jlink_spi_data *spi_data = data; - if (spi_data->jaylink_devh) - jaylink_close(spi_data->jaylink_devh); + struct jlink_spi_data *jlink_data = data; + if (jlink_data->devh) + jaylink_close(jlink_data->devh); - jaylink_exit(spi_data->jaylink_ctx); + jaylink_exit(jlink_data->ctx); - /* jaylink_ctx, jaylink_devh are freed by jaylink_close and jaylink_exit */ - free(spi_data); + /* jlink_data->ctx, jlink_data->devh are freed by jaylink_close and jaylink_exit */ + free(jlink_data); return 0; } @@ -188,7 +188,7 @@ struct jaylink_context *jaylink_ctx = NULL; struct jaylink_device_handle *jaylink_devh = NULL; bool reset_cs; - struct jlink_spi_data *spi_data = NULL; + struct jlink_spi_data *jlink_data = NULL; arg = extract_programmer_param("spispeed"); @@ -454,23 +454,23 @@ msg_pdbg("SPI speed: %lu kHz\n", speed); - spi_data = calloc(1, sizeof(*spi_data)); - if (!spi_data) { + jlink_data = calloc(1, sizeof(*jlink_data)); + if (!jlink_data) { msg_perr("Unable to allocate space for SPI master data\n"); goto init_err; } /* jaylink_ctx, jaylink_devh are allocated by jaylink_init and jaylink_open */ - spi_data->jaylink_ctx = jaylink_ctx; - spi_data->jaylink_devh = jaylink_devh; - spi_data->reset_cs = reset_cs; - spi_master_jlink_spi.data = spi_data; + jlink_data->ctx = jaylink_ctx; + jlink_data->devh = jaylink_devh; + jlink_data->reset_cs = reset_cs; + spi_master_jlink_spi.data = jlink_data; /* Ensure that the CS signal is not active initially. */ - if (!deassert_cs(spi_data)) + if (!deassert_cs(jlink_data)) goto init_err; - if (register_shutdown(jlink_spi_shutdown, spi_data)) + if (register_shutdown(jlink_spi_shutdown, jlink_data)) goto init_err; register_spi_master(&spi_master_jlink_spi); @@ -483,8 +483,8 @@ jaylink_exit(jaylink_ctx); /* jaylink_ctx, jaylink_devh are freed by jaylink_close and jaylink_exit */ - if (spi_data) - free(spi_data); + if (jlink_data) + free(jlink_data); return 1; } -- To view, visit https://review.coreboot.org/c/flashrom/+/52661 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If6e68e0dabb6bfad1088ff975445961294bbc03d Gerrit-Change-Number: 52661 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: 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-MessageType: merged

1 0

Change in flashrom[master]: jlink_spi.c: Drop jaylink_ prefix for spi data struct members
by Nico Huber (Code Review) 27 Apr '21

27 Apr '21

Attention is currently required from: Angel Pons, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52661 ) Change subject: jlink_spi.c: Drop jaylink_ prefix for spi data struct members ...................................................................... Patch Set 1: Verified+1 (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/52661/comment/4d129554_cbe953e3 PS1, Line 15: TEST=builds Should result in the same binaries. File jlink_spi.c: https://review.coreboot.org/c/flashrom/+/52661/comment/0ca48cf3_2551c3b4 PS1, Line 486: if (jlink_data) Unrelated: We have decided to trust `free(NULL)` to do the right (no)thing. -- To view, visit https://review.coreboot.org/c/flashrom/+/52661 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If6e68e0dabb6bfad1088ff975445961294bbc03d Gerrit-Change-Number: 52661 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: 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-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 27 Apr 2021 09:49:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: chipset_enable.c: Add IDs for H310C and B365 PCHs
by Nico Huber (Code Review) 27 Apr '21

27 Apr '21

Nico Huber has submitted this change. ( https://review.coreboot.org/c/flashrom/+/52605 ) Change subject: chipset_enable.c: Add IDs for H310C and B365 PCHs ...................................................................... chipset_enable.c: Add IDs for H310C and B365 PCHs The device ID for H310C can be found in Intel document 335192-004, but the device ID for B365 is not there. Other sites list these IDs: https://linux-hardware.org/index.php?id=pci:8086-a2ca-1462-7c09 (H310C) https://linux-hardware.org/index.php?id=pci:8086-a2cc-1849-a2cc (B365) Both of these PCHs have been tested as well. Change-Id: If9f0a49a0f1821e5592213e07962ee48654cdc07 Tested-by: Timofey Komarov <happycorsair(a)yandex.ru> Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/52605 Reviewed-by: Nico Huber <nico.h(a)gmx.de> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M chipset_enable.c 1 file changed, 2 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved diff --git a/chipset_enable.c b/chipset_enable.c index 5a1129d..259902e 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -2071,6 +2071,8 @@ {0x8086, 0xa2c7, B_S, NT, "Intel", "Q250", enable_flash_pch100}, {0x8086, 0xa2c8, B_S, NT, "Intel", "B250", enable_flash_pch100}, {0x8086, 0xa2c9, B_S, NT, "Intel", "Z370", enable_flash_pch100}, + {0x8086, 0xa2ca, B_S, DEP, "Intel", "H310C", enable_flash_pch100}, + {0x8086, 0xa2cc, B_S, DEP, "Intel", "B365", enable_flash_pch100}, {0x8086, 0xa2d2, B_S, NT, "Intel", "X299", enable_flash_pch100}, {0x8086, 0x5ae8, B_S, DEP, "Intel", "Apollo Lake", enable_flash_apl}, {0x8086, 0x5af0, B_S, DEP, "Intel", "Apollo Lake", enable_flash_apl}, -- To view, visit https://review.coreboot.org/c/flashrom/+/52605 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If9f0a49a0f1821e5592213e07962ee48654cdc07 Gerrit-Change-Number: 52605 Gerrit-PatchSet: 6 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Timofey Komarov <happycorsair(a)yandex.ru> 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]: chipset_enable.c: Add IDs for H310C and B365 PCHs
by Nico Huber (Code Review) 27 Apr '21

27 Apr '21

Attention is currently required from: Angel Pons, Arthur Heymans. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52605 ) Change subject: chipset_enable.c: Add IDs for H310C and B365 PCHs ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/52605 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If9f0a49a0f1821e5592213e07962ee48654cdc07 Gerrit-Change-Number: 52605 Gerrit-PatchSet: 5 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Timofey Komarov <happycorsair(a)yandex.ru> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Tue, 27 Apr 2021 09:26:51 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: chipset_enable.c: Add IDs for H310C and B365 PCHs
by Angel Pons (Code Review) 27 Apr '21

27 Apr '21

Attention is currently required from: Nico Huber, Angel Pons, Arthur Heymans. Hello Timofey Komarov, build bot (Jenkins), Nico Huber, Arthur Heymans, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/52605 to look at the new patch set (#4). Change subject: chipset_enable.c: Add IDs for H310C and B365 PCHs ...................................................................... chipset_enable.c: Add IDs for H310C and B365 PCHs The device ID for H310C can be found in Intel document 335192-004, but the device ID for B365 is not there. Other sites list these IDs: https://linux-hardware.org/index.php?id=pci:8086-a2ca-1462-7c09 (H310C) https://linux-hardware.org/index.php?id=pci:8086-a2cc-1849-a2cc (B365) Both of these PCHs have been tested as well. Change-Id: If9f0a49a0f1821e5592213e07962ee48654cdc07 Tested-by: Timofey Komarov <happycorsair(a)yandex.ru> Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M chipset_enable.c 1 file changed, 2 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/05/52605/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/52605 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If9f0a49a0f1821e5592213e07962ee48654cdc07 Gerrit-Change-Number: 52605 Gerrit-PatchSet: 4 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Timofey Komarov <happycorsair(a)yandex.ru> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: s25f.c: Fix mismatched function definitions
by Angel Pons (Code Review) 27 Apr '21

27 Apr '21

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52473 ) Change subject: s25f.c: Fix mismatched function definitions ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/52473/comment/4f27b447_fbd10277 PS1, Line 8: : This was missed because `uint32_t` is `unsigned int` in most cases. : However, it is not the case for DJGPP 6.1.0 for some reason. > Last couple of weeks have been pretty painful :| I've just been on and off review unofficially to help Anastasia which probably made it even more unclear. Since I rarely -2 I erroneously assumed you figured something might be strange. Any ways, cheers Angel and sorry for any confusion as well. Things like it would have been easier if I was full time Flashrom and hanging on IRC but sadly I am not. Ouch. I'm sorry to hear that. Hope things get better soon! > I figured addr is of a specific length because of how it is decoded into precisely four unsigned bytes: > > ``` > (addr >> 24) & 0xff, > (addr >> 16) & 0xff, > (addr >> 8) & 0xff, > (addr & 0xff) > ``` Good point! Yes, addr is decoded into 4 bytes for 4BA (4-Byte Address) SPI commands. This would be 3 bytes for regular SPI commands, and I haven't checked what non-SPI flash chips do. So, I agree that addr needs to be at least 4 bytes in size. The point of this patch is to fix build issues on unusual targets like DJGPP. These build issues are regressions: flashrom 1.2 builds successfully on DJGPP. In this case, the easiest solution is to fix the mismatched function definitions. I used `unsigned int` because it's what the rest of flashrom uses. Still, if we agree that `uint32_t` would be a better choice (at least you and I do), we can simply retype everything in another patch. Aaand since `uint32_t` is `unsigned int` on x86, replacing the type should be a reproducible change! 😊 > Is `blocklen` just unused or am I not seeing it? About the blocklen parameter: yes, is unused in many of the block erasers for SPI flash chips. There are some SPI block erasers that use blocklen, and I imagine some non-SPI block erasers do so as well. I suppose blocklen is unused because these block erasers always erase blocks of a certain size, and the block erasers don't need to use the value of blocklen. We could add a sanity check and complain when the value of blocklen doesn't match the expected value. However, I'm not sure how useful this would be, and doing this involves reading *lots* of datasheets: for each block eraser function that does not use blocklen (func) { for each chip that uses $func (chip) { read datasheet of $chip to know the erase block size } } -- To view, visit https://review.coreboot.org/c/flashrom/+/52473 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I656a72b85d4c70b57f6ff9268186a4a60933f8a9 Gerrit-Change-Number: 52473 Gerrit-PatchSet: 3 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Tue, 27 Apr 2021 08:12:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

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

27 Apr '21

Attention is currently required from: Nico Huber, Nicola Corna, Jack Rosenthal, Paul Menzel, Stefan Reinauer, David Hendricks, Daniel Campello, Arthur Heymans, Carl-Daniel Hailfinger. Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/23021 ) Change subject: layout: Add -i <region>[:<file>] support ...................................................................... Patch Set 35: Code-Review+2 (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/23021/comment/7c03ecda_9a616a2f PS35, Line 1493: #ifdef __LIBPAYLOAD__ It feels a bit weird for write_buf_to_include_args() to fail eagerly for __LIBPAYLOAD__ while read_buf_from_include_args() does not. As far as I can tell, it doesn't have any meaningful effect since the first (read|write)_buf_(from|to)_file will fail in that configuration anyway (so read codepaths will never reach read_buf_from_include_args()), but it feels like an unnecessary difference requiring a little bit of digging to understand. -- 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: 35 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: Nico Huber <nico.h(a)gmx.de> 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: Tue, 27 Apr 2021 08:10:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: lspcon_i2c_spi: support a devpath option
by Edward O'Callaghan (Code Review) 27 Apr '21

27 Apr '21

Attention is currently required from: Angel Pons, Peter Marheine. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/51967 ) Change subject: lspcon_i2c_spi: support a devpath option ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/51967 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id2adf8a307b9205ce5e5804a6c3e22f19d0c34c9 Gerrit-Change-Number: 51967 Gerrit-PatchSet: 5 Gerrit-Owner: Peter Marheine <pmarheine(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: Daniel Campello <campello(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Tue, 27 Apr 2021 06:41:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2021