[U-Boot] [PATCH] ppc: transform init_sequence into a function.

[Joakim Tjernlund]

Wolfgang Denk <wd at denx.de> wrote on 27/11/2009 21:18:28:

> From: Wolfgang Denk <wd at denx.de>
> To: Joakim Tjernlund <joakim.tjernlund at transmode.se>
> Cc: u-boot at lists.denx.de
> Date: 27/11/2009 21:18
> Subject: Re: [U-Boot] [PATCH] ppc: transform init_sequence into a function.
>
> Dear Joakim Tjernlund,
>
> In message <OF012B1DA7.446FFBCD-ONC125767B.004DD4EE-C125767B.
> 00508FF3 at transmode.se> you wrote:
> >
> > > You know that I'm a really big fan of small code, and I tend to
> > > accept a certain amount of ugliness if it saves memory. But here I
> > > just disagree.
> >
> > I think most of the ugliness comes from the #ifdef hell in this list.
> > replacing the #ifdefs is another matter so looking behind
> > the #ifdef mess, I don't think it looks too bad.
>
> Well, remove the #ifdef's from the pointer array, and it will be
> beautiful, too?

Both versions will look a lot better when the #ifdefs are gone. I find
the current #ifdefs more ugly than my new function. You don't
obviously as otherwise you would not have the #ifdefs in the first place.

>
> > > This is much more ugly, and I cannot see why it would be easier to
> > > debug.
> >
> > You can set breakpoints anywhere you like. When it crashes in here
> > somewhere, it isn't easy to tell where it did so.
>
> Umm... you can set breakpoints on any of the functions now, too?
> And printing the funtion pointer will show you easily where you are,
> right?

Yes, but it isn't as easy as placing bp's in the new function.
To find the problem function you would have print the fptr, stepover
repeat until done. You probably need to do this a few times as
mistakes are common and you will have to reboot and start from the beginning.
Also, depending on the board and emulator, it might not be possible/easy to continue
after you hit a BP because the emulator flushes the cache(or something else), then
you have to find each function source code(lots of unwanted typing) and place
the BP inside the function. If you are very lucky, it was the right one, otherwise
go find some other function and repeat.
Finally, if you don't have debugger handy, you will have stick some debug code
into each function to see which one that causes the problem as opposed to
add some debug code into my new function and move it around a bit until
you have identified the problematic function.

While it is possible to do all this with the current way, it isn't
as easy as with my new function, period.

>
> > There is one more minor advantage too. When bringing up a board, relocation
> > may be off and you will get an very early crash, avoiding relocs here might
> > get you a bit further, possibly making it easier to pin point the problem.
> > This is all speculation though.
>
> Of all the problems I ever had with bringing up new  hardware,  relo-
> cation  has  never  been  one  of them. Relocation is pretty hardware
> indepentent - either it works, or it doesn't.
>
> > Anyhow, I don't feel too strongly about this. If you want to drop it, I won't
> > cry :)
>
> I am not convinced yet that the new code is actually an improvement.
> Changing the array of pointers into a list of function calls does not
> solve any of the real issues we have with the init seuqnece - like
> that some board need the PCI initialization early, and others later,
> etc.

What has PCI initialization with my new function to do? I never
claimed it would solve such problems. You are side tracking
the point here which is:
- reduce code size.
- easier debugging.
against keeping a beautiful list of function ptrs, while it is
pretty, it is not as useful as my new function.

 Jocke