Attention is currently required from: Nico Huber, Angel Pons.
Hello Felix Singer, build bot (Jenkins), Nico Huber, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/61523
to look at the new patch set (#3).
Change subject: Makefile: check if librt is an external library
......................................................................
Makefile: check if librt is an external library
Some systems, e.g. OpenBSD, have clock_gettime / librt build into the
libc and therefore fail linking against it with -lrt. Thus, detect this
and link only if needed.
Change-Id: I2c1668a350aa0806fccfb4e9cd8b04861f085ee9
Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com>
---
M Makefile
M Makefile.include
2 files changed, 8 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/23/61523/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/61523
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2c1668a350aa0806fccfb4e9cd8b04861f085ee9
Gerrit-Change-Number: 61523
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Angel Pons, Sergii Dmytruk.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58480 )
Change subject: flashchips,writeprotect_ranges: add range decoding function
......................................................................
Patch Set 30:
(1 comment)
File flash.h:
https://review.coreboot.org/c/flashrom/+/58480/comment/83cbc782_b5807fce
PS23, Line 203: const struct flashrom_wp_chip_config *cfg, uint32_t chip_len
> It seems this is exactly the input we need for the `w25` implementation. […]
Having the chip's length is quite convenient for w25 chips since their ranges generalize very nicely with the length, but I don't think it's overspecialized and some other chip families have similar patterns.
We could go with a full `struct flashchip` input, but I'm hesitant to let the functions know too much in case they start accumulating special cases based on random chip details.
We should keep the inputs to the functions data-only, since they should just define config bits -> protection range mappings in a convenient way. If the function takes a flashctx, we can't construct a bidirectional mapping between config bit values and protection ranges.
--
To view, visit https://review.coreboot.org/c/flashrom/+/58480
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id163ed80938a946a502ed116e48e8236e36eb203
Gerrit-Change-Number: 58480
Gerrit-PatchSet: 30
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: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
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: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Tue, 01 Feb 2022 10:03:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, Sergii Dmytruk.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58479 )
Change subject: libflashrom,writeprotect: add functions for reading/writing WP configs
......................................................................
Patch Set 29:
(5 comments)
Patchset:
PS22:
> That sounds good. I'm working on implementing it now.
Done.
I've also moved all libflashrom functions to libflashrom.c to make way for linux_mtd support.
File libflashrom.h:
https://review.coreboot.org/c/flashrom/+/58479/comment/3e0bb11b_3647a7f6
PS27, Line 133: };
> That sounds good to me, it's good to address ABI stability problems early on. […]
I went with the second option for the range list. Resolving as the original comment is done and CB:58481 is a better place to discuss the range list functions.
File libflashrom.h:
https://review.coreboot.org/c/flashrom/+/58479/comment/1f1abd47_8c6370b8
PS22, Line 125: software
> I've removed the comment now, mentioning software protection is confusing/inaccurate since software […]
Resolving.
File writeprotect.c:
https://review.coreboot.org/c/flashrom/+/58479/comment/a0df967a_3f696fbe
PS22, Line 56: struct wp_chip_config *cfg
> Undoing done
Done
https://review.coreboot.org/c/flashrom/+/58479/comment/e34d3c83_ffcfb806
PS22, Line 145: which of two configurations should be used if they both select the same
: * protection range
> This was kind of an adaptation of an earlier function that just compared ranges so they could be sor […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/58479
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3ad25708c3321b8fb0216c3eaf6ffc07616537ad
Gerrit-Change-Number: 58479
Gerrit-PatchSet: 29
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: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Tue, 01 Feb 2022 10:03:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58477 )
Change subject: flashchips: add writeprotect bit layout map to chips
......................................................................
Patch Set 27:
(1 comment)
File flashchips.c:
https://review.coreboot.org/c/flashrom/+/58477/comment/e2ec5feb_a94dda0f
PS20, Line 6353: .tb = {STATUS1, 5, RW}, /* Called BP3 in datasheet, acts like TB */
: .sec = {STATUS1, 6, RW}, /* Called BP4 in datasheet, acts like SEC */
> IMO we should definitely use the naming by the bit's function, like you did. […]
Done in the code, though I'm not sure how I should create an account to edit the wiki.
--
To view, visit https://review.coreboot.org/c/flashrom/+/58477
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id08d77e6d4ca5109c0d698271146d026dbc21284
Gerrit-Change-Number: 58477
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: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
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: Edward O'Callaghan <quasisec(a)chromium.org>
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: Tue, 01 Feb 2022 10:02:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com>
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/+/58570 )
Change subject: spi25_statusreg,flashchips: add SR2 read/write support
......................................................................
Patch Set 24:
(3 comments)
File flashchips.c:
https://review.coreboot.org/c/flashrom/+/58570/comment/f0312092_2cdbb77a
PS18, Line 6711: FEATURE_WRSR_EXT
> > And a comment about the datasheet oddity would be nice. […]
I just added it as a comment at the end of the line, that seems to be fairly standard for flashchips.c.
File spi25_statusreg.c:
https://review.coreboot.org/c/flashrom/+/58570/comment/b2e85df1_4d1f47f0
PS23, Line 94:
> A bright pink space :)
Done
https://review.coreboot.org/c/flashrom/+/58570/comment/9b22900c_7c880664
PS23, Line 116: *
> This can be removed (you have two asterisks).
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/58570
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I34a503b0958e8f2f22a2a993a6ea529eb46b41db
Gerrit-Change-Number: 58570
Gerrit-PatchSet: 24
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: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
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-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Tue, 01 Feb 2022 10:02:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com>
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/+/58475 )
Change subject: spi25_statusreg: make register read/write functions generic
......................................................................
Patch Set 21:
(2 comments)
File spi25_statusreg.c:
https://review.coreboot.org/c/flashrom/+/58475/comment/55d80574_821a45d0
PS20, Line 67: unlikley
> unlikely (typo)
Done
https://review.coreboot.org/c/flashrom/+/58475/comment/a2d76588_764175f1
PS20, Line 74: i -= 10;
> I can't understand this after reading the comment. […]
Oops, the delay should be 100*1000 not 10*1000.
It would probably fine without modifying `i`, it's just so that the total time is always 5 seconds.
--
To view, visit https://review.coreboot.org/c/flashrom/+/58475
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0a3951bbf993f2d8d830143b29d3ce16cc6901d7
Gerrit-Change-Number: 58475
Gerrit-PatchSet: 21
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: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
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-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Tue, 01 Feb 2022 10:02:12 +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, Sergii Dmytruk.
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/58478
to look at the new patch set (#28).
Change subject: writeprotect.h: add structure to represent chip wp configuration bits
......................................................................
writeprotect.h: add structure to represent chip wp configuration bits
Add `struct wp_bits` for representing values of all WP bits in a chip's
status/config register(s).
It allows most WP code to store and manipulate a chip's configuration
without knowing the exact layout of bits in the chip's status registers.
Supporting other chips may require additional fields to be added to the
structure.
BUG=b:195381327,b:153800563
TEST=flashrom --wp-{enable,disable,range,list,status} at end of patch series
BRANCH=none
Change-Id: I17dee630248ce7b51e624a6e46d7097d5d0de809
Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com>
---
M flash.h
M writeprotect.h
2 files changed, 32 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/78/58478/28
--
To view, visit https://review.coreboot.org/c/flashrom/+/58478
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I17dee630248ce7b51e624a6e46d7097d5d0de809
Gerrit-Change-Number: 58478
Gerrit-PatchSet: 28
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: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
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: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Edward O'Callaghan, Nikolai Artemiev, Sergii Dmytruk.
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/+/59528
to look at the new patch set (#25).
Change subject: spi25_statusreg: inline spi_write_register_flag()
......................................................................
spi25_statusreg: inline spi_write_register_flag()
Creating the entire SPI command that should be sent to the chip in
spi_write_register() is simpler than splitting it across two functions
that have to pass multiple parameters between them.
Additionally, having separate spi_write_register_flag() function
provided little benefit, as it was only ever called from
spi_write_register().
BUG=b:195381327,b:153800563
TEST=builds
BRANCH=none
Change-Id: I4996b0848d0ed09032bad2ab13ab1f40bbfc0304
Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com>
---
M spi25_statusreg.c
1 file changed, 56 insertions(+), 70 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/28/59528/25
--
To view, visit https://review.coreboot.org/c/flashrom/+/59528
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4996b0848d0ed09032bad2ab13ab1f40bbfc0304
Gerrit-Change-Number: 59528
Gerrit-PatchSet: 25
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-CC: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-MessageType: newpatchset