Attention is currently required from: Nico Huber, Furquan Shaikh, Tim Wawrzynczak, Edward O'Callaghan.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56721 )
Change subject: internal: Return early from map_flash for >16MiB on x86
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS1:
> Sorry that was Patchset 1, where I got asked to switch it to the physmap area... [鈥
I do admit I haven't read the full history of the patch (sorry), I added my comment just based on what I saw in the latest patchset.
Ok, so now I did read comments from the beginning.
Is this what you are referring to:
platform specific code it probably should be encapsulated within internal.c
itself. The flashrom.c implementation is generic dispatch and handling code and
so should not really have platform specifics here.
Now I understand where is comes from, because in internal.c
.map_flash_region = physmap,
In this case, if internal needs something special, can it have its own internal_map function which does all the special things? I can see why flashrom.c is not the right place, but I still think physmap_common is too deep (so, not the right place either). The problem is, at the moment there is nothing in between, but maybe let's create something in between?
What do people think?
--
To view, visit https://review.coreboot.org/c/flashrom/+/56721
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I60244b6970118dbdfdbb5b8424fc0a8acd9def2e
Gerrit-Change-Number: 56721
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Fri, 27 Aug 2021 06:33:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Angel Pons.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57156 )
Change subject: par_master: Use new API to register shutdown function
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/57156/comment/9d443708_67296b98
PS1, Line 17: not ignored anymore.
> You do this for a lot of programmer drivers that are otherwise [鈥
You are right, I agree! I initially did "the same thing as before" but actually par masters are different, most of them don't have shutdown function. Totally makes sense to have a separate commit for those.
I left two programmers here, which do have shutdown function. All the rest is in next commit, and the commit message explains what's happening (fix propagation of return values).
--
To view, visit https://review.coreboot.org/c/flashrom/+/57156
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ief7be907f53878b4b6567b52889735e5fff64ead
Gerrit-Change-Number: 57156
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: Fri, 27 Aug 2021 06:12:09 +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: Angel Pons, 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/+/57156
to look at the new patch set (#2).
Change subject: par_master: Use new API to register shutdown function
......................................................................
par_master: Use new API to register shutdown function
This allows par masters to register shutdown function in par_master
struct, which means there is no need to call register_shutdown in init
function, since this call is now a part of register_par_master.
As a consequence of using new API, this patch also fixes propagation
of register_par_master() return values.
BUG=b:185191942
TEST=builds and ninja test
Change-Id: Ief7be907f53878b4b6567b52889735e5fff64ead
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M nic3com.c
M nicrealtek.c
2 files changed, 4 insertions(+), 15 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/56/57156/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/57156
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ief7be907f53878b4b6567b52889735e5fff64ead
Gerrit-Change-Number: 57156
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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Furquan Shaikh, Edward O'Callaghan, Anastasia Klimchuk.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56721 )
Change subject: internal: Return early from map_flash for >16MiB on x86
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS1:
> To me, it looks like we already went too deep, all the way to physmap_common, and only then realised [鈥
Sorry that was Patchset 1, where I got asked to switch it to the physmap area... is there some way to get this implemented? I get told physmap isn't the right place and also flashrom.c is not the right place and also it is... I'm confused 馃槙
--
To view, visit https://review.coreboot.org/c/flashrom/+/56721
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I60244b6970118dbdfdbb5b8424fc0a8acd9def2e
Gerrit-Change-Number: 56721
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Furquan Shaikh <furquan(a)google.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Thu, 26 Aug 2021 22:43:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Anastasia Klimchuk.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57156 )
Change subject: par_master: Use new API to register shutdown function
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/57156/comment/99fc16bc_5ca3d28d
PS1, Line 17: not ignored anymore.
You do this for a lot of programmer drivers that are otherwise
not affected by this patch. IMO, this should be done in a separate
commit. Or, at least, the commit summary line should somehow
catch it.
--
To view, visit https://review.coreboot.org/c/flashrom/+/57156
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ief7be907f53878b4b6567b52889735e5fff64ead
Gerrit-Change-Number: 57156
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: 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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Thu, 26 Aug 2021 08:59:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment