Attention is currently required from: Angel Pons, Anastasia Klimchuk.
Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59921 )
Change subject: flashrom.c: extract operation only uses layout files
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/59921/comment/dd313a41_50bdd5b4
PS2, Line 9: a bug
> Could you please specify the commit that introduced the bug? For example, using a Fixes: tag.
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/59921
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ibc9a4e2966385863345f06662521d6d0e4685121
Gerrit-Change-Number: 59921
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Tue, 07 Dec 2021 18:55:37 +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: Daniel Campello, Anastasia Klimchuk.
Hello build bot (Jenkins), Edward O'Callaghan, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/59921
to look at the new patch set (#3).
Change subject: flashrom.c: extract operation only uses layout files
......................................................................
flashrom.c: extract operation only uses layout files
This change fixes a bug on handling the extract operation. The extract
operation reads out the layout regions to filenames corresponding to the
respective layout region names. read_flash_to_file() does this work via
write_buf_to_include_args(). This change makes the call to
write_buf_to_file() optional as it is still required for -r (read
operation) but not for -x (extract operation).
BUG=b:209512852
TEST=flashrom -x
Fixes: https://review.coreboot.org/c/flashrom/+/52450
Change-Id: Ibc9a4e2966385863345f06662521d6d0e4685121
Signed-off-by: Daniel Campello <campello(a)chromium.org>
---
M flashrom.c
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/59921/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/59921
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ibc9a4e2966385863345f06662521d6d0e4685121
Gerrit-Change-Number: 59921
Gerrit-PatchSet: 3
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Thomas Heijligen.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58623 )
Change subject: Makefile: Make pkg-config mandatory to find libftdi1
......................................................................
Patch Set 8:
(1 comment)
File Makefile.include:
https://review.coreboot.org/c/flashrom/+/58623/comment/15413044_234cb51c
PS5, Line 40: 2>/dev/null
Looking at CB:59230 made me realize that we don't output into
`build_details.txt` anymore. Is the plan to drop it entirely?
Looking at 3f73ffec, it seems we'd be walking in circles.
--
To view, visit https://review.coreboot.org/c/flashrom/+/58623
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I41f5186d9f3e063c12c8c6eea888d0b0bf534259
Gerrit-Change-Number: 58623
Gerrit-PatchSet: 8
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: 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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Comment-Date: Tue, 07 Dec 2021 15:09:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59050 )
Change subject: Makefile: Make pkg-config mandatory to find libpci
......................................................................
Patch Set 12:
(1 comment)
Patchset:
PS12:
> I'm not 100% sure it's a move into the right direction.
Functionality wise. The Makefile code looks much better of course :)
--
To view, visit https://review.coreboot.org/c/flashrom/+/59050
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I21b4a261b34b7e688635fc6e20b7beebfa64c7ed
Gerrit-Change-Number: 59050
Gerrit-PatchSet: 12
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: 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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Comment-Date: Tue, 07 Dec 2021 15:01:03 +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: Thomas Heijligen.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59050 )
Change subject: Makefile: Make pkg-config mandatory to find libpci
......................................................................
Patch Set 12: Code-Review+1
(2 comments)
Patchset:
PS12:
I'm not 100% sure it's a move into the right direction. If we
want reliable build results at some point, i.e. don't disable
programmers automatically, we'd have to bring back some of the
dropped functionality later. For instance, listing the (enabled)
programmers that require libpci. Would it be hard to maintain
this at the moment?
File Makefile:
https://review.coreboot.org/c/flashrom/+/59050/comment/0f77180f_9fb798c8
PS7, Line 772: # This is a dirty hack, but it saves us from checking all PCI drivers and all platforms manually.
: # libpci may need raw memory, MSR or PCI port I/O on some platforms.
: # Individual drivers might have the same needs as well.
Doesn't this comment stay valid? I guess we should at some point untangle our
use of `physmap.c`/`hwaccess.c` from libpci dependencies. i.e. if libpci needs
something, it would be to link with another library, but libpci would never
need anything in our code (`physmap.c` etc.), right?
--
To view, visit https://review.coreboot.org/c/flashrom/+/59050
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I21b4a261b34b7e688635fc6e20b7beebfa64c7ed
Gerrit-Change-Number: 59050
Gerrit-PatchSet: 12
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: 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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Comment-Date: Tue, 07 Dec 2021 15:00:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Daniel Campello, Anastasia Klimchuk.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59921 )
Change subject: flashrom.c: extract operation only uses layout files
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/59921/comment/0c6cf51b_f85385be
PS2, Line 9: a bug
Could you please specify the commit that introduced the bug? For example, using a Fixes: tag.
--
To view, visit https://review.coreboot.org/c/flashrom/+/59921
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ibc9a4e2966385863345f06662521d6d0e4685121
Gerrit-Change-Number: 59921
Gerrit-PatchSet: 2
Gerrit-Owner: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Tue, 07 Dec 2021 13:10:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59049 )
Change subject: Makefile: Make pkg-config mandatory to find libusb1
......................................................................
Patch Set 11: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/59049
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4f24be647d25dde24c41514f8964b7134205867c
Gerrit-Change-Number: 59049
Gerrit-PatchSet: 11
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: 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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Comment-Date: Tue, 07 Dec 2021 11:11:57 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59048 )
Change subject: Makefile: Make pkg-config mandatory to find libjaylink
......................................................................
Patch Set 11: Code-Review+2
(1 comment)
Patchset:
PS11:
NB. We could update Manibuilder tests that don't need an explicit
`CONFIG_...=no` anymore. OTOH, I would like to get rid of automatic
disabling of programmer drivers eventually (makes it harder to get
reproducible build results).
--
To view, visit https://review.coreboot.org/c/flashrom/+/59048
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I99df547046bb9820ab502f89f6d4452c1bc0cfd4
Gerrit-Change-Number: 59048
Gerrit-PatchSet: 11
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: 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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Comment-Date: Tue, 07 Dec 2021 11:08:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Miklós Márton, Thomas Heijligen, Angel Pons.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59047 )
Change subject: Makefile: Rework NI-845x detection
......................................................................
Patch Set 11:
(2 comments)
Patchset:
PS11:
Miklós can you have a look at this? We installed the library on
a 64-bit Windows and everything worked fine (without the Program
Files (x86) kludge). Is it worthwhile to maintain it?
File Makefile:
https://review.coreboot.org/c/flashrom/+/59047/comment/64f25d20_9cb43c65
PS11, Line 740: # hack to access env variable containing brackets...
: PROGRAMFILES_X86DIR = $(shell env | sed -n "s/^PROGRAMFILES(X86)=//p")
:
: ifneq ($(PROGRAMFILES_X86DIR),)
: ifneq ($(PROGRAMFILES_X86DIR), ${PROGRAMFILES})
: NI845X_LIBS += -L'$(PROGRAMFILES_X86DIR)\National Instruments\NI-845x\MS Visual C'
: NI845X_INCLUDES += -I'$(PROGRAMFILES_X86DIR)\National Instruments\NI-845x\MS Visual C'
: endif
: endif
I would like to maintain this unless Miklós agrees that it's not really needed
(as this covers only 32-bit library on 64-bit Windows and 64-bit libraries
seem to be available anyway).
--
To view, visit https://review.coreboot.org/c/flashrom/+/59047
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I918c3605a5ac168708a6a10fd92ee2a1aae9729b
Gerrit-Change-Number: 59047
Gerrit-PatchSet: 11
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
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-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Tue, 07 Dec 2021 11:02:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58623 )
Change subject: Makefile: Make pkg-config mandatory to find libftdi1
......................................................................
Patch Set 8: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/58623
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I41f5186d9f3e063c12c8c6eea888d0b0bf534259
Gerrit-Change-Number: 58623
Gerrit-PatchSet: 8
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: 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: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Comment-Date: Tue, 07 Dec 2021 10:48:50 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment