Attention is currently required from: Edward O'Callaghan, Felix Singer, Nico Huber, Angel Pons, Anastasia Klimchuk.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60055 )
Change subject: SFDP: drop static table length check to make newer SFDP revisions work
......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/60055/comment/6d591bd3_6b636547
PS1, Line 28: ***RFC*** We could maintain a list of revisions/lengths but is it really
: worth it?
:
> What is the price of a false-positive (length is wrong, but execution continues)?
Good question. I think there are three cases:
1) read error -> execution will continue just fine
2) defective chip
3) manufacturing/design error
In any case a wrong value could also happen later during read of the table values and we wouldn't even know bc there are no checksums/parity.
File sfdp.c:
https://review.coreboot.org/c/flashrom/+/60055/comment/f4182ef6_ba500326
PS2, Line 372: } else if (sfdp_fill_flash(flash->chip, tbuf, len) == 0)
> Oh so length of table was checked twice?
yes
--
To view, visit https://review.coreboot.org/c/flashrom/+/60055
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id84cde4ebc805d68e2984e8041fbc48d7ceebe34
Gerrit-Change-Number: 60055
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <edward.ocallaghan(a)koparo.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-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <edward.ocallaghan(a)koparo.com>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
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, 10 Jan 2022 07:47:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Nikolai Artemiev, Sergii Dmytruk.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60230 )
Change subject: spi25_statusreg.c: add SR3 read/write support
......................................................................
Patch Set 2:
(1 comment)
File flash.h:
https://review.coreboot.org/c/flashrom/+/60230/comment/f3903249_49a63863
PS2, Line 148: FEATURE_SR3
Maybe it's silly question, but why the name is different from the others above?
For SR2 there are two flags (for write and read as I understand?) and for SR3 only one flag?
--
To view, visit https://review.coreboot.org/c/flashrom/+/60230
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id987c544c02da2b956e6ad2c525265cac8f15be1
Gerrit-Change-Number: 60230
Gerrit-PatchSet: 2
Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(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: 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: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Mon, 10 Jan 2022 03:29:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Edward O'Callaghan, Nikolai Artemiev, Sergii Dmytruk.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58479 )
Change subject: libflashrom,writeprotect: add flashrom_wp_{read,write}_chip_config()
......................................................................
Patch Set 23:
(1 comment)
File writeprotect.c:
https://review.coreboot.org/c/flashrom/+/58479/comment/000717cd_430ae0a4
PS22, Line 122: INVALID_REG
> I've changed it to (INVALID_REG + 1), perhaps STATUS1 would be better though. […]
IMO it is a bit confusing when loop starts with "invalid" value. And even when it's "invalid" + 1 , it takes time to understand why this is fine. I would vote for STATUS1, then it reads naturally like "start with 1 and loop until max value".
--
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: 23
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: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Mon, 10 Jan 2022 03:15:05 +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, Angel Pons, Anastasia Klimchuk.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59238 )
Change subject: tests: Add comprehensive comment for chip.c
......................................................................
Patch Set 4:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/59238/comment/3067252b_6f365ac2
PS4, Line 8:
```
The following describes the two mechanisms of testing done for flash chip operations.
BUG=?
TEST=none
```
File tests/chip.c:
https://review.coreboot.org/c/flashrom/+/59238/comment/ba543e61_d3f3d3d2
PS4, Line 30: Importantly
: * there are two types of tests here, corresponding to two ways of
: * emulating a chip:
This is all very difficult and verbose sentence to parse. I think you are really only just saying the following short sharp points.
```
Two flash chip test variants are used:
i ) Mock chip state backed by `g_chip_state` and,
ii) Mock chip operations backed by `dummyflasher` emulation.
```
It is odd to have this comment juxtaposition next to to `g_chip_state` as only part (i) is relevant to it. A better position would be after the license as a preamble to the files content.
--
To view, visit https://review.coreboot.org/c/flashrom/+/59238
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie498ec55cce8460fc0b2e1fe27254d3a9f763fac
Gerrit-Change-Number: 59238
Gerrit-PatchSet: 4
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-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 10 Jan 2022 03:00:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment