[U-Boot] [RFC][PATCH] Replace clunky init sequence architecture

Simon Glass sjg at chromium.org
Thu Aug 18 18:41:45 CEST 2011


Hi Graeme,

This looks great to me.

- perhaps INIT_FUNC() for post-relocation ones, have a separate
INIT_FUNC_F or INIT_FUNC_PRE_RELOC for the others (which should be
uncommon).

- I wonder if do_init_loop could panic and pass as message the
function address which failed, or perhaps return that information?

Regards
Simon

On Thu, Aug 18, 2011 at 5:34 AM, Graeme Russ <graeme.russ at gmail.com> wrote:
> NOTE: This is an x86 only patch - I didn't include x86 in the patch heading
> because it is a proof of concept for a global patch - Sorry for the blatant
> etiquette violation ;)
>
> Replace the init_fnc_t *init_sequence[] style init sequence with one based
> on the Linux __initcall macros.
>
> Functions are declared as initialisation functions by using the new
> INIT_FUNC() macro. The three parameters to INIT_FUNC() are:
>  - Type - f = pre-relocation (flash), r = post-relocation (RAM)
>  - Sequence - Lower numbers are run first
>  - Function - The function to call during the init sequence
>
>  INIT_FUNC() creates a static variable which is a pointer to the init
>  function to be called. Each variable is placed in a sub-section under
>  the .initfuncs_f or .initfuncs_r sections. The sub-section name is the
>  sequence number. There is no problem giving multiple functions the same
>  sequence number, but the order of execution of functions with the same
>  sequence number is undefined. The resulting raw image layout is a
>  sequence of function pointers (much like how the command table is built)
>
>  do_init_loop() simply takes a pointer to the start and end of either the
>  initfuncs_f or initfuncs_r sections and sequentially dereferences the
>  function pointer and calls the function.
>
>  For a board to add an arbitrary initialisation function is trivial -
>  simply add a INIT_FUNC() entry with an appropriate sequence number
>
>  I imagine the sequence numbers could be #defined, but I don't exactly
>  now what the linker will do - I use leading zeros in the sequence numbers
>  to ensure correct ordering.
>
>  This has been build and run-tested on my eNET board and works perfectly
>  (after a few false starts)
>
>  Enjoy :)
>
> ---
>  arch/x86/cpu/cpu.c               |    1 +
>  arch/x86/cpu/sc520/sc520.c       |    1 +
>  arch/x86/cpu/sc520/sc520_sdram.c |    2 +
>  arch/x86/cpu/sc520/sc520_timer.c |    1 +
>  arch/x86/cpu/u-boot.lds          |   10 ++++++
>  arch/x86/lib/board.c             |   66 ++++++++++++++++----------------------
>  arch/x86/lib/pcat_interrupts.c   |    1 +
>  board/eNET/eNET.c                |    2 +
>  common/console.c                 |    1 +
>  common/env_flash.c               |    2 +
>  common/serial.c                  |    1 +
>  include/common.h                 |    8 ++++
>  12 files changed, 58 insertions(+), 38 deletions(-)
>
> diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
> index cac12c0..610b8d8 100644
> --- a/arch/x86/cpu/cpu.c
> +++ b/arch/x86/cpu/cpu.c
> @@ -119,6 +119,7 @@ int x86_cpu_init_r(void)
>        return 0;
>  }
>  int cpu_init_r(void) __attribute__((weak, alias("x86_cpu_init_r")));
> +INIT_FUNC(r, 010, cpu_init_r);
>
>  int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  {
> diff --git a/arch/x86/cpu/sc520/sc520.c b/arch/x86/cpu/sc520/sc520.c
> index e37c403..fc2996a 100644
> --- a/arch/x86/cpu/sc520/sc520.c
> +++ b/arch/x86/cpu/sc520/sc520.c
> @@ -53,6 +53,7 @@ int cpu_init_f(void)
>
>        return x86_cpu_init_f();
>  }
> +INIT_FUNC(f, 010, cpu_init_f);
>
>  int cpu_init_r(void)
>  {
> diff --git a/arch/x86/cpu/sc520/sc520_sdram.c b/arch/x86/cpu/sc520/sc520_sdram.c
> index f3623f5..5f836f3 100644
> --- a/arch/x86/cpu/sc520/sc520_sdram.c
> +++ b/arch/x86/cpu/sc520/sc520_sdram.c
> @@ -57,6 +57,7 @@ int dram_init_f(void)
>
>        return 0;
>  }
> +INIT_FUNC(f, 070, dram_init_f);
>
>  static inline void sc520_dummy_write(void)
>  {
> @@ -530,3 +531,4 @@ int dram_init(void)
>
>        return 0;
>  }
> +INIT_FUNC(r, 030, dram_init);
> diff --git a/arch/x86/cpu/sc520/sc520_timer.c b/arch/x86/cpu/sc520/sc520_timer.c
> index 5cccda1..56a53bc 100644
> --- a/arch/x86/cpu/sc520/sc520_timer.c
> +++ b/arch/x86/cpu/sc520/sc520_timer.c
> @@ -69,6 +69,7 @@ int timer_init(void)
>
>        return 0;
>  }
> +INIT_FUNC(r, 050, timer_init);
>
>  /* Allow boards to override udelay implementation */
>  void __udelay(unsigned long usec)
> diff --git a/arch/x86/cpu/u-boot.lds b/arch/x86/cpu/u-boot.lds
> index fe28030..7b2ab7a 100644
> --- a/arch/x86/cpu/u-boot.lds
> +++ b/arch/x86/cpu/u-boot.lds
> @@ -54,6 +54,16 @@ SECTIONS
>        .got : { *(.got*) }
>
>        . = ALIGN(4);
> +       __initfuncs_r_start = .;
> +       .initfuncs_r : { KEEP(*(SORT_BY_NAME(.initfuncs_r*))) }
> +       __initfuncs_r_end = .;
> +
> +       . = ALIGN(4);
> +       __initfuncs_f_start = .;
> +       .initfuncs_f : { KEEP(*(SORT_BY_NAME(.initfuncs_f*))) }
> +       __initfuncs_f_end = .;
> +
> +       . = ALIGN(4);
>        __data_end = .;
>
>        . = ALIGN(4);
> diff --git a/arch/x86/lib/board.c b/arch/x86/lib/board.c
> index b1b8680..e023f58 100644
> --- a/arch/x86/lib/board.c
> +++ b/arch/x86/lib/board.c
> @@ -64,6 +64,10 @@ extern ulong __rel_dyn_start;
>  extern ulong __rel_dyn_end;
>  extern ulong __bss_start;
>  extern ulong __bss_end;
> +extern ulong __initfuncs_f_start;
> +extern ulong __initfuncs_f_end;
> +extern ulong __initfuncs_r_start;
> +extern ulong __initfuncs_r_end;
>
>  /************************************************************************
>  * Init Utilities                                                      *
> @@ -83,6 +87,7 @@ static int init_baudrate (void)
>
>        return (0);
>  }
> +INIT_FUNC(f, 040, init_baudrate);
>
>  static int display_banner (void)
>  {
> @@ -101,6 +106,7 @@ static int display_banner (void)
>
>        return (0);
>  }
> +INIT_FUNC(r, 060, display_banner);
>
>  static int display_dram_config (void)
>  {
> @@ -115,6 +121,7 @@ static int display_dram_config (void)
>
>        return (0);
>  }
> +INIT_FUNC(r, 070, display_dram_config);
>
>  static void display_flash_config (ulong size)
>  {
> @@ -152,34 +159,6 @@ static int copy_uboot_to_ram(void);
>  static int clear_bss(void);
>  static int do_elf_reloc_fixups(void);
>
> -init_fnc_t *init_sequence_f[] = {
> -       cpu_init_f,
> -       board_early_init_f,
> -       env_init,
> -       init_baudrate,
> -       serial_init,
> -       console_init_f,
> -       dram_init_f,
> -       calculate_relocation_address,
> -       copy_uboot_to_ram,
> -       clear_bss,
> -       do_elf_reloc_fixups,
> -
> -       NULL,
> -};
> -
> -init_fnc_t *init_sequence_r[] = {
> -       cpu_init_r,             /* basic cpu dependent setup */
> -       board_early_init_r,     /* basic board dependent setup */
> -       dram_init,              /* configure available RAM banks */
> -       interrupt_init,         /* set up exceptions */
> -       timer_init,
> -       display_banner,
> -       display_dram_config,
> -
> -       NULL,
> -};
> -
>  gd_t *gd;
>
>  static int calculate_relocation_address(void)
> @@ -201,6 +180,7 @@ static int calculate_relocation_address(void)
>
>        return 0;
>  }
> +INIT_FUNC(f, 080, calculate_relocation_address);
>
>  static int copy_uboot_to_ram(void)
>  {
> @@ -213,6 +193,7 @@ static int copy_uboot_to_ram(void)
>
>        return 0;
>  }
> +INIT_FUNC(f, 090, copy_uboot_to_ram);
>
>  static int clear_bss(void)
>  {
> @@ -227,6 +208,7 @@ static int clear_bss(void)
>
>        return 0;
>  }
> +INIT_FUNC(f, 100, clear_bss);
>
>  static int do_elf_reloc_fixups(void)
>  {
> @@ -241,18 +223,25 @@ static int do_elf_reloc_fixups(void)
>
>        return 0;
>  }
> +INIT_FUNC(f, 110, do_elf_reloc_fixups);
> +
> +static void do_init_loop(init_fnc_t **fnc, init_fnc_t **end)
> +{
> +       do {
> +               if ((*fnc)() != 0)
> +                               hang();
> +       } while (fnc++ < end);
> +}
>
>  /* Load U-Boot into RAM, initialize BSS, perform relocation adjustments */
>  void board_init_f(ulong boot_flags)
>  {
> -       init_fnc_t **init_fnc_ptr;
> +       init_fnc_t **init_fnc_start = (init_fnc_t **)(&__initfuncs_f_start);
> +       init_fnc_t **init_fnc_end = (init_fnc_t **)(&__initfuncs_f_end);
>
>        gd->flags = boot_flags;
>
> -       for (init_fnc_ptr = init_sequence_f; *init_fnc_ptr; ++init_fnc_ptr) {
> -               if ((*init_fnc_ptr)() != 0)
> -                       hang();
> -       }
> +       do_init_loop(init_fnc_start, --init_fnc_end);
>
>        gd->flags |= GD_FLG_RELOC;
>
> @@ -269,7 +258,9 @@ void board_init_r(gd_t *id, ulong dest_addr)
>        ulong size;
>        static bd_t bd_data;
>        static gd_t gd_data;
> -       init_fnc_t **init_fnc_ptr;
> +
> +       init_fnc_t **init_fnc_start = (init_fnc_t **)(&__initfuncs_r_start);
> +       init_fnc_t **init_fnc_end = (init_fnc_t **)(&__initfuncs_r_end);
>
>        show_boot_progress(0x21);
>
> @@ -289,12 +280,11 @@ void board_init_r(gd_t *id, ulong dest_addr)
>        mem_malloc_init((((ulong)dest_addr - CONFIG_SYS_MALLOC_LEN)+3)&~3,
>                        CONFIG_SYS_MALLOC_LEN);
>
> -       for (init_fnc_ptr = init_sequence_r; *init_fnc_ptr; ++init_fnc_ptr) {
> -               if ((*init_fnc_ptr)() != 0)
> -                       hang ();
> -       }
> +       do_init_loop(init_fnc_start, --init_fnc_end);
>        show_boot_progress(0x23);
>
> +
> +
>  #ifdef CONFIG_SERIAL_MULTI
>        serial_initialize();
>  #endif
> diff --git a/arch/x86/lib/pcat_interrupts.c b/arch/x86/lib/pcat_interrupts.c
> index 2caae20..46514b0 100644
> --- a/arch/x86/lib/pcat_interrupts.c
> +++ b/arch/x86/lib/pcat_interrupts.c
> @@ -82,6 +82,7 @@ int interrupt_init(void)
>
>        return 0;
>  }
> +INIT_FUNC(r, 040, interrupt_init);
>
>  void mask_irq(int irq)
>  {
> diff --git a/board/eNET/eNET.c b/board/eNET/eNET.c
> index 2a5636c..f49c3a5 100644
> --- a/board/eNET/eNET.c
> +++ b/board/eNET/eNET.c
> @@ -106,6 +106,7 @@ int board_early_init_f(void)
>
>        return 0;
>  }
> +INIT_FUNC(f, 020, board_early_init_f);
>
>  static void enet_setup_pars(void)
>  {
> @@ -161,6 +162,7 @@ int board_early_init_r(void)
>
>        return 0;
>  }
> +INIT_FUNC(r, 020, board_early_init_r);
>
>  void show_boot_progress(int val)
>  {
> diff --git a/common/console.c b/common/console.c
> index 8c650e0..559c799 100644
> --- a/common/console.c
> +++ b/common/console.c
> @@ -531,6 +531,7 @@ int console_init_f(void)
>
>        return 0;
>  }
> +INIT_FUNC(f, 060, console_init_f);
>
>  void stdio_print_current_devices(void)
>  {
> diff --git a/common/env_flash.c b/common/env_flash.c
> index 50ca4ffa..a63c108 100644
> --- a/common/env_flash.c
> +++ b/common/env_flash.c
> @@ -126,6 +126,7 @@ int  env_init(void)
>
>        return 0;
>  }
> +INIT_FUNC(f, 030, env_init);
>
>  #ifdef CMD_SAVEENV
>  int saveenv(void)
> @@ -252,6 +253,7 @@ int  env_init(void)
>        gd->env_valid = 0;
>        return 0;
>  }
> +INIT_FUNC(f, 030, env_init);
>
>  #ifdef CMD_SAVEENV
>
> diff --git a/common/serial.c b/common/serial.c
> index 995d268..5d097d8 100644
> --- a/common/serial.c
> +++ b/common/serial.c
> @@ -168,6 +168,7 @@ int serial_init (void)
>
>        return serial_current->init ();
>  }
> +INIT_FUNC(f, 050, serial_init);
>
>  void serial_setbrg (void)
>  {
> diff --git a/include/common.h b/include/common.h
> index 12a1074..61126f1 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -39,8 +39,16 @@ typedef volatile unsigned char       vu_char;
>  #include <linux/bitops.h>
>  #include <linux/types.h>
>  #include <linux/string.h>
> +#include <linux/compiler.h>
>  #include <asm/ptrace.h>
>  #include <stdarg.h>
> +
> +typedef int (*initfunc_t)(void);
> +
> +#define INIT_FUNC(stage,step,fn) \
> +       static initfunc_t __initfunc_ ## fn ## stage __used \
> +       __attribute__((__section__(".initfuncs_" #stage "." #step))) = fn
> +
>  #if defined(CONFIG_PCI) && (defined(CONFIG_4xx) && !defined(CONFIG_AP1000))
>  #include <pci.h>
>  #endif
> --
> 1.7.5.2.317.g391b14
>
>


More information about the U-Boot mailing list