On Wed, 21 Aug 2024 at 10:59, Sughosh Ganu <sughosh.g...@linaro.org> wrote: > > On Wed, 21 Aug 2024 at 07:41, Simon Glass <s...@chromium.org> wrote: > > > > Hi Sughosh, > > > > On Tue, 20 Aug 2024 at 02:17, Sughosh Ganu <sughosh.g...@linaro.org> wrote: > > > > > > On Fri, 16 Aug 2024 at 02:04, Simon Glass <s...@chromium.org> wrote: > > > > > > > > Hi Sughosh, > > > > > > > > On Wed, 14 Aug 2024 at 12:01, 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 V1: > > > > > * Call the lmb_add_memory() from lmb_init() instead of > > > > > lmb_mem_regions_init(). > > > > > > > > > > > > > > > include/lmb.h | 10 ++++++++++ > > > > > lib/lmb.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > > > > > 2 files changed, 52 insertions(+) > > > > > > > > > > > > > Reviewed-by: Simon Glass <s...@chromium.org> > > > > > > > > But this should not be weak. > > > > > > This is being made weak, as there would be lmb_add_memory() > > > definitions added for powerpc and x86 arch's in the EFI part of my > > > patches. Moreover, the lmb_add_memory() function would be called even > > > in the SPL stage when LMB is enabled for that stage. So I am not sure > > > how do we get around this. You can check the relevant branch [1] on my > > > github to check for the specific commits [2][3] that I am referring > > > to. Thanks. > > > > This is really strange. > > > > The e820 is different on each x86 board. I'm not sure we want that in > > the lmb. What is the purpose of that? It is somewhat circular in most > > cases, since U-Boot sets it up itself. Where it comes from coreboot, > > it looks like we are mirroring it in the EFI memory map. I'm not sure > > I understand all this very well. > > Yes, me neither. And I want to keep the behaviour same as before the > patches. You would know that the function efi_add_known_memory() gets > the memory map from a function install_e820_map() which includes > conventional memory, which is the ram memory. And I am basically now > doing this as part of the lmb_add_memory() function instead. Are you > sure that we can do away with this function, and instead use the > ram_base and ram_top values from the global data structure instead? I > believe you have boards which exercise this code? So it will be great > if you can test this if I remove the function for the e820 module. > > > > > For fsl, perhaps copy the #ifdef and handle arch.resv_ram in your code? > > This is for adding ram to the lmb memory map, but yes, I can check by > putting an ifdef in the function. Although the function might look > ugly and hackish. Thanks.
I have taken an alternative approach to this, whereby boards/archs can still define their own memory map addition without using the weak attribute. Please check the relevant branch [1] on my github, and these commits [2][3][4] specifically. Thanks. -sughosh [1] - https://github.com/sughoshg/u-boot/tree/lmb_efi_sep_apis_nrfc_next_noweak_v3 [2] - https://github.com/u-boot/u-boot/commit/0b5dbaf07a3615230b9df06296e40267e838f459 [3] - https://github.com/u-boot/u-boot/commit/916a36dcb5cc66d801067924607fd94d8bad2162 [4] - https://github.com/u-boot/u-boot/commit/9da75a20e9a6af00cfaac71f09e4a1f89f10a804 > > -sughosh > > > > > [..] > > Regards, > > Simon > > > > > > > [1] - > > > https://github.com/sughoshg/u-boot/tree/lmb_efi_sep_apis_nrfc_next_v3 > > > [2] - > > > https://github.com/u-boot/u-boot/commit/077ced7aaa6d495b1b87b324fb1c60658c203ce1 > > > [3] - > > > https://github.com/u-boot/u-boot/commit/d0fa3a89865b796f3bbebffebbe4f7b5b048c140 > > >
