Dear Joakim Tjernlund,

In message <1259317926-9820-1-git-send-email-joakim.tjernl...@transmode.se> you 
wrote:
> init_sequence is an array with function pointers.
> It produces lots of relocation data and it
> is hard to debug when something fails.
> 
> Transform it into a function, making it smaller
> and easier to debug.
>    text          data     bss     dec     hex filename
>    1268           212       0    1480     5c8 lib_ppc/board.org
>    1224            92       0    1316     524 lib_ppc/board.new

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.

> -init_fnc_t *init_sequence[] = {
> +void init_sequence(void)
> +{
>  #if defined(CONFIG_MPC85xx) || defined(CONFIG_MPC86xx)
> -     probecpu,
> +     if (probecpu())
> +             goto err_out;
>  #endif
>  #if defined(CONFIG_BOARD_EARLY_INIT_F)
> -     board_early_init_f,
> +     if (board_early_init_f())
> +             goto err_out;
>  #endif
>  #if !defined(CONFIG_8xx_CPUCLK_DEFAULT)
> -     get_clocks,             /* get CPU and bus clocks (etc.) */
> +     if (get_clocks())
> +             goto err_out;   /* get CPU and bus clocks (etc.) */
>  #if defined(CONFIG_TQM8xxL) && !defined(CONFIG_TQM866M) \
>      && !defined(CONFIG_TQM885D)
> -     adjust_sdram_tbs_8xx,
> +     if (adjust_sdram_tbs_8xx())
> +             goto err_out;
>  #endif
> -     init_timebase,
> +     if (init_timebase())
> +             goto err_out;

This is much more ugly, and I cannot see why it would be easier to
debug.

The original idea of defining an array of function pointed was to
introduce a bigger level of flexibility. There was a time when people
complained about the fixed initialization sequence. So my thinking
was that it should be possible to simply #define in you board config
file a list of function pointers to initialize init_sequence[], i. e.
allow for completely board specific init sequences.

OK, you can argument that nobody used this feature yeat, or that you
could provide a weak implementation of the new init_sequence()
function, or ... but just for saving 164 Bytes and adding a lot of
ugliness?

Thank you, but no.

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: w...@denx.de
An Elephant is a mouse with an Operating System.              - Knuth
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to