[U-Boot] [PATCH] cfi flash: add status polling method for amd flash

[U-Boot] [PATCH] cfi flash: add status polling method for amd flash

[Thomas Chou]

This patch adds status polling method to offer an alternative to
data toggle method for amd flash chips.

This patch is needed for nios2 cfi flash interface, where the bus
controller performs 4 bytes read cycles for a single byte read
instruction. The data toggle method can not detect chip busy
status correctly. So we have to poll DQ7, which will be inverted
when the chip is busy.

This feature is enabled with the config def,
CONFIG_SYS_CFI_FLASH_STATUS_POLL

Include patch, "drivers/mtd/cfi_flash: precision and underflow
problem in tout calculation", submitted by
  Alessandro Rubini <rub...@gnudd.com>
  Renato Andreola <renato.andre...@imagos.it>

Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
---
 drivers/mtd/cfi_flash.c |   78 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 77 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index fdba297..db22255 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -537,7 +537,10 @@ static int flash_status_check (flash_info_t * info, flash_sect_t sector,
 	ulong start;
 
 #if CONFIG_SYS_HZ != 1000
-	tout *= CONFIG_SYS_HZ/1000;
+	if ((ulong)CONFIG_SYS_HZ > 100000)
+		tout *= (ulong)CONFIG_SYS_HZ / 1000;  /* for a big HZ, avoid overflow */
+	else
+		tout = DIV_ROUND_UP (tout * (ulong)CONFIG_SYS_HZ, 1000);
 #endif
 
 	/* Wait for command completion */
@@ -555,6 +558,53 @@ static int flash_status_check (flash_info_t * info, flash_sect_t sector,
 	return ERR_OK;
 }
 
+#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
+static int flash_status_poll (flash_info_t *info, void *src, void *dst,
+			     ulong tout, char *prompt)
+{
+	ulong start;
+	int ready;
+
+#if CONFIG_SYS_HZ != 1000
+	if ((ulong)CONFIG_SYS_HZ > 100000)
+		tout *= (ulong)CONFIG_SYS_HZ / 1000;  /* for a big HZ, avoid overflow */
+	else
+		tout = DIV_ROUND_UP (tout * (ulong)CONFIG_SYS_HZ, 1000);
+#endif
+
+	/* Wait for command completion */
+	start = get_timer (0);
+	while (1) {
+		switch (info->portwidth) {
+		case FLASH_CFI_8BIT:
+			ready = flash_read8(dst) == flash_read8(src);
+			break;
+		case FLASH_CFI_16BIT:
+			ready = flash_read16(dst) == flash_read16(src);
+			break;
+		case FLASH_CFI_32BIT:
+			ready = flash_read32(dst) == flash_read32(src);
+			break;
+		case FLASH_CFI_64BIT:
+			ready = flash_read64(dst) == flash_read64(src);
+			break;
+		default:
+			ready = 0;
+			break;
+		}
+		if (ready)
+			break;
+		if (get_timer (start) > tout) {
+			printf ("Flash %s timeout at address %lx data %lx\n",
+			       prompt, (ulong)dst, (ulong)flash_read8(dst));
+			return ERR_TIMOUT;
+		}
+		udelay (1);		/* also triggers watchdog */
+	}
+	return ERR_OK;
+}
+#endif
+
 /*-----------------------------------------------------------------------
  * Wait for XSR.7 to be set, if it times out print an error, otherwise
  * do a full status check.
@@ -749,6 +799,13 @@ static int flash_write_cfiword (flash_info_t * info, ulong dest,
 	if (!sect_found)
 		sect = find_sector (info, dest);
 
+#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
+	if (info->vendor == CFI_CMDSET_AMD_EXTENDED ||
+	    info->vendor == CFI_CMDSET_AMD_STANDARD)
+		return flash_status_poll (info, &cword, dstaddr,
+					 info->write_tout, "write");
+	else
+#endif
 	return flash_full_status_check (info, sect, info->write_tout, "write");
 }
 
@@ -911,9 +968,14 @@ static int flash_write_cfibuffer (flash_info_t * info, ulong dest, uchar * cp,
 		}
 
 		flash_write_cmd (info, sector, 0, AMD_CMD_WRITE_BUFFER_CONFIRM);
+#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
+		retcode = flash_status_poll (info, src - (1 << shift),
+			dst - (1 << shift), info->buffer_write_tout, "buffer write");
+#else
 		retcode = flash_full_status_check (info, sector,
 						   info->buffer_write_tout,
 						   "buffer write");
+#endif
 		break;
 
 	default:
@@ -935,6 +997,10 @@ int flash_erase (flash_info_t * info, int s_first, int s_last)
 	int rcode = 0;
 	int prot;
 	flash_sect_t sect;
+#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
+	cfiword_t cword = (cfiword_t)0xffffffffffffffffULL;
+	void *dest;
+#endif
 
 	if (info->flash_id != FLASH_MAN_CFI) {
 		puts ("Can't erase unknown flash type - aborted\n");
@@ -998,6 +1064,16 @@ int flash_erase (flash_info_t * info, int s_first, int s_last)
 				break;
 			}
 
+#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
+			dest = flash_map (info, sect, 0);
+			flash_unmap (info, sect, 0, dest);
+			if ((info->vendor == CFI_CMDSET_AMD_EXTENDED ||
+			     info->vendor == CFI_CMDSET_AMD_STANDARD) &&
+			    flash_status_poll (info, &cword, dest,
+					      info->erase_blk_tout, "erase"))
+				rcode = 1;
+			else
+#endif
 			if (flash_full_status_check
 			    (info, sect, info->erase_blk_tout, "erase")) {
 				rcode = 1;
-- 
1.6.6.1

[U-Boot] [PATCH] cfi flash: add status polling method for amd flash

[Stefan Roese]

Hi Thomas,

On Tuesday 23 March 2010 04:24:08 Thomas Chou wrote:
> This patch adds status polling method to offer an alternative to
> data toggle method for amd flash chips.
> 
> This patch is needed for nios2 cfi flash interface, where the bus
> controller performs 4 bytes read cycles for a single byte read
> instruction. The data toggle method can not detect chip busy
> status correctly. So we have to poll DQ7, which will be inverted
> when the chip is busy.
> 
> This feature is enabled with the config def,
> CONFIG_SYS_CFI_FLASH_STATUS_POLL
> 
> Include patch, "drivers/mtd/cfi_flash: precision and underflow
> problem in tout calculation", submitted by
>   Alessandro Rubini <rub...@gnudd.com>
>   Renato Andreola <renato.andre...@imagos.it>

Thanks for taking care of this. But we shouldn't really integrate this patch 
into your flash_status_poll() patch. Please re-send this part as a separate 
patch (some subject as original patch with S-o-b line of original author 
please). Thanks again for this.

Please find still more enhancement suggestions below.
 
> Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
> ---
>  drivers/mtd/cfi_flash.c |   78
> ++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 77
> insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
> index fdba297..db22255 100644
> --- a/drivers/mtd/cfi_flash.c
> +++ b/drivers/mtd/cfi_flash.c
> @@ -537,7 +537,10 @@ static int flash_status_check (flash_info_t * info,
> flash_sect_t sector, ulong start;
> 
>  #if CONFIG_SYS_HZ != 1000
> -	tout *= CONFIG_SYS_HZ/1000;
> +	if ((ulong)CONFIG_SYS_HZ > 100000)
> +		tout *= (ulong)CONFIG_SYS_HZ / 1000;  /* for a big HZ, avoid 
overflow */
> +	else
> +		tout = DIV_ROUND_UP (tout * (ulong)CONFIG_SYS_HZ, 1000);
>  #endif
> 
>  	/* Wait for command completion */
> @@ -555,6 +558,53 @@ static int flash_status_check (flash_info_t * info,
> flash_sect_t sector, return ERR_OK;
>  }
> 
> +#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
> +static int flash_status_poll (flash_info_t *info, void *src, void *dst,
> +			     ulong tout, char *prompt)
> +{
> +	ulong start;
> +	int ready;
> +
> +#if CONFIG_SYS_HZ != 1000
> +	if ((ulong)CONFIG_SYS_HZ > 100000)
> +		tout *= (ulong)CONFIG_SYS_HZ / 1000;  /* for a big HZ, avoid 
overflow */
> +	else
> +		tout = DIV_ROUND_UP (tout * (ulong)CONFIG_SYS_HZ, 1000);
> +#endif
> +
> +	/* Wait for command completion */
> +	start = get_timer (0);
> +	while (1) {
> +		switch (info->portwidth) {
> +		case FLASH_CFI_8BIT:
> +			ready = flash_read8(dst) == flash_read8(src);
> +			break;
> +		case FLASH_CFI_16BIT:
> +			ready = flash_read16(dst) == flash_read16(src);
> +			break;
> +		case FLASH_CFI_32BIT:
> +			ready = flash_read32(dst) == flash_read32(src);
> +			break;
> +		case FLASH_CFI_64BIT:
> +			ready = flash_read64(dst) == flash_read64(src);
> +			break;
> +		default:
> +			ready = 0;
> +			break;
> +		}
> +		if (ready)
> +			break;
> +		if (get_timer (start) > tout) {
> +			printf ("Flash %s timeout at address %lx data %lx\n",
> +			       prompt, (ulong)dst, (ulong)flash_read8(dst));
> +			return ERR_TIMOUT;
> +		}
> +		udelay (1);		/* also triggers watchdog */
> +	}
> +	return ERR_OK;
> +}
> +#endif
> +
>  /*-----------------------------------------------------------------------
>   * Wait for XSR.7 to be set, if it times out print an error, otherwise
>   * do a full status check.
> @@ -749,6 +799,13 @@ static int flash_write_cfiword (flash_info_t * info,
> ulong dest, if (!sect_found)
>  		sect = find_sector (info, dest);
> 
> +#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
> +	if (info->vendor == CFI_CMDSET_AMD_EXTENDED ||
> +	    info->vendor == CFI_CMDSET_AMD_STANDARD)
> +		return flash_status_poll (info, &cword, dstaddr,
> +					 info->write_tout, "write");
> +	else
> +#endif
>  	return flash_full_status_check (info, sect, info->write_tout, 
"write");

I don't like this ugly "incorrect" indentation of the line below the "#endif" 
above. Perhaps you could implement this in another way. By moving the #ifdef 
and the vendor == amd check into a separate function:

static int use_flash_status_poll(flash_info_t *info)
{
#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
	if (info->vendor == CFI_CMDSET_AMD_EXTENDED ||
	    info->vendor == CFI_CMDSET_AMD_STANDARD)
		return 1;
#endif
	return 0;
}

And then use this code in flash_write_cfiword():

	if (use_flash_status_poll(info)) {
		return flash_status_poll(info, &cword, dstaddr,
					 info->write_tout, "write");
	} else {
		return flash_full_status_check(info, sect,
					       info->write_tout, "write");
	}

What do you think? Looks nicer to me. And we only need to implement this 
vendor == AMD check once (see below).

>  }
> 
> @@ -911,9 +968,14 @@ static int flash_write_cfibuffer (flash_info_t * info,
> ulong dest, uchar * cp, }
> 
>  		flash_write_cmd (info, sector, 0, 
AMD_CMD_WRITE_BUFFER_CONFIRM);
> +#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
> +		retcode = flash_status_poll (info, src - (1 << shift),
> +			dst - (1 << shift), info->buffer_write_tout, "buffer 
write");
> +#else
>  		retcode = flash_full_status_check (info, sector,
>  						   info->buffer_write_tout,
>  						   "buffer write");
> +#endif
>  		break;
> 
>  	default:
> @@ -935,6 +997,10 @@ int flash_erase (flash_info_t * info, int s_first, int
> s_last) int rcode = 0;
>  	int prot;
>  	flash_sect_t sect;
> +#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
> +	cfiword_t cword = (cfiword_t)0xffffffffffffffffULL;
> +	void *dest;
> +#endif
> 
>  	if (info->flash_id != FLASH_MAN_CFI) {
>  		puts ("Can't erase unknown flash type - aborted\n");
> @@ -998,6 +1064,16 @@ int flash_erase (flash_info_t * info, int s_first,
> int s_last) break;
>  			}
> 
> +#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
> +			dest = flash_map (info, sect, 0);
> +			flash_unmap (info, sect, 0, dest);
> +			if ((info->vendor == CFI_CMDSET_AMD_EXTENDED ||
> +			     info->vendor == CFI_CMDSET_AMD_STANDARD) &&
> +			    flash_status_poll (info, &cword, dest,
> +					      info->erase_blk_tout, "erase"))
> +				rcode = 1;
> +			else
> +#endif

Again, you could use the newly created function here.

Please give it a try and let me know if this works.

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] [Nios2-dev] [PATCH] cfi flash: add status polling method for amd flash

[Michael Schnell]

>> With 16 bit flash configurations I suppose the Avalon bus will do four Word reads instead of a single one

Supposedly it will do two Word reads, of course, still resulting in the same problem.

-Michael

[U-Boot] [PATCH v4] cfi flash: add status polling method for amd flash

[Thomas Chou]

This patch adds status polling method to offer an alternative to
data toggle method for amd flash chips.

This patch is needed for nios2 cfi flash interface, where the bus
controller performs 4 bytes read cycles for a single byte read
instruction. The data toggle method can not detect chip busy
status correctly. So we have to poll DQ7, which will be inverted
when the chip is busy.

This feature is enabled with the config def,
CONFIG_SYS_CFI_FLASH_STATUS_POLL

In flash_erase, the verbose printing of '.' is changed that it will
go with each sector whether successed or failed.

Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
---
 drivers/mtd/cfi_flash.c |   95 +++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 87 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index cd1a86e..aae93bd 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -605,6 +605,63 @@ static int flash_full_status_check (flash_info_t * info, flash_sect_t sector,
 	return retcode;
 }
 
+static int use_flash_status_poll(flash_info_t *info)
+{
+#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
+	if (info->vendor == CFI_CMDSET_AMD_EXTENDED ||
+	    info->vendor == CFI_CMDSET_AMD_STANDARD)
+		return 1;
+#endif
+	return 0;
+}
+
+static int flash_status_poll(flash_info_t *info, void *src, void *dst,
+			     ulong tout, char *prompt)
+{
+#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
+	ulong start;
+	int ready;
+
+#if CONFIG_SYS_HZ != 1000
+	if ((ulong)CONFIG_SYS_HZ > 100000)
+		tout *= (ulong)CONFIG_SYS_HZ / 1000;  /* for a big HZ, avoid overflow */
+	else
+		tout = DIV_ROUND_UP(tout * (ulong)CONFIG_SYS_HZ, 1000);
+#endif
+
+	/* Wait for command completion */
+	start = get_timer(0);
+	while (1) {
+		switch (info->portwidth) {
+		case FLASH_CFI_8BIT:
+			ready = flash_read8(dst) == flash_read8(src);
+			break;
+		case FLASH_CFI_16BIT:
+			ready = flash_read16(dst) == flash_read16(src);
+			break;
+		case FLASH_CFI_32BIT:
+			ready = flash_read32(dst) == flash_read32(src);
+			break;
+		case FLASH_CFI_64BIT:
+			ready = flash_read64(dst) == flash_read64(src);
+			break;
+		default:
+			ready = 0;
+			break;
+		}
+		if (ready)
+			break;
+		if (get_timer(start) > tout) {
+			printf("Flash %s timeout at address %lx data %lx\n",
+			       prompt, (ulong)dst, (ulong)flash_read8(dst));
+			return ERR_TIMOUT;
+		}
+		udelay(1);		/* also triggers watchdog */
+	}
+#endif /* CONFIG_SYS_CFI_FLASH_STATUS_POLL */
+	return ERR_OK;
+}
+
 /*-----------------------------------------------------------------------
  */
 static void flash_add_byte (flash_info_t * info, cfiword_t * cword, uchar c)
@@ -752,7 +809,12 @@ static int flash_write_cfiword (flash_info_t * info, ulong dest,
 	if (!sect_found)
 		sect = find_sector (info, dest);
 
-	return flash_full_status_check (info, sect, info->write_tout, "write");
+	if (use_flash_status_poll(info))
+		return flash_status_poll(info, &cword, dstaddr,
+					 info->write_tout, "write");
+	else
+		return flash_full_status_check(info, sect,
+					       info->write_tout, "write");
 }
 
 #ifdef CONFIG_SYS_FLASH_USE_BUFFER_WRITE
@@ -914,9 +976,15 @@ static int flash_write_cfibuffer (flash_info_t * info, ulong dest, uchar * cp,
 		}
 
 		flash_write_cmd (info, sector, 0, AMD_CMD_WRITE_BUFFER_CONFIRM);
-		retcode = flash_full_status_check (info, sector,
-						   info->buffer_write_tout,
-						   "buffer write");
+		if (use_flash_status_poll(info))
+			retcode = flash_status_poll(info, src - (1 << shift),
+						    dst - (1 << shift),
+						    info->buffer_write_tout,
+						    "buffer write");
+		else
+			retcode = flash_full_status_check(info, sector,
+							  info->buffer_write_tout,
+							  "buffer write");
 		break;
 
 	default:
@@ -1001,10 +1069,21 @@ int flash_erase (flash_info_t * info, int s_first, int s_last)
 				break;
 			}
 
-			if (flash_full_status_check
-			    (info, sect, info->erase_blk_tout, "erase")) {
-				rcode = 1;
-			} else if (flash_verbose)
+			if (use_flash_status_poll(info)) {
+				cfiword_t cword = (cfiword_t)0xffffffffffffffffULL;
+				void *dest;
+				dest = flash_map(info, sect, 0);
+				if (flash_status_poll(info, &cword, dest,
+						      info->erase_blk_tout, "erase"))
+					rcode = 1;
+				flash_unmap(info, sect, 0, dest);
+			} else {
+				if (flash_full_status_check(info, sect,
+							    info->erase_blk_tout,
+							    "erase"))
+					rcode = 1;
+			}
+			if (flash_verbose)
 				putc ('.');
 		}
 	}
-- 
1.6.6.1

[U-Boot] [PATCH] cfi flash: add status polling method for amd flash

[Thomas Chou]

On 03/23/2010 06:12 PM, Stefan Roese wrote:
> Hi Thomas,
>
> On Tuesday 23 March 2010 04:24:08 Thomas Chou wrote:
>    
>> This patch adds status polling method to offer an alternative to
>> data toggle method for amd flash chips.
>>
>> This patch is needed for nios2 cfi flash interface, where the bus
>> controller performs 4 bytes read cycles for a single byte read
>> instruction. The data toggle method can not detect chip busy
>> status correctly. So we have to poll DQ7, which will be inverted
>> when the chip is busy.
>>
>> This feature is enabled with the config def,
>> CONFIG_SYS_CFI_FLASH_STATUS_POLL
>>
>> Include patch, "drivers/mtd/cfi_flash: precision and underflow
>> problem in tout calculation", submitted by
>>    Alessandro Rubini<rub...@gnudd.com>
>>    Renato Andreola<renato.andre...@imagos.it>
>>      
> Thanks for taking care of this. But we shouldn't really integrate this patch
> into your flash_status_poll() patch. Please re-send this part as a separate
> patch (some subject as original patch with S-o-b line of original author
> please). Thanks again for this.
>
> Please find still more enhancement suggestions below.
>    
Hi Stefen,

I resent the tout patch from Renato. I have updated  and tested the 
status poll patch as you suggested. Any suggestion and improvement is 
always welcome.

Cheers,
Thomas

[U-Boot] [Nios2-dev] [PATCH] cfi flash: add status polling method for amd flash

[Thomas Chou]

On 03/23/2010 06:39 PM, Michael Schnell wrote:
>>> With 16 bit flash configurations I suppose the Avalon bus will do four Word reads instead of a single one
>>>        
> Supposedly it will do two Word reads, of course, still resulting in the same problem.
>    
Hi Michael,

The write cycle of the avalon bus controller is correct. Just the read 
cycle (8 bits or 16 bits) caused problem with toggle status checking.

There is no problem with Intel flash, because the busy checking is 
different.

In u-boot, there are nios2 specific flash drivers 
(board/altera/common/flash.c and AMDLV065D.c) to handle status polling 
for amd flash. But they didn't use the cfi info to detect flash 
parameters. You have to define them for each board.

I have tested 8 bits/16 bits amd flash and 16 bits intel flash on four 
altera dev boards without problem.

- Thomas

[U-Boot] [PATCH v4] cfi flash: add status polling method for amd flash

[Stefan Roese]

On Wednesday 24 March 2010 03:29:21 Thomas Chou wrote:
> This patch adds status polling method to offer an alternative to
> data toggle method for amd flash chips.
> 
> This patch is needed for nios2 cfi flash interface, where the bus
> controller performs 4 bytes read cycles for a single byte read
> instruction. The data toggle method can not detect chip busy
> status correctly. So we have to poll DQ7, which will be inverted
> when the chip is busy.
> 
> This feature is enabled with the config def,
> CONFIG_SYS_CFI_FLASH_STATUS_POLL
> 
> In flash_erase, the verbose printing of '.' is changed that it will
> go with each sector whether successed or failed.

Applied to u-boot-cfi-flash/next. 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] [PATCH v4] cfi flash: add status polling method for amd flash

[Wolfgang Denk]

Dear Thomas Chou,

In message <1269397761-23394-1-git-send-email-thomas@wytron.com.tw> you wrote:
> This patch adds status polling method to offer an alternative to
> data toggle method for amd flash chips.
> 
> This patch is needed for nios2 cfi flash interface, where the bus
> controller performs 4 bytes read cycles for a single byte read
> instruction. The data toggle method can not detect chip busy
> status correctly. So we have to poll DQ7, which will be inverted
> when the chip is busy.
> 
> This feature is enabled with the config def,
> CONFIG_SYS_CFI_FLASH_STATUS_POLL
> 
> In flash_erase, the verbose printing of '.' is changed that it will
> go with each sector whether successed or failed.

Please split this part off into a separate patch. This is a global
change to the user interface and must be discussed separately.

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
The easiest way to figure the cost of living is to take  your  income
and add ten percent.

[U-Boot] [PATCH v4] cfi flash: add status polling method for amd flash

[Wolfgang Denk]

Dear Stefan Roese,

In message <201003250943.17140.sr@denx.de> you wrote:
> On Wednesday 24 March 2010 03:29:21 Thomas Chou wrote:
> > This patch adds status polling method to offer an alternative to
> > data toggle method for amd flash chips.
> > 
> > This patch is needed for nios2 cfi flash interface, where the bus
> > controller performs 4 bytes read cycles for a single byte read
> > instruction. The data toggle method can not detect chip busy
> > status correctly. So we have to poll DQ7, which will be inverted
> > when the chip is busy.
> > 
> > This feature is enabled with the config def,
> > CONFIG_SYS_CFI_FLASH_STATUS_POLL
> > 
> > In flash_erase, the verbose printing of '.' is changed that it will
> > go with each sector whether successed or failed.
> 
> Applied to u-boot-cfi-flash/next. Thanks.

I object against pulling in a global change to the user interface just
like that, without any discussion.

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
No journaling file system can recover your data if the disk dies.
                                 - Steve Rago in <D4Cw1p.L9E@plc.com>

[U-Boot] [PATCH v4] cfi flash: add status polling method for amd flash

[Thomas Chou]

On 03/25/2010 07:49 PM, Wolfgang Denk wrote:
> Dear Thomas Chou,
>
> In message<1269397761-23394-1-git-send-email-thomas@wytron.com.tw>  you wrote:
>    
>> This patch adds status polling method to offer an alternative to
>> data toggle method for amd flash chips.
>>
>> This patch is needed for nios2 cfi flash interface, where the bus
>> controller performs 4 bytes read cycles for a single byte read
>> instruction. The data toggle method can not detect chip busy
>> status correctly. So we have to poll DQ7, which will be inverted
>> when the chip is busy.
>>
>> This feature is enabled with the config def,
>> CONFIG_SYS_CFI_FLASH_STATUS_POLL
>>
>> In flash_erase, the verbose printing of '.' is changed that it will
>> go with each sector whether successed or failed.
>>      
> Please split this part off into a separate patch. This is a global
> change to the user interface and must be discussed separately.
>
> Best regards,
>
> Wolfgang Denk
>
>    
Hi Wolfgang and Stefan,

I will restore the original printing so that the user interface won't 
change.

Best regards,
Thomas

[U-Boot] [PATCH v5] cfi flash: add status polling method for amd flash

[Thomas Chou]

This patch adds status polling method to offer an alternative to
data toggle method for amd flash chips.

This patch is needed for nios2 cfi flash interface, where the bus
controller performs 4 bytes read cycles for a single byte read
instruction. The data toggle method can not detect chip busy
status correctly. So we have to poll DQ7, which will be inverted
when the chip is busy.

This feature is enabled with the config def,
CONFIG_SYS_CFI_FLASH_STATUS_POLL

Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
---
 drivers/mtd/cfi_flash.c |   99 ++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 90 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index cd1a86e..8d2c623 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -605,6 +605,63 @@ static int flash_full_status_check (flash_info_t * info, flash_sect_t sector,
 	return retcode;
 }
 
+static int use_flash_status_poll(flash_info_t *info)
+{
+#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
+	if (info->vendor == CFI_CMDSET_AMD_EXTENDED ||
+	    info->vendor == CFI_CMDSET_AMD_STANDARD)
+		return 1;
+#endif
+	return 0;
+}
+
+static int flash_status_poll(flash_info_t *info, void *src, void *dst,
+			     ulong tout, char *prompt)
+{
+#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
+	ulong start;
+	int ready;
+
+#if CONFIG_SYS_HZ != 1000
+	if ((ulong)CONFIG_SYS_HZ > 100000)
+		tout *= (ulong)CONFIG_SYS_HZ / 1000;  /* for a big HZ, avoid overflow */
+	else
+		tout = DIV_ROUND_UP(tout * (ulong)CONFIG_SYS_HZ, 1000);
+#endif
+
+	/* Wait for command completion */
+	start = get_timer(0);
+	while (1) {
+		switch (info->portwidth) {
+		case FLASH_CFI_8BIT:
+			ready = flash_read8(dst) == flash_read8(src);
+			break;
+		case FLASH_CFI_16BIT:
+			ready = flash_read16(dst) == flash_read16(src);
+			break;
+		case FLASH_CFI_32BIT:
+			ready = flash_read32(dst) == flash_read32(src);
+			break;
+		case FLASH_CFI_64BIT:
+			ready = flash_read64(dst) == flash_read64(src);
+			break;
+		default:
+			ready = 0;
+			break;
+		}
+		if (ready)
+			break;
+		if (get_timer(start) > tout) {
+			printf("Flash %s timeout at address %lx data %lx\n",
+			       prompt, (ulong)dst, (ulong)flash_read8(dst));
+			return ERR_TIMOUT;
+		}
+		udelay(1);		/* also triggers watchdog */
+	}
+#endif /* CONFIG_SYS_CFI_FLASH_STATUS_POLL */
+	return ERR_OK;
+}
+
 /*-----------------------------------------------------------------------
  */
 static void flash_add_byte (flash_info_t * info, cfiword_t * cword, uchar c)
@@ -752,7 +809,12 @@ static int flash_write_cfiword (flash_info_t * info, ulong dest,
 	if (!sect_found)
 		sect = find_sector (info, dest);
 
-	return flash_full_status_check (info, sect, info->write_tout, "write");
+	if (use_flash_status_poll(info))
+		return flash_status_poll(info, &cword, dstaddr,
+					 info->write_tout, "write");
+	else
+		return flash_full_status_check(info, sect,
+					       info->write_tout, "write");
 }
 
 #ifdef CONFIG_SYS_FLASH_USE_BUFFER_WRITE
@@ -914,9 +976,15 @@ static int flash_write_cfibuffer (flash_info_t * info, ulong dest, uchar * cp,
 		}
 
 		flash_write_cmd (info, sector, 0, AMD_CMD_WRITE_BUFFER_CONFIRM);
-		retcode = flash_full_status_check (info, sector,
-						   info->buffer_write_tout,
-						   "buffer write");
+		if (use_flash_status_poll(info))
+			retcode = flash_status_poll(info, src - (1 << shift),
+						    dst - (1 << shift),
+						    info->buffer_write_tout,
+						    "buffer write");
+		else
+			retcode = flash_full_status_check(info, sector,
+							  info->buffer_write_tout,
+							  "buffer write");
 		break;
 
 	default:
@@ -1001,11 +1069,24 @@ int flash_erase (flash_info_t * info, int s_first, int s_last)
 				break;
 			}
 
-			if (flash_full_status_check
-			    (info, sect, info->erase_blk_tout, "erase")) {
-				rcode = 1;
-			} else if (flash_verbose)
-				putc ('.');
+			if (use_flash_status_poll(info)) {
+				cfiword_t cword = (cfiword_t)0xffffffffffffffffULL;
+				void *dest;
+				dest = flash_map(info, sect, 0);
+				if (flash_status_poll(info, &cword, dest,
+						      info->erase_blk_tout, "erase"))
+					rcode = 1;
+				else if (flash_verbose)
+					putc('.');
+				flash_unmap(info, sect, 0, dest);
+			} else {
+				if (flash_full_status_check(info, sect,
+							    info->erase_blk_tout,
+							    "erase"))
+					rcode = 1;
+				else if (flash_verbose)
+					putc('.');
+			}
 		}
 	}
 
-- 
1.6.6.1

[U-Boot] [PATCH v6] cfi flash: add status polling method for amd flash

[Thomas Chou]

This patch adds status polling method to offer an alternative to
data toggle method for amd flash chips.

This patch is needed for nios2 cfi flash interface, where the bus
controller performs 4 bytes read cycles for a single byte read
instruction. The data toggle method can not detect chip busy
status correctly. So we have to poll DQ7, which will be inverted
when the chip is busy.

This feature is enabled with the config def,
CONFIG_SYS_CFI_FLASH_STATUS_POLL

Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
---
 drivers/mtd/cfi_flash.c |   93 +++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 86 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index cd1a86e..d0240f5 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -605,6 +605,63 @@ static int flash_full_status_check (flash_info_t * info, flash_sect_t sector,
 	return retcode;
 }
 
+static int use_flash_status_poll(flash_info_t *info)
+{
+#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
+	if (info->vendor == CFI_CMDSET_AMD_EXTENDED ||
+	    info->vendor == CFI_CMDSET_AMD_STANDARD)
+		return 1;
+#endif
+	return 0;
+}
+
+static int flash_status_poll(flash_info_t *info, void *src, void *dst,
+			     ulong tout, char *prompt)
+{
+#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
+	ulong start;
+	int ready;
+
+#if CONFIG_SYS_HZ != 1000
+	if ((ulong)CONFIG_SYS_HZ > 100000)
+		tout *= (ulong)CONFIG_SYS_HZ / 1000;  /* for a big HZ, avoid overflow */
+	else
+		tout = DIV_ROUND_UP(tout * (ulong)CONFIG_SYS_HZ, 1000);
+#endif
+
+	/* Wait for command completion */
+	start = get_timer(0);
+	while (1) {
+		switch (info->portwidth) {
+		case FLASH_CFI_8BIT:
+			ready = flash_read8(dst) == flash_read8(src);
+			break;
+		case FLASH_CFI_16BIT:
+			ready = flash_read16(dst) == flash_read16(src);
+			break;
+		case FLASH_CFI_32BIT:
+			ready = flash_read32(dst) == flash_read32(src);
+			break;
+		case FLASH_CFI_64BIT:
+			ready = flash_read64(dst) == flash_read64(src);
+			break;
+		default:
+			ready = 0;
+			break;
+		}
+		if (ready)
+			break;
+		if (get_timer(start) > tout) {
+			printf("Flash %s timeout at address %lx data %lx\n",
+			       prompt, (ulong)dst, (ulong)flash_read8(dst));
+			return ERR_TIMOUT;
+		}
+		udelay(1);		/* also triggers watchdog */
+	}
+#endif /* CONFIG_SYS_CFI_FLASH_STATUS_POLL */
+	return ERR_OK;
+}
+
 /*-----------------------------------------------------------------------
  */
 static void flash_add_byte (flash_info_t * info, cfiword_t * cword, uchar c)
@@ -752,7 +809,12 @@ static int flash_write_cfiword (flash_info_t * info, ulong dest,
 	if (!sect_found)
 		sect = find_sector (info, dest);
 
-	return flash_full_status_check (info, sect, info->write_tout, "write");
+	if (use_flash_status_poll(info))
+		return flash_status_poll(info, &cword, dstaddr,
+					 info->write_tout, "write");
+	else
+		return flash_full_status_check(info, sect,
+					       info->write_tout, "write");
 }
 
 #ifdef CONFIG_SYS_FLASH_USE_BUFFER_WRITE
@@ -914,9 +976,15 @@ static int flash_write_cfibuffer (flash_info_t * info, ulong dest, uchar * cp,
 		}
 
 		flash_write_cmd (info, sector, 0, AMD_CMD_WRITE_BUFFER_CONFIRM);
-		retcode = flash_full_status_check (info, sector,
-						   info->buffer_write_tout,
-						   "buffer write");
+		if (use_flash_status_poll(info))
+			retcode = flash_status_poll(info, src - (1 << shift),
+						    dst - (1 << shift),
+						    info->buffer_write_tout,
+						    "buffer write");
+		else
+			retcode = flash_full_status_check(info, sector,
+							  info->buffer_write_tout,
+							  "buffer write");
 		break;
 
 	default:
@@ -938,6 +1006,7 @@ int flash_erase (flash_info_t * info, int s_first, int s_last)
 	int rcode = 0;
 	int prot;
 	flash_sect_t sect;
+	int st;
 
 	if (info->flash_id != FLASH_MAN_CFI) {
 		puts ("Can't erase unknown flash type - aborted\n");
@@ -1001,10 +1070,20 @@ int flash_erase (flash_info_t * info, int s_first, int s_last)
 				break;
 			}
 
-			if (flash_full_status_check
-			    (info, sect, info->erase_blk_tout, "erase")) {
+			if (use_flash_status_poll(info)) {
+				cfiword_t cword = (cfiword_t)0xffffffffffffffffULL;
+				void *dest;
+				dest = flash_map(info, sect, 0);
+				st = flash_status_poll(info, &cword, dest,
+						       info->erase_blk_tout, "erase");
+				flash_unmap(info, sect, 0, dest);
+			} else
+				st = flash_full_status_check(info, sect,
+							     info->erase_blk_tout,
+							     "erase");
+			if (st)
 				rcode = 1;
-			} else if (flash_verbose)
+			else if (flash_verbose)
 				putc ('.');
 		}
 	}
-- 
1.6.6.1

[U-Boot] [PATCH v4] cfi flash: add status polling method for amd flash

[Stefan Roese]

Hi Wolfgang,

On Thursday 25 March 2010 12:49:54 Wolfgang Denk wrote:
> > > In flash_erase, the verbose printing of '.' is changed that it will
> > > go with each sector whether successed or failed.
> > 
> > Applied to u-boot-cfi-flash/next. Thanks.
> 
> I object against pulling in a global change to the user interface just
> like that, without any discussion.

OK, I'll remove this patch and push the new version soon.

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] [PATCH] cfi flash: add status polling method for amd flash

[Thomas Chou]

This patch adds status polling method to offer an alternative to
data toggle method for amd flash chips.

This patch is needed for nios2 cfi flash interface, where the bus
controller performs 4 bytes read cycles for a single byte read
instruction. The data toggle method can not detect chip busy
status correctly. So we have to poll DQ7, which will be inverted
when the chip is busy.

This feature is enabled with the config def,
CONFIG_SYS_CFI_FLASH_STATUS_POLL

Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
---
 drivers/mtd/cfi_flash.c |   91 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 91 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
index 24eb33f..dd02ed1 100644
--- a/drivers/mtd/cfi_flash.c
+++ b/drivers/mtd/cfi_flash.c
@@ -555,6 +555,50 @@ static int flash_status_check (flash_info_t * info, flash_sect_t sector,
 	return ERR_OK;
 }
 
+#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
+static int flash_status_poll (flash_info_t * info, cfiword_t cword, void *addr,
+			       ulong tout, char *prompt)
+{
+	ulong start;
+	int ready;
+
+#if CONFIG_SYS_HZ != 1000
+	tout *= CONFIG_SYS_HZ/1000;
+#endif
+
+	/* Wait for command completion */
+	start = get_timer (0);
+	while (1) {
+		switch (info->portwidth) {
+		case FLASH_CFI_8BIT:
+			ready = flash_read8(addr) == cword.c;
+			break;
+		case FLASH_CFI_16BIT:
+			ready = flash_read16(addr) == cword.w;
+			break;
+		case FLASH_CFI_32BIT:
+			ready = flash_read32(addr) == cword.l;
+			break;
+		case FLASH_CFI_64BIT:
+			ready = flash_read64(addr) == cword.ll;
+			break;
+		default:
+			ready = 0;
+			break;
+		}
+		if (ready)
+			break;
+		if (get_timer (start) > tout) {
+			printf ("Flash %s timeout at address %lx data %lx\n",
+				prompt, (ulong)addr, (ulong)flash_read8 (addr));
+			return ERR_TIMOUT;
+		}
+		udelay (1);		/* also triggers watchdog */
+	}
+	return ERR_OK;
+}
+#endif
+
 /*-----------------------------------------------------------------------
  * Wait for XSR.7 to be set, if it times out print an error, otherwise
  * do a full status check.
@@ -749,6 +793,13 @@ static int flash_write_cfiword (flash_info_t * info, ulong dest,
 	if (!sect_found)
 		sect = find_sector (info, dest);
 
+#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
+	if (info->vendor == CFI_CMDSET_AMD_EXTENDED ||
+	    info->vendor == CFI_CMDSET_AMD_STANDARD)
+		return flash_status_poll (info, cword, dstaddr,
+					  info->write_tout, "write");
+	else
+#endif
 	return flash_full_status_check (info, sect, info->write_tout, "write");
 }
 
@@ -767,6 +818,7 @@ static int flash_write_cfibuffer (flash_info_t * info, ulong dest, uchar * cp,
 	uint offset = 0;
 	unsigned int shift;
 	uchar write_cmd;
+	cfiword_t cword;
 
 	switch (info->portwidth) {
 	case FLASH_CFI_8BIT:
@@ -911,9 +963,35 @@ static int flash_write_cfibuffer (flash_info_t * info, ulong dest, uchar * cp,
 		}
 
 		flash_write_cmd (info, sector, 0, AMD_CMD_WRITE_BUFFER_CONFIRM);
+#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
+		switch (info->portwidth) {
+		case FLASH_CFI_8BIT:
+			cword.c = flash_read8(src - 1);
+			dst -= 1;
+			break;
+		case FLASH_CFI_16BIT:
+			cword.w = flash_read16(src - 2);
+			dst -= 2;
+			break;
+		case FLASH_CFI_32BIT:
+			cword.l = flash_read32(src - 4);
+			dst -= 4;
+			break;
+		case FLASH_CFI_64BIT:
+			cword.ll = flash_read64(src - 8);
+			dst -= 8;
+			break;
+		default:
+			retcode = ERR_INVAL;
+			goto out_unmap;
+		}
+		retcode = flash_status_poll (info, cword, dst,
+					     info->buffer_write_tout, "buffer write");
+#else
 		retcode = flash_full_status_check (info, sector,
 						   info->buffer_write_tout,
 						   "buffer write");
+#endif
 		break;
 
 	default:
@@ -935,6 +1013,8 @@ int flash_erase (flash_info_t * info, int s_first, int s_last)
 	int rcode = 0;
 	int prot;
 	flash_sect_t sect;
+	cfiword_t cword = (cfiword_t) 0xffffffffffffffffULL;
+	ulong dest;
 
 	if (info->flash_id != FLASH_MAN_CFI) {
 		puts ("Can't erase unknown flash type - aborted\n");
@@ -998,6 +1078,17 @@ int flash_erase (flash_info_t * info, int s_first, int s_last)
 				break;
 			}
 
+#ifdef CONFIG_SYS_CFI_FLASH_STATUS_POLL
+			dest = (ulong) flash_map(info, sect, 0);
+			flash_unmap(info, sect, 0, (void *)dest);
+			if ((info->vendor == CFI_CMDSET_AMD_EXTENDED ||
+			     info->vendor == CFI_CMDSET_AMD_STANDARD) &&
+			    flash_status_poll (info, cword, (void *)dest,
+					       info->erase_blk_tout, "erase"))
+				rcode = 1;
+			else
+
+#endif
 			if (flash_full_status_check
 			    (info, sect, info->erase_blk_tout, "erase")) {
 				rcode = 1;
-- 
1.6.6.1