| Message ID | 20241208105223.2821049-2-ilias.apalodimas@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Cleanup the LMB subsystem | expand |
On Sun, Dec 08, 2024 at 12:52:04PM +0200, Ilias Apalodimas wrote: > lmb_reserve() is just calling lmb_reserve_flags() with LMB_NONE. > There's not much we gain from this abstraction, so let's remove the > former and make the code a bit easier to follow. > > The code size increase is minimal - e.g for sandbox which includes all > of the LMB tests > > add/remove: 0/1 grow/shrink: 12/0 up/down: 46/-4 (42) > Function old new delta > lib_test_lmb_overlapping_reserve 1018 1030 +12 > version_string 70 76 +6 > test_get_unreserved_size 1032 1038 +6 > test_alloc_addr 2933 2939 +6 > test_multi_alloc.constprop 3034 3036 +2 > test_bigblock 911 913 +2 > load_serial 946 948 +2 > lib_test_lmb_flags 2101 2103 +2 > do_bootz 526 528 +2 > do_bootm_linux 2067 2069 +2 > bootm_run_states 5275 5277 +2 > boot_relocate_fdt 599 601 +2 > lmb_reserve 4 - -4 > Total: Before=2492742, After=2492784, chg +0.00% > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> Reviewed-by: Tom Rini <trini@konsulko.com>
On Sun, 8 Dec 2024 at 16:22, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > lmb_reserve() is just calling lmb_reserve_flags() with LMB_NONE. > There's not much we gain from this abstraction, so let's remove the > former and make the code a bit easier to follow. > > The code size increase is minimal - e.g for sandbox which includes all > of the LMB tests > > add/remove: 0/1 grow/shrink: 12/0 up/down: 46/-4 (42) > Function old new delta > lib_test_lmb_overlapping_reserve 1018 1030 +12 > version_string 70 76 +6 > test_get_unreserved_size 1032 1038 +6 > test_alloc_addr 2933 2939 +6 > test_multi_alloc.constprop 3034 3036 +2 > test_bigblock 911 913 +2 > load_serial 946 948 +2 > lib_test_lmb_flags 2101 2103 +2 > do_bootz 526 528 +2 > do_bootm_linux 2067 2069 +2 > bootm_run_states 5275 5277 +2 > boot_relocate_fdt 599 601 +2 > lmb_reserve 4 - -4 > Total: Before=2492742, After=2492784, chg +0.00% > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- Fwiw, I had attempted this earlier on similar lines -- have a single API, which would take the flag as a parameter [1], but was asked not to take this approach as part of the review [2]. -sughosh [1] - https://lists.denx.de/pipermail/u-boot/2024-July/559790.html [2] - https://lists.denx.de/pipermail/u-boot/2024-August/561607.html > arch/powerpc/cpu/mpc85xx/mp.c | 2 +- > arch/powerpc/lib/misc.c | 2 +- > boot/bootm.c | 3 ++- > boot/image-board.c | 2 +- > boot/image-fdt.c | 5 +++-- > cmd/booti.c | 2 +- > cmd/bootz.c | 2 +- > cmd/load.c | 2 +- > include/lmb.h | 1 - > lib/lmb.c | 5 ----- > test/lib/lmb.c | 30 +++++++++++++++--------------- > 11 files changed, 26 insertions(+), 30 deletions(-) > > diff --git a/arch/powerpc/cpu/mpc85xx/mp.c b/arch/powerpc/cpu/mpc85xx/mp.c > index bed465cb2cba..22252c1448a8 100644 > --- a/arch/powerpc/cpu/mpc85xx/mp.c > +++ b/arch/powerpc/cpu/mpc85xx/mp.c > @@ -412,7 +412,7 @@ void cpu_mp_lmb_reserve(void) > { > u32 bootpg = determine_mp_bootpg(NULL); > > - lmb_reserve(bootpg, 4096); > + lmb_reserve_flags(bootpg, 4096, LMB_NONE); > } > > void setup_mp(void) > diff --git a/arch/powerpc/lib/misc.c b/arch/powerpc/lib/misc.c > index 4cd23b3406d1..c08d4b35118b 100644 > --- a/arch/powerpc/lib/misc.c > +++ b/arch/powerpc/lib/misc.c > @@ -40,7 +40,7 @@ int arch_misc_init(void) > > printf("WARNING: adjusting available memory from 0x%lx to 0x%llx\n", > size, (unsigned long long)bootm_size); > - lmb_reserve(base, bootm_size - size); > + lmb_reserve_flags(base, bootm_size - size, LMB_NONE); > } > > #ifdef CONFIG_MP > diff --git a/boot/bootm.c b/boot/bootm.c > index 16a43d519a8a..c8662442e403 100644 > --- a/boot/bootm.c > +++ b/boot/bootm.c > @@ -696,7 +696,8 @@ static int bootm_load_os(struct bootm_headers *images, int boot_progress) > } > > if (CONFIG_IS_ENABLED(LMB)) > - lmb_reserve(images->os.load, (load_end - images->os.load)); > + lmb_reserve_flags(images->os.load, (load_end - images->os.load), > + LMB_NONE); > > return 0; > } > diff --git a/boot/image-board.c b/boot/image-board.c > index b726bd6b3035..c4d914fd6074 100644 > --- a/boot/image-board.c > +++ b/boot/image-board.c > @@ -562,7 +562,7 @@ int boot_ramdisk_high(ulong rd_data, ulong rd_len, ulong *initrd_start, > debug(" in-place initrd\n"); > *initrd_start = rd_data; > *initrd_end = rd_data + rd_len; > - lmb_reserve(rd_data, rd_len); > + lmb_reserve_flags(rd_data, rd_len, LMB_NONE); > } else { > if (initrd_high) > *initrd_start = (ulong)lmb_alloc_base(rd_len, > diff --git a/boot/image-fdt.c b/boot/image-fdt.c > index 3d5b6f9e2dc7..fd68b8594325 100644 > --- a/boot/image-fdt.c > +++ b/boot/image-fdt.c > @@ -184,7 +184,8 @@ int boot_relocate_fdt(char **of_flat_tree, ulong *of_size) > if (desired_addr == ~0UL) { > /* All ones means use fdt in place */ > of_start = fdt_blob; > - lmb_reserve(map_to_sysmem(of_start), of_len); > + lmb_reserve_flags(map_to_sysmem(of_start), of_len, > + LMB_NONE); > disable_relocation = 1; > } else if (desired_addr) { > addr = lmb_alloc_base(of_len, 0x1000, desired_addr); > @@ -675,7 +676,7 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob, bool lmb) > > /* Create a new LMB reservation */ > if (CONFIG_IS_ENABLED(LMB) && lmb) > - lmb_reserve(map_to_sysmem(blob), of_size); > + lmb_reserve_flags(map_to_sysmem(blob), of_size, LMB_NONE); > > #if defined(CONFIG_ARCH_KEYSTONE) > if (IS_ENABLED(CONFIG_OF_BOARD_SETUP)) > diff --git a/cmd/booti.c b/cmd/booti.c > index 43e79e87201b..58d355726cc5 100644 > --- a/cmd/booti.c > +++ b/cmd/booti.c > @@ -87,7 +87,7 @@ static int booti_start(struct bootm_info *bmi) > images->os.start = relocated_addr; > images->os.end = relocated_addr + image_size; > > - lmb_reserve(images->ep, le32_to_cpu(image_size)); > + lmb_reserve_flags(images->ep, le32_to_cpu(image_size), LMB_NONE); > > /* > * Handle the BOOTM_STATE_FINDOTHER state ourselves as we do not > diff --git a/cmd/bootz.c b/cmd/bootz.c > index 787203f5bd70..ab2bf12eb8b0 100644 > --- a/cmd/bootz.c > +++ b/cmd/bootz.c > @@ -56,7 +56,7 @@ static int bootz_start(struct cmd_tbl *cmdtp, int flag, int argc, > if (ret != 0) > return 1; > > - lmb_reserve(images->ep, zi_end - zi_start); > + lmb_reserve_flags(images->ep, zi_end - zi_start, LMB_NONE); > > /* > * Handle the BOOTM_STATE_FINDOTHER state ourselves as we do not > diff --git a/cmd/load.c b/cmd/load.c > index 20d802502ae6..dfbb6d2db0c9 100644 > --- a/cmd/load.c > +++ b/cmd/load.c > @@ -179,7 +179,7 @@ static ulong load_serial(long offset) > { > void *dst; > > - ret = lmb_reserve(store_addr, binlen); > + ret = lmb_reserve_flags(store_addr, binlen, LMB_NONE); > if (ret) { > printf("\nCannot overwrite reserved area (%08lx..%08lx)\n", > store_addr, store_addr + binlen); > diff --git a/include/lmb.h b/include/lmb.h > index f221f0cce8f7..62882464f866 100644 > --- a/include/lmb.h > +++ b/include/lmb.h > @@ -82,7 +82,6 @@ int lmb_init(void); > void lmb_add_memory(void); > > long lmb_add(phys_addr_t base, phys_size_t size); > -long lmb_reserve(phys_addr_t base, phys_size_t size); > /** > * lmb_reserve_flags - Reserve one region with a specific flags bitfield. > * > diff --git a/lib/lmb.c b/lib/lmb.c > index b03237bc06cb..a7ecbb58831f 100644 > --- a/lib/lmb.c > +++ b/lib/lmb.c > @@ -698,11 +698,6 @@ long lmb_reserve_flags(phys_addr_t base, phys_size_t size, enum lmb_flags flags) > return lmb_map_update_notify(base, size, MAP_OP_RESERVE, flags); > } > > -long lmb_reserve(phys_addr_t base, phys_size_t size) > -{ > - return lmb_reserve_flags(base, size, LMB_NONE); > -} > - > static phys_addr_t _lmb_alloc_base(phys_size_t size, ulong align, > phys_addr_t max_addr, enum lmb_flags flags) > { > diff --git a/test/lib/lmb.c b/test/lib/lmb.c > index 0bd29e2a4fe7..8af5dcad2ecc 100644 > --- a/test/lib/lmb.c > +++ b/test/lib/lmb.c > @@ -117,7 +117,7 @@ static int test_multi_alloc(struct unit_test_state *uts, const phys_addr_t ram, > } > > /* reserve 64KiB somewhere */ > - ret = lmb_reserve(alloc_64k_addr, 0x10000); > + ret = lmb_reserve_flags(alloc_64k_addr, 0x10000, LMB_NONE); > ut_asserteq(ret, 0); > ASSERT_LMB(mem_lst, used_lst, 0, 0, 1, alloc_64k_addr, 0x10000, > 0, 0, 0, 0); > @@ -264,7 +264,7 @@ static int test_bigblock(struct unit_test_state *uts, const phys_addr_t ram) > ut_asserteq(ret, 0); > > /* reserve 64KiB in the middle of RAM */ > - ret = lmb_reserve(alloc_64k_addr, 0x10000); > + ret = lmb_reserve_flags(alloc_64k_addr, 0x10000, LMB_NONE); > ut_asserteq(ret, 0); > ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, alloc_64k_addr, 0x10000, > 0, 0, 0, 0); > @@ -466,35 +466,35 @@ static int lib_test_lmb_overlapping_reserve(struct unit_test_state *uts) > ret = lmb_add(ram, ram_size); > ut_asserteq(ret, 0); > > - ret = lmb_reserve(0x40010000, 0x10000); > + ret = lmb_reserve_flags(0x40010000, 0x10000, LMB_NONE); > ut_asserteq(ret, 0); > ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, 0x40010000, 0x10000, > 0, 0, 0, 0); > > /* allocate overlapping region should return the coalesced count */ > - ret = lmb_reserve(0x40011000, 0x10000); > + ret = lmb_reserve_flags(0x40011000, 0x10000, LMB_NONE); > ut_asserteq(ret, 0); > ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, 0x40010000, 0x11000, > 0, 0, 0, 0); > /* allocate 3nd region */ > - ret = lmb_reserve(0x40030000, 0x10000); > + ret = lmb_reserve_flags(0x40030000, 0x10000, LMB_NONE); > ut_asserteq(ret, 0); > ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 2, 0x40010000, 0x11000, > 0x40030000, 0x10000, 0, 0); > /* allocate 2nd region , This should coalesced all region into one */ > - ret = lmb_reserve(0x40020000, 0x10000); > + ret = lmb_reserve_flags(0x40020000, 0x10000, LMB_NONE); > ut_assert(ret >= 0); > ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, 0x40010000, 0x30000, > 0, 0, 0, 0); > > /* allocate 2nd region, which should be added as first region */ > - ret = lmb_reserve(0x40000000, 0x8000); > + ret = lmb_reserve_flags(0x40000000, 0x8000, LMB_NONE); > ut_assert(ret >= 0); > ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 2, 0x40000000, 0x8000, > 0x40010000, 0x30000, 0, 0); > > /* allocate 3rd region, coalesce with first and overlap with second */ > - ret = lmb_reserve(0x40008000, 0x10000); > + ret = lmb_reserve_flags(0x40008000, 0x10000, LMB_NONE); > ut_assert(ret >= 0); > ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, 0x40000000, 0x40000, > 0, 0, 0, 0); > @@ -550,11 +550,11 @@ static int test_alloc_addr(struct unit_test_state *uts, const phys_addr_t ram) > ut_asserteq(ret, 0); > > /* reserve 3 blocks */ > - ret = lmb_reserve(alloc_addr_a, 0x10000); > + ret = lmb_reserve_flags(alloc_addr_a, 0x10000, LMB_NONE); > ut_asserteq(ret, 0); > - ret = lmb_reserve(alloc_addr_b, 0x10000); > + ret = lmb_reserve_flags(alloc_addr_b, 0x10000, LMB_NONE); > ut_asserteq(ret, 0); > - ret = lmb_reserve(alloc_addr_c, 0x10000); > + ret = lmb_reserve_flags(alloc_addr_c, 0x10000, LMB_NONE); > ut_asserteq(ret, 0); > ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 3, alloc_addr_a, 0x10000, > alloc_addr_b, 0x10000, alloc_addr_c, 0x10000); > @@ -680,11 +680,11 @@ static int test_get_unreserved_size(struct unit_test_state *uts, > ut_asserteq(ret, 0); > > /* reserve 3 blocks */ > - ret = lmb_reserve(alloc_addr_a, 0x10000); > + ret = lmb_reserve_flags(alloc_addr_a, 0x10000, LMB_NONE); > ut_asserteq(ret, 0); > - ret = lmb_reserve(alloc_addr_b, 0x10000); > + ret = lmb_reserve_flags(alloc_addr_b, 0x10000, LMB_NONE); > ut_asserteq(ret, 0); > - ret = lmb_reserve(alloc_addr_c, 0x10000); > + ret = lmb_reserve_flags(alloc_addr_c, 0x10000, LMB_NONE); > ut_asserteq(ret, 0); > ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 3, alloc_addr_a, 0x10000, > alloc_addr_b, 0x10000, alloc_addr_c, 0x10000); > @@ -789,7 +789,7 @@ static int lib_test_lmb_flags(struct unit_test_state *uts) > ut_asserteq(lmb_is_nomap(&used[1]), 0); > > /* test that old API use LMB_NONE */ > - ret = lmb_reserve(0x40040000, 0x10000); > + ret = lmb_reserve_flags(0x40040000, 0x10000, LMB_NONE); > ut_asserteq(ret, 0); > ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 2, 0x40000000, 0x30000, > 0x40030000, 0x20000, 0, 0); > -- > 2.45.2 >
Hi, On Tue, 10 Dec 2024 at 02:54, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > On Sun, 8 Dec 2024 at 16:22, Ilias Apalodimas > <ilias.apalodimas@linaro.org> wrote: > > > > lmb_reserve() is just calling lmb_reserve_flags() with LMB_NONE. > > There's not much we gain from this abstraction, so let's remove the > > former and make the code a bit easier to follow. > > > > The code size increase is minimal - e.g for sandbox which includes all > > of the LMB tests > > > > add/remove: 0/1 grow/shrink: 12/0 up/down: 46/-4 (42) > > Function old new delta > > lib_test_lmb_overlapping_reserve 1018 1030 +12 > > version_string 70 76 +6 > > test_get_unreserved_size 1032 1038 +6 > > test_alloc_addr 2933 2939 +6 > > test_multi_alloc.constprop 3034 3036 +2 > > test_bigblock 911 913 +2 > > load_serial 946 948 +2 > > lib_test_lmb_flags 2101 2103 +2 > > do_bootz 526 528 +2 > > do_bootm_linux 2067 2069 +2 > > bootm_run_states 5275 5277 +2 > > boot_relocate_fdt 599 601 +2 > > lmb_reserve 4 - -4 > > Total: Before=2492742, After=2492784, chg +0.00% > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > --- > > Fwiw, I had attempted this earlier on similar lines -- have a single > API, which would take the flag as a parameter [1], but was asked not > to take this approach as part of the review [2]. > You missed that sandbox has LTO enabled. With that disabled (like many boards): sandbox: (for 1/1 boards) all -32.0 text -32.0 We just had a patch and discussion to remove a single function call to save 8 bytes[3] For firefly-rk3399 this adds 36 bytes: aarch64: (for 1/1 boards) all +68.0 bss +48.0 text +20.0 You could run buildman on all arm64 boards to check it, perhaps. The other thing is that there is actually only one place apart from tests where the flag is needed - in boot_fdt_add_mem_rsv_regions(). Boards don't use that flag. It is the flags themselves which are confusing. Regards, Simon > -sughosh > > [1] - https://lists.denx.de/pipermail/u-boot/2024-July/559790.html > [2] - https://lists.denx.de/pipermail/u-boot/2024-August/561607.html > > > arch/powerpc/cpu/mpc85xx/mp.c | 2 +- > > arch/powerpc/lib/misc.c | 2 +- > > boot/bootm.c | 3 ++- > > boot/image-board.c | 2 +- > > boot/image-fdt.c | 5 +++-- > > cmd/booti.c | 2 +- > > cmd/bootz.c | 2 +- > > cmd/load.c | 2 +- > > include/lmb.h | 1 - > > lib/lmb.c | 5 ----- > > test/lib/lmb.c | 30 +++++++++++++++--------------- > > 11 files changed, 26 insertions(+), 30 deletions(-) > > [3] https://patchwork.ozlabs.org/project/uboot/patch/f79dc1fbf796dd5ad290f6080608ee68d7652cfc.1730452668.git.michal.simek@amd.com/
Hi Simon, On Tue, 10 Dec 2024 at 18:16, Simon Glass <sjg@chromium.org> wrote: > > Hi, > > On Tue, 10 Dec 2024 at 02:54, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > On Sun, 8 Dec 2024 at 16:22, Ilias Apalodimas > > <ilias.apalodimas@linaro.org> wrote: > > > > > > lmb_reserve() is just calling lmb_reserve_flags() with LMB_NONE. > > > There's not much we gain from this abstraction, so let's remove the > > > former and make the code a bit easier to follow. > > > > > > The code size increase is minimal - e.g for sandbox which includes all > > > of the LMB tests > > > > > > add/remove: 0/1 grow/shrink: 12/0 up/down: 46/-4 (42) > > > Function old new delta > > > lib_test_lmb_overlapping_reserve 1018 1030 +12 > > > version_string 70 76 +6 > > > test_get_unreserved_size 1032 1038 +6 > > > test_alloc_addr 2933 2939 +6 > > > test_multi_alloc.constprop 3034 3036 +2 > > > test_bigblock 911 913 +2 > > > load_serial 946 948 +2 > > > lib_test_lmb_flags 2101 2103 +2 > > > do_bootz 526 528 +2 > > > do_bootm_linux 2067 2069 +2 > > > bootm_run_states 5275 5277 +2 > > > boot_relocate_fdt 599 601 +2 > > > lmb_reserve 4 - -4 > > > Total: Before=2492742, After=2492784, chg +0.00% > > > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > --- > > > > Fwiw, I had attempted this earlier on similar lines -- have a single > > API, which would take the flag as a parameter [1], but was asked not > > to take this approach as part of the review [2]. > > > > You missed that sandbox has LTO enabled. With that disabled (like many boards): > > sandbox: (for 1/1 boards) all -32.0 text -32.0 > > We just had a patch and discussion to remove a single function call to > save 8 bytes[3] > > For firefly-rk3399 this adds 36 bytes: > > aarch64: (for 1/1 boards) all +68.0 bss +48.0 text +20.0 Did you include DM tests in that config? Because applying the series says otherwise for firefly-rk3399_defconfig add/remove: 0/5 grow/shrink: 14/0 up/down: 480/-544 (-64) Function old new delta lmb_alloc_base_flags 76 324 +248 lmb_alloc_addr_flags 4 144 +140 version_string 50 70 +20 boot_relocate_fdt 488 508 +20 boot_ramdisk_high 268 284 +16 tftp_handler 1304 1308 +4 load_serial 512 516 +4 lmb_alloc 8 12 +4 image_setup_libfdt 380 384 +4 do_spi_flash 2164 2168 +4 do_load 732 736 +4 do_bootz 332 336 +4 do_booti 520 524 +4 bootm_run_states 2176 2180 +4 lmb_reserve 8 - -8 lmb_alloc_addr 8 - -8 lmb_alloc_base 80 - -80 _lmb_alloc_addr 144 - -144 _lmb_alloc_base 304 - -304 Total: Before=691587, After=691523, chg -0.01% > > You could run buildman on all arm64 boards to check it, perhaps. I did test on non-LTO platforms as well, building for all is kind of pointless no? It's either LTO or no LTO. FWIW you need to apply the entire series, not just a single patch. rpi_arm64_defconfig add/remove: 0/5 grow/shrink: 11/0 up/down: 452/-544 (-92) Function old new delta lmb_alloc_base_flags 76 324 +248 lmb_alloc_addr_flags 4 144 +140 boot_relocate_fdt 516 536 +20 boot_ramdisk_high 268 284 +16 tftp_handler 1552 1556 +4 load_serial 512 516 +4 lmb_alloc 8 12 +4 image_setup_libfdt 456 460 +4 do_load 728 732 +4 do_booti 520 524 +4 bootm_run_states 1824 1828 +4 lmb_reserve 8 - -8 lmb_alloc_addr 8 - -8 lmb_alloc_base 80 - -80 _lmb_alloc_addr 144 - -144 _lmb_alloc_base 304 - -304 Total: Before=542062, After=541970, chg -0.02% qemu_arm64_lwip_defconfig add/remove: 0/5 grow/shrink: 11/0 up/down: 452/-544 (-92) Function old new delta lmb_alloc_base_flags 76 324 +248 lmb_alloc_addr_flags 4 144 +140 boot_relocate_fdt 488 508 +20 boot_ramdisk_high 268 284 +16 load_serial 548 552 +4 lmb_alloc 8 12 +4 image_setup_libfdt 368 372 +4 do_load 728 732 +4 do_bootz 332 336 +4 do_booti 520 524 +4 bootm_run_states 2176 2180 +4 lmb_reserve 8 - -8 lmb_alloc_addr 8 - -8 lmb_alloc_base 80 - -80 _lmb_alloc_addr 144 - -144 _lmb_alloc_base 304 - -304 Total: Before=1020122, After=1020030, chg -0.01% > > The other thing is that there is actually only one place apart from > tests where the flag is needed - in boot_fdt_add_mem_rsv_regions(). > Boards don't use that flag. It is the flags themselves which are > confusing. > I still think it's way easier to read without two levels of abstraction. Since the series decreases the size -- unless you include all DM Tests, I'd like to keep the flags as is for now, so I can send the next round which is the actual cleanup. We can re-introduce them if someone finds a 'size problem'. But since it's only when you enable DM I doubt anyone wants to have them working in SPL or production firmware. /Ilias > Regards, > Simon > > > -sughosh > > > > [1] - https://lists.denx.de/pipermail/u-boot/2024-July/559790.html > > [2] - https://lists.denx.de/pipermail/u-boot/2024-August/561607.html > > > > > arch/powerpc/cpu/mpc85xx/mp.c | 2 +- > > > arch/powerpc/lib/misc.c | 2 +- > > > boot/bootm.c | 3 ++- > > > boot/image-board.c | 2 +- > > > boot/image-fdt.c | 5 +++-- > > > cmd/booti.c | 2 +- > > > cmd/bootz.c | 2 +- > > > cmd/load.c | 2 +- > > > include/lmb.h | 1 - > > > lib/lmb.c | 5 ----- > > > test/lib/lmb.c | 30 +++++++++++++++--------------- > > > 11 files changed, 26 insertions(+), 30 deletions(-) > > > > > [3] https://patchwork.ozlabs.org/project/uboot/patch/f79dc1fbf796dd5ad290f6080608ee68d7652cfc.1730452668.git.michal.simek@amd.com/
Hi Ilias, On Tue, 10 Dec 2024 at 11:00, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Simon, > > > On Tue, 10 Dec 2024 at 18:16, Simon Glass <sjg@chromium.org> wrote: > > > > Hi, > > > > On Tue, 10 Dec 2024 at 02:54, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > On Sun, 8 Dec 2024 at 16:22, Ilias Apalodimas > > > <ilias.apalodimas@linaro.org> wrote: > > > > > > > > lmb_reserve() is just calling lmb_reserve_flags() with LMB_NONE. > > > > There's not much we gain from this abstraction, so let's remove the > > > > former and make the code a bit easier to follow. > > > > > > > > The code size increase is minimal - e.g for sandbox which includes all > > > > of the LMB tests > > > > > > > > add/remove: 0/1 grow/shrink: 12/0 up/down: 46/-4 (42) > > > > Function old new delta > > > > lib_test_lmb_overlapping_reserve 1018 1030 +12 > > > > version_string 70 76 +6 > > > > test_get_unreserved_size 1032 1038 +6 > > > > test_alloc_addr 2933 2939 +6 > > > > test_multi_alloc.constprop 3034 3036 +2 > > > > test_bigblock 911 913 +2 > > > > load_serial 946 948 +2 > > > > lib_test_lmb_flags 2101 2103 +2 > > > > do_bootz 526 528 +2 > > > > do_bootm_linux 2067 2069 +2 > > > > bootm_run_states 5275 5277 +2 > > > > boot_relocate_fdt 599 601 +2 > > > > lmb_reserve 4 - -4 > > > > Total: Before=2492742, After=2492784, chg +0.00% > > > > > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > > --- > > > > > > Fwiw, I had attempted this earlier on similar lines -- have a single > > > API, which would take the flag as a parameter [1], but was asked not > > > to take this approach as part of the review [2]. > > > > > > > You missed that sandbox has LTO enabled. With that disabled (like many boards): > > > > sandbox: (for 1/1 boards) all -32.0 text -32.0 > > > > We just had a patch and discussion to remove a single function call to > > save 8 bytes[3] > > > > For firefly-rk3399 this adds 36 bytes: > > > > aarch64: (for 1/1 boards) all +68.0 bss +48.0 text +20.0 > > Did you include DM tests in that config? Because applying the series > says otherwise > > for firefly-rk3399_defconfig > add/remove: 0/5 grow/shrink: 14/0 up/down: 480/-544 (-64) > Function old new delta > lmb_alloc_base_flags 76 324 +248 > lmb_alloc_addr_flags 4 144 +140 > version_string 50 70 +20 > boot_relocate_fdt 488 508 +20 > boot_ramdisk_high 268 284 +16 > tftp_handler 1304 1308 +4 > load_serial 512 516 +4 > lmb_alloc 8 12 +4 > image_setup_libfdt 380 384 +4 > do_spi_flash 2164 2168 +4 > do_load 732 736 +4 > do_bootz 332 336 +4 > do_booti 520 524 +4 > bootm_run_states 2176 2180 +4 > lmb_reserve 8 - -8 > lmb_alloc_addr 8 - -8 > lmb_alloc_base 80 - -80 > _lmb_alloc_addr 144 - -144 > _lmb_alloc_base 304 - -304 > Total: Before=691587, After=691523, chg -0.01% OK good > > > > > You could run buildman on all arm64 boards to check it, perhaps. > > I did test on non-LTO platforms as well, building for all is kind of > pointless no? It's either LTO or no LTO. Right > FWIW you need to apply the entire series, not just a single patch. > Yes, understood, I was just replying to that one patch. > rpi_arm64_defconfig > add/remove: 0/5 grow/shrink: 11/0 up/down: 452/-544 (-92) > Function old new delta > lmb_alloc_base_flags 76 324 +248 > lmb_alloc_addr_flags 4 144 +140 > boot_relocate_fdt 516 536 +20 > boot_ramdisk_high 268 284 +16 > tftp_handler 1552 1556 +4 > load_serial 512 516 +4 > lmb_alloc 8 12 +4 > image_setup_libfdt 456 460 +4 > do_load 728 732 +4 > do_booti 520 524 +4 > bootm_run_states 1824 1828 +4 > lmb_reserve 8 - -8 > lmb_alloc_addr 8 - -8 > lmb_alloc_base 80 - -80 > _lmb_alloc_addr 144 - -144 > _lmb_alloc_base 304 - -304 > Total: Before=542062, After=541970, chg -0.02% > > qemu_arm64_lwip_defconfig > add/remove: 0/5 grow/shrink: 11/0 up/down: 452/-544 (-92) > Function old new delta > lmb_alloc_base_flags 76 324 +248 > lmb_alloc_addr_flags 4 144 +140 > boot_relocate_fdt 488 508 +20 > boot_ramdisk_high 268 284 +16 > load_serial 548 552 +4 > lmb_alloc 8 12 +4 > image_setup_libfdt 368 372 +4 > do_load 728 732 +4 > do_bootz 332 336 +4 > do_booti 520 524 +4 > bootm_run_states 2176 2180 +4 > lmb_reserve 8 - -8 > lmb_alloc_addr 8 - -8 > lmb_alloc_base 80 - -80 > _lmb_alloc_addr 144 - -144 > _lmb_alloc_base 304 - -304 > Total: Before=1020122, After=1020030, chg -0.01% > > > > > > The other thing is that there is actually only one place apart from > > tests where the flag is needed - in boot_fdt_add_mem_rsv_regions(). > > Boards don't use that flag. It is the flags themselves which are > > confusing. > > > > I still think it's way easier to read without two levels of > abstraction. Since the series decreases the size -- unless you include > all DM Tests, I'd like to keep the flags as is for now, so I can send > the next round which is the actual cleanup. > We can re-introduce them if someone finds a 'size problem'. But since > it's only when you enable DM I doubt anyone wants to have them working > in SPL or production firmware. OK, well I don't mind, if there is no growth. I wonder if you could do better overall without this patch, but perhaps not. > > /Ilias > > Regards, > > Simon > > > > > -sughosh > > > > > > [1] - https://lists.denx.de/pipermail/u-boot/2024-July/559790.html > > > [2] - https://lists.denx.de/pipermail/u-boot/2024-August/561607.html > > > > > > > arch/powerpc/cpu/mpc85xx/mp.c | 2 +- > > > > arch/powerpc/lib/misc.c | 2 +- > > > > boot/bootm.c | 3 ++- > > > > boot/image-board.c | 2 +- > > > > boot/image-fdt.c | 5 +++-- > > > > cmd/booti.c | 2 +- > > > > cmd/bootz.c | 2 +- > > > > cmd/load.c | 2 +- > > > > include/lmb.h | 1 - > > > > lib/lmb.c | 5 ----- > > > > test/lib/lmb.c | 30 +++++++++++++++--------------- > > > > 11 files changed, 26 insertions(+), 30 deletions(-) > > > > > > > > [3] https://patchwork.ozlabs.org/project/uboot/patch/f79dc1fbf796dd5ad290f6080608ee68d7652cfc.1730452668.git.michal.simek@amd.com/ Regards, SImon
Thanks Simon, On Tue, 10 Dec 2024 at 20:58, Simon Glass <sjg@chromium.org> wrote: > > Hi Ilias, > > On Tue, 10 Dec 2024 at 11:00, Ilias Apalodimas > <ilias.apalodimas@linaro.org> wrote: > > > > Hi Simon, > > > > > > On Tue, 10 Dec 2024 at 18:16, Simon Glass <sjg@chromium.org> wrote: > > > > > > Hi, > > > > > > On Tue, 10 Dec 2024 at 02:54, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > > > On Sun, 8 Dec 2024 at 16:22, Ilias Apalodimas > > > > <ilias.apalodimas@linaro.org> wrote: > > > > > > > > > > lmb_reserve() is just calling lmb_reserve_flags() with LMB_NONE. > > > > > There's not much we gain from this abstraction, so let's remove the > > > > > former and make the code a bit easier to follow. > > > > > > > > > > The code size increase is minimal - e.g for sandbox which includes all > > > > > of the LMB tests > > > > > > > > > > add/remove: 0/1 grow/shrink: 12/0 up/down: 46/-4 (42) > > > > > Function old new delta > > > > > lib_test_lmb_overlapping_reserve 1018 1030 +12 > > > > > version_string 70 76 +6 > > > > > test_get_unreserved_size 1032 1038 +6 > > > > > test_alloc_addr 2933 2939 +6 > > > > > test_multi_alloc.constprop 3034 3036 +2 > > > > > test_bigblock 911 913 +2 > > > > > load_serial 946 948 +2 > > > > > lib_test_lmb_flags 2101 2103 +2 > > > > > do_bootz 526 528 +2 > > > > > do_bootm_linux 2067 2069 +2 > > > > > bootm_run_states 5275 5277 +2 > > > > > boot_relocate_fdt 599 601 +2 > > > > > lmb_reserve 4 - -4 > > > > > Total: Before=2492742, After=2492784, chg +0.00% > > > > > > > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > > > > --- > > > > > > > > Fwiw, I had attempted this earlier on similar lines -- have a single > > > > API, which would take the flag as a parameter [1], but was asked not > > > > to take this approach as part of the review [2]. > > > > > > > > > > You missed that sandbox has LTO enabled. With that disabled (like many boards): > > > > > > sandbox: (for 1/1 boards) all -32.0 text -32.0 > > > > > > We just had a patch and discussion to remove a single function call to > > > save 8 bytes[3] > > > > > > For firefly-rk3399 this adds 36 bytes: > > > > > > aarch64: (for 1/1 boards) all +68.0 bss +48.0 text +20.0 > > > > Did you include DM tests in that config? Because applying the series > > says otherwise > > > > for firefly-rk3399_defconfig > > add/remove: 0/5 grow/shrink: 14/0 up/down: 480/-544 (-64) > > Function old new delta > > lmb_alloc_base_flags 76 324 +248 > > lmb_alloc_addr_flags 4 144 +140 > > version_string 50 70 +20 > > boot_relocate_fdt 488 508 +20 > > boot_ramdisk_high 268 284 +16 > > tftp_handler 1304 1308 +4 > > load_serial 512 516 +4 > > lmb_alloc 8 12 +4 > > image_setup_libfdt 380 384 +4 > > do_spi_flash 2164 2168 +4 > > do_load 732 736 +4 > > do_bootz 332 336 +4 > > do_booti 520 524 +4 > > bootm_run_states 2176 2180 +4 > > lmb_reserve 8 - -8 > > lmb_alloc_addr 8 - -8 > > lmb_alloc_base 80 - -80 > > _lmb_alloc_addr 144 - -144 > > _lmb_alloc_base 304 - -304 > > Total: Before=691587, After=691523, chg -0.01% > > OK good > > > > > > > > > You could run buildman on all arm64 boards to check it, perhaps. > > > > I did test on non-LTO platforms as well, building for all is kind of > > pointless no? It's either LTO or no LTO. > > Right > > > FWIW you need to apply the entire series, not just a single patch. > > > > Yes, understood, I was just replying to that one patch. > > > rpi_arm64_defconfig > > add/remove: 0/5 grow/shrink: 11/0 up/down: 452/-544 (-92) > > Function old new delta > > lmb_alloc_base_flags 76 324 +248 > > lmb_alloc_addr_flags 4 144 +140 > > boot_relocate_fdt 516 536 +20 > > boot_ramdisk_high 268 284 +16 > > tftp_handler 1552 1556 +4 > > load_serial 512 516 +4 > > lmb_alloc 8 12 +4 > > image_setup_libfdt 456 460 +4 > > do_load 728 732 +4 > > do_booti 520 524 +4 > > bootm_run_states 1824 1828 +4 > > lmb_reserve 8 - -8 > > lmb_alloc_addr 8 - -8 > > lmb_alloc_base 80 - -80 > > _lmb_alloc_addr 144 - -144 > > _lmb_alloc_base 304 - -304 > > Total: Before=542062, After=541970, chg -0.02% > > > > qemu_arm64_lwip_defconfig > > add/remove: 0/5 grow/shrink: 11/0 up/down: 452/-544 (-92) > > Function old new delta > > lmb_alloc_base_flags 76 324 +248 > > lmb_alloc_addr_flags 4 144 +140 > > boot_relocate_fdt 488 508 +20 > > boot_ramdisk_high 268 284 +16 > > load_serial 548 552 +4 > > lmb_alloc 8 12 +4 > > image_setup_libfdt 368 372 +4 > > do_load 728 732 +4 > > do_bootz 332 336 +4 > > do_booti 520 524 +4 > > bootm_run_states 2176 2180 +4 > > lmb_reserve 8 - -8 > > lmb_alloc_addr 8 - -8 > > lmb_alloc_base 80 - -80 > > _lmb_alloc_addr 144 - -144 > > _lmb_alloc_base 304 - -304 > > Total: Before=1020122, After=1020030, chg -0.01% > > > > > > > > > > The other thing is that there is actually only one place apart from > > > tests where the flag is needed - in boot_fdt_add_mem_rsv_regions(). > > > Boards don't use that flag. It is the flags themselves which are > > > confusing. > > > > > > > I still think it's way easier to read without two levels of > > abstraction. Since the series decreases the size -- unless you include > > all DM Tests, I'd like to keep the flags as is for now, so I can send > > the next round which is the actual cleanup. > > We can re-introduce them if someone finds a 'size problem'. But since > > it's only when you enable DM I doubt anyone wants to have them working > > in SPL or production firmware. > > OK, well I don't mind, if there is no growth. I wonder if you could do > better overall without this patch, but perhaps not. Probably yes, but we are not looking into anything more than a few bytes -- probably under 4 as well. TBH if we all agree that removing the non-flag variants is fine, I can name the functions without it. /Ilias > > > > > /Ilias > > > Regards, > > > Simon > > > > > > > -sughosh > > > > > > > > [1] - https://lists.denx.de/pipermail/u-boot/2024-July/559790.html > > > > [2] - https://lists.denx.de/pipermail/u-boot/2024-August/561607.html > > > > > > > > > arch/powerpc/cpu/mpc85xx/mp.c | 2 +- > > > > > arch/powerpc/lib/misc.c | 2 +- > > > > > boot/bootm.c | 3 ++- > > > > > boot/image-board.c | 2 +- > > > > > boot/image-fdt.c | 5 +++-- > > > > > cmd/booti.c | 2 +- > > > > > cmd/bootz.c | 2 +- > > > > > cmd/load.c | 2 +- > > > > > include/lmb.h | 1 - > > > > > lib/lmb.c | 5 ----- > > > > > test/lib/lmb.c | 30 +++++++++++++++--------------- > > > > > 11 files changed, 26 insertions(+), 30 deletions(-) > > > > > > > > > > > [3] https://patchwork.ozlabs.org/project/uboot/patch/f79dc1fbf796dd5ad290f6080608ee68d7652cfc.1730452668.git.michal.simek@amd.com/ > > Regards, > SImon
diff --git a/arch/powerpc/cpu/mpc85xx/mp.c b/arch/powerpc/cpu/mpc85xx/mp.c index bed465cb2cba..22252c1448a8 100644 --- a/arch/powerpc/cpu/mpc85xx/mp.c +++ b/arch/powerpc/cpu/mpc85xx/mp.c @@ -412,7 +412,7 @@ void cpu_mp_lmb_reserve(void) { u32 bootpg = determine_mp_bootpg(NULL); - lmb_reserve(bootpg, 4096); + lmb_reserve_flags(bootpg, 4096, LMB_NONE); } void setup_mp(void) diff --git a/arch/powerpc/lib/misc.c b/arch/powerpc/lib/misc.c index 4cd23b3406d1..c08d4b35118b 100644 --- a/arch/powerpc/lib/misc.c +++ b/arch/powerpc/lib/misc.c @@ -40,7 +40,7 @@ int arch_misc_init(void) printf("WARNING: adjusting available memory from 0x%lx to 0x%llx\n", size, (unsigned long long)bootm_size); - lmb_reserve(base, bootm_size - size); + lmb_reserve_flags(base, bootm_size - size, LMB_NONE); } #ifdef CONFIG_MP diff --git a/boot/bootm.c b/boot/bootm.c index 16a43d519a8a..c8662442e403 100644 --- a/boot/bootm.c +++ b/boot/bootm.c @@ -696,7 +696,8 @@ static int bootm_load_os(struct bootm_headers *images, int boot_progress) } if (CONFIG_IS_ENABLED(LMB)) - lmb_reserve(images->os.load, (load_end - images->os.load)); + lmb_reserve_flags(images->os.load, (load_end - images->os.load), + LMB_NONE); return 0; } diff --git a/boot/image-board.c b/boot/image-board.c index b726bd6b3035..c4d914fd6074 100644 --- a/boot/image-board.c +++ b/boot/image-board.c @@ -562,7 +562,7 @@ int boot_ramdisk_high(ulong rd_data, ulong rd_len, ulong *initrd_start, debug(" in-place initrd\n"); *initrd_start = rd_data; *initrd_end = rd_data + rd_len; - lmb_reserve(rd_data, rd_len); + lmb_reserve_flags(rd_data, rd_len, LMB_NONE); } else { if (initrd_high) *initrd_start = (ulong)lmb_alloc_base(rd_len, diff --git a/boot/image-fdt.c b/boot/image-fdt.c index 3d5b6f9e2dc7..fd68b8594325 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -184,7 +184,8 @@ int boot_relocate_fdt(char **of_flat_tree, ulong *of_size) if (desired_addr == ~0UL) { /* All ones means use fdt in place */ of_start = fdt_blob; - lmb_reserve(map_to_sysmem(of_start), of_len); + lmb_reserve_flags(map_to_sysmem(of_start), of_len, + LMB_NONE); disable_relocation = 1; } else if (desired_addr) { addr = lmb_alloc_base(of_len, 0x1000, desired_addr); @@ -675,7 +676,7 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob, bool lmb) /* Create a new LMB reservation */ if (CONFIG_IS_ENABLED(LMB) && lmb) - lmb_reserve(map_to_sysmem(blob), of_size); + lmb_reserve_flags(map_to_sysmem(blob), of_size, LMB_NONE); #if defined(CONFIG_ARCH_KEYSTONE) if (IS_ENABLED(CONFIG_OF_BOARD_SETUP)) diff --git a/cmd/booti.c b/cmd/booti.c index 43e79e87201b..58d355726cc5 100644 --- a/cmd/booti.c +++ b/cmd/booti.c @@ -87,7 +87,7 @@ static int booti_start(struct bootm_info *bmi) images->os.start = relocated_addr; images->os.end = relocated_addr + image_size; - lmb_reserve(images->ep, le32_to_cpu(image_size)); + lmb_reserve_flags(images->ep, le32_to_cpu(image_size), LMB_NONE); /* * Handle the BOOTM_STATE_FINDOTHER state ourselves as we do not diff --git a/cmd/bootz.c b/cmd/bootz.c index 787203f5bd70..ab2bf12eb8b0 100644 --- a/cmd/bootz.c +++ b/cmd/bootz.c @@ -56,7 +56,7 @@ static int bootz_start(struct cmd_tbl *cmdtp, int flag, int argc, if (ret != 0) return 1; - lmb_reserve(images->ep, zi_end - zi_start); + lmb_reserve_flags(images->ep, zi_end - zi_start, LMB_NONE); /* * Handle the BOOTM_STATE_FINDOTHER state ourselves as we do not diff --git a/cmd/load.c b/cmd/load.c index 20d802502ae6..dfbb6d2db0c9 100644 --- a/cmd/load.c +++ b/cmd/load.c @@ -179,7 +179,7 @@ static ulong load_serial(long offset) { void *dst; - ret = lmb_reserve(store_addr, binlen); + ret = lmb_reserve_flags(store_addr, binlen, LMB_NONE); if (ret) { printf("\nCannot overwrite reserved area (%08lx..%08lx)\n", store_addr, store_addr + binlen); diff --git a/include/lmb.h b/include/lmb.h index f221f0cce8f7..62882464f866 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -82,7 +82,6 @@ int lmb_init(void); void lmb_add_memory(void); long lmb_add(phys_addr_t base, phys_size_t size); -long lmb_reserve(phys_addr_t base, phys_size_t size); /** * lmb_reserve_flags - Reserve one region with a specific flags bitfield. * diff --git a/lib/lmb.c b/lib/lmb.c index b03237bc06cb..a7ecbb58831f 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -698,11 +698,6 @@ long lmb_reserve_flags(phys_addr_t base, phys_size_t size, enum lmb_flags flags) return lmb_map_update_notify(base, size, MAP_OP_RESERVE, flags); } -long lmb_reserve(phys_addr_t base, phys_size_t size) -{ - return lmb_reserve_flags(base, size, LMB_NONE); -} - static phys_addr_t _lmb_alloc_base(phys_size_t size, ulong align, phys_addr_t max_addr, enum lmb_flags flags) { diff --git a/test/lib/lmb.c b/test/lib/lmb.c index 0bd29e2a4fe7..8af5dcad2ecc 100644 --- a/test/lib/lmb.c +++ b/test/lib/lmb.c @@ -117,7 +117,7 @@ static int test_multi_alloc(struct unit_test_state *uts, const phys_addr_t ram, } /* reserve 64KiB somewhere */ - ret = lmb_reserve(alloc_64k_addr, 0x10000); + ret = lmb_reserve_flags(alloc_64k_addr, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, 0, 0, 1, alloc_64k_addr, 0x10000, 0, 0, 0, 0); @@ -264,7 +264,7 @@ static int test_bigblock(struct unit_test_state *uts, const phys_addr_t ram) ut_asserteq(ret, 0); /* reserve 64KiB in the middle of RAM */ - ret = lmb_reserve(alloc_64k_addr, 0x10000); + ret = lmb_reserve_flags(alloc_64k_addr, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, alloc_64k_addr, 0x10000, 0, 0, 0, 0); @@ -466,35 +466,35 @@ static int lib_test_lmb_overlapping_reserve(struct unit_test_state *uts) ret = lmb_add(ram, ram_size); ut_asserteq(ret, 0); - ret = lmb_reserve(0x40010000, 0x10000); + ret = lmb_reserve_flags(0x40010000, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, 0x40010000, 0x10000, 0, 0, 0, 0); /* allocate overlapping region should return the coalesced count */ - ret = lmb_reserve(0x40011000, 0x10000); + ret = lmb_reserve_flags(0x40011000, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, 0x40010000, 0x11000, 0, 0, 0, 0); /* allocate 3nd region */ - ret = lmb_reserve(0x40030000, 0x10000); + ret = lmb_reserve_flags(0x40030000, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 2, 0x40010000, 0x11000, 0x40030000, 0x10000, 0, 0); /* allocate 2nd region , This should coalesced all region into one */ - ret = lmb_reserve(0x40020000, 0x10000); + ret = lmb_reserve_flags(0x40020000, 0x10000, LMB_NONE); ut_assert(ret >= 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, 0x40010000, 0x30000, 0, 0, 0, 0); /* allocate 2nd region, which should be added as first region */ - ret = lmb_reserve(0x40000000, 0x8000); + ret = lmb_reserve_flags(0x40000000, 0x8000, LMB_NONE); ut_assert(ret >= 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 2, 0x40000000, 0x8000, 0x40010000, 0x30000, 0, 0); /* allocate 3rd region, coalesce with first and overlap with second */ - ret = lmb_reserve(0x40008000, 0x10000); + ret = lmb_reserve_flags(0x40008000, 0x10000, LMB_NONE); ut_assert(ret >= 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, 0x40000000, 0x40000, 0, 0, 0, 0); @@ -550,11 +550,11 @@ static int test_alloc_addr(struct unit_test_state *uts, const phys_addr_t ram) ut_asserteq(ret, 0); /* reserve 3 blocks */ - ret = lmb_reserve(alloc_addr_a, 0x10000); + ret = lmb_reserve_flags(alloc_addr_a, 0x10000, LMB_NONE); ut_asserteq(ret, 0); - ret = lmb_reserve(alloc_addr_b, 0x10000); + ret = lmb_reserve_flags(alloc_addr_b, 0x10000, LMB_NONE); ut_asserteq(ret, 0); - ret = lmb_reserve(alloc_addr_c, 0x10000); + ret = lmb_reserve_flags(alloc_addr_c, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 3, alloc_addr_a, 0x10000, alloc_addr_b, 0x10000, alloc_addr_c, 0x10000); @@ -680,11 +680,11 @@ static int test_get_unreserved_size(struct unit_test_state *uts, ut_asserteq(ret, 0); /* reserve 3 blocks */ - ret = lmb_reserve(alloc_addr_a, 0x10000); + ret = lmb_reserve_flags(alloc_addr_a, 0x10000, LMB_NONE); ut_asserteq(ret, 0); - ret = lmb_reserve(alloc_addr_b, 0x10000); + ret = lmb_reserve_flags(alloc_addr_b, 0x10000, LMB_NONE); ut_asserteq(ret, 0); - ret = lmb_reserve(alloc_addr_c, 0x10000); + ret = lmb_reserve_flags(alloc_addr_c, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 3, alloc_addr_a, 0x10000, alloc_addr_b, 0x10000, alloc_addr_c, 0x10000); @@ -789,7 +789,7 @@ static int lib_test_lmb_flags(struct unit_test_state *uts) ut_asserteq(lmb_is_nomap(&used[1]), 0); /* test that old API use LMB_NONE */ - ret = lmb_reserve(0x40040000, 0x10000); + ret = lmb_reserve_flags(0x40040000, 0x10000, LMB_NONE); ut_asserteq(ret, 0); ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 2, 0x40000000, 0x30000, 0x40030000, 0x20000, 0, 0);
lmb_reserve() is just calling lmb_reserve_flags() with LMB_NONE. There's not much we gain from this abstraction, so let's remove the former and make the code a bit easier to follow. The code size increase is minimal - e.g for sandbox which includes all of the LMB tests add/remove: 0/1 grow/shrink: 12/0 up/down: 46/-4 (42) Function old new delta lib_test_lmb_overlapping_reserve 1018 1030 +12 version_string 70 76 +6 test_get_unreserved_size 1032 1038 +6 test_alloc_addr 2933 2939 +6 test_multi_alloc.constprop 3034 3036 +2 test_bigblock 911 913 +2 load_serial 946 948 +2 lib_test_lmb_flags 2101 2103 +2 do_bootz 526 528 +2 do_bootm_linux 2067 2069 +2 bootm_run_states 5275 5277 +2 boot_relocate_fdt 599 601 +2 lmb_reserve 4 - -4 Total: Before=2492742, After=2492784, chg +0.00% Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> --- arch/powerpc/cpu/mpc85xx/mp.c | 2 +- arch/powerpc/lib/misc.c | 2 +- boot/bootm.c | 3 ++- boot/image-board.c | 2 +- boot/image-fdt.c | 5 +++-- cmd/booti.c | 2 +- cmd/bootz.c | 2 +- cmd/load.c | 2 +- include/lmb.h | 1 - lib/lmb.c | 5 ----- test/lib/lmb.c | 30 +++++++++++++++--------------- 11 files changed, 26 insertions(+), 30 deletions(-)