diff mbox

[U-Boot,V5,1/2] MTD/SPI: introduce table driven probing

Message ID 1286290600-18988-2-git-send-email-u-boot@emk-elektronik.de
State Accepted
Delegated to: Mike Frysinger
Headers show

Commit Message

Reinhard Meyer Oct. 5, 2010, 2:56 p.m. UTC 
for JEDEC devices without and with extension bytes
for non JEDEC devices
Signed-off-by: Reinhard Meyer <u-boot@emk-elektronik.de>
---
 drivers/mtd/spi/spi_flash.c |  137 +++++++++++++++++++++++++++++--------------
 1 files changed, 93 insertions(+), 44 deletions(-)

Comments

Mike Frysinger Oct. 11, 2010, 7:28 a.m. UTC | #1 
On Tuesday, October 05, 2010 10:56:39 Reinhard Meyer wrote:
> +	/*
> +	 * count the number of continuation bytes, but
> +	 * leave at least 3 bytes to the end of the buffer untouched
> +	 */
> +	for (shift = 0, idp = idbuf;
> +		shift < (sizeof(idbuf) - 3) && *idp == 0x7f;
> +			shift++, idp++)
> +		;
> +
> +	/* search the table for matches in shift and id */
> +	for (i = 0; i < ARRAY_SIZE(flashes); i++) {
> +		if (flashes[i].shift == shift && flashes[i].idcode == *idp) {
> +			/* we have a match, call probe */
> +			flash = flashes[i].probe(spi, idp);
> +			if (flash)
> +				break;
> +		}
>  	}

thinking about this some more, i see two problems.  (1) we've been providing 4 
bytes to probe funcs (like the spansion), so 3 is too small.  (2) all of the 
probe devices today expect to get the whole idcode buffer.  so if we're going 
to change this API, we need to fix all the probe funcs too.  but that is a lot 
of changes to roll into one commit, so let's first focus on converting to a 
table without changing any probe semantics.

see the follow up patch that addresses these issues and some other stuff.
-mike
Reinhard Meyer Oct. 11, 2010, 7:56 a.m. UTC | #2 
Dear Mike Frysinger,
> On Tuesday, October 05, 2010 10:56:39 Reinhard Meyer wrote:
>> +	/*
>> +	 * count the number of continuation bytes, but
>> +	 * leave at least 3 bytes to the end of the buffer untouched
>> +	 */
>> +	for (shift = 0, idp = idbuf;
>> +		shift < (sizeof(idbuf) - 3) && *idp == 0x7f;
>> +			shift++, idp++)
>> +		;
>> +
>> +	/* search the table for matches in shift and id */
>> +	for (i = 0; i < ARRAY_SIZE(flashes); i++) {
>> +		if (flashes[i].shift == shift && flashes[i].idcode == *idp) {
>> +			/* we have a match, call probe */
>> +			flash = flashes[i].probe(spi, idp);
>> +			if (flash)
>> +				break;
>> +		}
>>  	}
> 
> thinking about this some more, i see two problems.  (1) we've been providing 4 
> bytes to probe funcs (like the spansion), so 3 is too small.  (2) all of the 
> probe devices today expect to get the whole idcode buffer.  so if we're going 
> to change this API, we need to fix all the probe funcs too.  but that is a lot 
> of changes to roll into one commit, so let's first focus on converting to a 
> table without changing any probe semantics.

NONE of the existing functions work with expansion, so they all get the unshifted,
full ID buffer.

If you provide the unshifted buffer to the function, the function should get the
shift amount as well. Otherwise the function will again have look how many 0x7e's
to skip....

Reinhard
diff mbox

Patch

diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index ea875dc..e19f47d 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -1,8 +1,26 @@ 
 /*
- * SPI flash interface
+ * (C) Copyright 2010
+ * Reinhard Meyer, EMK Elektronik, reinhard.meyer@emk-elektronik.de
  *
  * Copyright (C) 2008 Atmel Corporation
- * Licensed under the GPL-2 or later.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
  */
 
 #include <common.h>
@@ -96,13 +114,59 @@  int spi_flash_read_common(struct spi_flash *flash, const u8 *cmd,
 	return ret;
 }
 
+/*
+ * the following table holds all device probe functions
+ *
+ * shift:  number of continuation bytes before the ID
+ * idcode: the expected IDCODE or 0xff for non JEDEC devices
+ * probe:  the function to call
+ *
+ * non JEDEC devices should be ordered in the table such that
+ * the probe functions with best detection algorithms come first
+ *
+ * several matching entries are permitted, they will be tried
+ * in sequence until a probe function returns non NULL
+ *
+ * IDBUF_LEN may be redefined if a device detection requires
+ * more then 5 bytes read.
+ */
+#define IDBUF_LEN 5
+const struct {
+	const u8 shift;
+	const u8 idcode;
+	struct spi_flash *(*probe) (struct spi_slave *spi, u8 *idcode);
+} flashes[] = {
+#ifdef CONFIG_SPI_FLASH_SPANSION
+	{ 0, 0x01, spi_flash_probe_spansion, },
+#endif
+#ifdef CONFIG_SPI_FLASH_ATMEL
+	{ 0, 0x1F, spi_flash_probe_atmel, },
+#endif
+#ifdef CONFIG_SPI_FLASH_MACRONIX
+	{ 0, 0xc2, spi_flash_probe_macronix, },
+#endif
+#ifdef CONFIG_SPI_FLASH_WINBOND
+	{ 0, 0xef, spi_flash_probe_winbond, },
+#endif
+#ifdef CONFIG_SPI_FLASH_STMICRO
+	{ 0, 0x20, spi_flash_probe_stmicro, },
+	{ 0, 0xff, spi_flash_probe_stmicro, },
+#endif
+#ifdef CONFIG_SPI_FLASH_SST
+	{ 0, 0xBF, spi_flash_probe_sst, },
+#endif
+};
+
 struct spi_flash *spi_flash_probe(unsigned int bus, unsigned int cs,
 		unsigned int max_hz, unsigned int spi_mode)
 {
 	struct spi_slave *spi;
-	struct spi_flash *flash;
+	struct spi_flash *flash = NULL;
 	int ret;
-	u8 idcode[5];
+	int shift;
+	int i;
+	u8 idbuf[IDBUF_LEN];
+	u8 *idp;
 
 	spi = spi_setup_slave(bus, cs, max_hz, spi_mode);
 	if (!spi) {
@@ -117,53 +181,38 @@  struct spi_flash *spi_flash_probe(unsigned int bus, unsigned int cs,
 	}
 
 	/* Read the ID codes */
-	ret = spi_flash_cmd(spi, CMD_READ_ID, &idcode, sizeof(idcode));
+	ret = spi_flash_cmd(spi, CMD_READ_ID, &idbuf, sizeof(idbuf));
 	if (ret)
 		goto err_read_id;
 
-	debug("SF: Got idcode %02x %02x %02x %02x %02x\n", idcode[0],
-			idcode[1], idcode[2], idcode[3], idcode[4]);
-
-	switch (idcode[0]) {
-#ifdef CONFIG_SPI_FLASH_SPANSION
-	case 0x01:
-		flash = spi_flash_probe_spansion(spi, idcode);
-		break;
-#endif
-#ifdef CONFIG_SPI_FLASH_ATMEL
-	case 0x1F:
-		flash = spi_flash_probe_atmel(spi, idcode);
-		break;
-#endif
-#ifdef CONFIG_SPI_FLASH_MACRONIX
-	case 0xc2:
-		flash = spi_flash_probe_macronix(spi, idcode);
-		break;
-#endif
-#ifdef CONFIG_SPI_FLASH_WINBOND
-	case 0xef:
-		flash = spi_flash_probe_winbond(spi, idcode);
-		break;
-#endif
-#ifdef CONFIG_SPI_FLASH_STMICRO
-	case 0x20:
-	case 0xff: /* Let the stmicro func handle non-JEDEC ids */
-		flash = spi_flash_probe_stmicro(spi, idcode);
-		break;
+#ifdef DEBUG
+	printf("SF: Got idcodes\n");
+	print_buffer(0, idbuf, 1, sizeof(idbuf), 0);
 #endif
-#ifdef CONFIG_SPI_FLASH_SST
-	case 0xBF:
-		flash = spi_flash_probe_sst(spi, idcode);
-		break;
-#endif
-	default:
-		printf("SF: Unsupported manufacturer %02X\n", idcode[0]);
-		flash = NULL;
-		break;
+
+	/*
+	 * count the number of continuation bytes, but
+	 * leave at least 3 bytes to the end of the buffer untouched
+	 */
+	for (shift = 0, idp = idbuf;
+		shift < (sizeof(idbuf) - 3) && *idp == 0x7f;
+			shift++, idp++)
+		;
+
+	/* search the table for matches in shift and id */
+	for (i = 0; i < ARRAY_SIZE(flashes); i++) {
+		if (flashes[i].shift == shift && flashes[i].idcode == *idp) {
+			/* we have a match, call probe */
+			flash = flashes[i].probe(spi, idp);
+			if (flash)
+				break;
+		}
 	}
 
-	if (!flash)
+	if (!flash) {
+		printf("SF: Unsupported manufacturer %02X\n", *idp);
 		goto err_manufacturer_probe;
+	}
 
 	spi_release_bus(spi);