Attention is currently required from: Angel Pons, Nikolai Artemiev.
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/53946 )
Change subject: flashchips.c: add support for W25Q32JW...M
......................................................................
Patch Set 6:
(1 comment)
File flashchips.c:
https://review.coreboot.org/c/flashrom/+/53946/comment/0e3985ce_408db8be
PS1, Line 17583: ..IM
> Angel - I think we're safe w.r.t. device IDs. The 3.3V and 1.8V parts have different device IDs: […]
Oh wait, I was looking at the wrong datasheets. The W25Q32JW and W25Q32FV chips do share IDs between 1.8V and 3.3V parts when the W25Q32FV is operating in QPI mode:
W25Q32JW: https://www.winbond.com/resource-files/W25Q32JW%20SPI%20RevH%2003172020%20P…
W25Q32JW-IQ 15h 6016h
W25Q32JW-IM* 15h 8016h
W25Q32FV: https://www.winbond.com/resource-files/w25q32fv%20revj%2006032016.pdf
W25Q32FV (SPI Mode) 15h 4016h
W25Q32FV (QPI Mode) 15h 6016h
*sigh*
--
To view, visit https://review.coreboot.org/c/flashrom/+/53946
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7425e12658dd69c4ec8d3309dd591d09a935bb4d
Gerrit-Change-Number: 53946
Gerrit-PatchSet: 6
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
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: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Sun, 16 May 2021 23:09:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: David Hendricks <david.hendricks(a)gmail.com>
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
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: Angel Pons, Nikolai Artemiev.
David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/53946 )
Change subject: flashchips.c: add support for W25Q32JW...M
......................................................................
Patch Set 6: Code-Review+2
(1 comment)
File flashchips.c:
https://review.coreboot.org/c/flashrom/+/53946/comment/6d98ad49_ac56daa8
PS1, Line 17583: ..IM
> ni845x_spi.c and dediprog.c uses voltage afaik. […]
Angel - I think we're safe w.r.t. device IDs. The 3.3V and 1.8V parts have different device IDs:
W25Q64JV (3.3V): https://www.winbond.com/resource-files/W25Q64JV%20RevK%2003102021%20Plus.pdf
W25Q64JV-IQ/JQ 16h 4017h
W25Q64JV-IM/JM* 16h 7017h
W25Q64JW (1.8V): https://www.winbond.com/resource-files/W25Q64JW%20RevE%2003102021%20Plus.pdf
W25Q64JW-IQ/JQ 16h 6017h
W25Q64JW-IM/JM* 16h 8017h
I might be missing some details, though... These part numbers can get confusing!
If there are in fact 1.8V and 3.3V chips that share the same device ID, then perhaps we should consider it as a type of "evil twin" and bail out if a voltage isn't specified as a programmer parameter on the command-line (unless it's internal in which case we can assume the correct voltage is applied).
FWIW, I've also noticed that often times you can detect and read a chip using the wrong voltage, but erases and writes will fail. As Edward said, the chips are usually able to withstand it fine for short periods and it's probably harmless in practice.
--
To view, visit https://review.coreboot.org/c/flashrom/+/53946
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I7425e12658dd69c4ec8d3309dd591d09a935bb4d
Gerrit-Change-Number: 53946
Gerrit-PatchSet: 6
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
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: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Sun, 16 May 2021 22:59:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
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: Edward O'Callaghan, Angel Pons.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54171 )
Change subject: programmer: Make use of new register_opaque_master() API
......................................................................
Patch Set 3:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/54171/comment/c1de36cb_b55df95a
PS2, Line 10: some
> `a mutable global`, actually it's just one in this case ;)
Done, and also in the next patch, same thing :)
--
To view, visit https://review.coreboot.org/c/flashrom/+/54171
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I160810cd67f782131962e96fc6d20e2987fb0390
Gerrit-Change-Number: 54171
Gerrit-PatchSet: 3
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-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Sun, 16 May 2021 22:09: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: Edward O'Callaghan, 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/+/54172
to look at the new patch set (#3).
Change subject: programmer: Make use of new register_par_master() API
......................................................................
programmer: Make use of new register_par_master() API
Pass pointers to dynamically allocated data to register_par_master().
This way we can avoid a mutable global.
BUG=b:185191942
TEST=builds
Change-Id: I76572e43d01f8a5e1aa73b1b9e8a187465ed8fef
Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org>
---
M dummyflasher.c
1 file changed, 2 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/72/54172/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/54172
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I76572e43d01f8a5e1aa73b1b9e8a187465ed8fef
Gerrit-Change-Number: 54172
Gerrit-PatchSet: 3
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-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-MessageType: newpatchset