| Message ID | 20190409104247.5721-3-david.abdurachmanov@gmail.com |
|---|---|
| State | Accepted |
| Commit | 081c660201f7567c4fc000e4d758b5ab9f0e8db4 |
| Delegated to: | Andes |
| Headers | show |
| Series | riscv: set preboot and increase kernel size | expand |
> -----Original Message----- > From: David Abdurachmanov <david.abdurachmanov@gmail.com> > Sent: Tuesday, April 9, 2019 4:13 PM > To: rick@andestech.com; Atish Patra <Atish.Patra@wdc.com>; Anup Patel > <Anup.Patel@wdc.com>; lukas.auer@aisec.fraunhofer.de; u- > boot@lists.denx.de > Cc: David Abdurachmanov <david.abdurachmanov@gmail.com> > Subject: [PATCH 2/2] riscv: qemu-riscv.h: define CONFIG_PREBOOT (enables > extlinux) > > - Set fdt_addr variable, which is needed for extlinux to find FDT. > Otherwise booting kernel using extlinux results in missing FDT. > > - Also run fdt addr with FDT address so that fdt commands would > work out of the box in U-Boot prompt. > > This is successfully used by Fedora/RISCV with 5.1-rc3+ kernel using OpenSBI > -> U-Boot (S-mode) [extlinux] -> Kernel setup. > > Signed-off-by: David Abdurachmanov <david.abdurachmanov@gmail.com> > --- > include/configs/qemu-riscv.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/configs/qemu-riscv.h b/include/configs/qemu-riscv.h > index 22a5cd7365..b7110edebc 100644 > --- a/include/configs/qemu-riscv.h > +++ b/include/configs/qemu-riscv.h > @@ -48,4 +48,8 @@ > "ramdisk_addr_r=0x88300000\0" \ > BOOTENV > > +#define CONFIG_PREBOOT \ > + "setenv fdt_addr ${fdtcontroladdr};" \ > + "fdt addr ${fdtcontroladdr};" > + > #endif /* __CONFIG_H */ > -- > 2.20.1 Looks good to me. Reviewed-by: Anup Patel <anup.patel@wdc.com> Regards, Anup
+ Bin On Tue, 2019-04-09 at 12:42 +0200, David Abdurachmanov wrote: > - Set fdt_addr variable, which is needed for extlinux to find FDT. > Otherwise booting kernel using extlinux results in missing FDT. > > - Also run fdt addr with FDT address so that fdt commands would > work out of the box in U-Boot prompt. While often useful to have, the fdt command is not used during a normal boot. I think we should avoid calling commands, which are not normally needed. Can you remove this from your patch? Thanks, Lukas > > This is successfully used by Fedora/RISCV with 5.1-rc3+ kernel using > OpenSBI -> U-Boot (S-mode) [extlinux] -> Kernel setup. > > Signed-off-by: David Abdurachmanov <david.abdurachmanov@gmail.com> > --- > include/configs/qemu-riscv.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/include/configs/qemu-riscv.h b/include/configs/qemu-riscv.h > index 22a5cd7365..b7110edebc 100644 > --- a/include/configs/qemu-riscv.h > +++ b/include/configs/qemu-riscv.h > @@ -48,4 +48,8 @@ > "ramdisk_addr_r=0x88300000\0" \ > BOOTENV > > +#define CONFIG_PREBOOT \ > + "setenv fdt_addr ${fdtcontroladdr};" \ > + "fdt addr ${fdtcontroladdr};" > + > #endif /* __CONFIG_H */
On Thu, Apr 11, 2019 at 2:41 PM Auer, Lukas <lukas.auer@aisec.fraunhofer.de> wrote: > > + Bin > > On Tue, 2019-04-09 at 12:42 +0200, David Abdurachmanov wrote: > > - Set fdt_addr variable, which is needed for extlinux to find FDT. > > Otherwise booting kernel using extlinux results in missing FDT. > > > > - Also run fdt addr with FDT address so that fdt commands would > > work out of the box in U-Boot prompt. > > While often useful to have, the fdt command is not used during a normal > boot. I think we should avoid calling commands, which are not normally > needed. Can you remove this from your patch? I borrowed idea from other boards, and I find it useful to have fdt working out-of-the-box without trying to figure the correct variable which holds address of FDT. I can remove it from the patch, but I will probably keep adding it as Fedora specific patch then. > > Thanks, > Lukas > > > > > This is successfully used by Fedora/RISCV with 5.1-rc3+ kernel using > > OpenSBI -> U-Boot (S-mode) [extlinux] -> Kernel setup. > > > > Signed-off-by: David Abdurachmanov <david.abdurachmanov@gmail.com> > > --- > > include/configs/qemu-riscv.h | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/include/configs/qemu-riscv.h b/include/configs/qemu-riscv.h > > index 22a5cd7365..b7110edebc 100644 > > --- a/include/configs/qemu-riscv.h > > +++ b/include/configs/qemu-riscv.h > > @@ -48,4 +48,8 @@ > > "ramdisk_addr_r=0x88300000\0" \ > > BOOTENV > > > > +#define CONFIG_PREBOOT \ > > + "setenv fdt_addr ${fdtcontroladdr};" \ > > + "fdt addr ${fdtcontroladdr};" > > + > > #endif /* __CONFIG_H */
On Thu, 2019-04-11 at 14:51 +0200, David Abdurachmanov wrote: > On Thu, Apr 11, 2019 at 2:41 PM Auer, Lukas > <lukas.auer@aisec.fraunhofer.de> wrote: > > + Bin > > > > On Tue, 2019-04-09 at 12:42 +0200, David Abdurachmanov wrote: > > > - Set fdt_addr variable, which is needed for extlinux to find FDT. > > > Otherwise booting kernel using extlinux results in missing FDT. > > > > > > - Also run fdt addr with FDT address so that fdt commands would > > > work out of the box in U-Boot prompt. > > > > While often useful to have, the fdt command is not used during a normal > > boot. I think we should avoid calling commands, which are not normally > > needed. Can you remove this from your patch? > > I borrowed idea from other boards, and I find it useful to have fdt working > out-of-the-box without trying to figure the correct variable which holds > address of FDT. > > I can remove it from the patch, but I will probably keep adding it as Fedora > specific patch then. > That makes sense. I would still tend towards removing it, but will wait to see what everybody else thinks. Other than that, the patch looks good to me. Reviewed-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de> > > Thanks, > > Lukas > > > > > This is successfully used by Fedora/RISCV with 5.1-rc3+ kernel using > > > OpenSBI -> U-Boot (S-mode) [extlinux] -> Kernel setup. > > > > > > Signed-off-by: David Abdurachmanov <david.abdurachmanov@gmail.com> > > > --- > > > include/configs/qemu-riscv.h | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/include/configs/qemu-riscv.h b/include/configs/qemu-riscv.h > > > index 22a5cd7365..b7110edebc 100644 > > > --- a/include/configs/qemu-riscv.h > > > +++ b/include/configs/qemu-riscv.h > > > @@ -48,4 +48,8 @@ > > > "ramdisk_addr_r=0x88300000\0" \ > > > BOOTENV > > > > > > +#define CONFIG_PREBOOT \ > > > + "setenv fdt_addr ${fdtcontroladdr};" \ > > > + "fdt addr ${fdtcontroladdr};" > > > + > > > #endif /* __CONFIG_H */
On Fri, Apr 12, 2019 at 12:38 AM Auer, Lukas <lukas.auer@aisec.fraunhofer.de> wrote: > > On Thu, 2019-04-11 at 14:51 +0200, David Abdurachmanov wrote: > > On Thu, Apr 11, 2019 at 2:41 PM Auer, Lukas > > <lukas.auer@aisec.fraunhofer.de> wrote: > > > + Bin > > > > > > On Tue, 2019-04-09 at 12:42 +0200, David Abdurachmanov wrote: > > > > - Set fdt_addr variable, which is needed for extlinux to find FDT. > > > > Otherwise booting kernel using extlinux results in missing FDT. > > > > > > > > - Also run fdt addr with FDT address so that fdt commands would > > > > work out of the box in U-Boot prompt. > > > > > > While often useful to have, the fdt command is not used during a normal > > > boot. I think we should avoid calling commands, which are not normally > > > needed. Can you remove this from your patch? > > > > I borrowed idea from other boards, and I find it useful to have fdt working > > out-of-the-box without trying to figure the correct variable which holds > > address of FDT. > > > > I can remove it from the patch, but I will probably keep adding it as Fedora > > specific patch then. > > > > That makes sense. I would still tend towards removing it, but will wait > to see what everybody else thinks. Yes, I am inclined to not include "fdt addr ${fdtcontroladdr};" too. > > Other than that, the patch looks good to me. > > Reviewed-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de> > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> Regards, Bin
> Subject: Re: [PATCH 2/2] riscv: qemu-riscv.h: define CONFIG_PREBOOT (enables > extlinux) > > On Fri, Apr 12, 2019 at 12:38 AM Auer, Lukas <lukas.auer@aisec.fraunhofer.de> > wrote: > > > > On Thu, 2019-04-11 at 14:51 +0200, David Abdurachmanov wrote: > > > On Thu, Apr 11, 2019 at 2:41 PM Auer, Lukas > > > <lukas.auer@aisec.fraunhofer.de> wrote: > > > > + Bin > > > > > > > > On Tue, 2019-04-09 at 12:42 +0200, David Abdurachmanov wrote: > > > > - Set fdt_addr variable, which is needed for extlinux to find FDT. > > > > Otherwise booting kernel using extlinux results in missing FDT. > > > > > > > > - Also run fdt addr with FDT address so that fdt commands would > > > > work out of the box in U-Boot prompt. > > > > > > > While often useful to have, the fdt command is not used during a > > > > normal boot. I think we should avoid calling commands, which are > > > > not normally needed. Can you remove this from your patch? > > > > > > I borrowed idea from other boards, and I find it useful to have fdt > > > working out-of-the-box without trying to figure the correct variable > > > which holds address of FDT. > > > > > > I can remove it from the patch, but I will probably keep adding it > > > as Fedora specific patch then. > > > > > > > That makes sense. I would still tend towards removing it, but will > > wait to see what everybody else thinks. > > Yes, I am inclined to not include "fdt addr ${fdtcontroladdr};" too. > > > > > Other than that, the patch looks good to me. > > > > Reviewed-by: Lukas Auer <lukas.auer@aisec.fraunhofer.de> > > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com> Applied to u-boot-riscv/master, thanks!
diff --git a/include/configs/qemu-riscv.h b/include/configs/qemu-riscv.h index 22a5cd7365..b7110edebc 100644 --- a/include/configs/qemu-riscv.h +++ b/include/configs/qemu-riscv.h @@ -48,4 +48,8 @@ "ramdisk_addr_r=0x88300000\0" \ BOOTENV +#define CONFIG_PREBOOT \ + "setenv fdt_addr ${fdtcontroladdr};" \ + "fdt addr ${fdtcontroladdr};" + #endif /* __CONFIG_H */
- Set fdt_addr variable, which is needed for extlinux to find FDT. Otherwise booting kernel using extlinux results in missing FDT. - Also run fdt addr with FDT address so that fdt commands would work out of the box in U-Boot prompt. This is successfully used by Fedora/RISCV with 5.1-rc3+ kernel using OpenSBI -> U-Boot (S-mode) [extlinux] -> Kernel setup. Signed-off-by: David Abdurachmanov <david.abdurachmanov@gmail.com> --- include/configs/qemu-riscv.h | 4 ++++ 1 file changed, 4 insertions(+)