Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/c/flashrom/+/30380 )
Change subject: dediprog: Fix small, unaligned reads
......................................................................
dediprog: Fix small, unaligned reads
This never was a use case until now but the `--fmap` code makes it
obvious: Unaligned reads that were smaller than the `chunksize` here,
were extended without considering the length of the buffer read into.
With that fixed we run into the next problem: dediprog_spi_bulk_read()
shouldn't report an error when an empty read is unaligned.
Change-Id: Ie12b62499ebfdb467d5126c00d327c76077ddead
Signed-off-by: Nico Huber <nico.huber(a)secunet.com>
Reviewed-on: https://review.coreboot.org/c/30051
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: David Hendricks <david.hendricks(a)gmail.com>
Reviewed-on: https://review.coreboot.org/c/30380
Reviewed-by: Nico Huber <nico.h(a)gmx.de>
---
M dediprog.c
1 file changed, 4 insertions(+), 4 deletions(-)
Approvals:
build bot (Jenkins): Verified
Nico Huber: Looks good to me, approved
diff --git a/dediprog.c b/dediprog.c
index 2f5b441..70660d4 100644
--- a/dediprog.c
+++ b/dediprog.c
@@ -425,15 +425,15 @@
struct libusb_transfer *transfers[DEDIPROG_ASYNC_TRANSFERS] = { NULL, };
struct libusb_transfer *transfer;
+ if (len == 0)
+ return 0;
+
if ((start % chunksize) || (len % chunksize)) {
msg_perr("%s: Unaligned start=%i, len=%i! Please report a bug at flashrom(a)flashrom.org\n",
__func__, start, len);
return 1;
}
- if (len == 0)
- return 0;
-
/* Command packet size of protocols: new 10 B, old 5 B. */
uint8_t data_packet[is_new_prot() ? 10 : 5];
unsigned int value, idx;
@@ -503,7 +503,7 @@
int ret;
/* chunksize must be 512, other sizes will NOT work at all. */
const unsigned int chunksize = 0x200;
- unsigned int residue = start % chunksize ? chunksize - start % chunksize : 0;
+ unsigned int residue = start % chunksize ? min(len, chunksize - start % chunksize) : 0;
unsigned int bulklen;
dediprog_set_leds(LED_BUSY);
--
To view, visit https://review.coreboot.org/c/flashrom/+/30380
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: 1.0.x
Gerrit-Change-Id: Ie12b62499ebfdb467d5126c00d327c76077ddead
Gerrit-Change-Number: 30380
Gerrit-PatchSet: 4
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-MessageType: merged
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/28804 )
Change subject: dediprog: Enable 4BA support for SF600, protocol V2
......................................................................
Patch Set 2:
>> 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?
>
Alas, FEATURE_4BA includes _NATIVE and the latter is automatically selected if available. So it's likely you tested native instructions twice.
--
To view, visit https://review.coreboot.org/c/flashrom/+/28804
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I665d0806aec469a3509620a760815861fbe22841
Gerrit-Change-Number: 28804
Gerrit-PatchSet: 2
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: ron minnich <rminnich(a)gmail.com>
Gerrit-Comment-Date: Sat, 22 Dec 2018 17:03:15 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/30386 )
Change subject: dediprog: Disable 4BA completely
......................................................................
Patch Set 1:
I'd like to have this in for the 1.1 release. The follow-up too if somebody can confirm it's working.
--
To view, visit https://review.coreboot.org/c/flashrom/+/30386
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I08efcbb09ab3499ef6902a698e9ce3d6232237c4
Gerrit-Change-Number: 30386
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Stefan T <stefan.tauner(a)gmx.at>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Sat, 22 Dec 2018 16:59:17 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/28804 )
Change subject: dediprog: Enable 4BA support for SF600, protocol V2
......................................................................
Patch Set 2:
(1 comment)
I've reduced this to the single path that is expected to work and tried to minimize changes. If somebody figures out more working combinations, I'd be happy to merged them. But I'll probably not work on this myself (unless my employment comes into play).
Even the Dediprog specification I got my hands on is worth less than a USB trace. It doesn't tell what is going on on the SPI side. It's probably easier to write proper firmware for these devices than to use what Dediprog ships.
https://review.coreboot.org/#/c/28804/1/dediprog.c
File dediprog.c:
https://review.coreboot.org/#/c/28804/1/dediprog.c@399
PS1, Line 399: data_packet[13] = 0x00;
> Perhaps. But at least I have one known good configuration. Let's start with that.
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/28804
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I665d0806aec469a3509620a760815861fbe22841
Gerrit-Change-Number: 28804
Gerrit-PatchSet: 2
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: ron minnich <rminnich(a)gmail.com>
Gerrit-Comment-Date: Sat, 22 Dec 2018 16:48:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: David Hendricks <david.hendricks(a)gmail.com>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Hello David Hendricks, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/28804
to look at the new patch set (#2).
Change subject: dediprog: Enable 4BA support for SF600, protocol V2
......................................................................
dediprog: Enable 4BA support for SF600, protocol V2
The only combination we could successfully test so far is the SF600 with
protocol version V2 (firmware 7.2.21) and native 4BA commands. Let's
enable that at least.
Change-Id: I665d0806aec469a3509620a760815861fbe22841
Signed-off-by: Nico Huber <nico.h(a)gmx.de>
---
M dediprog.c
M spi.h
2 files changed, 28 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/04/28804/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/28804
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I665d0806aec469a3509620a760815861fbe22841
Gerrit-Change-Number: 28804
Gerrit-PatchSet: 2
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: ron minnich <rminnich(a)gmail.com>
Gerrit-MessageType: newpatchset
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/30370 )
Change subject: Fix verification with sparse layouts
......................................................................
Patch Set 3:
>> For the future, I wonder, should we normalize the "UI" layout to something where we can make better assumptions? e.g. sort the regions, merge overlapping/directly following regions and fill gaps with non-included regions
>
> sounds good. Maybe it can even be extended to handle things like read and write protected regions?
Yes, that seems to be the logical next step. Gather all the information first and when we prepare the layout for the actual flash access, check things like that. i.e. regions may overlap generally, but not if one says it's protected and the other says we are going to access it.
--
To view, visit https://review.coreboot.org/c/flashrom/+/30370
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I44e0cea621f2a3d4dc70fa7e93c52ed95e54014a
Gerrit-Change-Number: 30370
Gerrit-PatchSet: 3
Gerrit-Owner: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
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: Sat, 22 Dec 2018 14:55:43 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
