Wolfgang Denk wrote:
Dear Jean-Christophe PLAGNIOL-VILLARD,
In message 20090501232305.GI3291@game.jcrosoft.org you wrote:
+COBJS += board.o +COBJS += clock.o +COBJS += mem.o +COBJS += syslib.o +COBJS += sys_info.o +COBJS += timer.o
What do we win with this?
simple to allow vertical patch to be applied instead of have merge problem
so yes it's needed
But it must go in a separate patch.
diff --git a/lib_arm/board.c b/lib_arm/board.c index 5d05d9b..b678a63 100644 --- a/lib_arm/board.c +++ b/lib_arm/board.c @@ -265,8 +265,11 @@ init_fnc_t *init_sequence[] = { #if defined(CONFIG_ARCH_CPU_INIT) arch_cpu_init, /* basic arch cpu dependent setup */ #endif
- board_init, /* basic board dependent setup */
+#if defined(CONFIG_USE_IRQ) interrupt_init, /* set up exceptions */ +#endif
- timer_init, /* initialize timer */
- board_init, /* basic board dependent setup */ env_init, /* initialize environment */ init_baudrate, /* initialze baudrate settings */ serial_init, /* serial communications setup */
... if you tested this on an OMAP3 board: I'm not sure, but it seems to me that the initialization order might change by this?
maybe read the commit message will answer your question
Argh. Instead of snippy remarks you should read Dirks message yourself and answer his (very valid) questions:
| Is this correct? If yes, we have to check that there are no issues | with dependencies? | | On which OMAP3 board have you tested this?
Can you please explain on which boards this has actually been tested, and especially on which OMAP3 boards?
Also, I do not see why we need to implement such a critical change.
If I understand you corrctly, your argument goes that board_init() needs delays (like udelay()), delays need timers, and timers need interrupts, so we must initialize first interrupts, then timers, and only then we can run board_init()? Is this your argument?
But the I ask why udelay() would need timers and interrupts? This does not fit into the design philosophy of U-Boot, which attempts to bring up a board at least to a state where we have serial console output with as little as possible requirements. Your change breaks this, because now we have to initialize timers and interrupts (which are not exactly a trivial thing to set up or debug if they aren't working correctly) BEFORE we have a console output. [I ignore the case of CONFIG_USE_IRQ here, because only 4 boards actually use this feature, and they could probably be changed to do without, too.]
So while I really appreciate your attempts to clean up the timer code on ARM, the resulting consequences are expensive, and I am not yet convincet the advantages of the new code are bigger than this disadvantage, and especially I am not convinced thatthis is really necessary and unavoidable.
Can we not do delays without interrupts? And do we need full-blown timer services for delays? [Keep in mind that a delay is usually used to implement a timeout in the error branch; that means, it does not matter if it has not 10e-6 precision or better.]
Btw, it seems that this patch is already in u-boot-arm next
http://git.denx.de/?p=u-boot/u-boot-arm.git;a=commit;h=482d69eafb6a78c82251f...
Did I miss an ACK somewhere? It's my understanding that this patch is still under discussion? Sorry if I missed something ;)
Best regards
Dirk