Re: [PATCH 9/9] ARM: rockchip: fixup memory in device tree for TF-A

[Marco Felsch <m.felsch@pengutronix.de>]

On 25-05-27, Michael Tretter wrote:
> On Mon, 26 May 2025 19:25:01 +0200, Marco Felsch wrote:
> > On 25-05-26, Michael Tretter wrote:
> > > Add the memory nodes for the detected SDRAM configuration to the fdt
> > > before passing it to the TF-A.
> > > 
> > > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > > ---
> > >  arch/arm/mach-rockchip/atf.c | 22 ++++++++++++++++++++++
> > >  1 file changed, 22 insertions(+)
> > > 
> > > diff --git a/arch/arm/mach-rockchip/atf.c b/arch/arm/mach-rockchip/atf.c
> > > index 12cf13717b6972c2eafc5a044ae8d0b4de029c32..342af302aa25089acce3c91df0ea38cbe71e0add 100644
> > > --- a/arch/arm/mach-rockchip/atf.c
> > > +++ b/arch/arm/mach-rockchip/atf.c
> > > @@ -173,6 +173,26 @@ void rk3588_atf_load_bl31(void *fdt)
> > >  	rockchip_atf_load_bl31(RK3588, rk3588_bl31_bin, rk3588_bl32_bin, fdt);
> > >  }
> > >  
> > > +#ifdef CONFIG_ARCH_ROCKCHIP_ATF_PASS_FDT
> > > +static int rk3588_fixup_mem(void *fdt)
> > > +{
> > > +	/* Use 4 blocks since rk3588 has 3 gaps in the address space */
> > > +	unsigned long base[4];
> > > +	unsigned long size[ARRAY_SIZE(base)];
> > > +	phys_addr_t base_tmp[ARRAY_SIZE(base)];
> > > +	resource_size_t size_tmp[ARRAY_SIZE(base_tmp)];
> > > +	int i, n;
> > > +
> > > +	n = rk3588_ram_sizes(base_tmp, size_tmp, ARRAY_SIZE(base_tmp));
> > > +	for (i = 0; i < n; i++) {
> > > +		base[i] = base_tmp[i];
> > > +		size[i] = size_tmp[i];
> > > +	}
> > > +
> > > +	return fdt_fixup_mem(fdt, base, size, i);
> > 
> > This fixup will run on a RO marked section if I got the code correct.
> > Also the fixup logic doesn't work for compressed device-tree's.
> > 
> > I had an offlist discussion with Ahmad last week exactly targeting such
> > use-case (passing the dt from firmware to firmware). The conclusion was
> > that each firmware should generate an overlay which will be applied to
> > the barebox live-dt and the kernel-dt later on.
> > 
> > Question: Is this to late for OP-TEE? I don't know the RK3588 nor the
> > OP-TEE integration for it, but on i.MX8M the OP-TEE SHM doesn't require
> > any device-tree yet.
> 
> This is about passing a device tree from barebox to OP-TEE. OP-TEE uses
> the device tree and especially the memory nodes to initialize the
> dynamic shared memory. This is a platform independent implementation in
> OP-TEE.  It's also possible to configure the addresses and sizes via
> config parameter, but that doesn't work if a board supports different
> SDRAM configurations.

Because OP-TEE is loaded at the SDRAM end per default?

If this is the reason, we already do support putting OP-TEE at the SDRAM
start followed by barebox on i.MX8M* SoCs:

   OP-TEE membase parsing:
 - https://elixir.bootlin.com/barebox/v2025.05.0/source/arch/arm/mach-imx/atf.c#L168

   OP-TEE set membase:
 - https://elixir.bootlin.com/barebox/v2025.05.0/source/common/optee.c#L52
   https://elixir.bootlin.com/barebox/v2025.05.0/source/arch/arm/mach-imx/esdctl.c#L1016
   https://elixir.bootlin.com/barebox/v2025.05.0/source/drivers/soc/imx/soc-imx8m.c#L217

   Barebox bin loadaddr:
 - https://elixir.bootlin.com/barebox/v2025.05.0/source/arch/arm/mach-imx/atf.c#L218

> Im not sure if OP-TEE accepts an dt overlay here and if this is
> compliant with the Firmware handoff specification. If this is possible,
> OP-TEE needs its own device tree for the platform and be able to apply
> an overlay. Furthermore, the barebox PBL would have to generate an
> overlay fdt with the memory configuration. Not sure if this is actually
> better than fixing the device tree before passing it to OP-TEE.

The idea was more like this:

   OP-TEE:
 - https://elixir.bootlin.com/op-tee/4.6.0/source/mk/config.mk#L553

   Barebox:
 - https://elixir.bootlin.com/barebox/v2025.05.0/source/arch/arm/boards/webasto-ccbv2/board.c#L33

If I get the OP-TEE code correct, we can pass the location for the
overlay via boot-argument arg2. So barebox can do all the RAM detection
and pass the correct location for the DT overlay. This can be handed
over to barebox proper via the barebox handoff-data mechanism:

 - https://elixir.bootlin.com/barebox/v2025.05.0/source/include/pbl/handoff-data.h

Regards,
  Marco