[RFC PATCH 1/7] lmb: Replace lmb_reserve() with lmb_reserve_flags()

[Simon Glass]

Hi,

On Tue, 10 Dec 2024 at 02:54, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> On Sun, 8 Dec 2024 at 16:22, Ilias Apalodimas
> <ilias.apalodimas at 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 at 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/