| Message ID | cover.1734537135.git.jerome.forissier@linaro.org |
|---|---|
| Headers | show |
| Series | Static initcalls | expand |
On Wed, Dec 18, 2024 at 04:53:53PM +0100, Jerome Forissier wrote: > This series replaces the dynamic initcalls (with function pointers) with > static calls, and gets rid of initcall_run_list(), init_sequence_f, > init_sequence_f_r and init_sequence_r. This makes the code simpler and the > binary slighlty smaller: -2507 bytes/-0.23 % with LTO enabled and -1232 > bytes/-0.11 % with LTO disabled (xilinx_zynqmp_kria_defconfig). > > Execution time doesn't seem to change noticeably. There is no impact on > the SPL. This leads to run-time failures on SH: https://source.denx.de/u-boot/u-boot/-/jobs/986701
Hi Jerome, On Thu, 2 Jan 2025 at 06:55, Tom Rini <trini@konsulko.com> wrote: > > On Wed, Dec 18, 2024 at 04:53:53PM +0100, Jerome Forissier wrote: > > > This series replaces the dynamic initcalls (with function pointers) with > > static calls, and gets rid of initcall_run_list(), init_sequence_f, > > init_sequence_f_r and init_sequence_r. This makes the code simpler and the > > binary slighlty smaller: -2507 bytes/-0.23 % with LTO enabled and -1232 > > bytes/-0.11 % with LTO disabled (xilinx_zynqmp_kria_defconfig). > > > > Execution time doesn't seem to change noticeably. There is no impact on > > the SPL. > > This leads to run-time failures on SH: > https://source.denx.de/u-boot/u-boot/-/jobs/986701 I'm not a huge fan of this series in terms of the style, but this is a significant code-size reduction! For the board_init_f() etc. functions, can you do a follow-up with a comment indicating that logic must not be added?...i.e. that we don't end up with variables, if(), etc. in these functions. I think that would be a good rule to have. Regards, Simon
Hi 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,
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,
This series replaces the dynamic initcalls (with function pointers) with static calls, and gets rid of initcall_run_list(), init_sequence_f, init_sequence_f_r and init_sequence_r. This makes the code simpler and the binary slighlty smaller: -2507 bytes/-0.23 % with LTO enabled and -1232 bytes/-0.11 % with LTO disabled (xilinx_zynqmp_kria_defconfig). Execution time doesn't seem to change noticeably. There is no impact on the SPL. Changes in v2: - INTICALL() and INITCALL_EVT() now call hang() immediately on error - Fixed typo: s/intcall_run_f_r/initcall_run_f_r/ Jerome Forissier (4): board_init_f(): use static calls board_init_f_r(): use static calls board_init_r(): use static calls initcall: remove initcall_run_list() Michal Simek (1): common: board: Simplify array with function pointers with CONFIG_IS_ENABLED common/board_f.c | 210 ++++++++++++++----------------- common/board_r.c | 241 ++++++++++++++---------------------- include/initcall.h | 45 +++---- lib/Makefile | 1 - lib/initcall.c | 102 --------------- test/py/tests/test_trace.py | 8 +- 6 files changed, 210 insertions(+), 397 deletions(-) delete mode 100644 lib/initcall.c