Hi Thomas,
On Friday 19 March 2010 23:57:47 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
Thanks. Please find some comments below.
> Signed-off-by: Thomas Chou <tho...@wytron.com.tw>
> ---
> drivers/mtd/cfi_flash.c | 94
> ++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 92
> insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
> index fdba297..4baa9dc 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
I realise that this is copied from flash_status_check(). There has been a
discussion and a not yet finished patch for this part. Please take a look at
this thread:
http://www.mail-archive.com/u-boot@lists.denx.de/msg19297.html
And the last version was posted here, with still a smaller change to be made:
http://www.mail-archive.com/u-boot@lists.denx.de/msg20515.html
It would be great if you could handle this in your patch as suggested in this
email thread.
> + /* 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;
Won't this result in a "unused variable" warning, when
CONFIG_SYS_CFI_FLASH_STATUS_POLL is not defined?
> 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");
Hmmm. I don't like this extra switch statement here. Thinking more about it,
wouldn't it make more sense to add this code into the new flash_status_poll()
functions instead and don't pass cword at all?
> +#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;
Again, doesn't this introduce compilation warnings?
>
> 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
> +
Please remove this empty line.
> +#endif
> if (flash_full_status_check
> (info, sect, info->erase_blk_tout, "erase")) {
> rcode = 1;
> @@ -1980,8 +2071,7 @@ unsigned long flash_init (void)
> }
>
> /* Monitor protection ON by default */
> -#if (CONFIG_SYS_MONITOR_BASE >= CONFIG_SYS_FLASH_BASE) && \
> - (!defined(CONFIG_MONITOR_IS_IN_RAM))
> +#if (CONFIG_SYS_MONITOR_BASE >= CONFIG_SYS_FLASH_BASE)
This seems wrong. You probably based your patch on an older driver version.
Please rebase against the latest version.
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: off...@denx.de
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
