Attention is currently required from: Felix Singer, Paul Menzel, Edward O'Callaghan, Angel Pons, Light.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62726 )
Change subject: serprog.c: Avoid calling memcpy with NULL pointer arguments
......................................................................
Patch Set 16: Code-Review+2
(1 comment)
Patchset:
PS16:
> Is this ready for merge?
This does look like ready. Last call everyone! :)
--
To view, visit https://review.coreboot.org/c/flashrom/+/62726
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I850123237e328f9548ba7f77a01888be2cbc9e7b
Gerrit-Change-Number: 62726
Gerrit-PatchSet: 16
Gerrit-Owner: Light <aarya.chaumal(a)gmail.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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Light <aarya.chaumal(a)gmail.com>
Gerrit-Comment-Date: Tue, 10 May 2022 23:18:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Thomas Heijligen, Angel Pons, Sergii Dmytruk.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59072 )
Change subject: dummyflasher: add SR2 and SR3 emulation harness
......................................................................
Patch Set 37: Code-Review+2
(2 comments)
File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/59072/comment/026af603_9cb72bd9
PS34, Line 169: assert
> Returned assert() keeping the function generic and added a comment as to why it's there.
Wow thanks so everyone much for discussion, that's great!
My original question is answered in full. I am marking comment as resolved.
File flashrom.8.tmpl:
https://review.coreboot.org/c/flashrom/+/59072/comment/bd81a9d0_2be7416f
PS34, Line 817: is a hexadecimal value of up to 24 bits. For example, 0x332211 assigns 0x11 to
: SR1, 0x22 to SR2 and 0x33 to SR3. Shorter value is padded to 24 bits with
: zeroes on the left. See datasheet for chosen chip for details about the
: registers content.
This is awkward but the comment below I typed yesterday and forgot to send... discovered it today as a saved draft :D It was meant to explain the comment in the next patch. Sorry!
However you still resolved everything, thank you!
> I looked into the next patch in the chain CB:59073 and I see this would be better in CB:59073. So my comment is the same, just for the next patch. Sorry for confusion.
--
To view, visit https://review.coreboot.org/c/flashrom/+/59072
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I177ae3f068f03380f5b3941d9996a07205672e59
Gerrit-Change-Number: 59072
Gerrit-PatchSet: 37
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: 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: Thomas Heijligen <src(a)posteo.de>
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: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Tue, 10 May 2022 22:53:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Thomas Heijligen, Paul Menzel, Edward O'Callaghan.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59291 )
Change subject: flashrom: Drop read_flash_to_file() usage
......................................................................
Patch Set 15: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/59291
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I4b690b688acf9d5deb46e8642a252a2132ea8c73
Gerrit-Change-Number: 59291
Gerrit-PatchSet: 15
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Nico Huber <nico.h(a)gmx.de>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Tue, 10 May 2022 21:57:45 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Thomas Heijligen, Anastasia Klimchuk, Sergii Dmytruk.
Hello build bot (Jenkins), Nico Huber, Thomas Heijligen, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/59072
to look at the new patch set (#36).
Change subject: dummyflasher: add SR2 and SR3 emulation harness
......................................................................
dummyflasher: add SR2 and SR3 emulation harness
Prepare everything for emulating SR2 and SR3 for chips that have it.
This is needed for accessing SRP1 and WPS bits which are involved in
write protection. The emulated register doesn't affect anything yet
and will be tested by write-protection tests.
TEST=check how input value affects status registers of emulated chip
flashrom -V -p dummy:emulate=W25Q128FV,spi_status=0x12 |
grep 'Initial status register'
flashrom -V -p dummy:emulate=W25Q128FV,spi_status=0x1234 |
grep 'Initial status register'
flashrom -V -p dummy:emulate=W25Q128FV,spi_status=0x123456 |
grep 'Initial status register'
Mind that at this point there are no chips that emulate more than one
status register.
Change-Id: I177ae3f068f03380f5b3941d9996a07205672e59
Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
---
M dummyflasher.c
M flashrom.8.tmpl
2 files changed, 112 insertions(+), 15 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/72/59072/36
--
To view, visit https://review.coreboot.org/c/flashrom/+/59072
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I177ae3f068f03380f5b3941d9996a07205672e59
Gerrit-Change-Number: 59072
Gerrit-PatchSet: 36
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: 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: Thomas Heijligen <src(a)posteo.de>
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: Thomas Heijligen <src(a)posteo.de>
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, Thomas Heijligen, Anastasia Klimchuk, Sergii Dmytruk.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59072 )
Change subject: dummyflasher: add SR2 and SR3 emulation harness
......................................................................
Patch Set 35:
(1 comment)
File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/59072/comment/cad457f4_4b973b9b
PS34, Line 169: assert
> I would split it into three functions, but it would complicate CB:59074 where WP starts influencing […]
I'd rather use assert() than print a warning. If this function gets called with an invalid parameter, then there's a bug in the code. Printing a warning may seem like it could be user error.
--
To view, visit https://review.coreboot.org/c/flashrom/+/59072
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I177ae3f068f03380f5b3941d9996a07205672e59
Gerrit-Change-Number: 59072
Gerrit-PatchSet: 35
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: 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: Thomas Heijligen <src(a)posteo.de>
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: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Tue, 10 May 2022 14:35:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-MessageType: comment