[U-Boot] [PATCH V2] arm: timer and interrupt init rework

[Wolfgang Denk]

Dear Jean-Christophe PLAGNIOL-VILLARD,

In message <20090501232305.GI3291 at 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.]


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
Thought for the day: What if there were no hypothetical situations?