[PATCH v2 2/5] board_init_f(): use static calls
Jerome Forissier
jerome.forissier at linaro.org
Wed Dec 18 19:21:57 CET 2024
On 12/18/24 18:47, Quentin Schulz wrote:
> Hi Jerome,
>
> On 12/18/24 6:38 PM, Jerome Forissier wrote:
>> Hi Quentin,
>>
>> On 12/18/24 18:27, Quentin Schulz wrote:
>>> 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?
>>
>> The debug print is still there, the line just before hang() ;-)
>>
>
> It's now a debug() instead of a printf() is what I meant. So it's not shown by default anymore I believe, unless we have #define DEBUG in the file?
Oh, right. There was a debug() before the initcall and a printf() in
case of error. Sorry for overlooking this. I will change the debug() to
a printf().
Thanks,
--
Jerome
>
> Cheers,
> Quentin
More information about the U-Boot
mailing list