[U-Boot] [PATCH] ARM: tegra: CONFIG_{SYS_, }LOAD{_, }ADDR rationalization
From: Stephen Warren swarren@nvidia.com
As best I can tell, CONFIG_SYS_LOAD_ADDR and CONFIG_LOADADDR/$loadaddr serve essentially the same purpose. Roughly, if a command takes a load address, then CONFIG_SYS_LOAD_ADDR or $loadaddr (or both) are the default if the command-line does not specify the address. Different U-Boot commands are inconsistent re: which of the two default values they use. As such, set the two to the same value, and move the logic that does this into tegra-common-post.h so it's not duplicated. A number of other non- Tegra boards do this too.
The values chosen for these macros are no longer consistent with anything in MEM_LAYOUT_ENV_SETTINGS. Regain consistency by setting $kernel_addr_r to CONFIG_LOADADDR. Older scripts tend to use $loadaddr for the default kernel load address, whereas newer scripts and features tend to use $kernel_addr_r, along with other variables for other purposes such as DTBs and initrds. Hence, it's logical they should share the same value.
I had originally thought to make the $kernel_addr_r and CONFIG_LOADADDR have different values. This would guarantee no interference if a script used the two variables for different purposes. However, that scenario is unlikely given the semantic meaning associated with the two variables. The lowest available value is 0x90200000; see comments for MEM_LAYOUT_ENV_SETTINGS in tegra30-common-post.h for details. However, that value would be problematic for a script that loaded a raw zImage to $loadaddr, since it's more than 128MB beyond the start of SDRAM, which would interfere with the kernel's CONFIG_AUTO_ZRELADDR. So, let's not do that.
The only potential fallout I could foresee from this patch is if someone has a script that loads the kernel to $loadaddr, but some other file (DTB, initrd) to a hard-coded address that the new value of $loadaddr interferes with. This seems unlikely. A user should not do that; they should either hard-code all load addresses, or use U-Boot-supplied variables for all load addresses. Equally, any fallout due to this change is trivial to fix; simply modify the load addresses in that script.
Cc: Paul Walmsley pwalmsley@nvidia.com Signed-off-by: Stephen Warren swarren@nvidia.com --- include/configs/tegra-common-post.h | 2 ++ include/configs/tegra114-common.h | 7 ++----- include/configs/tegra124-common.h | 7 ++----- include/configs/tegra20-common.h | 7 ++----- include/configs/tegra30-common.h | 7 ++----- 5 files changed, 10 insertions(+), 20 deletions(-)
diff --git a/include/configs/tegra-common-post.h b/include/configs/tegra-common-post.h index c3ad8beb903d..31096d068bb1 100644 --- a/include/configs/tegra-common-post.h +++ b/include/configs/tegra-common-post.h @@ -50,6 +50,8 @@ #define BOARD_EXTRA_ENV_SETTINGS #endif
+#define CONFIG_SYS_LOAD_ADDR CONFIG_LOADADDR + #define CONFIG_EXTRA_ENV_SETTINGS \ TEGRA_DEVICE_SETTINGS \ MEM_LAYOUT_ENV_SETTINGS \ diff --git a/include/configs/tegra114-common.h b/include/configs/tegra114-common.h index 9eba5d517db7..252e607d73f4 100644 --- a/include/configs/tegra114-common.h +++ b/include/configs/tegra114-common.h @@ -26,13 +26,9 @@ */ #define V_NS16550_CLK 408000000 /* 408MHz (pllp_out0) */
-/* Environment information, boards can override if required */ -#define CONFIG_LOADADDR 0x80408000 /* def. location for kernel */ - /* * Miscellaneous configurable options */ -#define CONFIG_SYS_LOAD_ADDR 0x80A00800 /* default */ #define CONFIG_STACKBASE 0x82800000 /* 40MB */
/*----------------------------------------------------------------------- @@ -64,10 +60,11 @@ * ramdisk_addr_r simply shouldn't overlap anything else. Choosing 33M allows * for the FDT/DTB to be up to 1M, which is hopefully plenty. */ +#define CONFIG_LOADADDR 0x81000000 #define MEM_LAYOUT_ENV_SETTINGS \ "scriptaddr=0x90000000\0" \ "pxefile_addr_r=0x90100000\0" \ - "kernel_addr_r=0x81000000\0" \ + "kernel_addr_r=" __stringify(CONFIG_LOADADDR) "\0" \ "fdt_addr_r=0x82000000\0" \ "ramdisk_addr_r=0x82100000\0"
diff --git a/include/configs/tegra124-common.h b/include/configs/tegra124-common.h index f2b3774da8ff..1aee5c89f4c4 100644 --- a/include/configs/tegra124-common.h +++ b/include/configs/tegra124-common.h @@ -18,13 +18,9 @@ */ #define V_NS16550_CLK 408000000 /* 408MHz (pllp_out0) */
-/* Environment information, boards can override if required */ -#define CONFIG_LOADADDR 0x80408000 /* def. location for kernel */ - /* * Miscellaneous configurable options */ -#define CONFIG_SYS_LOAD_ADDR 0x80A00800 /* default */ #define CONFIG_STACKBASE 0x82800000 /* 40MB */
/*----------------------------------------------------------------------- @@ -56,10 +52,11 @@ * ramdisk_addr_r simply shouldn't overlap anything else. Choosing 33M allows * for the FDT/DTB to be up to 1M, which is hopefully plenty. */ +#define CONFIG_LOADADDR 0x81000000 #define MEM_LAYOUT_ENV_SETTINGS \ "scriptaddr=0x90000000\0" \ "pxefile_addr_r=0x90100000\0" \ - "kernel_addr_r=0x81000000\0" \ + "kernel_addr_r=" __stringify(CONFIG_LOADADDR) "\0" \ "fdt_addr_r=0x82000000\0" \ "ramdisk_addr_r=0x82100000\0"
diff --git a/include/configs/tegra20-common.h b/include/configs/tegra20-common.h index 6330281df71b..0841f33bfc9e 100644 --- a/include/configs/tegra20-common.h +++ b/include/configs/tegra20-common.h @@ -24,13 +24,9 @@ */ #define V_NS16550_CLK 216000000 /* 216MHz (pllp_out0) */
-/* Environment information, boards can override if required */ -#define CONFIG_LOADADDR 0x00408000 /* def. location for kernel */ - /* * Miscellaneous configurable options */ -#define CONFIG_SYS_LOAD_ADDR 0x00A00800 /* default */ #define CONFIG_STACKBASE 0x02800000 /* 40MB */
/*----------------------------------------------------------------------- @@ -62,10 +58,11 @@ * ramdisk_addr_r simply shouldn't overlap anything else. Choosing 33M allows * for the FDT/DTB to be up to 1M, which is hopefully plenty. */ +#define CONFIG_LOADADDR 0x01000000 #define MEM_LAYOUT_ENV_SETTINGS \ "scriptaddr=0x10000000\0" \ "pxefile_addr_r=0x10100000\0" \ - "kernel_addr_r=0x01000000\0" \ + "kernel_addr_r=" __stringify(CONFIG_LOADADDR) "\0" \ "fdt_addr_r=0x02000000\0" \ "ramdisk_addr_r=0x02100000\0"
diff --git a/include/configs/tegra30-common.h b/include/configs/tegra30-common.h index bfdbeb70d296..3e8e3c1e5bd9 100644 --- a/include/configs/tegra30-common.h +++ b/include/configs/tegra30-common.h @@ -23,13 +23,9 @@ */ #define V_NS16550_CLK 408000000 /* 408MHz (pllp_out0) */
-/* Environment information, boards can override if required */ -#define CONFIG_LOADADDR 0x80408000 /* def. location for kernel */ - /* * Miscellaneous configurable options */ -#define CONFIG_SYS_LOAD_ADDR 0x80A00800 /* default */ #define CONFIG_STACKBASE 0x82800000 /* 40MB */
/*----------------------------------------------------------------------- @@ -61,10 +57,11 @@ * ramdisk_addr_r simply shouldn't overlap anything else. Choosing 33M allows * for the FDT/DTB to be up to 1M, which is hopefully plenty. */ +#define CONFIG_LOADADDR 0x81000000 #define MEM_LAYOUT_ENV_SETTINGS \ "scriptaddr=0x90000000\0" \ "pxefile_addr_r=0x90100000\0" \ - "kernel_addr_r=0x81000000\0" \ + "kernel_addr_r=" __stringify(CONFIG_LOADADDR) "\0" \ "fdt_addr_r=0x82000000\0" \ "ramdisk_addr_r=0x82100000\0"
On 04/01/2015 03:40 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
As best I can tell, CONFIG_SYS_LOAD_ADDR and CONFIG_LOADADDR/$loadaddr serve essentially the same purpose. Roughly, if a command takes a load address, then CONFIG_SYS_LOAD_ADDR or $loadaddr (or both) are the default if the command-line does not specify the address. Different U-Boot commands are inconsistent re: which of the two default values they use. As such, set the two to the same value, and move the logic that does this into tegra-common-post.h so it's not duplicated. A number of other non- Tegra boards do this too.
The values chosen for these macros are no longer consistent with anything in MEM_LAYOUT_ENV_SETTINGS. Regain consistency by setting $kernel_addr_r to CONFIG_LOADADDR. Older scripts tend to use $loadaddr for the default kernel load address, whereas newer scripts and features tend to use $kernel_addr_r, along with other variables for other purposes such as DTBs and initrds. Hence, it's logical they should share the same value.
I had originally thought to make the $kernel_addr_r and CONFIG_LOADADDR have different values. This would guarantee no interference if a script used the two variables for different purposes. However, that scenario is unlikely given the semantic meaning associated with the two variables. The lowest available value is 0x90200000; see comments for MEM_LAYOUT_ENV_SETTINGS in tegra30-common-post.h for details. However, that value would be problematic for a script that loaded a raw zImage to $loadaddr, since it's more than 128MB beyond the start of SDRAM, which would interfere with the kernel's CONFIG_AUTO_ZRELADDR. So, let's not do that.
The only potential fallout I could foresee from this patch is if someone has a script that loads the kernel to $loadaddr, but some other file (DTB, initrd) to a hard-coded address that the new value of $loadaddr interferes with. This seems unlikely. A user should not do that; they should either hard-code all load addresses, or use U-Boot-supplied variables for all load addresses. Equally, any fallout due to this change is trivial to fix; simply modify the load addresses in that script.
Cc: Paul Walmsley pwalmsley@nvidia.com Signed-off-by: Stephen Warren swarren@nvidia.com
Reviewed-by: Paul Walmsley pwalmsley@nvidia.com
Thanks Stephen
- Paul
----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. -----------------------------------------------------------------------------------
On 1 April 2015 at 15:40, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
As best I can tell, CONFIG_SYS_LOAD_ADDR and CONFIG_LOADADDR/$loadaddr serve essentially the same purpose. Roughly, if a command takes a load address, then CONFIG_SYS_LOAD_ADDR or $loadaddr (or both) are the default if the command-line does not specify the address. Different U-Boot commands are inconsistent re: which of the two default values they use. As such, set the two to the same value, and move the logic that does this into tegra-common-post.h so it's not duplicated. A number of other non- Tegra boards do this too.
The values chosen for these macros are no longer consistent with anything in MEM_LAYOUT_ENV_SETTINGS. Regain consistency by setting $kernel_addr_r to CONFIG_LOADADDR. Older scripts tend to use $loadaddr for the default kernel load address, whereas newer scripts and features tend to use $kernel_addr_r, along with other variables for other purposes such as DTBs and initrds. Hence, it's logical they should share the same value.
I had originally thought to make the $kernel_addr_r and CONFIG_LOADADDR have different values. This would guarantee no interference if a script used the two variables for different purposes. However, that scenario is unlikely given the semantic meaning associated with the two variables. The lowest available value is 0x90200000; see comments for MEM_LAYOUT_ENV_SETTINGS in tegra30-common-post.h for details. However, that value would be problematic for a script that loaded a raw zImage to $loadaddr, since it's more than 128MB beyond the start of SDRAM, which would interfere with the kernel's CONFIG_AUTO_ZRELADDR. So, let's not do that.
The only potential fallout I could foresee from this patch is if someone has a script that loads the kernel to $loadaddr, but some other file (DTB, initrd) to a hard-coded address that the new value of $loadaddr interferes with. This seems unlikely. A user should not do that; they should either hard-code all load addresses, or use U-Boot-supplied variables for all load addresses. Equally, any fallout due to this change is trivial to fix; simply modify the load addresses in that script.
Reviewed-by: Simon Glass
participants (3)
-
Paul Walmsley -
Simon Glass -
Stephen Warren