Attention is currently required from: Anastasia Klimchuk.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68197 )
Change subject: tests/meson.build: Turn file list into list of file objects
......................................................................
Patch Set 4:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/68197/comment/6f9f3107_f65ee2e4
PS3, Line 12: as it is done elsewhere
> I would say the main reason : as it gives better, more useful error message. […]
Right :) I have updated the commit message. Is it better?
--
To view, visit https://review.coreboot.org/c/flashrom/+/68197
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0b9144a6b76c1772833817b4e6873818dcf36b05
Gerrit-Change-Number: 68197
Gerrit-PatchSet: 4
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 10 Oct 2022 05:13:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer.
Hello build bot (Jenkins), Thomas Heijligen, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/68197
to look at the new patch set (#4).
Change subject: tests/meson.build: Turn file list into list of file objects
......................................................................
tests/meson.build: Turn file list into list of file objects
When a file object is created, Meson also checks if the file actually
exists and an error points to the specific line of meson.build if not.
If just a list of filenames is used, then the error occurs at the line
where the list is used.
Thus, use file objects in tests/meson.build for more useful error
messages.
Signed-off-by: Felix Singer <felixsinger(a)posteo.net>
Change-Id: I0b9144a6b76c1772833817b4e6873818dcf36b05
---
M tests/meson.build
1 file changed, 20 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/97/68197/4
--
To view, visit https://review.coreboot.org/c/flashrom/+/68197
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0b9144a6b76c1772833817b4e6873818dcf36b05
Gerrit-Change-Number: 68197
Gerrit-PatchSet: 4
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: newpatchset
Attention is currently required from: Thomas Heijligen, Angel Pons, Anastasia Klimchuk.
Hello build bot (Jenkins), Thomas Heijligen, Angel Pons, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/68162
to look at the new patch set (#11).
Change subject: meson: Move programmer test sources into programmer definition
......................................................................
meson: Move programmer test sources into programmer definition
Move the definition of the programmer test source files into the
dictionary defining the programmers itself. This way there is a better
overview about which of the available programmers have tests and which
don't.
Also, to keep the tests working, iterate over all programmers and add
their test source files to the list of sources that should be built.
Signed-off-by: Felix Singer <felixsinger(a)posteo.net>
Change-Id: I307faaf8a9f7ae3c54bd96e7d871a3abb8aadea3
---
M meson.build
M tests/meson.build
2 files changed, 34 insertions(+), 9 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/62/68162/11
--
To view, visit https://review.coreboot.org/c/flashrom/+/68162
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I307faaf8a9f7ae3c54bd96e7d871a3abb8aadea3
Gerrit-Change-Number: 68162
Gerrit-PatchSet: 11
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
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-MessageType: newpatchset
Attention is currently required from: Felix Singer, Thomas Heijligen.
Hello build bot (Jenkins), Thomas Heijligen, Angel Pons, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/68228
to look at the new patch set (#3).
Change subject: tests/meson.build: Rename list of source files to `test_srcs`
......................................................................
tests/meson.build: Rename list of source files to `test_srcs`
Rename the list of source files to `test_srcs` so that there is less
confusion with the variable `srcs` from the top-level meson.build file
containing the flashrom source files.
Signed-off-by: Felix Singer <felixsinger(a)posteo.net>
Change-Id: Ica0fc3923070bff63323204bd58edb5276dc9493
---
M tests/meson.build
1 file changed, 18 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/28/68228/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/68228
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ica0fc3923070bff63323204bd58edb5276dc9493
Gerrit-Change-Number: 68228
Gerrit-PatchSet: 3
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Angel Pons.
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68068 )
Change subject: [WIP] test_build.sh: Rework programmer selection using Meson
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
> Is that file not in a standard format? I wouldn't know, is it python?
No, it's not Python. The Meson syntax is just similar to it in some ways. See https://mesonbuild.com/Syntax.html
> Maybe the problem is that we are trying to write a build system on top of build systems.
Yeah, I think the same. Inventing a parser seems too much.
> The repeated builds are easy enough to write in make, how is it in meson? If it's not possible, maybe that's something we should teach it?
As far as I know, the Meson tool only has a pre-defined set of commands and they don't allow just printing the possible values for an option. `meson configure <builddir>` gives an overview about options and values, but that's not parseable at all.
However, I might have found a nice solution. Meson allows you to spawn a temporary shell environment, called dev environment, which has additional shell variables for debugging for example.
So, I have created CB:68248 which creates such a dev environment in the top-level meosn.build and the list of programmers is added to `FLASHROM_ALL_PROGRAMMERS`, a shell variable. The dev environment can be entered with `meson devenv -C <build_dir>` but you also can execute commands directly if you add them at the end, which is what I have done to read the content of `FLASHROM_ALL_PROGRAMMERS`.
I think this is the best and easiest solution which we can get. I would keep the discussion here so that it is not scattered.
--
To view, visit https://review.coreboot.org/c/flashrom/+/68068
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I9f41ce2ff219c70c3c05a90134291b01a084c859
Gerrit-Change-Number: 68068
Gerrit-PatchSet: 3
Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Mon, 10 Oct 2022 04:44:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Thomas Heijligen, Anastasia Klimchuk, Alexander Goncharov.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66373 )
Change subject: tree: provide flashrom context into programmer_delay()
......................................................................
Patch Set 9: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/66373
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ibb0bce26ce2052853ee52158d7ba742967a9e229
Gerrit-Change-Number: 66373
Gerrit-PatchSet: 9
Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Mon, 10 Oct 2022 04:42:28 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/68250 )
Change subject: flashrom.c: Drop programmer global singleton handle [WIP]
......................................................................
flashrom.c: Drop programmer global singleton handle [WIP]
What is remaining:
[ ] - delay framework patches still need to land.
[?] - do something about programmer name printing?
Change-Id: I23061ef2f965cf59d74fc3f29952c09117ef6c4f
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M flashrom.c
1 file changed, 17 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/50/68250/1
diff --git a/flashrom.c b/flashrom.c
index 7996e6b..695c3f0 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -37,8 +37,6 @@
const char flashrom_version[] = FLASHROM_VERSION;
const char *chip_to_probe = NULL;
-static const struct programmer_entry *programmer = NULL;
-
/*
* Programmers supporting multiple buses can have differing size limits on
* each bus. Store the limits for each bus in a common struct.
@@ -133,7 +131,6 @@
msg_perr("Invalid programmer specified!\n");
return -1;
}
- programmer = prog;
/* Initialize all programmer specific data. */
/* Default to unlimited decode sizes. */
max_rom_decode = (const struct decode_sizes) {
@@ -863,8 +860,11 @@
if (master_uses_physmap(mst))
msg_cinfo("mapped at physical address 0x%0*" PRIxPTR ".\n",
PRIxPTR_WIDTH, flash->physical_memory);
+#if 0
+ /* TODO(quasisec): No good ideas, maybe just delete the else branch??? */
else
msg_cinfo("on %s.\n", programmer->name);
+#endif
/* Flash registers may more likely not be mapped if the chip was forced.
* Lock info may be stored in registers, so avoid lock info printing. */
--
To view, visit https://review.coreboot.org/c/flashrom/+/68250
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I23061ef2f965cf59d74fc3f29952c09117ef6c4f
Gerrit-Change-Number: 68250
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: newchange
Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/68249 )
Change subject: flashrom.c: Use programmer_init() func params over global
......................................................................
flashrom.c: Use programmer_init() func params over global
The 'programmer' intended to be used in the control flow
of 'programmer_init()' is a parameter to the function. Therefore
use that symbol directly over the global copy of it.
Change-Id: I71e61f0633bac2fc472971249910bf3bf57cd0eb
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M flashrom.c
1 file changed, 16 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/49/68249/1
diff --git a/flashrom.c b/flashrom.c
index e0fcf6e..7996e6b 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -161,8 +161,8 @@
cfg.params = NULL;
}
- msg_pdbg("Initializing %s programmer\n", programmer->name);
- ret = programmer->init(&cfg);
+ msg_pdbg("Initializing %s programmer\n", prog->name);
+ ret = prog->init(&cfg);
if (cfg.params && strlen(cfg.params)) {
if (ret != 0) {
/* It is quite possible that any unhandled programmer parameter would have been valid,
--
To view, visit https://review.coreboot.org/c/flashrom/+/68249
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I71e61f0633bac2fc472971249910bf3bf57cd0eb
Gerrit-Change-Number: 68249
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: newchange
Attention is currently required from: Felix Singer, Thomas Heijligen, Anastasia Klimchuk.
Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/67476
to look at the new patch set (#2).
Change subject: flashrom.c: Embed is_internal state within the flashctx
......................................................................
flashrom.c: Embed is_internal state within the flashctx
This avoids a fragil stack pointer check and another reliance
upon the 'programmer' handle as a global state.
Change-Id: I25717afc3a8f61ca9f4dfb4bb60d6cb048a2ad37
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M cli_classic.c
M flashrom.c
M include/flash.h
M include/programmer.h
M internal.c
5 files changed, 23 insertions(+), 15 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/76/67476/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/67476
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I25717afc3a8f61ca9f4dfb4bb60d6cb048a2ad37
Gerrit-Change-Number: 67476
Gerrit-PatchSet: 2
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset