[U-Boot] [RFC] ARM timing code refactoring

[Wolfgang Denk <wd@denx.de>]

Dear Reinhard Meyer,

In message <4D3C77BC.50006@emk-elektronik.de> you wrote:
>
> >> u32 timeout = timeout_init(100); /* 100ms timeout */
> >>
> >> do {...} while (!timed_out(timeout));
> >
> > I dislike this approach. I immediately fear the same problem I just
> > saw (incorrectly) in Albert's proposal - timeout_init() seems to
> > store the timeouut information in some internal varoable, which is
> > then checked by timed_out() - this is bound to fail as soon as
> > somebody atttempts to nest timeouts.
> 
> Excuse me: the timeout info is stored in the user's variable "timeout",
> that should be quite obvious!

Is it? Why?  It could be as well that timeout_init(100) keeps some
internal information about an initialized timeout - the name suggests
it.

> Nested timeouts pose no problem as long as the user uses different (local)
> vars to store the return value of timeout_init().

Yes, but you need to see the implementation of timeout_init() and
timed_out() to make sure.

> > Your implementation may be different, but you can bet sooner or later
> > comes up with such a bugy implementation.
> 
> There is nothing buggy.

Not in your proposal - but the interface allows (or even invites) to
come up with incorrect implementations, and history has shown that we
don't catch all of these.  So let's chose an interface which has no
such problems.

> > PLease see my proposal: we do not needs several timer or timeout
> > related functions, all we need is a plain "get timer" function,
> > without any arguments.  And the resulting code makes it obvious to the
> > reader that such loops can be nested as you like.
> 
> If you demand that this get_timer returns in HZ units,

If we agree on get_timer(), then indeed the resolution shall ne
milliseconds, as this is how this function is defined.

On the other hand, milliseconds are too long for some types of
timeouts, so for short timeouts we would use get_ticks().

For reference, please see the PPC implementation (in
arch/powerpc/lib/ticks.S, arch/powerpc/lib/time.c,
arch/powerpc/lib/interrupts.c).

At the moment I would suggest to change the existing interface like
that:

* Drop the set_timer() function.

* Change get_timer() to take no argument, i. e.:

	unsigned long get_timer(void);

  get_timer() returns a monotonous upward counting time stamp with a
  resolution of milliseconds. After reaching ULONG_MAX the timer wraps
  around to 0.

  The get_timer() implementation may be interrupt based and is only
  available after relocation.

* Provide a fast, low-level, system dependent timer function

	unsigned long long get_ticks(void);

  get_ticks() returns a monotonous upward counting time stamp with a
  system-specific resolution. No assumptions should be made about the
  resolution. After reaching ULLONG_MAX the timer wraps around to 0.

  It is mandatory that get_ticks() is available before relocation.

* Provide a set of utility functions:

  ->	void wait_ticks(unsigned long ticks);

  Delay execution for "ticks" ticks.

  ->	unsigned long usec2ticks(unsigned long usec);

  Convert microseconds into ticks; intended for SHORT delays only
  (maximum depending on system clock, usually below 1 second).

  ->	void __udelay(unsigned long usec);

  Delay execution for "usec" microseconds; intended for SHORT delays
  only (maximum depending on system clock, usually below 1 second).
  If all architectures followed the above suggestion, we could move
  the PPC implementation to common code:

  	void __udelay(unsigned long usec) 
	{
		ulong ticks = usec2ticks(usec);
		wait_ticks(ticks); 
	}

  __udelay() can reliably be used before relocation.

  ->	void udelay(unsigned long usec)

  Similar to __udelay() with the additional functionality to trigger
  the watchdog timer for long delays.



> that will not be possible on most hardware without complicated code.
> We have discussed that long ago...

I am aware of this.

> Well, you could try to understand:
> tick=the "at hardware speed running" timer, if that's incrementing too fast for
> 32 bit "timeout" vars for reasonable timeouts (up to a minute?),

See above.  For short, high resolution timeouts you can use
get_ticks() and friends.  For long delays you can use get_timer().

Note that "reasonable timeouts (up to a minute?)" are only very
infrequently needed, and don't need the high resolution of
get_ticks(), so these would naturally be implemented on the base of
get_timer().


We have been using this implementation for more than a decade on
PowerPC.  The only thing you need is a monotonous upward counting
64 bit "time base" counter where you can read the system ticks from.

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
"Just think of a computer as hardware you can program."
- Nigel de la Tierre