diff mbox series

[3/5] vexpress64: Clean up BASE_FVP boot configuration

Message ID 20211104165619.2684533-4-peter.hoyes@arm.com
State Superseded
Delegated to: Tom Rini
Headers show
Series VExpress64 board family improvements | expand

Commit Message

Peter Hoyes Nov. 4, 2021, 4:56 p.m. UTC
From: Peter Hoyes <Peter.Hoyes@arm.com>

Move env var address values to #defines so they can be reused elsewhere.

Rename env var names to those recommended in the README.

Fix issue where fdt is called with invalid arguments when booting
without a ramdisk.

Signed-off-by: Peter Hoyes <Peter.Hoyes@arm.com>
---
 include/configs/vexpress_aemv8.h | 50 ++++++++++++++++++++------------
 1 file changed, 31 insertions(+), 19 deletions(-)

Comments

Andre Przywara Nov. 9, 2021, 3:06 p.m. UTC | #1
On Thu,  4 Nov 2021 16:56:17 +0000
Peter Hoyes <peter.hoyes@arm.com> wrote:

> From: Peter Hoyes <Peter.Hoyes@arm.com>
> 
> Move env var address values to #defines so they can be reused elsewhere.
> 
> Rename env var names to those recommended in the README.

Many thanks for cleaning this up and fixing the wrong variable names.
I think we should use opportunity to relax the load addresses, see below.

> Fix issue where fdt is called with invalid arguments when booting
> without a ramdisk.
> 
> Signed-off-by: Peter Hoyes <Peter.Hoyes@arm.com>
> ---
>  include/configs/vexpress_aemv8.h | 50 ++++++++++++++++++++------------
>  1 file changed, 31 insertions(+), 19 deletions(-)
> 
> diff --git a/include/configs/vexpress_aemv8.h b/include/configs/vexpress_aemv8.h
> index 49517a60b0..48c21082a6 100644
> --- a/include/configs/vexpress_aemv8.h
> +++ b/include/configs/vexpress_aemv8.h
> @@ -7,6 +7,8 @@
>  #ifndef __VEXPRESS_AEMV8_H
>  #define __VEXPRESS_AEMV8_H
>  
> +#include <linux/stringify.h>
> +
>  #define CONFIG_REMAKE_ELF
>  
>  /* Link Definitions */
> @@ -172,33 +174,43 @@
>  				BOOTENV
>  
>  #elif CONFIG_TARGET_VEXPRESS64_BASE_FVP
> +
> +#define VEXPRESS_FDT_ADDR	0x83000000

So this layout is somewhat miserly. My debug kernel of the day is 42MB
already, so the 47.5 MB we have reserved for the kernel now sound a bit tight.
In arm64 we don't have real constraints, and the model has surely enough
RAM, so can we become more generous?
- Kernel at 512KB (for compatibility with older Linux versions)
- ramdisk just below (or at?) 256MB
- everything else (script, DTB) just below the ramdisk

Cheers,
Andre

> +#define VEXPRESS_RAMDISK_ADDR	0x88000000
> +#define VEXPRESS_KERNEL_ADDR	0x80080000
> +#define VEXPRESS_BOOT_ADDR	0x8007f800
> +
>  #define CONFIG_EXTRA_ENV_SETTINGS	\
>  				"kernel_name=Image\0"		\
> -				"kernel_addr=0x80080000\0"	\
> -				"initrd_name=ramdisk.img\0"	\
> -				"initrd_addr=0x88000000\0"	\
> -				"fdtfile=devtree.dtb\0"		\
> -				"fdt_addr=0x83000000\0"		\
> -				"boot_name=boot.img\0"		\
> -				"boot_addr=0x8007f800\0"
> +				"kernel_addr_r=" __stringify(VEXPRESS_KERNEL_ADDR) "\0"	\
> +				"ramdisk_name=ramdisk.img\0"	\
> +				"ramdisk_addr_r=" __stringify(VEXPRESS_RAMDISK_ADDR) "\0" \
> +				"fdtfile=devtree.dtb\0"	\
> +				"fdt_addr_r=" __stringify(VEXPRESS_FDT_ADDR) "\0"	\
> +				"boot_name=boot.img\0" \
> +				"boot_addr_r=" __stringify(VEXPRESS_BOOT_ADDR) "\0"
>  
>  #ifndef CONFIG_BOOTCOMMAND
> -#define CONFIG_BOOTCOMMAND	"if smhload ${boot_name} ${boot_addr}; then " \
> +#define CONFIG_BOOTCOMMAND	"if smhload ${boot_name} ${boot_addr_r}; then " \
>  				"  set bootargs; " \
> -				"  abootimg addr ${boot_addr}; " \
> -				"  abootimg get dtb --index=0 fdt_addr; " \
> -				"  bootm ${boot_addr} ${boot_addr} " \
> -				"  ${fdt_addr}; " \
> +				"  abootimg addr ${boot_addr_r}; " \
> +				"  abootimg get dtb --index=0 fdt_addr_r; " \
> +				"  bootm ${boot_addr_r} ${boot_addr_r} " \
> +				"  ${fdt_addr_r}; " \
>  				"else; " \
>  				"  set fdt_high 0xffffffffffffffff; " \
>  				"  set initrd_high 0xffffffffffffffff; " \
> -				"  smhload ${kernel_name} ${kernel_addr}; " \
> -				"  smhload ${fdtfile} ${fdt_addr}; " \
> -				"  smhload ${initrd_name} ${initrd_addr} "\
> -				"  initrd_end; " \
> -				"  fdt addr ${fdt_addr}; fdt resize; " \
> -				"  fdt chosen ${initrd_addr} ${initrd_end}; " \
> -				"  booti $kernel_addr - $fdt_addr; " \
> +				"  smhload ${kernel_name} ${kernel_addr_r}; " \
> +				"  smhload ${fdtfile} ${fdt_addr_r}; " \
> +				"  smhload ${ramdisk_name} ${ramdisk_addr_r} "\
> +				"  ramdisk_end; " \
> +				"  fdt addr ${fdt_addr_r}; fdt resize; " \
> +				"  if test -n ${ramdisk_end}; then "\
> +				"    fdt chosen ${ramdisk_addr_r} ${ramdisk_end}; " \
> +				"  else; " \
> +				"    fdt chosen; " \
> +				"  fi; " \
> +				"  booti $kernel_addr_r - $fdt_addr_r; " \
>  				"fi"
>  #endif
>
diff mbox series

Patch

diff --git a/include/configs/vexpress_aemv8.h b/include/configs/vexpress_aemv8.h
index 49517a60b0..48c21082a6 100644
--- a/include/configs/vexpress_aemv8.h
+++ b/include/configs/vexpress_aemv8.h
@@ -7,6 +7,8 @@ 
 #ifndef __VEXPRESS_AEMV8_H
 #define __VEXPRESS_AEMV8_H
 
+#include <linux/stringify.h>
+
 #define CONFIG_REMAKE_ELF
 
 /* Link Definitions */
@@ -172,33 +174,43 @@ 
 				BOOTENV
 
 #elif CONFIG_TARGET_VEXPRESS64_BASE_FVP
+
+#define VEXPRESS_FDT_ADDR	0x83000000
+#define VEXPRESS_RAMDISK_ADDR	0x88000000
+#define VEXPRESS_KERNEL_ADDR	0x80080000
+#define VEXPRESS_BOOT_ADDR	0x8007f800
+
 #define CONFIG_EXTRA_ENV_SETTINGS	\
 				"kernel_name=Image\0"		\
-				"kernel_addr=0x80080000\0"	\
-				"initrd_name=ramdisk.img\0"	\
-				"initrd_addr=0x88000000\0"	\
-				"fdtfile=devtree.dtb\0"		\
-				"fdt_addr=0x83000000\0"		\
-				"boot_name=boot.img\0"		\
-				"boot_addr=0x8007f800\0"
+				"kernel_addr_r=" __stringify(VEXPRESS_KERNEL_ADDR) "\0"	\
+				"ramdisk_name=ramdisk.img\0"	\
+				"ramdisk_addr_r=" __stringify(VEXPRESS_RAMDISK_ADDR) "\0" \
+				"fdtfile=devtree.dtb\0"	\
+				"fdt_addr_r=" __stringify(VEXPRESS_FDT_ADDR) "\0"	\
+				"boot_name=boot.img\0" \
+				"boot_addr_r=" __stringify(VEXPRESS_BOOT_ADDR) "\0"
 
 #ifndef CONFIG_BOOTCOMMAND
-#define CONFIG_BOOTCOMMAND	"if smhload ${boot_name} ${boot_addr}; then " \
+#define CONFIG_BOOTCOMMAND	"if smhload ${boot_name} ${boot_addr_r}; then " \
 				"  set bootargs; " \
-				"  abootimg addr ${boot_addr}; " \
-				"  abootimg get dtb --index=0 fdt_addr; " \
-				"  bootm ${boot_addr} ${boot_addr} " \
-				"  ${fdt_addr}; " \
+				"  abootimg addr ${boot_addr_r}; " \
+				"  abootimg get dtb --index=0 fdt_addr_r; " \
+				"  bootm ${boot_addr_r} ${boot_addr_r} " \
+				"  ${fdt_addr_r}; " \
 				"else; " \
 				"  set fdt_high 0xffffffffffffffff; " \
 				"  set initrd_high 0xffffffffffffffff; " \
-				"  smhload ${kernel_name} ${kernel_addr}; " \
-				"  smhload ${fdtfile} ${fdt_addr}; " \
-				"  smhload ${initrd_name} ${initrd_addr} "\
-				"  initrd_end; " \
-				"  fdt addr ${fdt_addr}; fdt resize; " \
-				"  fdt chosen ${initrd_addr} ${initrd_end}; " \
-				"  booti $kernel_addr - $fdt_addr; " \
+				"  smhload ${kernel_name} ${kernel_addr_r}; " \
+				"  smhload ${fdtfile} ${fdt_addr_r}; " \
+				"  smhload ${ramdisk_name} ${ramdisk_addr_r} "\
+				"  ramdisk_end; " \
+				"  fdt addr ${fdt_addr_r}; fdt resize; " \
+				"  if test -n ${ramdisk_end}; then "\
+				"    fdt chosen ${ramdisk_addr_r} ${ramdisk_end}; " \
+				"  else; " \
+				"    fdt chosen; " \
+				"  fi; " \
+				"  booti $kernel_addr_r - $fdt_addr_r; " \
 				"fi"
 #endif