[PATCH v1 2/8] lmb: Move enum lmb_flags to a u32
Ilias Apalodimas
ilias.apalodimas at linaro.org
Fri Dec 13 06:53:43 CET 2024
Hi Sam,
On Thu, 12 Dec 2024 at 23:58, 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 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 at 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:
Sure, the CI didn't complain so I assumed it was fine. I'll change this in v2
>
> Reviewed-by: Sam Protsenko <semen.protsenko at linaro.org>
Thanks
/Ilias
>
> [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
> >
More information about the U-Boot
mailing list