Attention is currently required from: Nico Huber, David Hendricks, Angel Pons, Arthur Heymans.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57580 )
Change subject: ich_descriptors: Explicitly check for Ibex Peak ISL value
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
File ich_descriptors.c:
https://review.coreboot.org/c/flashrom/+/57580/comment/f921ac86_e0ef3909
PS1, Line 936: == 16
Maybe I am recognising a false pattern here but are the cases of [11, 15] also IBEX_PEAK? ifdtool.c considers the strap length with the case `<= 16` which seems actually what we want here?
--
To view, visit https://review.coreboot.org/c/flashrom/+/57580
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8a880251a393666757f3879629a165b7fe3533ef
Gerrit-Change-Number: 57580
Gerrit-PatchSet: 1
Gerrit-Owner: David Hendricks
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
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: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: David Hendricks
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Mon, 13 Sep 2021 04:28:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Bora Guvendik, Anil Kumar K.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/57560 )
Change subject: chipset_enable.c: Add Ids for recent Intel chipsets
......................................................................
Patch Set 3:
(2 comments)
File chipset_enable.c:
https://review.coreboot.org/c/flashrom/+/57560/comment/391bac75_2117923e
PS3, Line 2018: {0x8086, 0x9d24, B_FS, OK, "Intel", "Skylake", enable_flash_pch100}
This entry will cause a duplicate chipset warning - see https://crrev.com/c/3157025.
https://review.coreboot.org/c/flashrom/+/57560/comment/de7d7055_5941c2ae
PS3, Line 2020: {0x8086, 0x31f0, B_FS, OK, "Intel", "Geminilake", enable_flash_apl},
This entry will cause a duplicate chipset warning - see https://crrev.com/c/3157024.
--
To view, visit https://review.coreboot.org/c/flashrom/+/57560
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Iff387076567e639485ac15c25c1bcc8c37a07037
Gerrit-Change-Number: 57560
Gerrit-PatchSet: 3
Gerrit-Owner: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Selma Bensaid <selma.bensaid(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: AndreX Andraos <andrex.andraos(a)intel.com>
Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.corp-partner.google.com>
Gerrit-CC: Bernardo Perez Priego <bernardo.perez.priego(a)intel.com>
Gerrit-CC: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-CC: Cliff Huang <cliff.huang(a)intel.com>
Gerrit-CC: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com>
Gerrit-Attention: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Comment-Date: Mon, 13 Sep 2021 03:57:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Angel Pons.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56415 )
Change subject: dediprog.c: Drop dediprog_ prefix for spi data struct members
......................................................................
Patch Set 6:
(1 comment)
File dediprog.c:
https://review.coreboot.org/c/flashrom/+/56415/comment/25882c15_3b8d97eb
PS4, Line 1058:
> Looks like one tab too much now, maybe it would even look better w/o […]
Yes, this one fits into one line now. I missed it :( I went through the diffs again to check that nothing else has been missed.
--
To view, visit https://review.coreboot.org/c/flashrom/+/56415
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I688d50926b78a6c3f1c5a8ba4ef88a0d5b495bd0
Gerrit-Change-Number: 56415
Gerrit-PatchSet: 6
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: 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-Comment-Date: Mon, 13 Sep 2021 03:46:40 +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: Angel Pons, Anastasia Klimchuk.
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/56415
to look at the new patch set (#6).
Change subject: dediprog.c: Drop dediprog_ prefix for spi data struct members
......................................................................
dediprog.c: Drop dediprog_ prefix for spi data struct members
The name of the struct type already contains dp_ prefix, so
prefix doesn't need to be repeated in members name.
BUG=b:185191942
TEST=builds and ninja test
Change-Id: I688d50926b78a6c3f1c5a8ba4ef88a0d5b495bd0
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M dediprog.c
1 file changed, 54 insertions(+), 60 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/15/56415/6
--
To view, visit https://review.coreboot.org/c/flashrom/+/56415
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I688d50926b78a6c3f1c5a8ba4ef88a0d5b495bd0
Gerrit-Change-Number: 56415
Gerrit-PatchSet: 6
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Angel Pons.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56414 )
Change subject: dediprog.c: Refactor singleton states into reentrant pattern
......................................................................
Patch Set 5:
(1 comment)
File dediprog.c:
https://review.coreboot.org/c/flashrom/+/56414/comment/f68abb50_bf72d8ab
PS3, Line 221: const int finish,
> I don't think there is a written rule. There are three common ways […]
I made one more patchset, these three functions now look consistent:
dediprog_bulk_read_poll
dediprog_read
dediprog_write
For some reason I thought I can only use tabs, no spaces! With spaces it looks better.
There are 5 other occurrences where a function call (not declaration) gets too long and arguments list needs to be split, which introduces extra line. But all those 5 places are returned back in the next patch, after removing prefix extra line not needed anymore.
--
To view, visit https://review.coreboot.org/c/flashrom/+/56414
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I72085e750af97b94dfa94f2ebf2a134e41a2ec8d
Gerrit-Change-Number: 56414
Gerrit-PatchSet: 5
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: 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-Comment-Date: Mon, 13 Sep 2021 03:26:17 +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>
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons, Anastasia Klimchuk.
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/56415
to look at the new patch set (#5).
Change subject: dediprog.c: Drop dediprog_ prefix for spi data struct members
......................................................................
dediprog.c: Drop dediprog_ prefix for spi data struct members
The name of the struct type already contains dp_ prefix, so
prefix doesn't need to be repeated in members name.
BUG=b:185191942
TEST=builds and ninja test
Change-Id: I688d50926b78a6c3f1c5a8ba4ef88a0d5b495bd0
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M dediprog.c
1 file changed, 54 insertions(+), 59 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/15/56415/5
--
To view, visit https://review.coreboot.org/c/flashrom/+/56415
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I688d50926b78a6c3f1c5a8ba4ef88a0d5b495bd0
Gerrit-Change-Number: 56415
Gerrit-PatchSet: 5
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Angel Pons, Anastasia Klimchuk.
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/56414
to look at the new patch set (#5).
Change subject: dediprog.c: Refactor singleton states into reentrant pattern
......................................................................
dediprog.c: Refactor singleton states into reentrant pattern
Move global singleton states into a struct and store within
the spi_master data field for the life-time of the driver.
This is one of the steps on the way to move spi_master data
memory management behind the initialisation API, for more
context see other patches under the same topic "register_master_api".
BUG=b:185191942
TEST=builds and ninja test
Change-Id: I72085e750af97b94dfa94f2ebf2a134e41a2ec8d
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M dediprog.c
1 file changed, 152 insertions(+), 114 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/14/56414/5
--
To view, visit https://review.coreboot.org/c/flashrom/+/56414
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I72085e750af97b94dfa94f2ebf2a134e41a2ec8d
Gerrit-Change-Number: 56414
Gerrit-PatchSet: 5
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Felix Singer, Nico Huber, Michał Żygowski, Paul Menzel, Angel Pons.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56023 )
Change subject: board_enable.c: Add ME unlock function for TUXEDO laptops
......................................................................
Patch Set 8:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/56023/comment/ce554599_55a0335c
PS8, Line 7: TUXEDO
clevo - all clevo laptops I checked have that unlock capability :-)
--
To view, visit https://review.coreboot.org/c/flashrom/+/56023
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6bf1c40900aa674e3ea4f6add12dae8b73759fbb
Gerrit-Change-Number: 56023
Gerrit-PatchSet: 8
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
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: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Sun, 12 Sep 2021 23:51:12 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Felix Singer, Nico Huber, Michał Żygowski, Paul Menzel, Angel Pons.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56023 )
Change subject: board_enable.c: Add ME unlock function for TUXEDO laptops
......................................................................
Patch Set 8:
(1 comment)
File board_enable.c:
https://review.coreboot.org/c/flashrom/+/56023/comment/ddf9d231_a8cad7a6
PS2, Line 2339: return 0;
I see, that's indeed a very good reason to implement it differently. Well, I had already had asked that above btw :P
> Sorry, I thought one had to apply a command (-r -w -v), too, but that's not the case. For me this is fine. What do others say?
> ...
> I too, would either add this functionality to intelmetool or bail out after setting the unlock bit (handle it like a independent command just like -r, -w, -v).
>
> One argument for having it in flashrom is that unlocking IFD/ME is very related to flashing.
Another argument for having that functionality in flashrom is that users don't need to install another independent tool, that might not yet be available on their distro, while flashrom already is available broadly.
TL;DR what about adding `--unlock-me` as operation, so the command would be `flashrom -p internal --unlock-me` or `... -u`.
--
To view, visit https://review.coreboot.org/c/flashrom/+/56023
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I6bf1c40900aa674e3ea4f6add12dae8b73759fbb
Gerrit-Change-Number: 56023
Gerrit-PatchSet: 8
Gerrit-Owner: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Michael Niewöhner <foss(a)mniewoehner.de>
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: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Sun, 12 Sep 2021 23:49:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/57581
to look at the new patch set (#3).
Change subject: ich_descriptors: Add check for ISL == 139 for LBG
......................................................................
ich_descriptors: Add check for ISL == 139 for LBG
Purley and Whitley BKC images have an ISL (PSL) value of 0x8B (139).
This matches the documented value in the LBG SPI programming guide.
Change-Id: Icb8d19fd6771ed787b488044aa5ed5516812b27c
Signed-off-by: David Hendricks <ddaveh(a)amazon.com>
---
M ich_descriptors.c
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/81/57581/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/57581
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Icb8d19fd6771ed787b488044aa5ed5516812b27c
Gerrit-Change-Number: 57581
Gerrit-PatchSet: 3
Gerrit-Owner: David Hendricks
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset