[PATCH v1 6/8] lmb: Remove lmb_alloc_addr_flags()

[Ilias Apalodimas]

On Fri, 13 Dec 2024 at 00:34, Sam Protsenko <semen.protsenko at linaro.org> wrote:
>
> On Wed, Dec 11, 2024 at 4:55 AM Ilias Apalodimas
> <ilias.apalodimas at linaro.org> wrote:
> >
> > lmb_alloc_addr() is just calling lmb_alloc_addr_flags() with LMB_NONE
> > There's not much we gain from this abstraction, so let's remove the
> > latter, add a flags argument to lmb_alloc_addr() and make the code a
> > bit easier to follow.
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > ---
> >  fs/fs.c                     |  2 +-
> >  include/lmb.h               | 10 ++++------
> >  lib/efi_loader/efi_memory.c |  2 +-
> >  lib/lmb.c                   | 15 ++-------------
> >  test/lib/lmb.c              | 34 +++++++++++++++++-----------------
> >  5 files changed, 25 insertions(+), 38 deletions(-)
> >
> > diff --git a/fs/fs.c b/fs/fs.c
> > index 21a23efd932d..99ddcc5e37be 100644
> > --- a/fs/fs.c
> > +++ b/fs/fs.c
> > @@ -554,7 +554,7 @@ static int fs_read_lmb_check(const char *filename, ulong addr, loff_t offset,
> >
> >         lmb_dump_all();
> >
> > -       if (lmb_alloc_addr(addr, read_len) == addr)
> > +       if (lmb_alloc_addr(addr, read_len, LMB_NONE) == addr)
> >                 return 0;
> >
> >         log_err("** Reading file would overwrite reserved memory **\n");
> > diff --git a/include/lmb.h b/include/lmb.h
> > index 3db35992df8d..5e59915340b7 100644
> > --- a/include/lmb.h
> > +++ b/include/lmb.h
> > @@ -92,7 +92,6 @@ long lmb_reserve(phys_addr_t base, phys_size_t size, u32 flags);
> >
> >  phys_addr_t lmb_alloc(phys_size_t size, ulong align);
> >  phys_addr_t lmb_alloc_base(phys_size_t size, ulong align, phys_addr_t max_addr);
> > -phys_addr_t lmb_alloc_addr(phys_addr_t base, phys_size_t size);
> >  phys_size_t lmb_get_free_size(phys_addr_t addr);
> >
> >  /**
> > @@ -113,8 +112,8 @@ phys_addr_t lmb_alloc_base_flags(phys_size_t size, ulong align,
> >                                  phys_addr_t max_addr, uint flags);
> >
> >  /**
> > - * lmb_alloc_addr_flags() - Allocate specified memory address with specified
> > - *                         attributes
> > + * lmb_alloc_addr() - Allocate specified memory address with specified attributes
>
> One character over 80 limit trips my OCD :/ Any chance you can move
> "attributes" to the next line? It's ok to break the line in kernel-doc
> function briefs.
>
> > + *
> >   * @base: Base Address requested
> >   * @size: Size of the region requested
> >   * @flags: Memory region attributes to be set
> > @@ -125,8 +124,7 @@ phys_addr_t lmb_alloc_base_flags(phys_size_t size, ulong align,
> >   *
> >   * Return: Base address on success, 0 on error.
> >   */
> > -phys_addr_t lmb_alloc_addr_flags(phys_addr_t base, phys_size_t size,
> > -                                uint flags);
> > +phys_addr_t lmb_alloc_addr(phys_addr_t base, phys_size_t size, uint flags);
>
> Can we keep u32 for flags everywhere, for consistency?

We are, I thought it would make the patches easier to review if I only
changed the enum here.
This turns to a u32 in the last patch


>
> With above nitpicks addressed, feel free to add:
>
> Reviewed-by: Sam Protsenko <semen.protsenko at linaro.org>

Thanks!
/Ilias

>
> [snip]