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

Change in flashrom[master]: ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops
by Michael Niewöhner (Code Review) 24 Jun '21

24 Jun '21

Attention is currently required from: Felix Singer, Nico Huber, Michał Żygowski, Paul Menzel, Angel Pons. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55715 ) Change subject: ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops ...................................................................... Patch Set 7: (3 comments) Patchset: PS4: > After a second thought, I think not all laptops with ITE ECs may have a distinguishable SVID/SDID set. Do you know any? AFAIK they are unique. PS4: > Got some more information on the ITEString and found out what the autload patching is really doing. […] Done File tuxedo_ec.c: https://review.coreboot.org/c/flashrom/+/55715/comment/e36f52d0_aa7dda57 PS4, Line 822: if (strcmp(current_ec_project, new_ec_project)) { : msg_perr("Wrong EC project. This file can't be used on this machine\n"); : return 1; : } : > Should it be the programmer parameter or should I hook the global force flag? global force is fine. I like the current solution 😊 -- To view, visit https://review.coreboot.org/c/flashrom/+/55715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 Gerrit-Change-Number: 55715 Gerrit-PatchSet: 7 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 24 Jun 2021 17:56:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: nicintel_spi.c: Cache FLA register value
by Idwer Vollering (Code Review) 24 Jun '21

24 Jun '21

Attention is currently required from: Nico Huber, Angel Pons. Idwer Vollering has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55720 ) Change subject: nicintel_spi.c: Cache FLA register value ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Tested with a 8086:107c device (82541PI). -- To view, visit https://review.coreboot.org/c/flashrom/+/55720 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4013a71ec498be977f62fe2531ac8770bfffadcd Gerrit-Change-Number: 55720 Gerrit-PatchSet: 2 Gerrit-Owner: 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: Idwer Vollering <vidwer(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 24 Jun 2021 17:43:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: nicintel_spi.c: Implement `set_sck_set_mosi` and `set_sck_get_miso`
by Idwer Vollering (Code Review) 24 Jun '21

24 Jun '21

Attention is currently required from: Nico Huber, Angel Pons. Idwer Vollering has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/49267 ) Change subject: nicintel_spi.c: Implement `set_sck_set_mosi` and `set_sck_get_miso` ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: Tested with a 8086:107c device (82541PI). -- To view, visit https://review.coreboot.org/c/flashrom/+/49267 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I52a0ae5ff331ee2ed41652eb5c2ed7ebe7253d74 Gerrit-Change-Number: 49267 Gerrit-PatchSet: 3 Gerrit-Owner: 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: Idwer Vollering <vidwer(a)gmail.com> Gerrit-CC: Shawn C <citypw(a)hardenedlinux.org> Gerrit-CC: Xiang W <wxjstz(a)126.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 24 Jun 2021 17:43:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops
by Felix Singer (Code Review) 24 Jun '21

24 Jun '21

Attention is currently required from: Nico Huber, Michał Żygowski, Paul Menzel, Angel Pons, Michael Niewöhner. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55715 ) Change subject: ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops ...................................................................... Patch Set 7: (1 comment) Patchset: PS4: > After a second thought, I think not all laptops with ITE ECs may have a distinguishable SVID/SDID se […] Speaking for TUXEDO, we would like this to be generic as possible. The TUXEDO EC firmware differs in some parts to the Clevo one, but the flashing / updating procedure is the same as with the Clevo firmware. In other words, there is absolutely no need to do any DMI checks since the updating procedure is the same for every Clevo EC. I like the idea checking for the PCI subsystem vendor/device ID. Let's do this. -- To view, visit https://review.coreboot.org/c/flashrom/+/55715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 Gerrit-Change-Number: 55715 Gerrit-PatchSet: 7 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Comment-Date: Thu, 24 Jun 2021 17:07:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops
by Michał Żygowski (Code Review) 24 Jun '21

24 Jun '21

Attention is currently required from: Felix Singer, Nico Huber, Paul Menzel, Angel Pons, Michael Niewöhner. Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55715 ) Change subject: ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops ...................................................................... Patch Set 6: (12 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/55715/comment/2be3f4bf_1bfe778a PS4, Line 11: updates from IBV. > Can you please document the EC firmware version used, and give an example command? Done Patchset: PS4: > Some SVID/SDID examples: […] After a second thought, I think not all laptops with ITE ECs may have a distinguishable SVID/SDID set. In order to make this programmer more generic, leaving the DMI checks and will add DMI bypass option with force flag (however, we need to add a function to get the flash context somehow or pass it to programmer init) PS4: > > The IT8987E and IT5570E/IT5576E are a bit special. […] Got some more information on the ITEString and found out what the autload patching is really doing. It sets some kind of EC Signature flag and changes the EC flash size to be mirrored File Makefile: https://review.coreboot.org/c/flashrom/+/55715/comment/4cdf808d_af21a7ca PS1, Line 839: override CONFIG_TUXEDO_EC = no > Probably many other also don't, like MEC1308 from what I can see in the code... […] Done File flashrom.8.tmpl: https://review.coreboot.org/c/flashrom/+/55715/comment/df77aa1c_27184f2a PS4, Line 1376: The default is 1 > `tuxedo_ec_init_ctx` is setting these defaults Ack File flashrom.c: https://review.coreboot.org/c/flashrom/+/55715/comment/625f9aa6_0e3504de PS1, Line 2109: #if CONFIG_TUXEDO_EC == 1 > The coreboot check above is a very bad rolemodel. It does things flashrom shouldn't > do (look at the contents) and in a way (doing programmer-specific things in common > code; a layering violation) that we try to get rid of. > > I think it would be best to call _verify_file_project() from programmer init. The UI > would then have to feed the file path (or contents) two times. If we really want such > a check, flashrom (or the library) shouldn't offer any operation if the contents look > incompatible. > Flashrom shouldn't even touch the hardware before content compatibility fails IMO. The approach with calling the function from programmer init is simple, but different programmers may need different arguments to be passed. I wonder how to address that. Maybe the programmer_entry structure should expose an optional method like `check_file` which would assess compatibility of the contents with the target hardware/flash. File tuxedo_ec.c: https://review.coreboot.org/c/flashrom/+/55715/comment/a2a66eb1_575f5b17 PS1, Line 205: ec_read_reg > Because reading the AC adapter state was always done with the hardcoded default register pair 0x62/0 […] Ack https://review.coreboot.org/c/flashrom/+/55715/comment/db21edde_89d9ffbd PS1, Line 477: internal_sleep(15000 * 64); > Unfortunately no way that we are aware of. Ack File tuxedo_ec.c: https://review.coreboot.org/c/flashrom/+/55715/comment/743b23f2_538f6c30 PS4, Line 661: dmi_init(); : : if (!dmi_match("^TUXEDO$")) { : msg_perr("Not a TUXEDO device\n"); : return 1; : } : : for (i = 0; i < ARRAY_SIZE(tuxedo_supported_boards); i++) { : if (dmi_match(tuxedo_supported_boards[i])) { : match = true; : break; : } : } : : if (!match) { : msg_perr("TUXEDO EC programmer not yet supported on this device\n"); : return 1; : } > I was thinking about the same. Thanks for the piece of code. […] Done https://review.coreboot.org/c/flashrom/+/55715/comment/c7589d03_0a63061a PS4, Line 688: sizeof(struct tuxedo_ec_data) > sizeof(*ctx_data) Done https://review.coreboot.org/c/flashrom/+/55715/comment/9a6264a1_f6c1d0ba PS4, Line 740: uint8_t *const contents, char *buf > nit: I'd flip the parameter order to match the natural assignment semantics (leftmost parameter is t […] Done https://review.coreboot.org/c/flashrom/+/55715/comment/ad2f01bd_578f0f2c PS4, Line 822: if (strcmp(current_ec_project, new_ec_project)) { : msg_perr("Wrong EC project. This file can't be used on this machine\n"); : return 1; : } : > can we add a parameter for forcing here? One might use flashrom to switch from system76 ec to vendor […] Should it be the programmer parameter or should I hook the global force flag? -- To view, visit https://review.coreboot.org/c/flashrom/+/55715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 Gerrit-Change-Number: 55715 Gerrit-PatchSet: 6 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Comment-Date: Thu, 24 Jun 2021 15:48:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops
by Michał Żygowski (Code Review) 24 Jun '21

24 Jun '21

Attention is currently required from: Felix Singer, Nico Huber, Michał Żygowski, Paul Menzel, Angel Pons. Hello Felix Singer, build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/55715 to look at the new patch set (#7). Change subject: ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops ...................................................................... ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops Add a new programmer supporting ITE Embedded Controllers found on TUXEDO laptops. Tested on TUXEDO InfinityBook S 14 Gen6 and 15 Gen6 with EC firmware updates from IBV, EC versions 1.07.02TR1 and 1.07.04TR4. Example standard command to update EC firmware on a TUXEDO laptop: "flashrom -p ite_ec:romsize=128K,autoload=disable -w eL14MU02.TR1" Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 --- M Makefile M flashrom.8.tmpl A ite_ec.c A ite_ec.h M programmer.h M programmer_table.c 6 files changed, 1,028 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/15/55715/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/55715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 Gerrit-Change-Number: 55715 Gerrit-PatchSet: 7 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops
by Michał Żygowski (Code Review) 24 Jun '21

24 Jun '21

Attention is currently required from: Felix Singer, Nico Huber, Michał Żygowski, Paul Menzel, Angel Pons. Hello Felix Singer, build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/55715 to look at the new patch set (#6). Change subject: ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops ...................................................................... ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops Add a new programmer supporting ITE Embedded Controllers found on TUXEDO laptops. Tested on TUXEDO InfinityBook S 14 Gen6 and 15 Gen6 with EC firmware updates from IBV, EC versions 1.07.02TR1 and 1.07.04TR4. Example standard command to update EC firmware on a TUXEDO laptop: "flashrom -p ite_ec:romsize=128K,autoload=disable -w eL14MU02.TR1" Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 --- M Makefile M flashrom.8.tmpl A ite_ec.c A ite_ec.h M programmer.h M programmer_table.c 6 files changed, 1,024 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/15/55715/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/55715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 Gerrit-Change-Number: 55715 Gerrit-PatchSet: 6 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops
by Michał Żygowski (Code Review) 24 Jun '21

24 Jun '21

Attention is currently required from: Felix Singer, Nico Huber, Michał Żygowski, Paul Menzel, Angel Pons. Hello Felix Singer, build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/55715 to look at the new patch set (#5). Change subject: ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops ...................................................................... ite_ec: Implement support for flashing ITE ECs found on TUXEDO laptops Add a new programmer supporting ITE Embedded Controllers found on TUXEDO laptops. Tested on TUXEDO InfinityBook S 14 Gen6 and 15 Gen6 with EC firmware updates from IBV, EC versions 1.07.02TR1 and 1.07.04TR4. Signed-off-by: Michał Żygowski <michal.zygowski(a)3mdeb.com> Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 --- M Makefile M flashrom.8.tmpl A ite_ec.c A ite_ec.h M programmer.h M programmer_table.c 6 files changed, 1,024 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/15/55715/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/55715 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e42260155ffea38a6f60790871cd8da7b657031 Gerrit-Change-Number: 55715 Gerrit-PatchSet: 5 Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: layout: Use linked list for `struct romentry`
by Angel Pons (Code Review) 24 Jun '21

24 Jun '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Daniel Campello, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/33521 ) Change subject: layout: Use linked list for `struct romentry` ...................................................................... Patch Set 8: (3 comments) File layout.c: https://review.coreboot.org/c/flashrom/+/33521/comment/688a6790_df0f841c PS8, Line 388: *layout = malloc(sizeof(**layout)); Given that you memset() the allocated space afterwards, use calloc() instead? https://review.coreboot.org/c/flashrom/+/33521/comment/903fcdc0_b1e25a90 PS8, Line 418: entry->next = layout->head; > I missed the fact that in the beginning when layout has no entries, layout->head is null (or maybe undefined). It's null, flashrom_layout_new() zero-initialises the newly-allocated `struct layout`. The null pointer signals the end of the linked list. https://review.coreboot.org/c/flashrom/+/33521/comment/99b9de11_72f8a74f PS8, Line 458: if (layout == global_layout) : return; : : while (layout && layout->head) { : struct romentry *const entry = layout->head; : layout->head = entry->next; : free(entry->file); : free(entry->name); : free(entry); : } : free(layout); Given that `layout` is not modified in the while loop, how about evaluating it once? if (!layout || layout == global_layout) return; while (layout->head) { struct romentry *const entry = layout->head; layout->head = entry->next; free(entry->file); free(entry->name); free(entry); } free(layout); -- To view, visit https://review.coreboot.org/c/flashrom/+/33521 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I60a0aa1007ebcd5eb401db116f835d129b3e9732 Gerrit-Change-Number: 33521 Gerrit-PatchSet: 8 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> 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: 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: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 24 Jun 2021 10:12:22 +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]: buspirate_spi: Use non-zero return value from buspirate_sendrecv
by Angel Pons (Code Review) 24 Jun '21

24 Jun '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55818 ) Change subject: buspirate_spi: Use non-zero return value from buspirate_sendrecv ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/55818 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I31fd70f607dc965d5cac1cd0116faa447dbc177a Gerrit-Change-Number: 55818 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: 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 24 Jun 2021 09:58:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit June 2021