Skip to contentSkip to conversation

CFI Driver: fix "flash not ready" problem

PATCH New Wolfgang Denk 3 participants 3 messages Oct 31, 2008 Memory technology devices (MTD)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 CFI Driver: fix "flash not ready" problem

From: Wolfgang Denk <[email protected]>

This patch fixes a problem on systems where the NOR flash is attached to a 64 bit bus. The toggle bit detection in flash_toggle() is based on the assumption that the same flash address is read twice without any other interjacent flash accesses. However, on 32 bit systems the function flash_read64() [as currently implemented] does not perform an atomic 64 bit read - instead, this is broken down into two 32 bit read accesses on addresses "addr" and "addr + 4". So instead of reading a 64 bit value twice from "addr", we see a sequence of 4 32 bit reads from "addr", "addr + 4", "addr", and "addr + 4". The consequence is that flash_toggle() fails to work.

This patch implements a simple, but somewhat ugly solution, as it avoids the use of flash_read64() in this critical place (by breaking it down manually into 32 bit read operations) instead of rewriting flash_read64() such to perform atomic 64 bit reads as one could expect. However, such a rewrite would require the use of floating point load operations, which becomes pretty complex:

	save MSR;
	set Floating Point Enable bit in MSR;
	use "lfd" instruction to perform atomic 64 bit read;
	use "stfd" to store value to temporary variable on stack;
	load u64 value from temporary variable;
	restore saved MSR;
	return u64 value;

The benefit-cost ratio of such an implementation was considered too bad to actually attempt this, especially as we can expect that such an implementation would not only have a bigger memory footprint but also cause a performance degradation.

---
 drivers/mtd/cfi_flash.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/mtd/cfi_flash.c +2 -1
index c40bf66..24e9b9f 100644
@@ -596,7 +596,8 @@ static int flash_toggle (flash_info_t * info, flash_sect_t sect,
 		retval = flash_read32(addr) != flash_read32(addr);
 		break;
 	case FLASH_CFI_64BIT:
-		retval = flash_read64(addr) != flash_read64(addr);
+		retval = ( (flash_read32( addr ) != flash_read32( addr )) ||
+			   (flash_read32(addr+4) != flash_read32(addr+4)) );
 		break;
 	default:
 		retval = 0;
Signature · 1 line
1.5.4.rc1.23.g3a969
Applied
#1 lore raw

PATCH 1/1 CFI Driver: fix "flash not ready" problem

Quoted 1 line · Wolfgang Denk

From: Wolfgang Denk <[email protected]>

Applied to cfi/master. Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================

#2 lore raw

PATCH 1/1 CFI Driver: fix "flash not ready" problem

In reply to Wolfgang Denk
Quoted 1 line · Wolfgang Denk

From: Wolfgang Denk <[email protected]>

. . .

Quoted 6 lines

This patch implements a simple, but somewhat ugly solution, as it avoids the use of flash_read64() in this critical place (by breaking it down manually into 32 bit read operations) instead of rewriting flash_read64() such to perform atomic 64 bit reads as one could expect. However, such a rewrite would require the use of floating point load operations, which becomes pretty complex:

. . .

Minor suggestion...I'd drop the "However, ". It makes the reader think that sentence is about *not* doing what the first sentence just said the patch does.

Tom

Signature · 4 lines
A: Because it breaks the logical        |
    flow of the message.                |   Email to user 'CTZ001'
                                        |             at 'email.mot.com'
Q: Why is top posting frowned upon?     |