[U-Boot] [PATCH] MTD/SPI/FLASH: add support for Ramtron FRAMs using SPI

[Mike Frysinger <vapier@gentoo.org>]

On Thursday, August 26, 2010 16:00:50 Reinhard Meyer wrote:
> 1. according to Ramtron, 0x7f is a continuation byte defined in a
> JEDEC standard (I could not find a spec for that) and shall be
> ignored until a non-0x7f shows.

yep
http://www.jedec.org/standards-documents/results/taxonomy%3A3127

> That shall be Manufacturer Id (0xc2 for Ramtron), followed by the 2 device
> id bytes. Following that method, the spi_flash.c should do that and use the
> first non-0x7f value for the switch statement. The switch would then have a
> case 0xc2 to call the ramtron-specific code.
>
> about like (rough sketch):
> for (index=0; index < IDLENGTH-3 && idcode[index] == 0x7f; index++)
> 	;
> /*
>   * here we are on the first non-0x7f byte or still on one,
>   * the switch will sort that out...
>   */
> switch (idcode[index]) {
> case 0xc2: flash = spi_fram_probe_ramtron(spi, idcode+index);
> /* the function will only access its parameter idcode with index 1 and 2 */
> etc...
> 
> default: /* covers the 0x7f case as well */

interesting, but what if we push it further.  something like this (untested):

--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -96,6 +96,31 @@
 	return ret;
 }
 
+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, },
+#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)
 {
@@ -124,46 +149,34 @@ struct spi_flash *spi_flash_probe(unsigned int bus, 
unsigned int cs,
 	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;
-#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;
+	flash = NULL;
+	if (idcode[0] == 0xff) {
+		/* handle non-JEDEC flashes */
+#ifdef CONFIG_SPI_FLASH_STMICRO
+		flash = spi_flash_probe_stmicro(spi, idcode);
+#endif
+	} else {
+		int i, j;
+		for (i = 0; i < ARRAY_SIZE(flashes); ++i) {
+			/* See if we have any known manufacturers */
+			if (idcode[flashes[i].shift] != flashes[i].idcode)
+				continue;
+
+			/* Make sure the ID was jedec extended */
+			j = flashes[i].shift - 1;
+			while (j >= 0 && idcode[j] == 0x7f)
+				continue;
+			if (j == -1) {
+				flash = flashes[i].probe(spi, idcode);
+				break;
+			}
+		}
 	}
 
-	if (!flash)
+	if (!flash) {
+		printf("SF: Unsupported manufacturer %02X\n", idcode[0]);
 		goto err_manufacturer_probe;
+	}
 
 	spi_release_bus(spi);
 

you should now be able to add an entry to the table like:
	{ 6, 0xc2, spi_fram_probe_ramtron, },

> 2. the switch case 0xff would occur twice, if someone had defined
> STMICRO and RAMTRON. I would postulate here that it is not allowed
> to define both and issue an #error.

right

> 3. If someone decided to put the env into FRAM, the env-var describing
> the nonstandard type would not be there (yet). So this should have
> another solution. Since because of 2. another non-standard SPI device
> should not be expected - I would, after accessing the status register to
> verify something FRAM-like is there, use a CONFIG_FRAM_DEVICE to
> define the chip to be assumed. That would work for us, since there
> is only one non-standard FRAM in the list anyway.

this is what the default env is for ...

you hit a similar problem with CONFIG_SYS_CONSOLE_IS_IN_ENV
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20100828/0abaf518/attachment.pgp