Change in flashrom[master]: linux_spi: Add max_xfer programmer parameter

[David Hendricks (Code Review)]

David Hendricks has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/42405 )

Change subject: linux_spi: Add max_xfer programmer parameter
......................................................................

linux_spi: Add max_xfer programmer parameter

This enables the user to manually set the max transfer size, overriding
the value that would otherwise come from sysfs or getpagesize().

Change-Id: Iea868c2e48e61e981472ebcc5e15aaaaf161cab4
Signed-off-by: David Hendricks 
---
M linux_spi.c
1 file changed, 48 insertions(+), 39 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/05/42405/1

diff --git a/linux_spi.c b/linux_spi.c
index 1ef8f2b..d804b48 100644
--- a/linux_spi.c
+++ b/linux_spi.c
@@ -46,7 +46,7 @@

static int fd = -1;
 #define BUF_SIZE_FROM_SYSFS	"/sys/module/spidev/parameters/bufsiz"
-static size_t max_kernel_buf_size;
+static size_t max_xfer;

static int linux_spi_shutdown(void *data);
 static int linux_spi_send_command(const struct flashctx *flash, unsigned int writecnt,
@@ -110,6 +110,51 @@
    }
    free(dev);

+	p = extract_programmer_param("max_xfer");
+	if (p && strlen(p)) {
+		max_xfer = (uint32_t)strtoul(p, &endp, 0);
+		if (p == endp || max_xfer == 0) {
+			msg_perr("%s: invalid max transfer size: %s bytes\n", __func__, p);
+			free(p);
+			return 1;
+		}
+	} else {
+		/* Read max buffer size from sysfs, or use page size as fallback. */
+		FILE *fp = fopen(BUF_SIZE_FROM_SYSFS, "r");
+		if (!fp) {
+			msg_pwarn("Cannot open %s: %s.\n", BUF_SIZE_FROM_SYSFS, strerror(errno));
+			goto out;
+		}
+
+		char buf[10];
+		if (!fgets(buf, sizeof(buf), fp)) {
+			if (feof(fp))
+				msg_pwarn("Cannot read %s: file is empty.\n", BUF_SIZE_FROM_SYSFS);
+			else
+				msg_pwarn("Cannot read %s: %s.\n", BUF_SIZE_FROM_SYSFS, strerror(errno));
+			fclose(fp);
+			goto out;
+		}
+		fclose(fp);
+
+		long int tmp;
+		errno = 0;
+		tmp = strtol(buf, NULL, 0);
+		if ((tmp < 0) || errno) {
+			msg_pwarn("Buffer size %ld from %s seems wrong.\n", tmp, BUF_SIZE_FROM_SYSFS);
+		} else {
+			msg_pdbg("%s: Using value from %s as max_xfer size.\n", __func__, BUF_SIZE_FROM_SYSFS);
+			max_xfer = (size_t)tmp;
+		}
+
+		if (!max_xfer) {
+			msg_pdbg("%s: Using page size as max_xfer size.\n", __func__);
+			max_xfer = (size_t)getpagesize();
+		}
+	}
+	msg_pdbg("%s: max_xfer: %zu\n", __func__, max_xfer);
+	free(p);
+
    if (register_shutdown(linux_spi_shutdown, NULL))
    	return 1;
    /* We rely on the shutdown function for cleanup from here on. */
@@ -133,43 +178,7 @@
    	return 1;
    }

-	/* Read max buffer size from sysfs, or use page size as fallback. */
-	FILE *fp;
-	fp = fopen(BUF_SIZE_FROM_SYSFS, "r");
-	if (!fp) {
-		msg_pwarn("Cannot open %s: %s.\n", BUF_SIZE_FROM_SYSFS, strerror(errno));
-		goto out;
-	}
-
-	char buf[10];
-	if (!fgets(buf, sizeof(buf), fp)) {
-		if (feof(fp))
-			msg_pwarn("Cannot read %s: file is empty.\n", BUF_SIZE_FROM_SYSFS);
-		else
-			msg_pwarn("Cannot read %s: %s.\n", BUF_SIZE_FROM_SYSFS, strerror(errno));
-		goto out;
-	}
-
-	long int tmp;
-	errno = 0;
-	tmp = strtol(buf, NULL, 0);
-	if ((tmp < 0) || errno) {
-		msg_pwarn("Buffer size %ld from %s seems wrong.\n", tmp, BUF_SIZE_FROM_SYSFS);
-	} else {
-		msg_pdbg("%s: Using value from %s as max buffer size.\n", __func__, BUF_SIZE_FROM_SYSFS);
-		max_kernel_buf_size = (size_t)tmp;
-	}
-
 out:
-	if (fp)
-		fclose(fp);
-
-	if (!max_kernel_buf_size) {
-		msg_pdbg("%s: Using page size as max buffer size.\n", __func__);
-		max_kernel_buf_size = (size_t)getpagesize();
-	}
-
-	msg_pdbg("%s: max_kernel_buf_size: %zu\n", __func__, max_kernel_buf_size);
    register_spi_master(&spi_master_linux);
    return 0;
 }
@@ -225,13 +234,13 @@
 {
    /* Older kernels use a single buffer for combined input and output
       data. So account for longest possible command + address, too. */
-	return spi_read_chunked(flash, buf, start, len, max_kernel_buf_size - 5);
+	return spi_read_chunked(flash, buf, start, len, max_xfer - 5);
 }

static int linux_spi_write_256(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len)
 {
    /* 5 bytes must be reserved for longest possible command + address. */
-	return spi_write_chunked(flash, buf, start, len, max_kernel_buf_size - 5);
+	return spi_write_chunked(flash, buf, start, len, max_xfer - 5);
 }

#endif // CONFIG_LINUX_SPI == 1

[David Hendricks (Code Review)]

David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/42405 )

Change subject: linux_spi: Add max_xfer programmer parameter
......................................................................

Set Ready For Review

[David Hendricks (Code Review)]

David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/42405 )

Change subject: linux_spi: Add max_xfer programmer parameter
......................................................................

Patch Set 3:

For background, this is currently used as a workaround for buggy kernels: https://github.com/TinkerBoard/debian_kernel/issues/51

Ideally it should not be needed, but we don't live in an ideal world...

[David Hendricks (Code Review)]

David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/42405 )

Change subject: linux_spi: Add max_xfer programmer parameter
......................................................................

Set Ready For Review

[David Hendricks (Code Review)]

David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/42405 )

Change subject: linux_spi: Add max_xfer programmer parameter
......................................................................

Patch Set 4:

PS4 puts the code back roughly where it was before to minimize differences, and updates some error paths. Sorry for the noise.