Hello Brian Norris,
I'd like you to do a code review. Please visit
https://review.coreboot.org/22512
to review the following change.
Change subject: linux_mtd: make read/write loop chunks consistent, and documented
......................................................................
linux_mtd: make read/write loop chunks consistent, and documented
Theoretically, there should be no maximum size for the read() and
write() syscalls on an MTD (well, except for the size of the entire
device). But practical concerns (i.e., bugs) have meant we don't quite
do this.
For reads:
Bug https://b/35573113 shows that some SPI-based MTD drivers don't yet
handle very large transactions. So we artificially limit this to
block-sized chunks.
For writes:
It's not clear there is a hard limit. Some drivers will already split
large writes into smaller chunks automatically. Others don't do any
splitting. At any rate, using *small* chunks can actually be a problem
for some devices (b:35104688), as they get worse performance (doing an
internal read/modify/write). This could be fixed in other ways by
advertizing their true "write chunk size" to user space somehow, but
this isn't so easy.
As a simpler fix, we can just increase the loop increment to match the
read loop. Per David, the original implementation (looping over page
chunks) was just being paranoid.
So this patch:
* clarifies comments in linux_mtd_read(), to note that the chunking is
somewhat of a hack that ideally can be fixed (with bug reference)
* simplifies the linux_mtd_write() looping to match the structure in
linux_mtd_read(), including dropping several unnecessary seeks, and
correcting the error messages (they referred to "reads" and had the
wrong parameters)
* change linux_mtd_write() to align its chunks to eraseblocks, not page
sizes
Note that the "->page_size" parameter is still somewhat ill-defined, and
only set by the upper layers for "opaque" flash. And it's not actually
used in this driver now. If we could figure out what we really want to
use it for, then we could try to set it appropriately.
BRANCH=none
BUG=b:35104688
TEST=various flashrom tests on Kevin
TEST=Reading and writing to flash works on our zaius machines over mtd
Change-Id: I551bb85269c854f6888c6bfad8031b14fcf10273
Signed-off-by: Brian Norris <briannorris(a)chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/505409
Reviewed-by: David Hendricks <dhendrix(a)chromium.org>
Reviewed-by: Martin Roth <martinroth(a)chromium.org>
Reviewed-by: William Kennington <wak(a)google.com>
---
M linux_mtd.c
1 file changed, 29 insertions(+), 24 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/12/22512/1
diff --git a/linux_mtd.c b/linux_mtd.c
index 9320bd1..5d67d27 100644
--- a/linux_mtd.c
+++ b/linux_mtd.c
@@ -199,7 +199,12 @@
}
for (i = 0; i < len; ) {
- /* Try to align reads to eraseblock size */
+ /*
+ * Try to align reads to eraseblock size.
+ * FIXME: Shouldn't actually be necessary, but not all MTD
+ * drivers handle arbitrary large reads well. See, for example,
+ * https://b/35573113
+ */
unsigned int step = eb_size - ((start + i) % eb_size);
step = min(step, len - i);
@@ -215,39 +220,39 @@
return 0;
}
-/* this version assumes we must divide the write request into pages ourselves */
+/* this version assumes we must divide the write request into chunks ourselves */
static int linux_mtd_write(struct flashctx *flash, const uint8_t *buf,
unsigned int start, unsigned int len)
{
- unsigned int page;
- unsigned int chunksize, page_size;
-
- chunksize = page_size = flash->chip->page_size;
+ unsigned int chunksize = flash->chip->block_erasers[0].eraseblocks[0].size;
+ unsigned int i;
if (!mtd_device_is_writeable)
return 1;
- for (page = start / page_size;
- page <= (start + len - 1) / page_size; page++) {
- unsigned int i, starthere, lenhere;
+ if (lseek(dev_fd, start, SEEK_SET) != start) {
+ msg_perr("Cannot seek to 0x%06x: %s\n", start, strerror(errno));
+ return 1;
+ }
- starthere = max(start, page * page_size);
- lenhere = min(start + len, (page + 1) * page_size) - starthere;
- for (i = 0; i < lenhere; i += chunksize) {
- unsigned int towrite = min(chunksize, lenhere - i);
+ /*
+ * Try to align writes to eraseblock size. We want these large enough
+ * to give MTD room for optimizing performance.
+ * FIXME: Shouldn't need to divide this up at all, but not all MTD
+ * drivers handle arbitrary large writes well. See, for example,
+ * https://b/35573113
+ */
+ for (i = 0; i < len; ) {
+ unsigned int step = chunksize - ((start + i) % chunksize);
+ step = min(step, len - i);
- if (lseek(dev_fd, starthere, SEEK_SET) != starthere) {
- msg_perr("Cannot seek to 0x%06x: %s\n",
- start, strerror(errno));
- return 1;
- }
-
- if (write(dev_fd, &buf[starthere - start], towrite) != towrite) {
- msg_perr("Cannot read 0x%06x bytes at 0x%06x: "
- "%s\n", start, len, strerror(errno));
- return 1;
- }
+ if (write(dev_fd, buf + i, step) != step) {
+ msg_perr("Cannot write 0x%06x bytes at 0x%06x: %s\n",
+ step, start + i, strerror(errno));
+ return 1;
}
+
+ i += step;
}
return 0;
--
To view, visit https://review.coreboot.org/22512
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I551bb85269c854f6888c6bfad8031b14fcf10273
Gerrit-Change-Number: 22512
Gerrit-PatchSet: 1
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org>
Hello Brian Norris,
I'd like you to do a code review. Please visit
https://review.coreboot.org/22511
to review the following change.
Change subject: linux_mtd: Support for NO_ERASE type devices
......................................................................
linux_mtd: Support for NO_ERASE type devices
Some mtd devices have the MTD_NO_ERASE flag set. This means
these devices don't require an erase to write and might not have
implemented an erase function. We should be conservative and skip
erasing altogether, falling back to performing writes over the whole
flash.
BUG=b:35104688
TESTED=Zaius flash is now written correctly for the 0xff regions.
Change-Id: I4c2fc769a8e0865edf8507f6bd796dc3344b4226
Signed-off-by: William A. Kennington III <wak(a)google.com>
Reviewed-on: https://chromium-review.googlesource.com/472128
Commit-Ready: William Kennington <wak(a)google.com>
Tested-by: William Kennington <wak(a)google.com>
Reviewed-by: Brian Norris <briannorris(a)chromium.org>
---
M flash.h
M flashrom.c
M linux_mtd.c
3 files changed, 16 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/11/22511/1
diff --git a/flash.h b/flash.h
index 62f2e34..d16751b 100644
--- a/flash.h
+++ b/flash.h
@@ -120,6 +120,7 @@
#define FEATURE_OTP (1 << 8)
#define FEATURE_QPI (1 << 9)
#define FEATURE_4BA_SUPPORT (1 << 10)
+#define FEATURE_NO_ERASE (1 << 11)
enum test_state {
OK = 0,
diff --git a/flashrom.c b/flashrom.c
index 6329041..d7cafc2 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -1779,7 +1779,8 @@
ret = 1;
bool skipped = true;
uint8_t *const curcontents = info->curcontents + info->erase_start;
- if (need_erase(curcontents, newcontents, erase_len, flashctx->chip->gran)) {
+ if (!(flashctx->chip->feature_bits & FEATURE_NO_ERASE) &&
+ need_erase(curcontents, newcontents, erase_len, flashctx->chip->gran)) {
if (erase_block(flashctx, info, erasefn))
goto _free_ret;
/* Erase was successful. Adjust curcontents. */
diff --git a/linux_mtd.c b/linux_mtd.c
index 99a7ec0..9320bd1 100644
--- a/linux_mtd.c
+++ b/linux_mtd.c
@@ -48,6 +48,8 @@
static int mtd_device_is_writeable;
+static int mtd_no_erase;
+
/* Size info is presented in bytes in sysfs. */
static unsigned long int mtd_total_size;
static unsigned long int mtd_numeraseregions;
@@ -131,6 +133,9 @@
/* cache for later use by write function */
mtd_device_is_writeable = 1;
}
+ if (tmp & MTD_NO_ERASE) {
+ mtd_no_erase = 1;
+ }
/* Device name */
if (read_sysfs_string("name", mtd_device_name, sizeof(mtd_device_name)))
@@ -174,10 +179,8 @@
flash->chip->tested.read = OK;
flash->chip->tested.erase = OK;
flash->chip->tested.write = OK;
- flash->chip->total_size = mtd_total_size / 1024; /* bytes -> kB */
- flash->chip->block_erasers[0].eraseblocks[0].size = mtd_erasesize;
- flash->chip->block_erasers[0].eraseblocks[0].count =
- mtd_total_size / mtd_erasesize;
+ if (mtd_no_erase)
+ flash->chip->feature_bits |= FEATURE_NO_ERASE;
#if 0
flash->chip->wp = &wp_mtd;
#endif
@@ -255,6 +258,12 @@
{
uint32_t u;
+ if (mtd_no_erase) {
+ msg_perr("%s: device does not support erasing. Please file a "
+ "bug report at flashrom(a)flashrom.org\n", __func__);
+ return 1;
+ }
+
if (mtd_numeraseregions != 0) {
/* TODO: Support non-uniform eraseblock size using
use MEMGETREGIONCOUNT/MEMGETREGIONINFO ioctls */
--
To view, visit https://review.coreboot.org/22511
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4c2fc769a8e0865edf8507f6bd796dc3344b4226
Gerrit-Change-Number: 22511
Gerrit-PatchSet: 1
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org>
Hello Brian Norris,
I'd like you to do a code review. Please visit
https://review.coreboot.org/22510
to review the following change.
Change subject: mtd: print a hint for users when lock/unlock fails
......................................................................
mtd: print a hint for users when lock/unlock fails
Some people have trouble with error messages. Let's make this one more
obvious.
BRANCH=none
BUG=chrome-os-partner:54736
TEST=`flashrom --wp-disable`, when WP# is asserted (and therefore you
can't change the flash Status Register)
Change-Id: I4fe40c88da2faad9a9df40da9f9df216ed5119af
Signed-off-by: Brian Norris <briannorris(a)chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/363925
Reviewed-by: David Hendricks <dhendrix(a)chromium.org>
---
M linux_mtd.c
1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/10/22510/1
diff --git a/linux_mtd.c b/linux_mtd.c
index cd12d29..99a7ec0 100644
--- a/linux_mtd.c
+++ b/linux_mtd.c
@@ -450,6 +450,7 @@
if (ioctl(dev_fd, MEMUNLOCK, &entire_chip) == -1) {
msg_perr("%s: Failed to disable write-protection, ioctl: %s\n",
__func__, strerror(errno));
+ msg_perr("Did you disable WP#?\n");
return 1;
}
@@ -476,6 +477,7 @@
if (ioctl(dev_fd, MEMUNLOCK, &erase_info) == -1) {
msg_perr("%s: ioctl: %s\n", __func__, strerror(errno));
+ msg_perr("Did you disable WP#?\n");
return 1;
}
--
To view, visit https://review.coreboot.org/22510
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4fe40c88da2faad9a9df40da9f9df216ed5119af
Gerrit-Change-Number: 22510
Gerrit-PatchSet: 1
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org>
Hello Brian Norris,
I'd like you to do a code review. Please visit
https://review.coreboot.org/22508
to review the following change.
Change subject: linux_mtd: do reads in eraseblock-sized chunks
......................................................................
linux_mtd: do reads in eraseblock-sized chunks
It's probably not the best idea to try to do an 8MB read in one syscall.
Theoretically, this should work; but MTD just relies on the SPI driver
to deliver the whole read in one transfer, and many SPI drivers haven't
been tested well with large transfer sizes.
I'd consider this a workaround, but it's still good to have IMO.
BUG=chrome-os-partner:53215
TEST=boot kevin; `flashrom --read ...`
TEST=check for performance regression on oak
BRANCH=none
Change-Id: I3a8e160795cd24ec1851c00394892536a2a9d0c7
Signed-off-by: Brian Norris <briannorris(a)chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/344006
Reviewed-by: David Hendricks <dhendrix(a)chromium.org>
---
M linux_mtd.c
1 file changed, 15 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/08/22508/1
diff --git a/linux_mtd.c b/linux_mtd.c
index f09c572..7d1e286 100644
--- a/linux_mtd.c
+++ b/linux_mtd.c
@@ -187,15 +187,26 @@
static int linux_mtd_read(struct flashctx *flash, uint8_t *buf,
unsigned int start, unsigned int len)
{
+ unsigned int eb_size = flash->chip->block_erasers[0].eraseblocks[0].size;
+ unsigned int i;
+
if (lseek(dev_fd, start, SEEK_SET) != start) {
msg_perr("Cannot seek to 0x%06x: %s\n", start, strerror(errno));
return 1;
}
- if (read(dev_fd, buf, len) != len) {
- msg_perr("Cannot read 0x%06x bytes at 0x%06x: %s\n",
- start, len, strerror(errno));
- return 1;
+ for (i = 0; i < len; ) {
+ /* Try to align reads to eraseblock size */
+ unsigned int step = eb_size - ((start + i) % eb_size);
+ step = min(step, len - i);
+
+ if (read(dev_fd, buf + i, step) != step) {
+ msg_perr("Cannot read 0x%06x bytes at 0x%06x: %s\n",
+ step, start + i, strerror(errno));
+ return 1;
+ }
+
+ i += step;
}
return 0;
@@ -265,8 +276,6 @@
}
static struct opaque_master opaque_master_linux_mtd = {
- /* FIXME: Do we need to change these (especially write) as per
- * page size requirements? */
.max_data_read = MAX_DATA_UNSPECIFIED,
.max_data_write = MAX_DATA_UNSPECIFIED,
.probe = linux_mtd_probe,
--
To view, visit https://review.coreboot.org/22508
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3a8e160795cd24ec1851c00394892536a2a9d0c7
Gerrit-Change-Number: 22508
Gerrit-PatchSet: 1
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org>
Hello Nicolas Boichat,
I'd like you to do a code review. Please visit
https://review.coreboot.org/22506
to review the following change.
Change subject: linux_mtd: fix wp range detection in mtd_wp_status
......................................................................
linux_mtd: fix wp range detection in mtd_wp_status
The logic in wp range detection in mtd_wp_status is wrong. The first
block (start == 0) may possible be write-protected, thus we can't use
'if (!start)' for detecting start of write-protection. Since start and
end are both uint32_t, we add two new int variables to flag the status.
Signed-off-by: Wei-Ning Huang <wnhuang(a)google.com>
BUG=chrome-os-partner:40208
TEST=`flashrom --wp-enable --wp-range 0x0 0x200000` then `flashrom --wp-status`
should display: 'WP: write protect range: start=0x00000000, len=0x00200000'
Change-Id: Id8934c2ba997ff8558a9be3a5aa05b56272c9e85
Reviewed-on: https://chromium-review.googlesource.com/330225
Commit-Ready: Wei-Ning Huang <wnhuang(a)chromium.org>
Tested-by: Wei-Ning Huang <wnhuang(a)chromium.org>
Reviewed-by: David Hendricks <dhendrix(a)chromium.org>
Reviewed-by: Nicolas Boichat <drinkcat(a)chromium.org>
---
M linux_mtd.c
1 file changed, 9 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/22506/1
diff --git a/linux_mtd.c b/linux_mtd.c
index c707167..6055aef 100644
--- a/linux_mtd.c
+++ b/linux_mtd.c
@@ -476,6 +476,7 @@
static int mtd_wp_status(const struct flashchip *flash)
{
uint32_t start = 0, end = 0;
+ int start_found = 0, end_found = 0;
unsigned int u;
/* For now, assume only one contiguous region can be locked (NOR) */
@@ -492,14 +493,18 @@
msg_perr("%s: ioctl: %s\n", __func__, strerror(errno));
return 1;
} else if (rc == 1) {
- if (!start)
+ if (!start_found) {
start = erase_info.start;
+ start_found = 1;
+ }
} else if (rc == 0) {
- if (start)
- end = erase_info.start - 1;
+ if (start_found) {
+ end = erase_info.start;
+ end_found = 1;
+ }
}
- if (start && end) {
+ if (start_found && end_found) {
msg_pinfo("WP: write protect range: start=0x%08x, "
"len=0x%08x\n", start, end - start);
/* TODO: Replace this break with "start = end = 0" if
--
To view, visit https://review.coreboot.org/22506
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id8934c2ba997ff8558a9be3a5aa05b56272c9e85
Gerrit-Change-Number: 22506
Gerrit-PatchSet: 1
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Nicolas Boichat <drinkcat(a)chromium.org>
Gerrit-Reviewer: Wei-Ning Huang <wnhuang(a)google.com>
Hello Brian Norris,
I'd like you to do a code review. Please visit
https://review.coreboot.org/22504
to review the following change.
Change subject: MTD write-protect support
......................................................................
MTD write-protect support
** WIP: Do not merge (yet) **
This adds MTD write protect functionality.
Caveats:
- --wp-range and --wp-enable must be used simultaneously
- --wp-disable will clear the chip's block protect range by default.
If the user specifies a range using --wp-range in the same
invocation to flashrom, then that range will be cleared instead.
BUG=chrome-os-partner:40208,chromium:523575
BRANCH=none
TEST=needs testing.
Change-Id: Ifed2ad510a70fe1944f93f48e411769977afb5ca
Signed-off-by: David Hendricks <dhendrix(a)chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/275381
Reviewed-by: Brian Norris <briannorris(a)chromium.org>
---
M linux_mtd.c
1 file changed, 150 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/04/22504/1
diff --git a/linux_mtd.c b/linux_mtd.c
index 064065f..d8d835d 100644
--- a/linux_mtd.c
+++ b/linux_mtd.c
@@ -34,6 +34,9 @@
#include "file.h"
#include "flash.h"
#include "programmer.h"
+#if 0
+#include "writeprotect.h"
+#endif
#define LINUX_DEV_ROOT "/dev"
#define LINUX_MTD_SYSFS_ROOT "/sys/class/mtd"
@@ -49,6 +52,10 @@
static unsigned long int mtd_total_size;
static unsigned long int mtd_numeraseregions;
static unsigned long int mtd_erasesize; /* only valid if numeraseregions is 0 */
+
+#if 0
+static struct wp wp_mtd; /* forward declaration */
+#endif
/* read a string from a sysfs file and sanitize it */
static int read_sysfs_string(const char *filename, char *buf, int len)
@@ -171,6 +178,9 @@
flash->chip->block_erasers[0].eraseblocks[0].size = mtd_erasesize;
flash->chip->block_erasers[0].eraseblocks[0].count =
mtd_total_size / mtd_erasesize;
+#if 0
+ flash->chip->wp = &wp_mtd;
+#endif
return 1;
}
@@ -369,3 +379,143 @@
msg_pdbg("%s: %s\n", __func__, ret == 0 ? "success." : "failed.");
return ret;
}
+
+#if 0
+/*
+ * Write-protect functions.
+ */
+static int mtd_wp_list_ranges(const struct flashchip *flash)
+{
+ /* TODO: implement this */
+ msg_perr("--wp-list is not currently implemented for MTD.\n");
+ return 1;
+}
+
+/*
+ * We only have MEMLOCK to enable write-protection for a particular block,
+ * so we need to do force the user to use --wp-range and --wp-enable
+ * command-line arguments simultaneously. (Fortunately, CrOS factory
+ * installer does this already).
+ *
+ * The --wp-range argument is processed first and will set these variables
+ * which --wp-enable will use afterward.
+ */
+static unsigned int wp_range_start;
+static unsigned int wp_range_len;
+static int wp_set_range_called = 0;
+
+static int mtd_wp_set_range(const struct flashchip *flash,
+ unsigned int start, unsigned int len)
+{
+ wp_range_start = start;
+ wp_range_len = len;
+
+ wp_set_range_called = 1;
+ return 0;
+}
+
+static int mtd_wp_enable_writeprotect(const struct flashchip *flash, enum wp_mode mode)
+{
+ struct erase_info_user entire_chip = {
+ .start = 0,
+ .length = mtd_total_size,
+ };
+ struct erase_info_user desired_range = {
+ .start = wp_range_start,
+ .length = wp_range_len,
+ };
+
+ if (!wp_set_range_called) {
+ msg_perr("For MTD, --wp-range and --wp-enable must be "
+ "used simultaneously.\n");
+ return 1;
+ }
+
+ /*
+ * MTD handles write-protection additively, so whatever new range is
+ * specified is added to the range which is currently protected. To be
+ * consistent with flashrom behavior with other programmer interfaces,
+ * we need to disable the current write protection and then enable
+ * it for the desired range.
+ */
+ if (ioctl(dev_fd, MEMUNLOCK, &entire_chip) == -1) {
+ msg_perr("%s: Failed to disable write-protection, ioctl: %s\n",
+ __func__, strerror(errno));
+ return 1;
+ }
+
+ if (ioctl(dev_fd, MEMLOCK, &desired_range) == -1) {
+ msg_perr("%s: Failed to enable write-protection, ioctl: %s\n",
+ __func__, strerror(errno));
+ return 1;
+ }
+
+ return 0;
+}
+
+static int mtd_wp_disable_writeprotect(const struct flashchip *flash)
+{
+ struct erase_info_user erase_info;
+
+ if (wp_set_range_called) {
+ erase_info.start = wp_range_start;
+ erase_info.length = wp_range_len;
+ } else {
+ erase_info.start = 0;
+ erase_info.length = mtd_total_size;
+ }
+
+ if (ioctl(dev_fd, MEMUNLOCK, &erase_info) == -1) {
+ msg_perr("%s: ioctl: %s\n", __func__, strerror(errno));
+ return 1;
+ }
+
+ return 0;
+}
+
+static int mtd_wp_status(const struct flashchip *flash)
+{
+ uint32_t start = 0, end = 0;
+ unsigned int u;
+
+ /* For now, assume only one contiguous region can be locked (NOR) */
+ /* FIXME: use flash struct members instead of raw MTD values here */
+ for (u = 0; u < mtd_total_size / mtd_erasesize; u += mtd_erasesize) {
+ int rc;
+ struct erase_info_user erase_info = {
+ .start = u,
+ .length = mtd_erasesize,
+ };
+
+ rc = ioctl(dev_fd, MEMISLOCKED, &erase_info);
+ if (rc < 0) {
+ msg_perr("%s: ioctl: %s\n", __func__, strerror(errno));
+ return 1;
+ } else if (rc == 1) {
+ if (!start)
+ start = erase_info.start;
+ } else if (rc == 0) {
+ if (start)
+ end = erase_info.start - 1;
+ }
+
+ if (start && end) {
+ msg_pinfo("WP: write protect range: start=0x%08x, "
+ "len=0x%08x\n", start, end - start);
+ /* TODO: Replace this break with "start = end = 0" if
+ * we want to support non-contiguous locked regions */
+ break;
+ }
+ }
+
+ return 0;
+}
+
+static struct wp wp_mtd = {
+ .list_ranges = mtd_wp_list_ranges,
+ .set_range = mtd_wp_set_range,
+ .enable = mtd_wp_enable_writeprotect,
+ .disable = mtd_wp_disable_writeprotect,
+ .wp_status = mtd_wp_status,
+};
+#endif
--
To view, visit https://review.coreboot.org/22504
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifed2ad510a70fe1944f93f48e411769977afb5ca
Gerrit-Change-Number: 22504
Gerrit-PatchSet: 1
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org>
Hello Daniel Kurtz,
I'd like you to do a code review. Please visit
https://review.coreboot.org/22505
to review the following change.
Change subject: linux_mtd: fix mtd_wp_status
......................................................................
linux_mtd: fix mtd_wp_status
Fix invalid loop range in mtd_wp_status, which ends the for loop
prematurely.
BUG=chrome-os-partner:40208
TEST=`flashrom --wp-status` should output something like:
'WP: write protect range: start=0x00001000, len=0x0000efff'
Change-Id: Ic582bb034c0d2f811813fb6a25696ccddef43e8f
Reviewed-on: https://chromium-review.googlesource.com/329927
Commit-Ready: Wei-Ning Huang <wnhuang(a)chromium.org>
Tested-by: Wei-Ning Huang <wnhuang(a)chromium.org>
Reviewed-by: Daniel Kurtz <djkurtz(a)chromium.org>
---
M linux_mtd.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/05/22505/1
diff --git a/linux_mtd.c b/linux_mtd.c
index d8d835d..c707167 100644
--- a/linux_mtd.c
+++ b/linux_mtd.c
@@ -480,7 +480,7 @@
/* For now, assume only one contiguous region can be locked (NOR) */
/* FIXME: use flash struct members instead of raw MTD values here */
- for (u = 0; u < mtd_total_size / mtd_erasesize; u += mtd_erasesize) {
+ for (u = 0; u < mtd_total_size; u += mtd_erasesize) {
int rc;
struct erase_info_user erase_info = {
.start = u,
--
To view, visit https://review.coreboot.org/22505
To unsubscribe, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic582bb034c0d2f811813fb6a25696ccddef43e8f
Gerrit-Change-Number: 22505
Gerrit-PatchSet: 1
Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com>
Gerrit-Reviewer: Daniel Kurtz <djkurtz(a)chromium.org>
Gerrit-Reviewer: Wei-Ning Huang <wnhuang(a)google.com>