Attention is currently required from: Subrata Banik, Nico Huber, Paul Menzel, Angel Pons, Anastasia Klimchuk.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62867 )
Change subject: ichspi: Unify timeouts across all SPI operations to 30s
......................................................................
Patch Set 8:
(1 comment)
Patchset:
PS5:
> I think the reason to use smaller timeout values in some cases is to avoid unnecessary delays.
I don't understand the comment here about *delay*, 30sec is the timeout value for the worst case scenario to avoid the SPI contention issue among multi-master scenario, it's not *forced delay* that would impact every chipsets. You could see more of the operations are still getting over within few ms of timeout.
> 30 seconds is the recommended timeout for the worst-case scenario.
>
> Personally, I don't think using the 30-sec timeout everywhere is beneficial.
Unless increased the timeout value in general, how would we know if it is applicable or not? I guess you are referring about SPI read operation which might not get impacted due to multi-master worst case operation as chipset has *fast-read* feature.
--
To view, visit https://review.coreboot.org/c/flashrom/+/62867
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ifa910dea794175d8ee2ad277549e5a0d69cba45b
Gerrit-Change-Number: 62867
Gerrit-PatchSet: 8
Gerrit-Owner: Subrata Banik <subratabanik(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
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-CC: Subrata Banik <subi.banik(a)gmail.com>
Gerrit-Attention: Subrata Banik <subi.banik(a)gmail.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 04 Apr 2022 04:26:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Daniel Campello.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63193 )
Change subject: tests: use MOCK_FD instead of NON_ZERO
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/63193/comment/093c924a_e1ec6c37
PS3, Line 9: return a non-negative value for
: the file descriptor expected from open operations.
> to fail if there is no `open()` mocked in a test
Oh definitely not intentional.
NON_ZERO was intended to represent a valid (successful) return value. What I think has happened, the value of NON_ZERO was taken too large (not intentionally) and it overflows to negative value.
It is possible those too (NON_ZERO and MOCK_FD) can be converged into one macro, but I would better do it later in a separate patch. Realtek test was using its own mock fd anyway (although I forgot why).
--
To view, visit https://review.coreboot.org/c/flashrom/+/63193
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ib6bac051852aea2465665a6fd669b7f5e3772985
Gerrit-Change-Number: 63193
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: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-Comment-Date: Mon, 04 Apr 2022 03:36:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Daniel Campello <campello(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59071 )
Change subject: flashchips: enable write-protection for W25Q{64,128}.V
......................................................................
Patch Set 24:
(1 comment)
Patchset:
PS24:
Hi Sergii,
Thanks again for extending writeprotect support with these patches! If you have some time, could you please rebase them on the latest patchset of the original WP patch chain? I'm also happy to have a go at it if you're busy right now.
- Nik
--
To view, visit https://review.coreboot.org/c/flashrom/+/59071
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Iccb69a8d3a0dd2192e2c938caddaf07b1889ed35
Gerrit-Change-Number: 59071
Gerrit-PatchSet: 24
Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Mon, 04 Apr 2022 03:14:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62643 )
Change subject: writeprotect.c: refactor and fix wp_mode functions
......................................................................
Patch Set 11:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62643/comment/73b58b21_6c260208
PS10, Line 12: since a chip without SRP is just FLASHROM_WP_MODE_DISABLED
> Just to double-check, chip without SRP/SRL will have […]
Yep :)
--
To view, visit https://review.coreboot.org/c/flashrom/+/62643
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ib6c347453f9216e5816e4ed35bf9783fd3c720e0
Gerrit-Change-Number: 62643
Gerrit-PatchSet: 11
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
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: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 04 Apr 2022 03:10:10 +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: Nico Huber, Angel Pons, Anastasia Klimchuk.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61897 )
Change subject: libflashrom,linux_mtd: add linux_mtd writeprotect support
......................................................................
Patch Set 36:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/61897/comment/6e1a4461_b01e7fab
PS36, Line 10: and uses it support
> Maybe instead "and uses it support" -> "and uses the framework to support".
Done
File linux_mtd.c:
https://review.coreboot.org/c/flashrom/+/61897/comment/8ac26c96_ac518582
PS36, Line 380: if ((cfg->range.len == 0) != (cfg->mode != FLASHROM_WP_MODE_DISABLED)) {
: return FLASHROM_WP_ERR_OTHER;
: }
> Please help me, I spent some time staring at the code, also reading the comment above and commit mes […]
Good catch! I think this was a last minute rewrite from `cfg->mode != FLASHROM_WP_MODE_HARDWARE` where I didn't change the operator.
--
To view, visit https://review.coreboot.org/c/flashrom/+/61897
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I5c86e28cdec44bec49ba1d36f8ab62241b9b01da
Gerrit-Change-Number: 61897
Gerrit-PatchSet: 36
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
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: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 04 Apr 2022 03:08:28 +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: Nico Huber, Angel Pons, Nikolai Artemiev.
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/61897
to look at the new patch set (#38).
Change subject: libflashrom,linux_mtd: add linux_mtd writeprotect support
......................................................................
libflashrom,linux_mtd: add linux_mtd writeprotect support
This commit adds a generic framework to allow opaque programmers to
implement writeprotect operations and uses the framework to support
writeprotect operations on linux MTD device files.
The generic framework comprises three new functions in
`struct opaque_master` that are called from libflashrom:
- wp_write_cfg()
- wp_read_cfg()
- wp_get_ranges()
For linux_mtd, only the read/write functions are implemented. Linux's
MTD interface doesn't provide a way to get available ranges, so calling
get_wp_ranges() on the linux_mtd master will return
FLASHROM_WP_ERR_RANGE_LIST_UNAVAILABLE.
BUG=b:182223106
BRANCH=none
TEST=WP ops on hana DUT (MT8173) with W25Q32DW flash
TEST=flashrom --wp-enable --wp-range <non-empty> succeeds
TEST=flashrom --wp-enable --wp-range <empty> fails as expected
TEST=flashrom --wp-disable --wp-range <empty> succeeds
TEST=flashrom --wp-disable --wp-range <non-empty> fails as expected
TEST=flashrom --wp-status succeeds
Change-Id: I5c86e28cdec44bec49ba1d36f8ab62241b9b01da
Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com>
---
M libflashrom.c
M linux_mtd.c
M programmer.h
3 files changed, 132 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/97/61897/38
--
To view, visit https://review.coreboot.org/c/flashrom/+/61897
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I5c86e28cdec44bec49ba1d36f8ab62241b9b01da
Gerrit-Change-Number: 61897
Gerrit-PatchSet: 38
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
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: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Angel Pons, Nikolai Artemiev.
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/62214
to look at the new patch set (#27).
Change subject: flashchips.c: add writeprotect support for more chips
......................................................................
flashchips.c: add writeprotect support for more chips
Chips I had available for testing were tested with all writeprotect
commands and an FT232H adapter. Chips I wasn't able to test were just
checked against the datasheets.
Chips used for testing (including chips added in previous patches) are
listed in the table below:
Flashrom Chip name | Chip(s) tested
---------------------------------+----------------------------
AT25SL128A |
EN25QH128 |
GD25LQ128C/GD25LQ128D/GD25LQ128E | GD25LQ128DSIG
GD25LQ64(B) | GD25LQ64CWIG
GD25Q127C/GD25Q128C | GD25Q127CSIG, GD25Q128ESIG
GD25Q256D | GD25Q256DYIG
GD25Q64(B) | GD25Q64CSIG
W25Q128.JW.DTR |
W25Q128.V..M |
W25Q128.W |
W25Q256JV_M |
W25Q256.V |
W25Q64.W |
XM25QH128C |
XM25QH256C |
BUG=b:182223106
BRANCH=none
TEST=flashrom --wp-{enable,disable,range,list,status}
Change-Id: I7f3d4c4148056098a845b5c64308b0333ebda395
Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com>
---
M flashchips.c
1 file changed, 138 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/14/62214/27
--
To view, visit https://review.coreboot.org/c/flashrom/+/62214
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7f3d4c4148056098a845b5c64308b0333ebda395
Gerrit-Change-Number: 62214
Gerrit-PatchSet: 27
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
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: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Angel Pons, Nikolai Artemiev.
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/61897
to look at the new patch set (#37).
Change subject: libflashrom,linux_mtd: add linux_mtd writeprotect support
......................................................................
libflashrom,linux_mtd: add linux_mtd writeprotect support
This commit adds a generic framework to allow opaque programmers to
implement writeprotect operations and uses it support writeprotect
operations on linux MTD device files.
The generic framework comprises three new functions in
`struct opaque_master` that are called from libflashrom:
- wp_write_cfg()
- wp_read_cfg()
- wp_get_ranges()
For linux_mtd, only the read/write functions are implemented. Linux's
MTD interface doesn't provide a way to get available ranges, so calling
get_wp_ranges() on the linux_mtd master will return
FLASHROM_WP_ERR_RANGE_LIST_UNAVAILABLE.
BUG=b:182223106
BRANCH=none
TEST=WP ops on hana DUT (MT8173) with W25Q32DW flash
TEST=flashrom --wp-enable --wp-range <non-empty> succeeds
TEST=flashrom --wp-enable --wp-range <empty> fails as expected
TEST=flashrom --wp-disable --wp-range <empty> succeeds
TEST=flashrom --wp-disable --wp-range <non-empty> fails as expected
TEST=flashrom --wp-status succeeds
Change-Id: I5c86e28cdec44bec49ba1d36f8ab62241b9b01da
Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com>
---
M libflashrom.c
M linux_mtd.c
M programmer.h
3 files changed, 132 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/97/61897/37
--
To view, visit https://review.coreboot.org/c/flashrom/+/61897
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I5c86e28cdec44bec49ba1d36f8ab62241b9b01da
Gerrit-Change-Number: 61897
Gerrit-PatchSet: 37
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
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: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-MessageType: newpatchset