[U-Boot] [PATCH] cfi_flash: don't hide write/erase errors

[Baruch Siach]

Hi Stefan,

On Mon, Oct 06, 2014 at 10:20:43AM +0200, Stefan Roese wrote:
> On 06.10.2014 08:19, Baruch Siach wrote:
> >On Thu, Sep 04, 2014 at 12:23:09PM +0300, Baruch Siach wrote:
> >>Partially revert commit 0d01f66d235118 (CFI: cfi_flash write fix for AMD
> >>legacy).
> >>
> >>flash_full_status_check() used to skip status register parsing when
> >>flash_status_check() returns OK. This is wrong since flash_status_check()
> >>must return OK for other status bits to be valid.
> >>
> >>Cc: Ed Swarthout <Ed.Swarthout at freescale.com>
> >>Signed-off-by: Baruch Siach <baruch at tkos.co.il>
> >>---
> >>  drivers/mtd/cfi_flash.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >>diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c
> >>index c4b5bc1de553..9b3175d87fbd 100644
> >>--- a/drivers/mtd/cfi_flash.c
> >>+++ b/drivers/mtd/cfi_flash.c
> >>@@ -593,7 +593,7 @@ static int flash_full_status_check (flash_info_t * info, flash_sect_t sector,
> >>  	case CFI_CMDSET_INTEL_PROG_REGIONS:
> >>  	case CFI_CMDSET_INTEL_EXTENDED:
> >>  	case CFI_CMDSET_INTEL_STANDARD:
> >>-		if ((retcode != ERR_OK)
> >>+		if ((retcode == ERR_OK)
> >>  		    && !flash_isequal (info, sector, 0, FLASH_STATUS_DONE)) {
> >>  			retcode = ERR_INVAL;
> >>  			printf ("Flash %s error at address %lx\n", prompt,
> >
> >Ping?
> 
> Sorry, I forgot about this one.
> 
> I have to admit that I'm a bit hesitant here. Since your patch changes the
> behavior thats present for about than 6 years. You're the first encountering
> some problems here. And I'm not that actively using CFI NOR flash anymore as
> well, so my knowledge is a bit "rusty" here as well.
> 
> Could you please summarize again, what the real problem with this compare
> is. What is the error exactly in your case (which flash chip is used and
> which command was issued?)?

The Micron StrataFlash datasheet says this on Status Register bit 7 (Device 
status):

0 = Device is busy; SR[9,8,6:1] are invalid, SR[0] is valid
1 = Device is ready; SR[9:8], SR[6:1] are valid

This is was the original code behaviour, since your commit 
79b4cda076069d04122f. This commit log explicitly says:

    * Changes/fixes for drivers/cfi_flash.c:
      We *should* check if there are any error bits if the previous call
      returned ERR_OK (Otherwise we will have output an error message in
      flash_status_check() already.)  The original code would only check for
      error bits if flash_status_check() returns ERR_TIMEOUT.
      Patch by Marcus Hall, 23 Aug 2005

Currently U-Boot is just silent about NOR flash write errors.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -