[U-Boot-Users] [PATCH] CFI: synchronize command offsets with Linux CFI driver

[U-Boot-Users] [PATCH] CFI: synchronize command offsets with Linux CFI driver

[Bartlomiej Sieka]

Fixes non-working CFI Flash on the Inka4x0 board.

Signed-off-by: Bartlomiej Sieka <tur@semihalf.com>
---
This patch differs from what is being done by the Linux CFI driver in that it
uses 0x05 as the interface code for x16/x32 chips, per "Common Flash Memory
Interface Publication 100" dated December 1.

Tested on the following Flash chips:
x8 (interface 0x00) in 8 bit mode
x8/x16 (interface 0x02) in 8 bit mode
x8/x16 (interface 0x02) in 16 bit mode

The patch should be applied on top of commit
81b20ccc2d795ae9a1199db5a50ad9c28d1e4d22 in the u-boot-cfi-flash repo.

diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index 66754cd..b748a9f 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -1231,9 +1231,26 @@ static int flash_detect_cfi (flash_info_t * info)
 					debug ("port %d bits chip %d bits\n",
 						info->portwidth << CFI_FLASH_SHIFT_WIDTH,
 						info->chipwidth << CFI_FLASH_SHIFT_WIDTH);
-					/* this probably only works if info->interface == FLASH_CFI_X8X16 */
-					info->addr_unlock1 = (info->portwidth == FLASH_CFI_8BIT) ? 0xAAA : 0x555;
-					info->addr_unlock2 = (info->portwidth == FLASH_CFI_8BIT) ? 0x555 : 0x2AA;
+
+					/* calculate command offsets as in the Linux driver */
+					info->addr_unlock1 = 0x555;
+					info->addr_unlock2 = 0x2aa;
+
+					/*
+					 * modify the unlock address if we are
+					 * in compatibility mode
+					 */
+					if (	/* x8/x16 in x8 mode */
+						((info->chipwidth == FLASH_CFI_BY8) &&
+							(info->interface == FLASH_CFI_X8X16)) ||
+						/* x16/x32 in x16 mode */
+						((info->chipwidth == FLASH_CFI_BY16) &&
+							(info->interface == FLASH_CFI_X16X32)))
+					{
+						info->addr_unlock1 = 0xaaa;
+						info->addr_unlock2 = 0x555;
+					}
+ 
 					info->name = "CFI conformant";
 					return 1;
 				}
diff --git a/include/flash.h b/include/flash.h
index 6e252cc..2ed1e20 100644
--- a/include/flash.h
+++ b/include/flash.h
@@ -80,6 +80,7 @@ typedef struct {
 #define FLASH_CFI_X8		0x00
 #define FLASH_CFI_X16		0x01
 #define FLASH_CFI_X8X16		0x02
+#define FLASH_CFI_X16X32	0x05
 
 /* convert between bit value and numeric value */
 #define CFI_FLASH_SHIFT_WIDTH	3

[U-Boot-Users] [PATCH] CFI: synchronize command offsets with Linux CFI driver

[Stefan Roese]

On Tuesday 11 December 2007, Bartlomiej Sieka wrote:
> Fixes non-working CFI Flash on the Inka4x0 board.
>
> Signed-off-by: Bartlomiej Sieka <tur@semihalf.com>
> ---
> This patch differs from what is being done by the Linux CFI driver in that
> it uses 0x05 as the interface code for x16/x32 chips, per "Common Flash
> Memory Interface Publication 100" dated December 1.
>
> Tested on the following Flash chips:
> x8 (interface 0x00) in 8 bit mode
> x8/x16 (interface 0x02) in 8 bit mode
> x8/x16 (interface 0x02) in 16 bit mode
>
> The patch should be applied on top of commit
> 81b20ccc2d795ae9a1199db5a50ad9c28d1e4d22 in the u-boot-cfi-flash repo.
>
> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
> index 66754cd..b748a9f 100644
> --- a/drivers/mtd/cfi_flash.c
> +++ b/drivers/mtd/cfi_flash.c
> @@ -1231,9 +1231,26 @@ static int flash_detect_cfi (flash_info_t * info)
>  					debug ("port %d bits chip %d bits\n",
>  						info->portwidth << CFI_FLASH_SHIFT_WIDTH,
>  						info->chipwidth << CFI_FLASH_SHIFT_WIDTH);
> -					/* this probably only works if info->interface == FLASH_CFI_X8X16 */
> -					info->addr_unlock1 = (info->portwidth == FLASH_CFI_8BIT) ? 0xAAA :
> 0x555; -					info->addr_unlock2 = (info->portwidth == FLASH_CFI_8BIT) ?
> 0x555 : 0x2AA; +
> +					/* calculate command offsets as in the Linux driver */
> +					info->addr_unlock1 = 0x555;
> +					info->addr_unlock2 = 0x2aa;
> +
> +					/*
> +					 * modify the unlock address if we are
> +					 * in compatibility mode
> +					 */
> +					if (	/* x8/x16 in x8 mode */
> +						((info->chipwidth == FLASH_CFI_BY8) &&
> +							(info->interface == FLASH_CFI_X8X16)) ||
> +						/* x16/x32 in x16 mode */
> +						((info->chipwidth == FLASH_CFI_BY16) &&
> +							(info->interface == FLASH_CFI_X16X32)))
> +					{
> +						info->addr_unlock1 = 0xaaa;
> +						info->addr_unlock2 = 0x555;
> +					}
> +

Why not like this:

					if (((info->chipwidth == FLASH_CFI_BY8) &&
					     (info->interface == FLASH_CFI_X8X16)) ||
					    ((info->chipwidth == FLASH_CFI_BY16) &&
					     (info->interface == FLASH_CFI_X16X32))) {
						/*
						 * Compatibility mode:
						 * x8/x16 in x8 mode or
						 * x16/x32 in x16 mode
						 */
						info->addr_unlock1 = 0xaaa;
						info->addr_unlock2 = 0x555;
					} else {
						/* default */
						info->addr_unlock1 = 0x555;
						info->addr_unlock2 = 0x2aa;
					}


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-Users] [PATCH] CFI: synchronize command offsets with Linux CFI driver

[Bartlomiej Sieka]

Stefan Roese wrote:
[...]
>> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
>> index 66754cd..b748a9f 100644
>> --- a/drivers/mtd/cfi_flash.c
>> +++ b/drivers/mtd/cfi_flash.c
>> @@ -1231,9 +1231,26 @@ static int flash_detect_cfi (flash_info_t * info)
>>  					debug ("port %d bits chip %d bits\n",
>>  						info->portwidth << CFI_FLASH_SHIFT_WIDTH,
>>  						info->chipwidth << CFI_FLASH_SHIFT_WIDTH);
>> -					/* this probably only works if info->interface == FLASH_CFI_X8X16 */
>> -					info->addr_unlock1 = (info->portwidth == FLASH_CFI_8BIT) ? 0xAAA :
>> 0x555; -					info->addr_unlock2 = (info->portwidth == FLASH_CFI_8BIT) ?
>> 0x555 : 0x2AA; +
>> +					/* calculate command offsets as in the Linux driver */
>> +					info->addr_unlock1 = 0x555;
>> +					info->addr_unlock2 = 0x2aa;
>> +
>> +					/*
>> +					 * modify the unlock address if we are
>> +					 * in compatibility mode
>> +					 */
>> +					if (	/* x8/x16 in x8 mode */
>> +						((info->chipwidth == FLASH_CFI_BY8) &&
>> +							(info->interface == FLASH_CFI_X8X16)) ||
>> +						/* x16/x32 in x16 mode */
>> +						((info->chipwidth == FLASH_CFI_BY16) &&
>> +							(info->interface == FLASH_CFI_X16X32)))
>> +					{
>> +						info->addr_unlock1 = 0xaaa;
>> +						info->addr_unlock2 = 0x555;
>> +					}
>> +
> 
> Why not like this:
> 
> 					if (((info->chipwidth == FLASH_CFI_BY8) &&
> 					     (info->interface == FLASH_CFI_X8X16)) ||
> 					    ((info->chipwidth == FLASH_CFI_BY16) &&
> 					     (info->interface == FLASH_CFI_X16X32))) {
> 						/*
> 						 * Compatibility mode:
> 						 * x8/x16 in x8 mode or
> 						 * x16/x32 in x16 mode
> 						 */
> 						info->addr_unlock1 = 0xaaa;
> 						info->addr_unlock2 = 0x555;
> 					} else {
> 						/* default */
> 						info->addr_unlock1 = 0x555;
> 						info->addr_unlock2 = 0x2aa;
> 					}

I copied the code from Linux CFI driver and kept it as is (modulo
#define names). I'm fine with either of the two forms, although I think
that there is some benefit in keeping borrowed code as intact as possible.

Regards,
Bartlomiej

[U-Boot-Users] [PATCH] CFI: synchronize command offsets with Linux CFI driver

[Stefan Roese]

On Tuesday 11 December 2007, Bartlomiej Sieka wrote:
> > Why not like this:
> >
> > 					if (((info->chipwidth == FLASH_CFI_BY8) &&
> > 					     (info->interface == FLASH_CFI_X8X16)) ||
> > 					    ((info->chipwidth == FLASH_CFI_BY16) &&
> > 					     (info->interface == FLASH_CFI_X16X32))) {
> > 						/*
> > 						 * Compatibility mode:
> > 						 * x8/x16 in x8 mode or
> > 						 * x16/x32 in x16 mode
> > 						 */
> > 						info->addr_unlock1 = 0xaaa;
> > 						info->addr_unlock2 = 0x555;
> > 					} else {
> > 						/* default */
> > 						info->addr_unlock1 = 0x555;
> > 						info->addr_unlock2 = 0x2aa;
> > 					}
>
> I copied the code from Linux CFI driver and kept it as is (modulo
> #define names).

Ahh, I see. Even this ugly "{" in the next line is present in the Linux code.

> I'm fine with either of the two forms, although I think 
> that there is some benefit in keeping borrowed code as intact as possible.

Right. So if nobody else protests here, I'll add your original version to the 
cfi repo soon.

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-Users] [PATCH] CFI: synchronize command offsets with Linux CFI driver

[Hoi-Ho Chan]

Hi,

    I have a little problem while detecting the manufacturer and device IDs
on a Spansion/AMD S29GL256N. It's a x8/x16 flash and set to run in x8 mode.
When u-boot is trying to do the unlock sequence and retrieve the
manufacturer and device IDs, since info->portwidth is still 2, it's writing
to addrs 0x1554, 0xAAA, 0x1554 instead of addrs 0xaaa, 0x555, 0xaaa. I
modified the code to temporary set info->portwidth to 1 before doing the
unlock seq and retrieving the manufacturer & device IDs and then restore it
back, then the ID retrievals work. Does anyone see the same problem as I do?

Thanks
Donald



Bartlomiej Sieka wrote:
> 
> Fixes non-working CFI Flash on the Inka4x0 board.
> 
> Signed-off-by: Bartlomiej Sieka <tur@semihalf.com>
> ---
> This patch differs from what is being done by the Linux CFI driver in that
> it
> uses 0x05 as the interface code for x16/x32 chips, per "Common Flash
> Memory
> Interface Publication 100" dated December 1.
> 
> Tested on the following Flash chips:
> x8 (interface 0x00) in 8 bit mode
> x8/x16 (interface 0x02) in 8 bit mode
> x8/x16 (interface 0x02) in 16 bit mode
> 
> The patch should be applied on top of commit
> 81b20ccc2d795ae9a1199db5a50ad9c28d1e4d22 in the u-boot-cfi-flash repo.
> 
> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
> index 66754cd..b748a9f 100644
> --- a/drivers/mtd/cfi_flash.c
> +++ b/drivers/mtd/cfi_flash.c
> @@ -1231,9 +1231,26 @@ static int flash_detect_cfi (flash_info_t * info)
>  					debug ("port %d bits chip %d bits\n",
>  						info->portwidth << CFI_FLASH_SHIFT_WIDTH,
>  						info->chipwidth << CFI_FLASH_SHIFT_WIDTH);
> -					/* this probably only works if info->interface == FLASH_CFI_X8X16 */
> -					info->addr_unlock1 = (info->portwidth == FLASH_CFI_8BIT) ? 0xAAA :
> 0x555;
> -					info->addr_unlock2 = (info->portwidth == FLASH_CFI_8BIT) ? 0x555 :
> 0x2AA;
> +
> +					/* calculate command offsets as in the Linux driver */
> +					info->addr_unlock1 = 0x555;
> +					info->addr_unlock2 = 0x2aa;
> +
> +					/*
> +					 * modify the unlock address if we are
> +					 * in compatibility mode
> +					 */
> +					if (	/* x8/x16 in x8 mode */
> +						((info->chipwidth == FLASH_CFI_BY8) &&
> +							(info->interface == FLASH_CFI_X8X16)) ||
> +						/* x16/x32 in x16 mode */
> +						((info->chipwidth == FLASH_CFI_BY16) &&
> +							(info->interface == FLASH_CFI_X16X32)))
> +					{
> +						info->addr_unlock1 = 0xaaa;
> +						info->addr_unlock2 = 0x555;
> +					}
> + 
>  					info->name = "CFI conformant";
>  					return 1;
>  				}
> diff --git a/include/flash.h b/include/flash.h
> index 6e252cc..2ed1e20 100644
> --- a/include/flash.h
> +++ b/include/flash.h
> @@ -80,6 +80,7 @@ typedef struct {
>  #define FLASH_CFI_X8		0x00
>  #define FLASH_CFI_X16		0x01
>  #define FLASH_CFI_X8X16		0x02
> +#define FLASH_CFI_X16X32	0x05
>  
>  /* convert between bit value and numeric value */
>  #define CFI_FLASH_SHIFT_WIDTH	3
> 
> -------------------------------------------------------------------------
> SF.Net email is sponsored by: 
> Check out the new SourceForge.net Marketplace.
> It's the best place to buy or sell services for
> just about anything Open Source.
> http://sourceforge.net/services/buy/index.php
> _______________________________________________
> U-Boot-Users mailing list
> U-Boot-Users at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/u-boot-users
> 
> 

-- 
View this message in context: http://www.nabble.com/-PATCH--CFI%3A-synchronize-command-offsets-with-Linux-CFI-driver-tp14273476p14451676.html
Sent from the Uboot - Users mailing list archive at Nabble.com.