[U-Boot-Users] PATCH for drivers/cfi_flash.c

[Tolunay Orkun]

Andrew,

Thanks for bringing this up. Here is my opinion on the solutions you 
have proposed. Depending on the choice, we can provide a patch on top of 
my earlier patch to fix.

> Several of the timeout parameters in the CFI query structure return units of
> microseconds.  I use an AMD 29LV160 part.  In the CFI parser code these
> microsecond values are converted to milliseconds by integer division.  For
> the 29LV160 this leaves some of the timeout parameters in the info structure
> at zero.
> 
> Using these zero values, the code in flash_status_check() will sometimes read
> busy once, the timer count happens to roll over, get read, and a timeout is
> registered if tout is zero.
> 
> I can see a few ways to fix this:
> 
> 1) adjust the info values after the scaling to millisecond values

I think this would be an easy fix. We can round the values up one tick 
if there is a remainder during integer division.

Pro: Easy to do.
Con: flinfo would not report correct timing info.

One way to report actual values by flinfo would be to duplicate the 
timing values in info structure, one set representing display values and 
another set would be the one actually used in loops.

Still, since this calculation would be done once when driver is 
initialized it would be my first choice.

> 2) force a one tick delay no matter what in flash_status_check()

Similar to #1.

Pro: Easy to do. Info structure contains correct values
Con: Rounding up tout is done every time we are in flash_status_check.

This would be my second choice.

> 3) modify the code to report everything in microseconds and use udelay

Pro: Correct timeout.
Con: Requires more extensive re-work of the code.

Also using udelay would probably not work well here. The current design 
allows breaking out early (when flash is not busy) before timeout 
occurs. Changing get_timer() to report time in usec would probably 
trigger a lot more changes.

> 4) use set_timer() to make sure there is a full timer tick

I think #2 would be better way.

> 
> Any opinions about the best way to fix this?  Or is it fixed in someones patch
> already?
> 
> ===================================================================
> 
>>RCS file: /cvsroot/u-boot/u-boot/drivers/cfi_flash.c,v
>>retrieving revision 1.17
>>diff -p -u -r1.17 cfi_flash.c
>>--- drivers/cfi_flash.c 13 Apr 2005 10:02:47 -0000      1.17
>>+++ drivers/cfi_flash.c 6 May 2005 07:06:04 -0000
>>@@ -676,7 +676,7 @@ static int flash_status_check (flash_inf
>>        /* Wait for command completion */
>>        start = get_timer (0);
>>        while (flash_is_busy (info, sector)) {
>>-               if (get_timer (start) > info->erase_blk_tout * CFG_HZ) {
>>+               if (get_timer (start) > tout * CFG_HZ) {
>>                        printf ("Flash %s timeout at address %lx data %lx\n",
>>                                prompt, info->start[sector],
>>                                flash_read_long (info, sector, 0));
> 
> 
>