[U-Boot] [PATCH v2 24/29] riscv: store device tree passed by prior boot stage in environment

[Auer, Lukas]

On Tue, 2018-10-30 at 14:53 +0100, Alexander Graf wrote:
> 
> On 30.10.18 14:44, Auer, Lukas wrote:
> > On Tue, 2018-10-30 at 14:19 +0100, Alexander Graf wrote:
> > > 
> > > On 30.10.18 13:55, Lukas Auer wrote:
> > > > The device tree passed by the prior boot stage can be used to
> > > > boot
> > > > Linux. Store it as environment variable "prior_stage_dtb", so
> > > > that
> > > > it
> > > > can be used as part of the boot command.
> > > > 
> > > > Signed-off-by: Lukas Auer <lukas.auer at aisec.fraunhofer.de>
> > > > ---
> > > > 
> > > > Changes in v2:
> > > > - New patch
> > > > 
> > > >  arch/Kconfig                          | 1 +
> > > >  arch/riscv/cpu/cpu.c                  | 7 +++++++
> > > >  arch/riscv/include/asm/u-boot-riscv.h | 1 +
> > > >  3 files changed, 9 insertions(+)
> > > > 
> > > > diff --git a/arch/Kconfig b/arch/Kconfig
> > > > index 9fdd2f7e66..883e4a9308 100644
> > > > --- a/arch/Kconfig
> > > > +++ b/arch/Kconfig
> > > > @@ -74,6 +74,7 @@ config RISCV
> > > >  	imply MTD
> > > >  	imply TIMER
> > > >  	imply CMD_DM
> > > > +	imply ARCH_MISC_INIT
> > > >  
> > > >  config SANDBOX
> > > >  	bool "Sandbox"
> > > > diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
> > > > index d9f820c44c..e06a8c6bab 100644
> > > > --- a/arch/riscv/cpu/cpu.c
> > > > +++ b/arch/riscv/cpu/cpu.c
> > > > @@ -53,3 +53,10 @@ int print_cpuinfo(void)
> > > >  
> > > >  	return 0;
> > > >  }
> > > > +
> > > > +int arch_misc_init(void)
> > > > +{
> > > > +	env_set_hex("prior_stage_dtb",
> > > > prior_stage_fdt_address);
> > > 
> > > Please reuse one of the many other variable names we have for
> > > this
> > > already, such as "fdtaddr".
> > > 
> > 
> > I will change the variable name.
> > 
> > > One thing I would always recommend is that if you have a prior
> > > stage
> > > DT
> > > passed on to U-Boot is that U-Boot makes use of that
> > > (CONFIG_OF_BOARD).
> > > That again means that the DT is already available in
> > > $fdtcontroladdr
> > > and
> > > there is very little use for yet another env variable.
> > > 
> > > 
> > > Alex
> > > 
> > 
> > We are actually using the device tree passed to U-Boot (using
> > CONFIG_OF_PRIOR_STAGE). The reason I'm adding the device tree
> > address
> > to the environment is so that it can be used to boot Linux. RISC-V
> > Linux does not include any device trees, they are instead stored
> > directly on the SoC (though this might change). This is therefore
> > important to have and this is the best way I could think of, of
> > implementing it.
> 
> Oh, I'm in 100% agreement that this is the right choice. I'm just
> surprised we have 2 Kconfig options to basically do the same thing.
> The
> RPi uses CONFIG_OF_BOARD while newer BCM chips seem to use
> CONFIG_PRIOR_STAGE. It's slightly counterintuitive :).
> 

That's true. I actually picked CONFIG_OF_PRIOR_STAGE just because I
didn't need the flexibility of CONFIG_OF_BOARD.

> What I was saying is that U-Boot should be a consumer of said
> "platform
> device tree" and thus exposes it as $fdtcontroladdr into the
> environment. So instead of
> 
>   # bootefi $kernel_addr_r $fdtaddr
> 
> you could as well just use
> 
>   # bootefi $kernel_addr_r $fdtcontroladdr
> 
> which the distro scripts automatically do for you, so you already
> have
> what you asked for :).
> 
> 
> Alex

Oh right, I completely missed that. Thanks for pointing that out! :)

Lukas