[U-Boot] Question about M29W128G CFI QRY bug

[U-Boot] Question about M29W128G CFI QRY bug

[Richard Retanubun]

Hi Stefan,

I'm trying to get u-boot 2009.03 on coldfire to boot on flash part M29W128G.

I think I am getting hit by this bug in the flash chip

http://lists.infradead.org/pipermail/linux-mtd/2008-July/022252.html

The debug output is as follows:


U-Boot 2009.03dvl-00087-gd53876a-dirty (Apr 09 2009 - 19:00:37)

CPU:   Freescale ColdFire MCF5270 rev. 1, at 150 MHz
Board: Ruggedcom MCF5270
I2C:   ready
DRAM:   8 MB
Top of RAM usable for U-Boot at: 00800000
Reserving 172k for U-Boot at: 007d4000
Reserving 256k for malloc() at: 00794000
Reserving 58 Bytes for Board Info at: 00793fc6
Reserving 56 Bytes for Global Data at: 00793f8e
Reserving 64k for boot parameters at: 00783f8e
Stack Pointer at: 00783f68
Start relocate of code from ff000400 to 007d4000
Now running in RAM - U-Boot at: 007d4000
FLASH: flash detect cfi

fwc addr ff000000 cmd f0 f0f0 16bit x 8 bit
fwc addr ff000000 cmd ff ffff 16bit x 8 bit
fwc addr ff0000aa cmd 98 9898 16bit x 8 bit
is= cmd 51(Q) addr ff000020 is= ff00 5151
[snip]

Note that we suffer from the same bug, where QRY is not being setup properly.
The notes there offers a remedy by sending ff before f0. (or skipping ff altogether).

Am I on the right track here and if so, how can I make the same fix in u-boot?

Thanks for your time

- Richard


<Raw debug output>
U-Boot 2009.03dvl-00087-gd53876a-dirty (Apr 09 2009 - 19:00:37)

CPU:   Freescale ColdFire MCF5270 rev. 1, at 150 MHz
Board: Ruggedcom MCF5270
I2C:   ready
DRAM:   8 MB
Top of RAM usable for U-Boot at: 00800000
Reserving 172k for U-Boot at: 007d4000
Reserving 256k for malloc() at: 00794000
Reserving 58 Bytes for Board Info at: 00793fc6
Reserving 56 Bytes for Global Data at: 00793f8e
Reserving 64k for boot parameters at: 00783f8e
Stack Pointer at: 00783f68
Start relocate of code from ff000400 to 007d4000
Now running in RAM - U-Boot at: 007d4000
FLASH: flash detect cfi

fwc addr ff000000 cmd f0 f0f0 16bit x 8 bit
fwc addr ff000000 cmd ff ffff 16bit x 8 bit
fwc addr ff0000aa cmd 98 9898 16bit x 8 bit
is= cmd 51(Q) addr ff000020 is= ff00 5151
fwc addr ff000aaa cmd 98 9898 16bit x 8 bit
is= cmd 51(Q) addr ff000020 is= ff00 5151
fwc addr ff000000 cmd f0 00f0 16bit x 16 bit
fwc addr ff000000 cmd ff 00ff 16bit x 16 bit
fwc addr ff0000aa cmd 98 0098 16bit x 16 bit
is= cmd 51(Q) addr ff000020 is= ff00 0051
fwc addr ff000aaa cmd 98 0098 16bit x 16 bit
is= cmd 51(Q) addr ff000020 is= ff00 0051
fwc addr ff000000 cmd f0 f0f0f0f0 32bit x 8 bit
fwc addr ff000000 cmd ff ffffffff 32bit x 8 bit
fwc addr ff000154 cmd 98 98989898 32bit x 8 bit
is= cmd 51(Q) addr ff000040 is= ff0004b4 51515151
fwc addr ff001554 cmd 98 98989898 32bit x 8 bit
is= cmd 51(Q) addr ff000040 is= ff0004b4 51515151
fwc addr ff000000 cmd f0 00f000f0 32bit x 16 bit
fwc addr ff000000 cmd ff 00ff00ff 32bit x 16 bit
fwc addr ff000154 cmd 98 00980098 32bit x 16 bit
is= cmd 51(Q) addr ff000040 is= ff0004b4 00510051
fwc addr ff001554 cmd 98 00980098 32bit x 16 bit
is= cmd 51(Q) addr ff000040 is= ff0004b4 00510051
fwc addr ff000000 cmd f0 000000f0 32bit x 32 bit
fwc addr ff000000 cmd ff 000000ff 32bit x 32 bit
fwc addr ff000154 cmd 98 00000098 32bit x 32 bit
is= cmd 51(Q) addr ff000040 is= ff0004b4 00000051
fwc addr ff001554 cmd 98 00000098 32bit x 32 bit
is= cmd 51(Q) addr ff000040 is= ff0004b4 00000051
fwrite addr ff000000 cmd f0 fffffffffffffffffffff0 64 bit x 8 bit
fwrite addr ff000000 cmd ff ffffffffffffffffffffff 64 bit x 8 bit
fwrite addr ff0002a8 cmd 98 ffffffffffffffffffff98 64 bit x 8 bit
is= cmd 51(Q) addr ff000080 is= ff0004ffff0004ffffffb4 6234005151515151
fwrite addr ff002aa8 cmd 98 ffffffffffffffffffff98 64 bit x 8 bit
is= cmd 51(Q) addr ff000080 is= ff0004ffff0004ffffffb4 6234005151515151
fwrite addr ff000000 cmd f0 00ff00ff00ff00fffffff0 64 bit x 16 bit
fwrite addr ff000000 cmd ff 00ff00ff00ff00ffffffff 64 bit x 16 bit
fwrite addr ff0002a8 cmd 98 00ff00ff00ff00ffffff98 64 bit x 16 bit
is= cmd 51(Q) addr ff000080 is= ff0004ffff0004ffffffb4 6234005100510051
fwrite addr ff002aa8 cmd 98 00ff00ff00ff00ffffff98 64 bit x 16 bit
is= cmd 51(Q) addr ff000080 is= ff0004ffff0004ffffffb4 6234005100510051
fwrite addr ff000000 cmd f0 000000ff000000fffffff0 64 bit x 32 bit
fwrite addr ff000000 cmd ff 000000ff000000ffffffff 64 bit x 32 bit
fwrite addr ff0002a8 cmd 98 000000ff000000ffffff98 64 bit x 32 bit
is= cmd 51(Q) addr ff000080 is= ff0004ffff0004ffffffb4 6234005100000051
fwrite addr ff002aa8 cmd 98 000000ff000000ffffff98 64 bit x 32 bit
is= cmd 51(Q) addr ff000080 is= ff0004ffff0004ffffffb4 6234005100000051
fwrite addr ff000000 cmd f0 00000000000000fffffff0 64 bit x 64 bit
fwrite addr ff000000 cmd ff 00000000000000ffffffff 64 bit x 64 bit
fwrite addr ff0002a8 cmd 98 00000000000000ffffff98 64 bit x 64 bit
is= cmd 51(Q) addr ff000080 is= ff0004ffff0004ffffffb4 6234000000000051
fwrite addr ff002aa8 cmd 98 00000000000000ffffff98 64 bit x 64 bit
is= cmd 51(Q) addr ff000080 is= ff0004ffff0004ffffffb4 6234000000000051
not found
## Unknown FLASH on Bank 1 - Size = 0x00000000 = 0 MB
flash_protect ON: from 0xFF000400 to 0xFF0231FF
flash_protect ON: from 0xFF004000 to 0xFF005FFF
*** failed ***
Ignoring flash failure, see if we boot
In:    serial
Out:   serial
Err:   serial
MAC:   ethaddr  00:00:00:00:01:00
U-Boot relocated to 007d4000
Net:   FEC0 [PRIME]
### main_loop entered: bootdelay=0

[U-Boot] Question about M29W128G CFI QRY bug

[Richard Retanubun]

Hi Stefan,

As a follow up, This hack seems to fix the issue.

diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index 631b969..d386143 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -1691,7 +1691,7 @@ static int __flash_detect_cfi (flash_info_t * info, struct cfi_qry *qry)
            the reset command in both Intel and AMD variants, in the hope
            that AMD flash roms ignore the Intel command. */
         flash_write_cmd (info, 0, 0, AMD_CMD_RESET);
-       flash_write_cmd (info, 0, 0, FLASH_CMD_RESET);
+       //flash_write_cmd (info, 0, 0, FLASH_CMD_RESET);

         for (cfi_offset=0;
              cfi_offset < sizeof(flash_offset_cfi) / sizeof(uint);


I don't like it because removes support for the Intel command set, no?
Looks like "the hope" is now dashed by the M29W128G.

Any recommendation on how to proceed?

Thanks for your time.

Richard

[U-Boot] Question about M29W128G CFI QRY bug

[Stefan Roese]

Hi Richard,

On Monday 13 April 2009, Richard Retanubun wrote:
> As a follow up, This hack seems to fix the issue.
>
> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
> index 631b969..d386143 100644
> --- a/drivers/mtd/cfi_flash.c
> +++ b/drivers/mtd/cfi_flash.c
> @@ -1691,7 +1691,7 @@ static int __flash_detect_cfi (flash_info_t * info,
> struct cfi_qry *qry) the reset command in both Intel and AMD variants, in
> the hope that AMD flash roms ignore the Intel command. */
>          flash_write_cmd (info, 0, 0, AMD_CMD_RESET);
> -       flash_write_cmd (info, 0, 0, FLASH_CMD_RESET);
> +       //flash_write_cmd (info, 0, 0, FLASH_CMD_RESET);
>
>          for (cfi_offset=0;
>               cfi_offset < sizeof(flash_offset_cfi) / sizeof(uint);
>
>
> I don't like it because removes support for the Intel command set, no?
> Looks like "the hope" is now dashed by the M29W128G.

Let's quote the comment from the code above again for all users:

	/* We do not yet know what kind of commandset to use, so we issue
	   the reset command in both Intel and AMD variants, in the hope
	   that AMD flash roms ignore the Intel command. */
	flash_write_cmd (info, 0, 0, AMD_CMD_RESET);
	flash_write_cmd (info, 0, 0, FLASH_CMD_RESET);

Yes, you could be correct here. This 2nd RESET command seems to break M29W128G 
support.

> Any recommendation on how to proceed?

How is this problem solved in the Linux MTD driver (referring to your first 
email and the link you posted there)? 

Did you take a look at the CONFIG_SYS_FLASH_CFI_AMD_RESET define? I suggest 
you give it a try and let me know if this helps.

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
=====================================================================

[U-Boot] Question about M29W128G CFI QRY bug

[Richard Retanubun]

Hi Stefan,

  > Did you take a look at the CONFIG_SYS_FLASH_CFI_AMD_RESET define? I suggest
> you give it a try and let me know if this helps.
This does help but for the wrong reason:

#ifdef CONFIG_SYS_FLASH_CFI_AMD_RESET /* needed for STM_ID_29W320DB on UC100 */
# undef  FLASH_CMD_RESET
# define FLASH_CMD_RESET	AMD_CMD_RESET /* use AMD-Reset instead */
#endif

This just makes the reset code send two AMD_CMD_RESET.

> How is this problem solved in the Linux MTD driver (referring to your first 
> email and the link you posted there)?

The Linux way, it is addressed with this patch
==============================================
http://lists.infradead.org/pipermail/linux-mtd/2008-August/022499.html

Which does this:

  static int __xipram cfi_probe_chip(struct map_info *map, __u32 base,
  				   unsigned long *chip_map, struct cfi_private *cfi)
@@ -116,11 +87,7 @@ static int __xipram cfi_probe_chip(struct map_info *map, __u32 base,
  	}

  	xip_disable();
 >> This is the code that is similar to our cfi-flash code <<
-	cfi_send_gen_cmd(0xF0, 0, base, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0xFF, 0, base, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x98, 0x55, base, map, cfi, cfi->device_type, NULL);
-
-	if (!qry_present(map,base,cfi)) {
+	if (!qry_mode_on(base, map, cfi)) {
  		xip_enable(base, map, cfi);
  		return 0;
  	}

The new qry_mode_on function handles several "odd" flash cases (yeah, more of these... *sob*)

The quick and dirty way
====================================================
1. Swap the order: (suggested in http://lists.infradead.org/pipermail/linux-mtd/2008-July/022252.html )
         flash_write_cmd (info, 0, 0, FLASH_CMD_RESET);
         flash_write_cmd (info, 0, 0, AMD_CMD_RESET);

2. Follow AMD_CMD_RESET with a FLASH_CMD_RESET: (Suggested by an e-mail from Numonyx)
         flash_write_cmd (info, 0, 0, AMD_CMD_RESET);
         flash_write_cmd (info, 0, 0, FLASH_CMD_RESET);
         flash_write_cmd (info, 0, 0, AMD_CMD_RESET);

I tested both methods and they both work on Numonyx M29W128GH (AMD cmdset) and Numonyx PC28F320J3D75 (Intel cmdset)

For our local development, I'm probably going to adopt method #1 or method #2.
If either method #1 or #2 have a chance to be mainlined, I can submit a patch for them with explanations.

Obviously, the Linux method is probably the better solution (until another odd part comes along),
the porting of the linux method seems like it will require a lot of time and testing, more than I am capable of committing right now.

Thanks for your time,

Regards,

- Richard Retanubun

	

[U-Boot] Question about M29W128G CFI QRY bug

[Stefan Roese]

Richard,

On Wednesday 15 April 2009, Richard Retanubun wrote:
>   > Did you take a look at the CONFIG_SYS_FLASH_CFI_AMD_RESET define? I
>   > suggest
> >
> > you give it a try and let me know if this helps.
>
> This does help but for the wrong reason:
>
> #ifdef CONFIG_SYS_FLASH_CFI_AMD_RESET /* needed for STM_ID_29W320DB on
> UC100 */ # undef  FLASH_CMD_RESET
> # define FLASH_CMD_RESET	AMD_CMD_RESET /* use AMD-Reset instead */
> #endif
>
> This just makes the reset code send two AMD_CMD_RESET.

And do you need an Intel reset command on your board? Is there an option
for Intel command style FLASH chips?

> > How is this problem solved in the Linux MTD driver (referring to your
> > first email and the link you posted there)?
>
> The Linux way, it is addressed with this patch
> ==============================================
> http://lists.infradead.org/pipermail/linux-mtd/2008-August/022499.html
>
> Which does this:
>
>   static int __xipram cfi_probe_chip(struct map_info *map, __u32 base,
>   				   unsigned long *chip_map, struct cfi_private 
*cfi)
> @@ -116,11 +87,7 @@ static int __xipram cfi_probe_chip(struct map_info
> *map, __u32 base, }
>
>   	xip_disable();
>
>  >> This is the code that is similar to our cfi-flash code <<
>
> -	cfi_send_gen_cmd(0xF0, 0, base, map, cfi, cfi->device_type, NULL);
> -	cfi_send_gen_cmd(0xFF, 0, base, map, cfi, cfi->device_type, NULL);
> -	cfi_send_gen_cmd(0x98, 0x55, base, map, cfi, cfi->device_type, NULL);
> -
> -	if (!qry_present(map,base,cfi)) {
> +	if (!qry_mode_on(base, map, cfi)) {
>   		xip_enable(base, map, cfi);
>   		return 0;
>   	}
>
> The new qry_mode_on function handles several "odd" flash cases (yeah, more
> of these... *sob*)
>
> The quick and dirty way
> ====================================================
> 1. Swap the order: (suggested in
> http://lists.infradead.org/pipermail/linux-mtd/2008-July/022252.html )
> flash_write_cmd (info, 0, 0, FLASH_CMD_RESET);
>          flash_write_cmd (info, 0, 0, AMD_CMD_RESET);
>
> 2. Follow AMD_CMD_RESET with a FLASH_CMD_RESET: (Suggested by an e-mail
> from Numonyx) flash_write_cmd (info, 0, 0, AMD_CMD_RESET);
>          flash_write_cmd (info, 0, 0, FLASH_CMD_RESET);
>          flash_write_cmd (info, 0, 0, AMD_CMD_RESET);
>
> I tested both methods and they both work on Numonyx M29W128GH (AMD cmdset)
> and Numonyx PC28F320J3D75 (Intel cmdset)

I don't like both versions. We should implement something which doesn't
change the current behavior probably needed on some other boards. So how
about something like this:

=== Cut here ====
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index 175d82a..c370a64 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -1683,15 +1683,25 @@ static void flash_read_cfi (flash_info_t *info, void *buf,
 		p[i] = flash_read_uchar(info, start + i);
 }
 
+void __flash_cmd_reset(flash_info_t *info)
+{
+	/*
+	 * We do not yet know what kind of commandset to use, so we issue
+	 * the reset command in both Intel and AMD variants, in the hope
+	 * that AMD flash roms ignore the Intel command.
+	 */
+	flash_write_cmd(info, 0, 0, AMD_CMD_RESET);
+	flash_write_cmd(info, 0, 0, FLASH_CMD_RESET);
+}
+void flash_cmd_reset(flash_info_t *info)
+	__attribute__((weak,alias("__flash_cmd_reset")));
+
 static int __flash_detect_cfi (flash_info_t * info, struct cfi_qry *qry)
 {
 	int cfi_offset;
 
-	/* We do not yet know what kind of commandset to use, so we issue
-	   the reset command in both Intel and AMD variants, in the hope
-	   that AMD flash roms ignore the Intel command. */
-	flash_write_cmd (info, 0, 0, AMD_CMD_RESET);
-	flash_write_cmd (info, 0, 0, FLASH_CMD_RESET);
+	/* Issue FLASH reset command */
+	flash_cmd_reset(info);
 
 	for (cfi_offset=0;
 	     cfi_offset < sizeof(flash_offset_cfi) / sizeof(uint);
=== Cut here ====

By introducing a "weak" default reset function, you could overwrite it with
your own board specific version.

> For our local development, I'm probably going to adopt method #1 or method
> #2. If either method #1 or #2 have a chance to be mainlined, I can submit a
> patch for them with explanations.
>
> Obviously, the Linux method is probably the better solution (until another
> odd part comes along), the porting of the linux method seems like it will
> require a lot of time and testing, more than I am capable of committing
> right now.

Understood.

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
=====================================================================

[U-Boot] Question about M29W128G CFI QRY bug

[Richard Retanubun]

Hi Stefan,

Thanks for working through this with me,

> And do you need an Intel reset command on your board? Is there an option
> for Intel command style FLASH chips?

Maybe I am young and naive, but I am hoping to give the HW guys flexibility in choosing
the command sets for their flash.


> I don't like both versions. We should implement something which doesn't
> change the current behavior probably needed on some other boards. So how
> about something like this:

I took your proposal and tried to implement it, hit a snag with
flash_write_cmd currently being a private function of cfi-flash.c
so, here is my 'counter-offer' pasted at the end of the message.

By the way, since either reversing the reset order (option 1)
or adding another AMD_RESET_CMD (option 2) still have to be implemented
in our board specific flash_rst_command anyway, any recommendations of which
approach is less likely to break other flashes?

- Richard

=== Cut here ===
---
  drivers/mtd/cfi_flash.c |  101 +++++++-----------------------------------
  include/mtd/cfi-flash.h |  112 +++++++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 129 insertions(+), 84 deletions(-)
  create mode 100644 include/mtd/cfi-flash.h

diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index 631b969..3431b65 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -39,6 +39,7 @@
  #include <asm/io.h>
  #include <asm/byteorder.h>
  #include <environment.h>
+#include <mtd/cfi-flash.h>

  /*
   * This file implements a Common Flash Interface (CFI) driver for
@@ -65,84 +66,6 @@
  #define CONFIG_SYS_FLASH_BANKS_LIST { CONFIG_SYS_FLASH_BASE }
  #endif

-#define FLASH_CMD_CFI			0x98
-#define FLASH_CMD_READ_ID		0x90
-#define FLASH_CMD_RESET			0xff
-#define FLASH_CMD_BLOCK_ERASE		0x20
-#define FLASH_CMD_ERASE_CONFIRM		0xD0
-#define FLASH_CMD_WRITE			0x40
-#define FLASH_CMD_PROTECT		0x60
-#define FLASH_CMD_PROTECT_SET		0x01
-#define FLASH_CMD_PROTECT_CLEAR		0xD0
-#define FLASH_CMD_CLEAR_STATUS		0x50
-#define FLASH_CMD_READ_STATUS		0x70
-#define FLASH_CMD_WRITE_TO_BUFFER	0xE8
-#define FLASH_CMD_WRITE_BUFFER_PROG	0xE9
-#define FLASH_CMD_WRITE_BUFFER_CONFIRM	0xD0
-
-#define FLASH_STATUS_DONE		0x80
-#define FLASH_STATUS_ESS		0x40
-#define FLASH_STATUS_ECLBS		0x20
-#define FLASH_STATUS_PSLBS		0x10
-#define FLASH_STATUS_VPENS		0x08
-#define FLASH_STATUS_PSS		0x04
-#define FLASH_STATUS_DPS		0x02
-#define FLASH_STATUS_R			0x01
-#define FLASH_STATUS_PROTECT		0x01
-
-#define AMD_CMD_RESET			0xF0
-#define AMD_CMD_WRITE			0xA0
-#define AMD_CMD_ERASE_START		0x80
-#define AMD_CMD_ERASE_SECTOR		0x30
-#define AMD_CMD_UNLOCK_START		0xAA
-#define AMD_CMD_UNLOCK_ACK		0x55
-#define AMD_CMD_WRITE_TO_BUFFER		0x25
-#define AMD_CMD_WRITE_BUFFER_CONFIRM	0x29
-
-#define AMD_STATUS_TOGGLE		0x40
-#define AMD_STATUS_ERROR		0x20
-
-#define ATM_CMD_UNLOCK_SECT		0x70
-#define ATM_CMD_SOFTLOCK_START		0x80
-#define ATM_CMD_LOCK_SECT		0x40
-
-#define FLASH_OFFSET_MANUFACTURER_ID	0x00
-#define FLASH_OFFSET_DEVICE_ID		0x01
-#define FLASH_OFFSET_DEVICE_ID2		0x0E
-#define FLASH_OFFSET_DEVICE_ID3		0x0F
-#define FLASH_OFFSET_CFI		0x55
-#define FLASH_OFFSET_CFI_ALT		0x555
-#define FLASH_OFFSET_CFI_RESP		0x10
-#define FLASH_OFFSET_PRIMARY_VENDOR	0x13
-/* extended query table primary address */
-#define FLASH_OFFSET_EXT_QUERY_T_P_ADDR	0x15
-#define FLASH_OFFSET_WTOUT		0x1F
-#define FLASH_OFFSET_WBTOUT		0x20
-#define FLASH_OFFSET_ETOUT		0x21
-#define FLASH_OFFSET_CETOUT		0x22
-#define FLASH_OFFSET_WMAX_TOUT		0x23
-#define FLASH_OFFSET_WBMAX_TOUT		0x24
-#define FLASH_OFFSET_EMAX_TOUT		0x25
-#define FLASH_OFFSET_CEMAX_TOUT		0x26
-#define FLASH_OFFSET_SIZE		0x27
-#define FLASH_OFFSET_INTERFACE		0x28
-#define FLASH_OFFSET_BUFFER_SIZE	0x2A
-#define FLASH_OFFSET_NUM_ERASE_REGIONS	0x2C
-#define FLASH_OFFSET_ERASE_REGIONS	0x2D
-#define FLASH_OFFSET_PROTECT		0x02
-#define FLASH_OFFSET_USER_PROTECTION	0x85
-#define FLASH_OFFSET_INTEL_PROTECTION	0x81
-
-#define CFI_CMDSET_NONE			0
-#define CFI_CMDSET_INTEL_EXTENDED	1
-#define CFI_CMDSET_AMD_STANDARD		2
-#define CFI_CMDSET_INTEL_STANDARD	3
-#define CFI_CMDSET_AMD_EXTENDED		4
-#define CFI_CMDSET_MITSU_STANDARD	256
-#define CFI_CMDSET_MITSU_EXTENDED	257
-#define CFI_CMDSET_SST			258
-#define CFI_CMDSET_INTEL_PROG_REGIONS	512
-
  #ifdef CONFIG_SYS_FLASH_CFI_AMD_RESET /* needed for STM_ID_29W320DB on UC100 */
  # undef  FLASH_CMD_RESET
  # define FLASH_CMD_RESET	AMD_CMD_RESET /* use AMD-Reset instead */
@@ -450,7 +373,7 @@ static ulong flash_read_long (flash_info_t * info, flash_sect_t sect,
  /*
   * Write a proper sized command to the correct address
   */
-static void flash_write_cmd (flash_info_t * info, flash_sect_t sect,
+void flash_write_cmd (flash_info_t * info, flash_sect_t sect,
  			     uint offset, u32 cmd)
  {

@@ -1683,15 +1606,25 @@ static void flash_read_cfi (flash_info_t *info, void *buf,
  		p[i] = flash_read_uchar(info, start + i);
  }

+void __flash_cmd_reset(flash_info_t *info)
+{
+	/*
+	 * We do not yet know what kind of commandset to use, so we issue
+	 * the reset command in both Intel and AMD variants, in the hope
+	 * that AMD flash roms ignore the Intel command.
+	 */
+	flash_write_cmd(info, 0, 0, AMD_CMD_RESET);
+	flash_write_cmd(info, 0, 0, FLASH_CMD_RESET);
+}
+void flash_cmd_reset(flash_info_t *info)
+	__attribute__((weak,alias("__flash_cmd_reset")));
+
  static int __flash_detect_cfi (flash_info_t * info, struct cfi_qry *qry)
  {
  	int cfi_offset;

-	/* We do not yet know what kind of commandset to use, so we issue
-	   the reset command in both Intel and AMD variants, in the hope
-	   that AMD flash roms ignore the Intel command. */
-	flash_write_cmd (info, 0, 0, AMD_CMD_RESET);
-	flash_write_cmd (info, 0, 0, FLASH_CMD_RESET);
+	/* Issue FLASH reset command */
+	flash_cmd_reset(info);

  	for (cfi_offset=0;
  	     cfi_offset < sizeof(flash_offset_cfi) / sizeof(uint);
diff --git a/include/mtd/cfi-flash.h b/include/mtd/cfi-flash.h
new file mode 100644
index 0000000..f02c07f
--- /dev/null
+++ b/include/mtd/cfi-flash.h
@@ -0,0 +1,112 @@
+/*
+ * (C) Copyright 2002-2004
+ * Brad Kemp, Seranoa Networks, Brad.Kemp at seranoa.com
+ *
+ * Copyright (C) 2003 Arabella Software Ltd.
+ * Yuli Barcohen <yuli@arabellasw.com>
+ *
+ * Copyright (C) 2004
+ * Ed Okerson
+ *
+ * Copyright (C) 2006
+ * Tolunay Orkun <listmember@orkun.us>
+ *
+ * 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
+ *
+ */
+#define FLASH_CMD_CFI			0x98
+#define FLASH_CMD_READ_ID		0x90
+#define FLASH_CMD_RESET			0xff
+#define FLASH_CMD_BLOCK_ERASE		0x20
+#define FLASH_CMD_ERASE_CONFIRM		0xD0
+#define FLASH_CMD_WRITE			0x40
+#define FLASH_CMD_PROTECT		0x60
+#define FLASH_CMD_PROTECT_SET		0x01
+#define FLASH_CMD_PROTECT_CLEAR		0xD0
+#define FLASH_CMD_CLEAR_STATUS		0x50
+#define FLASH_CMD_READ_STATUS		0x70
+#define FLASH_CMD_WRITE_TO_BUFFER	0xE8
+#define FLASH_CMD_WRITE_BUFFER_PROG	0xE9
+#define FLASH_CMD_WRITE_BUFFER_CONFIRM	0xD0
+
+#define FLASH_STATUS_DONE		0x80
+#define FLASH_STATUS_ESS		0x40
+#define FLASH_STATUS_ECLBS		0x20
+#define FLASH_STATUS_PSLBS		0x10
+#define FLASH_STATUS_VPENS		0x08
+#define FLASH_STATUS_PSS		0x04
+#define FLASH_STATUS_DPS		0x02
+#define FLASH_STATUS_R			0x01
+#define FLASH_STATUS_PROTECT		0x01
+
+#define AMD_CMD_RESET			0xF0
+#define AMD_CMD_WRITE			0xA0
+#define AMD_CMD_ERASE_START		0x80
+#define AMD_CMD_ERASE_SECTOR		0x30
+#define AMD_CMD_UNLOCK_START		0xAA
+#define AMD_CMD_UNLOCK_ACK		0x55
+#define AMD_CMD_WRITE_TO_BUFFER		0x25
+#define AMD_CMD_WRITE_BUFFER_CONFIRM	0x29
+
+#define AMD_STATUS_TOGGLE		0x40
+#define AMD_STATUS_ERROR		0x20
+
+#define ATM_CMD_UNLOCK_SECT		0x70
+#define ATM_CMD_SOFTLOCK_START		0x80
+#define ATM_CMD_LOCK_SECT		0x40
+
+#define FLASH_OFFSET_MANUFACTURER_ID	0x00
+#define FLASH_OFFSET_DEVICE_ID		0x01
+#define FLASH_OFFSET_DEVICE_ID2		0x0E
+#define FLASH_OFFSET_DEVICE_ID3		0x0F
+#define FLASH_OFFSET_CFI		0x55
+#define FLASH_OFFSET_CFI_ALT		0x555
+#define FLASH_OFFSET_CFI_RESP		0x10
+#define FLASH_OFFSET_PRIMARY_VENDOR	0x13
+/* extended query table primary address */
+#define FLASH_OFFSET_EXT_QUERY_T_P_ADDR	0x15
+#define FLASH_OFFSET_WTOUT		0x1F
+#define FLASH_OFFSET_WBTOUT		0x20
+#define FLASH_OFFSET_ETOUT		0x21
+#define FLASH_OFFSET_CETOUT		0x22
+#define FLASH_OFFSET_WMAX_TOUT		0x23
+#define FLASH_OFFSET_WBMAX_TOUT		0x24
+#define FLASH_OFFSET_EMAX_TOUT		0x25
+#define FLASH_OFFSET_CEMAX_TOUT		0x26
+#define FLASH_OFFSET_SIZE		0x27
+#define FLASH_OFFSET_INTERFACE		0x28
+#define FLASH_OFFSET_BUFFER_SIZE	0x2A
+#define FLASH_OFFSET_NUM_ERASE_REGIONS	0x2C
+#define FLASH_OFFSET_ERASE_REGIONS	0x2D
+#define FLASH_OFFSET_PROTECT		0x02
+#define FLASH_OFFSET_USER_PROTECTION	0x85
+#define FLASH_OFFSET_INTEL_PROTECTION	0x81
+
+#define CFI_CMDSET_NONE			0
+#define CFI_CMDSET_INTEL_EXTENDED	1
+#define CFI_CMDSET_AMD_STANDARD		2
+#define CFI_CMDSET_INTEL_STANDARD	3
+#define CFI_CMDSET_AMD_EXTENDED		4
+#define CFI_CMDSET_MITSU_STANDARD	256
+#define CFI_CMDSET_MITSU_EXTENDED	257
+#define CFI_CMDSET_SST			258
+#define CFI_CMDSET_INTEL_PROG_REGIONS	512
+
+void flash_write_cmd (flash_info_t * info, flash_sect_t sect,
+		      uint offset, u32 cmd);
-- 
=== Cut here ===

[U-Boot] Question about M29W128G CFI QRY bug

[Stefan Roese]

On Wednesday 15 April 2009, Richard Retanubun wrote:
> > And do you need an Intel reset command on your board? Is there an option
> > for Intel command style FLASH chips?
>
> Maybe I am young and naive, but I am hoping to give the HW guys flexibility
> in choosing the command sets for their flash.

I have to admit that I've never seen such a board, but hey, it should be 
possible.

> > I don't like both versions. We should implement something which doesn't
> > change the current behavior probably needed on some other boards. So how
> > about something like this:
>
> I took your proposal and tried to implement it, hit a snag with
> flash_write_cmd currently being a private function of cfi-flash.c
> so, here is my 'counter-offer' pasted at the end of the message.

Thanks. Looks good. I'll take it and prepare a proper patch.

> By the way, since either reversing the reset order (option 1)
> or adding another AMD_RESET_CMD (option 2) still have to be implemented
> in our board specific flash_rst_command anyway, any recommendations of
> which approach is less likely to break other flashes?

I really have no idea. That's the main reason why I don't want this to be the 
default implementation. The current version is tested on many boards. And 
changing it could generate problems on some of them.

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
=====================================================================

[U-Boot] Question about M29W128G CFI QRY bug

[Stefan Roese]

On Wednesday 15 April 2009, Stefan Roese wrote:
> > > I don't like both versions. We should implement something which doesn't
> > > change the current behavior probably needed on some other boards. So
> > > how about something like this:
> >
> > I took your proposal and tried to implement it, hit a snag with
> > flash_write_cmd currently being a private function of cfi-flash.c
> > so, here is my 'counter-offer' pasted at the end of the message.
>
> Thanks. Looks good. I'll take it and prepare a proper patch.

Hmm. After looking a bit closer this doesn't seem to be this easy. I would 
like to add this function prototype to include/flash.h where other defines 
and function prototypes are collected right now. But this breaks some other 
boards implementing "their own" flash driver in their board directories, like 
AP1000, cpci5200, G2000, ppmc8260 and sbc405 (maintainers added to CC).

All those boards should be able to use the common CFI flash driver. I would 
really like to see those boards converted to this driver. It should be pretty 
easy. So maintainers from the boards mentioned above, if you need any help 
with this just let me know.

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
=====================================================================

[U-Boot] Question about M29W128G CFI QRY bug

[Richard Retanubun]

Stefan Roese wrote:
> On Wednesday 15 April 2009, Stefan Roese wrote:
>>>> I don't like both versions. We should implement something which doesn't
>>>> change the current behavior probably needed on some other boards. So
>>>> how about something like this:
>>> I took your proposal and tried to implement it, hit a snag with
>>> flash_write_cmd currently being a private function of cfi-flash.c
>>> so, here is my 'counter-offer' pasted at the end of the message.
>> Thanks. Looks good. I'll take it and prepare a proper patch.
> 
> Hmm. After looking a bit closer this doesn't seem to be this easy. I would 
> like to add this function prototype to include/flash.h where other defines 
> and function prototypes are collected right now. But this breaks some other 
> boards implementing "their own" flash driver in their board directories, like 
> AP1000, cpci5200, G2000, ppmc8260 and sbc405 (maintainers added to CC).
> 
> All those boards should be able to use the common CFI flash driver. I would 
> really like to see those boards converted to this driver. It should be pretty 
> easy. So maintainers from the boards mentioned above, if you need any help 
> with this just let me know.
> 
Hi Stefan,

Would I be foolish to hope that this can be merged in for 2009.06? :)

Thanks

- Richard

[U-Boot] Question about M29W128G CFI QRY bug

[Stefan Roese]

Richard,

On Friday 01 May 2009 23:29:22 Richard Retanubun wrote:
> Would I be foolish to hope that this can be merged in for 2009.06? :)

I just sent a patch with the weak default for this reset function. Please give 
it a try on your system and let me know if this works for you.

Thanks.

Cheers,
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

[U-Boot] Question about M29W128G CFI QRY bug

[Richard Retanubun]

Stefan Roese wrote:
> Richard,
> I just sent a patch with the weak default for this reset function. Please give 
> it a try on your system and let me know if this works for you.
> 
> Thanks.
> 
> Cheers,
> Stefan

Tested and works, thanks!

- Richard

[U-Boot] Question about M29W128G CFI QRY bug

[Wolfgang Denk]

Dear Stefan,

In message <4AE7303E.3000307@RuggedCom.com> Richard Retanubun wrote:
> Stefan Roese wrote:
> > Richard,
> > I just sent a patch with the weak default for this reset function. Please give 
> > it a try on your system and let me know if this works for you.
> > 
> > Thanks.
> > 
> > Cheers,
> > Stefan
> 
> Tested and works, thanks!

If you send a CFI pull req, I'm willing to consider this a bug fix.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There is is no reason for any individual to have a computer in  their
home.      -- Ken Olsen (President of Digital Equipment Corporation),
              Convention of the World Future Society, in Boston, 1977