Re: [PATCH v1 2/8] lmb: Move enum lmb_flags to a u32

On Wed, Dec 11, 2024 at 4:55 AM Ilias Apalodimas ilias.apalodimas@linaro.org wrote:

LMB flags is not an enum anymore. It's currently used as a bitmask in various places of our code. So make it a u32 which is more appropriate when dealing with masks.

Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org

boot/image-fdt.c | 4 ++-- include/lmb.h | 25 +++++++++++-------------- lib/lmb.c | 18 +++++++++--------- test/cmd/bdinfo.c | 2 +- 4 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/boot/image-fdt.c b/boot/image-fdt.c index 73c43c30684f..cda7c3aa9e38 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -68,7 +68,7 @@ static const struct legacy_img_hdr *image_get_fdt(ulong fdt_addr) } #endif

-static void boot_fdt_reserve_region(u64 addr, u64 size, enum lmb_flags flags) +static void boot_fdt_reserve_region(u64 addr, u64 size, u32 flags) { long ret;

@@ -100,7 +100,7 @@ void boot_fdt_add_mem_rsv_regions(void *fdt_blob) int i, total, ret; int nodeoffset, subnode; struct fdt_resource res;

•   enum lmb_flags flags;
  

•   u32 flags;
  
    if (fdt_check_header(fdt_blob) != 0)
            return;
  

diff --git a/include/lmb.h b/include/lmb.h index 03d5fac6aa79..7db9e0b62840 100644 --- a/include/lmb.h +++ b/include/lmb.h @@ -19,18 +19,15 @@ #define LMB_ALIST_INITIAL_SIZE 4

/**

• • enum lmb_flags - Definition of memory region attributes
• • @LMB_NONE: No special request
• • @LMB_NOMAP: Don't add to MMU configuration
• • @LMB_NOOVERWRITE: The memory region cannot be overwritten/re-reserved
• • @LMB_NONOTIFY: Do not notify other modules of changes to this memory region
• */

-enum lmb_flags {

•   LMB_NONE                = 0,
  

•   LMB_NOMAP               = BIT(1),
  

•   LMB_NOOVERWRITE         = BIT(2),
  

•   LMB_NONOTIFY            = BIT(3),
  

-};

• • LMB_NONE: No special request
• • LMB_NOMAP: Don't add to MMU configuration
• • LMB_NOOVERWRITE: The memory region cannot be overwritten/re-reserved
• • LMB_NONOTIFY: Do not notify other modules of changes to this memory region
• */

This causes a kernel-doc warning:

$ scripts/kernel-doc -v -none include/lmb.h

include/lmb.h:22: warning: missing initial short description on line: * LMB_NONE: No special request

Suggest using DOC stanza in this kernel-doc comment, as described at [1], like this:

/** * DOC: Memory region attribute flags. * * %LMB_NONE: No special request * %LMB_NOMAP: Don't add to MMU configuration * %LMB_NOOVERWRITE: The memory region cannot be overwritten/re-reserved * %LMB_NONOTIFY: Do not notify other modules of changes to this memory region */

That's helpful if you want to keep Sphinx generated doc [2] for LMB subsystem tidy.

Other than that, LGTM:

Reviewed-by: Sam Protsenko semen.protsenko@linaro.org

[1] https://docs.kernel.org/doc-guide/kernel-doc.html [2] https://docs.u-boot.org/en/latest/api/lmb.html

*/

+#define LMB_NONE 0 +#define LMB_NOMAP BIT(0) +#define LMB_NOOVERWRITE BIT(1) +#define LMB_NONOTIFY BIT(2)

/**

• struct lmb_region - Description of one region

@@ -41,7 +38,7 @@ enum lmb_flags { struct lmb_region { phys_addr_t base; phys_size_t size;

•   enum lmb_flags flags;
  

•   u32 flags;
  

};

/** @@ -101,7 +98,7 @@ long lmb_reserve(phys_addr_t base, phys_size_t size);

• • %-1 - Failure

*/ long lmb_reserve_flags(phys_addr_t base, phys_size_t size,

•                  enum lmb_flags flags);
  

•                  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); diff --git a/lib/lmb.c b/lib/lmb.c index b9c26cb02e10..edecdb8e9cbf 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -188,7 +188,7 @@ static long lmb_resize_regions(struct alist *lmb_rgn_lst,

• • %-1 - Failure

*/ static long lmb_add_region_flags(struct alist *lmb_rgn_lst, phys_addr_t base,

•                            phys_size_t size, enum lmb_flags flags)
  

•                            phys_size_t size, u32 flags)
  

{ unsigned long coalesced = 0; long ret, i; @@ -201,7 +201,7 @@ static long lmb_add_region_flags(struct alist *lmb_rgn_lst, phys_addr_t base, for (i = 0; i < lmb_rgn_lst->count; i++) { phys_addr_t rgnbase = rgn[i].base; phys_size_t rgnsize = rgn[i].size;

•           enum lmb_flags rgnflags = rgn[i].flags;
  

•           u32 rgnflags = rgn[i].flags;
  
            ret = lmb_addrs_adjacent(base, size, rgnbase, rgnsize);
            if (ret > 0) {
  

@@ -430,14 +430,14 @@ long io_lmb_free(struct lmb *io_lmb, phys_addr_t base, phys_size_t size)

static struct lmb lmb;

-static bool lmb_should_notify(enum lmb_flags flags) +static bool lmb_should_notify(u32 flags) { return !lmb.test && !(flags & LMB_NONOTIFY) && CONFIG_IS_ENABLED(EFI_LOADER); }

static int lmb_map_update_notify(phys_addr_t addr, phys_size_t size, u8 op,

•                            enum lmb_flags flags)
  

•                            u32 flags)
  

{ u64 efi_addr; u64 pages; @@ -470,7 +470,7 @@ static int lmb_map_update_notify(phys_addr_t addr, phys_size_t size, u8 op, return 0; }

-static void lmb_print_region_flags(enum lmb_flags flags) +static void lmb_print_region_flags(u32 flags) { const char * const flag_str[] = { "none", "no-map", "no-overwrite", "no-notify" }; @@ -495,7 +495,7 @@ static void lmb_dump_region(struct alist *lmb_rgn_lst, char *name) { struct lmb_region *rgn = lmb_rgn_lst->data; unsigned long long base, size, end;

•   enum lmb_flags flags;
  

•   u32 flags;
    int i;
  
    printf(" %s.count = %#x\n", name, lmb_rgn_lst->count);
  

@@ -669,7 +669,7 @@ long lmb_free(phys_addr_t base, phys_size_t size) return lmb_free_flags(base, size, LMB_NONE); }

-long lmb_reserve_flags(phys_addr_t base, phys_size_t size, enum lmb_flags flags) +long lmb_reserve_flags(phys_addr_t base, phys_size_t size, u32 flags) { long ret = 0; struct alist *lmb_rgn_lst = &lmb.used_mem; @@ -687,7 +687,7 @@ long lmb_reserve(phys_addr_t base, phys_size_t size) }

static phys_addr_t _lmb_alloc_base(phys_size_t size, ulong align,

•                              phys_addr_t max_addr, enum lmb_flags flags)
  

•                              phys_addr_t max_addr, u32 flags)
  

{ int ret; long i, rgn; @@ -774,7 +774,7 @@ phys_addr_t lmb_alloc_base_flags(phys_size_t size, ulong align, }

static phys_addr_t _lmb_alloc_addr(phys_addr_t base, phys_size_t size,

•                              enum lmb_flags flags)
  

•                              u32 flags)
  

{ long rgn; struct lmb_region *lmb_memory = lmb.free_mem.data; diff --git a/test/cmd/bdinfo.c b/test/cmd/bdinfo.c index bb419ab2394e..014391b38ac0 100644 --- a/test/cmd/bdinfo.c +++ b/test/cmd/bdinfo.c @@ -104,7 +104,7 @@ static int lmb_test_dump_region(struct unit_test_state *uts, { struct lmb_region *rgn = lmb_rgn_lst->data; unsigned long long base, size, end;

•   enum lmb_flags flags;
  

•   u32 flags;
    int i;
  
    ut_assert_nextline(" %s.count = %#x", name, lmb_rgn_lst->count);
  

-- 2.45.2