[PATCH v2 2/5] board_init_f(): use static calls
Quentin Schulz
quentin.schulz at cherry.de
Wed Dec 18 18:27:37 CET 2024
Hi Jerome,
On 12/18/24 4:53 PM, Jerome Forissier wrote:
> Make static calls instead of iterating over the init_sequence_f arrays.
> Tested on a KV260 board (xilinx_zynqmp_kria_defconfig).
>
> - With LTO enabled, the code size reported by bloat-o-meter is 1200
> bytes less (-0.11%)
> - With LTO disabled, the code is 594 bytes smaller (-0.05%)
> - Execution time does not change in a noticeable way
>
> Signed-off-by: Jerome Forissier <jerome.forissier at linaro.org>
> ---
> common/board_f.c | 165 +++++++++++++++++++++++----------------------
> include/initcall.h | 27 ++++++++
> 2 files changed, 110 insertions(+), 82 deletions(-)
>
> diff --git a/common/board_f.c b/common/board_f.c
> index a4d8850cb7d..cebed85ed4d 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -38,6 +38,7 @@
> #include <spl.h>
> #include <status_led.h>
> #include <sysreset.h>
> +#include <time.h>
> #include <timer.h>
> #include <trace.h>
> #include <upl.h>
> @@ -870,58 +871,60 @@ static int initf_upl(void)
> return 0;
> }
>
> -static const init_fnc_t init_sequence_f[] = {
> - setup_mon_len,
> - CONFIG_IS_ENABLED(OF_CONTROL, (fdtdec_setup,))
> - CONFIG_IS_ENABLED(TRACE_EARLY, (trace_early_init,))
> - initf_malloc,
> - initf_upl,
> - log_init,
> - initf_bootstage, /* uses its own timer, so does not need DM */
> - event_init,
> - bloblist_maybe_init,
> - setup_spl_handoff,
> - CONFIG_IS_ENABLED(CONSOLE_RECORD_INIT_F, (console_record_init,))
> - INITCALL_EVENT(EVT_FSP_INIT_F),
> - arch_cpu_init, /* basic arch cpu dependent setup */
> - mach_cpu_init, /* SoC/machine dependent CPU setup */
> - initf_dm,
> - CONFIG_IS_ENABLED(BOARD_EARLY_INIT_F, (board_early_init_f,))
> +static void initcall_run_f(void)
> +{
> + INITCALL(setup_mon_len);
> + CONFIG_IS_ENABLED(OF_CONTROL, (INITCALL(fdtdec_setup)));
> + CONFIG_IS_ENABLED(TRACE_EARLY, (INITCALL(trace_early_init)));
> + INITCALL(initf_malloc);
> + INITCALL(initf_upl);
> + INITCALL(log_init);
> + INITCALL(initf_bootstage); /* uses its own timer, so does not need DM */
> + INITCALL(event_init);
> + INITCALL(bloblist_maybe_init);
> + INITCALL(setup_spl_handoff);
> + CONFIG_IS_ENABLED(CONSOLE_RECORD_INIT_F,
> + (INITCALL(console_record_init);))
> + INITCALL_EVT(EVT_FSP_INIT_F);
> + INITCALL(arch_cpu_init); /* basic arch cpu dependent setup */
> + INITCALL(mach_cpu_init); /* SoC/machine dependent CPU setup */
> + INITCALL(initf_dm);
> + CONFIG_IS_ENABLED(BOARD_EARLY_INIT_F, (INITCALL(board_early_init_f);))
> #if defined(CONFIG_PPC) || defined(CONFIG_SYS_FSL_CLK) || defined(CONFIG_M68K)
> /* get CPU and bus clocks according to the environment variable */
> - get_clocks, /* get CPU and bus clocks (etc.) */
> + INITCALL(get_clocks); /* get CPU and bus clocks (etc.) */
> #endif
> #if !defined(CONFIG_M68K) || (defined(CONFIG_M68K) && !defined(CONFIG_MCFTMR))
> - timer_init, /* initialize timer */
> + INITCALL(timer_init); /* initialize timer */
> #endif
> - CONFIG_IS_ENABLED(BOARD_POSTCLK_INIT, (board_postclk_init,))
> - env_init, /* initialize environment */
> - init_baud_rate, /* initialze baudrate settings */
> - serial_init, /* serial communications setup */
> - console_init_f, /* stage 1 init of console */
> - display_options, /* say that we are here */
> - display_text_info, /* show debugging info if required */
> - checkcpu,
> - CONFIG_IS_ENABLED(SYSRESET, (print_resetinfo,))
> + CONFIG_IS_ENABLED(BOARD_POSTCLK_INIT,
> + (INITCALL(board_postclk_init);))
> + INITCALL(env_init); /* initialize environment */
> + INITCALL(init_baud_rate); /* initialze baudrate settings */
> + INITCALL(serial_init); /* serial communications setup */
> + INITCALL(console_init_f); /* stage 1 init of console */
> + INITCALL(display_options); /* say that we are here */
> + INITCALL(display_text_info); /* show debugging info if required */
> + INITCALL(checkcpu);
> + CONFIG_IS_ENABLED(SYSRESET, (INITCALL(print_resetinfo);))
> /* display cpu info (and speed) */
> - CONFIG_IS_ENABLED(DISPLAY_CPUINFO, (print_cpuinfo,))
> - CONFIG_IS_ENABLED(DTB_RESELECT, (embedded_dtb_select,))
> - CONFIG_IS_ENABLED(DISPLAY_BOARDINFO, (show_board_info,))
> - INIT_FUNC_WATCHDOG_INIT
> - INITCALL_EVENT(EVT_MISC_INIT_F),
> - INIT_FUNC_WATCHDOG_RESET
> - CONFIG_IS_ENABLED(SYS_I2C_LEGACY, (init_func_i2c,))
> - announce_dram_init,
> - dram_init, /* configure available RAM banks */
> - CONFIG_IS_ENABLED(POST, (post_init_f,))
> - INIT_FUNC_WATCHDOG_RESET
> + CONFIG_IS_ENABLED(DISPLAY_CPUINFO, (INITCALL(print_cpuinfo);))
> + CONFIG_IS_ENABLED(DTB_RESELECT, (INITCALL(embedded_dtb_select);))
> + CONFIG_IS_ENABLED(DISPLAY_BOARDINFO, (INITCALL(show_board_info);))
> + WATCHDOG_INIT();
> + INITCALL_EVT(EVT_MISC_INIT_F);
> + WATCHDOG_RESET();
> + CONFIG_IS_ENABLED(SYS_I2C_LEGACY, (INITCALL(init_func_i2c);))
> + INITCALL(announce_dram_init);
> + INITCALL(dram_init); /* configure available RAM banks */
> + CONFIG_IS_ENABLED(POST, (INITCALL(post_init_f);))
> + WATCHDOG_INIT();
> #if defined(CFG_SYS_DRAM_TEST)
> - testdram,
> + INITCALL(testdram);
> #endif /* CFG_SYS_DRAM_TEST */
> - INIT_FUNC_WATCHDOG_RESET
> -
> - CONFIG_IS_ENABLED(POST, (init_post,))
> - INIT_FUNC_WATCHDOG_RESET
> + WATCHDOG_RESET();
> + CONFIG_IS_ENABLED(POST, (INITCALL(init_post);))
> + WATCHDOG_RESET();
> /*
> * Now that we have DRAM mapped and working, we can
> * relocate the code and continue running from DRAM.
> @@ -934,48 +937,48 @@ static const init_fnc_t init_sequence_f[] = {
> * - monitor code
> * - board info struct
> */
> - setup_dest_addr,
> + INITCALL(setup_dest_addr);
> #if defined(CONFIG_OF_BOARD_FIXUP) && !defined(CONFIG_OF_INITIAL_DTB_READONLY)
> - fix_fdt,
> + INITCALL(fix_fdt);
> #endif
> #ifdef CFG_PRAM
> - reserve_pram,
> + INITCALL(reserve_pram);
> #endif
> - reserve_round_4k,
> - setup_relocaddr_from_bloblist,
> - arch_reserve_mmu,
> - reserve_video,
> - reserve_trace,
> - reserve_uboot,
> - reserve_malloc,
> - reserve_board,
> - reserve_global_data,
> - reserve_fdt,
> + INITCALL(reserve_round_4k);
> + INITCALL(setup_relocaddr_from_bloblist);
> + INITCALL(arch_reserve_mmu);
> + INITCALL(reserve_video);
> + INITCALL(reserve_trace);
> + INITCALL(reserve_uboot);
> + INITCALL(reserve_malloc);
> + INITCALL(reserve_board);
> + INITCALL(reserve_global_data);
> + INITCALL(reserve_fdt);
> #if defined(CONFIG_OF_BOARD_FIXUP) && defined(CONFIG_OF_INITIAL_DTB_READONLY)
> - reloc_fdt,
> - fix_fdt,
> + INITCALL(reloc_fdt);
> + INITCALL(fix_fdt);
> #endif
> - reserve_bootstage,
> - reserve_bloblist,
> - reserve_arch,
> - reserve_stacks,
> - dram_init_banksize,
> - show_dram_config,
> - INIT_FUNC_WATCHDOG_RESET
> - setup_bdinfo,
> - display_new_sp,
> - INIT_FUNC_WATCHDOG_RESET
> + INITCALL(reserve_bootstage);
> + INITCALL(reserve_bloblist);
> + INITCALL(reserve_arch);
> + INITCALL(reserve_stacks);
> + INITCALL(dram_init_banksize);
> + INITCALL(show_dram_config);
> + WATCHDOG_RESET();
> + INITCALL(setup_bdinfo);
> + INITCALL(display_new_sp);
> + WATCHDOG_RESET();
> #if !defined(CONFIG_OF_BOARD_FIXUP) || !defined(CONFIG_OF_INITIAL_DTB_READONLY)
> - reloc_fdt,
> + INITCALL(reloc_fdt);
> #endif
> - reloc_bootstage,
> - reloc_bloblist,
> - setup_reloc,
> + INITCALL(reloc_bootstage);
> + INITCALL(reloc_bloblist);
> + INITCALL(setup_reloc);
> #if defined(CONFIG_X86) || defined(CONFIG_ARC)
> - copy_uboot_to_ram,
> - do_elf_reloc_fixups,
> + INITCALL(copy_uboot_to_ram);
> + INITCALL(do_elf_reloc_fixups);
> #endif
> - clear_bss,
> + INITCALL(clear_bss);
> /*
> * Deregister all cyclic functions before relocation, so that
> * gd->cyclic_list does not contain any references to pre-relocation
> @@ -985,12 +988,11 @@ static const init_fnc_t init_sequence_f[] = {
> * This should happen as late as possible so that the window where a
> * watchdog device is not serviced is as small as possible.
> */
> - cyclic_unregister_all,
> + INITCALL(cyclic_unregister_all);
> #if !defined(CONFIG_ARM) && !defined(CONFIG_SANDBOX)
> - jump_to_copy,
> + INITCALL(jump_to_copy);
> #endif
> - NULL,
> -};
> +}
>
> void board_init_f(ulong boot_flags)
> {
> @@ -1000,8 +1002,7 @@ void board_init_f(ulong boot_flags)
> gd->flags &= ~GD_FLG_HAVE_CONSOLE;
> gd->boardf = &boardf;
>
> - if (initcall_run_list(init_sequence_f))
> - hang();
> + initcall_run_f();
>
> #if !defined(CONFIG_ARM) && !defined(CONFIG_SANDBOX) && \
> !defined(CONFIG_EFI_APP) && !CONFIG_IS_ENABLED(X86_64) && \
> diff --git a/include/initcall.h b/include/initcall.h
> index 62d3bb67f08..9398a3ec7d5 100644
> --- a/include/initcall.h
> +++ b/include/initcall.h
> @@ -8,6 +8,7 @@
>
> #include <asm/types.h>
> #include <event.h>
> +#include <hang.h>
>
> _Static_assert(EVT_COUNT < 256, "Can only support 256 event types with 8 bits");
>
> @@ -35,4 +36,30 @@ typedef int (*init_fnc_t)(void);
> */
> int initcall_run_list(const init_fnc_t init_sequence[]);
>
> +#define INITCALL(_call) \
> + do { \
> + if (_call()) { \
> + debug("%s(): calling %s() failed\n", __func__, \
> + #_call); \
> + hang(); \
> + } \
> + } while (0)
> +
> +#define INITCALL_EVT(_evt) \
> + do { \
> + if (event_notify_null(_evt)) { \
> + debug("%s(): event %d/%s failed\n", __func__, _evt, \
> + event_type_name(_evt)) ; \
> + hang(); \
> + } \
> + } while (0)
> +
initcall_run_list() currently prints (printf) whenever an initcall
fails. It's happened to me a lot already while debugging/bringing up
boards to have an initcall fail on me and that message wasn't really
enough to put me on the right track, but at least I had something. Now I
believe this would just hang without notifying you before. Is my
understanding correct?
Would it be possible to print an error message (or raise the loglevel
from debug to error or something like that?) before hang()?
Cheers,
Quentin
More information about the U-Boot
mailing list