[U-Boot] [U-BOOT] cfi_flash.c fails to program NOR Flash SST39LF040

[U-Boot] [U-BOOT] cfi_flash.c fails to program NOR Flash SST39LF040

[Po-Yu Chuang]

Hello,


I have a board on which there is a NOR Flash SST39LF040.

Previously, I copied flash.c from other boards.

Now I am trying to migrate NOR Flash driver to use CFI, but it fails
to program sectors which
are not on the 0x10000 boundaries.

I found that in function flash_write_cfiword()
@drivers/mtd/cfi_flash.c it issues command
with address relative to sector address.
However, according to SST39 series data sheet, it seems that we should
issue command
with address relative to flash base address?

After I modified flash_write_cfiword() to issue command to sector 0,
everything works fine.

Is it a bug?

Index: cfi_flash.c
===================================================================
RCS file: /usr/local/cvsroot/ctd/FA5A320LINUX26_u-boot/drivers/mtd/cfi_flash.c,v
retrieving revision 1.1.1.1
diff -u -r1.1.1.1 cfi_flash.c
--- cfi_flash.c	23 Mar 2009 02:54:57 -0000	1.1.1.1
+++ cfi_flash.c	24 Apr 2009 02:28:32 -0000
@@ -839,8 +839,8 @@
 	case CFI_CMDSET_AMD_LEGACY:
 #endif
 		sect = find_sector(info, dest);
-		flash_unlock_seq (info, sect);
-		flash_write_cmd (info, sect, info->addr_unlock1, AMD_CMD_WRITE);
+		flash_unlock_seq (info, 0);
+		flash_write_cmd (info, 0, info->addr_unlock1, AMD_CMD_WRITE);
 		sect_found = 1;
 		break;
 	}

regards

Po-Yu Chuang

[U-Boot] [U-BOOT] cfi_flash.c fails to program NOR Flash SST39LF040

[Stefan Roese]

On Friday 24 April 2009, Po-Yu Chuang wrote:
> I have a board on which there is a NOR Flash SST39LF040.
>
> Previously, I copied flash.c from other boards.
>
> Now I am trying to migrate NOR Flash driver to use CFI, but it fails
> to program sectors which
> are not on the 0x10000 boundaries.
>
> I found that in function flash_write_cfiword()
> @drivers/mtd/cfi_flash.c it issues command
> with address relative to sector address.
> However, according to SST39 series data sheet, it seems that we should
> issue command
> with address relative to flash base address?
>
> After I modified flash_write_cfiword() to issue command to sector 0,
> everything works fine.
>
> Is it a bug?

Did you define CONFIG_FLASH_CFI_LEGACY (and CONFIG_SYS_FLASH_LEGACY_512Kx8)? 
IIRC then the SST FLASH chips are not completely CFI compatible. So you need 
to enable the JEDEC framework.

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] [U-BOOT] cfi_flash.c fails to program NOR Flash SST39LF040

[Po-Yu Chuang]

Dear Stefan Roese,

2009/4/27 Stefan Roese <sr@denx.de>
> On Friday 24 April 2009, Po-Yu Chuang wrote:
> > I have a board on which there is a NOR Flash SST39LF040.
> > Previously, I copied flash.c from other boards.
> > Now I am trying to migrate NOR Flash driver to use CFI, but it fails
> > to program sectors which
> > are not on the 0x10000 boundaries.
> > I found that in function flash_write_cfiword()
> > @drivers/mtd/cfi_flash.c it issues command
> > with address relative to sector address.
> > However, according to SST39 series data sheet, it seems that we should
> > issue command
> > with address relative to flash base address?
> >
> > After I modified flash_write_cfiword() to issue command to sector 0,
> > everything works fine.
> >
> > Is it a bug?
> Did you define CONFIG_FLASH_CFI_LEGACY (and CONFIG_SYS_FLASH_LEGACY_512Kx8)?
> IIRC then the SST FLASH chips are not completely CFI compatible. So you need
> to enable the JEDEC framework.

Yes, I have defined the fallowing macros:

#define CONFIG_SYS_FLASH_CFI
#define CONFIG_FLASH_CFI_DRIVER
#define CONFIG_FLASH_CFI_LEGACY
#define CONFIG_SYS_FLASH_LEGACY_512Kx8

SST39LF040 use addr1=0x5555 and addr2=0x2AAA, however, each sector is
0x1000 bytes.

Thus, if we issue command to "sector base (0x41000) + offset(0x5555)",
it sends to 0x46555 and the chip fails to recognize that address.

I guess that SST39LF020 might suffer the same problem.


best regards,

Po-Yu Chuang

[U-Boot] [U-BOOT] cfi_flash.c fails to program NOR Flash SST39LF040

[Po-Yu Chuang]

Dear Stefan Roese,

2009/4/28 Po-Yu Chuang <ratbert.chuang@gmail.com>:
> Dear Stefan Roese,
>> Did you define CONFIG_FLASH_CFI_LEGACY (and CONFIG_SYS_FLASH_LEGACY_512Kx8)?
>> IIRC then the SST FLASH chips are not completely CFI compatible. So you need
>> to enable the JEDEC framework.
>
> Yes, I have defined the fallowing macros:
>
> #define CONFIG_SYS_FLASH_CFI
> #define CONFIG_FLASH_CFI_DRIVER
> #define CONFIG_FLASH_CFI_LEGACY
> #define CONFIG_SYS_FLASH_LEGACY_512Kx8
>
> SST39LF040 use addr1=0x5555 and addr2=0x2AAA, however, each sector is
> 0x1000 bytes.
>
> Thus, if we issue command to "sector base (0x41000) + offset(0x5555)",
> it sends to 0x46555 and the chip fails to recognize that address.
>
> I guess that SST39LF020 might suffer the same problem.

How do you think about this?
I can submit a patch for it.

regards,
Po-Yu Chuang

[U-Boot] [U-BOOT] cfi_flash.c fails to program NOR Flash SST39LF040

[Stefan Roese]

Hi Po-Yu Chuang,

On Friday 03 July 2009 09:37:19 Po-Yu Chuang wrote:
> > SST39LF040 use addr1=0x5555 and addr2=0x2AAA, however, each sector is
> > 0x1000 bytes.
> >
> > Thus, if we issue command to "sector base (0x41000) + offset(0x5555)",
> > it sends to 0x46555 and the chip fails to recognize that address.
> >
> > I guess that SST39LF020 might suffer the same problem.
>
> How do you think about this?
> I can submit a patch for it.

Yes, I understand your problem and it needs to get fixed. But your original 
patch changed the unlock address not only for this "legacy" FLASH type but for 
all AMD type FLASH chips. And I'm not sure if this is correct. Meaning if the 
AMD/Spansion chips will accept these unlock addresses.

Perhaps you could check some data-sheets (some older and more recent Spansion 
CFI chips) which unlock addresses should be used here.

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] [U-BOOT] cfi_flash.c fails to program NOR Flash SST39LF040

[Po-Yu Chuang]

Dear Stefan Roese,

2009/7/3 Stefan Roese <sr@denx.de>:
> Yes, I understand your problem and it needs to get fixed. But your original
> patch changed the unlock address not only for this "legacy" FLASH type but for
> all AMD type FLASH chips. And I'm not sure if this is correct. Meaning if the
> AMD/Spansion chips will accept these unlock addresses.
>
> Perhaps you could check some data-sheets (some older and more recent Spansion
> CFI chips) which unlock addresses should be used here.

After checked the data sheets of jedec flash currently supported in u-boot -

SST 39LF020
AMD AM29LV040B
SST 39LF040
ST Micro M29W040B
AMD AM29LV400BB
AMD AM29LV800BB

I found that the addresses to program a word of these chips are
related to 0 (not sector base).

And in flash_erase() in drivers/mtd/cfi_flash.c

			case CFI_CMDSET_AMD_STANDARD:
			case CFI_CMDSET_AMD_EXTENDED:
				flash_unlock_seq (info, sect);
				flash_write_cmd (info, sect,
						info->addr_unlock1,
						AMD_CMD_ERASE_START);
				flash_unlock_seq (info, sect);
				flash_write_cmd (info, sect, 0,
						 AMD_CMD_ERASE_SECTOR);
				break;
#ifdef CONFIG_FLASH_CFI_LEGACY
			case CFI_CMDSET_AMD_LEGACY:
				flash_unlock_seq (info, 0);
				flash_write_cmd (info, 0, info->addr_unlock1,
						AMD_CMD_ERASE_START);
				flash_unlock_seq (info, 0);
				flash_write_cmd (info, sect, 0,
						AMD_CMD_ERASE_SECTOR);
				break;
#endif

It look like CFI_CMDSET_AMD_LEGACY use base 0
CFI_CMDSET_AMD_STANDARD and CFI_CMDSET_AMD_EXTENDED use sector base
for erase.

It probably is the same case while programing a word.

The following patch might be feasible.
However, I don't have those chips for testing.

regards,
Po-Yu Chuang

---
 drivers/mtd/cfi_flash.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index d0732f5..81ac5d3 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -835,14 +835,19 @@ static int flash_write_cfiword (flash_info_t *
info, ulong dest,
 		break;
 	case CFI_CMDSET_AMD_EXTENDED:
 	case CFI_CMDSET_AMD_STANDARD:
-#ifdef CONFIG_FLASH_CFI_LEGACY
-	case CFI_CMDSET_AMD_LEGACY:
-#endif
 		sect = find_sector(info, dest);
 		flash_unlock_seq (info, sect);
 		flash_write_cmd (info, sect, info->addr_unlock1, AMD_CMD_WRITE);
 		sect_found = 1;
 		break;
+#ifdef CONFIG_FLASH_CFI_LEGACY
+	case CFI_CMDSET_AMD_LEGACY:
+		sect = find_sector(info, dest);
+		flash_unlock_seq (info, 0);
+		flash_write_cmd (info, 0, info->addr_unlock1, AMD_CMD_WRITE);
+		sect_found = 1;
+		break;
+#endif
 	}

 	switch (info->portwidth) {
-- 
1.6.3.3

[U-Boot] [U-BOOT] cfi_flash.c fails to program NOR Flash SST39LF040

[Stefan Roese]

Hi Po-Yu Chuang,

On Friday 03 July 2009 11:11:55 Po-Yu Chuang wrote:
> > Yes, I understand your problem and it needs to get fixed. But your
> > original patch changed the unlock address not only for this "legacy"
> > FLASH type but for all AMD type FLASH chips. And I'm not sure if this is
> > correct. Meaning if the AMD/Spansion chips will accept these unlock
> > addresses.
> >
> > Perhaps you could check some data-sheets (some older and more recent
> > Spansion CFI chips) which unlock addresses should be used here.
>
> After checked the data sheets of jedec flash currently supported in u-boot
> -
>
> SST 39LF020
> AMD AM29LV040B
> SST 39LF040
> ST Micro M29W040B
> AMD AM29LV400BB
> AMD AM29LV800BB
>
> I found that the addresses to program a word of these chips are
> related to 0 (not sector base).
>
> And in flash_erase() in drivers/mtd/cfi_flash.c
>
> 			case CFI_CMDSET_AMD_STANDARD:
> 			case CFI_CMDSET_AMD_EXTENDED:
> 				flash_unlock_seq (info, sect);
> 				flash_write_cmd (info, sect,
> 						info->addr_unlock1,
> 						AMD_CMD_ERASE_START);
> 				flash_unlock_seq (info, sect);
> 				flash_write_cmd (info, sect, 0,
> 						 AMD_CMD_ERASE_SECTOR);
> 				break;
> #ifdef CONFIG_FLASH_CFI_LEGACY
> 			case CFI_CMDSET_AMD_LEGACY:
> 				flash_unlock_seq (info, 0);
> 				flash_write_cmd (info, 0, info->addr_unlock1,
> 						AMD_CMD_ERASE_START);
> 				flash_unlock_seq (info, 0);
> 				flash_write_cmd (info, sect, 0,
> 						AMD_CMD_ERASE_SECTOR);
> 				break;
> #endif
>
> It look like CFI_CMDSET_AMD_LEGACY use base 0
> CFI_CMDSET_AMD_STANDARD and CFI_CMDSET_AMD_EXTENDED use sector base
> for erase.
>
> It probably is the same case while programing a word.
>
> The following patch might be feasible.
> However, I don't have those chips for testing.

I don't have those chips either, but your patch below looks like an 
improvement to me. So I suggest that we push this patch into mainline so that 
other users with such chips can easily test it.

Could you please sent the patch below as a proper patch with a description 
your s-o-b line again to the list?

Thanks.

Best regards,
Stefan

> regards,
> Po-Yu Chuang
>
> ---
>  drivers/mtd/cfi_flash.c |   11 ++++++++---
>  1 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
> index d0732f5..81ac5d3 100644
> --- a/drivers/mtd/cfi_flash.c
> +++ b/drivers/mtd/cfi_flash.c
> @@ -835,14 +835,19 @@ static int flash_write_cfiword (flash_info_t *
> info, ulong dest,
>  		break;
>  	case CFI_CMDSET_AMD_EXTENDED:
>  	case CFI_CMDSET_AMD_STANDARD:
> -#ifdef CONFIG_FLASH_CFI_LEGACY
> -	case CFI_CMDSET_AMD_LEGACY:
> -#endif
>  		sect = find_sector(info, dest);
>  		flash_unlock_seq (info, sect);
>  		flash_write_cmd (info, sect, info->addr_unlock1, AMD_CMD_WRITE);
>  		sect_found = 1;
>  		break;
> +#ifdef CONFIG_FLASH_CFI_LEGACY
> +	case CFI_CMDSET_AMD_LEGACY:
> +		sect = find_sector(info, dest);
> +		flash_unlock_seq (info, 0);
> +		flash_write_cmd (info, 0, info->addr_unlock1, AMD_CMD_WRITE);
> +		sect_found = 1;
> +		break;
> +#endif
>  	}
>
>  	switch (info->portwidth) {

Viele Gr??e,
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
=====================================================================