[RFC PATCH 7/7] lmb: Rename _lmb_alloc_addr_flags()

[Ilias Apalodimas]

On Sun, 8 Dec 2024 at 14:12, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Heinrich
>
> On Sun, 8 Dec 2024 at 14:05, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >
> > Am 8. Dezember 2024 11:52:10 MEZ schrieb Ilias Apalodimas <ilias.apalodimas at linaro.org>:
> > >After all the cleanups of the previous patches there's not point anymore
> > >to define lmb_alloc_addr_flags() and _lmb_alloc_addr_flags().
> > >
> > >Fix the incocistency of the flags type, which one function was
> > >definining as uintn and the other as an enum lmb_flags and rename
> > >_lmb_alloc_addr_flags() to lmb_alloc_addr_flags()
> > >
> > >Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > >---
> > > include/lmb.h |  2 +-
> > > lib/lmb.c     | 34 ++++++++++++++--------------------
> > > 2 files changed, 15 insertions(+), 21 deletions(-)
> > >
> > >diff --git a/include/lmb.h b/include/lmb.h
> > >index a35a69d69f77..4f3c861b15a6 100644
> > >--- a/include/lmb.h
> > >+++ b/include/lmb.h
> > >@@ -111,7 +111,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);
> > >+                               enum lmb_flags flags);
> >
> > This change implies that flags IS NOT a bitfield. Flags may only hold a SINGLE value from the enum.
> >
> > This looks wrong looking at the definition of the constants via BIT().
> >
> > Defining the constants of a bitfield in an enum was wrong (or at best misleading) from the beginning.
>
> Ah yes correct, I'll just leave it to uint and change the function prototype

Hmm thinking about it again. Those flags are never used as bitfield.
They are always used as discrete values. So I think we should keep it
as is

Thanks
/Ilias
>
> Thanks
> /Ilias
> >
> > Best regards
> >
> > Heinrich
> >
> > >
> > > /**
> > >  * lmb_is_reserved_flags() - test if address is in reserved region with flag bits set
> > >diff --git a/lib/lmb.c b/lib/lmb.c
> > >index c09f1a1277ad..67bde60b74be 100644
> > >--- a/lib/lmb.c
> > >+++ b/lib/lmb.c
> > >@@ -777,8 +777,20 @@ phys_addr_t lmb_alloc_base_flags(phys_size_t size, ulong align,
> > >       return alloc;
> > > }
> > >
> > >-static phys_addr_t _lmb_alloc_addr(phys_addr_t base, phys_size_t size,
> > >-                                  enum lmb_flags flags)
> > >+/**
> > >+ * lmb_alloc_addr_flags() - Allocate specified memory address with specified attributes
> > >+ * @base: Base Address requested
> > >+ * @size: Size of the region requested
> > >+ * @flags: Memory region attributes to be set
> > >+ *
> > >+ * Allocate a region of memory with the attributes specified through the
> > >+ * parameter. The base parameter is used to specify the base address
> > >+ * of the requested region.
> > >+ *
> > >+ * Return: base address on success, 0 on error
> > >+ */
> > >+phys_addr_t lmb_alloc_addr_flags(phys_addr_t base, phys_size_t size,
> > >+                               enum lmb_flags flags)
> > > {
> > >       long rgn;
> > >       struct lmb_region *lmb_memory = lmb.available_mem.data;
> > >@@ -802,24 +814,6 @@ static phys_addr_t _lmb_alloc_addr(phys_addr_t base, phys_size_t size,
> > >       return 0;
> > > }
> > >
> > >-/**
> > >- * lmb_alloc_addr_flags() - Allocate specified memory address with specified attributes
> > >- * @base: Base Address requested
> > >- * @size: Size of the region requested
> > >- * @flags: Memory region attributes to be set
> > >- *
> > >- * Allocate a region of memory with the attributes specified through the
> > >- * parameter. The base parameter is used to specify the base address
> > >- * of the requested region.
> > >- *
> > >- * 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)
> > >-{
> > >-      return _lmb_alloc_addr(base, size, flags);
> > >-}
> > >-
> > > /* Return number of bytes from a given address that are free */
> > > phys_size_t lmb_get_free_size(phys_addr_t addr)
> > > {
> >