Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk.
Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66343 )
Change subject: cli_classic: refactor arguments parsing into separate func
......................................................................
Patch Set 8:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/66343/comment/5ac0df55_582bf3eb
PS7, Line 21: TEST=builds
> I would be good to test the patch. Dummyflasher is perfectly fine for the purpose. […]
Done
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/66343/comment/6b308a35_e86afa63
PS7, Line 51: unsigned int wp_start, wp_len;
> I think, grouping by purpose would be better than grouping by data type. […]
I see your point. Current patch stores them in the next layout:
```
operations
--
programmer + param
--
layout
--
wp
--
others
```
--
To view, visit https://review.coreboot.org/c/flashrom/+/66343
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id573bc74e3bb46b7dc42f1452fff6394d4f091bc
Gerrit-Change-Number: 66343
Gerrit-PatchSet: 8
Gerrit-Owner: 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: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Tue, 14 Mar 2023 12:51:58 +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, Thomas Heijligen, Edward O'Callaghan, Alexander Goncharov.
Hello build bot (Jenkins), Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/66343
to look at the new patch set (#8).
Change subject: cli_classic: refactor arguments parsing into separate func
......................................................................
cli_classic: refactor arguments parsing into separate func
Move variables that represent parsed options to `cli_options`
structure. This patchset also introduces the following functions:
- parse_options() which parses command line arguments and fills
the structure.
- free_options() that releases an allocated data from the
structure.
This is one of the steps on the way to simplify the main function
by using helper functions with descriptive names.
TOPIC=split_main_func
TEST=the following scenarious run successfully
./flashrom -p dummy:emulate=S25FL128L -V
./flashrom -p dummy:size=8388608,emulate=VARIABLE_SIZE \
-r /tmp/dump.rom
./flashrom -p dummy:emulate=W25Q128FV -l /tmp/rom.layout \
-i boot -w /tmp/rom.tr.img
./flashrom -p dummy:emulate=W25Q128FV --wp-list
./flashrom -p dummy:emulate=W25Q128FV,hwwp=yes \
--wp-enable \
--wp-range=0x00c00000,0x00400000 \
--wp-status
$ head -c 16MiB </dev/urandom >/tmp/image.rom
./flashrom -p dummy:image=/tmp/image.rom,emulate=S25FL128L \
-c S25FL128L -E -V
Change-Id: Id573bc74e3bb46b7dc42f1452fff6394d4f091bc
Signed-off-by: Alexander Goncharov <chat(a)joursoir.net>
---
M cli_classic.c
1 file changed, 406 insertions(+), 343 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/66343/8
--
To view, visit https://review.coreboot.org/c/flashrom/+/66343
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id573bc74e3bb46b7dc42f1452fff6394d4f091bc
Gerrit-Change-Number: 66343
Gerrit-PatchSet: 8
Gerrit-Owner: 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: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Nikolai Artemiev.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73697 )
Change subject: MAINTAINERS: Add Nikolai Artemiev for raiden_debug_spi
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/73697
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie70eafb301b3363caa8e670b67a16cd25c7e15a8
Gerrit-Change-Number: 73697
Gerrit-PatchSet: 4
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
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: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Tue, 14 Mar 2023 11:58:29 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Thomas Heijligen has submitted this change. ( https://review.coreboot.org/c/flashrom/+/73571 )
Change subject: fmap: ignore areas with zero size
......................................................................
fmap: ignore areas with zero size
It's impossible for flashrom_layout to represent zero-sized flash
regions but it is possible for a fmap to contain a zero-sized region
which causes the resulting layout to fail layout_sanity_checks(),
preventing use of that fmap.
Because it would very rarely make sense to be able to operate on
zero-sized regions anyway and changing layouts to be able to support
zero-size regions would entail large changes, instead ignore zero-size
regions when present in fmap.
TEST=Warning is now printed when using fmap that contains a zero-sized
area, and operations on other regions are allowed to proceed.
BUG=b:271933192
Change-Id: Ie20971f779acece7a0b3b8f38796fff128ce689a
Signed-off-by: Peter Marheine <pmarheine(a)chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/73571
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Thomas Heijligen <src(a)posteo.de>
Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org>
---
M libflashrom.c
1 file changed, 38 insertions(+), 0 deletions(-)
Approvals:
build bot (Jenkins): Verified
Thomas Heijligen: Looks good to me, approved
Edward O'Callaghan: Looks good to me, approved
diff --git a/libflashrom.c b/libflashrom.c
index 2e89fe5..4a59e2a 100644
--- a/libflashrom.c
+++ b/libflashrom.c
@@ -362,6 +362,16 @@
return 1;
for (i = 0, area = fmap->areas; i < fmap->nareas; i++, area++) {
+ if (area->size == 0) {
+ /* Layout regions use inclusive upper and lower bounds,
+ * so it's impossible to represent a region with zero
+ * size although it's allowed in fmap. */
+ msg_gwarn("Ignoring zero-size fmap region \"%s\";"
+ " empty regions are unsupported.\n",
+ area->name);
+ continue;
+ }
+
snprintf(name, sizeof(name), "%s", area->name);
if (flashrom_layout_add_region(l, area->offset, area->offset + area->size - 1, name)) {
flashrom_layout_release(l);
--
To view, visit https://review.coreboot.org/c/flashrom/+/73571
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie20971f779acece7a0b3b8f38796fff128ce689a
Gerrit-Change-Number: 73571
Gerrit-PatchSet: 2
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged
Attention is currently required from: Anastasia Klimchuk, Peter Marheine, Alexander Goncharov.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73649 )
Change subject: tests: Add wrap for __fstat50 to fix tests for NetBSD
......................................................................
Patch Set 1: Verified+1 Code-Review+2
(1 comment)
Patchset:
PS1:
Tested on NetBSD 9.2 amd64
--
To view, visit https://review.coreboot.org/c/flashrom/+/73649
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Icb8e453328cb40ab9d628f01ecdc3886a233dad5
Gerrit-Change-Number: 73649
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Peter Marheine <pmarheine(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-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Tue, 14 Mar 2023 10:31:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73570 )
Change subject: pcidev: remove pcidev_getdevfn() function
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
> PCI access in flashrom is a big mess. […]
Thanks Edward! I didn't cached this. The revert is at CB:73730.
I think the OOP stuff is not necessary. `pacc` is overwritten to make the `rpci_` functions working. When removing the `rpci_` functions and using proper shutdown functions there is no need to have `pacc` globally accessible any longer.
--
To view, visit https://review.coreboot.org/c/flashrom/+/73570
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id2983420080f75ae6992edfb032bf5c83b29c803
Gerrit-Change-Number: 73570
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Tue, 14 Mar 2023 10:22:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment