On 07/10/2012 08:36 PM, Stephan Linz wrote:
Am Dienstag, den 10.07.2012, 10:16 +0200 schrieb Michal Simek:
On 07/09/2012 09:06 PM, Stephan Linz wrote:
Am Montag, den 09.07.2012, 11:20 +0200 schrieb Michal Simek:
microblaze: Fix CONFIG_SYS_HZ usage in board config
Do not use hardcoded value. Use CONFIG_SYS_HZ instead. Separate static configuration to single block.
Signed-off-by: Michal Simekmonstr@monstr.eu
arch/microblaze/cpu/timer.c | 69 ++++++++++++----------- arch/microblaze/include/asm/microblaze_timer.h | 3 + arch/microblaze/lib/board.c | 5 -- include/configs/microblaze-generic.h | 12 +---- 4 files changed, 41 insertions(+), 48 deletions(-)
diff --git a/arch/microblaze/cpu/timer.c b/arch/microblaze/cpu/timer.c index cc6b897..dfaaaf5 100644 --- a/arch/microblaze/cpu/timer.c +++ b/arch/microblaze/cpu/timer.c @@ -27,42 +27,30 @@ #include<asm/microblaze_intc.h>
volatile int timestamp = 0; +microblaze_timer_t *tmr;
-#ifdef CONFIG_SYS_TIMER_0 ulong get_timer (ulong base) {
- return (timestamp - base);
- if (tmr)
return timestamp - base;- return timestamp++ - base; }
-#else -ulong get_timer (ulong base) -{
- return (timestamp++ - base);
-} -#endif
-#ifdef CONFIG_SYS_TIMER_0 void __udelay(unsigned long usec) {
- int i;
- u32 i;
- i = get_timer(0);
- while ((get_timer(0) - i)< (usec / 1000))
;
- if (tmr) {
i = get_timer(0);while ((get_timer(0) - i)< (usec / 1000));Hi Michal,
- } else {
for (i = 0; i< (usec * XILINX_CLOCK_FREQ / 10000000); i++);this part should be enclosed by #ifdef XILINX_CLOCK_FREQ
It was intentional because XILINX_CLOCK_FREQ must be define.
Hm, it's a catch-22 situation in between compilation and run time.
With a prober fdt setup you will implicitly fetch this value from dts (as tmr->loadreg) and you will never reach this code. In the case of a wrong dts content, for example a missing "clock-frequency" entry, this code snipped could be a fall back solution. But what is wrong and what is right?
Correct - u-boot is able to work without timer and it is fall back solution. My point was that XILINX_CLOCK_FREQ will be setup to any default value just for sure.
I think you should make a hard cut here and make a strict distinguish between the old (undef CONFIG_OF_CONTROL) and the new timer setup. Your current code base leads to a hardware description with overdetermined definition of timer clock frequency:
Not going to do hard cut. We can do it in future that we will use only DTS initialization but it is long way to go.
first one here: board/xilinx/microblaze-generic/xparameters.h
It should be define here for non FDT platform and my preference is completely remove xparameters for FDT platform.
second one here: board/xilinx/microblaze-generic/dts/microblaze.dts
For FDT platform with timer connected to intc.
Maybe it could be handle by
#ifndef XILINX_CLOCK_FREQ # error Please define XILINX_CLOCK_FREQ #endif
better would be:
#if !defined(CONFIG_OF_CONTROL)&& !defined(XILINX_CLOCK_FREQ) # error Please define XILINX_CLOCK_FREQ #endif
Still make sense to define default value for CONFIG_OF_CONTROL for that fall back solution which should be there.
There is also one more case for system which contains several timers where some of them have IRQ connected and some not. This parsing just read the first timer with selected compatible property. But this is different topic.
Thanks, Michal