Attention is currently required from: Felix Singer, Nico Huber.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62196 )
Change subject: Makefile: use HAS_ USE_ pattern for serial support
......................................................................
Patch Set 2:
(1 comment)
File Makefile:
https://review.coreboot.org/c/flashrom/+/62196/comment/b916e4de_e67b85ac
PS1, Line 795: ,$(DEPENDS_ON_SERIAL)),yes,no)
> add missing spaces
the `filter_deps` function can't handle spaces yet. All existing calls are like this.
--
To view, visit https://review.coreboot.org/c/flashrom/+/62196
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ica951e76d6362b01f09d23a729a2a6049e7f0b66
Gerrit-Change-Number: 62196
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
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-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Mon, 21 Feb 2022 13:19:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62195 )
Change subject: Makefile: Rework the EXEC_SUFFIX determination
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62195/comment/f5b44fb3_12f55fdb
PS1, Line 10: multible
> multi_p_le
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/62195
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Iee66dbc609bd5c6eb9d04b457f4508911b2e6560
Gerrit-Change-Number: 62195
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
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-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Comment-Date: Mon, 21 Feb 2022 13:17:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Thomas Heijligen.
Hello Felix Singer, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62196
to look at the new patch set (#2).
Change subject: Makefile: use HAS_ USE_ pattern for serial support
......................................................................
Makefile: use HAS_ USE_ pattern for serial support
Align the usage of serial function with the selection of other
dependencies.
Change-Id: Ica951e76d6362b01f09d23a729a2a6049e7f0b66
Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com>
---
M Makefile
1 file changed, 7 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/62196/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/62196
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ica951e76d6362b01f09d23a729a2a6049e7f0b66
Gerrit-Change-Number: 62196
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
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-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Nico Huber.
Hello Felix Singer, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62198
to look at the new patch set (#2).
Change subject: Makefile: use libflashrom.a as input to build the flashrom executable
......................................................................
Makefile: use libflashrom.a as input to build the flashrom executable
Change-Id: Ib0091a23611cd5a1d915e56c6d0f061d74198e88
Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com>
---
M Makefile
1 file changed, 5 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/98/62198/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/62198
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ib0091a23611cd5a1d915e56c6d0f061d74198e88
Gerrit-Change-Number: 62198
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
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-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Thomas Heijligen.
Hello Felix Singer, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62199
to look at the new patch set (#2).
Change subject: Makefile: print version info as part of the config target
......................................................................
Makefile: print version info as part of the config target
Change-Id: I1a846acfd8d2e0a9fc8b02c078b6ac0342438490
Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com>
---
M Makefile
1 file changed, 1 insertion(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/99/62199/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/62199
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1a846acfd8d2e0a9fc8b02c078b6ac0342438490
Gerrit-Change-Number: 62199
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
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-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Thomas Heijligen.
Hello Felix Singer, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62197
to look at the new patch set (#2).
Change subject: Makefile: use the HAS_ USE_ scheme for linux i2c dependend programmer
......................................................................
Makefile: use the HAS_ USE_ scheme for linux i2c dependend programmer
Change-Id: I47acdf89a369441b9fc664352c27c43b461545b1
Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com>
---
M Makefile
1 file changed, 7 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/97/62197/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/62197
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I47acdf89a369441b9fc664352c27c43b461545b1
Gerrit-Change-Number: 62197
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
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-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Thomas Heijligen.
Hello Felix Singer, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62195
to look at the new patch set (#2).
Change subject: Makefile: Rework the EXEC_SUFFIX determination
......................................................................
Makefile: Rework the EXEC_SUFFIX determination
Use a conditional function for the statement. This limits the decision
to one line instead of multiple places.
Change-Id: Iee66dbc609bd5c6eb9d04b457f4508911b2e6560
Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com>
---
M Makefile
1 file changed, 1 insertion(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/62195/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/62195
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Iee66dbc609bd5c6eb9d04b457f4508911b2e6560
Gerrit-Change-Number: 62195
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
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-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Subrata Banik, Edward O'Callaghan.
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62213 )
Change subject: flashrom.c: Implement file-based locking semantics
......................................................................
Patch Set 2:
(3 comments)
File big_lock.c:
https://review.coreboot.org/c/flashrom/+/62213/comment/1bda94c6_23a8a0ff
PS1, Line 39: return acquire_lock(&big_lock, timeout_secs * 1000);
> Seems unnecessary complexity to make it conditional since we would always want it to be the case tha […]
I'm concerned the file-based locking may not work on all systems, for example Android or Windows, or any other systems that previously supports flashrom without problems.
File big_lock.c:
https://review.coreboot.org/c/flashrom/+/62213/comment/f8ca8977_704e1087
PS2, Line 37: firmware_utility_lock
should this be flashrom_lock ?
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/62213/comment/e2d2e50f_5c714c0f
PS1, Line 142: programmer_entry
> afaik, this is issue is specifically related to futility tests not yet running under platform2_wrapp […]
Not just futility - this is more like "can you run flashrom without getting root permission (required by lock)". I think people will want to simulate a flashrom call using dummy programmer (to create an image file) without adding 'sudo'.
But on the other hand, it's also probably not a bad idea if we always require 'sudo' - even for unit tests. I'm fine if you want to keep the biglock always there.
One more thing to check: we won't use two programmers simultaneously right? Because currently the lock is a simple 1/0 state. We may need to consider a real singleton (e.g., add ref counter) if some programs calling libflashrom will try to get two programmers , for example read from one and write to the other. (flashrom doesn't do this but we don't know what libflashrom users will do)
--
To view, visit https://review.coreboot.org/c/flashrom/+/62213
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I19cb4e3bf14caeb67c3e8100a20395b264c5113a
Gerrit-Change-Number: 62213
Gerrit-PatchSet: 2
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Mon, 21 Feb 2022 05:29:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Hung-Te Lin <hungte(a)chromium.org>
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Hung-Te Lin, Nico Huber, Subrata Banik.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62213 )
Change subject: flashrom.c: Implement file-based locking semantics
......................................................................
Patch Set 1:
(5 comments)
File big_lock.c:
https://review.coreboot.org/c/flashrom/+/62213/comment/8d54b14c_fa792ee1
PS1, Line 39: return acquire_lock(&big_lock, timeout_secs * 1000);
> should we check #if BIG_LOCK and decide if lock is required?
Seems unnecessary complexity to make it conditional since we would always want it to be the case that two flashrom instances never run concurrently?
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/62213/comment/dc7b4f24_14b85557
PS1, Line 51: #ifndef USE_BIG_LOCK
: #define USE_BIG_LOCK 0
: #endif
> Not really used?
Done
https://review.coreboot.org/c/flashrom/+/62213/comment/587389ae_175cce24
PS1, Line 142: programmer_entry
> Is it possible to add an attribute to programmer_entry so we can know if big_lock is required? Then […]
afaik, this is issue is specifically related to futility tests not yet running under platform2_wrapper so that is just a tmp workaround until we fix that. Therefore upstream shouldn't be burden with the issue that is our own. WDYT?
https://review.coreboot.org/c/flashrom/+/62213/comment/7b97253e_de6983ed
PS1, Line 151: /* Get big lock before doing any work that touches hardware. */
: msg_gdbg("Acquiring lock (timeout=%d sec)...\n", LOCK_TIMEOUT_SECS);
: if (acquire_big_lock(LOCK_TIMEOUT_SECS) < 0) {
: msg_gerr("Could not acquire lock.\n");
: return -1;
: }
: big_lock_acquired = true;
: msg_gdbg("Lock acquired.\n");
:
> If we're going to have a big_lock. […]
Sounds good, Done.
https://review.coreboot.org/c/flashrom/+/62213/comment/77b6a8f3_a57b7df2
PS1, Line 217: if (big_lock_acquired) {
: release_big_lock();
: big_lock_acquired = false;
: }
> we should just call release_big_lock() and let it check/handle big_lock_acquired internally
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/62213
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I19cb4e3bf14caeb67c3e8100a20395b264c5113a
Gerrit-Change-Number: 62213
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Comment-Date: Mon, 21 Feb 2022 05:05:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Hung-Te Lin <hungte(a)chromium.org>
Gerrit-MessageType: comment