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

Change in flashrom[master]: programmer: Introduce default shutdown function
by Anastasia Klimchuk (Code Review) 07 Jun '21

07 Jun '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54890 ) Change subject: programmer: Introduce default shutdown function ...................................................................... Patch Set 3: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/54890/comment/b1bc1b67_2586b30d PS2, Line 558: if (data) : free(data); > The check is redundant. Remember that CB:54905 has an explanation? 😊 […] Oh yes... done! thank you! I didn't add a comment, but tell me if I misunderstood you, if you think this can benefit from a comment I can compose something. The other thing about "it would be a bug in the programmer code anyway", yes - but not for long! :) We are walking on the route to move data alloc and free into the core. And then, it would be a bug in the register_master code (not in programmer code). Good thing there is only one register_master vs many many programmers. -- To view, visit https://review.coreboot.org/c/flashrom/+/54890 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8b20717ba549e12edffbc5d1643ff064a4f0c517 Gerrit-Change-Number: 54890 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: 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 07 Jun 2021 04:50:17 +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]: programmer: Introduce default shutdown function
by Anastasia Klimchuk (Code Review) 07 Jun '21

07 Jun '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/54890 to look at the new patch set (#3). Change subject: programmer: Introduce default shutdown function ...................................................................... programmer: Introduce default shutdown function Default shutdown function is only freeing master data (if data exists) and not doing anything else. There are five spi masters which can use default shutdown function (all included in this patch), and a lot of par masters that can benefit from it as well. BUG=b:185191942 TEST=builds Change-Id: I8b20717ba549e12edffbc5d1643ff064a4f0c517 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M flashrom.c M mcp6x_spi.c M nicintel_spi.c M ogp_spi.c M programmer.h M rayer_spi.c M usbblaster_spi.c 7 files changed, 12 insertions(+), 35 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/90/54890/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/54890 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8b20717ba549e12edffbc5d1643ff064a4f0c517 Gerrit-Change-Number: 54890 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: 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-MessageType: newpatchset

1 0

Change in flashrom[master]: enum programmer: not needed anymore. Remove
by Edward O'Callaghan (Code Review) 07 Jun '21

07 Jun '21

Attention is currently required from: Thomas Heijligen. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55124 ) Change subject: enum programmer: not needed anymore. Remove ...................................................................... Patch Set 5: Code-Review+2 (1 comment) Patchset: PS5: > \o/ ヽ(•‿•)ノ -- To view, visit https://review.coreboot.org/c/flashrom/+/55124 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I53cdb160616911a4beea6b5e8e56d582621818a4 Gerrit-Change-Number: 55124 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Xiang W <wxjstz(a)126.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Mon, 07 Jun 2021 04:21:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: replace enum programmer by struct programmer_entry*
by Edward O'Callaghan (Code Review) 07 Jun '21

07 Jun '21

Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55119 ) Change subject: flashrom.c: replace enum programmer by struct programmer_entry* ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/55119 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I478c56354d5f482010bfe1560489700bc889717a Gerrit-Change-Number: 55119 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: 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-CC: Xiang W <wxjstz(a)126.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 07 Jun 2021 04:19:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer_table: print.c use fixed fixed name instead of reference
by Edward O'Callaghan (Code Review) 07 Jun '21

07 Jun '21

Attention is currently required from: Thomas Heijligen. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55118 ) Change subject: programmer_table: print.c use fixed fixed name instead of reference ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/55118/comment/fbe3d2ff_15b61317 PS3, Line 7: programmer_table: print.c use fixed fixed name instead of reference How about `print.c: Use static-string in print` -- To view, visit https://review.coreboot.org/c/flashrom/+/55118 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If2cf95c71425efdd864457e213dd34b929fe8805 Gerrit-Change-Number: 55118 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Xiang W <wxjstz(a)126.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Mon, 07 Jun 2021 04:18:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer_table: replace PROGRAMMER_INVALID to programmer_table_size...
by Edward O'Callaghan (Code Review) 07 Jun '21

07 Jun '21

Attention is currently required from: Nico Huber, Thomas Heijligen. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55117 ) Change subject: programmer_table: replace PROGRAMMER_INVALID to programmer_table_size in itterations ...................................................................... Patch Set 3: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/55117/comment/c62af569_934ef5d4 PS3, Line 7: programmer_table: replace PROGRAMMER_INVALID to programmer_table_size in itterations > It's just too long, how about […] could this just be squashed into the previous patch that introduced it earlier in the serires? Seems reasonable to have it introduced and used in the same patch. -- To view, visit https://review.coreboot.org/c/flashrom/+/55117 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Icaaeefe001de604df9d7fdd06f05a5ed39fdbd84 Gerrit-Change-Number: 55117 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> 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-CC: Xiang W <wxjstz(a)126.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Mon, 07 Jun 2021 04:17:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer_table: move each entry to the associated programmer source
by Edward O'Callaghan (Code Review) 07 Jun '21

07 Jun '21

Attention is currently required from: Thomas Heijligen. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52946 ) Change subject: programmer_table: move each entry to the associated programmer source ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/52946 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3d02bd789f0299e936eb86819b3b15b5ea2bb921 Gerrit-Change-Number: 52946 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Xiang W <wxjstz(a)126.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Mon, 07 Jun 2021 04:15:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer_table: add table size variable
by Edward O'Callaghan (Code Review) 07 Jun '21

07 Jun '21

Attention is currently required from: Thomas Heijligen, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55116 ) Change subject: programmer_table: add table size variable ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/55116 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Idacb4b7b0dd98af8bc7cbecf567d33daafb4b24d Gerrit-Change-Number: 55116 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: 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-CC: Xiang W <wxjstz(a)126.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 07 Jun 2021 04:13:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: programmer_table: move array content to an own file
by Edward O'Callaghan (Code Review) 07 Jun '21

07 Jun '21

Attention is currently required from: Thomas Heijligen. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52944 ) Change subject: programmer_table: move array content to an own file ...................................................................... Patch Set 3: Code-Review+1 (1 comment) Patchset: PS3: I look forward to this landing. Thanks for all the work Thomas! -- To view, visit https://review.coreboot.org/c/flashrom/+/52944 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8e6d704e845ee4152c8676dd19dff0934fff007b Gerrit-Change-Number: 52944 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Xiang W <wxjstz(a)126.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Mon, 07 Jun 2021 04:13:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: dummyflasher.c: Fix data leak in params processing error paths
by Anastasia Klimchuk (Code Review) 07 Jun '21

07 Jun '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54748 ) Change subject: dummyflasher.c: Fix data leak in params processing error paths ...................................................................... Patch Set 5: (4 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/54748/comment/f27ff257_c8034a9a PS4, Line 11: A good side-effect of the change is: 13 error paths of params : processing are not leaking data anymore. All those error paths : return 1 back to init function which frees data. : : And there was just one more error path in init function were free(data) : needed to be added. > These are the actual changes. Everything else is a mere refactoring. […] You are back!! This is awesome! :) Done, I updated commit message. https://review.coreboot.org/c/flashrom/+/54748/comment/01fbda58_c934e882 PS4, Line 20: > Could add a […] I haven't seen this tag before, is it used often? I saw people mention in commit message "follow-up something something that was introduced in CB:xxxxx" or similar. Can I do like this instead? File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/54748/comment/e16fcb36_8bdb1c01 PS1, Line 26: /* Remove the #define below if you don't want SPI flash chip emulation. */ : #define EMULATE_SPI_CHIP 1 > IMO, both defines should just be dropped. If they were compile-time configs […] Just to confirm, you mean, let's assume (EMULATE_CHIP && EMULATE_SPI_CHIP) always true and drop the code outside the #if? So that dummy will always be dummy_spi? But then maybe have another one dummy_par? Maybe someone needs those different values. I don't know what was the initial meaning of !EMULATE_CHIP use case. What dummy is doing if it is not emulating any chip? I see it can emulate spi or par, I can understand, but not emulating anything? File dummyflasher.c: https://review.coreboot.org/c/flashrom/+/54748/comment/e8f94f97_caade0bd PS4, Line 994: dummy_buses_supported > Makes one wonder why this is not part of `emu_data`... I was thinking about it, and my guess was, because it is only needed for init time, and not needed later. It would never be used as a part of emu_data. Yes adding second parameter in init_data felt slightly awkward, but adding into emu_data and then never using it would also be... -- To view, visit https://review.coreboot.org/c/flashrom/+/54748 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I04f55f77bb4703f1d88b2191c45a22be3c97bf87 Gerrit-Change-Number: 54748 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: Namyoon Woo <namyoon(a)google.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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 07 Jun 2021 02:02:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit June 2021