[flashrom-gerrit] Change in flashrom[master]: linux_mtd: do reads in eraseblock-sized chunks

[David Hendricks (Code Review)]

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 
Reviewed-on: https://chromium-review.googlesource.com/344006
Reviewed-by: David Hendricks 
---
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,