Every SPI host controller implemented its own way to read flash chips

This was partly due to a design problem in the abstraction layer.

There should be exactly two different functions for reading SPI chips:
- memory mapped reads
- SPI command reads.

Each of them should be contained in a separate function, optionally
taking parameters where needed.

This patch solves the problems mentioned above, shortens the code and
makes the code logic a lot more obvious.

Since open-coding the min() function leads to errors, include it in this
patch as well.

Corresponding to flashrom svn r589.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
Acked-by: Ronald G. Minnich <rminnich@gmail.com>

flash.h

@@ -721,6 +721,7 @@ extern int verbose;
 #define printf_debug(x...) { if (verbose) printf(x); }
 void map_flash_registers(struct flashchip *flash);
 int read_memmapped(struct flashchip *flash, uint8_t *buf);
+int min(int a, int b);
 extern char *pcidev_bdf;
 
 /* layout.c */
@@ -768,6 +769,7 @@ int spi_disable_blockprotect(void);
 void spi_byte_program(int address, uint8_t byte);
 int spi_nbyte_program(int address, uint8_t *bytes, int len);
 int spi_nbyte_read(int address, uint8_t *bytes, int len);
+int spi_read_chunked(struct flashchip *flash, uint8_t *buf, int chunksize);
 int spi_aai_write(struct flashchip *flash, uint8_t *buf);
 uint32_t spi_get_valid_read_addr(void);
 

flashrom.c

@@ -200,6 +200,11 @@ int read_memmapped(struct flashchip *flash, uint8_t *buf)
 	return 0;
 }
 
+int min(int a, int b)
+{
+	return (a < b) ? a : b;
+}
+
 char *strcat_realloc(char *dest, const char *src)
 {
 	dest = realloc(dest, strlen(dest) + strlen(src) + 1);

ichspi.c

@@ -148,8 +148,6 @@ static int generate_opcodes(OPCODES * op);
 static int program_opcodes(OPCODES * op);
 static int run_opcode(OPCODE op, uint32_t offset,
 		      uint8_t datalength, uint8_t * data);
-static int ich_spi_read_page(struct flashchip *flash, uint8_t * buf,
-			     int offset, int maxdata);
 static int ich_spi_write_page(struct flashchip *flash, uint8_t * bytes,
 			      int offset, int maxdata);
 
@@ -614,38 +612,6 @@ static int run_opcode(OPCODE op, uint32_t offset,
 	return -1;
 }
 
-static int ich_spi_read_page(struct flashchip *flash, uint8_t * buf, int offset,
-			     int maxdata)
-{
-	int page_size = flash->page_size;
-	uint32_t remaining = flash->page_size;
-	int a;
-
-	printf_debug("ich_spi_read_page: offset=%d, number=%d, buf=%p\n",
-		     offset, page_size, buf);
-
-	for (a = 0; a < page_size; a += maxdata) {
-		if (remaining < maxdata) {
-
-			if (spi_nbyte_read(offset + (page_size - remaining),
-				&buf[page_size - remaining], remaining)) {
-				printf_debug("Error reading");
-				return 1;
-			}
-			remaining = 0;
-		} else {
-			if (spi_nbyte_read(offset + (page_size - remaining),
-				&buf[page_size - remaining], maxdata)) {
-				printf_debug("Error reading");
-				return 1;
-			}
-			remaining -= maxdata;
-		}
-	}
-
-	return 0;
-}
-
 static int ich_spi_write_page(struct flashchip *flash, uint8_t * bytes,
 			      int offset, int maxdata)
 {
@@ -683,21 +649,12 @@ static int ich_spi_write_page(struct flashchip *flash, uint8_t * bytes,
 
 int ich_spi_read(struct flashchip *flash, uint8_t * buf)
 {
-	int i, rc = 0;
-	int total_size = flash->total_size * 1024;
-	int page_size = flash->page_size;
 	int maxdata = 64;
 
-	if (spi_controller == SPI_CONTROLLER_VIA) {
+	if (spi_controller == SPI_CONTROLLER_VIA)
 		maxdata = 16;
-	}
 
-	for (i = 0; (i < total_size / page_size) && (rc == 0); i++) {
-		rc = ich_spi_read_page(flash, (void *)(buf + i * page_size),
-				       i * page_size, maxdata);
-	}
-
-	return rc;
+	return spi_read_chunked(flash, buf, maxdata);
 }
 
 int ich_spi_write_256(struct flashchip *flash, uint8_t * buf)

it87spi.c

@@ -260,18 +260,12 @@ int it8716f_spi_chip_write_1(struct flashchip *flash, uint8_t *buf)
 int it8716f_spi_chip_read(struct flashchip *flash, uint8_t *buf)
 {
 	int total_size = 1024 * flash->total_size;
-	int i;
 	fast_spi = 0;
 
 	if ((programmer == PROGRAMMER_IT87SPI) || (total_size > 512 * 1024)) {
-		for (i = 0; i < total_size; i += 3) {
-			int toread = 3;
-			if (total_size - i < toread)
-				toread = total_size - i;
-			spi_nbyte_read(i, buf + i, toread);
-		}
+		spi_read_chunked(flash, buf, 3);
 	} else {
-		memcpy(buf, (const char *)flash->virtual_memory, total_size);
+		read_memmapped(flash, buf);
 	}
 
 	return 0;

sb600spi.c

@@ -41,14 +41,8 @@ uint8_t *sb600_spibar;
 
 int sb600_spi_read(struct flashchip *flash, uint8_t *buf)
 {
-	int rc = 0, i;
-	int total_size = flash->total_size * 1024;
-	int page_size = 8;
-
-	for (i = 0; i < total_size / page_size; i++)
-		spi_nbyte_read(i * page_size, (void *)(buf + i * page_size),
-			       page_size);
-	return rc;
+	/* Maximum read length is 8 bytes. */
+	return spi_read_chunked(flash, buf, 8);
 }
 
 uint8_t sb600_read_status_register(void)

spi.c

@@ -673,6 +673,33 @@ int spi_nbyte_read(int address, uint8_t *bytes, int len)
 	return spi_command(sizeof(cmd), len, cmd, bytes);
 }
 
+/*
+ * Read a complete flash chip.
+ * Each page is read separately in chunks with a maximum size of chunksize.
+ */
+int spi_read_chunked(struct flashchip *flash, uint8_t *buf, int chunksize)
+{
+	int rc = 0;
+	int i, j;
+	int total_size = flash->total_size * 1024;
+	int page_size = flash->page_size;
+	int toread;
+
+	for (j = 0; j < total_size / page_size; j++) {
+		for (i = 0; i < page_size; i += chunksize) {
+			toread = min(chunksize, page_size - i);
+			rc = spi_nbyte_read(j * page_size + i,
+					    buf + j * page_size + i, toread);
+			if (rc)
+				break;
+		}
+		if (rc)
+			break;
+	}
+
+	return rc;
+}
+
 int spi_chip_read(struct flashchip *flash, uint8_t *buf)
 {
 	switch (spi_controller) {

wbsio_spi.c

@@ -177,12 +177,12 @@ int wbsio_spi_read(struct flashchip *flash, uint8_t *buf)
 {
 	int size = flash->total_size * 1024;
 
-	if (flash->total_size > 1024) {
+	if (size > 1024 * 1024) {
 		fprintf(stderr, "%s: Winbond saved on 4 register bits so max chip size is 1024 KB!\n", __func__);
 		return 1;
 	}
 
-	memcpy(buf, (const char *)flash->virtual_memory, size);
+	read_memmapped(flash, buf);
 	return 0;
 }