Hi Sughosh,
On Mon, 29 Jul 2024 at 02:00, Sughosh Ganu <sughosh.g...@linaro.org> wrote:
>
> 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.
Can you please point me to that? I cannot see it. I am looking for
lmb_add_memory().
>
> > 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.
In fact, before driver model, almost everything was a weak function.
I am not a fan of them in general...it can be a useful get-of-jail
card, but we should not base things on it. It is hard to figure out
what is actually being called...sometimes I have to resort to
disassembly to work it out. Normally there is a better way.
Events were created to reduce the need for weak functions. The
overhead is small, so long as events are already enabled.
> 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.
That's what everyone says :-) Good design is very important,
particularly the project is under rapid development.
Regards,
Simon
