Hi Jonas, On Sat, 8 Aug 2026 at 14:45, Jonas Karlman <[email protected]> wrote: > > 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].
OK good. Ilias may have some comment too. Regards, Simon > > [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 >
