Re: [RFC PATCH v2 16/48] lmb: config: add lmb config symbols for SPL

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2582 bytes --]

On Mon, Jul 08, 2024 at 05:06:45PM +0530, Sughosh Ganu wrote:
> On Sat, 6 Jul 2024 at 01:18, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Jul 04, 2024 at 01:05:12PM +0530, Sughosh Ganu wrote:
> >
> > > Add separate config symbols for enabling the LMB module for the SPL
> > > phase. The LMB module implementation now relies on alloced list data
> > > structure which requires heap area to be present. Add specific config
> > > symbol for the SPL phase of U-Boot so that this can be enabled on
> > > platforms which support a heap in SPL.
> > >
> > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > ---
> > > Changes since V1: New patch
> > >
> > >  lib/Kconfig | 12 +++++++++++-
> > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/Kconfig b/lib/Kconfig
> > > index 072ed0ecfa..7eea517b3b 100644
> > > --- a/lib/Kconfig
> > > +++ b/lib/Kconfig
> > > @@ -1103,7 +1103,17 @@ config LMB
> > >       default y if ARC || ARM || M68K || MICROBLAZE || MIPS || \
> > >                    NIOS2 || PPC || RISCV || SANDBOX || SH || X86 || XTENSA
> > >       help
> > > -       Support the library logical memory blocks.
> > > +       Support the library logical memory blocks. This will require
> > > +       a malloc() implementation for defining the data structures
> > > +       needed for maintaining the LMB memory map.
> >
> > Even today, LMB really should be def_bool y rather than an option, so
> > this series should correct that. That said...
> 
> Okay
> 
> >
> > > +config SPL_LMB
> > > +     bool "Enable LMB module for SPL"
> > > +     depends on SPL && SPL_FRAMEWORK && SPL_SYS_MALLOC
> > > +     help
> > > +       Enable support for Logical Memory Block library routines in
> > > +       SPL. This will require a malloc() implementation for defining
> > > +       the data structures needed for maintaining the LMB memory map.
> >
> > The question I guess becomes when do we need LMB in SPL, exactly? And I
> > guess it's another case where it should be def_bool y (but still depends
> > on what you have here) since we need to make sure we don't overwrite
> > running SPL.
> 
> So this is a question even I had. Do we really need to enable LMB in
> SPL ? The main reason for introducing the symbol was to have more
> granularity to remove the LMB code from SPL, but should this really be
> enabled in SPL is something that I am not too sure about.

Yes, we need to ensure we obey reservations in SPL, both for U-Boot and
for when we boot the OS from SPL.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]