diff mbox

[U-Boot] cfi_flash: Fix detection of 8-bit bus flash devices via address shift

Message ID afa0d82c-3fa8-439d-861f-bdf485b2f779@DB8EHSMHS017.ehs.local
State Deferred
Delegated to: Stefan Roese
Headers show

Commit Message

Jagannadha Sutradharudu Teki June 7, 2013, 1:02 p.m. UTC 
We had a problem detecting 8/16bit flash devices connected only via
8bits to the SoC for quite a while. Commit 239cb9d9
[mtd: cfi_flash: Fix CFI flash driver for 8-bit bus support] finally
fixed this 8-bit bus support. But also broke some other boards using
this cfi driver. So this patch had to be reverted.

I spotted a different, simpler approach for this 8-bit bus support
on the barebox mailing list posted by
Oleksij Rempel <bug-track@fisher-privat.net>:

http://www.spinics.net/lists/u-boot-v2/msg14687.html

Here the commit text:

"
Many cfi chips support 16 and 8 bit modes. Most important
difference is use of so called "Q15/A-1" pin. In 16bit mode this
pin is used for data IO. In 8bit mode, it is an address input
which add one more least significant bit (LSB). In this case
we should shift all adresses by one:
For example 0xaa << 1 = 0x154
"

This patch now is a port of this barebox patch to U-Boot.

Along with the change w.r.t from barebox,
Some flash chips can support multiple bus widths, override the
interface width and limit it to the port width.

Tested on 16-bit Spansion flash on sequoia.
Tested 8-bit flashes like 256M29EW, 512M29EW.

Signed-off-by: Stefan Roese <sr@denx.de>
Tested-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
---
 drivers/mtd/cfi_flash.c | 37 +++++++++++++++++++++++++++++++++----
 include/flash.h         |  2 ++
 2 files changed, 35 insertions(+), 4 deletions(-)

Comments

Stefan Roese June 8, 2013, 9:46 a.m. UTC | #1 
Hi Wolfgang,

On 06/07/2013 03:02 PM, Jagannadha Sutradharudu Teki wrote:
> We had a problem detecting 8/16bit flash devices connected only via
> 8bits to the SoC for quite a while. Commit 239cb9d9
> [mtd: cfi_flash: Fix CFI flash driver for 8-bit bus support] finally
> fixed this 8-bit bus support. But also broke some other boards using
> this cfi driver. So this patch had to be reverted.
> 
> I spotted a different, simpler approach for this 8-bit bus support
> on the barebox mailing list posted by
> Oleksij Rempel <bug-track@fisher-privat.net>:
> 
> http://www.spinics.net/lists/u-boot-v2/msg14687.html
> 
> Here the commit text:
> 
> "
> Many cfi chips support 16 and 8 bit modes. Most important
> difference is use of so called "Q15/A-1" pin. In 16bit mode this
> pin is used for data IO. In 8bit mode, it is an address input
> which add one more least significant bit (LSB). In this case
> we should shift all adresses by one:
> For example 0xaa << 1 = 0x154
> "
> 
> This patch now is a port of this barebox patch to U-Boot.
> 
> Along with the change w.r.t from barebox,
> Some flash chips can support multiple bus widths, override the
> interface width and limit it to the port width.
> 
> Tested on 16-bit Spansion flash on sequoia.
> Tested 8-bit flashes like 256M29EW, 512M29EW.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> Tested-by: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>

Wolfgang, you remember that s few weeks ago a similar CFI patch resulted
in breaking flash support for one TQ board. Which one was that? Could
you please either test this patch on this board or let me know which
board this was? Then I'll do the testing next week.

Thanks,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: office@denx.de
Wolfgang Denk June 10, 2013, 2:04 p.m. UTC | #2 
Dear Stefan,

In message <51B2FD5A.4060702@denx.de> you wrote:
> 
> Wolfgang, you remember that s few weeks ago a similar CFI patch resulted
> in breaking flash support for one TQ board. Which one was that? Could
> you please either test this patch on this board or let me know which
> board this was? Then I'll do the testing next week.

This was on a custom system based on the TQM8541 SoM (which has been
removed from mainline, to it is OOT, unfortunately).

This new patch does not work correctly either:

Before (without this patch):

	U-Boot 2013.04-00517-gb657f46 (Jun 10 2013 - 15:57:13)

	CPU:   8541E, Version: 1.1, (0x807a0011)
	Core:  E500, Version: 2.0, (0x80200020)
	Clock Configuration:
	       CPU0:1000 MHz, 
	       CCB:333.333 MHz,
	       DDR:166.667 MHz (333.333 MT/s data rate), LBC:41.667 MHz
	CPM:   333.333 MHz
	L1:    D-cache 32 kB enabled
	       I-cache 32 kB enabled
	Board: TQM8541, serial# TQM8541GC0AD39-EAQFBC.0112 11871127
	I2C:   ready
	DRAM:  256 MiB
	Flash: 256 MiB
	...

	=> fli

	Bank # 1: CFI conformant flash (32 x 16)  Size: 128 MB in 512 Sectors
	  AMD Standard command set, Manufacturer ID: 0x01, Device ID: 0x227E2301
	  Erase timeout: 16384 ms, write timeout: 2 ms
	  Buffer write timeout: 5 ms, buffer size: 32 bytes

	  Sector Start Addresses:
	  F0000000        F0040000        F0080000        F00C0000        F0100000      
	  F0140000 E      F0180000 E      F01C0000 E      F0200000 E      F0240000 E    
	  F0280000 E      F02C0000 E      F0300000 E      F0340000 E      F0380000 E    
	...
	  F7D00000 E      F7D40000 E      F7D80000 E      F7DC0000 E      F7E00000 E    
	  F7E40000 E      F7E80000 E      F7EC0000 E      F7F00000 E      F7F40000 E    
	  F7F80000 E      F7FC0000 E    

	Bank # 2: CFI conformant flash (32 x 16)  Size: 128 MB in 512 Sectors
	  AMD Standard command set, Manufacturer ID: 0x01, Device ID: 0x227E2301
	  Erase timeout: 16384 ms, write timeout: 2 ms
	  Buffer write timeout: 5 ms, buffer size: 32 bytes

	  Sector Start Addresses:
	  F8000000 E      F8040000 E      F8080000 E      F80C0000 E      F8100000 E    
	  F8140000 E      F8180000 E      F81C0000 E      F8200000 E      F8240000 E    
	  F8280000 E      F82C0000 E      F8300000 E      F8340000 E      F8380000 E    
	...
	  FFD00000 E      FFD40000 E      FFD80000 E      FFDC0000 E      FFE00000 E    
	  FFE40000 E      FFE80000 E      FFEC0000 E      FFF00000   RO   FFF40000   RO 
	  FFF80000   RO   FFFC0000   RO 
	=> 

After applying the patch:

	U-Boot 2013.04-00518-g6678bb9 (Jun 10 2013 - 15:52:12)

	CPU:   8541E, Version: 1.1, (0x807a0011)
	Core:  E500, Version: 2.0, (0x80200020)
	Clock Configuration:
	       CPU0:1000 MHz, 
	       CCB:333.333 MHz,
	       DDR:166.667 MHz (333.333 MT/s data rate), LBC:41.667 MHz
	CPM:   333.333 MHz
	L1:    D-cache 32 kB enabled
	       I-cache 32 kB enabled
	Board: TQM8541, serial# TQM8541GC0AD39-EAQFBC.0112 11871127
	I2C:   ready
	DRAM:  256 MiB
	Flash: 128 MiB
	...
	=> fli

	Bank # 1: CFI conformant flash (16 x 16)  Size: 64 MB in 512 Sectors
	  AMD Standard command set, Manufacturer ID: 0x01, Device ID: 0x227E2301
	  Erase timeout: 16384 ms, write timeout: 2 ms
	  Buffer write timeout: 5 ms, buffer size: 32 bytes

	  Sector Start Addresses:
	  F8000000        F8020000        F8040000        F8060000        F8080000      
	  F80A0000        F80C0000        F80E0000        F8100000        F8120000      
	  F8140000 E      F8160000 E      F8180000 E      F81A0000 E      F81C0000 E    
	...
	  FBE80000 E      FBEA0000 E      FBEC0000 E      FBEE0000 E      FBF00000 E    
	  FBF20000 E      FBF40000 E      FBF60000 E      FBF80000 E      FBFA0000 E    
	  FBFC0000 E      FBFE0000 E    

	Bank # 2: CFI conformant flash (16 x 16)  Size: 64 MB in 512 Sectors
	  AMD Standard command set, Manufacturer ID: 0x01, Device ID: 0x227E2301
	  Erase timeout: 16384 ms, write timeout: 2 ms
	  Buffer write timeout: 5 ms, buffer size: 32 bytes

	  Sector Start Addresses:
	  FC000000 E      FC020000 E      FC040000 E      FC060000 E      FC080000 E    
	  FC0A0000 E      FC0C0000 E      FC0E0000 E      FC100000 E      FC120000 E    
	  FC140000 E      FC160000 E      FC180000 E      FC1A0000 E      FC1C0000 E    
	  FFE80000 E      FFEA0000 E      FFEC0000 E      FFEE0000 E      FFF00000   RO 
	  FFF20000 E RO   FFF40000   RO   FFF60000 E RO   FFF80000   RO   FFFA0000   RO 
	  FFFC0000 E RO   FFFE0000   RO 
	=> 

I. e. it detects only half the correct flash size (due to detecting
the flash as 16 x 16 configuration, which is wrong - it is actually a
32 x 16 one with 2 x 16 bit devices in parallel).

In the result, accessing the flash does not work correctly, either:

	=> erase fff80000 +80000

	.... done
	Erased 4 sectors
	=> cp.b 100000 fff80000 80000
	Copy to Flash... Flash not Erased


So NAK to this patch from my side.

Best regards,

Wolfgang Denk
Stefan Roese June 10, 2013, 2:10 p.m. UTC | #3 
Hi Wolfgang,

On 10.06.2013 16:04, Wolfgang Denk wrote:
> Dear Stefan,
> 
> In message <51B2FD5A.4060702@denx.de> you wrote:
>>
>> Wolfgang, you remember that s few weeks ago a similar CFI patch resulted
>> in breaking flash support for one TQ board. Which one was that? Could
>> you please either test this patch on this board or let me know which
>> board this was? Then I'll do the testing next week.
> 
> This was on a custom system based on the TQM8541 SoM (which has been
> removed from mainline, to it is OOT, unfortunately).

Thanks for testing. I'll try to find some time in the next days to debug
this issue and fix it up.

Thanks again,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: office@denx.de
diff mbox

Patch

diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index 22d8440..e23e394 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -212,7 +212,7 @@  flash_map (flash_info_t * info, flash_sect_t sect, uint offset)
 {
 	unsigned int byte_offset = offset * info->portwidth;
 
-	return (void *)(info->start[sect] + byte_offset);
+	return (void *)(info->start[sect] + (byte_offset << info->chip_lsb));
 }
 
 static inline void flash_unmap(flash_info_t *info, flash_sect_t sect,
@@ -1892,12 +1892,27 @@  static int __flash_detect_cfi (flash_info_t * info, struct cfi_qry *qry)
 			flash_read_cfi(info, qry, FLASH_OFFSET_CFI_RESP,
 					sizeof(struct cfi_qry));
 			info->interface	= le16_to_cpu(qry->interface_desc);
+			/* Some flash chips can support multiple bus widths.
+			 * In this case, override the interface width and
+			 * limit it to the port width.
+			 */
+			if ((info->interface == FLASH_CFI_X8X16) &&
+					(info->portwidth == FLASH_CFI_8BIT)) {
+				debug("Overriding 16-bit interface width to"
+						" 8-bit port width\n");
+				info->interface = FLASH_CFI_X8;
+			} else if ((info->interface == FLASH_CFI_X16X32) &&
+					(info->portwidth == FLASH_CFI_16BIT)) {
+				debug("Overriding 16-bit interface width to"
+						" 16-bit port width\n");
+				info->interface = FLASH_CFI_X16;
+			}
 
 			info->cfi_offset = flash_offset_cfi[cfi_offset];
 			debug ("device interface is %d\n",
 			       info->interface);
-			debug ("found port %d chip %d ",
-			       info->portwidth, info->chipwidth);
+			debug("found port %d chip %d chip_lsb %d ",
+			      info->portwidth, info->chipwidth, info->chip_lsb);
 			debug ("port %d bits chip %d bits\n",
 			       info->portwidth << CFI_FLASH_SHIFT_WIDTH,
 			       info->chipwidth << CFI_FLASH_SHIFT_WIDTH);
@@ -1937,9 +1952,23 @@  static int flash_detect_cfi (flash_info_t * info, struct cfi_qry *qry)
 	     info->portwidth <= FLASH_CFI_64BIT; info->portwidth <<= 1) {
 		for (info->chipwidth = FLASH_CFI_BY8;
 		     info->chipwidth <= info->portwidth;
-		     info->chipwidth <<= 1)
+		     info->chipwidth <<= 1) {
+			/*
+			 * First, try detection without shifting the addresses
+			 * for 8bit devices (16bit wide connection)
+			 */
+			info->chip_lsb = 0;
+			if (__flash_detect_cfi(info, qry))
+				return 1;
+
+			/*
+			 * Not detected, so let's try with shifting
+			 * for 8bit devices
+			 */
+			info->chip_lsb = 1;
 			if (__flash_detect_cfi(info, qry))
 				return 1;
+		}
 	}
 	debug ("not found\n");
 	return 0;
diff --git a/include/flash.h b/include/flash.h
index c7acc97..af2fe41 100644
--- a/include/flash.h
+++ b/include/flash.h
@@ -38,6 +38,8 @@  typedef struct {
 #ifdef CONFIG_SYS_FLASH_CFI
 	uchar	portwidth;		/* the width of the port		*/
 	uchar	chipwidth;		/* the width of the chip		*/
+	uchar	chip_lsb;		/* extra Least Significant Bit in the */
+					/* address of chip	*/
 	ushort	buffer_size;		/* # of bytes in write buffer		*/
 	ulong	erase_blk_tout;		/* maximum block erase timeout		*/
 	ulong	write_tout;		/* maximum write timeout		*/