Re: [PATCH v3 4/5] board_init_r(): use static calls

Caleb Connolly
2025-01-04T01:55:05Z

Hi Jerome,

On 03/01/2025 12:13, Jerome Forissier wrote:
> 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: 8/23 up/down: 19743/-20923 (-1180)
> Total: Before=1067882, After=1066702, chg -0.11%
>
> - Without LTO:
> add/remove: 0/23 grow/shrink: 2/0 up/down: 795/-1112 (-317)
> Total: Before=1119494, After=1119177, chg -0.03%
>
> Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
> ---

[...]

> + 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);))

Not that the array was extremely readable, but I think this is
significantly worse. Particularly as my IDE will no longer grey-out
sections for config options which are disabled.

I would honestly rather still see pre-processor #if's here, or at least
something better than this semicolon-in-macro stuff.

Kind regards,
> /* 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 +740,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 44239da5280..1b072df73b6 100644
> --- a/test/py/tests/test_trace.py
> +++ b/test/py/tests/test_trace.py
> @@ -201,7 +201,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
> @@ -224,8 +224,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
> @@ -274,7 +274,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:

--
// Caleb (they/them)