[U-Boot] [PATCHv2 CFI flash]: Workaround for Numonyx Axcell P33/P30 256-Mbit 65nm bug

[Philippe De Muyter <phdm@macqel.be>]

Hi Stefan,

On Tue, Aug 10, 2010 at 01:20:42PM +0200, Stefan Roese wrote:
> Hi Philippe,
> 
> On Monday 09 August 2010 17:46:03 Philippe De Muyter wrote:
> > > Please move this variable declaration of "cmd" and assignment further
> > > down (see below). You can remove the "{" "}" of this block then.
> > 
> > Here is the excerpt from the errata :
> > 
> > Workaround:  If the interval between 60h and its subsequent command
> > 	     can be guaranteed within 20?s, Option I is recommended,
> > 	     otherwise Option II (involves hardware) should be selected.
> > Option I: The table below lists the detail command sequences:
> > Command
> > 	      Data bus           Address bus       Remarks
> > Sequence
> >   1              90h            Block Address
> > 						   Read Lock Status
> >   2             Read         Block Address + 02h
> >  (2)(3)                                      (1)
> > 3                60h           Block Address
> >  (2)(3)                                      (1)   Lock/Unlock/RCR
> > Configuration 4           D0h/01h/03h        Block Address
> > Notes:
> > (1) Block Address refers to RCR configuration data only when the 60h
> >     command sequence is used to set RCR register combined with 03h
> >     subsequent command.
> > (2) For the third and fourth command sequences, the Block Address must
> >     be the same.
> > (3) The interval between 60h command and its subsequent D0h/01h/2Fh/03h
> >     commands should be less than 20?s.
> > 
> > Because of requirement (3), I choosed to minimize the number of
> > instructions between the `read lock status' and the `unlock' commands,
> > hence the initialisation of `cmd' moved before the `read lock status'
> > (hidden in `flash_isequal').

Yes, you're right.  I had misread the doc :(

> 
> From my understanding, "only" the last 2 operations need to be in max.
> 20?s interval. If this is the case, then I would prefer this code
> version:

I prefer it too.

> 
> 		case CFI_CMDSET_INTEL_EXTENDED:
> 			/*
> 			 * see errata called
> 			 * "Numonyx Axcell P33/P30 Specification Update" :)
> 			 */
> 			flash_write_cmd (info, sector, 0, FLASH_CMD_READ_ID);
> 			if (!flash_isequal (info, sector, FLASH_OFFSET_PROTECT,
> 					    prot)) {
> 				int flag = disable_interrupts ();
> 				unsigned short cmd;
> 
> 				if (prot)
> 					cmd = FLASH_CMD_PROTECT_SET;
> 				else
> 					cmd = FLASH_CMD_PROTECT_CLEAR;
> 
> 				/*
> 				 * cmd must come before FLASH_CMD_PROTECT + 20us
> 				 * Disable interrupts which might cause a timeout here.
> 				 */

Should the above comment not stay closer to the disable_interrupts () call ?

> 				flash_write_cmd (info, sector, 0,
> 						  FLASH_CMD_PROTECT);
> 				flash_write_cmd (info, sector, 0, cmd);
> 				/* re-enable interrupts if necessary */
> 				if (flag)
> 					enable_interrupts ();
> 			}
> 
> > > > +                       /*
> > > > +                        * see errata called
> > > > +                        * "Numonyx Axcell P33/P30 Specification
> > > > Update" :) +                        */
> > > > +                       flash_write_cmd (info, sector, 0,
> > > > FLASH_CMD_READ_ID); +                       if (!flash_isequal (info,
> > > > sector, FLASH_OFFSET_PROTECT, +                                       
> > > >    prot)) {
> > > > +                               int flag = disable_interrupts ();
> > > 
> > > Declare and assign "cmd" here please.
> > > 
> > > Please change and resubmit. Thanks.
> > 
> > If you still prefer it changed, speak :)
> 
> Yes, please let me know if this patch version also fixes the bug. If
> this is the case, I would prefer that you change it accordingly.

I surmise it does, but I currently do not have a board to test.  I'll
get new boards (with the same bug) soon, and I'll let you know then.

Best regards

Philippe

-- 
Philippe De Muyter  phdm at macqel dot be  Tel +32 27029044
Macq Electronique SA  rue de l'Aeronef 2  B-1140 Bruxelles  Fax +32 27029077