[U-Boot] [PATCH] ARM: tegra: CONFIG_{SYS_, }LOAD{_, }ADDR rationalization

[U-Boot] [PATCH] ARM: tegra: CONFIG_{SYS_, }LOAD{_, }ADDR rationalization

[Stephen Warren]

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"
 
-- 
1.9.1

[U-Boot] [PATCH] ARM: tegra: CONFIG_{SYS_, }LOAD{_, }ADDR rationalization

[Paul Walmsley]

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.
-----------------------------------------------------------------------------------

[U-Boot] [PATCH] ARM: tegra: CONFIG_{SYS_, }LOAD{_, }ADDR rationalization

[Simon Glass]

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