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

[Simon Glass <sjg@chromium.org>]

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@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
>
>