David Hendricks has posted comments on this change. ( https://review.coreboot.org/29306 )
Change subject: flashchips: Add IS25LP256 and IS25WP256
......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/#/c/29306/1/flashchips.c
File flashchips.c:
https://review.coreboot.org/#/c/29306/1/flashchips.c@7399
PS1, Line 7399: .block_erase = spi_block_erase_20,
> Also aliased as d7. I wouldn't mind if you don't want to add it. […]
I tried adding d7, but ran into the NUM_ERASEFUNCTIONS limit. So I went with a comment instead - No need to change NUM_ERASEFUNCTIONS to accommodate a redundant instruction.
--
To view, visit https://review.coreboot.org/29306
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idf7a224abcde5f7935d9ef88309f78207de60a7a
Gerrit-Change-Number: 29306
Gerrit-PatchSet: 4
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Mon, 29 Oct 2018 23:08:11 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
Hello build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/29306
to look at the new patch set (#4).
Change subject: flashchips: Add IS25LP256 and IS25WP256
......................................................................
flashchips: Add IS25LP256 and IS25WP256
Tested IS25LP256 using Raspberry Pi and Dediprog SF600 programmers.
Tested IS25WP256 using Dediprog SF600.
Change-Id: Idf7a224abcde5f7935d9ef88309f78207de60a7a
Signed-off-by: David Hendricks <david.hendricks(a)gmail.com>
---
M flashchips.c
M flashchips.h
2 files changed, 102 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/29306/4
--
To view, visit https://review.coreboot.org/29306
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idf7a224abcde5f7935d9ef88309f78207de60a7a
Gerrit-Change-Number: 29306
Gerrit-PatchSet: 4
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Hello build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/29306
to look at the new patch set (#3).
Change subject: flashchips: Add IS25LP256 and IS25WP256
......................................................................
flashchips: Add IS25LP256 and IS25WP256
Tested IS25LP256 using Raspberry Pi and Dediprog SF600 programmers.
Tested IS25WP256 using Dediprog SF600.
Change-Id: Idf7a224abcde5f7935d9ef88309f78207de60a7a
Signed-off-by: David Hendricks <david.hendricks(a)gmail.com>
---
M flashchips.c
M flashchips.h
2 files changed, 106 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/29306/3
--
To view, visit https://review.coreboot.org/29306
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idf7a224abcde5f7935d9ef88309f78207de60a7a
Gerrit-Change-Number: 29306
Gerrit-PatchSet: 3
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
David Hendricks has posted comments on this change. ( https://review.coreboot.org/29305 )
Change subject: flashchips: Add W25Q256JV support
......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/29305/1/flashchips.c
File flashchips.c:
https://review.coreboot.org/#/c/29305/1/flashchips.c@15561
PS1, Line 15561: /* FOUR_BYTE_ADDR: supports 4-bytes addressing mode */
> doesn't add any information (I guess the copied comment above is only a leftover)
Agreed, the comment is stale. We should probably clean up all those in a follow-up patch.
--
To view, visit https://review.coreboot.org/29305
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I76d7362777d364594d2a733d7e478741b0bef7c4
Gerrit-Change-Number: 29305
Gerrit-PatchSet: 1
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Mon, 29 Oct 2018 22:50:44 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
David Hendricks has posted comments on this change. ( https://review.coreboot.org/28804 )
Change subject: [WIP]dediprog: Implement 4-byte-address support
......................................................................
Patch Set 1:
(2 comments)
> Patch Set 1:
>
> (2 comments)
>
> > I was able to test this and it works with one small change (see
> > comments).
> >
> > I used an SF600 with firmware 7.2.21, with W25Q256JVFM and
> > MX25L25735FMI and wrote to 1MB ranges in the lower and upper halves
> > of the chip.
>
> At least something works now...
>
> Did you test the native instructions only? e.g. you could remove
> the native flags / erase functions (and in another step the enter
> 4BA flags) for your chip to test different paths.
I think so... I tried with only FEATURE_4BA_NATIVE enabled and I also tried with only FEATURE_4BA enabled, the former should have used native instructions only, right?
> Maybe we should add a chip test mode to flashrom first.
Yes, I think that would be a great idea.
Built-in chip testing has been on my TODO list for a while. I hacked up something for testing write-protection capabilities a while back for CrOS flashrom: https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/…
> With all
> the possible Dedicrap programmers, hardware and firmware revisions
> we'll have a lot to test; or start to whitelist individual combi-
> nations :-/
May as well start with what we know works.
https://review.coreboot.org/#/c/28804/1/dediprog.c
File dediprog.c:
https://review.coreboot.org/#/c/28804/1/dediprog.c@390
PS1, Line 390: use_4ba = true;
> So this path seems to have failed for Ron but succeeded for David... […]
It's worth noting that Ron also hacked it to only do slow reads. I think there is an issue in that path, though, as I was unable to successfully test the fmap patch using a dediprog because the binary search algorithm would always use small reads.
I'll need to investigate that some other time though.
https://review.coreboot.org/#/c/28804/1/dediprog.c@399
PS1, Line 399: data_packet[3] = WRITE_MODE_4B_ADDR_256B_PAGE_PGM_0x12
> Hard to tell... it's obviously redundant with data_packet[4]. I could […]
Perhaps. But at least I have one known good configuration. Let's start with that.
--
To view, visit https://review.coreboot.org/28804
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I665d0806aec469a3509620a760815861fbe22841
Gerrit-Change-Number: 28804
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Ronald G. Minnich <rminnich(a)gmail.com>
Gerrit-Comment-Date: Mon, 29 Oct 2018 22:48:51 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
Nico Huber has posted comments on this change. ( https://review.coreboot.org/28804 )
Change subject: [WIP]dediprog: Implement 4-byte-address support
......................................................................
Patch Set 1:
(2 comments)
> I was able to test this and it works with one small change (see
> comments).
>
> I used an SF600 with firmware 7.2.21, with W25Q256JVFM and
> MX25L25735FMI and wrote to 1MB ranges in the lower and upper halves
> of the chip.
At least something works now...
Did you test the native instructions only? e.g. you could remove
the native flags / erase functions (and in another step the enter
4BA flags) for your chip to test different paths.
Maybe we should add a chip test mode to flashrom first. With all
the possible Dedicrap programmers, hardware and firmware revisions
we'll have a lot to test; or start to whitelist individual combi-
nations :-/
https://review.coreboot.org/#/c/28804/1/dediprog.c
File dediprog.c:
https://review.coreboot.org/#/c/28804/1/dediprog.c@390
PS1, Line 390: use_4ba = true;
So this path seems to have failed for Ron but succeeded for David...
I fear we have to get back to reverse engineering if we want reliable
support for this crapware.
https://review.coreboot.org/#/c/28804/1/dediprog.c@399
PS1, Line 399: data_packet[3] = WRITE_MODE_4B_ADDR_256B_PAGE_PGM_0x12
> For some reason I cannot explain, this needs to be WRITE_MODE_4B_ADDR_256B_PAGE_PGM. […]
Hard to tell... it's obviously redundant with data_packet[4]. I could
imagine bad things. For instance they could have used hard-coded
instructions first, and added data_packet[4] only later. Or maybe
they try to cache and check the 4BA mode setting? who knows. I get
the desperate feeling that we might need a more fine-grained version
distinction.
--
To view, visit https://review.coreboot.org/28804
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I665d0806aec469a3509620a760815861fbe22841
Gerrit-Change-Number: 28804
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Ronald G. Minnich <rminnich(a)gmail.com>
Gerrit-Comment-Date: Mon, 29 Oct 2018 21:37:09 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No