On Fri, 13 Dec 2024 at 00:34, Sam Protsenko <[email protected]> wrote: > > On Wed, Dec 11, 2024 at 4:55 AM Ilias Apalodimas > <[email protected]> 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 <[email protected]> > > --- > > 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 <[email protected]> Thanks! /Ilias > > [snip]
