[PATCH 2/5] board_init_f(): use static calls
Jerome Forissier
jerome.forissier at linaro.org
Wed Dec 18 15:02:51 CET 2024
On 12/18/24 13:38, Ilias Apalodimas wrote:
> Hi Jerome
>
> On Tue, 17 Dec 2024 at 18:00, Jerome Forissier
> <jerome.forissier at linaro.org> 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 1184
>> bytes less (-0.11%)
>> - With LTO disabled, the code is 592 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 | 167 +++++++++++++++++++++++----------------------
>> include/initcall.h | 25 +++++++
>> 2 files changed, 111 insertions(+), 81 deletions(-)
>>
>> diff --git a/common/board_f.c b/common/board_f.c
>> index a4d8850cb7d..ba1af3d8055 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,62 @@ 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 int initcall_run_f(void)
>> +{
>> + int ret = 0;
>> +
>> + 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 +939,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 +990,12 @@ 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,
>> -};
>> + return ret;
>> +}
>>
>> void board_init_f(ulong boot_flags)
>> {
>> @@ -1000,7 +1005,7 @@ void board_init_f(ulong boot_flags)
>> gd->flags &= ~GD_FLG_HAVE_CONSOLE;
>> gd->boardf = &boardf;
>>
>> - if (initcall_run_list(init_sequence_f))
>> + if (initcall_run_f())
>> hang();
>>
>> #if !defined(CONFIG_ARM) && !defined(CONFIG_SANDBOX) && \
>> diff --git a/include/initcall.h b/include/initcall.h
>> index 62d3bb67f08..72faab42ada 100644
>> --- a/include/initcall.h
>> +++ b/include/initcall.h
>> @@ -35,4 +35,29 @@ typedef int (*init_fnc_t)(void);
>> */
>> int initcall_run_list(const init_fnc_t init_sequence[]);
>>
>> +#define INITCALL(_call) \
>> + do { \
>> + if (!ret) { \
>> + debug("%s(): calling %s()\n", __func__, #_call); \
>> + ret = _call(); \
>> + } \
>> + } while (0)
>> +
>> +#define INITCALL_EVT(_evt) \
>> + do { \
>> + if (!ret) { \
>> + debug("%s(): event %d/%s\n", __func__, _evt, \
>> + event_type_name(_evt)) ; \
>> + ret = event_notify_null(_evt); \
>> + } \
>> + } while (0)
>> +
>
> I think you want to run the function and return the error code here?
No, we want to run the function only if the previous one has not failed
(in other words, stop running the initcalls as soon as one fails).
The macro acts on the caller's 'ret' variable, which is not good
practice, I know.
>
> Something like
> #define INITCALL(_call) \
> do { \
> int _ret; _ret = _call(); \
> if (!ret) \
> debug("%s(): calling %s() failed\n", __func__, #_call); \
> _ret; \
> } while (0)
In fact, since the caller of initcall_run_f() will call hang() on error
we may as well do it sooner:
#define INITCALL(_call) \
do { \
if (_call()) { \
debug("%s(): calling %s() failed\n", __func__, \
#_call); \
hang(); \
} \
} while(0)
I'll post a v2. Thanks for reviewing.
--
Jerome
>
> Cheers
> /Ilias
>
>> +#if defined(CONFIG_WATCHDOG) || defined(CONFIG_HW_WATCHDOG)
>> +#define WATCHDOG_INIT() INITCALL(init_func_watchdog_init)
>> +#define WATCHDOG_RESET() INITCALL(init_func_watchdog_reset)
>> +#else
>> +#define WATCHDOG_INIT()
>> +#define WATCHDOG_RESET()
>> +#endif
>> +
>> #endif
>> --
>> 2.43.0
>>
More information about the U-Boot
mailing list