[PATCH v2 0/5] Static initcalls
This series replaces the dynamic initcalls (with function pointers) with static calls, and gets rid of initcall_run_list(), init_sequence_f, init_sequence_f_r and init_sequence_r. This makes the code simpler and the binary slighlty smaller: -2507 bytes/-0.23 % with LTO enabled and -1232 bytes/-0.11 % with LTO disabled (xilinx_zynqmp_kria_defconfig).
Execution time doesn't seem to change noticeably. There is no impact on the SPL.
Changes in v2: - INTICALL() and INITCALL_EVT() now call hang() immediately on error - Fixed typo: s/intcall_run_f_r/initcall_run_f_r/
Jerome Forissier (4): board_init_f(): use static calls board_init_f_r(): use static calls board_init_r(): use static calls initcall: remove initcall_run_list()
Michal Simek (1): common: board: Simplify array with function pointers with CONFIG_IS_ENABLED
common/board_f.c | 210 ++++++++++++++----------------- common/board_r.c | 241 ++++++++++++++---------------------- include/initcall.h | 45 +++---- lib/Makefile | 1 - lib/initcall.c | 102 --------------- test/py/tests/test_trace.py | 8 +- 6 files changed, 210 insertions(+), 397 deletions(-) delete mode 100644 lib/initcall.c
From: Michal Simek michal.simek@amd.com
Convert all simple cases where current ifdef is used with using CONFIG_IS_ENABLED. The change doesn't have impact on code size and it is only cleaning up description.
Checkpatch is reporting issue: space required after that ',' (ctx:VxB)
When space is there another warning is coming up: space prohibited before that close parenthesis ')'
but there is no way how to fix it that's why leave it like it is.
Signed-off-by: Michal Simek michal.simek@amd.com Reviewed-by: Tom Rini trini@konsulko.com [jf: s/Simply/Simplify/ in subject] Signed-off-by: Jerome Forissier jerome.forissier@linaro.org Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- common/board_f.c | 49 +++++------------- common/board_r.c | 128 ++++++++++++----------------------------------- 2 files changed, 46 insertions(+), 131 deletions(-)
diff --git a/common/board_f.c b/common/board_f.c index 54c48d42ee9..a4d8850cb7d 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -872,12 +872,8 @@ static int initf_upl(void)
static const init_fnc_t init_sequence_f[] = { setup_mon_len, -#ifdef CONFIG_OF_CONTROL - fdtdec_setup, -#endif -#ifdef CONFIG_TRACE_EARLY - trace_early_init, -#endif + CONFIG_IS_ENABLED(OF_CONTROL, (fdtdec_setup,)) + CONFIG_IS_ENABLED(TRACE_EARLY, (trace_early_init,)) initf_malloc, initf_upl, log_init, @@ -885,16 +881,12 @@ static const init_fnc_t init_sequence_f[] = { event_init, bloblist_maybe_init, setup_spl_handoff, -#if defined(CONFIG_CONSOLE_RECORD_INIT_F) - console_record_init, -#endif + 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, -#if defined(CONFIG_BOARD_EARLY_INIT_F) - board_early_init_f, -#endif + CONFIG_IS_ENABLED(BOARD_EARLY_INIT_F, (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.) */ @@ -902,9 +894,7 @@ static const init_fnc_t init_sequence_f[] = { #if !defined(CONFIG_M68K) || (defined(CONFIG_M68K) && !defined(CONFIG_MCFTMR)) timer_init, /* initialize timer */ #endif -#if defined(CONFIG_BOARD_POSTCLK_INIT) - board_postclk_init, -#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 */ @@ -912,38 +902,25 @@ static const init_fnc_t init_sequence_f[] = { display_options, /* say that we are here */ display_text_info, /* show debugging info if required */ checkcpu, -#if defined(CONFIG_SYSRESET) - print_resetinfo, -#endif -#if defined(CONFIG_DISPLAY_CPUINFO) - print_cpuinfo, /* display cpu info (and speed) */ -#endif -#if defined(CONFIG_DTB_RESELECT) - embedded_dtb_select, -#endif -#if defined(CONFIG_DISPLAY_BOARDINFO) - show_board_info, -#endif + CONFIG_IS_ENABLED(SYSRESET, (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 -#if CONFIG_IS_ENABLED(SYS_I2C_LEGACY) - init_func_i2c, -#endif + CONFIG_IS_ENABLED(SYS_I2C_LEGACY, (init_func_i2c,)) announce_dram_init, dram_init, /* configure available RAM banks */ -#ifdef CONFIG_POST - post_init_f, -#endif + CONFIG_IS_ENABLED(POST, (post_init_f,)) INIT_FUNC_WATCHDOG_RESET #if defined(CFG_SYS_DRAM_TEST) testdram, #endif /* CFG_SYS_DRAM_TEST */ INIT_FUNC_WATCHDOG_RESET
-#ifdef CONFIG_POST - init_post, -#endif + CONFIG_IS_ENABLED(POST, (init_post,)) INIT_FUNC_WATCHDOG_RESET /* * Now that we have DRAM mapped and working, we can diff --git a/common/board_r.c b/common/board_r.c index 23ebc41868c..19bcf3950d5 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -612,19 +612,11 @@ static init_fnc_t init_sequence_r[] = { initr_malloc, log_init, initr_bootstage, /* Needs malloc() but has its own timer */ -#if defined(CONFIG_CONSOLE_RECORD) - console_record_init, -#endif -#ifdef CONFIG_SYS_NONCACHED_MEMORY - noncached_init, -#endif + CONFIG_IS_ENABLED(CONSOLE_RECORD, (console_record_init,)) + CONFIG_IS_ENABLED(SYS_NONCACHED_MEMORY, (noncached_init,)) initr_of_live, -#ifdef CONFIG_DM - initr_dm, -#endif -#ifdef CONFIG_ADDR_MAP - init_addr_map, -#endif + CONFIG_IS_ENABLED(DM, (initr_dm,)) + CONFIG_IS_ENABLED(ADDR_MAP, (init_addr_map,)) #if defined(CONFIG_ARM) || defined(CONFIG_RISCV) || defined(CONFIG_SANDBOX) board_init, /* Setup chipselects */ #endif @@ -634,36 +626,22 @@ static init_fnc_t init_sequence_r[] = { * davinci SOC's is added. Remove this check once all the board * implement this. */ -#ifdef CONFIG_CLOCKS - set_cpu_clk_info, /* Setup clock information */ -#endif + CONFIG_IS_ENABLED(CLOCKS, (set_cpu_clk_info,)) /* Setup clock information */ initr_lmb, -#ifdef CONFIG_EFI_LOADER - efi_memory_init, -#endif -#ifdef CONFIG_BINMAN_FDT - initr_binman, -#endif -#ifdef CONFIG_FSP_VERSION2 - arch_fsp_init_r, -#endif + CONFIG_IS_ENABLED(EFI_LOADER, (efi_memory_init,)) + CONFIG_IS_ENABLED(BINMAN_FDT, (initr_binman,)) + CONFIG_IS_ENABLED(FSP_VERSION2, (arch_fsp_init_r,)) initr_dm_devices, stdio_init_tables, serial_initialize, initr_announce, dm_announce, -#if CONFIG_IS_ENABLED(WDT) - initr_watchdog, -#endif + CONFIG_IS_ENABLED(WDT, (initr_watchdog,)) INIT_FUNC_WATCHDOG_RESET arch_initr_trap, -#if defined(CONFIG_BOARD_EARLY_INIT_R) - board_early_init_r, -#endif + CONFIG_IS_ENABLED(BOARD_EARLY_INIT_R, (board_early_init_r,)) INIT_FUNC_WATCHDOG_RESET -#ifdef CONFIG_POST - post_output_backlog, -#endif + CONFIG_IS_ENABLED(POST, (post_output_backlog,)) INIT_FUNC_WATCHDOG_RESET #if defined(CONFIG_PCI_INIT_R) && defined(CONFIG_SYS_EARLY_PCI_INIT) /* @@ -672,45 +650,25 @@ static init_fnc_t init_sequence_r[] = { */ pci_init, #endif -#ifdef CONFIG_ARCH_EARLY_INIT_R - arch_early_init_r, -#endif + CONFIG_IS_ENABLED(ARCH_EARLY_INIT_R, (arch_early_init_r,)) power_init_board, -#ifdef CONFIG_MTD_NOR_FLASH - initr_flash, -#endif + CONFIG_IS_ENABLED(MTD_NOR_FLASH, (initr_flash,)) INIT_FUNC_WATCHDOG_RESET #if defined(CONFIG_PPC) || defined(CONFIG_M68K) || defined(CONFIG_X86) /* initialize higher level parts of CPU like time base and timers */ cpu_init_r, #endif -#ifdef CONFIG_EFI_LOADER - efi_init_early, -#endif -#ifdef CONFIG_CMD_NAND - initr_nand, -#endif -#ifdef CONFIG_CMD_ONENAND - initr_onenand, -#endif -#ifdef CONFIG_MMC - initr_mmc, -#endif -#ifdef CONFIG_XEN - xen_init, -#endif -#ifdef CONFIG_PVBLOCK - initr_pvblock, -#endif + CONFIG_IS_ENABLED(EFI_LOADER, (efi_init_early,)) + CONFIG_IS_ENABLED(CMD_NAND, (initr_nand,)) + CONFIG_IS_ENABLED(CMD_ONENAND, (initr_onenand,)) + CONFIG_IS_ENABLED(MMC, (initr_mmc,)) + CONFIG_IS_ENABLED(XEN, (xen_init,)) + CONFIG_IS_ENABLED(PVBLOCK, (initr_pvblock,)) initr_env, -#ifdef CONFIG_SYS_MALLOC_BOOTPARAMS - initr_malloc_bootparams, -#endif + CONFIG_IS_ENABLED(SYS_MALLOC_BOOTPARAMS, (initr_malloc_bootparams,)) INIT_FUNC_WATCHDOG_RESET cpu_secondary_init_r, -#if defined(CONFIG_ID_EEPROM) - mac_read_from_eeprom, -#endif + CONFIG_IS_ENABLED(ID_EEPROM, (mac_read_from_eeprom,)) INITCALL_EVENT(EVT_SETTINGS_R), INIT_FUNC_WATCHDOG_RESET #if defined(CONFIG_PCI_INIT_R) && !defined(CONFIG_SYS_EARLY_PCI_INIT) @@ -721,24 +679,15 @@ static init_fnc_t init_sequence_r[] = { #endif stdio_add_devices, jumptable_init, -#ifdef CONFIG_API - api_init, -#endif + CONFIG_IS_ENABLED(API, (api_init,)) console_init_r, /* fully init console as a device */ -#ifdef CONFIG_DISPLAY_BOARDINFO_LATE - console_announce_r, - show_board_info, -#endif -#ifdef CONFIG_ARCH_MISC_INIT - arch_misc_init, /* miscellaneous arch-dependent init */ -#endif -#ifdef CONFIG_MISC_INIT_R - misc_init_r, /* miscellaneous platform-dependent init */ -#endif + CONFIG_IS_ENABLED(DISPLAY_BOARDINFO_LATE, (console_announce_r, show_board_info,)) + /* miscellaneous arch-dependent init */ + CONFIG_IS_ENABLED(ARCH_MISC_INIT, (arch_misc_init,)) + /* miscellaneous platform-dependent init */ + CONFIG_IS_ENABLED(MISC_INIT_R, (misc_init_r,)) INIT_FUNC_WATCHDOG_RESET -#ifdef CONFIG_CMD_KGDB - kgdb_init, -#endif + CONFIG_IS_ENABLED(CMD_KGDB, (kgdb_init,)) interrupt_init, #if defined(CONFIG_MICROBLAZE) || defined(CONFIG_M68K) timer_init, /* initialize timer */ @@ -746,22 +695,11 @@ static init_fnc_t init_sequence_r[] = { initr_status_led, initr_boot_led_blink, /* PPC has a udelay(20) here dating from 2002. Why? */ -#ifdef CONFIG_BOARD_LATE_INIT - board_late_init, -#endif -#ifdef CONFIG_BITBANGMII - bb_miiphy_init, -#endif -#ifdef CONFIG_PCI_ENDPOINT - pci_ep_init, -#endif -#if defined(CONFIG_CMD_NET) - INIT_FUNC_WATCHDOG_RESET - initr_net, -#endif -#ifdef CONFIG_POST - initr_post, -#endif + CONFIG_IS_ENABLED(BOARD_LATE_INIT, (board_late_init,)) + CONFIG_IS_ENABLED(BITBANGMII, (bb_miiphy_init,)) + CONFIG_IS_ENABLED(PCI_ENDPOINT, (pci_ep_init,)) + CONFIG_IS_ENABLED(CMD_NET, (INIT_FUNC_WATCHDOG_RESET initr_net,)) + CONFIG_IS_ENABLED(POST, (initr_post,)) INIT_FUNC_WATCHDOG_RESET INITCALL_EVENT(EVT_LAST_STAGE_INIT), #if defined(CFG_PRAM)
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@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) + +#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
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@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
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@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() ;-)
Cheers,
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@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?
Cheers, Quentin
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@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,
Hi Jerome
[...]
(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_INITINITCALL_EVENT(EVT_MISC_INIT_F),INIT_FUNC_WATCHDOG_RESETCONFIG_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();
The previous function was calling init, reset, reset for the watchdog. The new code does init, reset, init, reset etc. Is this intentional? Was that a bug of the existing code?
Thanks /Ilias
#if defined(CFG_SYS_DRAM_TEST)
testdram,
INITCALL(testdram);#endif /* CFG_SYS_DRAM_TEST */
INIT_FUNC_WATCHDOG_RESETCONFIG_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_RESETsetup_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)+#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
Hi Ilias,
On 12/21/24 09:10, Ilias Apalodimas wrote:
Hi Jerome
[...]
(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_INITINITCALL_EVENT(EVT_MISC_INIT_F),INIT_FUNC_WATCHDOG_RESETCONFIG_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();The previous function was calling init, reset, reset for the watchdog. The new code does init, reset, init, reset etc. Is this intentional? Was that a bug of the existing code?
Good catch, it's a mistake. I'll fix in v3.
Thanks,
Replace the init_sequence_f_r function array by direct calls.
Signed-off-by: Jerome Forissier jerome.forissier@linaro.org --- common/board_f.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/common/board_f.c b/common/board_f.c index cebed85ed4d..f47275a2230 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -1016,8 +1016,8 @@ void board_init_f(ulong boot_flags) /* * For now this code is only used on x86. * - * init_sequence_f_r is the list of init functions which are run when - * U-Boot is executing from Flash with a semi-limited 'C' environment. + * Run init functions which are run when U-Boot is executing from Flash with a + * semi-limited 'C' environment. * The following limitations must be considered when implementing an * '_f_r' function: * - 'static' variables are read-only @@ -1030,18 +1030,14 @@ void board_init_f(ulong boot_flags) * NOTE: At present only x86 uses this route, but it is intended that * all archs will move to this when generic relocation is implemented. */ -static const init_fnc_t init_sequence_f_r[] = { -#if !CONFIG_IS_ENABLED(X86_64) - init_cache_f_r, -#endif - - NULL, -}; +static void initcall_run_f_r(void) +{ + CONFIG_IS_ENABLED(X86_64, (INITCALL(init_cache_f_r);)) +}
void board_init_f_r(void) { - if (initcall_run_list(init_sequence_f_r)) - hang(); + initcall_run_f_r();
/* * The pre-relocation drivers may be using memory that has now gone
On Wed, 18 Dec 2024 at 17:54, Jerome Forissier jerome.forissier@linaro.org wrote:
Replace the init_sequence_f_r function array by direct calls.
Signed-off-by: Jerome Forissier jerome.forissier@linaro.org
common/board_f.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/common/board_f.c b/common/board_f.c index cebed85ed4d..f47275a2230 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -1016,8 +1016,8 @@ void board_init_f(ulong boot_flags) /*
- For now this code is only used on x86.
- init_sequence_f_r is the list of init functions which are run when
- U-Boot is executing from Flash with a semi-limited 'C' environment.
- Run init functions which are run when U-Boot is executing from Flash with a
- semi-limited 'C' environment.
- The following limitations must be considered when implementing an
- '_f_r' function:
- 'static' variables are read-only
@@ -1030,18 +1030,14 @@ void board_init_f(ulong boot_flags)
- NOTE: At present only x86 uses this route, but it is intended that
- all archs will move to this when generic relocation is implemented.
*/ -static const init_fnc_t init_sequence_f_r[] = { -#if !CONFIG_IS_ENABLED(X86_64)
init_cache_f_r,-#endif
NULL,-}; +static void initcall_run_f_r(void) +{
CONFIG_IS_ENABLED(X86_64, (INITCALL(init_cache_f_r);))+}
void board_init_f_r(void) {
if (initcall_run_list(init_sequence_f_r))hang();
initcall_run_f_r(); /* * The pre-relocation drivers may be using memory that has now gone-- 2.43.0
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Replace the init_sequence_r function array by direct calls. bloat-o-meter stats before/after on u-boot.bin for configuration xilinx_zynqmp_kria_defconfig are the following:
- With LTO: add/remove: 87/128 grow/shrink: 9/22 up/down: 19608/-20915 (-1307) Total: Before=1067719, After=1066412, chg -0.12%
- Without LTO: add/remove: 0/24 grow/shrink: 1/0 up/down: 488/-1124 (-636) Total: Before=1119091, After=1118455, chg -0.06%
Signed-off-by: Jerome Forissier jerome.forissier@linaro.org --- common/board_r.c | 171 ++++++++++++++++++------------------ test/py/tests/test_trace.py | 8 +- 2 files changed, 90 insertions(+), 89 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c index 19bcf3950d5..ee54654ac71 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -582,21 +582,20 @@ static int run_main_loop(void) }
/* - * Over time we hope to remove these functions with code fragments and - * stub functions, and instead call the relevant function directly. - * - * We also hope to remove most of the driver-related init and do it if/when - * the driver is later used. + * Over time we hope to remove most of the driver-related init and do it + * if/when the driver is later used. * * TODO: perhaps reset the watchdog in the initcall function after each call? */ -static init_fnc_t init_sequence_r[] = { - initr_trace, - initr_reloc, - event_init, + +static void initcall_run_r(void) +{ + INITCALL(initr_trace); + INITCALL(initr_reloc); + INITCALL(event_init); /* TODO: could x86/PPC have this also perhaps? */ #if defined(CONFIG_ARM) || defined(CONFIG_RISCV) - initr_caches, + INITCALL(initr_caches); /* Note: For Freescale LS2 SoCs, new MMU table is created in DDR. * A temporary mapping of IFC high region is since removed, * so environmental variables in NOR flash is not available @@ -604,21 +603,21 @@ static init_fnc_t init_sequence_r[] = { * region. */ #endif - initr_reloc_global_data, + INITCALL(initr_reloc_global_data); #if defined(CONFIG_SYS_INIT_RAM_LOCK) && defined(CONFIG_E500) - initr_unlock_ram_in_cache, + INITCALL(initr_unlock_ram_in_cache); #endif - initr_barrier, - initr_malloc, - log_init, - initr_bootstage, /* Needs malloc() but has its own timer */ - CONFIG_IS_ENABLED(CONSOLE_RECORD, (console_record_init,)) - CONFIG_IS_ENABLED(SYS_NONCACHED_MEMORY, (noncached_init,)) - initr_of_live, - CONFIG_IS_ENABLED(DM, (initr_dm,)) - CONFIG_IS_ENABLED(ADDR_MAP, (init_addr_map,)) + INITCALL(initr_barrier); + INITCALL(initr_malloc); + INITCALL(log_init); + INITCALL(initr_bootstage); /* Needs malloc() but has its own timer */ + CONFIG_IS_ENABLED(CONSOLE_RECORD, (INITCALL(console_record_init);)) + CONFIG_IS_ENABLED(SYS_NONCACHED_MEMORY, (INITCALL(noncached_init);)) + INITCALL(initr_of_live); + CONFIG_IS_ENABLED(DM, (INITCALL(initr_dm);)) + CONFIG_IS_ENABLED(ADDR_MAP, (INITCALL(init_addr_map);)) #if defined(CONFIG_ARM) || defined(CONFIG_RISCV) || defined(CONFIG_SANDBOX) - board_init, /* Setup chipselects */ + INITCALL(board_init); /* Setup chipselects */ #endif /* * TODO: printing of the clock inforamtion of the board is now @@ -626,88 +625,91 @@ static init_fnc_t init_sequence_r[] = { * davinci SOC's is added. Remove this check once all the board * implement this. */ - CONFIG_IS_ENABLED(CLOCKS, (set_cpu_clk_info,)) /* Setup clock information */ - initr_lmb, - CONFIG_IS_ENABLED(EFI_LOADER, (efi_memory_init,)) - CONFIG_IS_ENABLED(BINMAN_FDT, (initr_binman,)) - CONFIG_IS_ENABLED(FSP_VERSION2, (arch_fsp_init_r,)) - initr_dm_devices, - stdio_init_tables, - serial_initialize, - initr_announce, - dm_announce, - CONFIG_IS_ENABLED(WDT, (initr_watchdog,)) - INIT_FUNC_WATCHDOG_RESET - arch_initr_trap, - CONFIG_IS_ENABLED(BOARD_EARLY_INIT_R, (board_early_init_r,)) - INIT_FUNC_WATCHDOG_RESET - CONFIG_IS_ENABLED(POST, (post_output_backlog,)) - INIT_FUNC_WATCHDOG_RESET + CONFIG_IS_ENABLED(CLOCKS, (INITCALL(set_cpu_clk_info);)) + INITCALL(initr_lmb); + CONFIG_IS_ENABLED(EFI_LOADER, (INITCALL(efi_memory_init);)) + CONFIG_IS_ENABLED(BINMAN_FDT, (INITCALL(initr_binman);)) + CONFIG_IS_ENABLED(FSP_VERSION2, (INITCALL(arch_fsp_init_r);)) + INITCALL(initr_dm_devices); + INITCALL(stdio_init_tables); + INITCALL(serial_initialize); + INITCALL(initr_announce); + INITCALL(dm_announce); + CONFIG_IS_ENABLED(WDT, (INITCALL(initr_watchdog);)) + WATCHDOG_RESET(); + INITCALL(arch_initr_trap); + CONFIG_IS_ENABLED(BOARD_EARLY_INIT_R, (INITCALL(board_early_init_r);)) + WATCHDOG_RESET(); + CONFIG_IS_ENABLED(POST, (INITCALL(post_output_backlog);)) + WATCHDOG_RESET(); #if defined(CONFIG_PCI_INIT_R) && defined(CONFIG_SYS_EARLY_PCI_INIT) /* * Do early PCI configuration _before_ the flash gets initialised, * because PCU resources are crucial for flash access on some boards. */ - pci_init, + INITCALL(pci_init); #endif - CONFIG_IS_ENABLED(ARCH_EARLY_INIT_R, (arch_early_init_r,)) - power_init_board, - CONFIG_IS_ENABLED(MTD_NOR_FLASH, (initr_flash,)) - INIT_FUNC_WATCHDOG_RESET + CONFIG_IS_ENABLED(ARCH_EARLY_INIT_R, (INITCALL(arch_early_init_r);)) + INITCALL(power_init_board); + CONFIG_IS_ENABLED(MTD_NOR_FLASH, (INITCALL(initr_flash);)) + WATCHDOG_RESET(); #if defined(CONFIG_PPC) || defined(CONFIG_M68K) || defined(CONFIG_X86) /* initialize higher level parts of CPU like time base and timers */ - cpu_init_r, + INITCALL(cpu_init_r); #endif - CONFIG_IS_ENABLED(EFI_LOADER, (efi_init_early,)) - CONFIG_IS_ENABLED(CMD_NAND, (initr_nand,)) - CONFIG_IS_ENABLED(CMD_ONENAND, (initr_onenand,)) - CONFIG_IS_ENABLED(MMC, (initr_mmc,)) - CONFIG_IS_ENABLED(XEN, (xen_init,)) - CONFIG_IS_ENABLED(PVBLOCK, (initr_pvblock,)) - initr_env, - CONFIG_IS_ENABLED(SYS_MALLOC_BOOTPARAMS, (initr_malloc_bootparams,)) - INIT_FUNC_WATCHDOG_RESET - cpu_secondary_init_r, - CONFIG_IS_ENABLED(ID_EEPROM, (mac_read_from_eeprom,)) - INITCALL_EVENT(EVT_SETTINGS_R), - INIT_FUNC_WATCHDOG_RESET + CONFIG_IS_ENABLED(EFI_LOADER, (INITCALL(efi_init_early);)) + CONFIG_IS_ENABLED(CMD_NAND, (INITCALL(initr_nand);)) + CONFIG_IS_ENABLED(CMD_ONENAND, (INITCALL(initr_onenand);)) + CONFIG_IS_ENABLED(MMC, (INITCALL(initr_mmc);)) + CONFIG_IS_ENABLED(XEN, (INITCALL(xen_init);)) + CONFIG_IS_ENABLED(PVBLOCK, (INITCALL(initr_pvblock);)) + INITCALL(initr_env); + CONFIG_IS_ENABLED(SYS_MALLOC_BOOTPARAMS, + (INITCALL(initr_malloc_bootparams);)) + WATCHDOG_RESET(); + INITCALL(cpu_secondary_init_r); + CONFIG_IS_ENABLED(ID_EEPROM, (INITCALL(mac_read_from_eeprom);)) + INITCALL_EVT(EVT_SETTINGS_R); + WATCHDOG_RESET(); #if defined(CONFIG_PCI_INIT_R) && !defined(CONFIG_SYS_EARLY_PCI_INIT) /* * Do pci configuration */ - pci_init, + INITCALL(pci_init); #endif - stdio_add_devices, - jumptable_init, - CONFIG_IS_ENABLED(API, (api_init,)) - console_init_r, /* fully init console as a device */ - CONFIG_IS_ENABLED(DISPLAY_BOARDINFO_LATE, (console_announce_r, show_board_info,)) + INITCALL(stdio_add_devices); + INITCALL(jumptable_init); + CONFIG_IS_ENABLED(API, (INITCALL(api_init);)) + INITCALL(console_init_r); /* fully init console as a device */ + CONFIG_IS_ENABLED(DISPLAY_BOARDINFO_LATE, + (INITCALL(console_announce_r); + INITCALL(show_board_info);)) /* miscellaneous arch-dependent init */ - CONFIG_IS_ENABLED(ARCH_MISC_INIT, (arch_misc_init,)) + CONFIG_IS_ENABLED(ARCH_MISC_INIT, (INITCALL(arch_misc_init);)) /* miscellaneous platform-dependent init */ - CONFIG_IS_ENABLED(MISC_INIT_R, (misc_init_r,)) - INIT_FUNC_WATCHDOG_RESET - CONFIG_IS_ENABLED(CMD_KGDB, (kgdb_init,)) - interrupt_init, + CONFIG_IS_ENABLED(MISC_INIT_R, (INITCALL(misc_init_r);)) + WATCHDOG_RESET(); + CONFIG_IS_ENABLED(CMD_KGDB, (INITCALL(kgdb_init);)) + INITCALL(interrupt_init); #if defined(CONFIG_MICROBLAZE) || defined(CONFIG_M68K) - timer_init, /* initialize timer */ + INITCALL(timer_init); /* initialize timer */ #endif - initr_status_led, - initr_boot_led_blink, + INITCALL(initr_status_led); + INITCALL(initr_boot_led_blink); /* PPC has a udelay(20) here dating from 2002. Why? */ - CONFIG_IS_ENABLED(BOARD_LATE_INIT, (board_late_init,)) - CONFIG_IS_ENABLED(BITBANGMII, (bb_miiphy_init,)) - CONFIG_IS_ENABLED(PCI_ENDPOINT, (pci_ep_init,)) - CONFIG_IS_ENABLED(CMD_NET, (INIT_FUNC_WATCHDOG_RESET initr_net,)) - CONFIG_IS_ENABLED(POST, (initr_post,)) - INIT_FUNC_WATCHDOG_RESET - INITCALL_EVENT(EVT_LAST_STAGE_INIT), + CONFIG_IS_ENABLED(BOARD_LATE_INIT, (INITCALL(board_late_init);)) + CONFIG_IS_ENABLED(BITBANGMII, (INITCALL(bb_miiphy_init);)) + CONFIG_IS_ENABLED(PCI_ENDPOINT, (INITCALL(pci_ep_init);)) + CONFIG_IS_ENABLED(CMD_NET, (WATCHDOG_RESET(); INITCALL(initr_net);)) + CONFIG_IS_ENABLED(POST, (INITCALL(initr_post);)) + WATCHDOG_RESET(); + INITCALL_EVT(EVT_LAST_STAGE_INIT); #if defined(CFG_PRAM) - initr_mem, + INITCALL(initr_mem); #endif - initr_boot_led_on, - run_main_loop, -}; + INITCALL(initr_boot_led_on); + INITCALL(run_main_loop); +}
void board_init_r(gd_t *new_gd, ulong dest_addr) { @@ -734,8 +736,7 @@ void board_init_r(gd_t *new_gd, ulong dest_addr) #endif gd->flags &= ~GD_FLG_LOG_READY;
- if (initcall_run_list(init_sequence_r)) - hang(); + initcall_run_r();
/* NOTREACHED - run_main_loop() does not return */ hang(); diff --git a/test/py/tests/test_trace.py b/test/py/tests/test_trace.py index ec1e624722c..4c159ad9b16 100644 --- a/test/py/tests/test_trace.py +++ b/test/py/tests/test_trace.py @@ -175,7 +175,7 @@ def check_funcgraph(cons, fname, proftool, map_fname, trace_dat): # Then look for this: # u-boot-1 0..... 282.101375: funcgraph_exit: 0.006 us | } # Then check for this: - # u-boot-1 0..... 282.101375: funcgraph_entry: 0.000 us | calc_reloc_ofs(); + # u-boot-1 0..... 282.101375: funcgraph_entry: 0.000 us | event_init();
expected_indent = None found_start = False @@ -198,8 +198,8 @@ def check_funcgraph(cons, fname, proftool, map_fname, trace_dat): found_end = True
# The next function after initf_bootstage() exits should be - # initcall_is_event() - assert upto == 'calc_reloc_ofs()' + # event_init() + assert upto == 'event_init()'
# Now look for initf_dm() and dm_timer_init() so we can check the bootstage # time @@ -248,7 +248,7 @@ def check_flamegraph(cons, fname, proftool, map_fname, trace_fg): # We expect dm_timer_init() to be called twice: once before relocation and # once after look1 = 'initf_dm;dm_timer_init 1' - look2 = 'board_init_r;initcall_run_list;initr_dm_devices;dm_timer_init 1' + look2 = 'board_init_r;initcall_run_r;initr_dm_devices;dm_timer_init 1' found = 0 with open(trace_fg, 'r') as fd: for line in fd:
Now that all initcalls have been converted to static calls, remove initcall_run_list().
Signed-off-by: Jerome Forissier jerome.forissier@linaro.org --- include/initcall.h | 24 ----------- lib/Makefile | 1 - lib/initcall.c | 102 --------------------------------------------- 3 files changed, 127 deletions(-) delete mode 100644 lib/initcall.c
diff --git a/include/initcall.h b/include/initcall.h index 9398a3ec7d5..9d1af44778b 100644 --- a/include/initcall.h +++ b/include/initcall.h @@ -12,30 +12,6 @@
_Static_assert(EVT_COUNT < 256, "Can only support 256 event types with 8 bits");
-/** - * init_fnc_t - Init function - * - * Return: 0 if OK -ve on error - */ -typedef int (*init_fnc_t)(void); - -/* Top bit indicates that the initcall is an event */ -#define INITCALL_IS_EVENT GENMASK(BITS_PER_LONG - 1, 8) -#define INITCALL_EVENT_TYPE GENMASK(7, 0) - -#define INITCALL_EVENT(_type) (void *)((_type) | INITCALL_IS_EVENT) - -/** - * initcall_run_list() - Run through a list of function calls - * - * This calls functions one after the other, stopping at the first error, or - * when NULL is obtained. - * - * @init_sequence: NULL-terminated init sequence to run - * Return: 0 if OK, or -ve error code from the first failure - */ -int initcall_run_list(const init_fnc_t init_sequence[]); - #define INITCALL(_call) \ do { \ if (_call()) { \ diff --git a/lib/Makefile b/lib/Makefile index d24ed629732..78d9d02369b 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -44,7 +44,6 @@ obj-$(CONFIG_GZIP_COMPRESSED) += gzip.o obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += smbios.o obj-$(CONFIG_SMBIOS_PARSER) += smbios-parser.o obj-$(CONFIG_IMAGE_SPARSE) += image-sparse.o -obj-y += initcall.o obj-y += ldiv.o obj-$(CONFIG_XXHASH) += xxhash.o obj-y += net_utils.o diff --git a/lib/initcall.c b/lib/initcall.c deleted file mode 100644 index 2686b9aed5c..00000000000 --- a/lib/initcall.c +++ /dev/null @@ -1,102 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ -/* - * Copyright (c) 2013 The Chromium OS Authors. - */ - -#include <efi.h> -#include <initcall.h> -#include <log.h> -#include <relocate.h> -#include <asm/global_data.h> - -DECLARE_GLOBAL_DATA_PTR; - -static ulong calc_reloc_ofs(void) -{ -#ifdef CONFIG_EFI_APP - return (ulong)image_base; -#endif - /* - * Sandbox is relocated by the OS, so symbols always appear at - * the relocated address. - */ - if (IS_ENABLED(CONFIG_SANDBOX) || (gd->flags & GD_FLG_RELOC)) - return gd->reloc_off; - - return 0; -} - -/** - * initcall_is_event() - Get the event number for an initcall - * - * func: Function pointer to check - * Return: Event number, if this is an event, else 0 - */ -static int initcall_is_event(init_fnc_t func) -{ - ulong val = (ulong)func; - - if ((val & INITCALL_IS_EVENT) == INITCALL_IS_EVENT) - return val & INITCALL_EVENT_TYPE; - - return 0; -} - -/* - * To enable debugging. add #define DEBUG at the top of the including file. - * - * To find a symbol, use grep on u-boot.map - */ -int initcall_run_list(const init_fnc_t init_sequence[]) -{ - ulong reloc_ofs; - const init_fnc_t *ptr; - enum event_t type; - init_fnc_t func; - int ret = 0; - - for (ptr = init_sequence; func = *ptr, func; ptr++) { - reloc_ofs = calc_reloc_ofs(); - type = initcall_is_event(func); - - if (type) { - if (!CONFIG_IS_ENABLED(EVENT)) - continue; - debug("initcall: event %d/%s\n", type, - event_type_name(type)); - } else if (reloc_ofs) { - debug("initcall: %p (relocated to %p)\n", - (char *)func - reloc_ofs, (char *)func); - } else { - debug("initcall: %p\n", (char *)func - reloc_ofs); - } - - ret = type ? event_notify_null(type) : func(); - if (ret) - break; - } - - if (ret) { - if (CONFIG_IS_ENABLED(EVENT)) { - char buf[60]; - - /* don't worry about buf size as we are dying here */ - if (type) { - sprintf(buf, "event %d/%s", type, - event_type_name(type)); - } else { - sprintf(buf, "call %p", - (char *)func - reloc_ofs); - } - - printf("initcall failed at %s (err=%dE)\n", buf, ret); - } else { - printf("initcall failed at call %p (err=%d)\n", - (char *)func - reloc_ofs, ret); - } - - return ret; - } - - return 0; -}
On Wed, Dec 18, 2024 at 04:53:53PM +0100, Jerome Forissier wrote:
This series replaces the dynamic initcalls (with function pointers) with static calls, and gets rid of initcall_run_list(), init_sequence_f, init_sequence_f_r and init_sequence_r. This makes the code simpler and the binary slighlty smaller: -2507 bytes/-0.23 % with LTO enabled and -1232 bytes/-0.11 % with LTO disabled (xilinx_zynqmp_kria_defconfig).
Execution time doesn't seem to change noticeably. There is no impact on the SPL.
This leads to run-time failures on SH: https://source.denx.de/u-boot/u-boot/-/jobs/986701
Hi Jerome,
On Thu, 2 Jan 2025 at 06:55, Tom Rini trini@konsulko.com wrote:
On Wed, Dec 18, 2024 at 04:53:53PM +0100, Jerome Forissier wrote:
This series replaces the dynamic initcalls (with function pointers) with static calls, and gets rid of initcall_run_list(), init_sequence_f, init_sequence_f_r and init_sequence_r. This makes the code simpler and the binary slighlty smaller: -2507 bytes/-0.23 % with LTO enabled and -1232 bytes/-0.11 % with LTO disabled (xilinx_zynqmp_kria_defconfig).
Execution time doesn't seem to change noticeably. There is no impact on the SPL.
This leads to run-time failures on SH: https://source.denx.de/u-boot/u-boot/-/jobs/986701
I'm not a huge fan of this series in terms of the style, but this is a significant code-size reduction!
For the board_init_f() etc. functions, can you do a follow-up with a comment indicating that logic must not be added?...i.e. that we don't end up with variables, if(), etc. in these functions. I think that would be a good rule to have.
Regards, Simon
Hi Simon,
On 1/3/25 02:41, Simon Glass wrote:
Hi Jerome,
On Thu, 2 Jan 2025 at 06:55, Tom Rini trini@konsulko.com wrote:
On Wed, Dec 18, 2024 at 04:53:53PM +0100, Jerome Forissier wrote:
This series replaces the dynamic initcalls (with function pointers) with static calls, and gets rid of initcall_run_list(), init_sequence_f, init_sequence_f_r and init_sequence_r. This makes the code simpler and the binary slighlty smaller: -2507 bytes/-0.23 % with LTO enabled and -1232 bytes/-0.11 % with LTO disabled (xilinx_zynqmp_kria_defconfig).
Execution time doesn't seem to change noticeably. There is no impact on the SPL.
This leads to run-time failures on SH: https://source.denx.de/u-boot/u-boot/-/jobs/986701
I'm not a huge fan of this series in terms of the style, but this is a significant code-size reduction!
For the board_init_f() etc. functions, can you do a follow-up with a comment indicating that logic must not be added?...i.e. that we don't end up with variables, if(), etc. in these functions. I think that would be a good rule to have.
I agree. I will add a comment in v3.
Thanks,
Hi Tom,
On 1/1/25 18:55, Tom Rini wrote:
On Wed, Dec 18, 2024 at 04:53:53PM +0100, Jerome Forissier wrote:
This series replaces the dynamic initcalls (with function pointers) with static calls, and gets rid of initcall_run_list(), init_sequence_f, init_sequence_f_r and init_sequence_r. This makes the code simpler and the binary slighlty smaller: -2507 bytes/-0.23 % with LTO enabled and -1232 bytes/-0.11 % with LTO disabled (xilinx_zynqmp_kria_defconfig).
Execution time doesn't seem to change noticeably. There is no impact on the SPL.
This leads to run-time failures on SH: https://source.denx.de/u-boot/u-boot/-/jobs/986701
Fixed as follows:
diff --git a/arch/sh/lib/board.c b/arch/sh/lib/board.c index 53b1c147c2e..2daf54e7c33 100644 --- a/arch/sh/lib/board.c +++ b/arch/sh/lib/board.c @@ -19,18 +19,13 @@ int dram_init(void)
void relocate_code(ulong start_addr_sp, gd_t *new_gd, ulong relocaddr) { - void (*reloc_board_init_r)(gd_t *gd, ulong dest) = board_init_r; - - if (new_gd->reloc_off) { + if (new_gd->reloc_off) memcpy((void *)new_gd->relocaddr, (void *)(new_gd->relocaddr - new_gd->reloc_off), new_gd->mon_len);
- reloc_board_init_r += new_gd->reloc_off; - } - __asm__ __volatile__("mov.l %0, r15\n" : : "m" (new_gd->start_addr_sp));
while (1) - reloc_board_init_r(new_gd, 0x0); + board_init_r(new_gd, 0x0); }
...which kind of makes sense to me but I must say I don't fully grasp what is happening with this relocation step.
If this is indeed the right fix I will fold it into "board_init_r(): use static calls" which is the commit that breaks the test.
Thanks,
participants (5)
-
Ilias Apalodimas -
Jerome Forissier -
Quentin Schulz -
Simon Glass -
Tom Rini