Re: [PATCH v2 3/4] fdtdec: Introduce fdtdec_setup_ram_size() to determine the memory size
On Wed, 1 Apr 2026 at 18:40, Marek Vasut wrote: > > On 4/1/26 5:29 PM, Ilias Apalodimas wrote: > > Hi Marek, > > > > On Wed, 1 Apr 2026 at 18:24, Marek Vasut wrote: > >> > >> On 4/1/26 8:43 AM, Ilias Apalodimas wrote: > >> > > +++ b/include/fdtdec.h > > @@ -965,6 +965,21 @@ int fdtdec_setup_mem_ram_top(void); > > * invalid > > */ > > int fdtdec_setup_memory_banksize(void); > > +/** > > + * fdtdec_setup_ram_size() - decode and setup setup gd->ram_size to > > the sum of > > + *all banks > > + * > > + * Decode the /memory 'reg' property to determine the memory available > > in all > > + * banks and populate the global data ram_size with it. > > + * > > + * This function should be called from a boards > > board_get_usable_ram_top(). > > Is the board_get_usable_ram_top() really the right place to call this ? > It feels like it is being conflated with ram_top here too. > >>> > >>> I don't think so. You are effectively calculating the top address you > >>> should relocate to. You should also calculate the difference in > >>> ramsize. Those two should not be separated actions > >> The address where you relocate to is gd->relocaddr, which is ram_top > >> minus all the reserved areas, see common/board_f.c setup_dest_addr(). > >> > >> This setup_dest_addr() function does manipulate with gd->ram_size , but > >> only to handle CONFIG_SYS_MEM_TOP_HIDE , which may not be the right > >> place for it, I think it would be better to have a separate initcall for > >> the ram_size calculation, which precedes setup_dest_addr(). > > > > Yes, but when you calculate the reloc address and use a bigger > > ram_top, the right thing to do is adjust the size as well > > That is the root of my concern, ram_top and ram_size are always the > same, relocaddr is not. Yes, relocaddr is basically ram_top. But I think we agree here. Changning ram_top without ram_size makes little sense. > > >> With that, the function description can claim that each function must be > >> called before that specific initcall, fdtdec_setup_mem_ram_top() before > >> setup_dest_addr() and fdtdec_setup_ram_size() before > >> setup_whatever_is_the_new_initcall_name(). > > > > I really don't mind. I just don't see the point of making things so > > granular. Unless I am wrong and someone thinks it makes sense to > > change ram_top without taking into account the size. > Maybe it would make sense to decouple relocaddr from ram_top/ram_size > calculation ? Where? In setup_dest_addr()? Yea that can be moved out. But please have a look at the rest of the thread. TL;DR Staring at the code more we can make things a lot simpler. We can move dram_init_banksize() higher up in the INITCALL chain and calculate the size and ram_top from that. Then deciding where to relocate becomes trivial. The only problem is that the dram bank info is stored in bd, not gd (and bd is allocated later). I don't remember why that ended up in bd, but if can move that information in gd, the code will look a lot simpler. What I suggest is apply patches #1 & #2 now. I'll play around with the idea above tomorrow and if I hit a brick wall I can resend #3 & #4 separately.
Re: [PATCH v2 3/4] fdtdec: Introduce fdtdec_setup_ram_size() to determine the memory size
On 4/1/26 5:29 PM, Ilias Apalodimas wrote: Hi Marek, On Wed, 1 Apr 2026 at 18:24, Marek Vasut wrote: On 4/1/26 8:43 AM, Ilias Apalodimas wrote: +++ b/include/fdtdec.h @@ -965,6 +965,21 @@ int fdtdec_setup_mem_ram_top(void); * invalid */ int fdtdec_setup_memory_banksize(void); +/** + * fdtdec_setup_ram_size() - decode and setup setup gd->ram_size to the sum of + *all banks + * + * Decode the /memory 'reg' property to determine the memory available in all + * banks and populate the global data ram_size with it. + * + * This function should be called from a boards board_get_usable_ram_top(). Is the board_get_usable_ram_top() really the right place to call this ? It feels like it is being conflated with ram_top here too. I don't think so. You are effectively calculating the top address you should relocate to. You should also calculate the difference in ramsize. Those two should not be separated actions The address where you relocate to is gd->relocaddr, which is ram_top minus all the reserved areas, see common/board_f.c setup_dest_addr(). This setup_dest_addr() function does manipulate with gd->ram_size , but only to handle CONFIG_SYS_MEM_TOP_HIDE , which may not be the right place for it, I think it would be better to have a separate initcall for the ram_size calculation, which precedes setup_dest_addr(). Yes, but when you calculate the reloc address and use a bigger ram_top, the right thing to do is adjust the size as well That is the root of my concern, ram_top and ram_size are always the same, relocaddr is not. With that, the function description can claim that each function must be called before that specific initcall, fdtdec_setup_mem_ram_top() before setup_dest_addr() and fdtdec_setup_ram_size() before setup_whatever_is_the_new_initcall_name(). I really don't mind. I just don't see the point of making things so granular. Unless I am wrong and someone thinks it makes sense to change ram_top without taking into account the size. Maybe it would make sense to decouple relocaddr from ram_top/ram_size calculation ?
Re: [PATCH v2 3/4] fdtdec: Introduce fdtdec_setup_ram_size() to determine the memory size
On 4/1/26 8:43 AM, Ilias Apalodimas wrote: +++ b/include/fdtdec.h @@ -965,6 +965,21 @@ int fdtdec_setup_mem_ram_top(void); * invalid */ int fdtdec_setup_memory_banksize(void); +/** + * fdtdec_setup_ram_size() - decode and setup setup gd->ram_size to the sum of + *all banks + * + * Decode the /memory 'reg' property to determine the memory available in all + * banks and populate the global data ram_size with it. + * + * This function should be called from a boards board_get_usable_ram_top(). Is the board_get_usable_ram_top() really the right place to call this ? It feels like it is being conflated with ram_top here too. I don't think so. You are effectively calculating the top address you should relocate to. You should also calculate the difference in ramsize. Those two should not be separated actions The address where you relocate to is gd->relocaddr, which is ram_top minus all the reserved areas, see common/board_f.c setup_dest_addr(). This setup_dest_addr() function does manipulate with gd->ram_size , but only to handle CONFIG_SYS_MEM_TOP_HIDE , which may not be the right place for it, I think it would be better to have a separate initcall for the ram_size calculation, which precedes setup_dest_addr(). With that, the function description can claim that each function must be called before that specific initcall, fdtdec_setup_mem_ram_top() before setup_dest_addr() and fdtdec_setup_ram_size() before setup_whatever_is_the_new_initcall_name().
Re: [PATCH v2 3/4] fdtdec: Introduce fdtdec_setup_ram_size() to determine the memory size
Hi Marek, On Wed, 1 Apr 2026 at 18:24, Marek Vasut wrote: > > On 4/1/26 8:43 AM, Ilias Apalodimas wrote: > > >>> +++ b/include/fdtdec.h > >>> @@ -965,6 +965,21 @@ int fdtdec_setup_mem_ram_top(void); > >>> * invalid > >>> */ > >>>int fdtdec_setup_memory_banksize(void); > >>> +/** > >>> + * fdtdec_setup_ram_size() - decode and setup setup gd->ram_size to the > >>> sum of > >>> + *all banks > >>> + * > >>> + * Decode the /memory 'reg' property to determine the memory available > >>> in all > >>> + * banks and populate the global data ram_size with it. > >>> + * > >>> + * This function should be called from a boards > >>> board_get_usable_ram_top(). > >> > >> Is the board_get_usable_ram_top() really the right place to call this ? > >> It feels like it is being conflated with ram_top here too. > > > > I don't think so. You are effectively calculating the top address you > > should relocate to. You should also calculate the difference in > > ramsize. Those two should not be separated actions > The address where you relocate to is gd->relocaddr, which is ram_top > minus all the reserved areas, see common/board_f.c setup_dest_addr(). > > This setup_dest_addr() function does manipulate with gd->ram_size , but > only to handle CONFIG_SYS_MEM_TOP_HIDE , which may not be the right > place for it, I think it would be better to have a separate initcall for > the ram_size calculation, which precedes setup_dest_addr(). Yes, but when you calculate the reloc address and use a bigger ram_top, the right thing to do is adjust the size as well > > With that, the function description can claim that each function must be > called before that specific initcall, fdtdec_setup_mem_ram_top() before > setup_dest_addr() and fdtdec_setup_ram_size() before > setup_whatever_is_the_new_initcall_name(). I really don't mind. I just don't see the point of making things so granular. Unless I am wrong and someone thinks it makes sense to change ram_top without taking into account the size. Thanks /Ilias
Re: [PATCH v2 3/4] fdtdec: Introduce fdtdec_setup_ram_size() to determine the memory size
On Tue, 31 Mar 2026 11:01:13 +0300, Ilias Apalodimas wrote: > We previously added a fdtdec_setup_mem_ram_top() to determine the > topmost address of the last DRAM bank. Similarly the overall memory > size must be calculated when U-Boot gets relocated to the actual end of > all available DRAM. > > Pre-patch: > U-Boot 2026.04-rc5-00012-g5789f787c8a6-dirty (Mar 31 2026 - 09:13:34 +0300) > DRAM: 4 GiB (total 8 GiB) > > [...] Reviewed-by: Anshul Dalal -- Anshul Dalal
Re: [PATCH v2 3/4] fdtdec: Introduce fdtdec_setup_ram_size() to determine the memory size
On Wed, 1 Apr 2026 at 01:01, Marek Vasut wrote: > > On 3/31/26 10:01 AM, Ilias Apalodimas wrote: > > We previously added a fdtdec_setup_mem_ram_top() to determine the > > topmost address of the last DRAM bank. Similarly the overall memory > > size must be calculated when U-Boot gets relocated to the actual end of > > all available DRAM. > > > > Pre-patch: > > U-Boot 2026.04-rc5-00012-g5789f787c8a6-dirty (Mar 31 2026 - 09:13:34 +0300) > > DRAM: 4 GiB (total 8 GiB) > > > > Post-patch: > > U-Boot 2026.04-rc5-00012-g5789f787c8a6 (Mar 31 2026 - 09:12:59 +0300) > > DRAM: 8 GiB > > > > Signed-off-by: Ilias Apalodimas > > --- > > include/fdtdec.h | 15 +++ > > lib/fdtdec.c | 18 ++ > > 2 files changed, 33 insertions(+) > > > > diff --git a/include/fdtdec.h b/include/fdtdec.h > > index 26e40c3cdb46..9bccd70bbdb6 100644 > > --- a/include/fdtdec.h > > +++ b/include/fdtdec.h > > @@ -965,6 +965,21 @@ int fdtdec_setup_mem_ram_top(void); > >* invalid > >*/ > > int fdtdec_setup_memory_banksize(void); > > +/** > > + * fdtdec_setup_ram_size() - decode and setup setup gd->ram_size to the > > sum of > > + *all banks > > + * > > + * Decode the /memory 'reg' property to determine the memory available in > > all > > + * banks and populate the global data ram_size with it. > > + * > > + * This function should be called from a boards board_get_usable_ram_top(). > > Is the board_get_usable_ram_top() really the right place to call this ? > It feels like it is being conflated with ram_top here too. I don't think so. You are effectively calculating the top address you should relocate to. You should also calculate the difference in ramsize. Those two should not be separated actions Thanks /Ilias
Re: [PATCH v2 3/4] fdtdec: Introduce fdtdec_setup_ram_size() to determine the memory size
On 3/31/26 10:01 AM, Ilias Apalodimas wrote: We previously added a fdtdec_setup_mem_ram_top() to determine the topmost address of the last DRAM bank. Similarly the overall memory size must be calculated when U-Boot gets relocated to the actual end of all available DRAM. Pre-patch: U-Boot 2026.04-rc5-00012-g5789f787c8a6-dirty (Mar 31 2026 - 09:13:34 +0300) DRAM: 4 GiB (total 8 GiB) Post-patch: U-Boot 2026.04-rc5-00012-g5789f787c8a6 (Mar 31 2026 - 09:12:59 +0300) DRAM: 8 GiB Signed-off-by: Ilias Apalodimas --- include/fdtdec.h | 15 +++ lib/fdtdec.c | 18 ++ 2 files changed, 33 insertions(+) diff --git a/include/fdtdec.h b/include/fdtdec.h index 26e40c3cdb46..9bccd70bbdb6 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -965,6 +965,21 @@ int fdtdec_setup_mem_ram_top(void); * invalid */ int fdtdec_setup_memory_banksize(void); +/** + * fdtdec_setup_ram_size() - decode and setup setup gd->ram_size to the sum of + * all banks + * + * Decode the /memory 'reg' property to determine the memory available in all + * banks and populate the global data ram_size with it. + * + * This function should be called from a boards board_get_usable_ram_top(). Is the board_get_usable_ram_top() really the right place to call this ? It feels like it is being conflated with ram_top here too.
