Patchwork Speedup SST 25VF032B and 25VF064C programming

login
register
about
Submitter Wagner, Helge (GE Intelligent Platforms)
Date 2010-08-06 13:28:06
Message ID <A3EBBCEF5B62BB44AA29816B1814095A053B806D@BUDMLVEM06.e2k.ad.ge.com>
Download mbox | patch
Permalink /patch/2050/
State Accepted
Commit r1193+r1194
Headers show

Comments

I have tested both SST 25VF032B and SST 25VF064C on a QM57 (IbexPeak)
system and found that the reprogramming of these chips is very slow.

So i have changed the programming engine for the 25VF064C from
"spi_chip_write_1" to "spi_chip_write_256".
I have tested the 25VF064C with the new engine and it works fine.

For the 25VF032B i have changed the programming engine from
"spi_chip_write_1" to "spi_aai_write".
I have tested this engine, too, and found it working (speedup was from
228 to 113 seconds).

While testing i found that some commands needed by some of the
algorithms are not in the OPCODE table. So i have implemented the
on-the-fly reprogramming of the OPCODE table (in the ICH engine).
Speedup for new 25VF032B programming algorithm: from 3091 [sic!] to 123
seconds


-----Original Message-----
From: flashrom-bounces@flashrom.org
[mailto:flashrom-bounces@flashrom.org] On Behalf Of Wagner, Helge (GE
Intelligent Platforms)
Sent: Freitag, 6. August 2010 15:28
To: flashrom@flashrom.org
Subject: [flashrom] Speedup SST 25VF032B and 25VF064C programming

I have tested both SST 25VF032B and SST 25VF064C on a QM57 (IbexPeak)
system and found that the reprogramming of these chips is very slow.

So i have changed the programming engine for the 25VF064C from
"spi_chip_write_1" to "spi_chip_write_256".
I have tested the 25VF064C with the new engine and it works fine.

For the 25VF032B i have changed the programming engine from
"spi_chip_write_1" to "spi_aai_write".
I have tested this engine, too, and found it working (speedup was from
228 to 113 seconds).

While testing i found that some commands needed by some of the
algorithms are not in the OPCODE table. So i have implemented the
on-the-fly reprogramming of the OPCODE table (in the ICH engine).
Carl-Daniel Hailfinger - 2010-10-05 22:40:28
Hi Helge,

I'm very sorry, it seems that the patch repeatedly slipped through the
cracks because patchwork didn't pick it up (application/octet-stream for
the attachment may have been the reason, but that is fixed now).

On 06.08.2010 15:28, Wagner, Helge (GE Intelligent Platforms) wrote:
> I have tested both SST 25VF032B and SST 25VF064C on a QM57 (IbexPeak)
> system and found that the reprogramming of these chips is very slow.
>
> So i have changed the programming engine for the 25VF064C from
> "spi_chip_write_1" to "spi_chip_write_256".
> I have tested the 25VF064C with the new engine and it works fine.
>
> For the 25VF032B i have changed the programming engine from
> "spi_chip_write_1" to "spi_aai_write".
> I have tested this engine, too, and found it working (speedup was from
> 228 to 113 seconds).
>
> While testing i found that some commands needed by some of the
> algorithms are not in the OPCODE table. So i have implemented the
> on-the-fly reprogramming of the OPCODE table (in the ICH engine).
>
> Signed-off-by: Helge Wagner <helge.wagner@ge.com>
>
> Speedup for new 25VF032B programming algorithm: from 3091 [sic!] to 123
> seconds
>   

This patch is a huge step forward for many Intel chipsets.
Thank you!


> diff -urN flashrom-0.9.2/ichspi.c flashrom/ichspi.c
> --- flashrom-0.9.2/ichspi.c	2010-07-28 00:41:39.000000000 +0200
> +++ flashrom/ichspi.c	2010-08-06 14:46:47.000000000 +0200
> @@ -708,11 +777,11 @@
>  	/* find cmd in opcodes-table */
>  	opcode_index = find_opcode(curopcodes, cmd);
>  	if (opcode_index == -1) {
> -		/* FIXME: Reprogram opcodes if possible. Autodetect type of
> -		 * opcode by checking readcnt/writecnt.
> -		 */
> -		msg_pdbg("Invalid OPCODE 0x%02x\n", cmd);
> -		return SPI_INVALID_OPCODE;
> +		opcode_index = reprogram_opcode_on_the_fly(cmd, writecnt, readcnt);
>   

This line should be conditional on !ichspi_lock.
I have changed that in the commit.


> +		if (opcode_index == -1) {
> +			msg_pdbg("Invalid OPCODE 0x%02x\n", cmd);
> +			return SPI_INVALID_OPCODE;
> +		}
>  	}
>  
>  	opcode = &(curopcodes->opcode[opcode_index]);
>   

Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>

ICH dynamic opcode reprogramming patch committed in r1193.
AAI write for SST25VF032B and page write for SST25VF064C committed in r1194.

Regards,
Carl-Daniel

Patch

diff -urN flashrom-0.9.2/chipdrivers.h flashrom/chipdrivers.h
--- flashrom-0.9.2/chipdrivers.h	2010-07-29 15:09:18.000000000 +0200
+++ flashrom/chipdrivers.h	2010-08-06 13:10:28.000000000 +0200
@@ -63,7 +63,8 @@ 
 int spi_nbyte_read(int addr, uint8_t *bytes, int len);
 int spi_read_chunked(struct flashchip *flash, uint8_t *buf, int start, int len, int chunksize);
 int spi_write_chunked(struct flashchip *flash, uint8_t *buf, int start, int len, int chunksize);
-int spi_aai_write(struct flashchip *flash, uint8_t *buf, int start, int len);
+int spi_aai_write_new(struct flashchip *flash, uint8_t *buf, int start, int len);
+int spi_aai_write(struct flashchip *flash, uint8_t *buf);
 
 /* 82802ab.c */
 uint8_t wait_82802ab(chipaddr bios);
diff -urN flashrom-0.9.2/flashchips.c flashrom/flashchips.c
--- flashrom-0.9.2/flashchips.c	2010-08-01 02:13:49.000000000 +0200
+++ flashrom/flashchips.c	2010-08-06 14:09:10.000000000 +0200
@@ -4753,7 +4753,7 @@ 
 		.model_id	= SST_25VF032B,
 		.total_size	= 4096,
 		.page_size	= 256,
-		.tested		= TEST_OK_PRW,
+		.tested		= TEST_OK_PREW,
 		.probe		= probe_spi_rdid,
 		.probe_timing	= TIMING_ZERO,
 		.block_erasers	=
@@ -4776,7 +4776,7 @@ 
 			},
 		},
 		.unlock		= spi_disable_blockprotect,
-		.write		= spi_chip_write_1,
+		.write		= spi_aai_write,
 		.read		= spi_chip_read,
 	},
 
@@ -4811,7 +4811,7 @@ 
 			},
 		},
 		.unlock		= spi_disable_blockprotect,
-		.write		= spi_chip_write_1,
+		.write		= spi_chip_write_256,
 		.read		= spi_chip_read,
 	},
 
diff -urN flashrom-0.9.2/ichspi.c flashrom/ichspi.c
--- flashrom-0.9.2/ichspi.c	2010-07-28 00:41:39.000000000 +0200
+++ flashrom/ichspi.c	2010-08-06 14:46:47.000000000 +0200
@@ -30,6 +30,7 @@ 
  * ST M25P32 already tested
  * ST M25P64
  * AT 25DF321 already tested
+ * ... and many more SPI flash devices
  *
  */
 
@@ -197,8 +198,76 @@ 
 	}
 };
 
+/* List of opcodes with their corresponding spi_type
+ * It is used to reprogram the chipset OPCODE table on-the-fly if an opcode
+ * is needed which is currently not in the chipset OPCODE table
+ */
+static OPCODE POSSIBLE_OPCODES[] = {
+	 {JEDEC_BYTE_PROGRAM, SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS, 0},	// Write Byte
+	 {JEDEC_READ, SPI_OPCODE_TYPE_READ_WITH_ADDRESS, 0},	// Read Data
+	 {JEDEC_BE_D8, SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS, 0},	// Erase Sector
+	 {JEDEC_RDSR, SPI_OPCODE_TYPE_READ_NO_ADDRESS, 0},	// Read Device Status Reg
+	 {JEDEC_REMS, SPI_OPCODE_TYPE_READ_WITH_ADDRESS, 0},	// Read Electronic Manufacturer Signature
+	 {JEDEC_WRSR, SPI_OPCODE_TYPE_WRITE_NO_ADDRESS, 0},	// Write Status Register
+	 {JEDEC_RDID, SPI_OPCODE_TYPE_READ_NO_ADDRESS, 0},	// Read JDEC ID
+	 {JEDEC_CE_C7, SPI_OPCODE_TYPE_WRITE_NO_ADDRESS, 0},	// Bulk erase
+	 {JEDEC_SE, SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS, 0},	// Sector erase
+	 {JEDEC_BE_52, SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS, 0},	// Block erase
+	 {JEDEC_AAI_WORD_PROGRAM, SPI_OPCODE_TYPE_WRITE_NO_ADDRESS, 0},	// Auto Address Increment
+};
+
 static OPCODES O_EXISTING = {};
 
+static uint8_t lookup_spi_type(uint8_t opcode)
+{
+	int a;
+
+	for (a = 0; a < sizeof(POSSIBLE_OPCODES)/sizeof(POSSIBLE_OPCODES[0]); a++) {
+		if (POSSIBLE_OPCODES[a].opcode == opcode)
+			return POSSIBLE_OPCODES[a].spi_type;
+	}
+
+	return 0xFF;
+}
+
+static int reprogram_opcode_on_the_fly(uint8_t opcode, unsigned int writecnt, unsigned int readcnt)
+{
+	uint8_t spi_type;
+
+	spi_type = lookup_spi_type(opcode);
+	if (spi_type > 3) {
+		/* Try to guess spi type from read/write sizes.
+		 * The following valid writecnt/readcnt combinations exist:
+		 * writecnt  = 4, readcnt >= 0
+		 * writecnt  = 1, readcnt >= 0
+		 * writecnt >= 4, readcnt  = 0
+		 * writecnt >= 1, readcnt  = 0
+		 * writecnt >= 1 is guaranteed for all commands.
+		 */
+		if (readcnt == 0)
+			/* if readcnt=0 and writecount >= 4, we don't know if it is WRITE_NO_ADDRESS
+			 * or WRITE_WITH_ADDRESS. But if we use WRITE_NO_ADDRESS and the first 3 data
+			 * bytes are actual the address, they go to the bus anyhow
+			 */
+			spi_type = SPI_OPCODE_TYPE_WRITE_NO_ADDRESS;
+		else if (writecnt == 1) // and readcnt is > 0
+			spi_type = SPI_OPCODE_TYPE_READ_NO_ADDRESS;
+		else if (writecnt == 4) // and readcnt is > 0
+			spi_type = SPI_OPCODE_TYPE_READ_WITH_ADDRESS;
+		// else we have an invalid case, will be handled below
+	}
+	if (spi_type <= 3) {
+		int oppos=2;	// use original JEDEC_BE_D8 offset
+		curopcodes->opcode[oppos].opcode = opcode;
+		curopcodes->opcode[oppos].spi_type = spi_type;
+		program_opcodes(curopcodes);
+		oppos = find_opcode(curopcodes, opcode);
+		msg_pdbg ("on-the-fly OPCODE (0x%02X) re-programmed, op-pos=%d\n", opcode, oppos);
+		return oppos;
+	}
+	return -1;
+}
+
 static int find_opcode(OPCODES *op, uint8_t opcode)
 {
 	int a;
@@ -708,11 +777,11 @@ 
 	/* find cmd in opcodes-table */
 	opcode_index = find_opcode(curopcodes, cmd);
 	if (opcode_index == -1) {
-		/* FIXME: Reprogram opcodes if possible. Autodetect type of
-		 * opcode by checking readcnt/writecnt.
-		 */
-		msg_pdbg("Invalid OPCODE 0x%02x\n", cmd);
-		return SPI_INVALID_OPCODE;
+		opcode_index = reprogram_opcode_on_the_fly(cmd, writecnt, readcnt);
+		if (opcode_index == -1) {
+			msg_pdbg("Invalid OPCODE 0x%02x\n", cmd);
+			return SPI_INVALID_OPCODE;
+		}
 	}
 
 	opcode = &(curopcodes->opcode[opcode_index]);
@@ -825,21 +894,10 @@ 
 				 * No need to bother with fixups.
 				 */
 				if (!ichspi_lock) {
-					msg_pdbg("%s: FIXME: Add on-the-fly"
-						     " reprogramming of the "
-						     "chipset opcode list.\n",
-						     __func__);
-				 	/* FIXME: Reprogram opcode menu.
-					 * Find a less-useful opcode, replace it
-					 * with the wanted opcode, detect optype
-					 * and reprogram the opcode menu.
-					 * Update oppos so the next if-statement
-					 * can do something useful.
-					 */
-					//curopcodes.opcode[lessusefulindex] = (cmds + 1)->writearr[0]);
-					//update_optypes(curopcodes);
-					//program_opcodes(curopcodes);
-					//oppos = find_opcode(curopcodes, (cmds + 1)->writearr[0]);
+					oppos = reprogram_opcode_on_the_fly((cmds + 1)->writearr[0], (cmds + 1)->writecnt, (cmds + 1)->readcnt);
+					if (oppos == -1)
+						continue;
+					curopcodes->opcode[oppos].atomic = preoppos + 1;
 					continue;
 				}
 			}
diff -urN flashrom-0.9.2/spi25.c flashrom/spi25.c
--- flashrom-0.9.2/spi25.c	2010-07-29 15:09:18.000000000 +0200
+++ flashrom/spi25.c	2010-08-06 13:13:23.000000000 +0200
@@ -1300,7 +1300,7 @@ 
 	return spi_chip_write_1_new(flash, buf, 0, flash->total_size * 1024);
 }
 
-int spi_aai_write(struct flashchip *flash, uint8_t *buf, int start, int len)
+int spi_aai_write_new(struct flashchip *flash, uint8_t *buf, int start, int len)
 {
 	uint32_t pos = start;
 	int result;
@@ -1391,3 +1391,17 @@ 
 	spi_write_disable();
 	return 0;
 }
+
+int spi_aai_write(struct flashchip *flash, uint8_t *buf)
+{
+	/* Erase first */
+	msg_cinfo("Erasing flash before programming... ");
+	if (erase_flash(flash)) {
+		msg_cerr("ERASE FAILED!\n");
+		return -1;
+	}
+	msg_cinfo("done.\n");
+
+	return spi_aai_write_new(flash, buf, 0, flash->total_size * 1024);
+}
+