On Fri, 26 Jul 2024 at 05:02, Simon Glass <s...@chromium.org> wrote:
>
> Hi Sughosh,
>
> On Wed, 24 Jul 2024 at 00:04, Sughosh Ganu <sughosh.g...@linaro.org> wrote:
> >
> > Introduce a function lmb_add_memory() to add available memory to the
> > LMB memory map. Call this function during board init once the LMB data
> > structures have been initialised.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.g...@linaro.org>
> > ---
> > Changes since rfc: None
> >
> > include/lmb.h | 12 ++++++++++++
> > lib/lmb.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 54 insertions(+)
> >
> > diff --git a/include/lmb.h b/include/lmb.h
> > index a308796d58..77e2a23c0d 100644
> > --- a/include/lmb.h
> > +++ b/include/lmb.h
> > @@ -40,6 +40,18 @@ struct lmb_region {
> > void lmb_init_and_reserve(struct bd_info *bd, void *fdt_blob);
> > void lmb_init_and_reserve_range(phys_addr_t base, phys_size_t size,
> > void *fdt_blob);
> > +
> > +/**
> > + * lmb_add_memory() - Add memory range for LMB allocations
> > + *
> > + * Add the entire available memory range to the pool of memory that
> > + * can be used by the LMB module for allocations.
> > + *
> > + * Return: None
> > + *
> > + */
> > +void lmb_add_memory(void);
> > +
> > long lmb_add(phys_addr_t base, phys_size_t size);
> > long lmb_reserve(phys_addr_t base, phys_size_t size);
> > /**
> > diff --git a/lib/lmb.c b/lib/lmb.c
> > index 88352e9a25..db0874371a 100644
> > --- a/lib/lmb.c
> > +++ b/lib/lmb.c
> > @@ -239,6 +239,46 @@ void lmb_init_and_reserve(struct bd_info *bd, void
> > *fdt_blob)
> > lmb_reserve_common(fdt_blob);
> > }
> >
> > +/**
> > + * lmb_add_memory() - Add memory range for LMB allocations
> > + *
> > + * Add the entire available memory range to the pool of memory that
> > + * can be used by the LMB module for allocations.
> > + *
> > + * This can be overridden for specific boards/architectures.
>
> Why is this needed?
There is a definition of this function for the freescale layerscape
platforms, and the e820 platform.
> I'm really not a fan of weak functions...it often
> means we should have a proper API for it.
Is there some coding style guideline which prohibits using weak
function definitions? I see it being used all across U-Boot. Moreover,
I don't think that this particular case asks for adding an API for
this one function. That would just add to the size without much
benefit.
-sughosh
>
> > + *
> > + * Return: None
> > + *
> > + */
> > +__weak void lmb_add_memory(void)
> > +{
> > + int i;
> > + phys_size_t size;
> > + phys_addr_t rgn_top;
> > + u64 ram_top = gd->ram_top;
> > + struct bd_info *bd = gd->bd;
> > +
> > + /* Assume a 4GB ram_top if not defined */
> > + if (!ram_top)
> > + ram_top = 0x100000000ULL;
> > +
> > + for (i = 0; i < CONFIG_NR_DRAM_BANKS; i++) {
> > + size = bd->bi_dram[i].size;
> > + if (size) {
> > + if (bd->bi_dram[i].start > ram_top)
> > + continue;
> > +
> > + rgn_top = bd->bi_dram[i].start +
> > + bd->bi_dram[i].size;
> > +
> > + if (rgn_top > ram_top)
> > + size -= rgn_top - ram_top;
> > +
> > + lmb_add(bd->bi_dram[i].start, size);
> > + }
> > + }
> > +}
> > +
> > /* Initialize the struct, add memory and call arch/board reserve functions
> > */
> > void lmb_init_and_reserve_range(phys_addr_t base, phys_size_t size,
> > void *fdt_blob)
> > @@ -728,5 +768,7 @@ int lmb_mem_regions_init(void)
> > return -1;
> > }
> >
> > + lmb_add_memory();
> > +
> > return 0;
> > }
> > --
> > 2.34.1
> >
>
> Regards,
> Simon
