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