Patchwork [1/n] SPI FAST READ infrastructure

login
register
about
Submitter Carl-Daniel Hailfinger
Date 2010-12-31 06:58:20
Message ID <4D1D7F0C.3080201@gmx.net>
Download mbox | patch
Permalink /patch/2481/
State Bitrotted
Headers show

Comments

Carl-Daniel Hailfinger - 2010-12-31 06:58:20
SPI FAST READ infrastructure.
Add feature bits to allow annotation of SPI flash chips with possible
read methods.
This includes the not-yet-supported Dual and Quad I/O read operations
just in case we ever encounter programmer hardware which can use that
feature.

An alternative design would be to create a separate spi_chip_fast_read
function and have a per-chip array of allowable read functions similar
to what we have for erase functions. A similar design problem exists for
chips supporting multiple write functions (e.g. classic SPI write and
SPI AAI write).

Thorough design review appreciated.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
Stefan Tauner - 2011-08-14 08:38:47
> SPI FAST READ infrastructure.
> Add feature bits to allow annotation of SPI flash chips with possible
> read methods.
> This includes the not-yet-supported Dual and Quad I/O read operations
> just in case we ever encounter programmer hardware which can use that
> feature.
> 
> An alternative design would be to create a separate spi_chip_fast_read
> function and have a per-chip array of allowable read functions similar
> to what we have for erase functions. A similar design problem exists for
> chips supporting multiple write functions (e.g. classic SPI write and
> SPI AAI write).
> 
> Thorough design review appreciated.

i think the alternative is much better, because it is generic.
the only drawback i see is that it uses more space. apart from that it
is clearly superior imo.
- the scheme is know from the erasers
- adding new functions is easy (there will be chips that do it slightly
  different... wanna bet? :)
- generic implementations (such as the example function in dediprog)
  can easily be supported with wrappers that call the generic function
  with an additional parameter. that's better than forcing the
  implementations to be generic functions (that have to check the
  flags).

i would start with adding support for multiple write functions. we have
a use case there already and if we encounter problems there, they are
probably similar to those we would have with read functions.

one thing i have not considered yet (did you?) is how the programmers
are involved. it is clear that they have to control the usage of
commands that need different i/o settings. getting the programmers
involved is already ugly, even without changing pin directions... i
am too tired atm to think that entirely through now.

dual and quad read/write operations should be possible to implement on
the serprog programmers for testing purposes. it would be very slow on
AVRs (bit banging instead of hw spi at f_cpu/2), but for testing
flashrom it would suffice. dunno if there are any (cheap)
microcontrollers supporting fast read/writes. afaics the small ARMs do
not support it either.

Patch

Index: flashrom-spi_fast_read/flash.h
===================================================================
--- flashrom-spi_fast_read/flash.h	(Revision 1249)
+++ flashrom-spi_fast_read/flash.h	(Arbeitskopie)
@@ -92,6 +92,11 @@ 
 #define FEATURE_WRSR_EWSR	(1 << 6)
 #define FEATURE_WRSR_WREN	(1 << 7)
 #define FEATURE_WRSR_EITHER	(FEATURE_WRSR_EWSR | FEATURE_WRSR_WREN)
+#define FEATURE_SPI_FAST_READ		(1 << 8)
+#define FEATURE_SPI_FAST_READ_DUAL_O	(1 << 9)
+#define FEATURE_SPI_FAST_READ_DUAL_IO	(1 << 10)
+#define FEATURE_SPI_FAST_READ_QUAD_O	(1 << 11)
+#define FEATURE_SPI_FAST_READ_QUAD_IO	(1 << 12)
 
 struct flashchip {
 	const char *vendor;
Index: flashrom-spi_fast_read/dediprog.c
===================================================================
--- flashrom-spi_fast_read/dediprog.c	(Revision 1249)
+++ flashrom-spi_fast_read/dediprog.c	(Arbeitskopie)
@@ -220,8 +220,15 @@ 
 
 	/* Round down. */
 	bulklen = (len - residue) / chunksize * chunksize;
-	ret = dediprog_spi_bulk_read(flash, buf + residue, start + residue,
-				     bulklen);
+	if (flash->feature_bits & FEATURE_SPI_FAST_READ) {
+		ret = dediprog_spi_bulk_read(flash, buf + residue,
+					     start + residue, bulklen);
+	} else {
+		msg_pdbg("Using slow read because your flash chip is not "
+			 "listed as supporting fast read (yet).\n");
+		ret = spi_read_chunked(flash, buf + residue, start + residue,
+				       bulklen, 16);
+	}
 	if (ret)
 		return ret;
 
Index: flashrom-spi_fast_read/flashchips.c
===================================================================
--- flashrom-spi_fast_read/flashchips.c	(Revision 1249)
+++ flashrom-spi_fast_read/flashchips.c	(Arbeitskopie)
@@ -282,6 +282,7 @@ 
 		.model_id	= AMIC_A25L05PT,
 		.total_size	= 64,
 		.page_size	= 256,
+		.feature_bits	= FEATURE_SPI_FAST_READ | FEATURE_SPI_FAST_READ_DUAL_O | FEATURE_SPI_FAST_READ_DUAL_IO,
 		.tested		= TEST_UNTESTED,
 		.probe		= probe_spi_rdid4,
 		.probe_timing	= TIMING_ZERO,
@@ -313,6 +314,7 @@ 
 		.model_id	= AMIC_A25L05PU,
 		.total_size	= 64,
 		.page_size	= 256,
+		.feature_bits	= FEATURE_SPI_FAST_READ | FEATURE_SPI_FAST_READ_DUAL_O | FEATURE_SPI_FAST_READ_DUAL_IO,
 		.tested		= TEST_UNTESTED,
 		.probe		= probe_spi_rdid4,
 		.probe_timing	= TIMING_ZERO,
@@ -344,6 +346,7 @@ 
 		.model_id	= AMIC_A25L10PT,
 		.total_size	= 128,
 		.page_size	= 256,
+		.feature_bits	= FEATURE_SPI_FAST_READ | FEATURE_SPI_FAST_READ_DUAL_O | FEATURE_SPI_FAST_READ_DUAL_IO,
 		.tested		= TEST_UNTESTED,
 		.probe		= probe_spi_rdid4,
 		.probe_timing	= TIMING_ZERO,
@@ -376,6 +379,7 @@ 
 		.model_id	= AMIC_A25L10PU,
 		.total_size	= 128,
 		.page_size	= 256,
+		.feature_bits	= FEATURE_SPI_FAST_READ | FEATURE_SPI_FAST_READ_DUAL_O | FEATURE_SPI_FAST_READ_DUAL_IO,
 		.tested		= TEST_UNTESTED,
 		.probe		= probe_spi_rdid4,
 		.probe_timing	= TIMING_ZERO,
@@ -408,6 +412,7 @@ 
 		.model_id	= AMIC_A25L20PT,
 		.total_size	= 256,
 		.page_size	= 256,
+		.feature_bits	= FEATURE_SPI_FAST_READ | FEATURE_SPI_FAST_READ_DUAL_O | FEATURE_SPI_FAST_READ_DUAL_IO,
 		.tested		= TEST_UNTESTED,
 		.probe		= probe_spi_rdid4,
 		.probe_timing	= TIMING_ZERO,
@@ -440,6 +445,7 @@ 
 		.model_id	= AMIC_A25L20PU,
 		.total_size	= 256,
 		.page_size	= 256,
+		.feature_bits	= FEATURE_SPI_FAST_READ | FEATURE_SPI_FAST_READ_DUAL_O | FEATURE_SPI_FAST_READ_DUAL_IO,
 		.tested		= TEST_UNTESTED,
 		.probe		= probe_spi_rdid4,
 		.probe_timing	= TIMING_ZERO,