Re: [PATCH 3/5] ARM: Rockchip: Add rk3568 specific barebox entry function

[Sascha Hauer <s.hauer@pengutronix.de>]

On Sun, Mar 26, 2023 at 10:36:19AM +0200, Rouven Czerwinski wrote:
> Hi,
> 
> On Fri, 2023-03-24 at 15:03 +0100, Sascha Hauer wrote:
> > Add a rk3568 specific barebox entry function to simplify board
> > code.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  arch/arm/mach-rockchip/atf.c | 22 ++++++++++++++++++++++
> >  include/mach/rockchip/atf.h  |  2 ++
> >  2 files changed, 24 insertions(+)
> > 
> > diff --git a/arch/arm/mach-rockchip/atf.c b/arch/arm/mach-
> > rockchip/atf.c
> > index 93025faf68..a7d626226e 100644
> > --- a/arch/arm/mach-rockchip/atf.c
> > +++ b/arch/arm/mach-rockchip/atf.c
> > @@ -5,6 +5,9 @@
> >  #include <mach/rockchip/atf.h>
> >  #include <elf.h>
> >  #include <asm/atf_common.h>
> > +#include <asm/barebox-arm.h>
> > +#include <mach/rockchip/dmc.h>
> > +#include <mach/rockchip/rockchip.h>
> >  
> >  static unsigned long load_elf64_image_phdr(const void *elf)
> >  {
> > @@ -69,3 +72,22 @@ void rk3568_atf_load_bl31(void *fdt)
> >  {
> >         rockchip_atf_load_bl31(RK3568, rk3568_bl31_bin,
> > rk3568_op_tee_bin, fdt);
> >  }
> > +
> > +void __noreturn rk3568_barebox_entry(void *fdt)
> > +{
> > +       unsigned long membase, memsize;
> > +
> > +       membase = RK3568_DRAM_BOTTOM;
> > +       memsize = rk3568_ram0_size() - RK3568_DRAM_BOTTOM;
> > +
> > +       if (current_el() == 3) {
> > +               relocate_to_adr_full(RK3568_BAREBOX_LOAD_ADDRESS);
> > +               setup_c();
> > +
> > +               rk3568_lowlevel_init();
> > +               rk3568_atf_load_bl31(fdt);
> 
> At least on my board I have to replace the fdt with a NULL pointer to
> get the downstream TF-A to work. What is the intention for barebox
> here? Do we want to support both upstream and downstream? Or do we need
> an fdt size check in the PBL to decide whether we want to pass the
> pointer?

There are two problems here. First one is that the downstream TF-A
cannot handle device trees that exceed a certain size. That is triggered
with enabling CONFIG_OF_OVERLAY_LIVE.

The second one comes with this patch. Without the common barebox entry
we had:

	relocate_to_adr_full(RK3568_BAREBOX_LOAD_ADDRESS);
	fdt = __dtb_rk3566_quartz64_a_start;

That means the device tree is taken from the place we have just
relocated to. With the common barebox entry the above is done the other
way round which means the device tree is used from the binary we just
relocated away from. It is overwritten by the TF-A.

There are some possible solutions to this, I haven't sorted out yet what
might be best.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |