| Message ID | 20240724060224.3071065-18-sughosh.ganu@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Make LMB memory map global and persistent | expand |
Hi Sughosh, On Wed, 24 Jul 2024 at 00:04, Sughosh Ganu <sughosh.ganu@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.ganu@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? I'm really not a fan of weak functions...it often means we should have a proper API for it. > + * > + * 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
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: > > > > 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.ganu@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
Hi Sughosh, On Mon, 29 Jul 2024 at 02:00, 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: > > > > > > 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.ganu@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
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. + * + * 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; }
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.ganu@linaro.org> --- Changes since rfc: None include/lmb.h | 12 ++++++++++++ lib/lmb.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+)