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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
