Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, 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/+/58479
to look at the new patch set (#29).
Change subject: libflashrom,writeprotect: add functions for reading/writing WP configs
......................................................................
libflashrom,writeprotect: add functions for reading/writing WP configs
New functions are exposed through the libflashrom API for
reading/writing chip WP configuartion: `flashrom_wp_{read,write}_cfg()`.
The input/output of these functions is a an opaque `struct
flashrom_wp_cfg` instance, which includes the flash protection range and
status register protection mode.
This commit also adds `{read,write}_wp_bits()` helper functions that
read/write chip-specific WP configuration bits.
BUG=b:195381327,b:153800563
TEST=flashrom --wp-{enable,disable,range,list,status} at end of patch series
BRANCH=none
Change-Id: I3ad25708c3321b8fb0216c3eaf6ffc07616537ad
Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com>
---
M libflashrom.c
M libflashrom.h
M writeprotect.c
M writeprotect.h
4 files changed, 352 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/58479/29
--
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-MessageType: newpatchset
Attention is currently required from: Nico Huber, Angel Pons, Nikolai Artemiev, Sergii Dmytruk.
Anastasia Klimchuk 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 20: Code-Review+1
(2 comments)
File spi25_statusreg.c:
https://review.coreboot.org/c/flashrom/+/58475/comment/dace9460_ac724415
PS20, Line 67: unlikley
unlikely (typo)
https://review.coreboot.org/c/flashrom/+/58475/comment/fda637b6_b264ffbc
PS20, Line 74: i -= 10;
I can't understand this after reading the comment. The comment says only wait for initial 100ms if the register is SR1. At line 74 initial 100ms already done. Why does `i` value matter at all? The loop is expected to have only one iteration for SR1?
So just to be clear, my confusion is: why `i` value is modified, from reading the comment it seems like it doesn't matter in this case?
--
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: 20
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: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Tue, 01 Feb 2022 06:42:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Angel Pons, Nikolai Artemiev.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58737 )
Change subject: ichspi: Extract initialisation of swseq and hwseq into a function
......................................................................
Patch Set 9:
(1 comment)
Patchset:
PS9:
Rebased because of the latest changes in CB:58735
--
To view, visit https://review.coreboot.org/c/flashrom/+/58737
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7d62b1b380e497b82dcae1284d752204cc541bd3
Gerrit-Change-Number: 58737
Gerrit-PatchSet: 9
Gerrit-Owner: 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: Nikolai Artemiev <nartemiev(a)google.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-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Tue, 01 Feb 2022 06:21:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Nicholas Chin.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58735 )
Change subject: ichspi: Split very long init function into two
......................................................................
Patch Set 8:
(3 comments)
Patchset:
PS7:
> Thank you so much Nicolas for help!
Nicolas, sorry for delay, I uploaded new version which (hopefully) fixes ich_spibar and ich_generation initialisation. Is it possible for you to try run it again on your machine? Thank you so much!
PS7:
> My memory tries to tell me something... back in 2012 we were […]
Wow this is a very special computer that you found! ;) Does it still work?
10 years ago... this is impressive! Maybe you can keep it as a memory even if it doesn't work anymore :)
If it still works... maybe you can keep it to test things sometimes?
File ichspi.c:
https://review.coreboot.org/c/flashrom/+/58735/comment/0a290381_292b00e5
PS7, Line 1771: ich_generation = ich_gen;
: ich_spibar = spibar;
> These two are globals and also affect the ICH7 paths. I guess we […]
Thank you so much for your analysis! This is helpful.
ich_spibar is used in program_opcodes ichspi.c#L566, which is called at some point of init (as I understand?), and ich_generation is needed for run_opcode ichspi.c#L1081 ? I missed this :( global state makes it hard to understand what is used where, I am hoping to remove global state one day.
I moved initialisation of both ich_spibar and ich_generation back into entry point ich_init_spi. This last patchset I tested on the device that I have, and now of course it needs to be tested on ICH7.
The question that I have in mind: does the code looks good enough to re-test it again? At the moment I have no way to run init_ich7_spi code, so I only know that the code builds.
--
To view, visit https://review.coreboot.org/c/flashrom/+/58735
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6789bc456a4878e6555831ae0b80ecbdbf62938b
Gerrit-Change-Number: 58735
Gerrit-PatchSet: 8
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
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: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
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: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-Comment-Date: Tue, 01 Feb 2022 06:19:58 +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: Nicholas Chin <nic.c3.14(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev.
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Nikolai Artemiev,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/58737
to look at the new patch set (#9).
Change subject: ichspi: Extract initialisation of swseq and hwseq into a function
......................................................................
ichspi: Extract initialisation of swseq and hwseq into a function
Initialisation of swseq_data and hwseq_data gets its own function,
which is called from init_ich_default. This makes init_ich_default
more readable.
This patch also gives a name to (previously anonymous) struct
swseq_data. Its sibling struct hwseq_data already has a name. Structs
need names to be able to declare function parameters.
BUG=b:204488958
TEST=Check that the following scenarios still behave properly:
1) probe-read-verify-erase section-write-reboot
on Intel octopus board with GD25LQ128C/GD25LQ128D/GD25LQ128E
2) probe and read on Panther Point (7 series PCH)
Change-Id: I7d62b1b380e497b82dcae1284d752204cc541bd3
Tested-by: Anastasia Klimchuk <aklm(a)chromium.org>
Tested-by: Nico Huber <nico.h(a)gmx.de>
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M ichspi.c
1 file changed, 57 insertions(+), 50 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/37/58737/9
--
To view, visit https://review.coreboot.org/c/flashrom/+/58737
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7d62b1b380e497b82dcae1284d752204cc541bd3
Gerrit-Change-Number: 58737
Gerrit-PatchSet: 9
Gerrit-Owner: 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: Nikolai Artemiev <nartemiev(a)google.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-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-MessageType: newpatchset