Attention is currently required from: Dmitry Zhadinets, Peter Marheine.
Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/87341?usp=email )
Change subject: libflashrom: Add probing v2 which can find all mathching chips
......................................................................
Patch Set 4:
(1 comment)
File tests/lifecycle.c:
https://review.coreboot.org/c/flashrom/+/87341/comment/350ea199_f1a0b446?us… :
PS1, Line 27: assert_int_equal
> For v2 tests, I want, in addition to return value, also assert which chip is in flashctx, how many m […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/87341?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Idfcf377a8071e22028ba98515f08495ed2a6e9f0
Gerrit-Change-Number: 87341
Gerrit-PatchSet: 4
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Dmitry Zhadinets <dzhadinets(a)gmail.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Fri, 25 Apr 2025 10:25:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Dmitry Zhadinets, Peter Marheine.
Hello Dmitry Zhadinets, Peter Marheine, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/87341?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: libflashrom: Add probing v2 which can find all mathching chips
......................................................................
libflashrom: Add probing v2 which can find all mathching chips
Probing v2 can (if requested) go through all flashchips and find
all the matching chip definitions. This is the way cli behaves,
so cli becomes a client of probing v2.
Previously cli and libflashrom had different probing logic, and
different code in different source files.
This patch also adds tests for probing v2.
Testing from the cli:
./flashrom -p dummy:emulate=W25Q128FV -r dump.rom
./flashrom -p dummy:emulate=MX25L6436 -r dump.rom
./flashrom -p dummy:emulate=MX25L6436 -c "MX25L6405" -r dump.rom
./flashrom -p dummy:emulate=SST25VF032B -E
./flashrom -p dummy:emulate=S25FL128L -r dump.rom
./flashrom -p dummy:emulate=INVALID -r dump.rom
./flashrom -p dummy:emulate=MX25L6436 -c "NONEXISTENT" -r dump.rom
Change-Id: Idfcf377a8071e22028ba98515f08495ed2a6e9f0
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M cli_classic.c
M include/libflashrom.h
M libflashrom.c
M libflashrom.map
M tests/dummyflasher.c
M tests/lifecycle.c
M tests/lifecycle.h
M tests/tests.c
M tests/tests.h
9 files changed, 204 insertions(+), 24 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/41/87341/4
--
To view, visit https://review.coreboot.org/c/flashrom/+/87341?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Idfcf377a8071e22028ba98515f08495ed2a6e9f0
Gerrit-Change-Number: 87341
Gerrit-PatchSet: 4
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Dmitry Zhadinets <dzhadinets(a)gmail.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Attention is currently required from: Dmitry Zhadinets, Peter Marheine.
Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/87341?usp=email )
Change subject: libflashrom: Add probing v2 which can find all mathching chips
......................................................................
Patch Set 3:
(3 comments)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/87341/comment/9a1a1cc4_18e6efac?us… :
PS1, Line 1050: struct flashctx flashes[8] = {{0}};
> This array won't be needed after this patch, in the next commit it can be replaced with just one fla […]
To elaborate what I was thinking about: I wanted to have some first reviews so that we decide that we go ahead with this. And then I will create separate commit to refactor the array and replace the array with one working context (because realistically only one context is used).
https://review.coreboot.org/c/flashrom/+/87341/comment/d0682f7d_cc7e28ad?us… :
PS1, Line 1215: flashrom_flash_probe_v2
> Check the return value
Done
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/87341/comment/3fa99173_f50cdc17?us… :
PS3, Line 1216: if (ret == 1) {
A comment here is that: probing never returns 1, and v1 probing never returned 1 either. The root cause is that probe_flash returns -1 for both the case when no match found and when any error happens during probing :(
I added this ret code handling because I think that it's reasonable for libflashrom client to check ret code.
The situation that libflashrom never returns 1 despite of API documentation is more of internal issue, maybe we will fix it later. Meanwhile clients can check ret code according to documentation.
I am also interested what do people think about it.
--
To view, visit https://review.coreboot.org/c/flashrom/+/87341?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Idfcf377a8071e22028ba98515f08495ed2a6e9f0
Gerrit-Change-Number: 87341
Gerrit-PatchSet: 3
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Dmitry Zhadinets <dzhadinets(a)gmail.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Fri, 25 Apr 2025 09:43:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Dmitry Zhadinets, Peter Marheine.
Hello Dmitry Zhadinets, Peter Marheine, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/87341?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: libflashrom: Add probing v2 which can find all mathching chips
......................................................................
libflashrom: Add probing v2 which can find all mathching chips
Probing v2 can (if requested) go through all flashchips and find
all the matching chip definitions. This is the way cli behaves,
so cli becomes a client of probing v2.
Previously cli and libflashrom had different probing logic, and
different code in different source files.
This patch also adds tests for probing v2.
Testing from the cli:
./flashrom -p dummy:emulate=W25Q128FV -r dump.rom
./flashrom -p dummy:emulate=MX25L6436 -r dump.rom
./flashrom -p dummy:emulate=MX25L6436 -c "MX25L6405" -r dump.rom
./flashrom -p dummy:emulate=SST25VF032B -E
./flashrom -p dummy:emulate=S25FL128L -r dump.rom
./flashrom -p dummy:emulate=INVALID -r dump.rom
./flashrom -p dummy:emulate=MX25L6436 -c "NONEXISTENT" -r dump.rom
Change-Id: Idfcf377a8071e22028ba98515f08495ed2a6e9f0
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M cli_classic.c
M include/libflashrom.h
M libflashrom.c
M libflashrom.map
M tests/dummyflasher.c
M tests/lifecycle.c
M tests/lifecycle.h
M tests/tests.c
M tests/tests.h
9 files changed, 197 insertions(+), 24 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/41/87341/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/87341?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Idfcf377a8071e22028ba98515f08495ed2a6e9f0
Gerrit-Change-Number: 87341
Gerrit-PatchSet: 3
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Dmitry Zhadinets <dzhadinets(a)gmail.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Attention is currently required from: Anastasia Klimchuk, Edward O'Callaghan, Sam McNally, Thomas Heijligen.
Hsuan-ting Chen has uploaded a new patch set (#9) to the change originally created by Edward O'Callaghan. ( https://review.coreboot.org/c/flashrom/+/73286?usp=email )
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: libflashrom: Provide flashrom_flash_getinfo() API
......................................................................
libflashrom: Provide flashrom_flash_getinfo() API
The libflashom API exposes a API to get the flash size however
not the flash name. Provide a API entry that instead provides
a complete flashrom_flashchip_info structure with the detected
flashchip for a given flashctx.
Copy the tested.wp status as well.
BUG=b:276981092
TEST=tested with cli_classic.c patch to show --flash-name using API.
Change-Id: I3dc32e261e6d54230d93f06b91f723811792112a
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
Signed-off-by: Hsuan Ting Chen <roccochen(a)google.com>
---
M include/libflashrom.h
M libflashrom.c
M libflashrom.map
3 files changed, 35 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/86/73286/9
--
To view, visit https://review.coreboot.org/c/flashrom/+/73286?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I3dc32e261e6d54230d93f06b91f723811792112a
Gerrit-Change-Number: 73286
Gerrit-PatchSet: 9
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
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>
Attention is currently required from: Anastasia Klimchuk, Edward O'Callaghan, Sam McNally, Thomas Heijligen.
Hsuan-ting Chen has posted comments on this change by Edward O'Callaghan. ( https://review.coreboot.org/c/flashrom/+/73286?usp=email )
Change subject: libflashrom: Provide flashrom_flash_getinfo() API
......................................................................
Patch Set 8:
(1 comment)
File libflashrom.c:
https://review.coreboot.org/c/flashrom/+/73286/comment/38d38bd7_4a86d88d?us… :
PS7, Line 258: info->vendor = flashctx->chip->vendor;
> +1
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/73286?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I3dc32e261e6d54230d93f06b91f723811792112a
Gerrit-Change-Number: 73286
Gerrit-PatchSet: 8
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
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, 22 Apr 2025 13:16:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Sam McNally <sammc(a)google.com>
Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com>
Attention is currently required from: Edward O'Callaghan, Hsuan-ting Chen, Sam McNally, Thomas Heijligen.
Hsuan-ting Chen has uploaded a new patch set (#8) to the change originally created by Edward O'Callaghan. ( https://review.coreboot.org/c/flashrom/+/73286?usp=email )
Change subject: libflashrom: Provide flashrom_flash_getinfo() API
......................................................................
libflashrom: Provide flashrom_flash_getinfo() API
The libflashom API exposes a API to get the flash size however
not the flash name. Provide a API entry that instead provides
a complete flashrom_flashchip_info structure with the detected
flashchip for a given flashctx.
BUG=b:276981092
TEST=tested with cli_classic.c patch to show --flash-name using API.
Change-Id: I3dc32e261e6d54230d93f06b91f723811792112a
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
Signed-off-by: Hsuan Ting Chen <roccochen(a)google.com>
---
M include/libflashrom.h
M libflashrom.c
M libflashrom.map
3 files changed, 34 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/86/73286/8
--
To view, visit https://review.coreboot.org/c/flashrom/+/73286?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I3dc32e261e6d54230d93f06b91f723811792112a
Gerrit-Change-Number: 73286
Gerrit-PatchSet: 8
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Sam McNally <sammc(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Sam McNally <sammc(a)google.com>
Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Attention is currently required from: Dmitry Zhadinets, Peter Marheine.
Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/87341?usp=email )
Change subject: libflashrom: Add probing v2 which can find all mathching chips
......................................................................
Patch Set 2:
(1 comment)
File tests/lifecycle.c:
https://review.coreboot.org/c/flashrom/+/87341/comment/7853522e_ffe0c793?us… :
PS1, Line 27: flashrom_flash_probe_v2
> Tests needs to be added (not replaced), leave existing probing tests and add new for v2.
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/87341?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Idfcf377a8071e22028ba98515f08495ed2a6e9f0
Gerrit-Change-Number: 87341
Gerrit-PatchSet: 2
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Dmitry Zhadinets <dzhadinets(a)gmail.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Mon, 21 Apr 2025 13:57:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Dmitry Zhadinets, Peter Marheine.
Hello Dmitry Zhadinets, Peter Marheine, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/87341?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: libflashrom: Add probing v2 which can find all mathching chips
......................................................................
libflashrom: Add probing v2 which can find all mathching chips
Probing v2 can (if requested) go through all flashchips and find
all the matching chip definitions. This is the way cli behaves,
so cli becomes a client of probing v2.
Previously cli and libflashrom had different probing logic, and
different code in different source files.
This patch also adds tests for probing v2.
Testing from the cli:
./flashrom -p dummy:emulate=W25Q128FV -r dump.rom
./flashrom -p dummy:emulate=MX25L6436 -r dump.rom
./flashrom -p dummy:emulate=MX25L6436 -c "MX25L6405" -r dump.rom
./flashrom -p dummy:emulate=SST25VF032B -E
./flashrom -p dummy:emulate=S25FL128L -r dump.rom
Change-Id: Idfcf377a8071e22028ba98515f08495ed2a6e9f0
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
---
M cli_classic.c
M include/libflashrom.h
M libflashrom.c
M libflashrom.map
M tests/dummyflasher.c
M tests/lifecycle.c
M tests/lifecycle.h
M tests/tests.c
M tests/tests.h
9 files changed, 191 insertions(+), 25 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/41/87341/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/87341?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Idfcf377a8071e22028ba98515f08495ed2a6e9f0
Gerrit-Change-Number: 87341
Gerrit-PatchSet: 2
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Dmitry Zhadinets <dzhadinets(a)gmail.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Attention is currently required from: Dmitry Zhadinets, Peter Marheine.
Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/87341?usp=email )
Change subject: libflashrom: Add probing v2 which can find all mathching chips
......................................................................
Patch Set 1:
(1 comment)
File tests/lifecycle.c:
https://review.coreboot.org/c/flashrom/+/87341/comment/c9f2fd1f_160c910e?us… :
PS1, Line 27: assert_int_equal
For v2 tests, I want, in addition to return value, also assert which chip is in flashctx, how many matches and names of all matches
--
To view, visit https://review.coreboot.org/c/flashrom/+/87341?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Idfcf377a8071e22028ba98515f08495ed2a6e9f0
Gerrit-Change-Number: 87341
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Dmitry Zhadinets <dzhadinets(a)gmail.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Wed, 16 Apr 2025 13:25:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No