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

[Wolfgang Denk]

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.


> 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...

>           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.

> 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.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Do not simplify the design of a program if a way can be found to make
it complex and wonderful.