[U-Boot] [PATCH] Fix CFI flash driver for 8-bit bus support

[Aaron Williams]

Hi Stefan,

It looks like the other reset is not needed. The delay is needed. Without it 
sometimes the reset would fail on some of our boards.

Here's what Garret Swalling at Spansion told me:

...
The CFI reset calls into two subroutines that resove to:
        flash_write_cmd(info, 0, 0, AMD_CMD_RESET);
        flash_write_cmd(info, 0, 0, FLASH_CMD_RESET);

According to the GL-N datasheet, even when there is no embedded operation 
ongoing, we need to allow at least 500ns for the reset to complete.  Maybe the 
extra command write and two additional function returns are allowing the flash 
enough time to complete the reset and respond to the next command.
---

While it seems to work without the delay at least on the one board I'm testing 
at the moment, I added it at at suggestion of Garret. I could try testing on 
all of the boards I'm supporting but I'd prefer not to (I'm supporting 15 
different boards and 7 different processor families at the moment).

-Aaron

On Tuesday, April 12, 2011 01:09:18 AM Stefan Roese wrote:
> Hi Aaron,
> 
> On Tuesday 12 April 2011 09:46:22 Aaron Williams wrote:
> > I believe this is correct.
> 
> Hmmm, I'm still not convinced about this reset call in the erase function.
> Do you really need it for the CFI driver to work correctly on your board?
> Could you please test without this reset command?
> 
> > I have redone the patch as two patches. The
> > first patch fixes the 8-bit addressing and has been tested with both 8
> > and 16-bit support with Spansion. The second patch adds a 1us delay
> > after every reset call. In my email correspondence with Spansion they
> > said that at least some devices require a 500ns delay after the reset
> > command.
> 
> Thanks. The only real concern I still have is the new reset command as
> mentioned above. Please either explain why it is needed, or remove it from
> your patch. I'll push the patches upstream once this issue is resolved.
> 
> <snip>
> 
> > > > @@ -1043,6 +1049,8 @@ int flash_erase (flash_info_t * info, int
> > > > s_first, int s_last) int prot;
> > > > 
> > > >  	flash_sect_t sect;
> > > >  	int st;
> > > > 
> > > > +
> > > > +	debug("%s: erasing sectors %d to %d\n", __func__, s_first, s_last);
> > > > 
> > > >  	if (info->flash_id != FLASH_MAN_CFI) {
> > > >  	
> > > >  		puts ("Can't erase unknown flash type - aborted\n");
> > > > 
> > > > @@ -1082,6 +1090,7 @@ int flash_erase (flash_info_t * info, int
> > > > s_first, int s_last) break;
> > > > 
> > > >  			case CFI_CMDSET_AMD_STANDARD:
> > > > 
> > > >  			case CFI_CMDSET_AMD_EXTENDED:
> > > > +				flash_write_cmd (info, 0, 0, AMD_CMD_RESET);
> > > 
> > > Is this correct? A reset command in every flash_erase() call?
> > > 
> > > Thanks.
> > > 
> > > Cheers,
> > > Stefan
> 
> 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