Hi Simon, On 8/8/2026 3:52 PM, Simon Glass wrote: > 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!
Will send out above struct global_data in a separate patch, CI seem to pass with above changes applied, see [2]. [2] https://git.u-boot-project.org/u-boot/contributors/kwiboo/u-boot/-/pipelines/924 Regards, Jonas > >> >> [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
