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

Ilias Apalodimas ilias.apalodimas at linaro.org
Sun Dec 8 13:12:03 CET 2024


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

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)
> > {
>


More information about the U-Boot mailing list