[22/40] lmb: init: initialise the lmb data structures during board init

The memory map maintained by the LMB module is now persistent and
global. This memory map is being maintained through the alloced list
structure which can be extended at runtime -- there is one list for
the available memory, and one for the used memory. Allocate and
initialise these lists during the board init.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
---
Changes since rfc: None

 common/board_r.c |  4 ++++
 common/spl/spl.c |  3 +++
 include/lmb.h    | 11 +++++++++++
 lib/lmb.c        | 20 ++++++++++++++++++++
 4 files changed, 38 insertions(+)

Hi Sughosh,

On Wed, 24 Jul 2024 at 00:04, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> The memory map maintained by the LMB module is now persistent and
> global. This memory map is being maintained through the alloced list
> structure which can be extended at runtime -- there is one list for
> the available memory, and one for the used memory. Allocate and
> initialise these lists during the board init.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
> Changes since rfc: None
>
>  common/board_r.c |  4 ++++
>  common/spl/spl.c |  3 +++
>  include/lmb.h    | 11 +++++++++++
>  lib/lmb.c        | 20 ++++++++++++++++++++
>  4 files changed, 38 insertions(+)
>
> diff --git a/common/board_r.c b/common/board_r.c
> index d4ba245ac6..eaf9b40ec0 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -22,6 +22,7 @@
>  #include <hang.h>
>  #include <image.h>
>  #include <irq_func.h>
> +#include <lmb.h>
>  #include <log.h>
>  #include <net.h>
>  #include <asm/cache.h>
> @@ -610,6 +611,9 @@ static init_fnc_t init_sequence_r[] = {
>  #ifdef CONFIG_CLOCKS
>         set_cpu_clk_info, /* Setup clock information */
>  #endif
> +#if CONFIG_IS_ENABLED(LMB)
> +       initr_lmb,
> +#endif

Can you put the if() in the function, to avoid #ifdefs here?

>  #ifdef CONFIG_EFI_LOADER
>         efi_memory_init,
>  #endif
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index 7794ddccad..38ac0608bb 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -723,6 +723,9 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
>             IS_ENABLED(CONFIG_SPL_ATF))
>                 dram_init_banksize();
>
> +       if (IS_ENABLED(CONFIG_SPL_LMB))
> +               initr_lmb();
> +

Still unsure why this is needed. I'm also not sure how it works, since
SPL allocations will not transfer to U-Boot proper.

>         if (CONFIG_IS_ENABLED(PCI) && !(gd->flags & GD_FLG_DM_DEAD)) {
>                 ret = pci_init();
>                 if (ret)
> diff --git a/include/lmb.h b/include/lmb.h
> index 2d8f9a6b71..cc4cf9f3c8 100644
> --- a/include/lmb.h
> +++ b/include/lmb.h
> @@ -37,6 +37,17 @@ struct lmb_region {
>         enum lmb_flags flags;
>  };
>
> +/**
> + * initr_lmb() - Initialise the LMB lists
> + *
> + * Initialise the LMB lists needed for keeping the memory map. There
> + * are two lists, in form of alloced list data structure. One for the
> + * available memory, and one for the used memory.
> + *
> + * Return: 0 on success, -ve on error
> + */
> +int initr_lmb(void);
> +
>  /**
>   * lmb_add_memory() - Add memory range for LMB allocations
>   *
> diff --git a/lib/lmb.c b/lib/lmb.c
> index 5afdfc32fb..5baa5c4c52 100644
> --- a/lib/lmb.c
> +++ b/lib/lmb.c
> @@ -783,3 +783,23 @@ int lmb_mem_regions_init(void)
>
>         return 0;
>  }
> +
> +/**
> + * initr_lmb() - Initialise the LMB lists
> + *
> + * Initialise the LMB lists needed for keeping the memory map. There
> + * are two lists, in form of alloced list data structure. One for the
> + * available memory, and one for the used memory.
> + *
> + * Return: 0 on success, -ve on error
> + */
> +int initr_lmb(void)
> +{
> +       int ret;
> +
> +       ret = lmb_mem_regions_init();
> +       if (ret)
> +               printf("Unable to initialise the LMB data structures\n");
> +
> +       return ret;
> +}
> --
> 2.34.1
>

Regards,
Simon

On Fri, 26 Jul 2024 at 05:02, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Sughosh,
>
> On Wed, 24 Jul 2024 at 00:04, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> >
> > The memory map maintained by the LMB module is now persistent and
> > global. This memory map is being maintained through the alloced list
> > structure which can be extended at runtime -- there is one list for
> > the available memory, and one for the used memory. Allocate and
> > initialise these lists during the board init.
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > ---
> > Changes since rfc: None
> >
> >  common/board_r.c |  4 ++++
> >  common/spl/spl.c |  3 +++
> >  include/lmb.h    | 11 +++++++++++
> >  lib/lmb.c        | 20 ++++++++++++++++++++
> >  4 files changed, 38 insertions(+)
> >
> > diff --git a/common/board_r.c b/common/board_r.c
> > index d4ba245ac6..eaf9b40ec0 100644
> > --- a/common/board_r.c
> > +++ b/common/board_r.c
> > @@ -22,6 +22,7 @@
> >  #include <hang.h>
> >  #include <image.h>
> >  #include <irq_func.h>
> > +#include <lmb.h>
> >  #include <log.h>
> >  #include <net.h>
> >  #include <asm/cache.h>
> > @@ -610,6 +611,9 @@ static init_fnc_t init_sequence_r[] = {
> >  #ifdef CONFIG_CLOCKS
> >         set_cpu_clk_info, /* Setup clock information */
> >  #endif
> > +#if CONFIG_IS_ENABLED(LMB)
> > +       initr_lmb,
> > +#endif
>
> Can you put the if() in the function, to avoid #ifdefs here?

Will do

>
> >  #ifdef CONFIG_EFI_LOADER
> >         efi_memory_init,
> >  #endif
> > diff --git a/common/spl/spl.c b/common/spl/spl.c
> > index 7794ddccad..38ac0608bb 100644
> > --- a/common/spl/spl.c
> > +++ b/common/spl/spl.c
> > @@ -723,6 +723,9 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
> >             IS_ENABLED(CONFIG_SPL_ATF))
> >                 dram_init_banksize();
> >
> > +       if (IS_ENABLED(CONFIG_SPL_LMB))
> > +               initr_lmb();
> > +
>
> Still unsure why this is needed. I'm also not sure how it works, since
> SPL allocations will not transfer to U-Boot proper.

I am not sure if I get your question right, but we initialise the LMB
lists, and add to it's available memory map, both in SPL and in U-Boot
proper. These are being initialised for SPL separately once the ram
memory has been set up, including the dram banksize values.

-sughosh

>
> >         if (CONFIG_IS_ENABLED(PCI) && !(gd->flags & GD_FLG_DM_DEAD)) {
> >                 ret = pci_init();
> >                 if (ret)
> > diff --git a/include/lmb.h b/include/lmb.h
> > index 2d8f9a6b71..cc4cf9f3c8 100644
> > --- a/include/lmb.h
> > +++ b/include/lmb.h
> > @@ -37,6 +37,17 @@ struct lmb_region {
> >         enum lmb_flags flags;
> >  };
> >
> > +/**
> > + * initr_lmb() - Initialise the LMB lists
> > + *
> > + * Initialise the LMB lists needed for keeping the memory map. There
> > + * are two lists, in form of alloced list data structure. One for the
> > + * available memory, and one for the used memory.
> > + *
> > + * Return: 0 on success, -ve on error
> > + */
> > +int initr_lmb(void);
> > +
> >  /**
> >   * lmb_add_memory() - Add memory range for LMB allocations
> >   *
> > diff --git a/lib/lmb.c b/lib/lmb.c
> > index 5afdfc32fb..5baa5c4c52 100644
> > --- a/lib/lmb.c
> > +++ b/lib/lmb.c
> > @@ -783,3 +783,23 @@ int lmb_mem_regions_init(void)
> >
> >         return 0;
> >  }
> > +
> > +/**
> > + * initr_lmb() - Initialise the LMB lists
> > + *
> > + * Initialise the LMB lists needed for keeping the memory map. There
> > + * are two lists, in form of alloced list data structure. One for the
> > + * available memory, and one for the used memory.
> > + *
> > + * Return: 0 on success, -ve on error
> > + */
> > +int initr_lmb(void)
> > +{
> > +       int ret;
> > +
> > +       ret = lmb_mem_regions_init();
> > +       if (ret)
> > +               printf("Unable to initialise the LMB data structures\n");
> > +
> > +       return ret;
> > +}
> > --
> > 2.34.1
> >
>
> Regards,
> Simon

Hi Sughosh,

On Mon, 29 Jul 2024 at 02:36, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
>
> On Fri, 26 Jul 2024 at 05:02, Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Wed, 24 Jul 2024 at 00:04, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > >
> > > The memory map maintained by the LMB module is now persistent and
> > > global. This memory map is being maintained through the alloced list
> > > structure which can be extended at runtime -- there is one list for
> > > the available memory, and one for the used memory. Allocate and
> > > initialise these lists during the board init.
> > >
> > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > ---
> > > Changes since rfc: None
> > >
> > >  common/board_r.c |  4 ++++
> > >  common/spl/spl.c |  3 +++
> > >  include/lmb.h    | 11 +++++++++++
> > >  lib/lmb.c        | 20 ++++++++++++++++++++
> > >  4 files changed, 38 insertions(+)
> > >
> > > diff --git a/common/board_r.c b/common/board_r.c
> > > index d4ba245ac6..eaf9b40ec0 100644
> > > --- a/common/board_r.c
> > > +++ b/common/board_r.c
> > > @@ -22,6 +22,7 @@
> > >  #include <hang.h>
> > >  #include <image.h>
> > >  #include <irq_func.h>
> > > +#include <lmb.h>
> > >  #include <log.h>
> > >  #include <net.h>
> > >  #include <asm/cache.h>
> > > @@ -610,6 +611,9 @@ static init_fnc_t init_sequence_r[] = {
> > >  #ifdef CONFIG_CLOCKS
> > >         set_cpu_clk_info, /* Setup clock information */
> > >  #endif
> > > +#if CONFIG_IS_ENABLED(LMB)
> > > +       initr_lmb,
> > > +#endif
> >
> > Can you put the if() in the function, to avoid #ifdefs here?
>
> Will do
>
> >
> > >  #ifdef CONFIG_EFI_LOADER
> > >         efi_memory_init,
> > >  #endif
> > > diff --git a/common/spl/spl.c b/common/spl/spl.c
> > > index 7794ddccad..38ac0608bb 100644
> > > --- a/common/spl/spl.c
> > > +++ b/common/spl/spl.c
> > > @@ -723,6 +723,9 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
> > >             IS_ENABLED(CONFIG_SPL_ATF))
> > >                 dram_init_banksize();
> > >
> > > +       if (IS_ENABLED(CONFIG_SPL_LMB))
> > > +               initr_lmb();
> > > +
> >
> > Still unsure why this is needed. I'm also not sure how it works, since
> > SPL allocations will not transfer to U-Boot proper.
>
> I am not sure if I get your question right, but we initialise the LMB
> lists, and add to it's available memory map, both in SPL and in U-Boot
> proper. These are being initialised for SPL separately once the ram
> memory has been set up, including the dram banksize values.

We can worry about it later, but we don't have a mechanism to transfer
the LMB from SPL to U-Boot proper, so any SPL allocations will go
away. I suspect that is fine for now.

Regards,
Simon