| Message ID | 20181030125553.5230-25-lukas.auer@aisec.fraunhofer.de |
|---|---|
| State | Superseded |
| Delegated to: | Andes |
| Headers | show |
| Series | General fixes / cleanup for RISC-V and improvements to qemu-riscv | expand |
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@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". 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 > + > + return 0; > +} > diff --git a/arch/riscv/include/asm/u-boot-riscv.h b/arch/riscv/include/asm/u-boot-riscv.h > index 49febd5881..937b2682dc 100644 > --- a/arch/riscv/include/asm/u-boot-riscv.h > +++ b/arch/riscv/include/asm/u-boot-riscv.h > @@ -13,6 +13,7 @@ > > /* cpu/.../cpu.c */ > int cleanup_before_linux(void); > +int arch_misc_init(void); > > /* board/.../... */ > int board_init(void); >
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@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. Thanks, Lukas > > + > > + return 0; > > +} > > diff --git a/arch/riscv/include/asm/u-boot-riscv.h > > b/arch/riscv/include/asm/u-boot-riscv.h > > index 49febd5881..937b2682dc 100644 > > --- a/arch/riscv/include/asm/u-boot-riscv.h > > +++ b/arch/riscv/include/asm/u-boot-riscv.h > > @@ -13,6 +13,7 @@ > > > > /* cpu/.../cpu.c */ > > int cleanup_before_linux(void); > > +int arch_misc_init(void); > > > > /* board/.../... */ > > int board_init(void); > >
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@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 :). 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
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@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
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); + + return 0; +} diff --git a/arch/riscv/include/asm/u-boot-riscv.h b/arch/riscv/include/asm/u-boot-riscv.h index 49febd5881..937b2682dc 100644 --- a/arch/riscv/include/asm/u-boot-riscv.h +++ b/arch/riscv/include/asm/u-boot-riscv.h @@ -13,6 +13,7 @@ /* cpu/.../cpu.c */ int cleanup_before_linux(void); +int arch_misc_init(void); /* board/.../... */ int board_init(void);
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@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(+)