[U-Boot] [PATCH v1 (WIP) 00/16] [Timer]API Rewrite

[J. William Campbell]

On 7/15/2011 12:17 AM, Wolfgang Denk wrote:
> Dear "J. William Campbell",
>
> In message<4E1F8127.8030008 at comcast.net>  you wrote:
>> I am pretty sure that is long enough for someone to notice. . I would be
>> interested in seeing an example of such code as you refer to. Could you
>> point me to one, because it seems to me that the only reason to code
>> such a delay is that for some reason the user didn't want to keep
>> looking at the termination condition so often. I think that that
> arch/powerpc/cpu/mpc512x/i2c.c:
>
>   80         while (timeout--&&  (status&  I2C_BB)) {
>   ...
>   88                 udelay (1000);
>   89                 status = mpc_reg_in (&regs->msr);
>   90         }
>
> 103         while (timeout--&&  !(*status&  I2C_IF)) {
> 104                 udelay (1000);
> 105                 *status = mpc_reg_in (&regs->msr);
> 106         }
> arch/powerpc/cpu/mpc512x/pci.c:
>
>   87         /* We need to wait at least a 1sec based on PCI specs */
>   88         for (i = 0; i<  1000; i++)
>   89                 udelay(1000);
>
> arch/powerpc/cpu/mpc5xx/spi.c:
>
> 258         for (i = 0; i<  1000; i++) {
> ...
> 265                 udelay(1000);
> 266         }
>
> 390         for (tm=0; tm<1000; ++tm) {
> ...
> 394                 udelay (1000);
> 395         }
>
> arch/powerpc/cpu/mpc5xxx/i2c.c:
>
> 102         while (timeout--&&  (status&  I2C_BB)) {
> ...
> 111                 udelay(15);
> 112                 status = mpc_reg_in(&regs->msr);
> 113         }
>
> 125         while (timeout--&&  !(*status&  I2C_IF)) {
> 126                 udelay(15);
> 127                 *status = mpc_reg_in(&regs->msr);
> 128         }
>
> And just that you don't think this is in a single CPU only:
>
> arch/powerpc/cpu/mpc8260/pci.c:
>
> 343                 for (i = 0; i<  1000; ++i)
> ...
> 345                         udelay (1000);
>
> Or in board code:
>
> board/altera/common/cfide.c:
>
>   29         /* wait 500 ms for power to stabilize */
>   30         for (i = 0; i<  500; i++)
>   31                 udelay(1000);
>
> board/amcc/bamboo/bamboo.c:
>
> 1019         for (i=0; i<500; i++)
> 1020                 udelay(1000);
>
> or common code:
>
> common/cmd_fdc.c:
>
> 213         while((read_fdc_reg(FDC_SRA)&0x80)==0) {
> 214                 timeout--;
> 215                 udelay(10);
> 216                 if(timeout==0) /* timeout occured */
> 217                         return FALSE;
> 218         }
>
> 228         while((read_fdc_reg(FDC_MSR)&0xC0)!=0xC0) {
> 229                 /* direction out and ready */
> 230                 udelay(10);
> 231                 timeout--;
> 232                 if(timeout==0) /* timeout occured */
> 233                         return -1;
> 234         }
> etc.etc.
>
> 375                 for(val=0;val<255;val++)
> 376                         udelay(500); /* wait some time to start motor */
>
> 418         for(i=0;i<100;i++)
> 419                 udelay(500); /* wait 500usec for fifo overrun */
>
> 600         for(i=0; i<255; i++) /* then we wait some time */
> 601                 udelay(500);
>
>
> common/usb.c:
>
>    93 inline void wait_ms(unsigned long ms)
>    94 {
>    95         while (ms-->  0)
>    96                 udelay(1000);
>    97 }
>
> etc. etc.  Note this last example which "spreads" the effect in a not
> so nice way.
>
> Note that there are even places where udelay() is part of the protocol
> timing implementation - like in the soft-I2C and soft-SPI drivers.
>
Hi All,
       Thanks for the pointers Wolfgang. From your examples, it seems we 
are talking exclusively about udelay. As I said in my previous post, on 
NIOS2, udelay needs to be mechanized on NIOS2 by a timed loop, not by 
using the same mechanism as the timer, as there is no way one can 
approximate microsecond resolution by the hardware timer. It is my 
opinion that there are many CPUs on which the udelay implementation must 
not be/should not be based on the same timer code as the get_time() 
operation, because in many cases the get_time() operation requires 
interrupts to be functional. As you have also pointed out, udelay needs 
to be available "early", before interrupts are available. Therefore, the 
udelay and get_timer must use the hardware  at least somewhat 
differently. The must also not interfere with each other, which on some 
existing boards they did.
       If the I2C protocol must be available before interrupts are 
available, then udelay must be used. In the above examples, there are 
some loops in i2c and spi that appear to be waiting a full second. I 
assume they are using udelay because the get_timer feature is not yet 
available to them. I also assume that the example in common/usb.c uses 
udelay to wait for millisecond sized values for the same reason, i.e. 
that it may run before get_time() is available. However, if you examine 
the timer code on of the existing CPUs, you will find that udelay is NOT 
available early ( before interrupts are enabled) on quite a few of them. 
Therefore, none of the above code would work on these CPUs at present 
either. These CPUs must not have I2C or spi busses on them that are 
accessed "early".
>> equivalent  operation can be produced by a pretty simple re-coding of
>> the loop. In any case, NIOS does not have the problem at present, so the
>> suggested new work-around would be no worse than the present situation.
> I think it is not correct to state that "NIOS does not have the
> problem at present" - it does, only this is not a blatant problem at
> the moment.  Which in turn might result from the fact that all
> presently supported NIOS boards has any support enabled for I2C or
> SPI or USB or MMC or watchdogs or ... you name it.
>
> The first user who adds any of the currently unused (and thus
> untested) standard features that work just fine on all other
> architectures might run into issue...
True, although I expect you will find the statement "on all the other 
architectures" to be false. Many other architectures, yes, all, no. 
These other architectures just don't have spi or I2C yet, or if they do, 
they don't use it "early".
>
>>            It is also true that the hardware timer cannot be used in a
>> reasonable version of udelay, as most of the desired delays may be very
>> short relative to the timebase. A calibrated timing loop would be the
>> best approach to the udelay problem.
> Just try using Soft-I2C on a NIOS board and you will immediately
> realize how crucial a working version of udelay() is.
Agreed. The definition of "working" would need to include that the 
routine is available "early" and that it is accurate to within say +/-5% 
(other numbers welcome) of the specified delay, unless the delay is 
"small" relative to the CPU speed (not an issue on most CPUs, but is on 
some). On these existing NIOS2 processors, that will require a udelay 
based on a timing loop, as there is no microsecond level timer 
available. The timing loop can be calibrated using the 10 ms timer, so 
it can be accurate enough. It may need re-calibration if icache is 
turned on, but this is also doable.
>
>> In general, that is true. There may be a few cases where a delay of less
>> than the resolution is essential to make something work. There are
> These are not just "a few cases".  There are tons of it, all over the
> place - from protocol implementations like soft-I2C or soft-I2C to
> timing requirements for RAM initializations etc. etc.  And there are
> enough places that mandate that the timing is at least in the
> requested oder, and not off by factors of tens or hundrets or worse.
>
>> probably lots of other cases where we can easily remove the restriction
>> on the resolution. We cannot fix the first kind, no matter what we do,
>> to work on a lower resolution timer. The second kind we can and probably
>> should fix, because they are coded in an overly-restrictive manner. In
>> any case, we don't absolutely have to fix them until somebody decides to
>> use the code on a CPU with a low resolution timer. Practically speaking,
>> the suggested solution will therefore work on all extant cases.
> It will work by chance, at best.  See above - any attempt to enable
> one of the currently not yet used standard features in U-Boot may
> break your system.  I don't consider this a sound base - to me it
> feels like building on qicksand.
In a perfect world, I would agree with you. Unfortunately (or maybe 
inevitably), large software projects are iterative. There are quite a 
few u-boot versions in the current code that do not meet all the 
requirements that exits for a fully compliant udelay. There were also 
quite a few board ports whose timers did not meet the implicit 
requirements to be fully "compliant". Full compliance is required for 
"plug and play" with u-boot code that has been written assuming this 
full compliance. Quite a few people have just made a port that works for 
them without worrying about this compliance. One could argue that 
compliance was not well enough documented, or that non-compliant boards 
should not have been accepted. All perhaps true (or not, because u-boot 
was still feeling its way forward), but we are where we are.
We can't retro-actively change the NIOS2 boards that are out there 
already. However, we can require that if any of the boards are to be 
upgraded to include I2C or SPI devices, they must be supplied with a 
compliant udelay, either by a timed loop or increasing the hardware 
timer resolution. The get_timer design is sound for all existing designs 
if the resolution compensation is added. It preserves all working boards 
as working . If you modify the configuration of existing boards that 
currently are non-compliant in some way,  they may quit working. That is 
true now, and it will be true if the change to include the resolution is 
made. If udelay doesn't have compliant resolution and accuracy, newly 
enabled code may fail. If you aren't using such code, you may not be 
motivated to fix the problem. It still should be fixed, but often will 
not be until it breaks something. So I think we are building on solid, 
backwards compatible ground. Yes, non-compliant boards need to be 
"fixed", but we need a way to move forward with minimum pain to the 
current users.

Best Regards,
Bill Campbell
>
>
> Best regards,
>
> Wolfgang Denk
>