Attention is currently required from: Nico Huber, Angel Pons.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55107 )
Change subject: nic3com.c: Refactor singleton states into reentrant pattern
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
> Looks like a duplicate of CB:55106?
Oh no, looks like I created a mess :( This patch is the right one, another one is duplicate. Sorry for mess. I squashed two commits, and uploaded as one, this one.
I will resolved your comments from CB:55106 here, let's keep this comment unresolved until I do this.
--
To view, visit https://review.coreboot.org/c/flashrom/+/55107
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I9834b82650cd070556cf82207796bc6bd6b31b28
Gerrit-Change-Number: 55107
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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Mon, 07 Jun 2021 05:26:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
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
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
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
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
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