[PATCH v2 2/5] board_init_f(): use static calls

Jerome Forissier jerome.forissier at linaro.org
Sat Dec 21 13:08:47 CET 2024


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_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();
> 
> 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,
-- 
 Jerome

> 
> Thanks
> /Ilias
>>  #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
>> --
>> 2.43.0
>>


More information about the U-Boot mailing list