[flashrom] [PATCH 4/5] Generify probe_spi_rdid_generic() and add probe_spi_rdid_edi().

Stefan Tauner stefan.tauner at student.tuwien.ac.at
Wed Oct 3 06:13:04 CEST 2012


Previously the "generic" function was working/used for exactly two
cases, namely for manufacturer IDs with 0 or 1 continuation bytes.
For this behavior it was overly complicated.

The new implementation handles up to 3 continuation bytes
automatically and allows for up to 4 model bytes which is directly
used to add support for an EDI probing function. The probe_spi_rdid4()
function is removed because it is no longer needed.

The new probe_spi_rdid_generic() tries to figure out the maximum
allowed read size by using the recently added check_trans() function
to reduce the request size if necessary. While this does not make
detection to succeed in all theoretical situations, it improves the
code and will hopefully work in all realworld situations.

Signed-off-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>
---
 flashchips.c |   24 +++++++--------
 flashchips.h |    2 +-
 spi25.c      |   97 ++++++++++++++++++++++++++++++----------------------------
 3 files changed, 63 insertions(+), 60 deletions(-)

diff --git a/flashchips.c b/flashchips.c
index e290e2f..a69d899 100644
--- a/flashchips.c
+++ b/flashchips.c
@@ -547,7 +547,7 @@ const struct flashchip flashchips[] = {
 		.page_size	= 256,
 		.feature_bits	= FEATURE_WRSR_WREN,
 		.tested		= TEST_UNTESTED,
-		.probe		= probe_spi_rdid4,
+		.probe		= probe_spi_rdid,
 		.probe_timing	= TIMING_ZERO,
 		.block_erasers	=
 		{
@@ -581,7 +581,7 @@ const struct flashchip flashchips[] = {
 		.page_size	= 256,
 		.feature_bits	= FEATURE_WRSR_WREN,
 		.tested		= TEST_UNTESTED,
-		.probe		= probe_spi_rdid4,
+		.probe		= probe_spi_rdid,
 		.probe_timing	= TIMING_ZERO,
 		.block_erasers	=
 		{
@@ -615,7 +615,7 @@ const struct flashchip flashchips[] = {
 		.page_size	= 256,
 		.feature_bits	= FEATURE_WRSR_WREN,
 		.tested		= TEST_UNTESTED,
-		.probe		= probe_spi_rdid4,
+		.probe		= probe_spi_rdid,
 		.probe_timing	= TIMING_ZERO,
 		.block_erasers	=
 		{
@@ -650,7 +650,7 @@ const struct flashchip flashchips[] = {
 		.page_size	= 256,
 		.feature_bits	= FEATURE_WRSR_WREN,
 		.tested		= TEST_UNTESTED,
-		.probe		= probe_spi_rdid4,
+		.probe		= probe_spi_rdid,
 		.probe_timing	= TIMING_ZERO,
 		.block_erasers	=
 		{
@@ -685,7 +685,7 @@ const struct flashchip flashchips[] = {
 		.page_size	= 256,
 		.feature_bits	= FEATURE_WRSR_WREN,
 		.tested		= TEST_UNTESTED,
-		.probe		= probe_spi_rdid4,
+		.probe		= probe_spi_rdid,
 		.probe_timing	= TIMING_ZERO,
 		.block_erasers	=
 		{
@@ -720,7 +720,7 @@ const struct flashchip flashchips[] = {
 		.page_size	= 256,
 		.feature_bits	= FEATURE_WRSR_WREN,
 		.tested		= TEST_UNTESTED,
-		.probe		= probe_spi_rdid4,
+		.probe		= probe_spi_rdid,
 		.probe_timing	= TIMING_ZERO,
 		.block_erasers	=
 		{
@@ -760,7 +760,7 @@ const struct flashchip flashchips[] = {
 		.page_size	= 256,
 		.feature_bits	= FEATURE_WRSR_WREN,
 		.tested		= TEST_OK_PR,
-		.probe		= probe_spi_rdid4,
+		.probe		= probe_spi_rdid,
 		.probe_timing	= TIMING_ZERO,
 		.block_erasers	=
 		{
@@ -795,7 +795,7 @@ const struct flashchip flashchips[] = {
 		.page_size	= 256,
 		.feature_bits	= FEATURE_WRSR_WREN,
 		.tested		= TEST_OK_PR,
-		.probe		= probe_spi_rdid4,
+		.probe		= probe_spi_rdid,
 		.probe_timing	= TIMING_ZERO,
 		.block_erasers	=
 		{
@@ -830,7 +830,7 @@ const struct flashchip flashchips[] = {
 		.page_size	= 256,
 		.feature_bits	= FEATURE_WRSR_WREN,
 		.tested		= TEST_OK_PRE,
-		.probe		= probe_spi_rdid4,
+		.probe		= probe_spi_rdid,
 		.probe_timing	= TIMING_ZERO,
 		.block_erasers	=
 		{
@@ -865,7 +865,7 @@ const struct flashchip flashchips[] = {
 		.page_size	= 256,
 		.feature_bits	= FEATURE_WRSR_WREN,
 		.tested		= TEST_UNTESTED,
-		.probe		= probe_spi_rdid4,
+		.probe		= probe_spi_rdid,
 		.probe_timing	= TIMING_ZERO,
 		.block_erasers	=
 		{
@@ -903,7 +903,7 @@ const struct flashchip flashchips[] = {
 		.page_size	= 256,
 		.feature_bits	= FEATURE_WRSR_WREN,
 		.tested		= TEST_OK_PR,
-		.probe		= probe_spi_rdid4,
+		.probe		= probe_spi_rdid,
 		.probe_timing	= TIMING_ZERO,
 		.block_erasers	=
 		{
@@ -9752,7 +9752,7 @@ const struct flashchip flashchips[] = {
 		.total_size	= 0,
 		.page_size	= 256,
 		.tested		= TEST_BAD_PREW,
-		.probe		= probe_spi_rdid4,
+		.probe		= probe_spi_rdid,
 		.probe_timing	= TIMING_ZERO,
 		.write		= NULL,
 		.read		= NULL,
diff --git a/flashchips.h b/flashchips.h
index 8e51d35..d353b9e 100644
--- a/flashchips.h
+++ b/flashchips.h
@@ -91,7 +91,7 @@
 #define AMD_AM29LV800BT		0xDA	/* Same as Am29LV800DT */
 
 #define AMIC_ID			0x7F37	/* AMIC */
-#define AMIC_ID_NOPREFIX	0x37	/* AMIC */
+#define AMIC_ID_NOPREFIX	0x37	/* not AMIC, but used nevertheless in some AMIC chips */
 #define AMIC_A25L05PT		0x2020
 #define AMIC_A25L05PU		0x2010
 #define AMIC_A25L10PT		0x2021
diff --git a/spi25.c b/spi25.c
index a65f548..6310cf0 100644
--- a/spi25.c
+++ b/spi25.c
@@ -35,10 +35,13 @@ static int spi_rdid(struct flashctx *flash, unsigned char *readarr, int bytes)
 	int ret;
 	int i;
 
+	ret = flash->pgm->spi.check_trans(flash, sizeof(cmd), bytes, cmd);
+	if (ret)
+		return ret;
 	ret = spi_send_command(flash, sizeof(cmd), bytes, cmd, readarr);
 	if (ret)
 		return ret;
-	msg_cspew("RDID returned");
+	msg_cspew("RDID returned %d bytes:", bytes);
 	for (i = 0; i < bytes; i++)
 		msg_cspew(" 0x%02x", readarr[i]);
 	msg_cspew(". ");
@@ -115,37 +118,54 @@ int spi_write_disable(struct flashctx *flash)
 	return spi_send_command(flash, sizeof(cmd), 0, cmd, NULL);
 }
 
-static int probe_spi_rdid_generic(struct flashctx *flash, int bytes)
+/* The model_bytes parameter indicates the number of bytes of the model ID that have to match.
+ * The current implementation does work for manufacturer IDs with up to 3 continuation bytes and up to 4 bytes
+ * for the model identifier. */
+static int probe_spi_rdid_generic(struct flashctx *flash, int model_bytes)
 {
+	int i, j;
 	const struct flashchip *chip = flash->chip;
-	unsigned char readarr[4];
-	uint32_t id1;
-	uint32_t id2;
-
-	if (spi_rdid(flash, readarr, bytes)) {
-		return 0;
-	}
-
-	if (!oddparity(readarr[0]))
-		msg_cdbg("RDID byte 0 parity violation. ");
-
-	/* Check if this is a continuation vendor ID.
-	 * FIXME: Handle continuation device IDs.
-	 */
-	if (readarr[0] == 0x7f) {
-		if (!oddparity(readarr[1]))
-			msg_cdbg("RDID byte 1 parity violation. ");
-		id1 = (readarr[0] << 8) | readarr[1];
-		id2 = readarr[2];
-		if (bytes > 3) {
-			id2 <<= 8;
-			id2 |= readarr[3];
+	uint32_t id1 = 0;
+	uint32_t id2 = 0;
+
+	/* As of 2012-09 (JEP106AJ) there exist 8 banks, so we need at most 8 bytes to identify a manufacturer:
+	 * 7x continuation indicators (0x7F) + one ID byte. The problem is that a number of programmers do not
+	 * support arbitrary transaction lenghts. This implementation does not support more than 3 continuation
+	 * bytes because we use 32 bits for the ID and currently do not need more than banks 1-2 anyway. */
+	const int MAX_RD = 4 + model_bytes;
+	uint8_t readarr[MAX_RD];
+	i = MAX_RD;
+	do {
+		int ret = spi_rdid(flash, readarr, i);
+		if (ret == 0)
+			break;
+		if (ret != SPI_INVALID_LENGTH)
+			return ret;
+		/* We need at least 1 vendor + n chip bytes, less does not make sense */
+		if (i <= 1 + model_bytes) {
+			msg_cinfo("%d byte RDID not supported on this SPI controller.\n", i);
+			return ret;
+		}
+	} while (i-- > 0);
+
+	i = 0;
+	while (i < MAX_RD) {
+		uint8_t tmp = readarr[i];
+		i++;
+		id1 = (id1 << 8) | tmp;
+		if (tmp != 0x7f) {
+			if (!oddparity(tmp)) {
+				msg_cdbg("Manufacturer ID parity violation at RDID byte %i. ID so far: 0x%x.\n",
+					 i - 1, id1);
+				return 0;
+			}
+			break;
 		}
-	} else {
-		id1 = readarr[0];
-		id2 = (readarr[1] << 8) | readarr[2];
 	}
 
+	for (j = 0; j < model_bytes; j++)
+		id2 = (id2 << 8) | readarr[i + j];
+
 	msg_cdbg("%s: id1 0x%02x, id2 0x%02x\n", __func__, id1, id2);
 
 	if (id1 == chip->manufacture_id && id2 == chip->model_id) {
@@ -170,29 +190,12 @@ static int probe_spi_rdid_generic(struct flashctx *flash, int bytes)
 
 int probe_spi_rdid(struct flashctx *flash)
 {
-	return probe_spi_rdid_generic(flash, 3);
+	return probe_spi_rdid_generic(flash, 2);
 }
 
-int probe_spi_rdid4(struct flashctx *flash)
+int probe_spi_rdid_edi(struct flashctx *flash)
 {
-	/* Some SPI controllers do not support commands with writecnt=1 and
-	 * readcnt=4.
-	 */
-	switch (flash->pgm->spi.type) {
-#if CONFIG_INTERNAL == 1
-#if defined(__i386__) || defined(__x86_64__)
-	case SPI_CONTROLLER_IT87XX:
-	case SPI_CONTROLLER_WBSIO:
-		msg_cinfo("4 byte RDID not supported on this SPI controller\n");
-		return 0;
-		break;
-#endif
-#endif
-	default:
-		return probe_spi_rdid_generic(flash, 4);
-	}
-
-	return 0;
+	return probe_spi_rdid_generic(flash, 4);
 }
 
 int probe_spi_rems(struct flashctx *flash)
-- 
Kind regards, Stefan Tauner





More information about the flashrom mailing list