* [PATCH v2 1/5] common: board: Simplify array with function pointers with CONFIG_IS_ENABLED
2024-12-18 15:53 [PATCH v2 0/5] Static initcalls Jerome Forissier
@ 2024-12-18 15:53 ` Jerome Forissier
2024-12-18 15:53 ` [PATCH v2 2/5] board_init_f(): use static calls Jerome Forissier
` (4 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Jerome Forissier @ 2024-12-18 15:53 UTC (permalink / raw)
To: u-boot
Cc: Ilias Apalodimas, Tom Rini, Simon Glass, Michal Simek,
Jerome Forissier, Jonas Karlman, Devarsh Thakkar,
Christian Marangi, Rasmus Villemoes, Sughosh Ganu,
Evgeny Bachinin
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)
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 2/5] board_init_f(): use static calls
2024-12-18 15:53 [PATCH v2 0/5] Static initcalls Jerome Forissier
2024-12-18 15:53 ` [PATCH v2 1/5] common: board: Simplify array with function pointers with CONFIG_IS_ENABLED Jerome Forissier
@ 2024-12-18 15:53 ` Jerome Forissier
2024-12-18 17:27 ` Quentin Schulz
2024-12-21 8:10 ` Ilias Apalodimas
2024-12-18 15:53 ` [PATCH v2 3/5] board_init_f_r(): " Jerome Forissier
` (3 subsequent siblings)
5 siblings, 2 replies; 17+ messages in thread
From: Jerome Forissier @ 2024-12-18 15:53 UTC (permalink / raw)
To: u-boot
Cc: Ilias Apalodimas, Tom Rini, Simon Glass, Michal Simek,
Jerome Forissier, Jonas Karlman, Devarsh Thakkar
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
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 2/5] board_init_f(): use static calls
2024-12-18 15:53 ` [PATCH v2 2/5] board_init_f(): use static calls Jerome Forissier
@ 2024-12-18 17:27 ` Quentin Schulz
2024-12-18 17:38 ` Jerome Forissier
2024-12-21 8:10 ` Ilias Apalodimas
1 sibling, 1 reply; 17+ messages in thread
From: Quentin Schulz @ 2024-12-18 17:27 UTC (permalink / raw)
To: Jerome Forissier, u-boot
Cc: Ilias Apalodimas, Tom Rini, Simon Glass, Michal Simek,
Jonas Karlman, Devarsh Thakkar
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2 2/5] board_init_f(): use static calls
2024-12-18 17:27 ` Quentin Schulz
@ 2024-12-18 17:38 ` Jerome Forissier
2024-12-18 17:47 ` Quentin Schulz
0 siblings, 1 reply; 17+ messages in thread
From: Jerome Forissier @ 2024-12-18 17:38 UTC (permalink / raw)
To: Quentin Schulz, u-boot
Cc: Ilias Apalodimas, Tom Rini, Simon Glass, Michal Simek,
Jonas Karlman, Devarsh Thakkar
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,
--
Jerome
>
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2 2/5] board_init_f(): use static calls
2024-12-18 17:38 ` Jerome Forissier
@ 2024-12-18 17:47 ` Quentin Schulz
2024-12-18 18:21 ` Jerome Forissier
0 siblings, 1 reply; 17+ messages in thread
From: Quentin Schulz @ 2024-12-18 17:47 UTC (permalink / raw)
To: Jerome Forissier, u-boot
Cc: Ilias Apalodimas, Tom Rini, Simon Glass, Michal Simek,
Jonas Karlman, Devarsh Thakkar
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2 2/5] board_init_f(): use static calls
2024-12-18 17:47 ` Quentin Schulz
@ 2024-12-18 18:21 ` Jerome Forissier
0 siblings, 0 replies; 17+ messages in thread
From: Jerome Forissier @ 2024-12-18 18:21 UTC (permalink / raw)
To: Quentin Schulz, u-boot
Cc: Ilias Apalodimas, Tom Rini, Simon Glass, Michal Simek,
Jonas Karlman, Devarsh Thakkar
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,
--
Jerome
>
> Cheers,
> Quentin
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/5] board_init_f(): use static calls
2024-12-18 15:53 ` [PATCH v2 2/5] board_init_f(): use static calls Jerome Forissier
2024-12-18 17:27 ` Quentin Schulz
@ 2024-12-21 8:10 ` Ilias Apalodimas
2024-12-21 12:08 ` Jerome Forissier
1 sibling, 1 reply; 17+ messages in thread
From: Ilias Apalodimas @ 2024-12-21 8:10 UTC (permalink / raw)
To: Jerome Forissier
Cc: u-boot, Tom Rini, Simon Glass, Michal Simek, Jonas Karlman,
Devarsh Thakkar
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?
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
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2 2/5] board_init_f(): use static calls
2024-12-21 8:10 ` Ilias Apalodimas
@ 2024-12-21 12:08 ` Jerome Forissier
0 siblings, 0 replies; 17+ messages in thread
From: Jerome Forissier @ 2024-12-21 12:08 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: u-boot, Tom Rini, Simon Glass, Michal Simek, Jonas Karlman,
Devarsh Thakkar
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
>>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 3/5] board_init_f_r(): use static calls
2024-12-18 15:53 [PATCH v2 0/5] Static initcalls Jerome Forissier
2024-12-18 15:53 ` [PATCH v2 1/5] common: board: Simplify array with function pointers with CONFIG_IS_ENABLED Jerome Forissier
2024-12-18 15:53 ` [PATCH v2 2/5] board_init_f(): use static calls Jerome Forissier
@ 2024-12-18 15:53 ` Jerome Forissier
2024-12-21 8:14 ` Ilias Apalodimas
2024-12-18 15:53 ` [PATCH v2 4/5] board_init_r(): " Jerome Forissier
` (2 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Jerome Forissier @ 2024-12-18 15:53 UTC (permalink / raw)
To: u-boot
Cc: Ilias Apalodimas, Tom Rini, Simon Glass, Michal Simek,
Jerome Forissier, Devarsh Thakkar, Jonas Karlman
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 3/5] board_init_f_r(): use static calls
2024-12-18 15:53 ` [PATCH v2 3/5] board_init_f_r(): " Jerome Forissier
@ 2024-12-21 8:14 ` Ilias Apalodimas
0 siblings, 0 replies; 17+ messages in thread
From: Ilias Apalodimas @ 2024-12-21 8:14 UTC (permalink / raw)
To: Jerome Forissier
Cc: u-boot, Tom Rini, Simon Glass, Michal Simek, Devarsh Thakkar,
Jonas Karlman
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>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 4/5] board_init_r(): use static calls
2024-12-18 15:53 [PATCH v2 0/5] Static initcalls Jerome Forissier
` (2 preceding siblings ...)
2024-12-18 15:53 ` [PATCH v2 3/5] board_init_f_r(): " Jerome Forissier
@ 2024-12-18 15:53 ` Jerome Forissier
2024-12-18 15:53 ` [PATCH v2 5/5] initcall: remove initcall_run_list() Jerome Forissier
2025-01-01 17:55 ` [PATCH v2 0/5] Static initcalls Tom Rini
5 siblings, 0 replies; 17+ messages in thread
From: Jerome Forissier @ 2024-12-18 15:53 UTC (permalink / raw)
To: u-boot
Cc: Ilias Apalodimas, Tom Rini, Simon Glass, Michal Simek,
Jerome Forissier, Sughosh Ganu, Christian Marangi,
Evgeny Bachinin, Caleb Connolly
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:
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 5/5] initcall: remove initcall_run_list()
2024-12-18 15:53 [PATCH v2 0/5] Static initcalls Jerome Forissier
` (3 preceding siblings ...)
2024-12-18 15:53 ` [PATCH v2 4/5] board_init_r(): " Jerome Forissier
@ 2024-12-18 15:53 ` Jerome Forissier
2025-01-01 17:55 ` [PATCH v2 0/5] Static initcalls Tom Rini
5 siblings, 0 replies; 17+ messages in thread
From: Jerome Forissier @ 2024-12-18 15:53 UTC (permalink / raw)
To: u-boot
Cc: Ilias Apalodimas, Tom Rini, Simon Glass, Michal Simek,
Jerome Forissier, Raymond Mao, Sughosh Ganu, Caleb Connolly,
Dan Carpenter
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;
-}
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 0/5] Static initcalls
2024-12-18 15:53 [PATCH v2 0/5] Static initcalls Jerome Forissier
` (4 preceding siblings ...)
2024-12-18 15:53 ` [PATCH v2 5/5] initcall: remove initcall_run_list() Jerome Forissier
@ 2025-01-01 17:55 ` Tom Rini
2025-01-03 1:41 ` Simon Glass
2025-01-03 8:47 ` Jerome Forissier
5 siblings, 2 replies; 17+ messages in thread
From: Tom Rini @ 2025-01-01 17:55 UTC (permalink / raw)
To: Jerome Forissier; +Cc: u-boot, Ilias Apalodimas, Simon Glass, Michal Simek
[-- Attachment #1: Type: text/plain, Size: 633 bytes --]
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
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2 0/5] Static initcalls
2025-01-01 17:55 ` [PATCH v2 0/5] Static initcalls Tom Rini
@ 2025-01-03 1:41 ` Simon Glass
2025-01-03 8:49 ` Jerome Forissier
2025-01-03 8:47 ` Jerome Forissier
1 sibling, 1 reply; 17+ messages in thread
From: Simon Glass @ 2025-01-03 1:41 UTC (permalink / raw)
To: Tom Rini; +Cc: Jerome Forissier, u-boot, Ilias Apalodimas, Michal Simek
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/5] Static initcalls
2025-01-03 1:41 ` Simon Glass
@ 2025-01-03 8:49 ` Jerome Forissier
0 siblings, 0 replies; 17+ messages in thread
From: Jerome Forissier @ 2025-01-03 8:49 UTC (permalink / raw)
To: Simon Glass, Tom Rini; +Cc: u-boot, Ilias Apalodimas, Michal Simek
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,
--
Jerome
>
> Regards,
> Simon
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/5] Static initcalls
2025-01-01 17:55 ` [PATCH v2 0/5] Static initcalls Tom Rini
2025-01-03 1:41 ` Simon Glass
@ 2025-01-03 8:47 ` Jerome Forissier
1 sibling, 0 replies; 17+ messages in thread
From: Jerome Forissier @ 2025-01-03 8:47 UTC (permalink / raw)
To: Tom Rini; +Cc: u-boot, Ilias Apalodimas, Simon Glass, Michal Simek
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,
--
Jerome
^ permalink raw reply related [flat|nested] 17+ messages in thread