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 <[email protected]>
---
   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

Reply via email to