Hi Jonas, On Fri, 7 Aug 2026 at 17:06, Jonas Karlman <[email protected]> wrote: > > Hi Simon, > > On 8/8/2026 12:00 AM, Simon Glass wrote: > > Hi Jonas, > > > > On Fri, 7 Aug 2026 at 15:41, Jonas Karlman <[email protected]> wrote: > >> > >> Hi Simon, > >> > >> On 8/7/2026 10:40 PM, Simon Glass wrote: > >>> Hi Jonas, > >>> > >>> On 2026-08-07T09:04:31, Jonas Karlman <[email protected]> wrote: > >>>> rockchip: px30: Fix TPL_STACK overflow into BootROM reserved region > >>>> > >>>> The PX30/RK3326 has 16 KiB SRAM at [0xff0e0000, 0xff0e4000), with the > >>>> initial 4 KiB at [0xff0e0000, 0xff0e1000) reserved for BootROM at boot. > >>>> > >>>> SRAM addressing only seem to use 14 bits, meaning that reading from > >>>> 0xff0e4000+ wraps around and instad reads back data at 0xff0e0000+. > >>> > >>> Typos: 'seem' -> 'seems', 'instad' -> 'instead' and 'possible looks' > >>> below -> 'possibly looks'. > >> > >> Thanks, will update in a v2. > >> > >>> > >>>> > >>>> Using a TPL_STACK at 0xff0e4ff0 (16-bytes aligned) means TPL use BootROM > >>>> reserved region for its global data, malloc area and runtime stack. > >>>> > >>>> TPL on PX30/RK3326 does not have any use for the malloc area and the > >>>> generated GD_SIZE is typically around 304 bytes. > >>>> > >>>> Current memory layout possible looks something like: > >>>> > >>>> [0xff0e0000, 0xff0e4000) 16 KiB SRAM > >>>> [0xff0e0000, 0xff0e1000) 4 KiB BROM area > >>>> [0xff0e09f0, 0xff0e0ff0) 1536 bytes malloc area (unused) > >>>> [0xff0e08c0, 0xff0e09f0) 304 bytes global data > >>>> [...] > >>>> > >>>> arch/arm/mach-rockchip/px30/Kconfig | 4 ++-- > >>>> common/spl/Kconfig.tpl | 2 +- > >>>> tools/rkcommon.c | 2 +- > >>>> 3 files changed, 4 insertions(+), 4 deletions(-) > >>> > >>>> diff --git a/common/spl/Kconfig.tpl b/common/spl/Kconfig.tpl > >>>> @@ -129,7 +129,7 @@ config TPL_MAX_SIZE > >>>> - default 0x2800 if ROCKCHIP_PX30 > >>>> + default 0x2c00 if ROCKCHIP_PX30 > >>> > >>> Just to check, the layout shows [0xff0e3c00, 0xff0e3ed0) as pure > >>> stack, but BSS (e.g. dram_info in sdram_px30.c) sits after > >>> __image_copy_end and is not counted by the TPL_MAX_SIZE assert in > >>> u-boot-tpl-v8.lds, so with an image close to 11 KiB the real stack > >>> headroom is a bit less than 720 bytes. > >> > >> You are correct, current BSS is 72 bytes, will update the commit message > >> to mention that the 720 bytes must fit both BSS and the stack in a v2. > >> > >> What is interesting is that global data is so large, especially when in > >> this case nothing here is really using global data. We could save 16 > >> bytes by fully disable CONFIG_TPL_SYS_MALLOC_F, but that was too > >> intrusive to be part of this fix. > > > > Well we have DRAM in there now. Actually now that Ilas' series is in, > > we could take a look at trimming and aligning things - some things are > > long which could be int, which matters on arm64. > > The px30/rk3326 boards do use CONFIG_NR_DRAM_BANKS=1 or 2, so dram > should at least be limited somewhat. > > I think there are some DM related fields that could be excluded to save > little bit more space. With below diff the GD_SIZE is down to 240 bytes, > 16 for TPL_SYS_MALLOC_F and remaining 48 due to the DM related fields. > Not sure this is safe, but the odroid-go2_defconfig could be built. > > I do not think the global data size is really a big issue for this > board, but rk3036 only have 4 KiB that need to fit SPL (2716b), > BSS (0b), global data (160b) and stack. SPL image is down to 2716 bytes > after upcoming TPL refactoring at [1] (tpl branch). > > diff --git a/configs/odroid-go2_defconfig b/configs/odroid-go2_defconfig > index dfdea66cd70c..c5fd6514010c 100644 > --- a/configs/odroid-go2_defconfig > +++ b/configs/odroid-go2_defconfig > @@ -118,3 +118,4 @@ CONFIG_USB_FUNCTION_ROCKUSB=y > CONFIG_TPL_TINY_MEMSET=y > CONFIG_LZO=y > CONFIG_ERRNO_STR=y > +# CONFIG_TPL_SYS_MALLOC_F is not set > diff --git a/include/asm-generic/global_data.h > b/include/asm-generic/global_data.h > index fd116b0cebf2..886c12900ddb 100644 > --- a/include/asm-generic/global_data.h > +++ b/include/asm-generic/global_data.h > @@ -174,10 +174,12 @@ struct global_data { > * @arch: architecture-specific data > */ > struct arch_global_data arch; > +#if CONFIG_IS_ENABLED(DM) > /** > * @dmtag_list: List of DM tags > */ > struct list_head dmtag_list; > +#endif > /** > * @timebase_h: high 32 bits of timer > */ > @@ -228,7 +230,7 @@ struct global_data { > */ > long precon_buf_idx; > #endif > -#ifdef CONFIG_DM > +#if CONFIG_IS_ENABLED(DM) > /** > * @dm_root: root instance for Driver Model > */
Yes please, that's a huge improvement! > > [1] > https://git.u-boot-project.org/u-boot/contributors/kwiboo/u-boot/-/commit/8c8b98a29aa35880b93bbacce8daf68edb537eca > > Regards, > Jonas > > > > > I also did some work on struct board_f which is another way to reduce > > space, if we can find more things which are only needed in early board > > start-up. I suppose we could even expand that to include the > > board_init_r() calls. > > > > Regards, > > Simon > Regards, Simon
