[U-Boot] [RFC] Review of U-Boot timer API

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

Hi All,

I've just had a good look through the timer API code with the view of
rationalising it and fixing up some of the inconsistencies. What I found
was a little scarier than I expected! Anyway, here is a write-up of what I
found - Please comment

Regards,

Graeme

--------

At the lowest level, the U-Boot timer API consists of a unsigned 32-bit
free running timestamp which increments every millisecond (wraps around
every 4294967 seconds, or about every 49.7 days). The U-Boot timer API
allows:
 - Time deltas to be measured between arbitrary code execution points
	ulong start_time;
	ulong elapsed_time;

	start_time = get_timer(0);
	...
	elapsed_time = get_timer(start_time);

 - Repetition of code for a specified duration
	ulong start_time;

	start_time = get_timer(0);

	while (get_timer(start_time) < REPEAT_TIME) {
		...
	}

 - Device timeout detection
	ulong start_time;

	send_command_to_device();
	start = get_timer (0);
	while (device_is_busy()) {
		if (get_timer(start) > TIMEOUT)
			return ERR_TIMOUT;
		udelay(1);
	}
	return ERR_OK;

The U-Boot timer API is not a 'callback' API and cannot 'trigger' a
function call after a pre-determined time.

NOTE: http://lists.denx.de/pipermail/u-boot/2010-June/073024.html appears
to imply the following implementation of get_timer() is wrong:

	ulong get_timer(ulong base)
	{
		return get_timer_masked() - base;
	}

U-Boot Timer API Details
========================
There are currently three functions in the U-Boot timer API:
	ulong get_timer(ulong start_time)
	void set_timer(ulong preset)
	void reset_timer(void)

get_timer() returns the number of milliseconds since 'start_time'. If
'start_time' is zero, therefore, it returns the current value of the
free running counter which can be used as a reference for subsequent
timing measurements.

set_timer() sets the free running counter to the value of 'preset'
reset_timer() sets the free running counter to the value of zero[1]. In
theory, the following examples are all identical

	----------------------------------------------------
	ulong start_time;
	ulong elapsed_time;

	start_time = get_timer(0);
	...
	elapsed_time = get_timer(start_time);
	----------------------------------------------------
	ulong elapsed_time;

	reset_timer();
	...
	elapsed_time = get_timer(0);
	----------------------------------------------------
	ulong elapsed_time;

	set_timer(0);
	...
	elapsed_time = get_timer(0);
	----------------------------------------------------

[1] arch/arm/cpu/arm926ejs/at91/ and arch/arm/cpu/arm926ejs/davinci/ are
exceptions, they set the free running counter to get_ticks() instead

Architecture Specific Peculiarities
===================================
ARM
 - Generally define get_timer_masked() and reset_timer_masked()
 - [get,reset]_timer_masked() are exposed outside arch\arm which is a bad
   idea as no other arches define these functions - build breakages are
   possible although the external files are most likely ARM specific (for
   now!)
 - Most CPUs define their own versions of the API get/set functions which
   are wrappers to the _masked equivalents. These all tend to be the same.
   The API functions could be moved into arch/arm/lib and made weak for
   the rare occasions where they need to be different
 - Implementations generally look sane[2] except for the following:
   - arm_intcm - No timer code (unused CPU arch?)
   - arm1136/mx35 - set_timer() is a NOP
   - arm926ejs/at91 - reset_timer() sets counter to get_ticks()
                      no implelemtation of set_timer()
   - arm926ejs/davinci - reset_timer() sets counter to get_ticks()
                         no implelemtation of set_timer()
   - arm926ejs/mb86r0x - No implementation of set_timer()
   - arm926ejs/nomadik - No implementation of set_timer()
   - arm946es - No timer code (unused CPU arch?)
   - ixp - No implementation of set_timer()
         - If CONFIG_TIMER_IRQ is defined, timer is incremented by an
           interrupt routine - reset_timer() writes directly to the
           counter without interrupts disables - Could cause corruption
           if reset_timer() is not atomic
         - If CONFIG_TIMER_IRQ is not defined, reset_timer() appears to
           not be implemented
   - pxa - set_timer() is a NOP
   - sa1100 - get_timer() does not subtract the argument

nios, powerpc, sparc, x86
 - All look sane[2]

avr32
 - Not obvious, but looks sane[2]

blackfin
 - Does not implement set_timer()
 - Provides a 64-bit get_ticks() which simply returns 32-bit get_timer(0)
 - reset_timer() calls timer_init()
 - Looks sane[2]

m68k
 - Looks sane[2] if CONFIG_MCFTMR et al are defined. If CONFIG_MCFPIT is
   defined instead, reset_timer() is unimplemented and build breakage will
   result if cfi driver is included in the build - No configurations use
   this define, so that code is dead anyway

microblaze
 - Only sane[2] if CONFIG_SYS_INTC_0 and CONFIG_SYS_TIMER_0 are both defined.
   Doing so enables a timer interrupt which increments the internal
   counter. Otherwise, it is incremented when get_timer() is called which
   will lead to horrible (i.e. none at all) accuracy

mips
 - Not obvious, but looks sane[2]

sh
 - Generally looks sane[2]
 - Writes 0xffff to the CMCOR_0 control register when resetting to zero,
   but writes the actual 'set' value when not zero
 - Uses a 32-bit microsecond based timebase:
	static unsigned long get_usec (void)
	{
	...
	}

	get_timer(ulong base)
	{
		return (get_usec() / 1000) - base;
	}

 - Timer will wrap after ~4295 seconds (71 minutes)

[2] Sane means something close to:
	void reset_timer (void)
	{
		timestamp = 0;
	}

	ulong get_timer(ulong base)
	{
		return timestamp - base;
	}

	void set_timer(ulong t)
	{
		timestamp = t;
	}

Rationalising the API
=====================
Realistically, the value of the free running timer at the start of a timing
operation is irrelevant (even if the counter wraps during the timed period).
Moreover, using reset_timer() and set_timer() makes nested usage of the
timer API impossible. So in theory, the entire API could be reduced to
simply get_timer()

0. Fix arch/arm/cpu/sa1100/timer.c:get_timer()
----------------------------------------------
This appears to be the only get_timer() which does not subtract the
argument from timestamp

1. Remove set_timer()
---------------------
regex "[^e]set_timer\s*\([^)]*\);" reveals 14 call sites for set_timer()
and all except arch/sh/lib/time_sh2:[timer_init(),reset_timer()] are
set_timer(0). The code in arch/sh is trivial to resolve in order to
remove set_timer()

2. Remove reset_timer()
-----------------------------------------------
regex "[\t ]*reset_timer\s*\([^)]*\);" reveals 22 callers across the
following groups:
 - timer_init() - Make reset_timer() static or fold into timer_init()
 - board_init_r() - Unnecessary
 - arch/m68k/lib/time.c:wait_ticks() - convert[3]
 - Board specific flash drivers - convert[3]
 - drivers/block/mg_disk.c:mg_wait() - Unnecessary
 - drivers/mtd/cfi_flash.c:flash_status_check() - Unnecessary
 - drivers/mtd/cfi_flash.c:flash_status_poll() - Unnecessary

[3] These instance can be trivially converted to use one of the three
examples at the beginning of this document

3. Remove reset_timer_masked()
------------------------------
This is only implemented in arm but has propagated outside arch/arm and
into board/ and drivers/ (bad!)

regex "[\t ]*reset_timer_masked\s*\([^)]*\);" reveals 135 callers!

A lot are in timer_init() and reset_timer(), but the list includes:
 - arch/arm/cpu/arm920t/at91rm9200/spi.c:AT91F_SpiWrite()
 - arch/arm/cpu/arm926ejs/omap/timer.c:__udelay()
 - arch/arm/cpu/arm926ejs/versatile/timer.c:__udelay()
 - arch/arm/armv7/s5p-common/timer.c:__udelay()
 - arch/arm/lh7a40x/timer.c:__udelay()
 - A whole bunch of board specific flash drivers
 - board/mx1ads/syncflash.c:flash_erase()
 - board/trab/cmd_trab.c:do_burn_in()
 - board/trab/cmd_trab.c:led_blink()
 - board/trab/cmd_trab.c:do_temp_log()
 - drivers/mtd/spi/eeprom_m95xxx.c:spi_write()
 - drivers/net/netarm_eth.c:na_mii_poll_busy()
 - drivers/net/netarm_eth.c:reset_eth()
 - drivers/spi/atmel_dataflash_spi.c/AT91F_SpiWrite()

Most of these instance can be converted to use one of the three examples
at the beginning of this document, but __udelay() will need closer
examination

This is preventing nesting of timed code - Any code which uses udelay()
has the potential to foul up outer-loop timers. The problem is somewhat
unique to ARM though

4. Implement microsecond API - get_usec_timer()
-----------------------------------------------
 - Useful for profiling
 - A 32-bit microsecond counter wraps in 71 minutes - Probably OK for most
   U-Boot usage scenarios
 - By default could return get_timer() * 1000 if hardware does not support
   microsecond accuracy - Beware of results > 32 bits!
 - If hardware supports microsecond resolution counters, get_timer() could
   simply use get_usec_timer() / 1000
 - get_usec_timer_64() could offer a longer period (584942 years!)

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

On 22/05/11 01:33, J. William Campbell wrote:
> On 5/21/2011 5:38 AM, Graeme Russ wrote:
>> Hi All,

>> 4. Implement microsecond API - get_usec_timer()
>> -----------------------------------------------
>>   - Useful for profiling
>>   - A 32-bit microsecond counter wraps in 71 minutes - Probably OK for most
>>     U-Boot usage scenarios
>>   - By default could return get_timer() * 1000 if hardware does not support
>>     microsecond accuracy - Beware of results>  32 bits!
> Hi All,
>       I think the multiply overflowing an unsigned long is ok here as long
> as the timeout value you desire is less than 71 seconds. This assumes that
> the CPU returns the correct lower 32 bits when overflow occurs, but I think
> this is the "normal" behavior(?)

I think you mean 71 minutes - 2^32 = 4294967296 (usec) = 4294967.296 (msec)
= 4294.967296 s = 71.582788267 minutes

So provided any millisecond timer using a 32-bit microsecond timer as a
time base is called every 71 minutes, a 32-bit microsecond  timer should
suffice to keep the msb's accurately maintained by software

>>   - If hardware supports microsecond resolution counters, get_timer() could
>>     simply use get_usec_timer() / 1000
>        I think this actually is NOT equivalent to the "current" API  in
> that the full 32 bits of the timer is not available and as a result the
> "wrapping" properties of a 32 bit subtract for delta times will not work
> properly. If a "larger"  counter is available in hardware, then it is
> certainly possible to do a 64 by 32 bit divide in get_timer, but probably
> you don't want to do that  either. As previously discussed, it is possible
> to extract a 32 bit monotonic counter of given resolution (microsecond or
> millisecond resolution) from a higher resolution counter using a shift to
> "approximately" the desired resolution followed by a couple of multiply/add
> functions of 32 bit resolution.  To do this with a microsecond resolution,
> a 42 bit or larger timer is required. The "extra" bits can be provided in

Of course, how silly of me - To downscale the microsecond timer down to
milliseconds you need to have at least 1000 times more resolution
(9.965784285 bits) - It was late ;)

> software as long as the get_timer/get_usec_timer routines are called more
> often than every 71/2 sec, so that a correct delta in microseconds can be
> obtained. Note that when the timer is not actively in use (not  called
> often enough), the millisecond timer msb would stop updating, but that
> wouldn't matter.

Minutes - see above

>        If the hardware supports sub-microsecond accuracy in a "longer"
> register, say 64 bits, you can just convert the 64 bit hardware timer to 32
> bit microseconds or milliseconds by a shift  and 32 bit multiplies

Yes

> 
>       Good luck with this effort. I think getting the timer API and also
> the method of implementation of the interface to the hardware to be the
> same across all u-boot architectures is a very good idea, and it is
> possible. However, it is a quite a bit of work and I am glad you are brave
> enough to try!

It's all there already - it just needs a little bit of housekeeping :)

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[J. William Campbell]

On 5/21/2011 5:06 PM, Graeme Russ wrote:
> On 22/05/11 01:33, J. William Campbell wrote:
>> On 5/21/2011 5:38 AM, Graeme Russ wrote:
>>> Hi All,
>>> 4. Implement microsecond API - get_usec_timer()
>>> -----------------------------------------------
>>>    - Useful for profiling
>>>    - A 32-bit microsecond counter wraps in 71 minutes - Probably OK for most
>>>      U-Boot usage scenarios
>>>    - By default could return get_timer() * 1000 if hardware does not support
>>>      microsecond accuracy - Beware of results>   32 bits!
>> Hi All,
>>        I think the multiply overflowing an unsigned long is ok here as long
>> as the timeout value you desire is less than 71 seconds. This assumes that
>> the CPU returns the correct lower 32 bits when overflow occurs, but I think
>> this is the "normal" behavior(?)
> I think you mean 71 minutes - 2^32 = 4294967296 (usec) = 4294967.296 (msec)
> = 4294.967296 s = 71.582788267 minutes
>
> So provided any millisecond timer using a 32-bit microsecond timer as a
> time base is called every 71 minutes, a 32-bit microsecond  timer should
> suffice to keep the msb's accurately maintained by software
>
Hi Graeme,
         Yes, you are certainly correct, it is minutes, not seconds.
>>>    - If hardware supports microsecond resolution counters, get_timer() could
>>>      simply use get_usec_timer() / 1000
>>         I think this actually is NOT equivalent to the "current" API  in
>> that the full 32 bits of the timer is not available and as a result the
>> "wrapping" properties of a 32 bit subtract for delta times will not work
>> properly. If a "larger"  counter is available in hardware, then it is
>> certainly possible to do a 64 by 32 bit divide in get_timer, but probably
>> you don't want to do that  either. As previously discussed, it is possible
>> to extract a 32 bit monotonic counter of given resolution (microsecond or
>> millisecond resolution) from a higher resolution counter using a shift to
>> "approximately" the desired resolution followed by a couple of multiply/add
>> functions of 32 bit resolution.  To do this with a microsecond resolution,
>> a 42 bit or larger timer is required. The "extra" bits can be provided in
> Of course, how silly of me - To downscale the microsecond timer down to
> milliseconds you need to have at least 1000 times more resolution
> (9.965784285 bits) - It was late ;)
>
>> software as long as the get_timer/get_usec_timer routines are called more
>> often than every 71/2 sec, so that a correct delta in microseconds can be
>> obtained. Note that when the timer is not actively in use (not  called
>> often enough), the millisecond timer msb would stop updating, but that
>> wouldn't matter.
> Minutes - see above
Correct.
>>         If the hardware supports sub-microsecond accuracy in a "longer"
>> register, say 64 bits, you can just convert the 64 bit hardware timer to 32
>> bit microseconds or milliseconds by a shift  and 32 bit multiplies
> Yes
>
>>        Good luck with this effort. I think getting the timer API and also
>> the method of implementation of the interface to the hardware to be the
>> same across all u-boot architectures is a very good idea, and it is
>> possible. However, it is a quite a bit of work and I am glad you are brave
>> enough to try!
> It's all there already - it just needs a little bit of housekeeping :)
Correct, if we do not worry too much about the "low level" details of 
get_timer. It looks to me like there is a lot of cruft there, depending 
on which architecture one looks at. Many implementations create a 
parallel universe of get_ticks or some similar timing system that is 
then used to support get_timer. However, the get_ticks routines are also 
used in timeouts elsewhere in the code. Searching on "get_timer" doesn't 
find these non-standard usages. It would be nice to fix these also, but 
every bit does help!

Best Regards,
Bill Campbell
> Regards,
>
> Graeme
>
>

[U-Boot] [RFC] Review of U-Boot timer API

[Reinhard Meyer]

Dear Graeme Russ,
> Hi All,
>
> I've just had a good look through the timer API code with the view of
> rationalising it and fixing up some of the inconsistencies. What I found
> was a little scarier than I expected! Anyway, here is a write-up of what I
> found - Please comment

We have been at this discussion a multiple of times :) but never reached a consent.

However, at current master, I have reduced at91 architecture to only use
get_timer(base), set_timer() never existed and reset_timer() has been removed.

As it showed up recently, common cfi code still calls reset_timer() - which certainly
can be fixed with little effort...

> At the lowest level, the U-Boot timer API consists of a unsigned 32-bit
> free running timestamp which increments every millisecond (wraps around
> every 4294967 seconds, or about every 49.7 days). The U-Boot timer API
> allows:
>   - Time deltas to be measured between arbitrary code execution points
> 	ulong start_time;
> 	ulong elapsed_time;
>
> 	start_time = get_timer(0);
> 	...
> 	elapsed_time = get_timer(start_time);
>
>   - Repetition of code for a specified duration
> 	ulong start_time;
>
> 	start_time = get_timer(0);
>
> 	while (get_timer(start_time)<  REPEAT_TIME) {
> 		...
> 	}
>
>   - Device timeout detection
> 	ulong start_time;
>
> 	send_command_to_device();
> 	start = get_timer (0);
> 	while (device_is_busy()) {
> 		if (get_timer(start)>  TIMEOUT)
> 			return ERR_TIMOUT;
> 		udelay(1);
> 	}
> 	return ERR_OK;
correct.

>
> The U-Boot timer API is not a 'callback' API and cannot 'trigger' a
> function call after a pre-determined time.
that would be too complex to implement and of little use in a single task
system. u-boot can do fine with polling.

>
> NOTE: http://lists.denx.de/pipermail/u-boot/2010-June/073024.html appears
> to imply the following implementation of get_timer() is wrong:
>
> 	ulong get_timer(ulong base)
> 	{
> 		return get_timer_masked() - base;
> 	}
Is is not wrong as long as get_timer_masked() returns the full 32 bit space
of numbers and 0xffffffff is followed by 0x00000000. Most implementations
probably do NOT have this property.

>
> U-Boot Timer API Details
> ========================
> There are currently three functions in the U-Boot timer API:
> 	ulong get_timer(ulong start_time)
As you point out in the following, this is the only function required.
However it REQUIRES that the internal timer value must exploit the full
32 bit range of 0x00000000 to 0xffffffff before it wraps back to 0x00000000.

> 	void set_timer(ulong preset)
> 	void reset_timer(void)
>
> get_timer() returns the number of milliseconds since 'start_time'. If
> 'start_time' is zero, therefore, it returns the current value of the
> free running counter which can be used as a reference for subsequent
> timing measurements.
>
> set_timer() sets the free running counter to the value of 'preset'
> reset_timer() sets the free running counter to the value of zero[1]. In
> theory, the following examples are all identical
>
> 	----------------------------------------------------
> 	ulong start_time;
> 	ulong elapsed_time;
>
> 	start_time = get_timer(0);
> 	...
> 	elapsed_time = get_timer(start_time);
> 	----------------------------------------------------
> 	ulong elapsed_time;
>
> 	reset_timer();
> 	...
> 	elapsed_time = get_timer(0);
> 	----------------------------------------------------
> 	ulong elapsed_time;
>
> 	set_timer(0);
> 	...
> 	elapsed_time = get_timer(0);
> 	----------------------------------------------------
>
> [1] arch/arm/cpu/arm926ejs/at91/ and arch/arm/cpu/arm926ejs/davinci/ are
> exceptions, they set the free running counter to get_ticks() instead
Not anymore on at91.
>
> Architecture Specific Peculiarities
> ===================================
> ARM
>   - Generally define get_timer_masked() and reset_timer_masked()
>   - [get,reset]_timer_masked() are exposed outside arch\arm which is a bad
>     idea as no other arches define these functions - build breakages are
>     possible although the external files are most likely ARM specific (for
>     now!)
>   - Most CPUs define their own versions of the API get/set functions which
>     are wrappers to the _masked equivalents. These all tend to be the same.
>     The API functions could be moved into arch/arm/lib and made weak for
>     the rare occasions where they need to be different
>   - Implementations generally look sane[2] except for the following:
>     - arm_intcm - No timer code (unused CPU arch?)
>     - arm1136/mx35 - set_timer() is a NOP
>     - arm926ejs/at91 - reset_timer() sets counter to get_ticks()
>                        no implelemtation of set_timer()
See current master for actual implementation!
>     - arm926ejs/davinci - reset_timer() sets counter to get_ticks()
>                           no implelemtation of set_timer()
>     - arm926ejs/mb86r0x - No implementation of set_timer()
>     - arm926ejs/nomadik - No implementation of set_timer()
>     - arm946es - No timer code (unused CPU arch?)
>     - ixp - No implementation of set_timer()
>           - If CONFIG_TIMER_IRQ is defined, timer is incremented by an
>             interrupt routine - reset_timer() writes directly to the
>             counter without interrupts disables - Could cause corruption
>             if reset_timer() is not atomic
>           - If CONFIG_TIMER_IRQ is not defined, reset_timer() appears to
>             not be implemented
>     - pxa - set_timer() is a NOP
>     - sa1100 - get_timer() does not subtract the argument
>
> nios, powerpc, sparc, x86
>   - All look sane[2]
>
> avr32
>   - Not obvious, but looks sane[2]
>
> blackfin
>   - Does not implement set_timer()
>   - Provides a 64-bit get_ticks() which simply returns 32-bit get_timer(0)
>   - reset_timer() calls timer_init()
>   - Looks sane[2]
>
> m68k
>   - Looks sane[2] if CONFIG_MCFTMR et al are defined. If CONFIG_MCFPIT is
>     defined instead, reset_timer() is unimplemented and build breakage will
>     result if cfi driver is included in the build - No configurations use
>     this define, so that code is dead anyway
>
> microblaze
>   - Only sane[2] if CONFIG_SYS_INTC_0 and CONFIG_SYS_TIMER_0 are both defined.
>     Doing so enables a timer interrupt which increments the internal
>     counter. Otherwise, it is incremented when get_timer() is called which
>     will lead to horrible (i.e. none at all) accuracy
>
> mips
>   - Not obvious, but looks sane[2]
>
> sh
>   - Generally looks sane[2]
>   - Writes 0xffff to the CMCOR_0 control register when resetting to zero,
>     but writes the actual 'set' value when not zero
>   - Uses a 32-bit microsecond based timebase:
> 	static unsigned long get_usec (void)
> 	{
> 	...
> 	}
>
> 	get_timer(ulong base)
> 	{
> 		return (get_usec() / 1000) - base;
> 	}
>
>   - Timer will wrap after ~4295 seconds (71 minutes)
>
> [2] Sane means something close to:
> 	void reset_timer (void)
> 	{
> 		timestamp = 0;
> 	}
>
> 	ulong get_timer(ulong base)
> 	{
> 		return timestamp - base;
> 	}
>
> 	void set_timer(ulong t)
> 	{
> 		timestamp = t;
> 	}
>
> Rationalising the API
> =====================
> Realistically, the value of the free running timer at the start of a timing
> operation is irrelevant (even if the counter wraps during the timed period).
> Moreover, using reset_timer() and set_timer() makes nested usage of the
> timer API impossible. So in theory, the entire API could be reduced to
> simply get_timer()
Full ACK here !!!
>
> 0. Fix arch/arm/cpu/sa1100/timer.c:get_timer()
> ----------------------------------------------
> This appears to be the only get_timer() which does not subtract the
> argument from timestamp
>
> 1. Remove set_timer()
> ---------------------
> regex "[^e]set_timer\s*\([^)]*\);" reveals 14 call sites for set_timer()
> and all except arch/sh/lib/time_sh2:[timer_init(),reset_timer()] are
> set_timer(0). The code in arch/sh is trivial to resolve in order to
> remove set_timer()
>
> 2. Remove reset_timer()
> -----------------------------------------------
> regex "[\t ]*reset_timer\s*\([^)]*\);" reveals 22 callers across the
> following groups:
>   - timer_init() - Make reset_timer() static or fold into timer_init()
>   - board_init_r() - Unnecessary
>   - arch/m68k/lib/time.c:wait_ticks() - convert[3]
>   - Board specific flash drivers - convert[3]
>   - drivers/block/mg_disk.c:mg_wait() - Unnecessary
>   - drivers/mtd/cfi_flash.c:flash_status_check() - Unnecessary
>   - drivers/mtd/cfi_flash.c:flash_status_poll() - Unnecessary
>
> [3] These instance can be trivially converted to use one of the three
> examples at the beginning of this document
>
> 3. Remove reset_timer_masked()
> ------------------------------
> This is only implemented in arm but has propagated outside arch/arm and
> into board/ and drivers/ (bad!)
>
> regex "[\t ]*reset_timer_masked\s*\([^)]*\);" reveals 135 callers!
>
> A lot are in timer_init() and reset_timer(), but the list includes:
>   - arch/arm/cpu/arm920t/at91rm9200/spi.c:AT91F_SpiWrite()
>   - arch/arm/cpu/arm926ejs/omap/timer.c:__udelay()
>   - arch/arm/cpu/arm926ejs/versatile/timer.c:__udelay()
>   - arch/arm/armv7/s5p-common/timer.c:__udelay()
>   - arch/arm/lh7a40x/timer.c:__udelay()
>   - A whole bunch of board specific flash drivers
>   - board/mx1ads/syncflash.c:flash_erase()
>   - board/trab/cmd_trab.c:do_burn_in()
>   - board/trab/cmd_trab.c:led_blink()
>   - board/trab/cmd_trab.c:do_temp_log()
>   - drivers/mtd/spi/eeprom_m95xxx.c:spi_write()
>   - drivers/net/netarm_eth.c:na_mii_poll_busy()
>   - drivers/net/netarm_eth.c:reset_eth()
>   - drivers/spi/atmel_dataflash_spi.c/AT91F_SpiWrite()
Fixed in current master.
>
> Most of these instance can be converted to use one of the three examples
> at the beginning of this document, but __udelay() will need closer
> examination
>
> This is preventing nesting of timed code - Any code which uses udelay()
> has the potential to foul up outer-loop timers. The problem is somewhat
> unique to ARM though
>
> 4. Implement microsecond API - get_usec_timer()
> -----------------------------------------------
>   - Useful for profiling
>   - A 32-bit microsecond counter wraps in 71 minutes - Probably OK for most
>     U-Boot usage scenarios
>   - By default could return get_timer() * 1000 if hardware does not support
>     microsecond accuracy - Beware of results>  32 bits!
>   - If hardware supports microsecond resolution counters, get_timer() could
>     simply use get_usec_timer() / 1000

That is wrong. Dividing 32 bits by any number will result in a result that does not
exploit the full 32 bit range, i.e. wrap from (0xffffffff/1000) to 0x00000000,
which makes time differences go wrong when they span across such a wrap!

>   - get_usec_timer_64() could offer a longer period (584942 years!)
Correct. And a "must be" when one uses such division.

But you have to realize that most hardware does not provide a simple means to
implement a timer that runs in either exact microseconds or exact milliseconds.

Powerpc for example has a 64 bit free running hardware counter at CPU clock,
which can be in the GHz range, making the lower 32 bits overflow within seconds,
so the full 64 bits MUST BE used to obtain a millisecond timer by division.

arm/at91 has a timer that can be made to appear like a 32 bit free running counter
at some fraction of cpu clock (can be brought into a few MHz value by a prescaler)
and the current implementation extends this to 64 bits by software, so it is
similar to powerpc.

A get timer() simply uses this 64 bit value by dividing it by (tickrate/1000).

Of course this results in a wrong wrap "gigaseconds" after the timer has been started,
but certainly this can be ignored...

On any account, I see only the following two functions to be implemented for use by
other u-boot code parts:

1. void udelay(u32 microseconds) with the following properties:
- must not affect get_timer() results
- must not delay less than the given time, may delay longer
(this might be true especially for very small delay values)
- shall not be used for delays in the seconds range and longer
(or any other limit we see practical)
Note: a loop doing "n" udelays of "m" microseconds might take _much_ longer than
"n * m" microseconds and therefore is the wrong approach to implement a timeout.
get_timer() must be used for any such timeouts instead!

2. u32 get_timer(u32 base) with the following properties:
- must return the elapsed milliseconds since base
- must work without wrapping problems for at least several hours

Best Regards,
Reinhard

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

On 22/05/11 14:26, Reinhard Meyer wrote:
> Dear Graeme Russ,
>> Hi All,
>>
>> I've just had a good look through the timer API code with the view of
>> rationalising it and fixing up some of the inconsistencies. What I found
>> was a little scarier than I expected! Anyway, here is a write-up of what I
>> found - Please comment
> 
> We have been at this discussion a multiple of times :) but never reached a
> consent.
> 
> However, at current master, I have reduced at91 architecture to only use
> get_timer(base), set_timer() never existed and reset_timer() has been removed.

Excellent

> As it showed up recently, common cfi code still calls reset_timer() - which
> certainly
> can be fixed with little effort...

Yes, this is one of the easy fixes as all call sites already use the start
= get_timer(0), elapsed = get_timer(start) convention anyway - The
reset_timer() calls are 100% redundant (provided get_timer() behaves
correctly at the 32-bit rollover for all arches)

>> The U-Boot timer API is not a 'callback' API and cannot 'trigger' a
>> function call after a pre-determined time.
> that would be too complex to implement and of little use in a single task
> system. u-boot can do fine with polling.

I am in no way suggesting this - I just want to clarify the API for anyone
who needs to use it

>> NOTE: http://lists.denx.de/pipermail/u-boot/2010-June/073024.html appears
>> to imply the following implementation of get_timer() is wrong:
>>
>>     ulong get_timer(ulong base)
>>     {
>>         return get_timer_masked() - base;
>>     }
> Is is not wrong as long as get_timer_masked() returns the full 32 bit space
> of numbers and 0xffffffff is followed by 0x00000000. Most implementations
> probably do NOT have this property.
>>
>> U-Boot Timer API Details
>> ========================
>> There are currently three functions in the U-Boot timer API:
>>     ulong get_timer(ulong start_time)
> As you point out in the following, this is the only function required.
> However it REQUIRES that the internal timer value must exploit the full
> 32 bit range of 0x00000000 to 0xffffffff before it wraps back to 0x00000000.

So this needs to be clearly spelt out in formal documentation

>> Rationalising the API
>> =====================
>> Realistically, the value of the free running timer at the start of a timing
>> operation is irrelevant (even if the counter wraps during the timed period).
>> Moreover, using reset_timer() and set_timer() makes nested usage of the
>> timer API impossible. So in theory, the entire API could be reduced to
>> simply get_timer()
> Full ACK here !!!

I don't think there will be much resistance to this

>> 3. Remove reset_timer_masked()
>> ------------------------------
>> This is only implemented in arm but has propagated outside arch/arm and
>> into board/ and drivers/ (bad!)
>>
>> regex "[\t ]*reset_timer_masked\s*\([^)]*\);" reveals 135 callers!
>>
>> A lot are in timer_init() and reset_timer(), but the list includes:
>>   - arch/arm/cpu/arm920t/at91rm9200/spi.c:AT91F_SpiWrite()
>>   - arch/arm/cpu/arm926ejs/omap/timer.c:__udelay()
>>   - arch/arm/cpu/arm926ejs/versatile/timer.c:__udelay()
>>   - arch/arm/armv7/s5p-common/timer.c:__udelay()
>>   - arch/arm/lh7a40x/timer.c:__udelay()
>>   - A whole bunch of board specific flash drivers
>>   - board/mx1ads/syncflash.c:flash_erase()
>>   - board/trab/cmd_trab.c:do_burn_in()
>>   - board/trab/cmd_trab.c:led_blink()
>>   - board/trab/cmd_trab.c:do_temp_log()
>>   - drivers/mtd/spi/eeprom_m95xxx.c:spi_write()
>>   - drivers/net/netarm_eth.c:na_mii_poll_busy()
>>   - drivers/net/netarm_eth.c:reset_eth()
>>   - drivers/spi/atmel_dataflash_spi.c/AT91F_SpiWrite()
> Fixed in current master.

Excellent. I have not pulled master for a little while, guess I should

>>   - If hardware supports microsecond resolution counters, get_timer() could
>>     simply use get_usec_timer() / 1000
> 
> That is wrong. Dividing 32 bits by any number will result in a result that
> does not
> exploit the full 32 bit range, i.e. wrap from (0xffffffff/1000) to 0x00000000,
> which makes time differences go wrong when they span across such a wrap!
> 

Yes, this has already been pointer out - 42 bits are needed as a bare
minimum. However, we can get away with 32-bits provided get_timer() is
called at least every 71 minutes

P.S. Can we use the main loop to kick the timer?

>>   - get_usec_timer_64() could offer a longer period (584942 years!)
> Correct. And a "must be" when one uses such division.

Unless we can rely on get_timer() to be called at least every 71 minutes in
which case we can handle the msb's without error in software

> But you have to realize that most hardware does not provide a simple means to
> implement a timer that runs in either exact microseconds or exact
> milliseconds.

This is where things get interesting and we need to start pushing a
mandated low-level HAL. For example, I believe get_timer() should be
implemented in /lib as:

	ulong get_timer(ulong base)
	{
		return get_raw_msec() - base;
	}

get_raw_ms() MUST:
 - Return an unsigned 32-but value which increments every 1ms
 - Wraps from 0xffffffff to 0x00000000
 - Be atomic (no possibility of corruption by an interrupt)

The counter behind get_raw_ms() can be maintained by either:

 1. A hardware timer programmed with a 1ms increment
 2. A hardware timer programmed with a non-1ms increment scaled in software
 3. A software counter ticked by a 1ms interrupt
 4. A software counter ticked by a non-1ms interrupt scaled in software

get_raw_ms() does not need a fixed epoch - It could be 1st Jan 1970, the
date the CPU/SOC was manufactured, when the device was turned on, your
eldest child's birthday - whatever. It will not matter provided the counter
wraps correctly

> Powerpc for example has a 64 bit free running hardware counter at CPU clock,
> which can be in the GHz range, making the lower 32 bits overflow within
> seconds,
> so the full 64 bits MUST BE used to obtain a millisecond timer by division.
> arm/at91 has a timer that can be made to appear like a 32 bit free running
> counter
> at some fraction of cpu clock (can be brought into a few MHz value by a
> prescaler)
> and the current implementation extends this to 64 bits by software, so it is
> similar to powerpc.

So these are all examples of #2

x86 is an example of #3

> A get timer() simply uses this 64 bit value by dividing it by (tickrate/1000).
> 
> Of course this results in a wrong wrap "gigaseconds" after the timer has
> been started,
> but certainly this can be ignored...

Strictly speaking, I don't think we should allow this - There should never
be timer glitches

> On any account, I see only the following two functions to be implemented
> for use by
> other u-boot code parts:
> 
> 1. void udelay(u32 microseconds) with the following properties:
> - must not affect get_timer() results

Absolutely

> - must not delay less than the given time, may delay longer
> (this might be true especially for very small delay values)

Hadn't though about that, but OK

> - shall not be used for delays in the seconds range and longer
> (or any other limit we see practical)

Any udelay up to the full range of a ulong should be handled without error
by udelay() - If the arch dependant implementation of udelay() uses
get_timer() to implement long delays due to hardware limitations then that
should be fine.

> Note: a loop doing "n" udelays of "m" microseconds might take _much_ longer
> than
> "n * m" microseconds and therefore is the wrong approach to implement a
> timeout.
> get_timer() must be used for any such timeouts instead!

ACK - as mentioned, udelay() can use get_timer() of the delay is >> 1ms if
such an implementation provides better accuracy. If the HAL implementation
of get_raw_ms() uses a hardware level microsecond time base, there will be
no accuracy advantage anyway.

> 2. u32 get_timer(u32 base) with the following properties:
> - must return the elapsed milliseconds since base

ACK

> - must work without wrapping problems for at least several hours

Provided that the architecture implementation of get_raw_ms() is
implemented as described, the only limitation will be the maximum
measurable delay. The timer API should work correctly no matter how long
the device has been running

I think the HAL should implement:
	- get_raw_ms() - 32-bit millisecond counter
	- get_raw_us() - 32-bit microsecond counter
	- get_raw_us64() - 64-bit microsecond counter

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[J. William Campbell]

On 5/21/2011 9:26 PM, Reinhard Meyer wrote:
> Dear Graeme Russ,
>> Hi All,
>>
>> I've just had a good look through the timer API code with the view of
>> rationalising it and fixing up some of the inconsistencies. What I found
>> was a little scarier than I expected! Anyway, here is a write-up of what I
>> found - Please comment
> We have been at this discussion a multiple of times :) but never reached a consent.
>
> However, at current master, I have reduced at91 architecture to only use
> get_timer(base), set_timer() never existed and reset_timer() has been removed.
>
> As it showed up recently, common cfi code still calls reset_timer() - which certainly
> can be fixed with little effort...
>
>> At the lowest level, the U-Boot timer API consists of a unsigned 32-bit
>> free running timestamp which increments every millisecond (wraps around
>> every 4294967 seconds, or about every 49.7 days). The U-Boot timer API
>> allows:
>>    - Time deltas to be measured between arbitrary code execution points
>> 	ulong start_time;
>> 	ulong elapsed_time;
>>
>> 	start_time = get_timer(0);
>> 	...
>> 	elapsed_time = get_timer(start_time);
>>
>>    - Repetition of code for a specified duration
>> 	ulong start_time;
>>
>> 	start_time = get_timer(0);
>>
>> 	while (get_timer(start_time)<   REPEAT_TIME) {
>> 		...
>> 	}
>>
>>    - Device timeout detection
>> 	ulong start_time;
>>
>> 	send_command_to_device();
>> 	start = get_timer (0);
>> 	while (device_is_busy()) {
>> 		if (get_timer(start)>   TIMEOUT)
>> 			return ERR_TIMOUT;
>> 		udelay(1);
>> 	}
>> 	return ERR_OK;
> correct.
>
>> The U-Boot timer API is not a 'callback' API and cannot 'trigger' a
>> function call after a pre-determined time.
> that would be too complex to implement and of little use in a single task
> system. u-boot can do fine with polling.
>
>> NOTE: http://lists.denx.de/pipermail/u-boot/2010-June/073024.html appears
>> to imply the following implementation of get_timer() is wrong:
>>
>> 	ulong get_timer(ulong base)
>> 	{
>> 		return get_timer_masked() - base;
>> 	}
> Is is not wrong as long as get_timer_masked() returns the full 32 bit space
> of numbers and 0xffffffff is followed by 0x00000000. Most implementations
> probably do NOT have this property.
>
>> U-Boot Timer API Details
>> ========================
>> There are currently three functions in the U-Boot timer API:
>> 	ulong get_timer(ulong start_time)
> As you point out in the following, this is the only function required.
> However it REQUIRES that the internal timer value must exploit the full
> 32 bit range of 0x00000000 to 0xffffffff before it wraps back to 0x00000000.
>
>> 	void set_timer(ulong preset)
>> 	void reset_timer(void)
>>
>> get_timer() returns the number of milliseconds since 'start_time'. If
>> 'start_time' is zero, therefore, it returns the current value of the
>> free running counter which can be used as a reference for subsequent
>> timing measurements.
>>
>> set_timer() sets the free running counter to the value of 'preset'
>> reset_timer() sets the free running counter to the value of zero[1]. In
>> theory, the following examples are all identical
>>
>> 	----------------------------------------------------
>> 	ulong start_time;
>> 	ulong elapsed_time;
>>
>> 	start_time = get_timer(0);
>> 	...
>> 	elapsed_time = get_timer(start_time);
>> 	----------------------------------------------------
>> 	ulong elapsed_time;
>>
>> 	reset_timer();
>> 	...
>> 	elapsed_time = get_timer(0);
>> 	----------------------------------------------------
>> 	ulong elapsed_time;
>>
>> 	set_timer(0);
>> 	...
>> 	elapsed_time = get_timer(0);
>> 	----------------------------------------------------
>>
>> [1] arch/arm/cpu/arm926ejs/at91/ and arch/arm/cpu/arm926ejs/davinci/ are
>> exceptions, they set the free running counter to get_ticks() instead
> Not anymore on at91.
>> Architecture Specific Peculiarities
>> ===================================
>> ARM
>>    - Generally define get_timer_masked() and reset_timer_masked()
>>    - [get,reset]_timer_masked() are exposed outside arch\arm which is a bad
>>      idea as no other arches define these functions - build breakages are
>>      possible although the external files are most likely ARM specific (for
>>      now!)
>>    - Most CPUs define their own versions of the API get/set functions which
>>      are wrappers to the _masked equivalents. These all tend to be the same.
>>      The API functions could be moved into arch/arm/lib and made weak for
>>      the rare occasions where they need to be different
>>    - Implementations generally look sane[2] except for the following:
>>      - arm_intcm - No timer code (unused CPU arch?)
>>      - arm1136/mx35 - set_timer() is a NOP
>>      - arm926ejs/at91 - reset_timer() sets counter to get_ticks()
>>                         no implelemtation of set_timer()
> See current master for actual implementation!
>>      - arm926ejs/davinci - reset_timer() sets counter to get_ticks()
>>                            no implelemtation of set_timer()
>>      - arm926ejs/mb86r0x - No implementation of set_timer()
>>      - arm926ejs/nomadik - No implementation of set_timer()
>>      - arm946es - No timer code (unused CPU arch?)
>>      - ixp - No implementation of set_timer()
>>            - If CONFIG_TIMER_IRQ is defined, timer is incremented by an
>>              interrupt routine - reset_timer() writes directly to the
>>              counter without interrupts disables - Could cause corruption
>>              if reset_timer() is not atomic
>>            - If CONFIG_TIMER_IRQ is not defined, reset_timer() appears to
>>              not be implemented
>>      - pxa - set_timer() is a NOP
>>      - sa1100 - get_timer() does not subtract the argument
>>
>> nios, powerpc, sparc, x86
>>    - All look sane[2]
>>
>> avr32
>>    - Not obvious, but looks sane[2]
>>
>> blackfin
>>    - Does not implement set_timer()
>>    - Provides a 64-bit get_ticks() which simply returns 32-bit get_timer(0)
>>    - reset_timer() calls timer_init()
>>    - Looks sane[2]
>>
>> m68k
>>    - Looks sane[2] if CONFIG_MCFTMR et al are defined. If CONFIG_MCFPIT is
>>      defined instead, reset_timer() is unimplemented and build breakage will
>>      result if cfi driver is included in the build - No configurations use
>>      this define, so that code is dead anyway
>>
>> microblaze
>>    - Only sane[2] if CONFIG_SYS_INTC_0 and CONFIG_SYS_TIMER_0 are both defined.
>>      Doing so enables a timer interrupt which increments the internal
>>      counter. Otherwise, it is incremented when get_timer() is called which
>>      will lead to horrible (i.e. none at all) accuracy
>>
>> mips
>>    - Not obvious, but looks sane[2]
>>
>> sh
>>    - Generally looks sane[2]
>>    - Writes 0xffff to the CMCOR_0 control register when resetting to zero,
>>      but writes the actual 'set' value when not zero
>>    - Uses a 32-bit microsecond based timebase:
>> 	static unsigned long get_usec (void)
>> 	{
>> 	...
>> 	}
>>
>> 	get_timer(ulong base)
>> 	{
>> 		return (get_usec() / 1000) - base;
>> 	}
>>
>>    - Timer will wrap after ~4295 seconds (71 minutes)
>>
>> [2] Sane means something close to:
>> 	void reset_timer (void)
>> 	{
>> 		timestamp = 0;
>> 	}
>>
>> 	ulong get_timer(ulong base)
>> 	{
>> 		return timestamp - base;
>> 	}
>>
>> 	void set_timer(ulong t)
>> 	{
>> 		timestamp = t;
>> 	}
>>
>> Rationalising the API
>> =====================
>> Realistically, the value of the free running timer at the start of a timing
>> operation is irrelevant (even if the counter wraps during the timed period).
>> Moreover, using reset_timer() and set_timer() makes nested usage of the
>> timer API impossible. So in theory, the entire API could be reduced to
>> simply get_timer()
> Full ACK here !!!
>> 0. Fix arch/arm/cpu/sa1100/timer.c:get_timer()
>> ----------------------------------------------
>> This appears to be the only get_timer() which does not subtract the
>> argument from timestamp
>>
>> 1. Remove set_timer()
>> ---------------------
>> regex "[^e]set_timer\s*\([^)]*\);" reveals 14 call sites for set_timer()
>> and all except arch/sh/lib/time_sh2:[timer_init(),reset_timer()] are
>> set_timer(0). The code in arch/sh is trivial to resolve in order to
>> remove set_timer()
>>
>> 2. Remove reset_timer()
>> -----------------------------------------------
>> regex "[\t ]*reset_timer\s*\([^)]*\);" reveals 22 callers across the
>> following groups:
>>    - timer_init() - Make reset_timer() static or fold into timer_init()
>>    - board_init_r() - Unnecessary
>>    - arch/m68k/lib/time.c:wait_ticks() - convert[3]
>>    - Board specific flash drivers - convert[3]
>>    - drivers/block/mg_disk.c:mg_wait() - Unnecessary
>>    - drivers/mtd/cfi_flash.c:flash_status_check() - Unnecessary
>>    - drivers/mtd/cfi_flash.c:flash_status_poll() - Unnecessary
>>
>> [3] These instance can be trivially converted to use one of the three
>> examples at the beginning of this document
>>
>> 3. Remove reset_timer_masked()
>> ------------------------------
>> This is only implemented in arm but has propagated outside arch/arm and
>> into board/ and drivers/ (bad!)
>>
>> regex "[\t ]*reset_timer_masked\s*\([^)]*\);" reveals 135 callers!
>>
>> A lot are in timer_init() and reset_timer(), but the list includes:
>>    - arch/arm/cpu/arm920t/at91rm9200/spi.c:AT91F_SpiWrite()
>>    - arch/arm/cpu/arm926ejs/omap/timer.c:__udelay()
>>    - arch/arm/cpu/arm926ejs/versatile/timer.c:__udelay()
>>    - arch/arm/armv7/s5p-common/timer.c:__udelay()
>>    - arch/arm/lh7a40x/timer.c:__udelay()
>>    - A whole bunch of board specific flash drivers
>>    - board/mx1ads/syncflash.c:flash_erase()
>>    - board/trab/cmd_trab.c:do_burn_in()
>>    - board/trab/cmd_trab.c:led_blink()
>>    - board/trab/cmd_trab.c:do_temp_log()
>>    - drivers/mtd/spi/eeprom_m95xxx.c:spi_write()
>>    - drivers/net/netarm_eth.c:na_mii_poll_busy()
>>    - drivers/net/netarm_eth.c:reset_eth()
>>    - drivers/spi/atmel_dataflash_spi.c/AT91F_SpiWrite()
> Fixed in current master.
>> Most of these instance can be converted to use one of the three examples
>> at the beginning of this document, but __udelay() will need closer
>> examination
>>
>> This is preventing nesting of timed code - Any code which uses udelay()
>> has the potential to foul up outer-loop timers. The problem is somewhat
>> unique to ARM though
>>
>> 4. Implement microsecond API - get_usec_timer()
>> -----------------------------------------------
>>    - Useful for profiling
>>    - A 32-bit microsecond counter wraps in 71 minutes - Probably OK for most
>>      U-Boot usage scenarios
>>    - By default could return get_timer() * 1000 if hardware does not support
>>      microsecond accuracy - Beware of results>   32 bits!
>>    - If hardware supports microsecond resolution counters, get_timer() could
>>      simply use get_usec_timer() / 1000
> That is wrong. Dividing 32 bits by any number will result in a result that does not
> exploit the full 32 bit range, i.e. wrap from (0xffffffff/1000) to 0x00000000,
> which makes time differences go wrong when they span across such a wrap!
Hi All,
       True, as previously discussed.
>>    - get_usec_timer_64() could offer a longer period (584942 years!)
> Correct. And a "must be" when one uses such division.
>
> But you have to realize that most hardware does not provide a simple means to
> implement a timer that runs in either exact microseconds or exact milliseconds.
Also True.
> Powerpc for example has a 64 bit free running hardware counter at CPU clock,
> which can be in the GHz range, making the lower 32 bits overflow within seconds,
> so the full 64 bits MUST BE used to obtain a millisecond timer by division.
       This statement, as written, is false. While it is true that the 
power PC (and others) have a 64 bit counter that runs at a many 
megahertz rate, it is NOT true that all 64 bits must be used to obtain a 
millisecond timer. A millisecond timer can be produced by using the 33 
bits of the 64 bit counter whose lsb is <= 1 ms and > 0.5 ms. This value 
can be extracted by a right shift and masking of the 64 bit counter. Two 
32 bit multiplies and adds will produce a correct monotonic 1 ms timer. 
This is certainly the best way to go on systems that do not have a 
hardware 64 bit by 32 bit divide. See the thread "ARM timing code 
refactoring" for an example of how this can be done. This timer will 
NEVER result in a wrong value, although it does loose  1 ms every 1193 
hours.
> arm/at91 has a timer that can be made to appear like a 32 bit free running counter
> at some fraction of cpu clock (can be brought into a few MHz value by a prescaler)
> and the current implementation extends this to 64 bits by software, so it is
> similar to powerpc.
>
> A get timer() simply uses this 64 bit value by dividing it by (tickrate/1000).
Or as above on the PPC
> Of course this results in a wrong wrap "gigaseconds" after the timer has been started,
> but certainly this can be ignored...
FWIW, this does not happen with the shift and multiply approach.
> On any account, I see only the following two functions to be implemented for use by
> other u-boot code parts:
>
> 1. void udelay(u32 microseconds) with the following properties:
> - must not affect get_timer() results
> - must not delay less than the given time, may delay longer
> (this might be true especially for very small delay values)
> - shall not be used for delays in the seconds range and longer
> (or any other limit we see practical)
> Note: a loop doing "n" udelays of "m" microseconds might take _much_ longer than
> "n * m" microseconds and therefore is the wrong approach to implement a timeout.
> get_timer() must be used for any such timeouts instead!
>
> 2. u32 get_timer(u32 base) with the following properties:
> - must return the elapsed milliseconds since base
> - must work without wrapping problems for at least several hours
>
> Best Regards,
> Reinhard
I think this is a correct appreciation of what is needed.

Best Regards.
Bill Campbell
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>

[U-Boot] [RFC] Review of U-Boot timer API

[J. William Campbell]

On 5/21/2011 11:23 PM, Graeme Russ wrote:
> On 22/05/11 14:26, Reinhard Meyer wrote:
>> Dear Graeme Russ,
>>> Hi All,
>>>
>>> I've just had a good look through the timer API code with the view of
>>> rationalising it and fixing up some of the inconsistencies. What I found
>>> was a little scarier than I expected! Anyway, here is a write-up of what I
>>> found - Please comment
>> We have been at this discussion a multiple of times :) but never reached a
>> consent.
>>
>> However, at current master, I have reduced at91 architecture to only use
>> get_timer(base), set_timer() never existed and reset_timer() has been removed.
> Excellent
>
>> As it showed up recently, common cfi code still calls reset_timer() - which
>> certainly
>> can be fixed with little effort...
> Yes, this is one of the easy fixes as all call sites already use the start
> = get_timer(0), elapsed = get_timer(start) convention anyway - The
> reset_timer() calls are 100% redundant (provided get_timer() behaves
> correctly at the 32-bit rollover for all arches)
>
>>> The U-Boot timer API is not a 'callback' API and cannot 'trigger' a
>>> function call after a pre-determined time.
>> that would be too complex to implement and of little use in a single task
>> system. u-boot can do fine with polling.
> I am in no way suggesting this - I just want to clarify the API for anyone
> who needs to use it
>
>>> NOTE: http://lists.denx.de/pipermail/u-boot/2010-June/073024.html appears
>>> to imply the following implementation of get_timer() is wrong:
>>>
>>>      ulong get_timer(ulong base)
>>>      {
>>>          return get_timer_masked() - base;
>>>      }
>> Is is not wrong as long as get_timer_masked() returns the full 32 bit space
>> of numbers and 0xffffffff is followed by 0x00000000. Most implementations
>> probably do NOT have this property.
>>> U-Boot Timer API Details
>>> ========================
>>> There are currently three functions in the U-Boot timer API:
>>>      ulong get_timer(ulong start_time)
>> As you point out in the following, this is the only function required.
>> However it REQUIRES that the internal timer value must exploit the full
>> 32 bit range of 0x00000000 to 0xffffffff before it wraps back to 0x00000000.
> So this needs to be clearly spelt out in formal documentation
>
>>> Rationalising the API
>>> =====================
>>> Realistically, the value of the free running timer at the start of a timing
>>> operation is irrelevant (even if the counter wraps during the timed period).
>>> Moreover, using reset_timer() and set_timer() makes nested usage of the
>>> timer API impossible. So in theory, the entire API could be reduced to
>>> simply get_timer()
>> Full ACK here !!!
> I don't think there will be much resistance to this
>
>>> 3. Remove reset_timer_masked()
>>> ------------------------------
>>> This is only implemented in arm but has propagated outside arch/arm and
>>> into board/ and drivers/ (bad!)
>>>
>>> regex "[\t ]*reset_timer_masked\s*\([^)]*\);" reveals 135 callers!
>>>
>>> A lot are in timer_init() and reset_timer(), but the list includes:
>>>    - arch/arm/cpu/arm920t/at91rm9200/spi.c:AT91F_SpiWrite()
>>>    - arch/arm/cpu/arm926ejs/omap/timer.c:__udelay()
>>>    - arch/arm/cpu/arm926ejs/versatile/timer.c:__udelay()
>>>    - arch/arm/armv7/s5p-common/timer.c:__udelay()
>>>    - arch/arm/lh7a40x/timer.c:__udelay()
>>>    - A whole bunch of board specific flash drivers
>>>    - board/mx1ads/syncflash.c:flash_erase()
>>>    - board/trab/cmd_trab.c:do_burn_in()
>>>    - board/trab/cmd_trab.c:led_blink()
>>>    - board/trab/cmd_trab.c:do_temp_log()
>>>    - drivers/mtd/spi/eeprom_m95xxx.c:spi_write()
>>>    - drivers/net/netarm_eth.c:na_mii_poll_busy()
>>>    - drivers/net/netarm_eth.c:reset_eth()
>>>    - drivers/spi/atmel_dataflash_spi.c/AT91F_SpiWrite()
>> Fixed in current master.
> Excellent. I have not pulled master for a little while, guess I should
>
>>>    - If hardware supports microsecond resolution counters, get_timer() could
>>>      simply use get_usec_timer() / 1000
>> That is wrong. Dividing 32 bits by any number will result in a result that
>> does not
>> exploit the full 32 bit range, i.e. wrap from (0xffffffff/1000) to 0x00000000,
>> which makes time differences go wrong when they span across such a wrap!
>>
> Yes, this has already been pointer out - 42 bits are needed as a bare
> minimum. However, we can get away with 32-bits provided get_timer() is
> called at least every 71 minutes
>
> P.S. Can we use the main loop to kick the timer?
>
>>>    - get_usec_timer_64() could offer a longer period (584942 years!)
>> Correct. And a "must be" when one uses such division.
> Unless we can rely on get_timer() to be called at least every 71 minutes in
> which case we can handle the msb's without error in software
>
>> But you have to realize that most hardware does not provide a simple means to
>> implement a timer that runs in either exact microseconds or exact
>> milliseconds.
> This is where things get interesting and we need to start pushing a
> mandated low-level HAL. For example, I believe get_timer() should be
> implemented in /lib as:
>
> 	ulong get_timer(ulong base)
> 	{
> 		return get_raw_msec() - base;
> 	}
>
> get_raw_ms() MUST:
>   - Return an unsigned 32-but value which increments every 1ms
>   - Wraps from 0xffffffff to 0x00000000
>   - Be atomic (no possibility of corruption by an interrupt)
Hi All,
       I agree that a mandated HAL would be a real good idea in reducing 
the uncertainty in what is required. This is a very good idea, IMHO.
> The counter behind get_raw_ms() can be maintained by either:
>
>   1. A hardware timer programmed with a 1ms increment
>   2. A hardware timer programmed with a non-1ms increment scaled in software
>   3. A software counter ticked by a 1ms interrupt
>   4. A software counter ticked by a non-1ms interrupt scaled in software
>
> get_raw_ms() does not need a fixed epoch - It could be 1st Jan 1970, the
> date the CPU/SOC was manufactured, when the device was turned on, your
> eldest child's birthday - whatever. It will not matter provided the counter
> wraps correctly
>
>> Powerpc for example has a 64 bit free running hardware counter at CPU clock,
>> which can be in the GHz range, making the lower 32 bits overflow within
>> seconds,
>> so the full 64 bits MUST BE used to obtain a millisecond timer by division.
>> arm/at91 has a timer that can be made to appear like a 32 bit free running
>> counter
>> at some fraction of cpu clock (can be brought into a few MHz value by a
>> prescaler)
>> and the current implementation extends this to 64 bits by software, so it is
>> similar to powerpc.
> So these are all examples of #2
>
> x86 is an example of #3
>
>> A get timer() simply uses this 64 bit value by dividing it by (tickrate/1000).
>>
>> Of course this results in a wrong wrap "gigaseconds" after the timer has
>> been started,
>> but certainly this can be ignored...
> Strictly speaking, I don't think we should allow this - There should never
> be timer glitches
Agreed, although this glitch probably would never be observed.
>> On any account, I see only the following two functions to be implemented
>> for use by
>> other u-boot code parts:
>>
>> 1. void udelay(u32 microseconds) with the following properties:
>> - must not affect get_timer() results
> Absolutely
>
>> - must not delay less than the given time, may delay longer
>> (this might be true especially for very small delay values)
> Hadn't though about that, but OK
>
>> - shall not be used for delays in the seconds range and longer
>> (or any other limit we see practical)
> Any udelay up to the full range of a ulong should be handled without error
> by udelay() - If the arch dependant implementation of udelay() uses
> get_timer() to implement long delays due to hardware limitations then that
> should be fine.
I wonder about this requirement. I think it might be better to say that 
udelay can only be used up to a delay of a short unsigned (65 ms), If a 
longer delay is required, a get_timer loop should be used.  Since udelay 
uses a different resolution, it implies a more precise delay is 
required. If you are delaying more than 65 ms, I suspect precision is 
not what you are after, and udelay should return an error (not be void). 
This makes implementing udelay a lot easier, in that one can always 
convert the desired time interval into an internal clock resolution 
without difficulty.
>> Note: a loop doing "n" udelays of "m" microseconds might take _much_ longer
>> than
>> "n * m" microseconds and therefore is the wrong approach to implement a
>> timeout.
>> get_timer() must be used for any such timeouts instead!
> ACK - as mentioned, udelay() can use get_timer() of the delay is>>  1ms if
> such an implementation provides better accuracy. If the HAL implementation
> of get_raw_ms() uses a hardware level microsecond time base, there will be
> no accuracy advantage anyway.
What if it provides worse accuracy? It is better if udelay can use the 
hardware timer resolution directly. This is easy if the delay is a short 
unsigned, so the desired delay can be internally expressed as a long 
unsigned in hardware timer ticks. Otherwise, it gets harder, and udelay 
is not oriented towards longer delays. It CAN be done, but why should we?
>> 2. u32 get_timer(u32 base) with the following properties:
>> - must return the elapsed milliseconds since base
> ACK
>
>> - must work without wrapping problems for at least several hours
> Provided that the architecture implementation of get_raw_ms() is
> implemented as described, the only limitation will be the maximum
> measurable delay. The timer API should work correctly no matter how long
> the device has been running
>
> I think the HAL should implement:
> 	- get_raw_ms() - 32-bit millisecond counter
Only the get_raw_ms should be mandatory. I propose adding udelay to the 
HAL, as it is best mechanized at the hardware timer tick level.

> 	- get_raw_us() - 32-bit microsecond counter
Used for performance monitor. Not mandatory.
> 	- get_raw_us64() - 64-bit microsecond counter
I can see no possible use for this in u-boot. What operation that we 
care about could take this long  and require microsecond resolution? 71 
seconds is certainly long enough for anything this precise. This 
certainly should NOT be mandatory. 64 bits is not for free on many 
processors.

Best Regards,
Bill Campbell
> Regards,
>
> Graeme
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

On 22/05/11 17:21, J. William Campbell wrote:
> On 5/21/2011 11:23 PM, Graeme Russ wrote:
>> On 22/05/11 14:26, Reinhard Meyer wrote:

>>> - shall not be used for delays in the seconds range and longer
>>> (or any other limit we see practical)
>> Any udelay up to the full range of a ulong should be handled without error
>> by udelay() - If the arch dependant implementation of udelay() uses
>> get_timer() to implement long delays due to hardware limitations then that
>> should be fine.
> I wonder about this requirement. I think it might be better to say that
> udelay can only be used up to a delay of a short unsigned (65 ms), If a

We would have to change the prototype for udelay()

> longer delay is required, a get_timer loop should be used.  Since udelay
> uses a different resolution, it implies a more precise delay is required.

Agree

> If you are delaying more than 65 ms, I suspect precision is not what you
> are after, and udelay should return an error (not be void). This makes
> implementing udelay a lot easier, in that one can always convert the
> desired time interval into an internal clock resolution without difficulty.

Who is ever going to check if udelay() returned an error? It is a primitive
that one assumes 'just works' and should work for all possible parameter values

>>> Note: a loop doing "n" udelays of "m" microseconds might take _much_ longer
>>> than
>>> "n * m" microseconds and therefore is the wrong approach to implement a
>>> timeout.
>>> get_timer() must be used for any such timeouts instead!
>> ACK - as mentioned, udelay() can use get_timer() of the delay is>>  1ms if
>> such an implementation provides better accuracy. If the HAL implementation
>> of get_raw_ms() uses a hardware level microsecond time base, there will be
>> no accuracy advantage anyway.
> What if it provides worse accuracy? It is better if udelay can use the
> hardware timer resolution directly. This is easy if the delay is a short
> unsigned, so the desired delay can be internally expressed as a long
> unsigned in hardware timer ticks. Otherwise, it gets harder, and udelay is
> not oriented towards longer delays. It CAN be done, but why should we?

What if the udelay parameter is calculated? You would then need an explicit
check for >65ms - Seems a bit onerous if you're expecting to occasionally
(if ever) to exceed this, but it not being out of the realm of possibility

>>> 2. u32 get_timer(u32 base) with the following properties:
>>> - must return the elapsed milliseconds since base
>> ACK
>>
>>> - must work without wrapping problems for at least several hours
>> Provided that the architecture implementation of get_raw_ms() is
>> implemented as described, the only limitation will be the maximum
>> measurable delay. The timer API should work correctly no matter how long
>> the device has been running
>>
>> I think the HAL should implement:
>>     - get_raw_ms() - 32-bit millisecond counter
> Only the get_raw_ms should be mandatory. I propose adding udelay to the
> HAL, as it is best mechanized at the hardware timer tick level.
> 
>>     - get_raw_us() - 32-bit microsecond counter
> Used for performance monitor. Not mandatory.

True, but useful

>>     - get_raw_us64() - 64-bit microsecond counter
> I can see no possible use for this in u-boot. What operation that we care
> about could take this long  and require microsecond resolution? 71 seconds

*cough* minutes *cough* ;)

> is certainly long enough for anything this precise. This certainly should
> NOT be mandatory. 64 bits is not for free on many processors.

True - I agree there is no need for it

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Reinhard Meyer]

Dear J. William Campbell,

please demonstrate for me (and others), by a practical example,
how _any_ arithmetic (even less with just shifts and multiplies)
can convert a free running 3.576 MHz (wild example) free running
32 bit counter (maybe software extended to 64 bits) into a ms
value that will properly wrap from 2**32-1 to 0 ?

I fail to see how that will be possible...

Best Regards,
Reinhard

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

Dear Reinhard,

On Sun, May 22, 2011 at 6:15 PM, Reinhard Meyer
<u-boot@emk-elektronik.de> wrote:
> Dear J. William Campbell,
>
> please demonstrate for me (and others), by a practical example,
> how _any_ arithmetic (even less with just shifts and multiplies)
> can convert a free running 3.576 MHz (wild example) free running
> 32 bit counter (maybe software extended to 64 bits) into a ms
> value that will properly wrap from 2**32-1 to 0 ?
>
> I fail to see how that will be possible...

These may help:

http://blogs.msdn.com/b/devdev/archive/2005/12/12/502980.aspx
http://www.hackersdelight.org/divcMore.pdf

of Google 'division of unsigned integer by a constant'

Basically you will be dividing by a constant value of 3576 which can be
highly optimised

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[J. William Campbell]

On 5/22/2011 1:15 AM, Reinhard Meyer wrote:
> Dear J. William Campbell,
>
> please demonstrate for me (and others), by a practical example,
> how _any_ arithmetic (even less with just shifts and multiplies)
> can convert a free running 3.576 MHz (wild example) free running
> 32 bit counter (maybe software extended to 64 bits) into a ms
> value that will properly wrap from 2**32-1 to 0 ?
Hi All
       I accept the challenge! I will present two ways to do this, one 
using a 32 bit by 16 bit divide, and one using only multiplies.
This first method is "exact", in that there is no difference in 
performance from a "hardware" counter ticking at the 1 ms rate. This is 
accomplished by operating the 1 ms counter based on the delta time in 
the hardware time base. It is necessary to call this routine often 
enough that the hardware counter does not wrap more than once between 
calls. This is not really a problem, as this time is 1201 seconds or so. 
If the routine is not called for a long time, or at the first call, it 
will return a timer_in_ms value that will work for all subsequent calls 
that are within a hardware rollover interval. Since the timer in ms is a 
32 bit number anyway. The same rollover issue will exist if you 
"software extend" the timer to 64 bits. You must assume 1 rollover. If 
it is more than 1, the timer is wrong.


The variables in the gd are
u32 prev_timer;
u32 timer_in_ms;
u16 timer_remainder;

/* gd->timer remainder must be initialized to 0 (actually, an number 
less than 3576, but 0 is nice). Other two variables don't matter but can 
be initialized if desired  */

u32 get_raw_ms()
{
         u32 delta;
        u32  t_save;

       read(t_save);   /* atomic read of the hardware 32 bit timer 
running at 3.576 MHz */
       delta_t = (t_save  - gd->prev_timer) ;

       gd->prev_timer =  t_save;
      /*
        Hopefully, the following two lines only results in one hardware 
divide when optimized. If your CPU has no hardware divide, or if it 
slow, see second method .
     */
      gd->timer_in_ms += delta_t  / 3576; /* convert elapsed time to ms */
      gd->timer_remainder += delta_t  % 3576; /* add in remaining part 
not included above */
      if (gd->timer_remainder >= 3576) /* a carry has been detected */
     {
        ++gd->timer_in_ms;
        gd->timer_remainder -= 3576; /* fix remainder for the carry above */
     }

      return(gd->timer_in_ms)
}

This approach works well when the number of ticks per ms is an exact 
number representable as a small integer, as it is in this case. It is 
exact with a clock rate of 600 MHz, but is not exact for a clock rate of 
666 MHz. 666667 is not an exact estimate of ticks per ms, It is off by 
0.00005 % That should be acceptable for use as a timeout delay. The 
accumulated error in a 10 second delay should be less than 0.5 ms.

There is a way that the divide above can be approximated by multiplying 
by an appropriate fraction, taking the resulting delta t in ms, 
multiplying it by 3576, and subtracting the product from the original 
delta to get the remainder.  This is the way to go if your CPU divides 
slowly or not at all. This approach is presented below.

the vaues in gd are as follows:

u32 prev_timer;
u32 timer_in_ms;

/*
     One tick of the 3.576 MHz timer corresponds to 1/3.576/1000 ms,
     or 0.000279642 ms. Scale the fraction by 65536 (16 bit shift),
     you get 37532.9217
*/
u32 get_raw_ms(void)
{
   u32  t_save;
   u32  temp;

   /* read the hardware 32 bit timer running at 3.576 MHz */
   read_timer_atomic(t_save);
   t_save         -= gd->prev_timer; /* get "delta time" since last call */
   gd->prev_timer += t_save; /* assume we will use all of the counts */

   /*
    * This first while loop is entered for any delta time > about 18.3 
ms. The
    * while loop will execute twice 2.734% of the time, otherwise once.
    */
   while (t_save > 65535)
   {
     temp = t_save >> 16;   /* extract msb */
     temp  = ((temp * 37532) + ((temp * 60404) >> 16)) >> 11;
     /* temp  = (temp * 37532) >> 11; */
     gd->timer_in_ms += temp;
     t_save          -= temp * 3576;
   }
   /*
    * This second while loop is entered for 94.837% of all possible 
delta times,
    * 0 through 0XFFFFFFFF. The second while loop will execute twice 
0.037% of
    * the time, otherwise once.
    */
   while (t_save >= 3576)
   {
     temp  = (t_save * 37532) >> (16 + 11);
     if (temp == 0)
       temp = 1;     /* we know that 1 works for sure */
     gd->timer_in_ms += temp;
     t_save          -= temp * 3576;
   }
   gd->prev_timer -= t_save; /* restore any counts we didn't use this 
time */
   return gd->timer_in_ms;
}

I have tested this code and it seems to work fine for me. I have 
attached a more readable copy as "example .c" for those who wish to play 
around with it. In my original post, I had a version of this code that 
could be used with different clock rates. I can provide the same 
functionality with this code, for CPUs/systems where the clock rate is 
unknown at compile time or is variable. I can also address the error 
encountered by non-integer ticks per millisecond, if people really think 
the error is enough to matter, but I won't post it unless people want to 
see it.

Best Regards,
Bill Campbell




>
> I fail to see how that will be possible...
>
> Best Regards,
> Reinhard
>
>

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: example.c
Url: http://lists.denx.de/pipermail/u-boot/attachments/20110522/8aee390d/attachment.txt 

[U-Boot] [RFC] Review of U-Boot timer API

[J. William Campbell]

On 5/22/2011 5:02 PM, Graeme Russ wrote:
> Dear Reinhard,
>
> On Sun, May 22, 2011 at 6:15 PM, Reinhard Meyer
> <u-boot@emk-elektronik.de>  wrote:
>> Dear J. William Campbell,
>>
>> please demonstrate for me (and others), by a practical example,
>> how _any_ arithmetic (even less with just shifts and multiplies)
>> can convert a free running 3.576 MHz (wild example) free running
>> 32 bit counter (maybe software extended to 64 bits) into a ms
>> value that will properly wrap from 2**32-1 to 0 ?
>>
>> I fail to see how that will be possible...
> These may help:
>
> http://blogs.msdn.com/b/devdev/archive/2005/12/12/502980.aspx
> http://www.hackersdelight.org/divcMore.pdf
>
> of Google 'division of unsigned integer by a constant'
>
> Basically you will be dividing by a constant value of 3576 which can be
> highly optimised
Hi All,
        Yes, you can do it this way. I prefer not to use properties of 
the constant because it doesn't allow multiple clock rates easily. The 
code I posted just a moment ago is easy to change for different rates 
because it does not rely much on the exact bit structure of 3576.
There are simple formulas for all the "magic numbers" in what I posted, 
so it is easy to change the input frequency.
Best Regards,
Bill Campbell
> Regards,
>
> Graeme
>
>

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

Hi Bill,

On Mon, May 23, 2011 at 10:14 AM, J. William Campbell
<jwilliamcampbell@comcast.net> wrote:
> On 5/22/2011 1:15 AM, Reinhard Meyer wrote:
>>
>> Dear J. William Campbell,
>>
>> please demonstrate for me (and others), by a practical example,
>> how _any_ arithmetic (even less with just shifts and multiplies)
>> can convert a free running 3.576 MHz (wild example) free running
>> 32 bit counter (maybe software extended to 64 bits) into a ms
>> value that will properly wrap from 2**32-1 to 0 ?
>
> Hi All
> ? ? ?I accept the challenge! I will present two ways to do this, one using a

[Snip first method]

> There is a way that the divide above can be approximated by multiplying by
> an appropriate fraction, taking the resulting delta t in ms, multiplying it
> by 3576, and subtracting the product from the original delta to get the
> remainder. ?This is the way to go if your CPU divides slowly or not at all.
> This approach is presented below.
>
> the vaues in gd are as follows:
>
> u32 prev_timer;
> u32 timer_in_ms;
>
> /*
> ? ?One tick of the 3.576 MHz timer corresponds to 1/3.576/1000 ms,
> ? ?or 0.000279642 ms. Scale the fraction by 65536 (16 bit shift),
> ? ?you get 37532.9217
> */
> u32 get_raw_ms(void)
> {
> ?u32 ?t_save;
> ?u32 ?temp;
>
> ?/* read the hardware 32 bit timer running at 3.576 MHz */
> ?read_timer_atomic(t_save);
> ?t_save ? ? ? ? -= gd->prev_timer; /* get "delta time" since last call */
> ?gd->prev_timer += t_save; /* assume we will use all of the counts */
>
> ?/*
> ? * This first while loop is entered for any delta time > about 18.3 ms. The
> ? * while loop will execute twice 2.734% of the time, otherwise once.
> ? */
> ?while (t_save > 65535)
> ?{
> ? ?temp = t_save >> 16; ? /* extract msb */
> ? ?temp ?= ((temp * 37532) + ((temp * 60404) >> 16)) >> 11;

Where does 60404 come from?

> ? ?/* temp ?= (temp * 37532) >> 11; */
> ? ?gd->timer_in_ms += temp;
> ? ?t_save ? ? ? ? ?-= temp * 3576;
> ?}
> ?/*
> ? * This second while loop is entered for 94.837% of all possible delta
> times,
> ? * 0 through 0XFFFFFFFF. The second while loop will execute twice 0.037% of
> ? * the time, otherwise once.
> ? */
> ?while (t_save >= 3576)
> ?{
> ? ?temp ?= (t_save * 37532) >> (16 + 11);
> ? ?if (temp == 0)
> ? ? ?temp = 1; ? ? /* we know that 1 works for sure */
> ? ?gd->timer_in_ms += temp;
> ? ?t_save ? ? ? ? ?-= temp * 3576;
> ?}
> ?gd->prev_timer -= t_save; /* restore any counts we didn't use this time */
> ?return gd->timer_in_ms;
> }
>
> I have tested this code and it seems to work fine for me. I have attached a
> more readable copy as "example .c" for those who wish to play around with
> it. In my original post, I had a version of this code that could be used
> with different clock rates. I can provide the same functionality with this
> code, for CPUs/systems where the clock rate is unknown at compile time or is
> variable. I can also address the error encountered by non-integer ticks per
> millisecond, if people really think the error is enough to matter, but I
> won't post it unless people want to see it.

Provided the hardware counter has high enough resolution (in this case an
extra 12 bits meaning the counter must be at least 44 bits) I assume this
methog guarantees full 32-bit resolution with correct roll-over, but if
the hardware counter is only 32-bit we run into problems. And does this
solution work for, say, the entire range of a 64-bit hardware counter
including when it wraps?

Questions:
 - If the hardware supports a native 64-bit counter, could we have a
   u64 get_raw_ticks64() HAL API and 'libify' the above function using
   either a 'tick rate' in gd or a get_tick_rate() HAL function
 - Can we reduce the HAL to u32 get_raw_ticks() and make some assumptions
   about how often get_raw_ms() is called (maybe in the main loop for
   example - We already call the watchdog periodically anyway)

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

OK, so in summary, we can (in theory) have:
 - A timer API in /lib/ with a single u32 get_timer(u32 base) function
 - A HAL with two functions
   - u32 get_raw_ticks()
   - u32 get_raw_tick_rate() which returns the tick rate in kHz (which
     max's out at just after 4GHz)
 - A helper function in /lib/ u32 get_raw_ms() which uses get_raw_ticks()
   and get_tick_rate() to correctly maintain the ms counter used by
   get_timer() - This function can be weak (so next point)
 - If the hardware supports a native 32-bit ms counter, get_raw_ms()
   can be overridden to simply return the hardware counter. In this case,
   get_raw_ticks() would return 1
 - Calling of get_raw_ticks() regularly in the main loop (how ofter will
   depend on the raw tick rate, but I image it will never be necessary
   to call more often than once every few minutes)
 - If the hardware implements a native 32-bit 1ms counter, no call in
   the main loop is required
 - An optional HAL function u32 get_raw_us() which can be used for
   performance profiling (must wrap correctly)

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Reinhard Meyer]

Dear J. William Campbell,
> On 5/22/2011 1:15 AM, Reinhard Meyer wrote:
>> Dear J. William Campbell,
>>
>> please demonstrate for me (and others), by a practical example,
>> how _any_ arithmetic (even less with just shifts and multiplies)
>> can convert a free running 3.576 MHz (wild example) free running
>> 32 bit counter (maybe software extended to 64 bits) into a ms
>> value that will properly wrap from 2**32-1 to 0 ?
> Hi All
> I accept the challenge! I will present two ways to do this, one using a 32 bit by 16 bit divide, and one using only multiplies.
> This first method is "exact", in that there is no difference in performance from a "hardware" counter ticking at the 1 ms rate. This is accomplished by operating the 1 ms counter based on the delta time in the hardware time base. It is necessary to call this routine often enough that the hardware counter does not wrap more than once between calls. This is not really a problem, as this time is 1201 seconds or so. If the routine is not called for a long time, or at the first call, it will return a timer_in_ms value that will work for all subsequent calls that are within a hardware rollover interval. Since the timer in ms is a 32 bit number anyway. The same rollover issue will exist if you "software extend" the timer to 64 bits. You must assume 1 rollover. If it is more than 1, the timer is wrong.
>
>
> The variables in the gd are
> u32 prev_timer;
> u32 timer_in_ms;
> u16 timer_remainder;
>
> /* gd->timer remainder must be initialized to 0 (actually, an number less than 3576, but 0 is nice). Other two variables don't matter but can be initialized if desired */
>
> u32 get_raw_ms()
> {
> u32 delta;
> u32 t_save;
>
> read(t_save); /* atomic read of the hardware 32 bit timer running at 3.576 MHz */
> delta_t = (t_save - gd->prev_timer) ;
>
> gd->prev_timer = t_save;
> /*
> Hopefully, the following two lines only results in one hardware divide when optimized. If your CPU has no hardware divide, or if it slow, see second method .
> */
> gd->timer_in_ms += delta_t / 3576; /* convert elapsed time to ms */
> gd->timer_remainder += delta_t % 3576; /* add in remaining part not included above */
> if (gd->timer_remainder >= 3576) /* a carry has been detected */
> {
> ++gd->timer_in_ms;
> gd->timer_remainder -= 3576; /* fix remainder for the carry above */
> }
>
> return(gd->timer_in_ms)
> }

Thank you! Basically this is similar to a Bresenham Algorithm.

>
> This approach works well when the number of ticks per ms is an exact number representable as a small integer, as it is in this case. It is exact with a clock rate of 600 MHz, but is not exact for a clock rate of 666 MHz. 666667 is not an exact estimate of ticks per ms, It is off by 0.00005 % That should be acceptable for use as a timeout delay. The accumulated error in a 10 second delay should be less than 0.5 ms.

I would think the non exact cases result in such a small error that can be
tolerated. We are using the ms tick for timeouts, not for providing a clock
or exact delays. We should just round up when calculating the divider.

Hence the hick-ups that result when this is not called frequent enough to
prevent a multiple rollover of the raw value between calls do not matter
either (they should be just documented).

>
> There is a way that the divide above can be approximated by multiplying by an appropriate fraction, taking the resulting delta t in ms, multiplying it by 3576, and subtracting the product from the original delta to get the remainder. This is the way to go if your CPU divides slowly or not at all. This approach is presented below.
>
[...]

Optimizations would be up to the implementer of such a hardware and work
only if the divider is a compile time constant. Often the divider will be
run time determined (AT91 for example).

Reinhard

[U-Boot] [RFC] Review of U-Boot timer API

[J. William Campbell]

On 5/22/2011 6:42 PM, Graeme Russ wrote:
> OK, so in summary, we can (in theory) have:
>   - A timer API in /lib/ with a single u32 get_timer(u32 base) function
>   - A HAL with two functions
>     - u32 get_raw_ticks()
>     - u32 get_raw_tick_rate() which returns the tick rate in kHz (which
>       max's out at just after 4GHz)
>   - A helper function in /lib/ u32 get_raw_ms() which uses get_raw_ticks()
>     and get_tick_rate() to correctly maintain the ms counter used by
>     get_timer() - This function can be weak (so next point)
>   - If the hardware supports a native 32-bit ms counter, get_raw_ms()
>     can be overridden to simply return the hardware counter. In this case,
>     get_raw_ticks() would return 1
>   - Calling of get_raw_ticks() regularly in the main loop (how ofter will
>     depend on the raw tick rate, but I image it will never be necessary
>     to call more often than once every few minutes)
>   - If the hardware implements a native 32-bit 1ms counter, no call in
>     the main loop is required
>   - An optional HAL function u32 get_raw_us() which can be used for
>     performance profiling (must wrap correctly)
Hi All,
       Graeme, I think you have stated exactly what is the "best" 
approach to this problem.  I will provide a version of "get_raw_ms" that 
is  initialized using get_raw_tick_rate that will work for all 
"reasonable" values of raw_tick_rate. This will be the "generic" 
solution. Both the initialization function and the get_raw_ms function 
can be overridden if there is reason to do so, like "exact" clock rates. 
I will do the same with get_raw_us. This will be posted sometime on 
Monday for people to review, and to make sure I didn't get too far off 
base. Thank you to both Graeme and Reinhard for looking at/working on 
this.. Hopefully, this solution will put this timing issue to rest for 
all future ports as well as the presently existing ones.
        On a different note, the "graylisting" application is causing 
some (about half) of my replies to the list from showing up. The 
messages are going to the specified individuals correctly, but not 
always to the list. This is apparently because my ISP has so many 
different network address available that the probability of using the 
same one (or same subnet) is not very high. Is there anything that can 
be done about this?

Best Regards,
Bill Campbell
> Regards,
>
> Graeme
>
>

[U-Boot] [RFC] Review of U-Boot timer API

[J. William Campbell]

On 5/22/2011 8:26 PM, Reinhard Meyer wrote:
> Dear J. William Campbell,
>> On 5/22/2011 1:15 AM, Reinhard Meyer wrote:
>>> Dear J. William Campbell,
>>>
>>> please demonstrate for me (and others), by a practical example,
>>> how _any_ arithmetic (even less with just shifts and multiplies)
>>> can convert a free running 3.576 MHz (wild example) free running
>>> 32 bit counter (maybe software extended to 64 bits) into a ms
>>> value that will properly wrap from 2**32-1 to 0 ?
>> Hi All
>> I accept the challenge! I will present two ways to do this, one using 
>> a 32 bit by 16 bit divide, and one using only multiplies.
>> This first method is "exact", in that there is no difference in 
>> performance from a "hardware" counter ticking at the 1 ms rate. This 
>> is accomplished by operating the 1 ms counter based on the delta time 
>> in the hardware time base. It is necessary to call this routine often 
>> enough that the hardware counter does not wrap more than once between 
>> calls. This is not really a problem, as this time is 1201 seconds or 
>> so. If the routine is not called for a long time, or at the first 
>> call, it will return a timer_in_ms value that will work for all 
>> subsequent calls that are within a hardware rollover interval. Since 
>> the timer in ms is a 32 bit number anyway. The same rollover issue 
>> will exist if you "software extend" the timer to 64 bits. You must 
>> assume 1 rollover. If it is more than 1, the timer is wrong.
>>
>>
>> The variables in the gd are
>> u32 prev_timer;
>> u32 timer_in_ms;
>> u16 timer_remainder;
>>
>> /* gd->timer remainder must be initialized to 0 (actually, an number 
>> less than 3576, but 0 is nice). Other two variables don't matter but 
>> can be initialized if desired */
>>
>> u32 get_raw_ms()
>> {
>> u32 delta;
>> u32 t_save;
>>
>> read(t_save); /* atomic read of the hardware 32 bit timer running at 
>> 3.576 MHz */
>> delta_t = (t_save - gd->prev_timer) ;
>>
>> gd->prev_timer = t_save;
>> /*
>> Hopefully, the following two lines only results in one hardware 
>> divide when optimized. If your CPU has no hardware divide, or if it 
>> slow, see second method .
>> */
>> gd->timer_in_ms += delta_t / 3576; /* convert elapsed time to ms */
>> gd->timer_remainder += delta_t % 3576; /* add in remaining part not 
>> included above */
>> if (gd->timer_remainder >= 3576) /* a carry has been detected */
>> {
>> ++gd->timer_in_ms;
>> gd->timer_remainder -= 3576; /* fix remainder for the carry above */
>> }
>>
>> return(gd->timer_in_ms)
>> }
>
> Thank you! Basically this is similar to a Bresenham Algorithm.
Hi All,
       Yes, I think you are correct. I didn't know it by that name, but 
i think you are correct. It is a bit different use of the idea, but it 
is very similar.
>
>>
>> This approach works well when the number of ticks per ms is an exact 
>> number representable as a small integer, as it is in this case. It is 
>> exact with a clock rate of 600 MHz, but is not exact for a clock rate 
>> of 666 MHz. 666667 is not an exact estimate of ticks per ms, It is 
>> off by 0.00005 % That should be acceptable for use as a timeout 
>> delay. The accumulated error in a 10 second delay should be less than 
>> 0.5 ms.
>
> I would think the non exact cases result in such a small error that 
> can be
> tolerated. We are using the ms tick for timeouts, not for providing a 
> clock
> or exact delays. We should just round up when calculating the divider.
Yes, we should round off the divider value, so 666.6666666666 MHz rounds 
to 666667 ticks/Ms while 333.3333333333 MHz rounds to 333333 ticks/Ms.
>
> Hence the hick-ups that result when this is not called frequent enough to
> prevent a multiple rollover of the raw value between calls do not matter
> either (they should be just documented).
Good, I am glad we agree on this also.
>
>>
>> There is a way that the divide above can be approximated by 
>> multiplying by an appropriate fraction, taking the resulting delta t 
>> in ms, multiplying it by 3576, and subtracting the product from the 
>> original delta to get the remainder. This is the way to go if your 
>> CPU divides slowly or not at all. This approach is presented below.
>>
> [...]
>
> Optimizations would be up to the implementer of such a hardware and work
> only if the divider is a compile time constant. Often the divider will be
> run time determined (AT91 for example).
Correct. I will provide a "generic" version that computes the constants 
at run time.  If the clock rate is a constant, these routines can be 
overridden at compile/link time. This generic version should be 
available on Monday for further review.

Best Regards,
Bill Campbell
>
> Reinhard
>
>

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

On Mon, May 23, 2011 at 3:02 PM, J. William Campbell
<jwilliamcampbell@comcast.net> wrote:
> On 5/22/2011 6:42 PM, Graeme Russ wrote:
>>
>> OK, so in summary, we can (in theory) have:
>> ?- A timer API in /lib/ with a single u32 get_timer(u32 base) function
>> ?- A HAL with two functions
>> ? ?- u32 get_raw_ticks()
>> ? ?- u32 get_raw_tick_rate() which returns the tick rate in kHz (which
>> ? ? ?max's out at just after 4GHz)
>> ?- A helper function in /lib/ u32 get_raw_ms() which uses get_raw_ticks()
>> ? ?and get_tick_rate() to correctly maintain the ms counter used by
>> ? ?get_timer() - This function can be weak (so next point)
>> ?- If the hardware supports a native 32-bit ms counter, get_raw_ms()
>> ? ?can be overridden to simply return the hardware counter. In this case,
>> ? ?get_raw_ticks() would return 1
>> ?- Calling of get_raw_ticks() regularly in the main loop (how ofter will
>> ? ?depend on the raw tick rate, but I image it will never be necessary
>> ? ?to call more often than once every few minutes)
>> ?- If the hardware implements a native 32-bit 1ms counter, no call in
>> ? ?the main loop is required
>> ?- An optional HAL function u32 get_raw_us() which can be used for
>> ? ?performance profiling (must wrap correctly)
>
> Hi All,
> ? ? ?Graeme, I think you have stated exactly what is the "best" approach to
> this problem. ?I will provide a version of "get_raw_ms" that is ?initialized
> using get_raw_tick_rate that will work for all "reasonable" values of
> raw_tick_rate. This will be the "generic" solution. Both the initialization
> function and the get_raw_ms function can be overridden if there is reason to
> do so, like "exact" clock rates. I will do the same with get_raw_us. This
> will be posted sometime on Monday for people to review, and to make sure I
> didn't get too far off base. Thank you to both Graeme and Reinhard for
> looking at/working on this.. Hopefully, this solution will put this timing
> issue to rest for all future ports as well as the presently existing ones.

Great - I am in the middle of cleaning up the current usages of the timer
API, reducing it all down to get_timer() - I will then 'libify'
get_timer() and setup the hooks into the HAL get_raw_ticks() and
get_raw_tick_rate() API

I think there will need to be a lot of cleanup, especially in arch/arm to
get this to all fit

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Albert ARIBAUD]

Hi all,

Sorry, could not follow the discussion although I find it very 
interesting, so I will handle the task of coming in late and asking the 
silly questions.

Le 23/05/2011 07:25, Graeme Russ a ?crit :

> On Mon, May 23, 2011 at 3:02 PM, J. William Campbell
> <jwilliamcampbell@comcast.net>  wrote:
>> On 5/22/2011 6:42 PM, Graeme Russ wrote:
>>>
>>> OK, so in summary, we can (in theory) have:
>>>   - A timer API in /lib/ with a single u32 get_timer(u32 base) function
>>>   - A HAL with two functions
>>>     - u32 get_raw_ticks()
>>>     - u32 get_raw_tick_rate() which returns the tick rate in kHz (which
>>>       max's out at just after 4GHz)
>>>   - A helper function in /lib/ u32 get_raw_ms() which uses get_raw_ticks()
>>>     and get_tick_rate() to correctly maintain the ms counter used by
>>>     get_timer() - This function can be weak (so next point)
>>>   - If the hardware supports a native 32-bit ms counter, get_raw_ms()
>>>     can be overridden to simply return the hardware counter. In this case,
>>>     get_raw_ticks() would return 1

Are you sure you did not mean 'get_raw_ticks_rate' here? Besides, I'd 
like the name to specify the units used: 'get_raw_ticks_rate_in_khz' (or 
conversively 'get_raw_ticks_per_ms', depending on which is simpler to 
implement and use).

>>>   - Calling of get_raw_ticks() regularly in the main loop (how ofter will
>>>     depend on the raw tick rate, but I image it will never be necessary
>>>     to call more often than once every few minutes)

That's to keep track of get_raw_ticks() rollovers, right? And then the 
get_timer function (which, again, I would prefer to have '... in ms' 
expressed in its name) would call get_raw_ticks() in confidence that at 
most one rollover may have occurred since the last time the helper 
function was called, so a simple difference of the current vs last tick 
value will always be correct.

>>>   - If the hardware implements a native 32-bit 1ms counter, no call in
>>>     the main loop is required
>>>   - An optional HAL function u32 get_raw_us() which can be used for
>>>     performance profiling (must wrap correctly)
>>
>> Hi All,
>>       Graeme, I think you have stated exactly what is the "best" approach to
>> this problem.  I will provide a version of "get_raw_ms" that is  initialized
>> using get_raw_tick_rate that will work for all "reasonable" values of
>> raw_tick_rate. This will be the "generic" solution. Both the initialization
>> function and the get_raw_ms function can be overridden if there is reason to
>> do so, like "exact" clock rates. I will do the same with get_raw_us. This
>> will be posted sometime on Monday for people to review, and to make sure I
>> didn't get too far off base. Thank you to both Graeme and Reinhard for
>> looking at/working on this.. Hopefully, this solution will put this timing
>> issue to rest for all future ports as well as the presently existing ones.

In Greame's description, I did not see a get_raw_ms, only a get_raw_us. 
Was this last one a typo or is that a third HAL function?

> Great - I am in the middle of cleaning up the current usages of the timer
> API, reducing it all down to get_timer() - I will then 'libify'
> get_timer() and setup the hooks into the HAL get_raw_ticks() and
> get_raw_tick_rate() API
>
> I think there will need to be a lot of cleanup, especially in arch/arm to
> get this to all fit
>
> Regards,
>
> Graeme

Amicalement,
-- 
Albert.

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

On 23/05/11 16:29, Albert ARIBAUD wrote:
> Hi all,
> 
> Sorry, could not follow the discussion although I find it very 
> interesting, so I will handle the task of coming in late and asking the 
> silly questions.
> 
> Le 23/05/2011 07:25, Graeme Russ a ?crit :
> 
>> On Mon, May 23, 2011 at 3:02 PM, J. William Campbell
>> <jwilliamcampbell@comcast.net>  wrote:
>>> On 5/22/2011 6:42 PM, Graeme Russ wrote:
>>>>
>>>> OK, so in summary, we can (in theory) have:
>>>>   - A timer API in /lib/ with a single u32 get_timer(u32 base) function
>>>>   - A HAL with two functions
>>>>     - u32 get_raw_ticks()
>>>>     - u32 get_raw_tick_rate() which returns the tick rate in kHz (which
>>>>       max's out at just after 4GHz)
>>>>   - A helper function in /lib/ u32 get_raw_ms() which uses get_raw_ticks()
>>>>     and get_tick_rate() to correctly maintain the ms counter used by
>>>>     get_timer() - This function can be weak (so next point)
>>>>   - If the hardware supports a native 32-bit ms counter, get_raw_ms()
>>>>     can be overridden to simply return the hardware counter. In this case,
>>>>     get_raw_ticks() would return 1
> 
> Are you sure you did not mean 'get_raw_ticks_rate' here? Besides, I'd 

Yes, I did mean get_raw_ticks_rate()

> like the name to specify the units used: 'get_raw_ticks_rate_in_khz' (or 
> conversively 'get_raw_ticks_per_ms', depending on which is simpler to 
> implement and use).
> 

indeed, get_raw_ticks_per_ms() is a better name

>>>>   - Calling of get_raw_ticks() regularly in the main loop (how ofter will
>>>>     depend on the raw tick rate, but I image it will never be necessary
>>>>     to call more often than once every few minutes)
> 
> That's to keep track of get_raw_ticks() rollovers, right? And then the 

Yes

> get_timer function (which, again, I would prefer to have '... in ms' 
> expressed in its name) would call get_raw_ticks() in confidence that at 
> most one rollover may have occurred since the last time the helper 
> function was called, so a simple difference of the current vs last tick 
> value will always be correct.

Correct - And a change to get_ms_timer() could be in order if generally
supported, but this would be a big patch across a lot of code for a simple
rename

>>>>   - If the hardware implements a native 32-bit 1ms counter, no call in
>>>>     the main loop is required
>>>>   - An optional HAL function u32 get_raw_us() which can be used for
>>>>     performance profiling (must wrap correctly)
>>>
>>> Hi All,
>>>       Graeme, I think you have stated exactly what is the "best" approach to
>>> this problem.  I will provide a version of "get_raw_ms" that is  initialized
>>> using get_raw_tick_rate that will work for all "reasonable" values of
>>> raw_tick_rate. This will be the "generic" solution. Both the initialization
>>> function and the get_raw_ms function can be overridden if there is reason to
>>> do so, like "exact" clock rates. I will do the same with get_raw_us. This
>>> will be posted sometime on Monday for people to review, and to make sure I
>>> didn't get too far off base. Thank you to both Graeme and Reinhard for
>>> looking at/working on this.. Hopefully, this solution will put this timing
>>> issue to rest for all future ports as well as the presently existing ones.
> 
> In Greame's description, I did not see a get_raw_ms, only a get_raw_us. 
> Was this last one a typo or is that a third HAL function?

get_raw_ms() is used by get_timer(). get_raw_us() is an optional function
which can be used for performance profiling


>> Great - I am in the middle of cleaning up the current usages of the timer
>> API, reducing it all down to get_timer() - I will then 'libify'
>> get_timer() and setup the hooks into the HAL get_raw_ticks() and
>> get_raw_tick_rate() API
>>
>> I think there will need to be a lot of cleanup, especially in arch/arm to
>> get this to all fit
>>
>> Regards,
>>
>> Graeme
> 
> Amicalement,

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear Graeme Russ,

sorry for jumping in late here (was busy with the ELDK 5.0 release).

In message <BANLkTimqGpnsbRSF1hwmjJaaMxV3M_K8gg@mail.gmail.com> you wrote:
> OK, so in summary, we can (in theory) have:
>  - A timer API in /lib/ with a single u32 get_timer(u32 base) function
>  - A HAL with two functions
>    - u32 get_raw_ticks()

What des this provide?

>    - u32 get_raw_tick_rate() which returns the tick rate in kHz (which
>      max's out at just after 4GHz)

Can we please omit the _raw part in these names?

>  - A helper function in /lib/ u32 get_raw_ms() which uses get_raw_ticks()
>    and get_tick_rate() to correctly maintain the ms counter used by
>    get_timer() - This function can be weak (so next point)

Ditto.  What would that do?  If it gets milliseconds as the name
suggest, that's already the function needed for get_timer()?

>  - If the hardware supports a native 32-bit ms counter, get_raw_ms()
>    can be overridden to simply return the hardware counter. In this case,
>    get_raw_ticks() would return 1

I don't think this is possible on may systems, so I doubt if this is
auseful approach.

>  - Calling of get_raw_ticks() regularly in the main loop (how ofter will
>    depend on the raw tick rate, but I image it will never be necessary
>    to call more often than once every few minutes)

NAK. This concept is fundamentally broken.  I will not accept it.

>  - If the hardware implements a native 32-bit 1ms counter, no call in
>    the main loop is required

We should make no such requirements.

>  - An optional HAL function u32 get_raw_us() which can be used for
>    performance profiling (must wrap correctly)

Sorry for rewinding the thread.

Can we not start simple, say by a plain free-runnign 64 bit counter,
be it implemented in hardwar eor in software?  On PowerPC, we have
this immediately in form of the time base register (or more precisely
in form of the two 32 bit registers tbu and tbl representing time base
upper and time base lower).

Modelling a similar interface using _any_ kind of timer service should
be trivial.  And from there, we can just use the existing code again.

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
I often quote myself; it adds spice to my conversation.  - G. B. Shaw

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear "J. William Campbell",

In message <4DD8B3ED.4000408@comcast.net> you wrote:
>
...
>        This statement, as written, is false. While it is true that the 
> power PC (and others) have a 64 bit counter that runs at a many 
> megahertz rate, it is NOT true that all 64 bits must be used to obtain a 
> millisecond timer. A millisecond timer can be produced by using the 33 
> bits of the 64 bit counter whose lsb is <= 1 ms and > 0.5 ms. This value 
> can be extracted by a right shift and masking of the 64 bit counter. Two 
> 32 bit multiplies and adds will produce a correct monotonic 1 ms timer. 
> This is certainly the best way to go on systems that do not have a 
> hardware 64 bit by 32 bit divide. See the thread "ARM timing code 
> refactoring" for an example of how this can be done. This timer will 
> NEVER result in a wrong value, although it does loose  1 ms every 1193 
> hours.

I think we should NOT bother about low level optimization details
wether or not we have a 64 bit divide instruction of if we need to
emulate it in software.  Let's keep such early optimizations out of
this phase and deal with the specific implementation later, once we
agreed on a design.

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
"...one of the main causes of the fall of the Roman Empire was  that,
lacking  zero,  they had no way to indicate successful termination of
their C programs."                                     - Robert Firth

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

On 23/05/11 22:09, Wolfgang Denk wrote:
> Dear Graeme Russ,
> 
> sorry for jumping in late here (was busy with the ELDK 5.0 release).

I thought you were a bit quite on such a 'touchy' subject ;)

> 
> In message <BANLkTimqGpnsbRSF1hwmjJaaMxV3M_K8gg@mail.gmail.com> you wrote:
>> OK, so in summary, we can (in theory) have:
>>  - A timer API in /lib/ with a single u32 get_timer(u32 base) function
>>  - A HAL with two functions
>>    - u32 get_raw_ticks()
> 
> What des this provide?
> 
>>    - u32 get_raw_tick_rate() which returns the tick rate in kHz (which
>>      max's out at just after 4GHz)
> 
> Can we please omit the _raw part in these names?
> 
>>  - A helper function in /lib/ u32 get_raw_ms() which uses get_raw_ticks()
>>    and get_tick_rate() to correctly maintain the ms counter used by
>>    get_timer() - This function can be weak (so next point)
> 
> Ditto.  What would that do?  If it gets milliseconds as the name
> suggest, that's already the function needed for get_timer()?

OK, there appears to be a consensus that not all hardware actually supports
a free-running timer with 1ms resolution. To overcome this, the idea is to
create a common library function which maintains the free running counter.
The library function acts as a pre-scaler using a 'raw tick counter' and a
'raw tick rate' supplied by the low level architecture. We define this weak
so that if the architecture can provide a free running 1ms counter, there
is no (code size) penalty

This approach eliminates all the existing per-arch code which (attempts) to
manage the time base behind get time. So we simplify each arch down to it's
bare essentials - Provide a counter which increments at a natural fixed
rate and what the rate is - Let common library code deal with the rest.

>>  - If the hardware supports a native 32-bit ms counter, get_raw_ms()
>>    can be overridden to simply return the hardware counter. In this case,
>>    get_raw_ticks() would return 1
> 
> I don't think this is possible on may systems, so I doubt if this is
> auseful approach.

x86 does as it implements a 1ms interrupt (I know other systems do as well)

>>  - Calling of get_raw_ticks() regularly in the main loop (how ofter will
>>    depend on the raw tick rate, but I image it will never be necessary
>>    to call more often than once every few minutes)
> 
> NAK. This concept is fundamentally broken.  I will not accept it.

Some existing timers are fundamentally broken - The glitch at the
0xffffffff to 0x00000000 rollover or rollover early - The method discussed
in this thread eliminates all such glitches. Provided pre-scaler in /lib/
(triggered by get_timer() usually) is called often enough (71 minutes for a
32-bit 1MHz counter) then there is no need. Even then, it is only important
over the time period you are measuring (i.e. two independent 5s delays 2
hours apart will not be a problem)

Kicking the pre-scaler (like kicking the watchdog) eliminates the problem
entirely.

> 
>>  - If the hardware implements a native 32-bit 1ms counter, no call in
>>    the main loop is required
> 
> We should make no such requirements.

No such requirement of what?

>>  - An optional HAL function u32 get_raw_us() which can be used for
>>    performance profiling (must wrap correctly)
> 
> Sorry for rewinding the thread.
> 
> Can we not start simple, say by a plain free-runnign 64 bit counter,
> be it implemented in hardwar eor in software?  On PowerPC, we have

That's exactly what we are suggesting - Let the hardware be free to
implement the counter at whatever frequency suits it. 64-bit is not needed
in reality

> this immediately in form of the time base register (or more precisely
> in form of the two 32 bit registers tbu and tbl representing time base
> upper and time base lower).
> 
> Modelling a similar interface using _any_ kind of timer service should
> be trivial.  And from there, we can just use the existing code again.

Yes, have the library function manage the actual '1ms' aspect

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear Graeme Russ,

In message <4DDA5334.4060308@gmail.com> you wrote:
>
> >>  - A helper function in /lib/ u32 get_raw_ms() which uses get_raw_ticks()
> >>    and get_tick_rate() to correctly maintain the ms counter used by
> >>    get_timer() - This function can be weak (so next point)
> > 
> > Ditto.  What would that do?  If it gets milliseconds as the name
> > suggest, that's already the function needed for get_timer()?
> 
> OK, there appears to be a consensus that not all hardware actually supports
> a free-running timer with 1ms resolution. To overcome this, the idea is to

Indeed.  I guess most of them do not.

> create a common library function which maintains the free running counter.
> The library function acts as a pre-scaler using a 'raw tick counter' and a
> 'raw tick rate' supplied by the low level architecture. We define this weak

What are "raw" ticks?  And what are "cooked" ticks, then?

> so that if the architecture can provide a free running 1ms counter, there
> is no (code size) penalty

Why do we need a free running 1ms counter at all?  Any free running
counter of at least millisecoind resolution should be good enough.

> This approach eliminates all the existing per-arch code which (attempts) to
> manage the time base behind get time. So we simplify each arch down to it's
> bare essentials - Provide a counter which increments at a natural fixed
> rate and what the rate is - Let common library code deal with the rest.

Did you have a look at the PowerPC implementation?  I'd like to see
this used as reference.

> >>  - Calling of get_raw_ticks() regularly in the main loop (how ofter will
> >>    depend on the raw tick rate, but I image it will never be necessary
> >>    to call more often than once every few minutes)
> > 
> > NAK. This concept is fundamentally broken.  I will not accept it.
> 
> Some existing timers are fundamentally broken - The glitch at the
> 0xffffffff to 0x00000000 rollover or rollover early - The method discussed
> in this thread eliminates all such glitches. Provided pre-scaler in /lib/
> (triggered by get_timer() usually) is called often enough (71 minutes for a
> 32-bit 1MHz counter) then there is no need. Even then, it is only important

We already have this nightmare of code for triggering the watchdog on
systems that use it.

Assuming there are places in the main loop that get executed often
enough is a broken concept, and I will not accept any such code.

> over the time period you are measuring (i.e. two independent 5s delays 2
> hours apart will not be a problem)

What is the practical purpose of get_timer()?  What is the longest
interval we have to cover?  And what is the problem with a rollover?

> >>  - If the hardware implements a native 32-bit 1ms counter, no call in
> >>    the main loop is required
> > 
> > We should make no such requirements.
> 
> No such requirement of what?

Of making any calls in the main loop.

> > Can we not start simple, say by a plain free-runnign 64 bit counter,
> > be it implemented in hardwar eor in software?  On PowerPC, we have
> 
> That's exactly what we are suggesting - Let the hardware be free to
> implement the counter at whatever frequency suits it. 64-bit is not needed
> in reality

It may not be needed on some systems, but may be needed on others
where 32 bit is too short.  Let's use the common base that is know to
work on all systems, even if it's not strictly needed on all of them.

> > this immediately in form of the time base register (or more precisely
> > in form of the two 32 bit registers tbu and tbl representing time base
> > upper and time base lower).
> > 
> > Modelling a similar interface using _any_ kind of timer service should
> > be trivial.  And from there, we can just use the existing code again.
> 
> Yes, have the library function manage the actual '1ms' aspect

This is what PPC is doing. And I understand that Reinhard did the same
in software for AT91.

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
Alles Gescheite ist schon gedacht worden, man mu? nur versuchen,
es noch einmal zu denken.          -- Goethe, Maximen und Reflexionen

[U-Boot] [RFC] Review of U-Boot timer API

[J. William Campbell]

On 5/22/2011 11:29 PM, Albert ARIBAUD wrote:
> Hi all,
>
> Sorry, could not follow the discussion although I find it very
> interesting, so I will handle the task of coming in late and asking the
> silly questions.
I am glad you are looking at our discussion. I am sure we are going to 
need all the help/oversight/questions that we can get, as this is a 
change that will affect all architectures.
> Le 23/05/2011 07:25, Graeme Russ a ?crit :
>
>> On Mon, May 23, 2011 at 3:02 PM, J. William Campbell
>> <jwilliamcampbell@comcast.net>   wrote:
>>> On 5/22/2011 6:42 PM, Graeme Russ wrote:
>>>> OK, so in summary, we can (in theory) have:
>>>>    - A timer API in /lib/ with a single u32 get_timer(u32 base) function
>>>>    - A HAL with two functions
>>>>      - u32 get_raw_ticks()
>>>>      - u32 get_raw_tick_rate() which returns the tick rate in kHz (which
>>>>        max's out at just after 4GHz)
>>>>    - A helper function in /lib/ u32 get_raw_ms() which uses get_raw_ticks()
>>>>      and get_tick_rate() to correctly maintain the ms counter used by
>>>>      get_timer() - This function can be weak (so next point)
>>>>    - If the hardware supports a native 32-bit ms counter, get_raw_ms()
>>>>      can be overridden to simply return the hardware counter. In this case,
>>>>      get_raw_ticks() would return 1
> Are you sure you did not mean 'get_raw_ticks_rate' here? Besides, I'd
> like the name to specify the units used: 'get_raw_ticks_rate_in_khz' (or
> conversively 'get_raw_ticks_per_ms', depending on which is simpler to
> implement and use).
I think you are correct, it was the rate function desired here. I think 
the best way to go is use a "get_raw_tick_rate_in_mhz" function, because 
it is probably the easiest one to implement, and in many cases something 
like it already exists.
>>>>    - Calling of get_raw_ticks() regularly in the main loop (how ofter will
>>>>      depend on the raw tick rate, but I image it will never be necessary
>>>>      to call more often than once every few minutes)
> That's to keep track of get_raw_ticks() rollovers, right? And then the
> get_timer function (which, again, I would prefer to have '... in ms'
> expressed in its name) would call get_raw_ticks() in confidence that at
> most one rollover may have occurred since the last time the helper
> function was called, so a simple difference of the current vs last tick
> value will always be correct.
Exactly so. Note that this same function probably needs to be called in 
udelay for the same reason. More precisely, the get_timer function will 
call get_raw_ms, which will call get_raw_ticks. I think it may be better 
to move get_timer "down" a level in the hierarchy,
so we don't need a get_raw_ms. get_timer would then be part of the HAL. 
One would use a get_timer(0) in order to do what get_raw_ms alone would 
have done. If the user had a good reason, he would then override 
get_timer with his own version. What do you think Graeme? It reduces the 
nesting depth by one level. As for the name change to get_timer_in_ms, I 
would support it. Naturally, such a change would be up to Mr. Denk. 
Since by definition that is what the function does, it seems to be a 
good change from my point of view.
>>>>    - If the hardware implements a native 32-bit 1ms counter, no call in
>>>>      the main loop is required
>>>>    - An optional HAL function u32 get_raw_us() which can be used for
>>>>      performance profiling (must wrap correctly)
>>> Hi All,
>>>        Graeme, I think you have stated exactly what is the "best" approach to
>>> this problem.  I will provide a version of "get_raw_ms" that is  initialized
>>> using get_raw_tick_rate that will work for all "reasonable" values of
>>> raw_tick_rate. This will be the "generic" solution. Both the initialization
>>> function and the get_raw_ms function can be overridden if there is reason to
>>> do so, like "exact" clock rates. I will do the same with get_raw_us. This
>>> will be posted sometime on Monday for people to review, and to make sure I
>>> didn't get too far off base. Thank you to both Graeme and Reinhard for
>>> looking at/working on this.. Hopefully, this solution will put this timing
>>> issue to rest for all future ports as well as the presently existing ones.
> In Greame's description, I did not see a get_raw_ms, only a get_raw_us.
> Was this last one a typo or is that a third HAL function?
get_raw_ms was referenced as a library function a few lines above.  
Right now, I think the functionality we require from the HAL is

   1. get_raw_tick_rate_in_mhz
   2. get_raw_ms
   3. get_raw_ticks
   4. (optional)get_raw_us

There is also  APIs for these functions called get_timer.

I think we need to add a call to another function called 
"initialize_timer_system" or similar that will initialize the data 
structures in gd by calling get_raw_tick_rate_in_mhz. Additionally, I 
think we need to provide a udelay function, simply because it can 
interact with calling get_raw_ms often enough. We are somewhat caught 
between two fires here, in that on the one hand we want to provide a 
very "generic" approach to the timing system that will work on "any" CPU 
while on the other hand we want to allow easy 
customization/simplification of the timer system where appropriate. 
There is more than one way to approach this. Another approach is to 
define the HAL as:

   1. get_timer (_in_ms if approved)
   2. udelay
   3. (optional)get_raw_us
   4. get_raw_tick_rate_in_mhz
   5.   initialize_timer_system

We would provide a "reference" implementation of get_timer and udelay 
with comments regarding the implementation of the get_raw_ticks 
functionality (static inline comes to mind). This would provide a 
functional but possibly not optimal timer system. Later, once everything 
is working, the user could "hack up" the get_timer routines and the 
initialize_timer_system routines to remove any functionality that was 
not relevant to his system, such as using a fixed clock rate etc. The 
benefit of this approach is that in simple systems where space is real 
tight one can still utilize a minimal, optimal timer system without the 
extra code required to make it generic. There are no "extra levels". The 
disadvantage of this approach is that it allows the user to wander off 
into an incompatible non-standard implementation. Thoughts on this 
subject are welcome. The design we have will work, but it will require 
somewhat more text than the "present" system. How much more, and does it 
matter, I don't know.
>> Great - I am in the middle of cleaning up the current usages of the timer
>> API, reducing it all down to get_timer() - I will then 'libify'
>> get_timer() and setup the hooks into the HAL get_raw_ticks() and
>> get_raw_tick_rate() API
>>
>> I think there will need to be a lot of cleanup, especially in arch/arm to
>> get this to all fit
I share your concern. We may have to iterate a bit.

Best Regards,
Bill Campbell

>> Regards,
>>
>> Graeme
> Amicalement,

[U-Boot] [RFC] Review of U-Boot timer API

[J. William Campbell]

On 5/23/2011 6:19 AM, Wolfgang Denk wrote:
> Dear Graeme Russ,
>
> In message<4DDA5334.4060308@gmail.com>  you wrote:
>>>>   - A helper function in /lib/ u32 get_raw_ms() which uses get_raw_ticks()
>>>>     and get_tick_rate() to correctly maintain the ms counter used by
>>>>     get_timer() - This function can be weak (so next point)
>>> Ditto.  What would that do?  If it gets milliseconds as the name
>>> suggest, that's already the function needed for get_timer()?
>> OK, there appears to be a consensus that not all hardware actually supports
>> a free-running timer with 1ms resolution. To overcome this, the idea is to
> Indeed.  I guess most of them do not.
>
>> create a common library function which maintains the free running counter.
>> The library function acts as a pre-scaler using a 'raw tick counter' and a
>> 'raw tick rate' supplied by the low level architecture. We define this weak
> What are "raw" ticks?  And what are "cooked" ticks, then?
Hi all,
      FWIW,  "cooked" ticks would be 1 ms ticks, although we never 
really use them as such.
>> so that if the architecture can provide a free running 1ms counter, there
>> is no (code size) penalty
> Why do we need a free running 1ms counter at all?  Any free running
> counter of at least millisecoind resolution should be good enough.
Correct. Any free running counter whose resolution is better than one 
millisecond and which is "long enough" that it will not overflow between 
calls to get_timer is sufficient.
>> This approach eliminates all the existing per-arch code which (attempts) to
>> manage the time base behind get time. So we simplify each arch down to it's
>> bare essentials - Provide a counter which increments at a natural fixed
>> rate and what the rate is - Let common library code deal with the rest.
> Did you have a look at the PowerPC implementation?  I'd like to see
> this used as reference.
I have looked at it as a reference. However, there is one disadvantage 
in using the PPC code as a reference. It has a 64 bit timestamp. Many 
systems do not have a 64 bit timestamp, but rather a 32 bit timestamp.  
It is possible to extend the 32 bit timestamp to a 64 bit timestamp if 
get_timer is called often enough that there will only be a single 
rollover of the bottom 32 bits  between uses. However, if that condition 
is met, there is no need to extend the timer to 64 bits. Instead, just 
convert the elapsed time since the last call (which you know) to ms and 
be done with it.  As Wolfgang said above, any counter that has better 
than 1 ms resolution  is adequate to the task. The additional 
requirement that we have stated is that the counter be long enough that 
it does not overflow between calls to get_timer. If the counter is 64 
bits long, it pretty much for sure meets this requirement  (although 
bits below 0.5 ms resolution really don't help any). If the timer is 32 
bits long, it will meet any requirements using get_timer to time out 
hardware intervals. My original implementation used a 32 bit divide and 
does exactly that. This is the shortest and simplest approach, and we 
can get that working in all cases quite easily I think. We can avoid the 
"no divide" optimization until everybody is satisfied with what we have.
>>>>   - Calling of get_raw_ticks() regularly in the main loop (how ofter will
>>>>     depend on the raw tick rate, but I image it will never be necessary
>>>>     to call more often than once every few minutes)
>>> NAK. This concept is fundamentally broken.  I will not accept it.
>> Some existing timers are fundamentally broken - The glitch at the
>> 0xffffffff to 0x00000000 rollover or rollover early - The method discussed
>> in this thread eliminates all such glitches. Provided pre-scaler in /lib/
>> (triggered by get_timer() usually) is called often enough (71 minutes for a
>> 32-bit 1MHz counter) then there is no need. Even then, it is only important
> We already have this nightmare of code for triggering the watchdog on
> systems that use it.
>
> Assuming there are places in the main loop that get executed often
> enough is a broken concept, and I will not accept any such code.
That is fine with me. The reason this was being done was to attempt to 
emulate, as much as possible, the power PC, where the 64 bit timestamp 
counter allows calls to get_timer separated by many minutes and several 
console commands to work properly. These get timer commands will NOT 
work properly on systems that have a 32 bit counter that overflows every 
200 seconds or so. The call in the idle loop was an attempt to make the 
32 bit systems work more like the 64 bit systems. One may then either

   1.   Define calls to get_timer to measure an elapsed interval
      separated by any returns to the command processor as broken.
   2. Require the use of interrupts to extend the 32 bit timestamp.
      (This may not be possible on all systems as the timer used for
      performance monitoring does not interrupt, etc.)
   3. Allow the call in the idle loop under the assumption that we are
      talking about timing in the minutes range, not a few seconds.

If you go with number 1, all problems are solved.
>> over the time period you are measuring (i.e. two independent 5s delays 2
>> hours apart will not be a problem)
> What is the practical purpose of get_timer()?  What is the longest
> interval we have to cover?  And what is the problem with a rollover?
If get_timer is used to timeout failing hardware operations, the CPU is 
usually in a reasonably tight loop and get_timer will certainly be 
called at least once a minute. In this case, all will be well. If 
get_timer is used to obtain a timestamp during one u-boot command to be 
compared with a timestamp taken by a later command (say inside a 
script), it will not function properly if the total delay (or the delay 
in any one command) exceeds the hardware timer rollover interval. The 
elapsed time returned by the second call to get_timer will be wring 
modulo the actual number of rollovers that occurred. With an added call 
to get_timer inside the command loop, the condition becomes the delay in 
any one command, not the total delay. This is still broken, just not as 
bad. If 1 was selected above, no problem.
>>>>   - If the hardware implements a native 32-bit 1ms counter, no call in
>>>>     the main loop is required
>>> We should make no such requirements.
>> No such requirement of what?
> Of making any calls in the main loop.
>
>>> Can we not start simple, say by a plain free-runnign 64 bit counter,
>>> be it implemented in hardwar eor in software?  On PowerPC, we have
>> That's exactly what we are suggesting - Let the hardware be free to
>> implement the counter at whatever frequency suits it. 64-bit is not needed
>> in reality
> It may not be needed on some systems, but may be needed on others
> where 32 bit is too short.  Let's use the common base that is know to
> work on all systems, even if it's not strictly needed on all of them.
When you say a "common base", consider that all systems do not have 64 
bit timers, and extending the 32 bit timer that they do have to 64 bits 
in software is not possible if the number of rollovers between calls to 
the timer update routine is unknown. OTOH, if the number of rollovers is 
assumed to be at most 1, there is no benefit to extending the counter to 
64 bits, as the elapsed time is already known exactly and can be 
converted to ms exactly. We would extending the timer to 64 bits 
(incrementing the top 32 bits)to then do another subtraction to get the 
number we already have. If 32 bits is too short, and that is all you 
have, you are doomed anyway.

Best Regards,
Bill Campbell

>>> this immediately in form of the time base register (or more precisely
>>> in form of the two 32 bit registers tbu and tbl representing time base
>>> upper and time base lower).
>>>
>>> Modelling a similar interface using _any_ kind of timer service should
>>> be trivial.  And from there, we can just use the existing code again.
>> Yes, have the library function manage the actual '1ms' aspect
> This is what PPC is doing. And I understand that Reinhard did the same
> in software for AT91.
>
> Best regards,
>
> Wolfgang Denk
>

[U-Boot] [RFC] Review of U-Boot timer API

[Albert ARIBAUD]

Le 23/05/2011 19:30, J. William Campbell a ?crit :

>> Did you have a look at the PowerPC implementation? I'd like to see
>> this used as reference.

> I have looked at it as a reference. However, there is one disadvantage
> in using the PPC code as a reference. It has a 64 bit timestamp. Many
> systems do not have a 64 bit timestamp, but rather a 32 bit timestamp.
> It is possible to extend the 32 bit timestamp to a 64 bit timestamp if
> get_timer is called often enough that there will only be a single
> rollover of the bottom 32 bits between uses. However, if that condition
> is met, there is no need to extend the timer to 64 bits.

Well, there is one: making sure that drivers do not need to know if 
timestamps are 32 or 64 bits, or even what resolution they are.

Granted, you could define a timestamp_t type to be either u32 or u64 
depending on the actual timestamp width, so that the driver would not 
need to know the details; but extending u32 to u64, or subtracting two 
u64 values, are not so much of an effort, and that would mean a single API.

Amicalement,
-- 
Albert.

[U-Boot] [RFC] Review of U-Boot timer API

[J. William Campbell]

On 5/23/2011 6:19 AM, Wolfgang Denk wrote:
> Dear Graeme Russ,
<snip>
> This is what PPC is doing. And I understand that Reinhard did the same
> in software for AT91.
Hi All,
      My apologies for being a little (perhaps more than a little) 
dense. As they say, "after further review", I think the key aspect of 
the PPC timer system is that it uses the decrementer register to 
generate an interrupt at a 1 KHz rate. What I have been attempting here 
is to produce a timer system that does not use interrupts at all. This 
is a fundamental design question. Naturally, systems that can generate 
an interrupt at a 1 KHz rate (or at any (reasonable) higher rate for 
that matter) using the decrementer register can produce a 1 ms 
resolution software counter that updates "by magic". If my understanding 
of this PPC  code is incorrect, somebody please stop me before I make a 
further fool of myself!  Is it then a design requirement that the timer 
system use interrupts? Is that what is meant by using the PPC system as 
a model? If so, is it possible/reasonable on all the u-boots that are 
out there to generate and process timer interrupts at some (hopefully 
but not necessarily) programmable rate?

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

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear "J. William Campbell",

In message <4DDA99BC.80809@comcast.net> you wrote:
>
> >> The library function acts as a pre-scaler using a 'raw tick counter' and a
> >> 'raw tick rate' supplied by the low level architecture. We define this weak
> > What are "raw" ticks?  And what are "cooked" ticks, then?
> Hi all,
>       FWIW,  "cooked" ticks would be 1 ms ticks, although we never 
> really use them as such.

Sorry, but I disagree here.

A tick is defined as the smallest increment of system time as measured by a
computer system (see http://en.wikipedia.org/wiki/System_time):

	System time is measured by a system clock, which is typically
	implemented as a simple count of the number of ticks that have
	transpired since some arbitrary starting date, called the
	epoch.

There is no such thing as "raw ticks", a tick is inherently "raw" and
needs scaling.

> I have looked at it as a reference. However, there is one disadvantage 
> in using the PPC code as a reference. It has a 64 bit timestamp. Many 
> systems do not have a 64 bit timestamp, but rather a 32 bit timestamp.  
> It is possible to extend the 32 bit timestamp to a 64 bit timestamp if 
> get_timer is called often enough that there will only be a single 
> rollover of the bottom 32 bits  between uses. However, if that condition 

Or if there re other methods to detect and handle such a situation. On
many systems, the timer can generate an interrupt, so it would be
trivial enough to implement the handling of this in the interrupt
handler.

> is met, there is no need to extend the timer to 64 bits. Instead, just 
> convert the elapsed time since the last call (which you know) to ms and 
> be done with it.  As Wolfgang said above, any counter that has better 
> than 1 ms resolution  is adequate to the task. The additional 
> requirement that we have stated is that the counter be long enough that 
> it does not overflow between calls to get_timer. If the counter is 64 

The design should not make any assumptions about when or how often
get_timer() is being called.  It should just work :-)

> can get that working in all cases quite easily I think. We can avoid the 
> "no divide" optimization until everybody is satisfied with what we have.

We should first get a common API and a working implementation for all
architectures before we consider any optimizations.

<insert quote Donald Knuth about premature optimization>


> That is fine with me. The reason this was being done was to attempt to 
> emulate, as much as possible, the power PC, where the 64 bit timestamp 
> counter allows calls to get_timer separated by many minutes and several 
> console commands to work properly. These get timer commands will NOT 
> work properly on systems that have a 32 bit counter that overflows every 
> 200 seconds or so. The call in the idle loop was an attempt to make the 
> 32 bit systems work more like the 64 bit systems. One may then either

Can you show me any system where it is not possible to implement a 64
bit periodic timer with sufficient resolution?

Please note:  even on PowerPC where we have a 64 bit time base
register for free, the timer services are not implemented on this
base.  Instead, the interrupt handler for the decrementer will
increment the "timestamp" counter variable (see timer_interrupt() in
arch/powerpc/lib/interrupts.c).

I bet that functionally equivalent code is available on all systems.

Note2: this is a fundamental difference between get_timer() and
udelay().  udelay() has to be available very early, but needs to be
good for a few hundred microsecods or so.  get_timer() is a high level
interface that should not suffer from any restrictions.


>    1.   Define calls to get_timer to measure an elapsed interval
>       separated by any returns to the command processor as broken.

No.

>    2. Require the use of interrupts to extend the 32 bit timestamp.
>       (This may not be possible on all systems as the timer used for
>       performance monitoring does not interrupt, etc.)

This is definitely an option.  It works fine on some systems.
For those systems where it doesn't work, there are probably other
alternatives.


>    3. Allow the call in the idle loop under the assumption that we are
>       talking about timing in the minutes range, not a few seconds.

We must not make any such assumptions.  Assume someone is running a
standalone application (like some burn-in test software), which runs
for a couple of hours and then returns to U-Boot.

> If get_timer is used to timeout failing hardware operations, the CPU is 
> usually in a reasonably tight loop and get_timer will certainly be 
> called at least once a minute. In this case, all will be well. If 
> get_timer is used to obtain a timestamp during one u-boot command to be 
> compared with a timestamp taken by a later command (say inside a 
> script), it will not function properly if the total delay (or the delay 
> in any one command) exceeds the hardware timer rollover interval. The 

get_timer() is defined to return milliseconds.  It returns an unsigned
long, so it is good for UINT32_MAX/1000 = 4294967 seconds or more than
49 days before the timer rolls over.

It is up to the architecture specific implementation to warranty such
behaviour.  I don't think this is an overly complicated task.

> When you say a "common base", consider that all systems do not have 64 
> bit timers, and extending the 32 bit timer that they do have to 64 bits 
> in software is not possible if the number of rollovers between calls to 
> the timer update routine is unknown. OTOH, if the number of rollovers is 

Then let's handle all rollovers? Are there systems out there where we
cannot either have a free runnign 64 bit timer, or cascaded 32 bit
timers, or a timer that is capable of generating interrupts?  Does
U-Boot really support any such CPU?


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
"The pathology is to want control, not that you ever get it,  because
of course you never do."                            - Gregory Bateson

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear "J. William Campbell",

In message <4DDAA705.1040702@comcast.net> you wrote:
>
>       My apologies for being a little (perhaps more than a little) 
> dense. As they say, "after further review", I think the key aspect of 
> the PPC timer system is that it uses the decrementer register to 
> generate an interrupt at a 1 KHz rate. What I have been attempting here 
> is to produce a timer system that does not use interrupts at all. This 
> is a fundamental design question. Naturally, systems that can generate 

No, it is not.  It is an implementation detail which is irrelevant to
almost all users of U-Boot.

Or do you actucally care if your UART driver uses polling or
interrupts?

> an interrupt at a 1 KHz rate (or at any (reasonable) higher rate for 
> that matter) using the decrementer register can produce a 1 ms 
> resolution software counter that updates "by magic". If my understanding 
> of this PPC  code is incorrect, somebody please stop me before I make a 
> further fool of myself!  Is it then a design requirement that the timer 
> system use interrupts? Is that what is meant by using the PPC system as 

No, it is not a design requirement.  It is just one possible
implementation.  Any other method that achieves the same or similar
results is as good.  As noted before, on PowerPC we could have
probably avoided this and just base all timer services on the timebase
register.

[The reason for this dual implementation is historical.  When I wrote
this code, I did not know if we would ever need any fancy timer-
controlled callbacks or similar.  And I needed to implement interrupt
handling for a few other purposes (for example for use in standalone
applications; this was an explicit requirement at that time).  And the
timer was something that made a good and simple example.]

> a model? If so, is it possible/reasonable on all the u-boots that are 
> out there to generate and process timer interrupts at some (hopefully 
> but not necessarily) programmable rate?

I consider this an implementation detail.  On all architectures it
should be possible to use interrupts, so if the hardware supports a
timer that can generate interrupts it should be possible to use this.
But it is not a requirement that all implementations must work like
this.

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
In theory, there is no difference between  theory  and  practice.  In
practice, however, there is.

[U-Boot] [RFC] Review of U-Boot timer API

[J. William Campbell]

On 5/23/2011 12:33 PM, Wolfgang Denk wrote:
> Dear "J. William Campbell",
>
> In message<4DDAA705.1040702@comcast.net>  you wrote:
>>        My apologies for being a little (perhaps more than a little)
>> dense. As they say, "after further review", I think the key aspect of
>> the PPC timer system is that it uses the decrementer register to
>> generate an interrupt at a 1 KHz rate. What I have been attempting here
>> is to produce a timer system that does not use interrupts at all. This
>> is a fundamental design question. Naturally, systems that can generate
> No, it is not.  It is an implementation detail which is irrelevant to
> almost all users of U-Boot.
>
> Or do you actucally care if your UART driver uses polling or
> interrupts?
Hi All,
       I might care a lot if I expect typeahead to work, or if I am 
sending a command script via a terminal emulator and I don't want to 
loose characters while u-boot is off executing a command. One might 
(correctly) say that that is too much to expect of a boot loader, and 
define polling as "good enough", which I advocate, or not. YMMV.
>> an interrupt at a 1 KHz rate (or at any (reasonable) higher rate for
>> that matter) using the decrementer register can produce a 1 ms
>> resolution software counter that updates "by magic". If my understanding
>> of this PPC  code is incorrect, somebody please stop me before I make a
>> further fool of myself!  Is it then a design requirement that the timer
>> system use interrupts? Is that what is meant by using the PPC system as
> No, it is not a design requirement.  It is just one possible
> implementation.  Any other method that achieves the same or similar
> results is as good.  As noted before, on PowerPC we could have
> probably avoided this and just base all timer services on the timebase
> register.
>
> [The reason for this dual implementation is historical.  When I wrote
> this code, I did not know if we would ever need any fancy timer-
> controlled callbacks or similar.  And I needed to implement interrupt
> handling for a few other purposes (for example for use in standalone
> applications; this was an explicit requirement at that time).  And the
> timer was something that made a good and simple example.]
>
>> a model? If so, is it possible/reasonable on all the u-boots that are
>> out there to generate and process timer interrupts at some (hopefully
>> but not necessarily) programmable rate?
> I consider this an implementation detail.  On all architectures it
> should be possible to use interrupts, so if the hardware supports a
> timer that can generate interrupts it should be possible to use this.
> But it is not a requirement that all implementations must work like
       Ok, this is very nice to understand. I will attempt to summarize 
what I think this email and the previous one means. First, the required 
properties of the get_timer routine.

   1. It must have 1 millisecond resolution and accuracy (more or less).
      For instance, the old NIOS timer that incremented the timestamp by
      10 every 10 milliseconds in response to an interrupt is not compliant.
   2. The get_timer routine must have full period accuracy without any
      assumptions regarding what is going on in u-boot. This period is
      4294967 seconds or so.

I then suggest that the minimum system requirements to support the 
u-boot timer are as follows:

    * Either there exists a free-running timer whose period is > 4294967
      and whose resolution is 1 millisecond or better. This probably
      includes all 64 bit timestamp counters.
    * Or there exists a method of generating interrupts at a known rate
      of 1 millisecond or faster. This is a superset of  the current PPC
      method.
    * Or there exists a method of generating interrupts at a known fixed
      rate slower than once a millisecond AND there exists a readable
      free running counter whose period is longer or the same as the
      interrupt rate AND whose resolution is at least 1 ms. This would
      include N bit counters that generate an interrupt when they
      overflow, or some long timestamp counter with another, possibly 
      unrelated interrupt generating methodology that is faster than the
      counter overflow interval.

There are many systems that are able to do all three cases, or two of 
three, so they have a choice on how to implement get_timer(). I claim 
that these are sufficient conditions. I also claim they are necessary. 
If a hardware system does not meet these criteria, I claim it can't meet 
the get_timer requirements. Do such systems exist today that use u-boot? 
I think probably so, but maybe they are corner cases. Note that you 
cannot extend a 32 bit counter to a 64 bit counter reliably without 
using interrupts to do so when get_timer is not being called.

Systems using the first approach above have essentially all their logic 
in the get_timer routine, but MUST use at least some 64 bit arithmetic 
(actually 33 bit arithmetic if you don't count shifts) to do so because 
the delta time between calls can be very large. The routine is not too 
complex to write however.

Systems using the second approach have a real simple get_timer routine 
but a more complex interrupt routine. The interrupt routine is still 
quite simple, it being either a single 32 bit add or 2 32 bit adds (if 
the update rate is a fraction of a ms).

Systems using the last approach are the most complex. They need a 
routine like the get_timer routine I provided yesterday that can be 
called in the interrupt routine to kick the timer up to present day, 
plus a version of the routine called by the user that disables 
interrupts, updates the timestamp, and re-enables interrupts. Are there 
any such systems out there? Probably, but I will wait for someone to 
identify them before I dive in! Certainly the other two methods are much 
easier to create.

Does that sound right?

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

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

On 23/05/11 23:19, Wolfgang Denk wrote:
> Dear Graeme Russ,
> 
> In message <4DDA5334.4060308@gmail.com> you wrote:
>>
>>>>  - A helper function in /lib/ u32 get_raw_ms() which uses get_raw_ticks()
>>>>    and get_tick_rate() to correctly maintain the ms counter used by
>>>>    get_timer() - This function can be weak (so next point)
>>>
>>> Ditto.  What would that do?  If it gets milliseconds as the name
>>> suggest, that's already the function needed for get_timer()?
>>
>> OK, there appears to be a consensus that not all hardware actually supports
>> a free-running timer with 1ms resolution. To overcome this, the idea is to
> 
> Indeed.  I guess most of them do not.
> 
>> create a common library function which maintains the free running counter.
>> The library function acts as a pre-scaler using a 'raw tick counter' and a
>> 'raw tick rate' supplied by the low level architecture. We define this weak
> 
> What are "raw" ticks?  And what are "cooked" ticks, then?
> 

We never talked about cooked tick - I agree 'raw' is superfluous

>> so that if the architecture can provide a free running 1ms counter, there
>> is no (code size) penalty
> 
> Why do we need a free running 1ms counter at all?  Any free running
> counter of at least millisecoind resolution should be good enough.
> 

Because it get_timer() is 1ms - Yes, get_timer() can calculate it from a
higher resolution source, but that creates multiple implementations of the
pre-scaler of multiple arches - The goal is to reduce code duplication
here. get_timer() calls get_raw_ms() (ok, we could just call that get_ms)
which is a common library routine which does the pre-scaling. Arches which
can maintain this 1ms counter (via interrupts or native hardware counters
for example) don't need it.

>> This approach eliminates all the existing per-arch code which (attempts) to
>> manage the time base behind get time. So we simplify each arch down to it's
>> bare essentials - Provide a counter which increments at a natural fixed
>> rate and what the rate is - Let common library code deal with the rest.
> 
> Did you have a look at the PowerPC implementation?  I'd like to see
> this used as reference.
> 
>>>>  - Calling of get_raw_ticks() regularly in the main loop (how ofter will
>>>>    depend on the raw tick rate, but I image it will never be necessary
>>>>    to call more often than once every few minutes)
>>>
>>> NAK. This concept is fundamentally broken.  I will not accept it.
>>
>> Some existing timers are fundamentally broken - The glitch at the
>> 0xffffffff to 0x00000000 rollover or rollover early - The method discussed
>> in this thread eliminates all such glitches. Provided pre-scaler in /lib/
>> (triggered by get_timer() usually) is called often enough (71 minutes for a
>> 32-bit 1MHz counter) then there is no need. Even then, it is only important
> 
> We already have this nightmare of code for triggering the watchdog on
> systems that use it.
> 
> Assuming there are places in the main loop that get executed often
> enough is a broken concept, and I will not accept any such code.
> 
>> over the time period you are measuring (i.e. two independent 5s delays 2
>> hours apart will not be a problem)
> 
> What is the practical purpose of get_timer()?  What is the longest
> interval we have to cover?  And what is the problem with a rollover?
> 
>>>>  - If the hardware implements a native 32-bit 1ms counter, no call in
>>>>    the main loop is required
>>>
>>> We should make no such requirements.
>>
>> No such requirement of what?
> 
> Of making any calls in the main loop.
> 
>>> Can we not start simple, say by a plain free-runnign 64 bit counter,
>>> be it implemented in hardwar eor in software?  On PowerPC, we have
>>
>> That's exactly what we are suggesting - Let the hardware be free to
>> implement the counter at whatever frequency suits it. 64-bit is not needed
>> in reality
> 
> It may not be needed on some systems, but may be needed on others
> where 32 bit is too short.  Let's use the common base that is know to
> work on all systems, even if it's not strictly needed on all of them.
> 
>>> this immediately in form of the time base register (or more precisely
>>> in form of the two 32 bit registers tbu and tbl representing time base
>>> upper and time base lower).
>>>
>>> Modelling a similar interface using _any_ kind of timer service should
>>> be trivial.  And from there, we can just use the existing code again.
>>
>> Yes, have the library function manage the actual '1ms' aspect
> 
> This is what PPC is doing. And I understand that Reinhard did the same
> in software for AT91.

Yes, duplicate code - lets 'libify' it

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear "J. William Campbell",

In message <4DDAC2F5.7060202@comcast.net> you wrote:
>
> Does that sound right?


I think so.

> --------------000308010205050804040502
> Content-Type: text/html; charset=ISO-8859-1
> Content-Transfer-Encoding: 7bit

Could you please stop posting HTML?  Thanks.

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
No more blah, blah, blah!
	-- Kirk, "Miri", stardate 2713.6

[U-Boot] [RFC] Review of U-Boot timer API

[Mike Frysinger]

On Saturday, May 21, 2011 08:38:29 Graeme Russ wrote:
> NOTE: http://lists.denx.de/pipermail/u-boot/2010-June/073024.html appears
> to imply the following implementation of get_timer() is wrong:
> 
> 	ulong get_timer(ulong base)
> 	{
> 		return get_timer_masked() - base;
> 	}

while this specific code might be wrong due to assumed timer rollover 
behavior, i think the code is wrong to ignore "base" completely.  but maybe it 
doesnt matter after we rewrite things :).

> blackfin
>  - Provides a 64-bit get_ticks() which simply returns 32-bit get_timer(0)

seems to me that most arches do this

>  - get_usec_timer_64() could offer a longer period (584942 years!)

if the hardware can support that large of a timer ...

otherwise, i'd love to see the whole timer API reduced to just get_timer() and 
a proper doc/README.timer written.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110523/866118e4/attachment.pgp 

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

Hi All,

OK, here goes - A much more in-depth generic analysis for a timer API

I hope this highlights why the current discussion thread has ended
up where is has

Regards,

Graeme

Definitions:
 'register' - A storage element - Can be:
   - A dedicated CPU register
   - A location in memory (as defined by a C variable for example)
   - A memory mapped storage location
   - A port-mapped storage location
   - etc

 'platform' - A complete hardware configuration which is capable of
   running U-Boot. An operation performed by 'the plaform' (such as
   incrementing a counter) is assumed to occur without software
   intervention (although initial setup and configuration may be
   required)

 'fixed' - Same across all platforms

 'constant' - Does not change between any two pre-determined events. For
   example 'between power up and power down', 'after being configured to
   the next time it is configured'

 'tick' - A constant period of time - The tick period is not fixed (see
   'fixed' above)

 'counter' - A register which increments by 1 in response to a given
   stimulus (an interupt, a programmed clock, a user pressing a button,
   a character recieved on a serial port etc)

 'tick counter' - A register which is incremented every tick (typically
   by the platform)

 'timer' - A register which stores a value representing a number of
   fixed time intervals (1ms unless otherwise noted)

 'ISR' - Interrupt Service Routine - A function which is called by the CPU
   in response to an interrupt

 'interrupt' - A asynchronous signal which causes the CPU to suspent the
   current flow of program execution and execute an ISR - Upon completion
   of the ISR, control is returned to the suspended program flow

Assumptions:
 - Every platform has a tick counter
 - Not all platforms support interrupts
 - The size of the tick counter is not fixed

Mandatory Platform Requirements:
 - A tick counter MUST exist
 - ticks MUST occur at least once every one millisecond
 - A means of reading the tick counter register MUST be provided
 - A means of obtaining the tick frequency MUST be provided
 - The tick counter MUST be unsigned
 - The tick counter MUST use the entire range of the tick counter register
   (i.e. from 'All 0's' to 'All 1's')
 - The tick counter MUST overflow from "all 1's" to "all 0's"

Goal:
 - Maintain a 1ms time base which can be obtained via a simple call to
   get_timer()
 - The timer MUST use the entire range of it's register (i.e. from
   'All 0's' to 'All 1's')
 - The timer MUST overflow from "all 1's" to "all 0's"
 - Reduce the amount of code duplication between platforms

So what do we need in order to implement a good[1] timer API?
 1. A tick counter
 2. If #1 does not have a period of 1ms, a prescaler to convert
    the tick counter to a 1ms timer
 3. If #2 is a software implementation, a way of regularly calling
    the prescaler

Lets look at each of these in detail:

The tick counter
----------------
There are a wide variety of implementations which can be divided into two
categories:
 Hardware Counters:
 - A non-programmable read-only hardware counter. An example is a simple
   hardware counter that is tick'd by the CPU clock starting at zero when
   the platform is powered-up
 - A non-programmable read-only hardware counter that can be reset to zero
   by writting to a control register
 - A non-programmable hardware counter which can be reset to any arbitrary
   value
 - A programmable hardware counter - Examples include:
   - A prescaler which increments the counter every 'x' CPU clocks where
     'x' is set programatically by writing to reserved control registers
   - A programmable clock source independent of the CPU clock

 Software Counters:
 - Implemented in U-Boot software (C or assembler)
 - Must be regularly triggered (usually by an interrupt)
 - Interrupt period can be programmable (arch/x86/cpu/sc520 has a good
   example) or non-programmable

If the period of the hardware counter or the interrupt generator can be
programmed to 1ms (or is fixed at 1ms) then there is no need to implement
#2 (or, therefore, #3)

Here is a list of some typical tick counter rollover durations. The
following intervals have been rounded down to the nearest whole number and
therefore represent a realistic minimum interval at which the tick counter
must be read to prevent timing glitches caused by multiple roll-overs.

 - 1ms (1kHz), 16-bit - 65 seconds
 - 1ms (1kHz), 32-bit - 49 days
 - 1ms (1kHz), 64-bit - 584,942,417 years
 - 1us (1MHz), 32-bit - 71 minutes
 - 1us (1MHz), 64-bit - 584,942 years
 - 1ns (1GHz), 32-bit - 4 seconds
 - 1ns (1GHz), 64-bit - 584 years

The Prescaler
-------------
The prescaler converts the tick counter (with period <1ms) to a timer with
period = 1ms. As with the tick counter, the prescaler may be implemented
in either hardware or software. Where the prescaler is implemented in
hardware (and provides a 1ms counter) there is no need to implement a
software prescaler. However, if no hardware prescaler exists (or the
hardware presaler does not increase the counter period to 1ms) a software
prescaler is required.

Currently, any required software prescaler is implented per-platform. We
now have a function (thanks Bill Campbell) which implements a software
prescaler by periodically reading the tick counter. This function MUST be
called more frequently that the roll-over period of the tick counter
provided. This function has a few 'nice' features:

 - It can be implemented in /lib/ thus elliminating a lot of platform
   specific implementations
 - It does not require the tick frequency to be known at compile time
 - Is not effected by roll-over of the tick-counter
 - Does not need to be called@contanst rate (only needs to be within
   the rollover period listed above)
 - Optionally does not use divides for platforms where divides are either
   slow or non-exitent
 - Optionally handles a tick period which is not an integer multiple of
   1ms

Regularly Servicing the Software Prescaler
------------------------------------------
If the prescaler is implemented in software, it must be called often
enough to avoid tiemr glitches due to multiple rollovers of the tick
counter between prescaler calls. There are several ways this can be
achieved:
 - Inside get_timer() - Taking into consideration the roll-over periods
   listed earlier (and the fact the common scenario is implementing
   time-outs in the order of a few seconds or so) this may not be such a
   bad approach on most platforms. Any platform with a 64-bit tick
   counter will be fine. Even a 32-bit 1us tick counter could be OK
   (when would we ever need to measure a 71 minute period?). Actually,
   this method should always be used to ensure the timer is up-to-date
   get_timer() returns the current timer value.
 - An ISR - If the platform supports interrupts. Because get_timer() calls
   the prescaler as well, the ISR only needs to be triggered once every
   tick rollover period (actually, slightly quicker to account for ISR
   latency)
 - Main Loop - Last resort option - Breaks when calling stand-alone
   applications such as burn-in test beds etc.


So, provided the platform can provide a tick counter, and the tick counter
can be serviced regularly[2], we can implement a timer API for which the
platform can COMPLETELY disregard the fact that a 1ms time base is
required[3].

[1]An implementation with minimal code duplication across platforms and
   requires as little design and implementation per platform as
   practicalibly possible
[2]During the interval over which a timing operation is being actively
   performed. It does not matter if the timer 'glitches' while it is not
   in use.
[3]Well not quite - The platform does need to make sure the tick period is
   less than 1ms

[U-Boot] [RFC] Review of U-Boot timer API

[Mike Frysinger]

rather than inventing our own set of terms, how about piggybacking what Linux 
has already done ?  they have "cycle counters" and "timer counters" (which 
bubble up into "clock sources", but i dont think we need those).

further, Linux probably already has implementations we can steal for the arch-
specific "cycle counters", and the common code which takes care of scaling 
those into actual measurements of time.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110524/f782b20c/attachment.pgp 

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

Mike,

On Tue, May 24, 2011 at 2:24 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> rather than inventing our own set of terms, how about piggybacking what Linux
> has already done ? ?they have "cycle counters" and "timer counters" (which
> bubble up into "clock sources", but i dont think we need those).

I only put the terms in to clarify them within the scope of the document
to avoid the inevitable 'but that means x' arguments ;)

> further, Linux probably already has implementations we can steal for the arch-
> specific "cycle counters", and the common code which takes care of scaling
> those into actual measurements of time.

Maybe - but I think the Linux implementation might be a bit too heavy
handed - We only need a very lightweight, simple implementation. Despite
the appearance of my write-up, the suggested implementation is very
simple:

 - A common software prescaler in /lib/ (not needed if hardware provides
   an exact 1ms period counter)
 - Arch specific implementation of tick counter (already there)
 - Arch specific hook into prescaler (not needed if tick counter has a
   'long enough' roll-over period or is 1ms, otherwise a tiny ISR which
   simply calls the prescaler)

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Reinhard Meyer]

I know its futile to repeat what I suggested about 9 months ago...

Since get_timer() is only used (to my knowledge) to break out of
loops that do not terminate normally because an expected event does
not occur, the following API would be simpler and take less time per
loop:

(I use the names A and B to avoid nitpicking on real function names
which can be later chosen arbitrarily if this idea prevails...)

uint timeout = A(timeout_in_ms);

do {...} while (!B(timeout));

OR

for (;;) {
   dowhatever...
   if (B(timeout)) {
     error_handling...
     break;
   }
}

Internally both functions would be rather trival:

uint A(uint timeout_in_ms):

return current_tick + timeout_in_ms * ticks_per_ms;

or - for better precision if ticks_per_ms is not a whole number:

return current_tick + (timeout_in_ms * ticks_per_second) / 1000;

or any fractional method to more exactly calculate that equation
without needing a 64 bit divide.

bool B(uint end_tick):

a simple unsigned subtraction of end_tick minus current_tick,
evaluating the carry flag. Since we don't get a carry flag in C
a simple methods will do:
return ((int)(end_tick - current_tick)) < 0;

options:
current_tick needs only to be uint32, to get to the largest possible timeout
any 64 bit tick counter can be right shifted to get a 32 bit value that
increments just faster than 1ms.

features:
the only complicated calculation is done before the loop starts, the loop
itself is not made significantly slower since only a subtraction (and maybe
shift to scale down a high speed tick) is required.

Reinhard

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

Dear Reinhard,

On Tue, May 24, 2011 at 3:31 PM, Reinhard Meyer
<u-boot@emk-elektronik.de> wrote:
> I know its futile to repeat what I suggested about 9 months ago...
>
> Since get_timer() is only used (to my knowledge) to break out of
> loops that do not terminate normally because an expected event does
> not occur, the following API would be simpler and take less time per
> loop:
>
> (I use the names A and B to avoid nitpicking on real function names
> which can be later chosen arbitrarily if this idea prevails...)
>
> uint timeout = A(timeout_in_ms);
>
> do {...} while (!B(timeout));
>
> OR
>
> for (;;) {
> ?dowhatever...
> ?if (B(timeout)) {
> ? ?error_handling...
> ? ?break;
> ?}
> }
>
> Internally both functions would be rather trival:
>
> uint A(uint timeout_in_ms):
>
> return current_tick + timeout_in_ms * ticks_per_ms;
>
> or - for better precision if ticks_per_ms is not a whole number:
>
> return current_tick + (timeout_in_ms * ticks_per_second) / 1000;
>
> or any fractional method to more exactly calculate that equation
> without needing a 64 bit divide.
>
> bool B(uint end_tick):
>
> a simple unsigned subtraction of end_tick minus current_tick,
> evaluating the carry flag. Since we don't get a carry flag in C
> a simple methods will do:
> return ((int)(end_tick - current_tick)) < 0;
>

Elegant and simple but precludes using get_timer() to perform any
meaningful time measurement - However this can be resolved by
doing

	start = get_current_tick();
	...
	duration = get_elapsed_ms(start);

get_elapsed_ms() converts (current_tick - start) to ms. It can still be
a common library routine which calls a HAL function get_tick_frequency()

> options:
> current_tick needs only to be uint32, to get to the largest possible timeout
> any 64 bit tick counter can be right shifted to get a 32 bit value that
> increments just faster than 1ms.
>
> features:
> the only complicated calculation is done before the loop starts, the loop
> itself is not made significantly slower since only a subtraction (and maybe
> shift to scale down a high speed tick) is required.
>

But we run into problems when the tick counter is only 32-bit and the tick
frequency very fast so we would need to extend the tick counter to 64-bit
and have to periodically update it.

Nevertheless, I think we have two very viable options :)

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Albert ARIBAUD]

Le 24/05/2011 07:43, Graeme Russ a ?crit :
> Dear Reinhard,
>
> On Tue, May 24, 2011 at 3:31 PM, Reinhard Meyer
> <u-boot@emk-elektronik.de>  wrote:
>> I know its futile to repeat what I suggested about 9 months ago...
>>
>> Since get_timer() is only used (to my knowledge) to break out of
>> loops that do not terminate normally because an expected event does
>> not occur, the following API would be simpler and take less time per
>> loop:
>>
>> (I use the names A and B to avoid nitpicking on real function names
>> which can be later chosen arbitrarily if this idea prevails...)
>>
>> uint timeout = A(timeout_in_ms);
>>
>> do {...} while (!B(timeout));
>>
>> OR
>>
>> for (;;) {
>>   dowhatever...
>>   if (B(timeout)) {
>>     error_handling...
>>     break;
>>   }
>> }
>>
>> Internally both functions would be rather trival:
>>
>> uint A(uint timeout_in_ms):
>>
>> return current_tick + timeout_in_ms * ticks_per_ms;
>>
>> or - for better precision if ticks_per_ms is not a whole number:
>>
>> return current_tick + (timeout_in_ms * ticks_per_second) / 1000;
>>
>> or any fractional method to more exactly calculate that equation
>> without needing a 64 bit divide.
>>
>> bool B(uint end_tick):
>>
>> a simple unsigned subtraction of end_tick minus current_tick,
>> evaluating the carry flag. Since we don't get a carry flag in C
>> a simple methods will do:
>> return ((int)(end_tick - current_tick))<  0;
>>
>
> Elegant and simple but precludes using get_timer() to perform any
> meaningful time measurement - However this can be resolved by
> doing
>
> 	start = get_current_tick();
> 	...
> 	duration = get_elapsed_ms(start);
>
> get_elapsed_ms() converts (current_tick - start) to ms. It can still be
> a common library routine which calls a HAL function get_tick_frequency()
>
>> options:
>> current_tick needs only to be uint32, to get to the largest possible timeout
>> any 64 bit tick counter can be right shifted to get a 32 bit value that
>> increments just faster than 1ms.
>>
>> features:
>> the only complicated calculation is done before the loop starts, the loop
>> itself is not made significantly slower since only a subtraction (and maybe
>> shift to scale down a high speed tick) is required.
>>
>
> But we run into problems when the tick counter is only 32-bit and the tick
> frequency very fast so we would need to extend the tick counter to 64-bit
> and have to periodically update it.
>
> Nevertheless, I think we have two very viable options :)


Not sure I still follow what the two options are -- a heads up is welcome.

However, I do like the simplicity in having a single time unit (ticks) 
for the timer API -- asuming it covers all needs -- and providing other 
time units only as helper functions.

 > Regards,
 >
 > Graeme

> Regards,
>
> Graeme

Amicalement,
-- 
Albert.

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

On 24/05/11 16:11, Albert ARIBAUD wrote:
> Le 24/05/2011 07:43, Graeme Russ a ?crit :
>> Dear Reinhard,
>>
>> On Tue, May 24, 2011 at 3:31 PM, Reinhard Meyer
>> <u-boot@emk-elektronik.de>  wrote:
>>> I know its futile to repeat what I suggested about 9 months ago...
>>>
>>> Since get_timer() is only used (to my knowledge) to break out of
>>> loops that do not terminate normally because an expected event does
>>> not occur, the following API would be simpler and take less time per
>>> loop:
>>>
>>> (I use the names A and B to avoid nitpicking on real function names
>>> which can be later chosen arbitrarily if this idea prevails...)
>>>
>>> uint timeout = A(timeout_in_ms);
>>>
>>> do {...} while (!B(timeout));
>>>
>>> OR
>>>
>>> for (;;) {
>>>   dowhatever...
>>>   if (B(timeout)) {
>>>     error_handling...
>>>     break;
>>>   }
>>> }
>>>
>>> Internally both functions would be rather trival:
>>>
>>> uint A(uint timeout_in_ms):
>>>
>>> return current_tick + timeout_in_ms * ticks_per_ms;
>>>
>>> or - for better precision if ticks_per_ms is not a whole number:
>>>
>>> return current_tick + (timeout_in_ms * ticks_per_second) / 1000;
>>>
>>> or any fractional method to more exactly calculate that equation
>>> without needing a 64 bit divide.
>>>
>>> bool B(uint end_tick):
>>>
>>> a simple unsigned subtraction of end_tick minus current_tick,
>>> evaluating the carry flag. Since we don't get a carry flag in C
>>> a simple methods will do:
>>> return ((int)(end_tick - current_tick))<  0;
>>>
>>
>> Elegant and simple but precludes using get_timer() to perform any
>> meaningful time measurement - However this can be resolved by
>> doing
>>
>>     start = get_current_tick();
>>     ...
>>     duration = get_elapsed_ms(start);
>>
>> get_elapsed_ms() converts (current_tick - start) to ms. It can still be
>> a common library routine which calls a HAL function get_tick_frequency()
>>
>>> options:
>>> current_tick needs only to be uint32, to get to the largest possible
>>> timeout
>>> any 64 bit tick counter can be right shifted to get a 32 bit value that
>>> increments just faster than 1ms.
>>>
>>> features:
>>> the only complicated calculation is done before the loop starts, the loop
>>> itself is not made significantly slower since only a subtraction (and maybe
>>> shift to scale down a high speed tick) is required.
>>>
>>
>> But we run into problems when the tick counter is only 32-bit and the tick
>> frequency very fast so we would need to extend the tick counter to 64-bit
>> and have to periodically update it.
>>
>> Nevertheless, I think we have two very viable options :)
> 
> 
> Not sure I still follow what the two options are -- a heads up is welcome.

1) get_timer() returning milliseconds
2) get_current_ticks() + ticks_to_ms()

> However, I do like the simplicity in having a single time unit (ticks) for
> the timer API -- asuming it covers all needs -- and providing other time
> units only as helper functions.

My preference is to expose SI (and derivative) units of time measurement
(seconds, milliseconds, microseconds) only to the outside world (drivers,
stand alone applications etc)

The truth of the matter is, the proposed API will expose both - There will
be a HAL function get_ticks() which is used by the prescaler and a
get_tick_frequency() HAL function. So there will be nothing stopping
someone (apart from a mandate not to) using 'ticks'

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Scott McNutt]

Graeme Russ wrote:
> Dear Reinhard,
> 
> On Tue, May 24, 2011 at 3:31 PM, Reinhard Meyer
> <u-boot@emk-elektronik.de> wrote:
>> I know its futile to repeat what I suggested about 9 months ago...
>>
>> Since get_timer() is only used (to my knowledge) to break out of
>> loops that do not terminate normally because an expected event does
>> not occur, the following API would be simpler and take less time per
>> loop:
...

> Elegant and simple but precludes using get_timer() to perform any
> meaningful time measurement

...

Why must get_timer() be used to perform "meaningful time measurement?"

If all of this is about time measurement, why not start with your dream
time measurement API, and go from there? And make it an optional
feature.

If the existing API, that has been used successfully for years,
primarily as a mechanism for detecting a time-out, has issues,
then let's resolve _those_ issues and avoid the "car-boat" ... the
vehicle that floats like a car and handles like a boat. ;-)

Regards,
--Scott

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear Albert ARIBAUD,

In message <4DDB4C1C.7030301@aribaud.net> you wrote:
>
> Not sure I still follow what the two options are -- a heads up is welcome.
> However, I do like the simplicity in having a single time unit (ticks)
> for the timer API -- asuming it covers all needs -- and providing other
> time units only as helper functions.

I don't think using ticks is a good idea.  You would need to change
all definitiuons of timeouts and delays and such.

Why not using a scaled unit like microsecods or the currently used
milliseconds?

I wonder why we suddenly have to change everything that has been
working fine for more than a decade (ignoring the large number of
incorrect, incomplete or broken implementations).  But so far we
really never needed anything else but  udelay()  for the typical short
device related timeouts, and  get_time()  for longer, often protocol
defined,  timeouts.

Is there anything wrong with these two solutions, based on standard
units (us and ms) ?

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 follow where the path may lead....go instead where there is no
path and leave a trail.

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear Graeme Russ,

In message <4DDB59CD.2020009@gmail.com> you wrote:
>
> My preference is to expose SI (and derivative) units of time measurement
> (seconds, milliseconds, microseconds) only to the outside world (drivers,
> stand alone applications etc)

Please delete the "to the outside world ..." part.

> The truth of the matter is, the proposed API will expose both - There will
> be a HAL function get_ticks() which is used by the prescaler and a
> get_tick_frequency() HAL function. So there will be nothing stopping
> someone (apart from a mandate not to) using 'ticks'

I am tempted to NAK this right here.  I would like to see ticks as an
internal implementation detail only, and only be used as far as it is
necessary to provide proper scaled timer services.  All public
interfaces shall always only used scaled units.

Again: what's wrong with the us based udelay() and the ms based
get_timer() we have now?


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
Syntactic sugar causes cancer of the semicolon.
                - Epigrams in Programming, ACM SIGPLAN Sept. 1982

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear Scott McNutt,

In message <4DDBB29D.2050102@psyent.com> you wrote:
>
> Why must get_timer() be used to perform "meaningful time measurement?"

Excellent question!  It was never intended to be used as such.

Also, neither udelay() nor get_timer() make any warranties about
accuracy or precision. If they are a few percent off, that's perfectly
fine. Even if they are 10% off this is not a big problem anywhere.

> If all of this is about time measurement, why not start with your dream
> time measurement API, and go from there? And make it an optional
> feature.

D'acore.

> If the existing API, that has been used successfully for years,
> primarily as a mechanism for detecting a time-out, has issues,
> then let's resolve _those_ issues and avoid the "car-boat" ... the
> vehicle that floats like a car and handles like a boat. ;-)

:-)

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
Sometimes a man will tell his bartender things he'll never tell his doctor.
	-- Dr. Phillip Boyce, "The Menagerie" ("The Cage"),
	   stardate unknown.

[U-Boot] [RFC] Review of U-Boot timer API

[J. William Campbell]

On 5/24/2011 7:12 AM, Wolfgang Denk wrote:
> Dear Albert ARIBAUD,
>
> In message<4DDB4C1C.7030301@aribaud.net>  you wrote:
>> Not sure I still follow what the two options are -- a heads up is welcome.
>> However, I do like the simplicity in having a single time unit (ticks)
>> for the timer API -- asuming it covers all needs -- and providing other
>> time units only as helper functions.
> I don't think using ticks is a good idea.  You would need to change
> all definitiuons of timeouts and delays and such.
>
> Why not using a scaled unit like microsecods or the currently used
> milliseconds?
>
> I wonder why we suddenly have to change everything that has been
> working fine for more than a decade (ignoring the large number of
> incorrect, incomplete or broken implementations).  But so far we
> really never needed anything else but  udelay()  for the typical short
> device related timeouts, and  get_time()  for longer, often protocol
> defined,  timeouts.
>
> Is there anything wrong with these two solutions, based on standard
> units (us and ms) ?
Hi All,
         After really looking into this, I think I agree with Wolfgang 
that using ms for a get_timer timebase is the best way to go. This 
thinking is heavily influenced (in my case anyway) by the fact that in 
the interrupt driven cases (and these are the ONLY fully compliant cases 
ATM I think), the "cost" of using ms is 0, because that is the "native" 
unit in which the timer ticks. This makes everything real simple. We 
can, right today, produce an API that supports u32 get_time_ms(void) for 
all CPUs in use. This would allow u32 get_timer(u32 base) to continue to 
exist as-is. These implementations would still be technically "broken" 
in the non-interrupt case, but they would work at least as well as they 
presently do. In fact, they would operate better because they would all 
use a single routine, not a bunch of different routines (some of which I 
am pretty sure have errors). Wolfgang would need to accept the fact that 
we are not yet "fixing" all the non-interrupt cases. This needs to be 
done, but is a different problem (I hope). In the non-interrupt case 
there is some cost in converting to ms from the native units, but we are 
in a timout loop, so a few extra instructions do not matter much. It is 
only a few 32 bit multiplies, which these days are real fast. If we can 
figure out how to use interrupts eventually, even this will go away.

         Then, we can ADD an optional performance measuring API like 
Scott suggests. This API would be something similar to the following:
struct ticks
{
     u32    tickmsb;
     u32   ticklsb;
} ;
struct time_value
{
   u32 seconds;
   u 32 nano_seconds;
};


and the functions would be get_ticks(struct ticks * p) , 
get_tick_delta(struct ticks * minuend, struct ticks * subtrahend),  
and   cvt_tick_delta(struct time_value * result, struct ticks *input). I 
didn't use u64 on purpose, because I don't want any un-necessary 
functions pulled from the library. get_ticks reads a "hi precision" 
counter. How high the precision is depends on the hardware. 
get_tick_delta subtracts two tick values, leaving the difference in the 
first operand. Yes, this is may be simple to do in open code but it is 
better to hide the details. (What if msb is seconds and lsb is 
nanoseconds, then it is not so simple). cvt_tick_delta converts from 
ticks to seconds and nano_seconds. We also need a u32 
get_tick_resolution, which would return the tick resolution in ns. The 
user never needs to do any arithmetic on ticks, so that makes his life 
much easier. However, the user may want to know the resolution of these 
measurements. All these functions are quite fast except possibly 
cvt_tick_delta, which is only needed for printing anyway.
         If the hardware has a performance monitoring counter, 
implementing these functions is quite simple. The PPC and the x86 
certainly do (well, any x86 pentium and above anyway). This is a whole 
lot of chips off our plate. In cases where no such counter exists, we 
will use whatever counters there are available already. Some of the ARM 
counters are running at 32 kHz, so their resolution won't be great, but 
it is what it is. If we find out that some of these other CPUs have a 
performance counter, we will use it. This API will be completely 
optional in u-boot and can be removed by changing a #define.
Thoughts welcome.

Best Regards,
Bill Campbell

> Best regards,
>
> Wolfgang Denk
>

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

On 25/05/11 00:19, Wolfgang Denk wrote:
> Dear Scott McNutt,
> 
> In message <4DDBB29D.2050102@psyent.com> you wrote:
>>
>> Why must get_timer() be used to perform "meaningful time measurement?"
> 
> Excellent question!  It was never intended to be used as such.

Because get_timer() as it currently stands can as it is assumed to return
milliseconds

> Also, neither udelay() nor get_timer() make any warranties about
> accuracy or precision. If they are a few percent off, that's perfectly
> fine. Even if they are 10% off this is not a big problem anywhere.
> 
>> If all of this is about time measurement, why not start with your dream
>> time measurement API, and go from there? And make it an optional
>> feature.
> 
> D'acore.
> 
>> If the existing API, that has been used successfully for years,
>> primarily as a mechanism for detecting a time-out, has issues,
>> then let's resolve _those_ issues and avoid the "car-boat" ... the
>> vehicle that floats like a car and handles like a boat. ;-)
> 
> :-)

OK, let's wind back - My original suggestion made no claim towards changing
what the API is used for, or how it looks to those who use it (for all
practical intents and purposes). I suggested:
 - Removing set_timer() and reset_timer()
 - Implement get_timer() as a platform independent function

Lets look at two real-world implementations of my suggested solution - One
which 'exposes' ticks and one which does not..

=============
Exposing ticks and tick_frequency to everyone via a 'tick' HAL

In /lib/timer.c

void prescaler(void)
{
	u32 ticks = get_ticks();
	u32 tick_frequency = get_tick_frequency();

	/* Bill's algorithm */

	/* result stored in gd->timer_in_ms; */
}

u32 get_timer(u32 base)
{
	prescaler(ticks, tick_frequency);

	return gd->timer_in_ms - base;
}

In /arch/cpu/soc/timer.c or /arch/cpu/timer.c or /board/<board>/timer.c

u32 get_ticks(void)
{
	u32 ticks;

	/* Get ticks from hardware counter */

	return ticks;
}

u32 get_tick_frequency(void)
{
	u32 tick_frequency;

	/* Determine tick frequency - likely very trivial */

	return tick_frequency;
}

=======================
Not exposing ticks and tick_frequency to everyone

In /lib/timer.c

void prescaler(u32 ticks, u32 tick_frequency)
{
	u32 current_ms;

	/* Bill's algorithm */

	/* result stored in gd->timer_in_ms; */
}

In /arch/cpu/soc/timer.c or /arch/cpu/timer.c or /board/<board>/timer.c

static u32 get_ticks(void)
{
	u32 ticks;

	/* Get ticks from hardware counter */

	return ticks;
}

static u32 get_tick_frequency(void)
{
	u32 tick_frequency;

	/* Determine tick frequency */

	return tick_frequency;
}

u32 get_timer(u32 base)
{
	u32 ticks = get_ticks();
	u32 tick_frequency = get_tick_frequency();

	prescaler(ticks, tick_frequency);

	return gd->timer_in_ms - base;
}

===============
I personally prefer the first - There is only one implementation of
get_timer() in the entire code and the platform implementer never has to
concern themselves with what the tick counter is used for. If the API gets
extended to include get_timer_in_seconds() there is ZERO impact on
platforms. Using the second method, any new feature would have to be
implemented on all platforms - and we all know how well that works ;)

And what about those few platforms that are actually capable of generating
a 1ms timebase (either via interrupts or natively in a hardware counter)
without the prescaler? Well, with prescaler() declared weak, all you need
to do in /arch/cpu/soc/timer.c or /arch/cpu/timer.c or
/board/<board>/timer.c is:

For platforms with a 1ms hardware counter:
void prescaler(void /* or u32 ticks, u32 tick_frequency*/)
{
	gd->timer_in_ms = get_milliseconds();
}

For platforms with a 1ms interrupt source:
void timer_isr(void *unused)
{
	gd->timer_in_ms++;
}

void prescaler(void /* or u32 ticks, u32 tick_frequency*/)
{
}


And finally, if the platform supports interrupts but either the hardware
counter has better accuracy than the interrupt generator or the interrupt
generator cannot generate 1ms interrupts, configure the interrupt generator
to fire at any rate better than the tick counter rollover listed in
previous post and:

void timer_isr(void *unused)
{
	/*
	 * We are here to stop the tick counter rolling over. All we
	 * need to do is kick the prescaler - get_timer() does that :)
	 */
	get_timer(0);
}

In summary, platform specific code reduces to:
 - For a platform that cannot generate 1ms interrupts AND the hardware
   counter is not in ms
   - Implement get_ticks() and get_tick_frequency()
   - Optionally implement as ISR to kick the prescaler
 - For a platform that can generate 1ms interrupts, or the hardware
   counter is in ms
   - Override the prescaler() function to do nothing

If none of the above not look 'simple', I invite you to have a look in
arch/arm ;)

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[J. William Campbell]

On 5/24/2011 9:51 AM, Graeme Russ wrote:
> On 25/05/11 00:19, Wolfgang Denk wrote:
> =============
<snip>
Hi all,
         I have a few of questions.
First, it seems that the get_timer interface is expected to work 
properly only after relocation and only when bss is available. I say 
this because the PPC version uses an (initialized) variable, timestamp, 
to hold the time. If that is the case, there is no need to hold the 
timer static data in gd, as I have been doing up to  now. Am I correct ?
      Second, udelay is expected to be available before  bss is 
available, very early in the startup process. There a comments to that 
effect in several places in the code. Therefore, udelay cannot use 
global or static variables. Is this true?
     And third, udelay is only expected/required to work correctly for 
"short", like say 10 seconds. I say this based on comments contained in 
powerpr/lib/time.c. Please ack or nak this as well.

I have a couple of "small" changes to the previously submitted code, 
based partly on the above
> Exposing ticks and tick_frequency to everyone via a 'tick' HAL
>
> In /lib/timer.c

ulong timestamp = 0;
/*
      This routine is called to initialize the timer after BSS is available
*/
void  __weak prescaler_init(void)
{
     u32 tick_frequency = get_tick_frequency();
      /* initialize prescaler variables */
}

> void __weak prescaler(void)
> {
> 	u32 ticks = get_ticks();
                     /* Bill's algorithm */
                    /* result stored in timestamp; */
> }
>
> u32 get_timer(u32 base)
> {
#if defined(CONFIG_SYS_NEED_PRESCALER)
> 	prescaler();
#endif
> 	return timestamp - base;
> }
>
> In /arch/cpu/soc/timer.c or /arch/cpu/timer.c or /board/<board>/timer.c
>
> u32 get_ticks(void)
> {
> 	u32 ticks;
>
> 	/* Get ticks from hardware counter */
>
> 	return ticks;
> }
>
> u32 get_tick_frequency(void)
> {
> 	u32 tick_frequency;
>
> 	/* Determine tick frequency - likely very trivial */
>
> 	return tick_frequency;
> }
or instead the user may override prescaler_init and prescaler if, for 
some reason, a highly optimized version is desired. Note also that if 
the configuration variable CONFIG_SYS_NEED_PRESCALER is not defined, the 
additional prescaler routines will not be called anywhere so the 
routines should not be loaded. Yes, it it a #define to manage, but it 
should allow the existing u-boots to be the same size as before, with no 
unused code. This size matters to some people a lot!
> =======================
> Not exposing ticks and tick_frequency to everyone
>
> In /lib/timer.c
>
> void prescaler(u32 ticks, u32 tick_frequency)
> {
> 	u32 current_ms;
>
> 	/* Bill's algorithm */
>
> 	/* result stored in gd->timer_in_ms; */
> }
>
> In /arch/cpu/soc/timer.c or /arch/cpu/timer.c or /board/<board>/timer.c
>
> static u32 get_ticks(void)
> {
> 	u32 ticks;
>
> 	/* Get ticks from hardware counter */
>
> 	return ticks;
> }
>
> static u32 get_tick_frequency(void)
> {
> 	u32 tick_frequency;
>
> 	/* Determine tick frequency */
>
> 	return tick_frequency;
> }
>
> u32 get_timer(u32 base)
> {
> 	u32 ticks = get_ticks();
> 	u32 tick_frequency = get_tick_frequency();
>
> 	prescaler(ticks, tick_frequency);
>
> 	return gd->timer_in_ms - base;
> }
>
> ===============
> I personally prefer the first - There is only one implementation of
> get_timer() in the entire code and the platform implementer never has to
> concern themselves with what the tick counter is used for. If the API gets
> extended to include get_timer_in_seconds() there is ZERO impact on
> platforms. Using the second method, any new feature would have to be
> implemented on all platforms - and we all know how well that works ;)
>
> And what about those few platforms that are actually capable of generating
> a 1ms timebase (either via interrupts or natively in a hardware counter)
> without the prescaler? Well, with prescaler() declared weak, all you need
> to do in /arch/cpu/soc/timer.c or /arch/cpu/timer.c or
> /board/<board>/timer.c is:
>
> For platforms with a 1ms hardware counter:
> void prescaler(void /* or u32 ticks, u32 tick_frequency*/)
> {
> 	gd->timer_in_ms = get_milliseconds();
> }
>
> For platforms with a 1ms interrupt source:
> void timer_isr(void *unused)
> {
> 	gd->timer_in_ms++;
> }
>
> void prescaler(void /* or u32 ticks, u32 tick_frequency*/)
> {
> }
>
>
> And finally, if the platform supports interrupts but either the hardware
> counter has better accuracy than the interrupt generator or the interrupt
> generator cannot generate 1ms interrupts, configure the interrupt generator
> to fire at any rate better than the tick counter rollover listed in
> previous post and:
>
> void timer_isr(void *unused)
> {
> 	/*
> 	 * We are here to stop the tick counter rolling over. All we
> 	 * need to do is kick the prescaler - get_timer() does that :)
> 	 */
> 	get_timer(0);
> }
>
> In summary, platform specific code reduces to:
>   - For a platform that cannot generate 1ms interrupts AND the hardware
>     counter is not in ms
>     - Implement get_ticks() and get_tick_frequency()
>     - Optionally implement as ISR to kick the prescaler
>   - For a platform that can generate 1ms interrupts, or the hardware
>     counter is in ms
>     - Override the prescaler() function to do nothing
>
> If none of the above not look 'simple', I invite you to have a look in
> arch/arm ;)
>
> Regards,
>
> Graeme
>
>

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear "J. William Campbell",

In message <4DDBCD69.9090408@comcast.net> you wrote:
>
>          After really looking into this, I think I agree with Wolfgang 
> that using ms for a get_timer timebase is the best way to go. This 
> thinking is heavily influenced (in my case anyway) by the fact that in 
> the interrupt driven cases (and these are the ONLY fully compliant cases 
> ATM I think), the "cost" of using ms is 0, because that is the "native" 
> unit in which the timer ticks. This makes everything real simple. We 
> can, right today, produce an API that supports u32 get_time_ms(void) for 
> all CPUs in use. This would allow u32 get_timer(u32 base) to continue to 
> exist as-is. These implementations would still be technically "broken" 
> in the non-interrupt case, but they would work at least as well as they 
> presently do. In fact, they would operate better because they would all 
> use a single routine, not a bunch of different routines (some of which I 
> am pretty sure have errors). Wolfgang would need to accept the fact that 
> we are not yet "fixing" all the non-interrupt cases. This needs to be 
> done, but is a different problem (I hope). In the non-interrupt case 

I see this as you do, so this there will be no problems for me to
"accept" this.

Thanks!

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
You know that feeling when you're leaning back  on  a  stool  and  it
starts to tip over? Well, that's how I feel all the time.
- Steven Wright

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear Graeme Russ,

In message <4DDBE22D.6050806@gmail.com> you wrote:
>
> >> Why must get_timer() be used to perform "meaningful time measurement?"
> > 
> > Excellent question!  It was never intended to be used as such.
> 
> Because get_timer() as it currently stands can as it is assumed to return
> milliseconds

Yes, but without any guarantee for accuracy or resolution.
This is good enough for timeouts, but nothing for time measurements.

> OK, let's wind back - My original suggestion made no claim towards changing
> what the API is used for, or how it looks to those who use it (for all
> practical intents and purposes). I suggested:
>  - Removing set_timer() and reset_timer()
>  - Implement get_timer() as a platform independent function

Trying to remember what I have read in this thread I believe we have
an agreement on these.

> Exposing ticks and tick_frequency to everyone via a 'tick' HAL

I skip this.  I don't even read it.

> =======================
> Not exposing ticks and tick_frequency to everyone
> 
> In /lib/timer.c
> 
> void prescaler(u32 ticks, u32 tick_frequency)
> {
> 	u32 current_ms;
> 
> 	/* Bill's algorithm */
> 
> 	/* result stored in gd->timer_in_ms; */
> }
> 
> In /arch/cpu/soc/timer.c or /arch/cpu/timer.c or /board/<board>/timer.c
> 
> static u32 get_ticks(void)

Currently we have unsigned long long get_ticks(void)  which is better
as it matches existing hardware.

Note that we also have void wait_ticks(u32) as needed for udelay().

> static u32 get_tick_frequency(void)
> {
> 	u32 tick_frequency;
> 
> 	/* Determine tick frequency */
> 
> 	return tick_frequency;
> }

Note that we also have u32 usec2ticks(u32 usec) and u32 ticks2usec(u32 ticks).

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
Madness has no purpose.  Or reason.  But it may have a goal.
	-- Spock, "The Alternative Factor", stardate 3088.7

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear "J. William Campbell",

In message <4DDBFFF5.1020601@comcast.net> you wrote:
>
> First, it seems that the get_timer interface is expected to work 
> properly only after relocation and only when bss is available. I say 
> this because the PPC version uses an (initialized) variable, timestamp, 
> to hold the time. If that is the case, there is no need to hold the 
> timer static data in gd, as I have been doing up to  now. Am I correct ?

Yes.

>       Second, udelay is expected to be available before  bss is 
> available, very early in the startup process. There a comments to that 
> effect in several places in the code. Therefore, udelay cannot use 
> global or static variables. Is this true?

Yes.

>      And third, udelay is only expected/required to work correctly for 
> "short", like say 10 seconds. I say this based on comments contained in 
> powerpr/lib/time.c. Please ack or nak this as well.

Depending on CPU type and clock, it may be as low as 1 second, or
less. udelay() is based on wait_ticks(), which receives a u32
argument. Assume we have a tick rate of 1 GHz or so, and be prepared
for faster processors.

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
"IBM uses what I like to call the 'hole-in-the-ground  technique'  to
destroy  the  competition.....  IBM digs a big HOLE in the ground and
covers it with leaves. It then puts a big POT OF GOLD nearby. Then it
gives the call, 'Hey, look at all this gold, get over here fast.'  As
soon  as  the competitor approaches the pot, he falls into the pit"
                                                     - John C. Dvorak

[U-Boot] [RFC] Review of U-Boot timer API

[J. William Campbell]

On 5/24/2011 12:19 PM, Wolfgang Denk wrote:
> Dear Graeme Russ,
>
> In message<4DDBE22D.6050806@gmail.com>  you wrote:
>>>> Why must get_timer() be used to perform "meaningful time measurement?"
>>> Excellent question!  It was never intended to be used as such.
>> Because get_timer() as it currently stands can as it is assumed to return
>> milliseconds
> Yes, but without any guarantee for accuracy or resolution.
> This is good enough for timeouts, but nothing for time measurements.
>
>> OK, let's wind back - My original suggestion made no claim towards changing
>> what the API is used for, or how it looks to those who use it (for all
>> practical intents and purposes). I suggested:
>>   - Removing set_timer() and reset_timer()
>>   - Implement get_timer() as a platform independent function
> Trying to remember what I have read in this thread I believe we have
> an agreement on these.
>
>> Exposing ticks and tick_frequency to everyone via a 'tick' HAL
> I skip this.  I don't even read it.
>
>> =======================
>> Not exposing ticks and tick_frequency to everyone
>>
>> In /lib/timer.c
>>
>> void prescaler(u32 ticks, u32 tick_frequency)
>> {
>> 	u32 current_ms;
>>
>> 	/* Bill's algorithm */
>>
>> 	/* result stored in gd->timer_in_ms; */
>> }
>>
>> In /arch/cpu/soc/timer.c or /arch/cpu/timer.c or /board/<board>/timer.c
>>
>> static u32 get_ticks(void)
> Currently we have unsigned long long get_ticks(void)  which is better
> as it matches existing hardware.
>
> Note that we also have void wait_ticks(u32) as needed for udelay().
Hi All,
        I didn't comment before on the definition of ticks, but I fear 
that was unwise. The stated definition  was:

A tick is defined as the smallest increment of system time as measured by a
computer system (seehttp://en.wikipedia.org/wiki/System_time):

	System time is measured by a system clock, which is typically
	implemented as a simple count of the number of ticks that have
	transpired since some arbitrary starting date, called the
	epoch.


Unfortunately, this definition is obsolete, and has been for quite some 
years.  When computers had a single timer, the above definition worked, 
but it no longer does, as many (most?) computers have several hardware 
timers. A "tick" today is the time increment of any particular timer of 
a computer system. So, when one writes a function called get_ticks on a 
PPC, does one mean read the decrementer, or does one read the RTC or 
does one read the TB register(s) A similar situation exists on the 
Blackfin BF531/2/3, that has a preformance counter, a real-time clock, 
and three programmable timers. Which tick do you want? For each u-boot 
implementation, we can pick one timer as the "master" timer, but it may 
not be the one with the most rapid tick rate. It may be the one with the 
most USEFUL tick rate for get_timer. If you take the above definition at 
face value, only the fastest counter value has ticks, and all other 
counters time increments are not ticks. If they are not ticks, what are 
they?

This is one of the reasons I favor the performance monitoring system be 
separate from the get_timer timing methodology, as it will often use a 
different counter and time base anyway. That is also why I prefer to 
have a conversion routine that converts timer values to seconds and 
nano-seconds without reference to "tick" rates, so the user never has to 
deal with these ambiguities. Yes, "under the hood", somebody does, but 
that need not be specified in the external interface. )Nobody has yet 
commented on my proposed performance measuring functions, and I know 
this group well enough not to assume that silence implies consent! )

The prescaler function defined by Graeme needs to read some timer value. 
It turns out that a u32 value is the most appropriate value for the 
get_timer operation.  The value desired is usually not of hardware timer 
tick resolution. It should be the specific bits in the timer, such that 
the LSB resolution is between 1 ms and 0.5 ms. We use greater resolution 
that this only when the counter is short enough that we need lower 
significance bits to make the counter 32 bits. For that reason, the 
function should probably be call something like u32 
read_timer_value(void). I really don't much care what it is called as 
long as we understand what it does.

Best Regards,
Bill Campbell


>> static u32 get_tick_frequency(void)
>> {
>> 	u32 tick_frequency;
>>
>> 	/* Determine tick frequency */
>>
>> 	return tick_frequency;
>> }
> Note that we also have u32 usec2ticks(u32 usec) and u32 ticks2usec(u32 ticks).
>
> Best regards,
>
> Wolfgang Denk
>

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

On Wed, May 25, 2011 at 5:19 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Graeme Russ,
>
> In message <4DDBE22D.6050806@gmail.com> you wrote:
>>
>> >> Why must get_timer() be used to perform "meaningful time measurement?"
>> >
>> > Excellent question! ?It was never intended to be used as such.
>>
>> Because get_timer() as it currently stands can as it is assumed to return
>> milliseconds
>
> Yes, but without any guarantee for accuracy or resolution.
> This is good enough for timeouts, but nothing for time measurements.

Out of curiosity, are there any platforms that do not use their most
accurate source(*) as the timebase for get_timer()? If a platform is using
it's most accurate, commonly available, source for get_timer() the the
whole accuracy argument is moot - You can't get any better anyway so
why sweat the details.

(*)I'm actually referring to what is commonly available for that platform,
and not where a board has a high precision/accuracy source in addition to
the common source.

As a followup question, how many platforms use two completely independent
sources for udelay() and get_timer() - x86 does, but I plan to change this
so the interrupt kicks the new prescaler which can be done at >> 1ms period
and udelay() and get_timer() will use the same tick source and therefore
have equivalent accuracy.

>> OK, let's wind back - My original suggestion made no claim towards changing
>> what the API is used for, or how it looks to those who use it (for all
>> practical intents and purposes). I suggested:
>> ?- Removing set_timer() and reset_timer()
>> ?- Implement get_timer() as a platform independent function
>
> Trying to remember what I have read in this thread I believe we have
> an agreement on these.
>
>> Exposing ticks and tick_frequency to everyone via a 'tick' HAL
>
> I skip this. ?I don't even read it.

Hmmm, I think it is worthwhile at least comparing the two - What is the
lesser of two evils

 1. Exposing 'ticks' through a HAL for the prescaler
 2. Duplicating a function with identical code 50+ times across the source
    tree

I personally think #2 is way worse - The massive redundant duplication and
blind copying of code is what has get us into this (and many other) messes

>> =======================
>> Not exposing ticks and tick_frequency to everyone
>>
>> In /lib/timer.c
>>
>> void prescaler(u32 ticks, u32 tick_frequency)
>> {
>> ? ? ? u32 current_ms;
>>
>> ? ? ? /* Bill's algorithm */
>>
>> ? ? ? /* result stored in gd->timer_in_ms; */
>> }
>>
>> In /arch/cpu/soc/timer.c or /arch/cpu/timer.c or /board/<board>/timer.c
>>
>> static u32 get_ticks(void)
>
> Currently we have unsigned long long get_ticks(void) ?which is better
> as it matches existing hardware.

Matches PPC - Does it match every other platform? I know it does not match
the sc520 which has a 16-bit millisecond and a 16-bit microsecond counter
(which only counts to 999 before resetting to 0)

Don't assume every platform can implement a 64-bit tick counter. But yes,
we should cater for those platforms that can

>
> Note that we also have void wait_ticks(u32) as needed for udelay().
>
>> static u32 get_tick_frequency(void)
>> {
>> ? ? ? u32 tick_frequency;
>>
>> ? ? ? /* Determine tick frequency */
>>
>> ? ? ? return tick_frequency;
>> }
>
> Note that we also have u32 usec2ticks(u32 usec) and u32 ticks2usec(u32 ticks).

Yes, they are better names

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[J. William Campbell]

On 5/24/2011 5:17 PM, Graeme Russ wrote:
> On Wed, May 25, 2011 at 5:19 AM, Wolfgang Denk<wd@denx.de>  wrote:
>> Dear Graeme Russ,
>>
>> In message<4DDBE22D.6050806@gmail.com>  you wrote:
>>>>> Why must get_timer() be used to perform "meaningful time measurement?"
>>>> Excellent question!  It was never intended to be used as such.
>>> Because get_timer() as it currently stands can as it is assumed to return
>>> milliseconds
>> Yes, but without any guarantee for accuracy or resolution.
>> This is good enough for timeouts, but nothing for time measurements.
> Out of curiosity, are there any platforms that do not use their most
> accurate source(*) as the timebase for get_timer()? If a platform is using
> it's most accurate, commonly available, source for get_timer() the the
> whole accuracy argument is moot - You can't get any better anyway so
> why sweat the details.
Hi All,
        Well, it is not quite that simple. The "accuracy" of the 1 ms 
interrupt rate is controlled in all cases I know about by the resolution 
of the programmable divider used to produce it. It appears that the x86 
uses a 1.19318 MHz crystal oscillator to produce the nominal 1 ms timer 
tick. (There is a typo in line 30 of arch/x86/lib/pcat_timer.c that says 
1.9318. I couldn't make any of the numbers work until I figured this 
out). The tick is produced by dividing the 1.19318 rate999.313 by 1194, 
which produces an interrupt rate of 999.3 Hz, or about 0.068% error. 
However, the performance counter on an x86 is as exact as the crystal 
frequency of the CPU is. FWIW, you can read the performance counter with 
rdtsc on a 386/486 and the CYCLES and CYCLES2 registers on later 
Intel/AMD chips. So yes, there is at least one example of a cpu that 
does not use it's most accurate (or highest resolution) time source.
> (*)I'm actually referring to what is commonly available for that platform,
> and not where a board has a high precision/accuracy source in addition to
> the common source.
>
> As a followup question, how many platforms use two completely independent
> sources for udelay() and get_timer() - x86 does, but I plan to change this
> so the interrupt kicks the new prescaler which can be done at>>  1ms period
> and udelay() and get_timer() will use the same tick source and therefore
> have equivalent accuracy.
Are you sure of this? From what I see in arch/x86/lib/pcat_timer.c, the 
timer 0 is programmed to produce the 1 kHz rate timer tick and is also 
read repeatedly in __udelay to produce the delay value. They even 
preserve the 1194 inaccuracy, for some strange reason. I see that the 
sc520 does appear to use different timers for the interrupt source, and 
it would appear that it may be "exact", but I don't know what the input 
to the prescaler is so I can't be sure. Is the input to the prescaler 
really 8.3 MHz exactly? Also, is the same crystal used for the input to 
the prescaler counter and the "software timer millisecond count". If 
not, then we may have different accuracies in this case as well.

Also of note, it appears that the pcat_timer.c, udelay is not available 
intil interrupts are enabled. That is technically non-compliant, 
although it obviously seems not to matter.

Best Regards,
Bill Campbell
>>> OK, let's wind back - My original suggestion made no claim towards changing
>>> what the API is used for, or how it looks to those who use it (for all
>>> practical intents and purposes). I suggested:
>>>   - Removing set_timer() and reset_timer()
>>>   - Implement get_timer() as a platform independent function
>> Trying to remember what I have read in this thread I believe we have
>> an agreement on these.
>>
>>> Exposing ticks and tick_frequency to everyone via a 'tick' HAL
>> I skip this.  I don't even read it.
> Hmmm, I think it is worthwhile at least comparing the two - What is the
> lesser of two evils
>
>   1. Exposing 'ticks' through a HAL for the prescaler
>   2. Duplicating a function with identical code 50+ times across the source
>      tree
>
> I personally think #2 is way worse - The massive redundant duplication and
> blind copying of code is what has get us into this (and many other) messes
>
>>> =======================
>>> Not exposing ticks and tick_frequency to everyone
>>>
>>> In /lib/timer.c
>>>
>>> void prescaler(u32 ticks, u32 tick_frequency)
>>> {
>>>        u32 current_ms;
>>>
>>>        /* Bill's algorithm */
>>>
>>>        /* result stored in gd->timer_in_ms; */
>>> }
>>>
>>> In /arch/cpu/soc/timer.c or /arch/cpu/timer.c or /board/<board>/timer.c
>>>
>>> static u32 get_ticks(void)
>> Currently we have unsigned long long get_ticks(void)  which is better
>> as it matches existing hardware.
> Matches PPC - Does it match every other platform? I know it does not match
> the sc520 which has a 16-bit millisecond and a 16-bit microsecond counter
> (which only counts to 999 before resetting to 0)
>
> Don't assume every platform can implement a 64-bit tick counter. But yes,
> we should cater for those platforms that can
>
>> Note that we also have void wait_ticks(u32) as needed for udelay().
>>
>>> static u32 get_tick_frequency(void)
>>> {
>>>        u32 tick_frequency;
>>>
>>>        /* Determine tick frequency */
>>>
>>>        return tick_frequency;
>>> }
>> Note that we also have u32 usec2ticks(u32 usec) and u32 ticks2usec(u32 ticks).
> Yes, they are better names
>
> Regards,
>
> Graeme
>
>

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

On Wed, May 25, 2011 at 12:53 PM, J. William Campbell
<jwilliamcampbell@comcast.net> wrote:
> On 5/24/2011 5:17 PM, Graeme Russ wrote:
>>
>> On Wed, May 25, 2011 at 5:19 AM, Wolfgang Denk<wd@denx.de> ?wrote:
>>>
>>> Dear Graeme Russ,
>>>
>>> In message<4DDBE22D.6050806@gmail.com> ?you wrote:
>>>>>>
>>>>>> Why must get_timer() be used to perform "meaningful time measurement?"
>>>>>
>>>>> Excellent question! ?It was never intended to be used as such.
>>>>
>>>> Because get_timer() as it currently stands can as it is assumed to
>>>> return
>>>> milliseconds
>>>
>>> Yes, but without any guarantee for accuracy or resolution.
>>> This is good enough for timeouts, but nothing for time measurements.
>>
>> Out of curiosity, are there any platforms that do not use their most
>> accurate source(*) as the timebase for get_timer()? If a platform is using
>> it's most accurate, commonly available, source for get_timer() the the
>> whole accuracy argument is moot - You can't get any better anyway so
>> why sweat the details.
>
> Hi All,
> ? ? ? Well, it is not quite that simple. The "accuracy" of the 1 ms
> interrupt rate is controlled in all cases I know about by the resolution of
> the programmable divider used to produce it. It appears that the x86 uses a
> 1.19318 MHz crystal oscillator to produce the nominal 1 ms timer tick.
> (There is a typo in line 30 of arch/x86/lib/pcat_timer.c that says 1.9318. I

Thanks, I will fix that (although pcat_timer.c is not used by any current x86
board)

> couldn't make any of the numbers work until I figured this out). The tick is
> produced by dividing the 1.19318 rate999.313 by 1194, which produces an
> interrupt rate of 999.3 Hz, or about 0.068% error. However, the performance
> counter on an x86 is as exact as the crystal frequency of the CPU is. FWIW,
> you can read the performance counter with rdtsc on a 386/486 and the CYCLES

Hmm, I hadn't thought of that

> and CYCLES2 registers on later Intel/AMD chips. So yes, there is at least
> one example of a cpu that does not use it's most accurate (or highest
> resolution) time source.
>>
>> (*)I'm actually referring to what is commonly available for that platform,
>> and not where a board has a high precision/accuracy source in addition to
>> the common source.
>>
>> As a followup question, how many platforms use two completely independent
>> sources for udelay() and get_timer() - x86 does, but I plan to change this
>> so the interrupt kicks the new prescaler which can be done at>> ?1ms
>> period
>> and udelay() and get_timer() will use the same tick source and therefore
>> have equivalent accuracy.
>
> Are you sure of this? From what I see in arch/x86/lib/pcat_timer.c, the

Well, the only x86 board is an sc520 and does not used pcat_timer.c :)

Look in arch/x86/cpu/sc520/ and you might get a better picture

> timer 0 is programmed to produce the 1 kHz rate timer tick and is also read
> repeatedly in __udelay to produce the delay value. They even preserve the
> 1194 inaccuracy, for some strange reason. I see that the sc520 does appear
> to use different timers for the interrupt source, and it would appear that

Ah, you did ;)

> it may be "exact", but I don't know what the input to the prescaler is so I
> can't be sure. Is the input to the prescaler really 8.3 MHz exactly? Also,
> is the same crystal used for the input to the prescaler counter and the
> "software timer millisecond count". If not, then we may have different
> accuracies in this case as well.

Yes, they are both derived from the onboard xtal which can be either
33.000MHz or 33.333MHz (there is a system register that must be written
to to tell the sc520 what crystal is installed)

> Also of note, it appears that the pcat_timer.c, udelay is not available
> intil interrupts are enabled. That is technically non-compliant, although it
> obviously seems not to matter.

OK, it looks like x86 needs a bit of a timer overhaul - Thanks for the
heads-up

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear "J. William Campbell",

In message <4DDC31EB.6040609@comcast.net> you wrote:
...
> A tick is defined as the smallest increment of system time as measured by a
> computer system (seehttp://en.wikipedia.org/wiki/System_time):
> 
> 	System time is measured by a system clock, which is typically
> 	implemented as a simple count of the number of ticks that have
> 	transpired since some arbitrary starting date, called the
> 	epoch.
> 
> Unfortunately, this definition is obsolete, and has been for quite some 

Do you have any proof for such a claim?

> years.  When computers had a single timer, the above definition worked, 
> but it no longer does, as many (most?) computers have several hardware 
> timers. A "tick" today is the time increment of any particular timer of 
> a computer system. So, when one writes a function called get_ticks on a 
> PPC, does one mean read the decrementer, or does one read the RTC or 
> does one read the TB register(s) A similar situation exists on the 
> Blackfin BF531/2/3, that has a preformance counter, a real-time clock, 
> and three programmable timers. Which tick do you want? For each u-boot 

Please re-read the definition.  At least as far as U-Boot and Linux
are concerned, there is only a single clock source used to implement
the _system_time_.  And I doubt that other OS do different.

> implementation, we can pick one timer as the "master" timer, but it may 
> not be the one with the most rapid tick rate. It may be the one with the 
> most USEFUL tick rate for get_timer. If you take the above definition at 
> face value, only the fastest counter value has ticks, and all other 
> counters time increments are not ticks. If they are not ticks, what are 
> they?

Clocks, timers?


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
Don't put off for tomorrow what you can  do  today,  because  if  you
enjoy it today you can do it again tomorrow.

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear Graeme Russ,

In message <BANLkTinK+bxtZ+sumYZtvuHcd_Ft+jfyPw@mail.gmail.com> you wrote:
>
> > Yes, but without any guarantee for accuracy or resolution.
> > This is good enough for timeouts, but nothing for time measurements.
> 
> Out of curiosity, are there any platforms that do not use their most
> accurate source(*) as the timebase for get_timer()? If a platform is using
> it's most accurate, commonly available, source for get_timer() the the
> whole accuracy argument is moot - You can't get any better anyway so
> why sweat the details.

Actually most boards do that, at least most of those who have a RTC
chip on board.  Almost always these RTCs provide much better accuracy
than the system clock used for the CPU-internal timers.  In an OS like
Linux you will frequently see NTP being used to keep the system time
in sync with the more accurate RTC (and eventually other, external
time sources).

> (*)I'm actually referring to what is commonly available for that platform,
> and not where a board has a high precision/accuracy source in addition to
> the common source.

I'm not sure what you mean here.  Quite frequently even SoCs have a
number of different clock sources, say a master clock which is used to
derive the processor clock, and a 32768 Hz clock which is used to
drive a SoC-internal RTC.

> As a followup question, how many platforms use two completely independent
> sources for udelay() and get_timer() - x86 does, but I plan to change this
> so the interrupt kicks the new prescaler which can be done at >> 1ms period
> and udelay() and get_timer() will use the same tick source and therefore
> have equivalent accuracy.

Define "completely independent" - on PowerPC one uses the timebase
register, while the other uses the decrementer. But in the end, both
of them are most likely derived from the same system clock (or not
- depending on SoC design).  But this does not matter at all.

> Hmmm, I think it is worthwhile at least comparing the two - What is the
> lesser of two evils
> 
>  1. Exposing 'ticks' through a HAL for the prescaler
>  2. Duplicating a function with identical code 50+ times across the source
>     tree

If it's identical code, all these implementations can share common
code, right?

> I personally think #2 is way worse - The massive redundant duplication and
> blind copying of code is what has get us into this (and many other) messes

That don;t do it, and use common code for this.

> > Currently we have unsigned long long get_ticks(void) =A0which is better
> > as it matches existing hardware.
> 
> Matches PPC - Does it match every other platform? I know it does not match
> the sc520 which has a 16-bit millisecond and a 16-bit microsecond counter
> (which only counts to 999 before resetting to 0)
> 
> Don't assume every platform can implement a 64-bit tick counter. But yes,
> we should cater for those platforms that can

There are counters of many different sizes around. You cannot squeeze
a bigger counter into a smaller data ubit, but you can easily stick
smaller data into a bigger container. So it seems most natural to me
to chose that data type that fits all, i. e. a bigger one.


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
When in doubt, mumble;   when in trouble, delegate;  when in  charge,
ponder.                                             -- James H. Boren

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear "J. William Campbell",

In message <4DDC6F46.4090809@comcast.net> you wrote:
>
>         Well, it is not quite that simple. The "accuracy" of the 1 ms 
> interrupt rate is controlled in all cases I know about by the resolution 
> of the programmable divider used to produce it. It appears that the x86 

I mentioned a couple of times before that accuracy has never been a
design criterion.  Please also keep in mind that there are areas in
the code where we block interrupts, and we make zero efforts to detect
or compensate for this in the timer code.  This simply is not
important.  We are not talking about ppm here, we are talking about
somting that is just good enough for what it is being used, and that
allows easily for +/- 10% tolerances.

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
"We don't care.  We don't have to.  We're the Phone Company."

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

Hi Wolfgang

On Wed, May 25, 2011 at 3:25 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Graeme Russ,
>
> In message <BANLkTinK+bxtZ+sumYZtvuHcd_Ft+jfyPw@mail.gmail.com> you wrote:
>>
>> > Yes, but without any guarantee for accuracy or resolution.
>> > This is good enough for timeouts, but nothing for time measurements.
>>
>> Out of curiosity, are there any platforms that do not use their most
>> accurate source(*) as the timebase for get_timer()? If a platform is using
>> it's most accurate, commonly available, source for get_timer() the the
>> whole accuracy argument is moot - You can't get any better anyway so
>> why sweat the details.
>
> Actually most boards do that, at least most of those who have a RTC
> chip on board. ?Almost always these RTCs provide much better accuracy
> than the system clock used for the CPU-internal timers. ?In an OS like
> Linux you will frequently see NTP being used to keep the system time
> in sync with the more accurate RTC (and eventually other, external
> time sources)

Well, I think we can leave NTP out of the picture for U-Boot - What I am
really getting at is are the time bases for udelay() and get_timer() the
same, and are they always the most accurate which is readily available
to U-Boot.

I am thinking the answer is not always 'Yes' as udelay() is needed before
relocation and therefore may only have access to a poor-man's time base
(using port-mapped I/O to force wait-states on the x86 for example)

>> (*)I'm actually referring to what is commonly available for that platform,
>> and not where a board has a high precision/accuracy source in addition to
>> the common source.
>
> I'm not sure what you mean here. ?Quite frequently even SoCs have a
> number of different clock sources, say a master clock which is used to
> derive the processor clock, and a 32768 Hz clock which is used to
> drive a SoC-internal RTC.

The eNET board for example has a microsecond counter in the FPGA which is
independently clocked to the CPU

>> As a followup question, how many platforms use two completely independent
>> sources for udelay() and get_timer() - x86 does, but I plan to change this
>> so the interrupt kicks the new prescaler which can be done at >> 1ms period
>> and udelay() and get_timer() will use the same tick source and therefore
>> have equivalent accuracy.
>
> Define "completely independent" - on PowerPC one uses the timebase
> register, while the other uses the decrementer. But in the end, both
> of them are most likely derived from the same system clock (or not
> - depending on SoC design). ?But this does not matter at all.

I mean udelay() clocked by a tick counter and get_timer() clocked by
an interrupt versus both being clocked by the same phyiscal tick counter
(even though both are clocked by the same on-board xtal)

>> Hmmm, I think it is worthwhile at least comparing the two - What is the
>> lesser of two evils
>>
>> ?1. Exposing 'ticks' through a HAL for the prescaler
>> ?2. Duplicating a function with identical code 50+ times across the source
>> ? ? tree
>
> If it's identical code, all these implementations can share common
> code, right?

Yes

>> I personally think #2 is way worse - The massive redundant duplication and
>> blind copying of code is what has get us into this (and many other) messes
>
> That don;t do it, and use common code for this.

But to do that, I need to make get_ticks() non static so the prescaler can
access the tick counter

>> > Currently we have unsigned long long get_ticks(void) =A0which is better
>> > as it matches existing hardware.
>>
>> Matches PPC - Does it match every other platform? I know it does not match
>> the sc520 which has a 16-bit millisecond and a 16-bit microsecond counter
>> (which only counts to 999 before resetting to 0)
>>
>> Don't assume every platform can implement a 64-bit tick counter. But yes,
>> we should cater for those platforms that can
>
> There are counters of many different sizes around. You cannot squeeze
> a bigger counter into a smaller data ubit, but you can easily stick
> smaller data into a bigger container. So it seems most natural to me
> to chose that data type that fits all, i. e. a bigger one.

Putting the smaller counter into a larger bit space causes problems because
it wraps to "All 0's" before it gets to "All 1's" - This makes a simple
unsigned 'current - last' calculation fail at the rollover point. The same
problem does not occur if you truncate a larger counter into a smaller bit
space

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

Hi Wolfgang,

On Wed, May 25, 2011 at 3:28 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear "J. William Campbell",
>
> In message <4DDC6F46.4090809@comcast.net> you wrote:
>>
>> ? ? ? ? Well, it is not quite that simple. The "accuracy" of the 1 ms
>> interrupt rate is controlled in all cases I know about by the resolution
>> of the programmable divider used to produce it. It appears that the x86
>
> I mentioned a couple of times before that accuracy has never been a
> design criterion. ?Please also keep in mind that there are areas in
> the code where we block interrupts, and we make zero efforts to detect
> or compensate for this in the timer code. ?This simply is not
> important. ?We are not talking about ppm here, we are talking about
> somting that is just good enough for what it is being used, and that
> allows easily for +/- 10% tolerances.

With the prescaler design we are discussing, as long as we have access to
a free-running tick counter, it does not matter one iota if the interrupt
is delayed by one tick, a hundred ticks or a thousand ticks privided it
eventually happens within the roll-over interval of the tick counter

And if get_timer() is called in the meantime, the roll-over interval gets
reset

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear Graeme Russ,

In message <BANLkTi==emvE+2Xu-R=hzh3TgFJXs9DnmA@mail.gmail.com> you wrote:
> 
> Well, I think we can leave NTP out of the picture for U-Boot - What I am
> really getting at is are the time bases for udelay() and get_timer() the
> same, and are they always the most accurate which is readily available
> to U-Boot.

Please assume "no" to both questions.

Even more important: stop worrying about this.  The implementation is
noit supposed to make ANY such assumptions.

> I mean udelay() clocked by a tick counter and get_timer() clocked by
> an interrupt versus both being clocked by the same phyiscal tick counter
> (even though both are clocked by the same on-board xtal)

You really should not care about those low-level details.  They can
and shall be ignored.

> > There are counters of many different sizes around. You cannot squeeze
> > a bigger counter into a smaller data ubit, but you can easily stick
> > smaller data into a bigger container. So it seems most natural to me
> > to chose that data type that fits all, i. e. a bigger one.
> 
> Putting the smaller counter into a larger bit space causes problems because
> it wraps to "All 0's" before it gets to "All 1's" - This makes a simple
> unsigned 'current - last' calculation fail at the rollover point. The same
> problem does not occur if you truncate a larger counter into a smaller bit
> space

But then you may significantly reduce the available time-span; you may
see the counter wrapping around every few seconds. You will not be
able to implement - say - a 10 second delay because you cannot know if
there were 1, 2, or more rollovers inbetween.


I see two options:

1) either we can have some signal when the timer wrapes around (like
   an interrupt); in this case it does not matter how big the counter
   itself is, we just increment our own local 64 bit timestamp.

2) Alternatively, I could imagine we use a 64 bit container plus a
   mask which specifiec how many bits are actually used - then we can 
   accommodate counters with any numbers of valid bits.



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
PUBLIC NOTICE AS REQUIRED BY LAW:  Any Use of This  Product,  in  Any
Manner  Whatsoever,  Will  Increase  the  Amount  of  Disorder in the
Universe. Although No Liability Is Implied Herein,  the  Consumer  Is
Warned  That  This  Process Will Ultimately Lead to the Heat Death of
the Universe.

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear Graeme Russ,

In message <BANLkTinZ_q8_vqd9mzm7BY64Hi=zuyFZTQ@mail.gmail.com> you wrote:
> 
> With the prescaler design we are discussing, as long as we have access to
> a free-running tick counter, it does not matter one iota if the interrupt
> is delayed by one tick, a hundred ticks or a thousand ticks privided it
> eventually happens within the roll-over interval of the tick counter

I don't get you.  In such a system, the interrupt would be the tick
(see the PPC implementation).  If you miss interrupts, you miss ticks.

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
Real computer scientists don't comment their  code.  The  identifiers
are so long they can't afford the disk space.

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

On 25/05/11 18:06, Wolfgang Denk wrote:
> Dear Graeme Russ,
> 
> In message <BANLkTi==emvE+2Xu-R=hzh3TgFJXs9DnmA@mail.gmail.com> you wrote:
>>
>> Well, I think we can leave NTP out of the picture for U-Boot - What I am
>> really getting at is are the time bases for udelay() and get_timer() the
>> same, and are they always the most accurate which is readily available
>> to U-Boot.
> 
> Please assume "no" to both questions.

Thought so

> 
> Even more important: stop worrying about this.  The implementation is
> noit supposed to make ANY such assumptions.

I'm not really worrying about from the perspective of the new timer API,
it's idle curiosity and getting a better understanding

>> I mean udelay() clocked by a tick counter and get_timer() clocked by
>> an interrupt versus both being clocked by the same phyiscal tick counter
>> (even though both are clocked by the same on-board xtal)
> 
> You really should not care about those low-level details.  They can
> and shall be ignored.

Yes, but you find that there is another level of simplification available...

>>> There are counters of many different sizes around. You cannot squeeze
>>> a bigger counter into a smaller data ubit, but you can easily stick
>>> smaller data into a bigger container. So it seems most natural to me
>>> to chose that data type that fits all, i. e. a bigger one.
>>
>> Putting the smaller counter into a larger bit space causes problems because
>> it wraps to "All 0's" before it gets to "All 1's" - This makes a simple
>> unsigned 'current - last' calculation fail at the rollover point. The same
>> problem does not occur if you truncate a larger counter into a smaller bit
>> space
> 
> But then you may significantly reduce the available time-span; you may
> see the counter wrapping around every few seconds. You will not be
> able to implement - say - a 10 second delay because you cannot know if
> there were 1, 2, or more rollovers inbetween.
> 
> 
> I see two options:
> 
> 1) either we can have some signal when the timer wrapes around (like
>    an interrupt); in this case it does not matter how big the counter
>    itself is, we just increment our own local 64 bit timestamp.
> 
> 2) Alternatively, I could imagine we use a 64 bit container plus a
>    mask which specifiec how many bits are actually used - then we can 
>    accommodate counters with any numbers of valid bits.
> 

3) Keep calling get_timer() (which you do when checking protocol time-outs)
- This keeps kicking the prescaler

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

On 25/05/11 18:08, Wolfgang Denk wrote:
> Dear Graeme Russ,
> 
> In message <BANLkTinZ_q8_vqd9mzm7BY64Hi=zuyFZTQ@mail.gmail.com> you wrote:
>>
>> With the prescaler design we are discussing, as long as we have access to
>> a free-running tick counter, it does not matter one iota if the interrupt
>> is delayed by one tick, a hundred ticks or a thousand ticks privided it
>> eventually happens within the roll-over interval of the tick counter
> 
> I don't get you.  In such a system, the interrupt would be the tick
> (see the PPC implementation).  If you miss interrupts, you miss ticks.

Yes, you miss ticks, but if the hardware is keeping the tick counter
current in the background (i.e. without software or interrupts) then this
does not matter - The prescaler takes care of this...

The key is the prescaler takes a 'tick delta' between the last time it was
called and now, uses the 'tick frequency' to calculate a corresponding
'timer delta' which it adds to the current timer

So the frequency of calling the prescaler _does not matter_ (it does not
even need to be a regular frequency) provided the tick-counter does not do
a complete wrap (at 4GHz this is 4 seconds for a 32-bit counter) - So as
long as you call the prescaler at least every 4 seconds (and get_timer()
calls the prescaler) then you will never have a timer glitch in your timing
loop.

This relies on the tick counter wrapping properly.

So, if your tick counter is accurate, your timer is accurate even if you
are using an ISR to call the prescaler and temporarily disable interrupts

How many platforms have no hardware counter whatsoever? - These will have
to implement the timer using interrupts and suffer the accuracy loss when
interrupts are disabled (and how would these implement udelay() anyway -
hard loops I guess)

And now for the kicker - If your tick counter is microsecond resolution or
better, you can maintain microsecond, millisecond (second, minute etc)
timers trivially with a new prescaler

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear Graeme Russ,

In message <4DDCBD3D.4080409@gmail.com> you wrote:
>
> > I see two options:
> > 
> > 1) either we can have some signal when the timer wrapes around (like
> >    an interrupt); in this case it does not matter how big the counter
> >    itself is, we just increment our own local 64 bit timestamp.
> > 
> > 2) Alternatively, I could imagine we use a 64 bit container plus a
> >    mask which specifiec how many bits are actually used - then we can 
> >    accommodate counters with any numbers of valid bits.
> > 
> 
> 3) Keep calling get_timer() (which you do when checking protocol time-outs)
> - This keeps kicking the prescaler

We cannot guarantee for this, so this is not really an option.

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
A door is what a dog is perpetually on the wrong side of.
                                                        - Ogden Nash

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear Graeme Russ,

In message <4DDCBFF4.40002@gmail.com> you wrote:
>
> > I don't get you.  In such a system, the interrupt would be the tick
> > (see the PPC implementation).  If you miss interrupts, you miss ticks.
> 
> Yes, you miss ticks, but if the hardware is keeping the tick counter
> current in the background (i.e. without software or interrupts) then this
> does not matter - The prescaler takes care of this...

I think you don't want to understand.

Look at the PPC implementation.  The decrementer causes an interrupt
every millisecond.  This is our tick.  We never ever actually read the
decrementer register itself.  We just use the interrupt.

> The key is the prescaler takes a 'tick delta' between the last time it was
> called and now, uses the 'tick frequency' to calculate a corresponding
> 'timer delta' which it adds to the current timer

The "tick frequency" in above example is 1,000 Hz, and determined by
the frequency of the interrupts.  There is not any "timer count
register" we are accessing here.

> This relies on the tick counter wrapping properly.

The "tick counter" is already in the higher level, i. e. implemented
in software, without any hardware based registers.

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
Diplomacy is the art of saying "nice doggy" until you can find a rock.

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

Hi Wolfgang,

On 25/05/11 21:37, Wolfgang Denk wrote:
> Dear Graeme Russ,
> 
> In message <4DDCBFF4.40002@gmail.com> you wrote:
>>
>>> I don't get you.  In such a system, the interrupt would be the tick
>>> (see the PPC implementation).  If you miss interrupts, you miss ticks.
>>
>> Yes, you miss ticks, but if the hardware is keeping the tick counter
>> current in the background (i.e. without software or interrupts) then this
>> does not matter - The prescaler takes care of this...
> 
> I think you don't want to understand.
> 
> Look at the PPC implementation.  The decrementer causes an interrupt
> every millisecond.  This is our tick.  We never ever actually read the
> decrementer register itself.  We just use the interrupt.

x86 is the same - a 1ms interrupt which increments the timer

>> The key is the prescaler takes a 'tick delta' between the last time it was
>> called and now, uses the 'tick frequency' to calculate a corresponding
>> 'timer delta' which it adds to the current timer
> 
> The "tick frequency" in above example is 1,000 Hz, and determined by
> the frequency of the interrupts.  There is not any "timer count
> register" we are accessing here.
> 
>> This relies on the tick counter wrapping properly.
> 
> The "tick counter" is already in the higher level, i. e. implemented
> in software, without any hardware based registers.

And in the PPC case - No prescaler and life is easy

BUT - I just found something very disturbing in /board/zylonite/flash.c -
Interrupts are disables before the timer is used to detect a timeout!

Yes, this is in a board specific flash driver which is bad but who knows
where else this might (quite unintentionally) occur

No if you have:

	start = get_timer(0)

	while (hardware_busy) {
		if (get_timer(start) > TIMEOUT) {
			/* deal with timeout */
			break;
		}
		/* Do stuf (poll hardware etc) */
	}

Now every time get_timer() is called, the prescaler updates the timer from
the current tick count - This cannot fail if interrupts are inadvertently
disabled.

And if you do not care about the timer updating while you are not 'using
it' (i.e. checking for a timeout) - it doesn't really matter - You will get
a glitch, but this glitch is 'reset' by the call to get_timer() and your
timeout check starts a new timer increment afresh

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

Hi Wolfgang,

On 25/05/11 21:32, Wolfgang Denk wrote:
> Dear Graeme Russ,
> 
> In message <4DDCBD3D.4080409@gmail.com> you wrote:
>>
>>> I see two options:
>>>
>>> 1) either we can have some signal when the timer wrapes around (like
>>>    an interrupt); in this case it does not matter how big the counter
>>>    itself is, we just increment our own local 64 bit timestamp.
>>>
>>> 2) Alternatively, I could imagine we use a 64 bit container plus a
>>>    mask which specifiec how many bits are actually used - then we can 
>>>    accommodate counters with any numbers of valid bits.
>>>
>>
>> 3) Keep calling get_timer() (which you do when checking protocol time-outs)
>> - This keeps kicking the prescaler
> 
> We cannot guarantee for this, so this is not really an option.
> 

You can when you are waiting for a timeout

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear Graeme Russ,

In message <4DDCED75.8030906@gmail.com> you wrote:
> 
> >>> I don't get you.  In such a system, the interrupt would be the tick
> >>> (see the PPC implementation).  If you miss interrupts, you miss ticks.
...
> > The "tick counter" is already in the higher level, i. e. implemented
> > in software, without any hardware based registers.
...

> Now every time get_timer() is called, the prescaler updates the timer from
> the current tick count - This cannot fail if interrupts are inadvertently
> disabled.

Arghh... But in this szenario, if interrupts are disabled, then the
ISR will not run, so the tick count will not be incremented BECAUSE
THE TICK COUNTER IS A VARIABLE THAT GETS INCREMENTED BY ONE EACH TIME
THE ISR RUNS.  So your prescaler will read a constant value.

Sorry for shouting, but I feel either you are not listening to me or I
cannot make myself understood to you.

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
Violence in reality is quite different from theory.
	-- Spock, "The Cloud Minders", stardate 5818.4

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear Graeme Russ,

In message <4DDCEDA1.2010101@gmail.com> you wrote:
> 
> >> 3) Keep calling get_timer() (which you do when checking protocol time-outs)
> >> - This keeps kicking the prescaler
> > 
> > We cannot guarantee for this, so this is not really an option.
> 
> You can when you are waiting for a timeout

We may not be waiting for any timeouts for arbitrary long times.

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
Men will always be men -- no matter where they are.
	-- Harry Mudd, "Mudd's Women", stardate 1329.8

[U-Boot] [RFC] Review of U-Boot timer API

[Scott McNutt]

Graeme Russ wrote:
> OK, let's wind back - My original suggestion made no claim towards changing
> what the API is used for, or how it looks to those who use it (for all
> practical intents and purposes). I suggested:
>  - Removing set_timer() and reset_timer()
>  - Implement get_timer() as a platform independent function

Why do you suggest removing set_timer() and reset_timer() ?

--Scott

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

Hi Wolfgang,

On 25/05/11 22:26, Wolfgang Denk wrote:
> Dear Graeme Russ,
> 
> In message <4DDCED75.8030906@gmail.com> you wrote:
>>
>>>>> I don't get you.  In such a system, the interrupt would be the tick
>>>>> (see the PPC implementation).  If you miss interrupts, you miss ticks.
> ...
>>> The "tick counter" is already in the higher level, i. e. implemented
>>> in software, without any hardware based registers.
> ...
> 
>> Now every time get_timer() is called, the prescaler updates the timer from
>> the current tick count - This cannot fail if interrupts are inadvertently
>> disabled.
> 
> Arghh... But in this szenario, if interrupts are disabled, then the
> ISR will not run, so the tick count will not be incremented BECAUSE
> THE TICK COUNTER IS A VARIABLE THAT GETS INCREMENTED BY ONE EACH TIME
> THE ISR RUNS.  So your prescaler will read a constant value.
> 
> Sorry for shouting, but I feel either you are not listening to me or I
> cannot make myself understood to you.

What you are saying is 100% correct - For PPC. Now please - step back from
PPC for a minute

Many platforms have a hardware counter which is incremented without
assistance from software - not by an ISR, nothing (I mentioned this before,
but I can't track down the exact email) - THIS is the tick counter returned
by get_ticks() which the prescaler uses to update the timer when you either:

 - Explicitly call get_timer()
 - Call the prescaler from an ISR

The thing is, I have now highlighted that relying on interrupts to keep
get_timer() working is a really bad idea - We have no way of stopping
anyone from calling disable_interrupts() before using the timer

The suggested architecture makes ISR servicing of the timer 'nice' for long
delays where the tick counter might wrap and 'always works' for timeout
loops which repeatedly call get_timer()

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

Hi Scott,

On 25/05/11 22:36, Scott McNutt wrote:
> Graeme Russ wrote:
>> OK, let's wind back - My original suggestion made no claim towards changing
>> what the API is used for, or how it looks to those who use it (for all
>> practical intents and purposes). I suggested:
>>  - Removing set_timer() and reset_timer()
>>  - Implement get_timer() as a platform independent function
> 
> Why do you suggest removing set_timer() and reset_timer() ?
> 

Because if the timer API is done right, they are not needed

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear Graeme Russ,

In message <4DDCF926.1050904@gmail.com> you wrote:
> 
> Many platforms have a hardware counter which is incremented without
> assistance from software - not by an ISR, nothing (I mentioned this before,
> but I can't track down the exact email) - THIS is the tick counter returned
> by get_ticks() which the prescaler uses to update the timer when you either:
> 
>  - Explicitly call get_timer()
>  - Call the prescaler from an ISR
> 
> The thing is, I have now highlighted that relying on interrupts to keep
> get_timer() working is a really bad idea - We have no way of stopping
> anyone from calling disable_interrupts() before using the timer

There may be code which doe snot call get_timer() for _long_ times.
And somebody disables interrupts. What's then?

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
Your csh still thinks true is false. Write to your vendor  today  and
tell them that next year Configure ought to "rm /bin/csh" unless they
fix  their blasted shell. :-)
                 - Larry Wall in Configure from the perl distribution

[U-Boot] [RFC] Review of U-Boot timer API

[Scott McNutt]

Graeme Russ wrote:
> Hi Scott,
> 
> On 25/05/11 22:36, Scott McNutt wrote:
>> Graeme Russ wrote:
>>> OK, let's wind back - My original suggestion made no claim towards changing
>>> what the API is used for, or how it looks to those who use it (for all
>>> practical intents and purposes). I suggested:
>>>  - Removing set_timer() and reset_timer()
>>>  - Implement get_timer() as a platform independent function
>> Why do you suggest removing set_timer() and reset_timer() ?
>>
> 
> Because if the timer API is done right, they are not needed

To continue the wind back ...

In several implementations, reset_timer() actually reloads
or re-initializes the hardware timer. This has the effect of
synchronizing get_timer() calls with subsequent interrupts.
This prevents the early timeouts if the implementer chooses
to use an interrupt rate less than 1 ms.

So my original question was, how do we address this issue?

--Scott

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

On Wednesday, May 25, 2011, Wolfgang Denk <wd@denx.de> wrote:
> Dear Graeme Russ,
>
> In message <4DDCF926.1050904@gmail.com> you wrote:
>>
>> Many platforms have a hardware counter which is incremented without
>> assistance from software - not by an ISR, nothing (I mentioned this before,
>> but I can't track down the exact email) - THIS is the tick counter returned
>> by get_ticks() which the prescaler uses to update the timer when you either:
>>
>> ?- Explicitly call get_timer()
>> ?- Call the prescaler from an ISR
>>
>> The thing is, I have now highlighted that relying on interrupts to keep
>> get_timer() working is a really bad idea - We have no way of stopping
>> anyone from calling disable_interrupts() before using the timer
>
> There may be code which doe snot call get_timer() for _long_ times.
> And somebody disables interrupts. What's then?
>
We're hosed, but at least we are one better than relying on interrupts alone ;)

And what about platforms that do not support interrupts? Are you
mandating that get_timer() MUST always be supported by a timer updated
by an ISR?

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

Hi Scott,

On Wednesday, May 25, 2011, Scott McNutt <smcnutt@psyent.com> wrote:
> Graeme Russ wrote:
>
> Hi Scott,
>
> On 25/05/11 22:36, Scott McNutt wrote:
>
> Graeme Russ wrote:
>
> OK, let's wind back - My original suggestion made no claim towards changing
> what the API is used for, or how it looks to those who use it (for all
> practical intents and purposes). I suggested:
> ?- Removing set_timer() and reset_timer()
> ?- Implement get_timer() as a platform independent function
>
> Why do you suggest removing set_timer() and reset_timer() ?
>
>
>
> Because if the timer API is done right, they are not needed
>
>
> To continue the wind back ...
>
> In several implementations, reset_timer() actually reloads
> or re-initializes the hardware timer. This has the effect of
> synchronizing get_timer() calls with subsequent interrupts.
> This prevents the early timeouts if the implementer chooses
> to use an interrupt rate less than 1 ms.
>
> So my original question was, how do we address this issue?

We fix them dear Liza dear Liza ;)

That is what this thread is trying to figure out

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear Graeme Russ,

In message <BANLkTimhtqYNWgvwr6w8kNOSV4U_tZs7yw@mail.gmail.com> you wrote:
>
> And what about platforms that do not support interrupts? Are you
> mandating that get_timer() MUST always be supported by a timer updated
> by an ISR?

No, not at all. And I already answered this. For example on PPC, just
reading the timebase would be perfectly sufficient, and simpler and
more reliable than the current interrupt based approach.

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
Ein weiser Herrscher kann in einem gro?en Land  mehr  Gutes  bewirken
als  in  einem kleinen - ein dummer Herrscher aber auch viel mehr Un-
fug. Da weise Herrscher seltener sind als dumme, war ich schon  immer
gegen gro?e Reiche skeptisch.                   - Herbert Rosendorfer

[U-Boot] [RFC] Review of U-Boot timer API

[Scott McNutt]

Graeme Russ wrote:
> Hi Scott,
> 
> On Wednesday, May 25, 2011, Scott McNutt <smcnutt@psyent.com> wrote:
>> Graeme Russ wrote:
>>
>> Hi Scott,
>>
>> On 25/05/11 22:36, Scott McNutt wrote:
>>
>> Graeme Russ wrote:
>>
>> OK, let's wind back - My original suggestion made no claim towards changing
>> what the API is used for, or how it looks to those who use it (for all
>> practical intents and purposes). I suggested:
>>  - Removing set_timer() and reset_timer()
>>  - Implement get_timer() as a platform independent function
>>
>> Why do you suggest removing set_timer() and reset_timer() ?
>>
>>
>>
>> Because if the timer API is done right, they are not needed
>>
>>
>> To continue the wind back ...
>>
>> In several implementations, reset_timer() actually reloads
>> or re-initializes the hardware timer. This has the effect of
>> synchronizing get_timer() calls with subsequent interrupts.
>> This prevents the early timeouts if the implementer chooses
>> to use an interrupt rate less than 1 ms.
>>
>> So my original question was, how do we address this issue?
> 
> We fix them dear Liza dear Liza ;)
> 
> That is what this thread is trying to figure out

I'm getting a bit frustrated here as I can see we're going
in circles. So I'll just make a few statements a leave well
enough alone.

- I'm not convinced that reset_timer() and set_timer() should be
removed simply because you state that "if the timer API is done
right, they are not needed."

- If an implementation can't use interrupts that occur less
frequently than 1 msec to satisfy the prevailing use of
get_timer(), without breaking existing capability, I consider
the interface that "is done right" to be wrong.

Best Regards,
--Scott

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

Hi Scott,

On Wednesday, May 25, 2011, Scott McNutt <smcnutt@psyent.com> wrote:
> Graeme Russ wrote:
>
> Hi Scott,
>
> On Wednesday, May 25, 2011, Scott McNutt <smcnutt@psyent.com> wrote:
>
> Graeme Russ wrote:
>
> Hi Scott,
>
> On 25/05/11 22:36, Scott McNutt wrote:
>
> Graeme Russ wrote:
>
> OK, let's wind back - My original suggestion made no claim towards changing
> what the API is used for, or how it looks to those who use it (for all
> practical intents and purposes). I suggested:
> ?- Removing set_timer() and reset_timer()
> ?- Implement get_timer() as a platform independent function
>
> Why do you suggest removing set_timer() and reset_timer() ?
>
>
>
> Because if the timer API is done right, they are not needed
>
>
> To continue the wind back ...
>
> In several implementations, reset_timer() actually reloads
> or re-initializes the hardware timer. This has the effect of
> synchronizing get_timer() calls with subsequent interrupts.
> This prevents the early timeouts if the implementer chooses
> to use an interrupt rate less than 1 ms.
>
> So my original question was, how do we address this issue?
>
>
> We fix them dear Liza dear Liza ;)
>
> That is what this thread is trying to figure out
>
>
> I'm getting a bit frustrated here as I can see we're going
> in circles. So I'll just make a few statements a leave well
> enough alone.
>
> - I'm not convinced that reset_timer() and set_timer() should be
> removed simply because you state that "if the timer API is done
> right, they are not needed."

Sorry, I should have been more specific - get and reset break nested
loops. Some implementations (mostly ARM) also do a silent reset in
udelay(), which breaks loops containing udelay.

>
> - If an implementation can't use interrupts that occur less
> frequently than 1 msec to satisfy the prevailing use of
> get_timer(), without breaking existing capability, I consider
> the interface that "is done right" to be wrong.
>
Sorry, I don't understand what you mean here

I hope to get an implementation agreed upon that does not require
interrupts at all, provided a tick counter with sufficiently long roll
over time is available (longer than the maximum expected period
between 'related' get_timer() calls, for example calls to get_timer()
in a timeout testing while loop). This 'bare minimum' implementation
can be optionally augmented with an ISR which kicks the timer
calculation in the background (typically by just calling get_timer())

It really is quite simple in the end.

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[J. William Campbell]

On 5/24/2011 10:17 PM, Wolfgang Denk wrote:
> Dear "J. William Campbell",
>
> In message<4DDC31EB.6040609@comcast.net>  you wrote:
> ...
>> A tick is defined as the smallest increment of system time as measured by a
>> computer system (seehttp://en.wikipedia.org/wiki/System_time):
>>
>> 	System time is measured by a system clock, which is typically
>> 	implemented as a simple count of the number of ticks that have
>> 	transpired since some arbitrary starting date, called the
>> 	epoch.
>>
>> Unfortunately, this definition is obsolete, and has been for quite some
> Do you have any proof for such a claim?
Hi Wolfgang,
      Well, yes, in fact the same reference you used. Note that the 
statement "A tick is defined as the smallest increment of system time as 
measured by a computer system" is NOT found in 
http://en.wikipedia.org/wiki/System_time. That page is defining system 
time, and what we are discussing is the definition of a "tick". In fact, 
on the same wiki page you cite, there IS the statement "Windows NT 
<http://en.wikipedia.org/wiki/Windows_NT> counts the number of 
100-nanosecond ticks since 1 January 1601 00:00:00 UT as reckoned in the 
proleptic Gregorian calendar 
<http://en.wikipedia.org/wiki/Proleptic_Gregorian_calendar>, but returns 
the current time to the nearest millisecond". Here 100 nanosecond ticks 
clearly does not refer to any "hardware" 100 ns clock that exists on the 
pc. The 100 ns is a computed (dare I say virtual) "tick" value. The 
point here is that the definition of tick is yours, not wikipedia.org's. 
(Although we all are aware if it is wikipedia, it MUST be so).  Further, 
http://en.wikipedia.org/wiki/Jiffy_%28time%29#Use_in_computing contains 
the statement "In computing <http://en.wikipedia.org/wiki/Computing>, a 
jiffy is the duration of one tick of the system timer 
<http://en.wikipedia.org/wiki/System_time> interrupt 
<http://en.wikipedia.org/wiki/Interrupt>.". If a tick is the smallest 
increment of system time as measured by the computer system, the "of the 
system timer interrupt" part of the statement would be unnecessary. The 
fact it IS present indicates there are other kinds of ticks present in 
the universe.
     AFAIK, all timers "tick", and the definition of the tick rate is 
1/timer resolution. The concept of "timer ticks" and "clock ticks" has 
been around forever, and exists independent of System Time. For example, 
there may be a watchdog timer on a system that does not measure time at 
all, yet the watchdog still ticks. When you read a value from a timer 
that was not the "system timer", what do you call the value you read? I 
would call it ticks, and I bet just about everybody else would too.
     The only reason I feel this point matters at all is that when one 
refers to a routine called "get_ticks", it is not obvious to me which 
timer ticks are being referenced. You are saying that, by definition, it 
refers to the "system clock".  My feeling is that it is not obvious why 
that is so on a system that has many clocks. The name of the function or 
an argument to the function should, IMNSHO,  specify which timer ticks 
are being returned.

Best Regards,
Bill Campbell
>> years.  When computers had a single timer, the above definition worked,
>> but it no longer does, as many (most?) computers have several hardware
>> timers. A "tick" today is the time increment of any particular timer of
>> a computer system. So, when one writes a function called get_ticks on a
>> PPC, does one mean read the decrementer, or does one read the RTC or
>> does one read the TB register(s) A similar situation exists on the
>> Blackfin BF531/2/3, that has a preformance counter, a real-time clock,
>> and three programmable timers. Which tick do you want? For each u-boot
> Please re-read the definition.  At least as far as U-Boot and Linux
> are concerned, there is only a single clock source used to implement
> the _system_time_.  And I doubt that other OS do different.
>
>> implementation, we can pick one timer as the "master" timer, but it may
>> not be the one with the most rapid tick rate. It may be the one with the
>> most USEFUL tick rate for get_timer. If you take the above definition at
>> face value, only the fastest counter value has ticks, and all other
>> counters time increments are not ticks. If they are not ticks, what are
>> they?
> Clocks, timers?
>
>
> Best regards,
>
> Wolfgang Denk
>

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear Graeme Russ,

In message <BANLkTikM3LPynzckNP64KjEQ5v+tE7yUhQ@mail.gmail.com> you wrote:
> 
> I hope to get an implementation agreed upon that does not require
> interrupts at all, provided a tick counter with sufficiently long roll
> over time is available (longer than the maximum expected period
> between 'related' get_timer() calls, for example calls to get_timer()
> in a timeout testing while loop). This 'bare minimum' implementation
> can be optionally augmented with an ISR which kicks the timer
> calculation in the background (typically by just calling get_timer())
> 
> It really is quite simple in the end.

The length of this thread shows that it is not as simple as you want
to make us believe.



If all you have in mind are timeouts, then we don't need get_timer()
at all.  We can implement all types of timeout where we wait for some
eent to happen using udelay() alone.

Need a 10 second timeout? Here we go:

	int cnt = 0;
	int limit = 10 * 1000;
	while (!condition) {
		usleep(1000); /* wait 1 millisec */
		if (++cnt > limit)
			break;
	}
	if (cnt > limit)
		error("timeout");

get_timer() comes into play only if we want to calculate a time
difference, for example if we want to run some code where we don't
know how long it runs, and later come back and check if a certain
amount of time has passed.

When we don't know how long this code runs, we also cannot know (and
espeically not in advance) wether or not this time will be longer or
not than the rollover time of the tick counter.


Your plan to require that get_timer() gets called "often enough" to
prevent or detect tick counter overflows is putting things on their
head.  It should be the opposite:  The implementation of get_timer()
should be such that it becomes independent from such low level
details.


I have stated this before:  I consider any design that requires
get_timer() to be called "often enough" broken.

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
Well, the way I see it, logic is only a way of being ignorant by num-
bers.                                 - Terry Pratchett, _Small Gods_

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear "J. William Campbell",

In message <4DDD3354.8030801@comcast.net> you wrote:
>
> >> A tick is defined as the smallest increment of system time as measured by a
> >> computer system (seehttp://en.wikipedia.org/wiki/System_time):
> >>
> >> 	System time is measured by a system clock, which is typically
> >> 	implemented as a simple count of the number of ticks that have
> >> 	transpired since some arbitrary starting date, called the
> >> 	epoch.
> >>
> >> Unfortunately, this definition is obsolete, and has been for quite some
> > Do you have any proof for such a claim?
> Hi Wolfgang,
>       Well, yes, in fact the same reference you used. Note that the 
> statement "A tick is defined as the smallest increment of system time as 
> measured by a computer system" is NOT found in 
> http://en.wikipedia.org/wiki/System_time. That page is defining system 

I derived this from
http://en.wikipedia.org/wiki/Tick_%28disambiguation%29: "Tick, the
smallest increment of system time as measured by a computer system"

> the current time to the nearest millisecond". Here 100 nanosecond ticks 
> clearly does not refer to any "hardware" 100 ns clock that exists on the 

Above definition nowhere requres that the tick must be a "hardware"
clock.

> pc. The 100 ns is a computed (dare I say virtual) "tick" value. The 
> point here is that the definition of tick is yours, not wikipedia.org's. 

Not quite.

But while I do now want to claim authorship for words I just copied &
pasted, I think the definition is still a good one.  Let's say it's
the definition I want to see used to this discussion here :-)

> <http://en.wikipedia.org/wiki/Interrupt>.". If a tick is the smallest 
> increment of system time as measured by the computer system, the "of the 
> system timer interrupt" part of the statement would be unnecessary. The 
> fact it IS present indicates there are other kinds of ticks present in 
> the universe.

I disagree here.

>      AFAIK, all timers "tick", and the definition of the tick rate is 
> 1/timer resolution. The concept of "timer ticks" and "clock ticks" has 
> been around forever, and exists independent of System Time. For example, 
> there may be a watchdog timer on a system that does not measure time at 
> all, yet the watchdog still ticks. When you read a value from a timer 
> that was not the "system timer", what do you call the value you read? I 
> would call it ticks, and I bet just about everybody else would too.

I have no reason to argument about this.

But we don't care about any timers that may be present or even in use
for one pourpose or anothe rin the system.

All we care about her ein this discussion is a single time, and a
single timer, and a single tick: the one that represents the system
time.

>      The only reason I feel this point matters at all is that when one 
> refers to a routine called "get_ticks", it is not obvious to me which 
> timer ticks are being referenced. You are saying that, by definition, it 
> refers to the "system clock".  My feeling is that it is not obvious why 
> that is so on a system that has many clocks. The name of the function or 
> an argument to the function should, IMNSHO,  specify which timer ticks 
> are being returned.

THe reason is simply that it is the only clock that is being used
anywhere in U-Boot.  We have a only one system time, or clock.

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
I mean, I . . . think to understand you, I just don't know  what  you
are saying ...                        - Terry Pratchett, _Soul Music_

[U-Boot] [RFC] Review of U-Boot timer API

[J. William Campbell]

On 5/25/2011 12:46 PM, Wolfgang Denk wrote:
> Dear Graeme Russ,
>
> In message<BANLkTikM3LPynzckNP64KjEQ5v+tE7yUhQ@mail.gmail.com>  you wrote:
>> I hope to get an implementation agreed upon that does not require
>> interrupts at all, provided a tick counter with sufficiently long roll
>> over time is available (longer than the maximum expected period
>> between 'related' get_timer() calls, for example calls to get_timer()
>> in a timeout testing while loop). This 'bare minimum' implementation
>> can be optionally augmented with an ISR which kicks the timer
>> calculation in the background (typically by just calling get_timer())
>>
>> It really is quite simple in the end.
> The length of this thread shows that it is not as simple as you want
> to make us believe.
>
>
>
> If all you have in mind are timeouts, then we don't need get_timer()
> at all.  We can implement all types of timeout where we wait for some
> eent to happen using udelay() alone.
>
> Need a 10 second timeout? Here we go:
>
> 	int cnt = 0;
> 	int limit = 10 * 1000;
> 	while (!condition) {
> 		usleep(1000); /* wait 1 millisec */
> 		if (++cnt>  limit)
> 			break;
> 	}
> 	if (cnt>  limit)
> 		error("timeout");
>
> get_timer() comes into play only if we want to calculate a time
> difference, for example if we want to run some code where we don't
> know how long it runs, and later come back and check if a certain
> amount of time has passed.
>
> When we don't know how long this code runs, we also cannot know (and
> espeically not in advance) wether or not this time will be longer or
> not than the rollover time of the tick counter.
>
>
> Your plan to require that get_timer() gets called "often enough" to
> prevent or detect tick counter overflows is putting things on their
> head.  It should be the opposite:  The implementation of get_timer()
> should be such that it becomes independent from such low level
> details.
>
HI all,
       We also cannot know if this code disables interrupts. If it does, 
the existing PPC design is broken. If interrupts are disabled during the 
entire code being executed, the elapsed run time will be 0. You can say 
that disabling interrupts in the code is not allowed. Fine, then that 
becomes a constraint on the code. Calling get_timer explicitly often 
enough is also a constraint on the code. One constraint is acceptable to 
you, the other is not. Ok, that's fine too. If the interrupt routine 
calls get_timer, then get_timer is called "often enough" and everything 
works the same as it presently does on PPC, It is not different than 
requiring the interrupt routine to be executed "often enough". The two 
methods are functionally identical. The only problems arise on systems 
that don't support interrupts and don't have any timers with enough bits 
available to meet the 4294967 seconds maximum interval requirement. 
Those systems will be "broken" no matter what we do, as we have all agreed.

     Right now, almost all ARM cases are broken, because they have short 
timers and don't use interrupts. In some cases, there are actual bugs 
involved. We can make these cases less broken than they now are with a 
common get_timer approach as outlined previously. However, we cannot fix 
them to the standard Wolfgang is stating here, to be not "broken". So, 
back to what I was asking before, is the improvement worth the effort if 
the result is still broken?

Best Regards,
Bill Campbell
> I have stated this before:  I consider any design that requires
> get_timer() to be called "often enough" broken.
>
> Best regards,
>
> Wolfgang Denk
>

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear "J. William Campbell",

In message <4DDD6930.2080202@comcast.net> you wrote:
>
> methods are functionally identical. The only problems arise on systems 
> that don't support interrupts and don't have any timers with enough bits 
> available to meet the 4294967 seconds maximum interval requirement. 

Where exactly is this requirement coming from?

>      Right now, almost all ARM cases are broken, because they have short 
> timers and don't use interrupts. In some cases, there are actual bugs 
> involved. We can make these cases less broken than they now are with a 
> common get_timer approach as outlined previously. However, we cannot fix 

We could also make them use interrupts for this type of timer service?

How do these systems implement system time in other environments, say
in Linux?  Or in Barebox?

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
Human beings were created by water to transport it uphill.

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

On 25/05/11 23:38, Wolfgang Denk wrote:
> Dear Graeme Russ,
> 
> In message <BANLkTimhtqYNWgvwr6w8kNOSV4U_tZs7yw@mail.gmail.com> you wrote:
>>
>> And what about platforms that do not support interrupts? Are you
>> mandating that get_timer() MUST always be supported by a timer updated
>> by an ISR?
> 
> No, not at all. And I already answered this. For example on PPC, just
> reading the timebase would be perfectly sufficient, and simpler and
> more reliable than the current interrupt based approach.

I assume by 'timebase' you mean the 64-bit tick counter. If so, that is
_exactly_ what I am suggesting we do (and what does already happen on ARM).
The big change is, the prescaler is common code rather than haven dozens
(some of them broken) implementations

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear Graeme Russ,

In message <4DDD7066.4000505@gmail.com> you wrote:
>
> > No, not at all. And I already answered this. For example on PPC, just
> > reading the timebase would be perfectly sufficient, and simpler and
> > more reliable than the current interrupt based approach.
> 
> I assume by 'timebase' you mean the 64-bit tick counter. If so, that is

By timebase I mean the timebase register, implemented as two 32 bit
registers tbu and tbl, holding the upper and the lower 32 bits of the
free-running 64 bit counter, respective.

> _exactly_ what I am suggesting we do (and what does already happen on ARM).

I don't think so.

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
"...this does not mean that some of us should not want, in  a  rather
dispassionate sort of way, to put a bullet through csh's head."
                   - Larry Wall in <1992Aug6.221512.5963@netlabs.com>

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

Hi Wolfgang

On Thu, May 26, 2011 at 7:16 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Graeme Russ,
>
> In message <4DDD7066.4000505@gmail.com> you wrote:
>>
>> > No, not at all. And I already answered this. For example on PPC, just
>> > reading the timebase would be perfectly sufficient, and simpler and
>> > more reliable than the current interrupt based approach.
>>
>> I assume by 'timebase' you mean the 64-bit tick counter. If so, that is
>
> By timebase I mean the timebase register, implemented as two 32 bit
> registers tbu and tbl, holding the upper and the lower 32 bits of the
> free-running 64 bit counter, respective.

And remember, not all platforms have this implementation. The AMD sc520
for example has a microsecond register which counts 0-999 that ticks a
16-bit millisecond register and resets to zero. And the millisecond
register latches the value of the microsecond register and resets
(the millisecond register) back to zero.

The thing is, this can all be abstracted away via get_tick() which
(provided it is called every 65 seconds or so) can maintain a software
version of the timebase register. So, every 65 seconds, the prescaler
needs to be kicked. Now, if all we want to use get_timer() for is to
monitor a timeout (which I think might be every single use in U-Boot
to date) then the while (get_timer(start) < timeout) loop will work. If
get_timer() is needed to measure time between two arbitrary events (which
I 100% agree it should be able to do) then the prescaler will need to be
kicked (typically by an interrupt)

>
>> _exactly_ what I am suggesting we do (and what does already happen on ARM).
>
> I don't think so.

On closer inspection, some do, some don't. All ARMv7 (OMAP, S5P, Tegra2)
do. at91 is odd - It looks like it uses interrupts, but get_timer() and
udelay() both end up calling get_timer_raw() (with udelay only having
millisecond resolution it seems). Some others can be configured to
increment the timer using an interrupt. ARM is, quite frankly, a complete
mess - It has a mass of *_timer_masked() functions which the core timer
functions are 'wafer thin' wrapper around, udelay() silently resets
the timebase trashing get_timer() loops etc.

So let's wind back and distill the approach I am suggesting:

 1) A common prescaler function in /lib/ - It's purpose is to maintain
    a 1ms resolution timer (if the platform cannot otherwise do so)[1]
    The prescaler utilises a platform provided get_ticks()[2]
 2) A get_ticks() function provided by the platform - This function must
    return an unsigned counter which wraps from all 1's to all 0's - It
    DOES NOT have to be initialised to zero at system start. get_ticks()
    hides the low-level tick counter implementation - The sc520 example
    above is a classic example, so is your PPC tbu/tbl example.
 3) [Optional]An ISR which calls the prescaler[3]

Now there is an optimisation if your tick counter has a 1ms resolution
and is not small (i.e. 64-bits) - The prescaler is defined weak, so in
the platform code, re-implement the prescaler to simply copy the tick
counter to the timer variable.

And what are the specific implementation types (in decending order of
preference)? I think:
 1) A 64-bit micro-second tick counter[5]
      - No interrupts needed
      - Can be used by udelay() and get_timer() trivially
 2) A 64-bit sub-micro-second tick counter
      - Interrupts most likely undeeded unless the tick frequency is
        insanely high
      - Can be used by udelay() and get_timer() trivially
 3) A 64-bit milli-second tick counter
      - No interrupts needed
      - No prescaler needed
      - Can be used by get_timer() trivially
      - udelay() needs another tick source (if available) or be reduced
        to millisecond resolution
 4) A 32-bit milli-second tick counter
      - No prescaler needed[6]
      - Max 'glitch free' duration is ~50 days
      - ISR needed to kick prescaler if events longer than 50 days need
        to be timed
      - Can be used by get_timer() trivially
      - udelay() needs another tick source (if available) or be reduced
        to millisecond resolution
 5) A 24-bit milli-second tick counter
      - No prescaler needed[6]
      - Max 'glitch free' duration is ~4.5 hours
      - ISR needed to kick prescaler if events longer than 4.5 hours need
        to be timed
      - Can be used by get_timer() trivially
      - udelay() needs another tick source (if available) or be reduced
        to millisecond resolution
 6) A 32-bit micro-second tick counter
      - No prescaler needed[6]
      - Max 'glitch free' duration is 71 minutes
      - ISR needed to kick prescaler if events longer than 71 minutes need
        to be timed
      - Can be used by get_timer() trivially
      - udelay() needs another tick source (if available) or be reduced
        to millisecond resolution

Any implementation which does not fit withing the above is going to
require an ISR to kick the prescaler in order to support timing of 'long
events' (i.e. not just simple timeout loops)

[1]The prescaler would still be needed by platforms which has a 64-bit
tick counter which ticks at a rate greater than 1ms
[2]Exposing get_ticks() reduces code duplication
[3]Only required if the rollover time of the tick counter (i.e. the maximum
permissible time between any two get_ticks() calls) is 'small'[4]
[4]'small' is at the discretion of the implementer - 1 second is always
small, 1 hour might be, 500 years is not
[5]A tick counter is something maintained by the underlying platform
independent of any U-Boot code
[6]Although wise to override the prescaler function so the timer ISR is
consistent with all other platforms

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[J. William Campbell]

On 5/25/2011 4:13 PM, Graeme Russ wrote:
> Hi Wolfgang
>
> On Thu, May 26, 2011 at 7:16 AM, Wolfgang Denk<wd@denx.de>  wrote:
>> Dear Graeme Russ,
>>
>> In message<4DDD7066.4000505@gmail.com>  you wrote:
>>>> No, not at all. And I already answered this. For example on PPC, just
>>>> reading the timebase would be perfectly sufficient, and simpler and
>>>> more reliable than the current interrupt based approach.
>>> I assume by 'timebase' you mean the 64-bit tick counter. If so, that is
>> By timebase I mean the timebase register, implemented as two 32 bit
>> registers tbu and tbl, holding the upper and the lower 32 bits of the
>> free-running 64 bit counter, respective.
> And remember, not all platforms have this implementation. The AMD sc520
> for example has a microsecond register which counts 0-999 that ticks a
> 16-bit millisecond register and resets to zero. And the millisecond
> register latches the value of the microsecond register and resets
> (the millisecond register) back to zero.
>
> The thing is, this can all be abstracted away via get_tick() which
> (provided it is called every 65 seconds or so) can maintain a software
> version of the timebase register. So, every 65 seconds, the prescaler
> needs to be kicked. Now, if all we want to use get_timer() for is to
> monitor a timeout (which I think might be every single use in U-Boot
> to date) then the while (get_timer(start)<  timeout) loop will work. If
> get_timer() is needed to measure time between two arbitrary events (which
> I 100% agree it should be able to do) then the prescaler will need to be
> kicked (typically by an interrupt)
>
>>> _exactly_ what I am suggesting we do (and what does already happen on ARM).
>> I don't think so.
Hi All,
       Just to be clear, while ARMv7 has a 64 bit performance counter, 
it is not presently used by get_time. This is a change we want to make 
correct?
> On closer inspection, some do, some don't. All ARMv7 (OMAP, S5P, Tegra2)
> do. at91 is odd - It looks like it uses interrupts, but get_timer() and
> udelay() both end up calling get_timer_raw() (with udelay only having
> millisecond resolution it seems).
I am not sure why you say at91 appears to use interrupts. There is a 
comment in arch/arm/cpu/arm930t/at91/timer.c that says "timer without 
interrupts" (line 73). There is the same comment in 
arch/arm/cpu/arm930t/at91rm9200/timer.c Nothing in either routine refers 
to interrupts, so I would say the timer doesn't use them. I could be 
wrong of course.
>   Some others can be configured to
> increment the timer using an interrupt. ARM is, quite frankly, a complete
> mess - It has a mass of *_timer_masked() functions which the core timer
> functions are 'wafer thin' wrapper around, udelay() silently resets
> the timebase trashing get_timer() loops etc.
I sure agree with this last part. The only arm timer I found that 
clearly thought it could use interrupts was in arch/arm/cpu/ixp, and 
that was conditional, not mandatory.
> So let's wind back and distill the approach I am suggesting:
>
>   1) A common prescaler function in /lib/ - It's purpose is to maintain
>      a 1ms resolution timer (if the platform cannot otherwise do so)[1]
>      The prescaler utilises a platform provided get_ticks()[2]
>   2) A get_ticks() function provided by the platform - This function must
>      return an unsigned counter which wraps from all 1's to all 0's - It
>      DOES NOT have to be initialised to zero at system start. get_ticks()
>      hides the low-level tick counter implementation - The sc520 example
>      above is a classic example, so is your PPC tbu/tbl example.
>   3) [Optional]An ISR which calls the prescaler[3]
>
> Now there is an optimisation if your tick counter has a 1ms resolution
> and is not small (i.e. 64-bits) - The prescaler is defined weak, so in
> the platform code, re-implement the prescaler to simply copy the tick
> counter to the timer variable.
>
> And what are the specific implementation types (in decending order of
> preference)? I think:
>   1) A 64-bit micro-second tick counter[5]
>        - No interrupts needed
>        - Can be used by udelay() and get_timer() trivially
>   2) A 64-bit sub-micro-second tick counter
>        - Interrupts most likely undeeded unless the tick frequency is
>          insanely high
>        - Can be used by udelay() and get_timer() trivially
>   3) A 64-bit milli-second tick counter
>        - No interrupts needed
>        - No prescaler needed
>        - Can be used by get_timer() trivially
>        - udelay() needs another tick source (if available) or be reduced
>          to millisecond resolution
>   4) A 32-bit milli-second tick counter
>        - No prescaler needed[6]
>        - Max 'glitch free' duration is ~50 days
>        - ISR needed to kick prescaler if events longer than 50 days need
>          to be timed
>        - Can be used by get_timer() trivially
>        - udelay() needs another tick source (if available) or be reduced
>          to millisecond resolution
>   5) A 24-bit milli-second tick counter
>        - No prescaler needed[6]
>        - Max 'glitch free' duration is ~4.5 hours
>        - ISR needed to kick prescaler if events longer than 4.5 hours need
>          to be timed
>        - Can be used by get_timer() trivially
>        - udelay() needs another tick source (if available) or be reduced
>          to millisecond resolution
>   6) A 32-bit micro-second tick counter
>        - No prescaler needed[6]
>        - Max 'glitch free' duration is 71 minutes
>        - ISR needed to kick prescaler if events longer than 71 minutes need
>          to be timed
>        - Can be used by get_timer() trivially
I think this should be "Can be used by udelay and get_timer trivially"
Best Regards,
Bill Campbell
>        - udelay() needs another tick source (if available) or be reduced
>          to millisecond resolution
>
> Any implementation which does not fit withing the above is going to
> require an ISR to kick the prescaler in order to support timing of 'long
> events' (i.e. not just simple timeout loops)
>
> [1]The prescaler would still be needed by platforms which has a 64-bit
> tick counter which ticks at a rate greater than 1ms
> [2]Exposing get_ticks() reduces code duplication
> [3]Only required if the rollover time of the tick counter (i.e. the maximum
> permissible time between any two get_ticks() calls) is 'small'[4]
> [4]'small' is at the discretion of the implementer - 1 second is always
> small, 1 hour might be, 500 years is not
> [5]A tick counter is something maintained by the underlying platform
> independent of any U-Boot code
> [6]Although wise to override the prescaler function so the timer ISR is
> consistent with all other platforms
>
> Regards,
>
> Graeme
>
>

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

On Thu, May 26, 2011 at 10:15 AM, J. William Campbell
<jwilliamcampbell@comcast.net> wrote:
> On 5/25/2011 4:13 PM, Graeme Russ wrote:
>>
>> Hi Wolfgang
>>
>> On Thu, May 26, 2011 at 7:16 AM, Wolfgang Denk<wd@denx.de> ?wrote:
>>>
>>> Dear Graeme Russ,
>>>
>>> In message<4DDD7066.4000505@gmail.com> ?you wrote:
>>>>>

[snip]

>>>> _exactly_ what I am suggesting we do (and what does already happen on
>>>> ARM).
>>>
>>> I don't think so.
>
> Hi All,
> ? ? ?Just to be clear, while ARMv7 has a 64 bit performance counter, it is
> not presently used by get_time. This is a change we want to make correct?

If it is a constant time-base then yes, that would be preferable - But I
doubt all ARM CPUs have these, so these need to be enabled on a case by
case basis - get_ticks() can then use them trivially

>>
>> On closer inspection, some do, some don't. All ARMv7 (OMAP, S5P, Tegra2)
>> do. at91 is odd - It looks like it uses interrupts, but get_timer() and
>> udelay() both end up calling get_timer_raw() (with udelay only having
>> millisecond resolution it seems).
>
> I am not sure why you say at91 appears to use interrupts. There is a comment
> in arch/arm/cpu/arm930t/at91/timer.c that says "timer without interrupts"
> (line 73). There is the same comment in
> arch/arm/cpu/arm930t/at91rm9200/timer.c Nothing in either routine refers to
> interrupts, so I would say the timer doesn't use them. I could be wrong of
> course.

You are correct

[snip]

>> ?6) A 32-bit micro-second tick counter
>> ? ? ? - No prescaler needed[6]
>> ? ? ? - Max 'glitch free' duration is 71 minutes
>> ? ? ? - ISR needed to kick prescaler if events longer than 71 minutes need
>> ? ? ? ? to be timed
>> ? ? ? - Can be used by get_timer() trivially
>
> I think this should be "Can be used by udelay and get_timer trivially"

Yes, you are again correct

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Reinhard Meyer]

Dear Graeme Russ,
> On closer inspection, some do, some don't. All ARMv7 (OMAP, S5P, Tegra2)
> do. at91 is odd - It looks like it uses interrupts, but get_timer() and
> udelay() both end up calling get_timer_raw() (with udelay only having
> millisecond resolution it seems). Some others can be configured to
> increment the timer using an interrupt. ARM is, quite frankly, a complete
> mess - It has a mass of *_timer_masked() functions which the core timer
> functions are 'wafer thin' wrapper around, udelay() silently resets
> the timebase trashing get_timer() loops etc.

Please look at current master for at91.

http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/arm926ejs/at91/timer.c;h=a0876879d3907af553d832bea187a062a22b9bd4;hb=5d1ee00b1fe1180503f6dfc10e87a6c6e74778f3

AT91 uses a 32 bit hardware register that by means of a prescaler is made
to increment at a rate in the low megahertz range.

This results in a wrap approximately every 1000 seconds.
Actually this would be sufficient for all known uses of udelay() and get_timer()
timeout loops. However, this hardware register is extended to 64 bits by software
every time it is read (by detecting rollovers).

Since a wrap of that 64 bit "tick" would occur after the earth has ended,
it is simple to obtain milliseconds from it by doing a 64 bit division.

Best Regards,
Reinhard

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

On Thu, May 26, 2011 at 2:19 PM, Reinhard Meyer
<u-boot@emk-elektronik.de> wrote:
>
> Dear Graeme Russ,
>>
>> On closer inspection, some do, some don't. All ARMv7 (OMAP, S5P, Tegra2)
>> do. at91 is odd - It looks like it uses interrupts, but get_timer() and
>> udelay() both end up calling get_timer_raw() (with udelay only having
>> millisecond resolution it seems). Some others can be configured to
>> increment the timer using an interrupt. ARM is, quite frankly, a complete
>> mess - It has a mass of *_timer_masked() functions which the core timer
>> functions are 'wafer thin' wrapper around, udelay() silently resets
>> the timebase trashing get_timer() loops etc.
>
> Please look at current master for at91.
>
> http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/arm926ejs/at91/timer.c;h=a0876879d3907af553d832bea187a062a22b9bd4;hb=5d1ee00b1fe1180503f6dfc10e87a6c6e74778f3
>
> AT91 uses a 32 bit hardware register that by means of a prescaler is made
> to increment at a rate in the low megahertz range.

Yes, I see that now

> This results in a wrap approximately every 1000 seconds.
> Actually this would be sufficient for all known uses of udelay() and get_timer()
> timeout loops. However, this hardware register is extended to 64 bits by software
> every time it is read (by detecting rollovers).

Which makes it 100% compatible with my proposed solution - The software
prescaler will trigger the 64-bit extension and rollover detection

> Since a wrap of that 64 bit "tick" would occur after the earth has ended,
> it is simple to obtain milliseconds from it by doing a 64 bit division.

Which would be done in the common prescaler in /lib/

Currently, most ARM specific utilisations of get_timer() enforce a reset
of the tick counter by calling reset_timer() - Subsequent calls to
get_timer() then assume a start time of zero. Provided the internal timer
rolls over currectly, the initial call of get_timer(0) will reset the ms
timer and remove and 'glitch' present due to not calling the 'extender'
function between 32-bit rollovers which makes the reset_timer() call
unneccessary - I believe at91 behaves correctly in this regard.

In any case, the underlying assumption made by the ARM timer interface
(call reset_timer() first always) is inherently broken as not all users
of the timer API do this - They assume a sane behaviour of:

	start = get_timer(0);
	elapsed_time = get_timer(start);

Add to this udelay() resetting the timer make the following very broken:

	start = get_timer(0);
	while(condition) {
		udelay(delay);
	}
	elapsed_time = get_timer(start);

NOTE: In this case, if udelay() also calls the prescaler then no interrupt
triggered every 1000s would be required in the above example to get
correct elapsed_time even if the loop ran for several hours (provided
udelay() is called at least every 1000s

However, to allow timing of independent events with no intervening
udelay() or get_timer() calls, an 1000s interrupt to kick the prescaler is
all that is needed to make this particular implementation behave correctly.
Of course disabling interruts and not calling get_timer() or udelay() will
break the timer - But there is nothing that can be done about that)

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[J. William Campbell]

On 5/25/2011 9:19 PM, Reinhard Meyer wrote:
> Dear Graeme Russ,
>> On closer inspection, some do, some don't. All ARMv7 (OMAP, S5P, Tegra2)
>> do. at91 is odd - It looks like it uses interrupts, but get_timer() and
>> udelay() both end up calling get_timer_raw() (with udelay only having
>> millisecond resolution it seems). Some others can be configured to
>> increment the timer using an interrupt. ARM is, quite frankly, a 
>> complete
>> mess - It has a mass of *_timer_masked() functions which the core timer
>> functions are 'wafer thin' wrapper around, udelay() silently resets
>> the timebase trashing get_timer() loops etc.
>
> Please look at current master for at91.
>
> http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/arm926ejs/at91/timer.c;h=a0876879d3907af553d832bea187a062a22b9bd4;hb=5d1ee00b1fe1180503f6dfc10e87a6c6e74778f3 
>
>
> AT91 uses a 32 bit hardware register that by means of a prescaler is made
> to increment at a rate in the low megahertz range.
>
> This results in a wrap approximately every 1000 seconds.
>
> Actually this would be sufficient for all known uses of udelay() and 
> get_timer()
Hi All
       Yes, you are correct. It would be sufficient for all known uses 
of udelay and get_timer(). However, Wolfgang has indicated a strong 
desire that the performance of the new API work properly over the full 
32 bit range of the millisecond delay time. That has been the basic 
issue for some time here.
> timeout loops. However, this hardware register is extended to 64 bits 
> by software
> every time it is read (by detecting rollovers).
Yes, but this extension ONLY happens if get_ticks is called via udelay 
or get_timer. It doesn't happen if you are sitting at the command prompt 
or off executing a downloaded stand alone program. You might ask "who 
cares", and I would answer that Wolfgang cares, at least to some level. 
If the timer overflow triggered an interrupt, we could call get_ticks to 
update the extended time inside the interrupt routine. But, as far as I 
know, it doesn't. There are some other ARM processors that have a 32 bit 
clock derived from a 32 kHz crystal, The will work much as you example 
does up to 134217 seconds, in fact much longer than your AT91 example 
does. However, that doesn't touch the 4294967 seconds that the PPC can 
manage. Without interrupts, the 32 bit (or smaller) counters will NEVER 
get to the PPC standard if their tick rate exceeds 1 msec. It may be 
that we need a lower standard, perhaps saying 1000 seconds is enough. 
But that is not my decision to make.
>
> Since a wrap of that 64 bit "tick" would occur after the earth has ended,
> it is simple to obtain milliseconds from it by doing a 64 bit division.
True, but moot because of the above.
Best Regards,
Bill Campbell
>
> Best Regards,
> Reinhard
>
>

[U-Boot] [RFC] Review of U-Boot timer API

[J. William Campbell]

On 5/25/2011 9:40 PM, Graeme Russ wrote:
> On Thu, May 26, 2011 at 2:19 PM, Reinhard Meyer
> <u-boot@emk-elektronik.de>  wrote:
>> Dear Graeme Russ,
>>> On closer inspection, some do, some don't. All ARMv7 (OMAP, S5P, Tegra2)
>>> do. at91 is odd - It looks like it uses interrupts, but get_timer() and
>>> udelay() both end up calling get_timer_raw() (with udelay only having
>>> millisecond resolution it seems). Some others can be configured to
>>> increment the timer using an interrupt. ARM is, quite frankly, a complete
>>> mess - It has a mass of *_timer_masked() functions which the core timer
>>> functions are 'wafer thin' wrapper around, udelay() silently resets
>>> the timebase trashing get_timer() loops etc.
>> Please look at current master for at91.
>>
>> http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/arm926ejs/at91/timer.c;h=a0876879d3907af553d832bea187a062a22b9bd4;hb=5d1ee00b1fe1180503f6dfc10e87a6c6e74778f3
>>
>> AT91 uses a 32 bit hardware register that by means of a prescaler is made
>> to increment at a rate in the low megahertz range.
> Yes, I see that now
>
>> This results in a wrap approximately every 1000 seconds.
>> Actually this would be sufficient for all known uses of udelay() and get_timer()
>> timeout loops. However, this hardware register is extended to 64 bits by software
>> every time it is read (by detecting rollovers).
> Which makes it 100% compatible with my proposed solution - The software
> prescaler will trigger the 64-bit extension and rollover detection
>
>> Since a wrap of that 64 bit "tick" would occur after the earth has ended,
>> it is simple to obtain milliseconds from it by doing a 64 bit division.
> Which would be done in the common prescaler in /lib/
>
> Currently, most ARM specific utilisations of get_timer() enforce a reset
> of the tick counter by calling reset_timer() - Subsequent calls to
> get_timer() then assume a start time of zero. Provided the internal timer
> rolls over currectly, the initial call of get_timer(0) will reset the ms
> timer and remove and 'glitch' present due to not calling the 'extender'
> function between 32-bit rollovers which makes the reset_timer() call
> unneccessary - I believe at91 behaves correctly in this regard.
>
> In any case, the underlying assumption made by the ARM timer interface
> (call reset_timer() first always) is inherently broken as not all users
> of the timer API do this - They assume a sane behaviour of:
>
> 	start = get_timer(0);
> 	elapsed_time = get_timer(start);
>
> Add to this udelay() resetting the timer make the following very broken:
>
> 	start = get_timer(0);
> 	while(condition) {
> 		udelay(delay);
> 	}
> 	elapsed_time = get_timer(start);
>
> NOTE: In this case, if udelay() also calls the prescaler then no interrupt
> triggered every 1000s would be required in the above example to get
> correct elapsed_time even if the loop ran for several hours (provided
> udelay() is called at least every 1000s
>
> However, to allow timing of independent events with no intervening
> udelay() or get_timer() calls, an 1000s interrupt to kick the prescaler is
> all that is needed to make this particular implementation behave correctly.
Hi All,
       True, if the processor supports timer interrupts. The problem is 
that the existing u-boots in many cases do not. I think that is really 
the crux of the problem. So what are we going to do? I am open to ideas 
here.

Best Regards,
Bill Campbell

> Of course disabling interruts and not calling get_timer() or udelay() will
> break the timer - But there is nothing that can be done about that)
>
> Regards,
>
> Graeme
>
>

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear "J. William Campbell",

In message <4DDDDF2E.8070602@comcast.net> you wrote:
>
>        True, if the processor supports timer interrupts. The problem is 
> that the existing u-boots in many cases do not. I think that is really 
> the crux of the problem. So what are we going to do? I am open to ideas 
> here.

Which of the processors we're discussing here does not support timer
interrupts?  If there is any, how is the Linux system time implemented
on this processor?

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
When properly administered, vacations do not  diminish  productivity:
for every week you're away and get nothing done, there's another when
your boss is away and you get twice as much done.  -- Daniel B. Luten

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

On Thu, May 26, 2011 at 3:16 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear "J. William Campbell",
>
> In message <4DDDDF2E.8070602@comcast.net> you wrote:
>>
>> ? ? ? ?True, if the processor supports timer interrupts. The problem is
>> that the existing u-boots in many cases do not. I think that is really
>> the crux of the problem. So what are we going to do? I am open to ideas
>> here.
>
> Which of the processors we're discussing here does not support timer
> interrupts? ?If there is any, how is the Linux system time implemented
> on this processor?

Well, they all support timer interrupts - It's just that that support
is not supported in U-Boot (yet)

I think we can still rationalise the timer API as suggested which, as
a minimum will:

 - Not diminish current functionality for any platform
 - Will dramatically reduce the current amount of code duplication
 - Improve functionality on some platforms
 - Simplify the whole API

and then start banging on arch maintainers heads to implement the trivial
ISR to kick the prescaler:

void timer_isr(void *unused)
{
	prescaler();
}

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Albert ARIBAUD]

Le 26/05/2011 07:25, Graeme Russ a ?crit :

> I think we can still rationalise the timer API as suggested which, as
> a minimum will:
>
>   - Not diminish current functionality for any platform
>   - Will dramatically reduce the current amount of code duplication
>   - Improve functionality on some platforms
>   - Simplify the whole API
>
> and then start banging on arch maintainers heads to implement the trivial
> ISR to kick the prescaler:

I personally am in favor of cooperation rather than coercion, so please, 
ask before banging on heads. :)

> void timer_isr(void *unused)
> {
> 	prescaler();
> }
>
> Regards,
>
> Graeme

Seems like we're asymptotically getting to some kind of agreement. For 
the sake of clarity, could someone summarize what the expected API would 
be, so that I save my scalp and start looking as the order in which 
changes should be done on ARM?

Amicalement,
-- 
Albert.

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

Hi Albert,

On Thu, May 26, 2011 at 3:55 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Le 26/05/2011 07:25, Graeme Russ a ?crit :
>
>> I think we can still rationalise the timer API as suggested which, as
>> a minimum will:
>>
>> ?- Not diminish current functionality for any platform
>> ?- Will dramatically reduce the current amount of code duplication
>> ?- Improve functionality on some platforms
>> ?- Simplify the whole API
>>
>> and then start banging on arch maintainers heads to implement the trivial
>> ISR to kick the prescaler:
>
> I personally am in favor of cooperation rather than coercion, so please, ask
> before banging on heads. :)

What, and take all the fun out of it ;)

But yes, giving everyone plenty of warning was all in the plan

>> void timer_isr(void *unused)
>> {
>> ? ? ? ?prescaler();
>> }
>>
>> Regards,
>>
>> Graeme
>
> Seems like we're asymptotically getting to some kind of agreement. For the
> sake of clarity, could someone summarize what the expected API would be, so
> that I save my scalp and start looking as the order in which changes should
> be done on ARM?
>

Here is one I prepared earlier :)

http://lists.denx.de/pipermail/u-boot/2011-May/093290.html

I'll write up another formal specification for the API soon - Got a couple
of things on so it may be another day or two

Regards,

Graeme

[U-Boot] [RFC] Review of U-Boot timer API

[Reinhard Meyer]

Dear Albert ARIBAUD,
> Le 26/05/2011 07:25, Graeme Russ a ?crit :
> 
>> I think we can still rationalise the timer API as suggested which, as
>> a minimum will:
>>
>>   - Not diminish current functionality for any platform
>>   - Will dramatically reduce the current amount of code duplication
>>   - Improve functionality on some platforms
>>   - Simplify the whole API
>>
>> and then start banging on arch maintainers heads to implement the trivial
>> ISR to kick the prescaler:
> 
> I personally am in favor of cooperation rather than coercion, so please, 
> ask before banging on heads. :)
> 
>> void timer_isr(void *unused)
>> {
>> 	prescaler();
>> }
>>
>> Regards,
>>
>> Graeme
> 
> Seems like we're asymptotically getting to some kind of agreement. For 
> the sake of clarity, could someone summarize what the expected API would 
> be, so that I save my scalp and start looking as the order in which 
> changes should be done on ARM?

No, I don't think so. We currently have and need only two timer API functions
which are used in u-boot:

(1) void udelay(u32 microseconds)

and

(2) u32 get_timer(u32 base)

******************************************
I think we do not need to change that API.
******************************************

(maybe rename get_timer to get_timer_ms, but really why??)

What needs to be done is to eliminate all other timer functions that are
still used in some places (outside the timer domain) like get_ticks(),
set/reset_timer() and maybe other exotic ones.

And what needs to be fixed is all broken implementations of above TWO
functions.

Exactly how that fixing is done, and maybe generalized, factorized,
needlessly complicazed;), awkwardized;) should be another thread!

What should be specified for thsi API is in what time ranges above
functions MUST work, example: udelay() upto 1 second, get_timer
upto 1 year ;), and for reasons of implementation the maximum time
between two "related" get_timer calls.

Related in that sense means calls that are used to calculate a time difference
between them. Consider the following example:

since_epoch = get_timer(0);
while (get_timer(since_epoch) < timeout1)
	/* whatever1 */

/* other actions */

since_epoch = get_timer(0);
while (get_timer(since_epoch) < timeout2)
	/* whatever2 */

It does not matter whether the internal timer has had any wraps that
went undetected while "other actions" were executed.

And clearly, in all sane case, the "whatever" execution part cannot take
many seconds, so it is futile to think about any undetected wraps that need
tens of seconds or more to occur.

Best Regards,

Reinhard

[U-Boot] [RFC] Review of U-Boot timer API

[Wolfgang Denk]

Dear Graeme Russ,

In message <BANLkTi=B1BRT-HR23xVPtOqufpdbiTxrRA@mail.gmail.com> you wrote:
>
> and then start banging on arch maintainers heads to implement the trivial
> ISR to kick the prescaler:

I guess a lot of my confusion could be removed if you could think of a
better name for this function.  For me a "prescaler" has a very
definitive meaning, and it is nothing you need to "kick".

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
Men will always be men -- no matter where they are.
	-- Harry Mudd, "Mudd's Women", stardate 1329.8

[U-Boot] [RFC] Review of U-Boot timer API

[Graeme Russ]

On Thursday, May 26, 2011, Wolfgang Denk <wd@denx.de> wrote:
> Dear Graeme Russ,
>
> In message <BANLkTi=B1BRT-HR23xVPtOqufpdbiTxrRA@mail.gmail.com> you wrote:
>>
>> and then start banging on arch maintainers heads to implement the trivial
>> ISR to kick the prescaler:
>
> I guess a lot of my confusion could be removed if you could think of a
> better name for this function. ?For me a "prescaler" has a very
> definitive meaning, and it is nothing you need to "kick".

sync_timebase ?

Regards,

Graeme