[RFC] MTD m25p80 3-byte addressing and boot problem

[RFC] MTD m25p80 3-byte addressing and boot problem

[Matteo Fortini]

If a Linux/Barebox system is using an SPI flash of size >= 16 MB, the 
driver is switching to 3-byte addressing to be able to use linear access 
to the whole memory.

This leads to the impossibility to boot a board after a warm reset, 
because all bootloaders use the standard 2-byte addressing (if the board 
doesn't physically reset the flash or do something similar).

Some documentation in the following links:
http://www.at91.com/discussions/viewtopic.php/f,30/t,22849.html
https://community.freescale.com/docs/DOC-93632

The solution proposed on freescale forums is not final: it involves 
switching back to 2-byte addressing after every access, which still 
leaves a small window in which a warm reset would be fatal.

One solution would be to use the bank command in the flash, using each 
16MB bank linearly, and changing bank depending on the address. This 
would be messy for accesses which are crossing the boundary, but it is 
doable.

I'm asking here for comments before I start patching the code. Right now 
I resorted to stick to 2-byte addressing and leave half of my NOR (32MB) 
unused.

TIA,
M

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [RFC] MTD m25p80 3-byte addressing and boot problem

[Geert Uytterhoeven]

Hi Matteo,

On Mon, Sep 1, 2014 at 11:43 AM, Matteo Fortini
<matteo.fortini@gmail.com> wrote:
> If a Linux/Barebox system is using an SPI flash of size >= 16 MB, the driver
> is switching to 3-byte addressing to be able to use linear access to the
> whole memory.
>
> This leads to the impossibility to boot a board after a warm reset, because
> all bootloaders use the standard 2-byte addressing (if the board doesn't
> physically reset the flash or do something similar).

> The solution proposed on freescale forums is not final: it involves
> switching back to 2-byte addressing after every access, which still leaves a
> small window in which a warm reset would be fatal.

Indeed. So the board won't boot anymore if that happens.

> One solution would be to use the bank command in the flash, using each 16MB
> bank linearly, and changing bank depending on the address. This would be
> messy for accesses which are crossing the boundary, but it is doable.

IMHO this is even more dangerous: what if the board is warm-resetted
while a non-first bank is in use? The board may not boot anymore, but worse,
if the boot loader would write to the FLASH, it would write to the wrong bank,
overwriting innocent data.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [RFC] MTD m25p80 3-byte addressing and boot problem

[Mark Marshall]

[-- Attachment #1: Type: text/plain, Size: 2569 bytes --]

Hi.

We had the same problem here, but luckily our Flash also supported a
different read command that took the larger address.  I made a (small)
modification to the m25p80 driver to use this command when the flash
supported it.

I've attached the two patches that come from my tree that are
relevant.  The first one (42)  just changes to the page size used to
access the flash, and is only needed so that you don;t get conflicts
wit the second.  The second patch (45) is the one of interest.  These
patches are against v3.2.52-rt73 from the real-time tree, and there
are (obviously) other patches either side of them, but they should
show what I have done.  At some point (!) I'll try to push these
up-stream, but as ou can see, we are well behind the curve at the
moment, which makes this harder.

Regards,

Mark Marshall.

PS.
On our original prototype hardware we had no reset line connected to
the flash, so even a hard reset wouldn't get things back to how they
should be when Linux was changing the bank register!  I think that the
longer commands should be used if the flash chip supports them in
preference to either bank switching or "converting" the small commands
to longer ones.


On 1 September 2014 11:43, Matteo Fortini <matteo.fortini@gmail.com> wrote:
> If a Linux/Barebox system is using an SPI flash of size >= 16 MB, the driver
> is switching to 3-byte addressing to be able to use linear access to the
> whole memory.
>
> This leads to the impossibility to boot a board after a warm reset, because
> all bootloaders use the standard 2-byte addressing (if the board doesn't
> physically reset the flash or do something similar).
>
> Some documentation in the following links:
> http://www.at91.com/discussions/viewtopic.php/f,30/t,22849.html
> https://community.freescale.com/docs/DOC-93632
>
> The solution proposed on freescale forums is not final: it involves
> switching back to 2-byte addressing after every access, which still leaves a
> small window in which a warm reset would be fatal.
>
> One solution would be to use the bank command in the flash, using each 16MB
> bank linearly, and changing bank depending on the address. This would be
> messy for accesses which are crossing the boundary, but it is doable.
>
> I'm asking here for comments before I start patching the code. Right now I
> resorted to stick to 2-byte addressing and leave half of my NOR (32MB)
> unused.
>
> TIA,
> M
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

[-- Attachment #2: 0042-m25p80-Use-a-512-byte-page-size-for-Spansion-flash-s.patch --]
[-- Type: text/x-patch, Size: 1941 bytes --]

From 9fe96f9fe893e4f8a42c37df5db1e29d5bab739e Mon Sep 17 00:00:00 2001
From: Mark Marshall <Mark.Marshall@omicron.at>
Date: Thu, 25 Apr 2013 13:50:30 +0200
Subject: [PATCH 42/96] m25p80: Use a 512 byte page size for Spansion flash
 s25fl512s

The s25fl512s flash from Spnasion has a 512 byte write page size,
which means that we can write 512 bytes at a time (instead of 256).

This single change makes writing to the flash about 2x's faster.

Signed-off-by: Mark Marshall <mark.marshall@omicron.at>
---
 drivers/mtd/devices/m25p80.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index b777697..3258179 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -637,6 +637,16 @@ struct flash_info {
 		.flags = (_flags),					\
 	})
 
+#define INFOP(_jedec_id, _ext_id, _sector_size, _n_sectors, _pg_sz, _flags) \
+	((kernel_ulong_t)&(struct flash_info) {				\
+		.jedec_id = (_jedec_id),				\
+		.ext_id = (_ext_id),					\
+		.sector_size = (_sector_size),				\
+		.n_sectors = (_n_sectors),				\
+		.page_size = (_pg_sz),					\
+		.flags = (_flags),					\
+	})
+
 #define CAT25_INFO(_sector_size, _n_sectors, _page_size, _addr_width)	\
 	((kernel_ulong_t)&(struct flash_info) {				\
 		.sector_size = (_sector_size),				\
@@ -698,7 +708,7 @@ static const struct spi_device_id m25p_ids[] = {
 	{ "s25sl064a",  INFO(0x010216,      0,  64 * 1024, 128, 0) },
 	{ "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
 	{ "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024, 512, 0) },
-	{ "s25fl512s",  INFO(0x010220, 0x4d00, 256 * 1024, 256, 0) },
+	{ "s25fl512s", INFOP(0x010220, 0x4d00, 256 * 1024, 256, 512, 0) },
 	{ "s70fl01gs",  INFO(0x010221, 0x4d00, 256 * 1024, 256, 0) },
 	{ "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024,  64, 0) },
 	{ "s25sl12801", INFO(0x012018, 0x0301,  64 * 1024, 256, 0) },
-- 
1.7.9.5


[-- Attachment #3: 0045-m25p80-Use-the-4-byte-address-read-command.patch --]
[-- Type: text/x-patch, Size: 4285 bytes --]

From 7c14b894f1884bf7915417ce8b747b11b6f603c6 Mon Sep 17 00:00:00 2001
From: Mark Marshall <Mark.Marshall@omicron.at>
Date: Wed, 31 Jul 2013 15:26:12 +0200
Subject: [PATCH 45/96] m25p80: Use the 4-byte address read command

It is better for some hardware platforms if we use the dedicated
4-byte address SPI read command when reading from SPI Flash chips
> 16 MB (rather than converting the 3-byte address command to take
4 address bytes).

The problem that we had is that on reset the boot loader tries to
use the 3-byte address command which, doesn't work if the Flash has
not been reset and Linux has converted it to accept 4-byte addresses.
---
 drivers/mtd/devices/m25p80.c |   36 +++++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index fa0f2c0..2d87aad 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -62,6 +62,11 @@
 #define OPCODE_OTP_WRITE	0x42	/* Write to the OTP region */
 #define OPCODE_OTP_READ		0x4b	/* Read from the OTP region */
 
+#define OPCODE_FAST_READ4B      0x0c	/* Read data, 4 byte addr (hi freq.) */
+#define OPCODE_NORM_READ4B      0x13	/* Read data, 4 byte addr (lo freq.) */
+#define OPCODE_PP4B		0x12	/* Page program (up to 256 bytes) */
+#define OPCODE_SE4B		0xdc	/* Sector erase (usually 256KiB) */
+
 /* Status Register bits. */
 #define	SR_WIP			1	/* Write in progress */
 #define	SR_WEL			2	/* Write enable latch */
@@ -77,9 +82,11 @@
 
 #ifdef CONFIG_M25PXX_USE_FAST_READ
 #define OPCODE_READ 	OPCODE_FAST_READ
+#define OPCODE_READ4B   OPCODE_FAST_READ4B
 #define FAST_READ_DUMMY_BYTE 1
 #else
 #define OPCODE_READ 	OPCODE_NORM_READ
+#define OPCODE_READ4B   OPCODE_NORM_READ4B
 #define FAST_READ_DUMMY_BYTE 0
 #endif
 
@@ -94,6 +101,8 @@ struct m25p {
 	u16			page_size;
 	u16			addr_width;
 	u8			erase_opcode;
+	u8			read_opcode;
+	u8			write_opcode;
 	u8			*command;
 };
 
@@ -173,10 +182,20 @@ static inline int set_4byte(struct m25p *flash, u32 jedec_id, int enable)
 		flash->command[0] = enable ? OPCODE_EN4B : OPCODE_EX4B;
 		return spi_write(flash->spi, flash->command, 1);
 	default:
-		/* Spansion style */
-		flash->command[0] = OPCODE_BRWR;
-		flash->command[1] = enable << 7;
-		return spi_write(flash->spi, flash->command, 2);
+		switch (jedec_id) {
+		case 0x010219: /* s25fl256s0/s25fl256s1 */
+		case 0x010220: /* s25fl512s */
+		case 0x010221: /* s70fl01gs */
+			flash->read_opcode = OPCODE_READ4B;
+			flash->write_opcode = OPCODE_PP4B;
+			flash->erase_opcode = OPCODE_SE4B;
+			return 0;
+		default:
+			/* Spansion style */
+			flash->command[0] = OPCODE_BRWR;
+			flash->command[1] = enable << 7;
+			return spi_write(flash->spi, flash->command, 2);
+		}
 	}
 }
 
@@ -402,7 +421,7 @@ static int m25p80_read(struct mtd_info *mtd, loff_t from, size_t len,
 		 */
 
 		/* Set up the write data buffer. */
-		flash->command[0] = OPCODE_READ;
+		flash->command[0] = flash->read_opcode;
 		m25p_addr2cmd(flash, from + done, flash->command);
 
 		spi_sync(flash->spi, &m);
@@ -462,7 +481,7 @@ static int m25p80_write(struct mtd_info *mtd, loff_t to, size_t len,
 	write_enable(flash);
 
 	/* Set up the opcode in the write buffer. */
-	flash->command[0] = OPCODE_PP;
+	flash->command[0] = flash->write_opcode;
 	m25p_addr2cmd(flash, to, flash->command);
 
 	page_offset = to & (flash->page_size - 1);
@@ -944,7 +963,7 @@ static const struct spi_device_id m25p_ids[] = {
 	{ "s25fl256s0", INFO(0x010219, 0x4d00, 256 * 1024, 128, 0) },
 	{ "s25fl256s1", INFO(0x010219, 0x4d01,  64 * 1024, 512, 0) },
 	{ "s25fl512s", INFOP(0x010220, 0x4d00, 256 * 1024, 256, 512, M25P_OTP) },
-	{ "s70fl01gs",  INFO(0x010221, 0x4d00, 256 * 1024, 256, 0) },
+	{ "s70fl01gs", INFOP(0x010221, 0x4d00, 256 * 1024, 256, 512, 0) },
 	{ "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024,  64, 0) },
 	{ "s25sl12801", INFO(0x012018, 0x0301,  64 * 1024, 256, 0) },
 	{ "s25fl129p0", INFO(0x012018, 0x4d00, 256 * 1024,  64, 0) },
@@ -1172,6 +1191,9 @@ static int __devinit m25p_probe(struct spi_device *spi)
 		flash->mtd.erasesize = info->sector_size;
 	}
 
+	flash->read_opcode = OPCODE_READ;
+	flash->write_opcode = OPCODE_PP;
+
 	if (info->flags & M25P_NO_ERASE)
 		flash->mtd.flags |= MTD_NO_ERASE;
 
-- 
1.7.9.5


[-- Attachment #4: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox