[U-Boot] [RFC] Review of U-Boot timer API
Graeme Russ
graeme.russ at gmail.com
Sat May 21 14:38:29 CEST 2011
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!)
More information about the U-Boot
mailing list