[U-Boot] [PATCH 1/3] configs: Remove unneeded overlay_files environment variable

[U-Boot] [PATCH 1/3] configs: Remove unneeded overlay_files environment variable

[Andrew F. Davis]

The variable 'name_overlays' serves the same purpose. Remove
'overlay_files' and use 'name_overlays' everywhere.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 include/configs/am65x_evm.h | 5 ++---
 include/configs/j721e_evm.h | 6 ++----
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/include/configs/am65x_evm.h b/include/configs/am65x_evm.h
index 1415bb1b15..6072e4a48c 100644
--- a/include/configs/am65x_evm.h
+++ b/include/configs/am65x_evm.h
@@ -66,8 +66,7 @@
 #define EXTRA_ENV_AM65X_BOARD_SETTINGS					\
 	"findfdt="							\
 		"setenv name_fdt k3-am654-base-board.dtb;"		\
-		"setenv fdtfile ${name_fdt};"				\
-		"setenv overlay_files ${name_overlays}\0"		\
+		"setenv fdtfile ${name_fdt}\0"				\
 	"loadaddr=0x80080000\0"						\
 	"fdtaddr=0x82000000\0"						\
 	"overlayaddr=0x83000000\0"					\
@@ -88,7 +87,7 @@
 	"get_overlay_mmc="						\
 		"fdt address ${fdtaddr};"				\
 		"fdt resize 0x100000;"					\
-		"for overlay in $overlay_files;"			\
+		"for overlay in $name_overlays;"			\
 		"do;"							\
 		"load mmc ${bootpart} ${overlayaddr} ${bootdir}/${overlay};"	\
 		"fdt apply ${overlayaddr};"				\
diff --git a/include/configs/j721e_evm.h b/include/configs/j721e_evm.h
index 5b35e22c85..f8d4d7bdf8 100644
--- a/include/configs/j721e_evm.h
+++ b/include/configs/j721e_evm.h
@@ -60,9 +60,7 @@
 /* U-Boot general configuration */
 #define EXTRA_ENV_J721E_BOARD_SETTINGS					\
 	"default_device_tree=" CONFIG_DEFAULT_DEVICE_TREE ".dtb\0"	\
-	"findfdt="							\
-		"setenv fdtfile ${default_device_tree};"		\
-		"setenv overlay_files ${name_overlays}\0"		\
+	"findfdt=setenv fdtfile ${default_device_tree}\0"		\
 	"loadaddr=0x80080000\0"						\
 	"fdtaddr=0x82000000\0"						\
 	"overlayaddr=0x83000000\0"					\
@@ -83,7 +81,7 @@
 	"get_overlay_mmc="						\
 		"fdt address ${fdtaddr};"				\
 		"fdt resize 0x100000;"					\
-		"for overlay in $overlay_files;"			\
+		"for overlay in $name_overlays;"			\
 		"do;"							\
 		"load mmc ${bootpart} ${overlayaddr} ${bootdir}/${overlay} && "	\
 		"fdt apply ${overlayaddr};"				\
-- 
2.17.1

[U-Boot] [PATCH 2/3] configs: Rename environment variable fit_loadaddr to addr_fit

[Andrew F. Davis]

This is the first part of a larger effort I would like to propose to
unify and simplify the default set of environment variables.

When many early environment variables were named there were fewer images
being loaded, usually just a kernel. At this time names like 'loadaddr'
would suffice. Now we have more images and many more commands that act on
them, often re-using the same variable for several different uses. The
contents of a variable are also not immediately known causing one to have
to look up a chain of variables to understand what a command is actually
doing. I suggest the following.

To start, all variables containing names should be prefixed with name_
and addresses with addr_. This is like how K2 already does things and
allows for simple universal commands like:

get_fdt_nfs=nfs ${addr_fdt} /boot/${name_fdt}

Which is very clear on what is intended here and would work across all
board that using the this naming convention.

We can do this one variable at a time, start here with addr_fit.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 include/configs/k2g_evm.h            |  2 +-
 include/configs/ti_armv7_common.h    |  4 ++--
 include/configs/ti_armv7_keystone2.h | 12 ++++++------
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/configs/k2g_evm.h b/include/configs/k2g_evm.h
index 3ec5a5acf5..b39e956def 100644
--- a/include/configs/k2g_evm.h
+++ b/include/configs/k2g_evm.h
@@ -69,7 +69,7 @@
 	"run run_mon_hs; "						\
 	"run init_${boot}; "						\
 	"run get_fit_${boot}; "						\
-	"bootm ${fit_loadaddr}#${name_fdt}"
+	"bootm ${addr_fit}#${name_fdt}"
 #endif
 
 /* NAND Configuration */
diff --git a/include/configs/ti_armv7_common.h b/include/configs/ti_armv7_common.h
index 828fb1b2a5..ece329fc7d 100644
--- a/include/configs/ti_armv7_common.h
+++ b/include/configs/ti_armv7_common.h
@@ -52,9 +52,9 @@
 
 #define DEFAULT_FIT_TI_ARGS \
 	"boot_fit=0\0" \
-	"fit_loadaddr=0x90000000\0" \
+	"addr_fit=0x90000000\0" \
 	"fit_bootfile=fitImage\0" \
-	"update_to_fit=setenv loadaddr ${fit_loadaddr}; setenv bootfile ${fit_bootfile}\0" \
+	"update_to_fit=setenv loadaddr ${addr_fit}; setenv bootfile ${fit_bootfile}\0" \
 	"loadfit=run args_mmc; bootm ${loadaddr}#${fdtfile};\0" \
 
 /*
diff --git a/include/configs/ti_armv7_keystone2.h b/include/configs/ti_armv7_keystone2.h
index b44b51bbd1..401dec4493 100644
--- a/include/configs/ti_armv7_keystone2.h
+++ b/include/configs/ti_armv7_keystone2.h
@@ -240,11 +240,11 @@
 	"get_mon_net=dhcp ${addr_mon} ${tftp_root}/${name_mon}\0"	\
 	"get_mon_nfs=nfs ${addr_mon} ${nfs_root}/boot/${name_mon}\0"	\
 	"get_mon_ubi=ubifsload ${addr_mon} ${bootdir}/${name_mon}\0"	\
-	"get_fit_net=dhcp ${fit_loadaddr} ${tftp_root}"			\
+	"get_fit_net=dhcp ${addr_fit} ${tftp_root}"			\
 						"/${fit_bootfile}\0"	\
-	"get_fit_nfs=nfs ${fit_loadaddr} ${nfs_root}/boot/${fit_bootfile}\0"\
-	"get_fit_ubi=ubifsload ${fit_loadaddr} ${bootdir}/${fit_bootfile}\0"\
-	"get_fit_mmc=load mmc ${bootpart} ${fit_loadaddr} "		\
+	"get_fit_nfs=nfs ${addr_fit} ${nfs_root}/boot/${fit_bootfile}\0"\
+	"get_fit_ubi=ubifsload ${addr_fit} ${bootdir}/${fit_bootfile}\0"\
+	"get_fit_mmc=load mmc ${bootpart} ${addr_fit} "			\
 					"${bootdir}/${fit_bootfile}\0"	\
 	"get_uboot_net=dhcp ${loadaddr} ${tftp_root}/${name_uboot}\0"	\
 	"get_uboot_nfs=nfs ${loadaddr} ${nfs_root}/boot/${name_uboot}\0" \
@@ -261,7 +261,7 @@
 	"get_fdt_ramfs=dhcp ${fdtaddr} ${tftp_root}/${name_fdt}\0"	\
 	"get_kern_ramfs=dhcp ${loadaddr} ${tftp_root}/${name_kern}\0"	\
 	"get_mon_ramfs=dhcp ${addr_mon} ${tftp_root}/${name_mon}\0"	\
-	"get_fit_ramfs=dhcp ${fit_loadaddr} ${tftp_root}"		\
+	"get_fit_ramfs=dhcp ${addr_fit} ${tftp_root}"			\
 						"/${fit_bootfile}\0"	\
 	"get_fs_ramfs=dhcp ${rdaddr} ${tftp_root}/${name_fs}\0"	\
 	"get_ubi_net=dhcp ${addr_ubi} ${tftp_root}/${name_ubi}\0"	\
@@ -290,7 +290,7 @@
 	"run run_mon_hs; "						\
 	"run init_${boot}; "						\
 	"run get_fit_${boot}; "						\
-	"bootm ${fit_loadaddr}#${name_fdt}"
+	"bootm ${addr_fit}#${name_fdt}"
 #endif
 #endif
 
-- 
2.17.1

[U-Boot] [PATCH 2/3] configs: Rename environment variable fit_loadaddr to addr_fit

[Andreas Dannenberg]

Andrew,

On Mon, Aug 12, 2019 at 03:59:54PM -0400, Andrew F. Davis wrote:
> This is the first part of a larger effort I would like to propose to
> unify and simplify the default set of environment variables.
> 
> When many early environment variables were named there were fewer images
> being loaded, usually just a kernel. At this time names like 'loadaddr'
> would suffice. Now we have more images and many more commands that act on
> them, often re-using the same variable for several different uses. The
> contents of a variable are also not immediately known causing one to have
> to look up a chain of variables to understand what a command is actually
> doing. I suggest the following.
> 
> To start, all variables containing names should be prefixed with name_
> and addresses with addr_. This is like how K2 already does things and
> allows for simple universal commands like:
> 
> get_fdt_nfs=nfs ${addr_fdt} /boot/${name_fdt}
> 
> Which is very clear on what is intended here and would work across all
> board that using the this naming convention.
> 
> We can do this one variable at a time, start here with addr_fit.

I think this is a good initiative. Just looking at the TI stuff there
seems to be some good opportunity for cleanup and consolidation. One
concern is we need to ensure to not break stuff, which this series from
what I looked at and my limited testing should not (unless somebody does
some custom ENV-side loading tapping into parts of the environment as
part of some of their own ENV config they may have created locally but
there is no way really for us to check that).

Two quick suggestions:

1) The PATCH $SUBJECT lines should more narrowly define the scope in
which changes are made (e.g., "config: ti: *") to help make things
clearer when browsing the commit log,

2) A cover letter would be good to more organically entertain general
discussions around the approach.

For the series...

Acked-by: Andreas Dannenberg <dannenberg@ti.com>

> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> ---
>  include/configs/k2g_evm.h            |  2 +-
>  include/configs/ti_armv7_common.h    |  4 ++--
>  include/configs/ti_armv7_keystone2.h | 12 ++++++------
>  3 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/include/configs/k2g_evm.h b/include/configs/k2g_evm.h
> index 3ec5a5acf5..b39e956def 100644
> --- a/include/configs/k2g_evm.h
> +++ b/include/configs/k2g_evm.h
> @@ -69,7 +69,7 @@
>  	"run run_mon_hs; "						\
>  	"run init_${boot}; "						\
>  	"run get_fit_${boot}; "						\
> -	"bootm ${fit_loadaddr}#${name_fdt}"
> +	"bootm ${addr_fit}#${name_fdt}"
>  #endif
>  
>  /* NAND Configuration */
> diff --git a/include/configs/ti_armv7_common.h b/include/configs/ti_armv7_common.h
> index 828fb1b2a5..ece329fc7d 100644
> --- a/include/configs/ti_armv7_common.h
> +++ b/include/configs/ti_armv7_common.h
> @@ -52,9 +52,9 @@
>  
>  #define DEFAULT_FIT_TI_ARGS \
>  	"boot_fit=0\0" \
> -	"fit_loadaddr=0x90000000\0" \
> +	"addr_fit=0x90000000\0" \
>  	"fit_bootfile=fitImage\0" \
> -	"update_to_fit=setenv loadaddr ${fit_loadaddr}; setenv bootfile ${fit_bootfile}\0" \
> +	"update_to_fit=setenv loadaddr ${addr_fit}; setenv bootfile ${fit_bootfile}\0" \
>  	"loadfit=run args_mmc; bootm ${loadaddr}#${fdtfile};\0" \
>  
>  /*
> diff --git a/include/configs/ti_armv7_keystone2.h b/include/configs/ti_armv7_keystone2.h
> index b44b51bbd1..401dec4493 100644
> --- a/include/configs/ti_armv7_keystone2.h
> +++ b/include/configs/ti_armv7_keystone2.h
> @@ -240,11 +240,11 @@
>  	"get_mon_net=dhcp ${addr_mon} ${tftp_root}/${name_mon}\0"	\
>  	"get_mon_nfs=nfs ${addr_mon} ${nfs_root}/boot/${name_mon}\0"	\
>  	"get_mon_ubi=ubifsload ${addr_mon} ${bootdir}/${name_mon}\0"	\
> -	"get_fit_net=dhcp ${fit_loadaddr} ${tftp_root}"			\
> +	"get_fit_net=dhcp ${addr_fit} ${tftp_root}"			\
>  						"/${fit_bootfile}\0"	\
> -	"get_fit_nfs=nfs ${fit_loadaddr} ${nfs_root}/boot/${fit_bootfile}\0"\
> -	"get_fit_ubi=ubifsload ${fit_loadaddr} ${bootdir}/${fit_bootfile}\0"\
> -	"get_fit_mmc=load mmc ${bootpart} ${fit_loadaddr} "		\
> +	"get_fit_nfs=nfs ${addr_fit} ${nfs_root}/boot/${fit_bootfile}\0"\
> +	"get_fit_ubi=ubifsload ${addr_fit} ${bootdir}/${fit_bootfile}\0"\
> +	"get_fit_mmc=load mmc ${bootpart} ${addr_fit} "			\
>  					"${bootdir}/${fit_bootfile}\0"	\
>  	"get_uboot_net=dhcp ${loadaddr} ${tftp_root}/${name_uboot}\0"	\
>  	"get_uboot_nfs=nfs ${loadaddr} ${nfs_root}/boot/${name_uboot}\0" \
> @@ -261,7 +261,7 @@
>  	"get_fdt_ramfs=dhcp ${fdtaddr} ${tftp_root}/${name_fdt}\0"	\
>  	"get_kern_ramfs=dhcp ${loadaddr} ${tftp_root}/${name_kern}\0"	\
>  	"get_mon_ramfs=dhcp ${addr_mon} ${tftp_root}/${name_mon}\0"	\
> -	"get_fit_ramfs=dhcp ${fit_loadaddr} ${tftp_root}"		\
> +	"get_fit_ramfs=dhcp ${addr_fit} ${tftp_root}"			\
>  						"/${fit_bootfile}\0"	\
>  	"get_fs_ramfs=dhcp ${rdaddr} ${tftp_root}/${name_fs}\0"	\
>  	"get_ubi_net=dhcp ${addr_ubi} ${tftp_root}/${name_ubi}\0"	\
> @@ -290,7 +290,7 @@
>  	"run run_mon_hs; "						\
>  	"run init_${boot}; "						\
>  	"run get_fit_${boot}; "						\
> -	"bootm ${fit_loadaddr}#${name_fdt}"
> +	"bootm ${addr_fit}#${name_fdt}"
>  #endif
>  #endif
>  
> -- 
> 2.17.1
> 

[U-Boot] [PATCH 2/3] configs: Rename environment variable fit_loadaddr to addr_fit

[Tom Rini]

On Mon, Aug 12, 2019 at 03:59:54PM -0400, Andrew F. Davis wrote:

> This is the first part of a larger effort I would like to propose to
> unify and simplify the default set of environment variables.
> 
> When many early environment variables were named there were fewer images
> being loaded, usually just a kernel. At this time names like 'loadaddr'
> would suffice. Now we have more images and many more commands that act on
> them, often re-using the same variable for several different uses. The
> contents of a variable are also not immediately known causing one to have
> to look up a chain of variables to understand what a command is actually
> doing. I suggest the following.
> 
> To start, all variables containing names should be prefixed with name_
> and addresses with addr_. This is like how K2 already does things and
> allows for simple universal commands like:
> 
> get_fdt_nfs=nfs ${addr_fdt} /boot/${name_fdt}
> 
> Which is very clear on what is intended here and would work across all
> board that using the this naming convention.
> 
> We can do this one variable at a time, start here with addr_fit.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>
> Acked-by: Andreas Dannenberg <dannenberg@ti.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190821/90d5ac60/attachment.sig>

[U-Boot] [PATCH 3/3] configs: Rename environment variable fit_bootfile to name_fit

[Andrew F. Davis]

Like we did with 'fit_loadaddr' to 'addr_fit', the variable
'fit_bootfile' contains a name and so should be prefixed with
name_. Make this change here.

Signed-off-by: Andrew F. Davis <afd@ti.com>
---
 include/configs/ti_armv7_common.h    |  4 ++--
 include/configs/ti_armv7_keystone2.h | 13 +++++--------
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/include/configs/ti_armv7_common.h b/include/configs/ti_armv7_common.h
index ece329fc7d..2058f8de0f 100644
--- a/include/configs/ti_armv7_common.h
+++ b/include/configs/ti_armv7_common.h
@@ -53,8 +53,8 @@
 #define DEFAULT_FIT_TI_ARGS \
 	"boot_fit=0\0" \
 	"addr_fit=0x90000000\0" \
-	"fit_bootfile=fitImage\0" \
-	"update_to_fit=setenv loadaddr ${addr_fit}; setenv bootfile ${fit_bootfile}\0" \
+	"name_fit=fitImage\0" \
+	"update_to_fit=setenv loadaddr ${addr_fit}; setenv bootfile ${name_fit}\0" \
 	"loadfit=run args_mmc; bootm ${loadaddr}#${fdtfile};\0" \
 
 /*
diff --git a/include/configs/ti_armv7_keystone2.h b/include/configs/ti_armv7_keystone2.h
index 401dec4493..03753c5b52 100644
--- a/include/configs/ti_armv7_keystone2.h
+++ b/include/configs/ti_armv7_keystone2.h
@@ -240,12 +240,10 @@
 	"get_mon_net=dhcp ${addr_mon} ${tftp_root}/${name_mon}\0"	\
 	"get_mon_nfs=nfs ${addr_mon} ${nfs_root}/boot/${name_mon}\0"	\
 	"get_mon_ubi=ubifsload ${addr_mon} ${bootdir}/${name_mon}\0"	\
-	"get_fit_net=dhcp ${addr_fit} ${tftp_root}"			\
-						"/${fit_bootfile}\0"	\
-	"get_fit_nfs=nfs ${addr_fit} ${nfs_root}/boot/${fit_bootfile}\0"\
-	"get_fit_ubi=ubifsload ${addr_fit} ${bootdir}/${fit_bootfile}\0"\
-	"get_fit_mmc=load mmc ${bootpart} ${addr_fit} "			\
-					"${bootdir}/${fit_bootfile}\0"	\
+	"get_fit_net=dhcp ${addr_fit} ${tftp_root}/${name_fit}\0"	\
+	"get_fit_nfs=nfs ${addr_fit} ${nfs_root}/boot/${name_fit}\0"	\
+	"get_fit_ubi=ubifsload ${addr_fit} ${bootdir}/${name_fit}\0"	\
+	"get_fit_mmc=load mmc ${bootpart} ${addr_fit} ${bootdir}/${name_fit}\0" \
 	"get_uboot_net=dhcp ${loadaddr} ${tftp_root}/${name_uboot}\0"	\
 	"get_uboot_nfs=nfs ${loadaddr} ${nfs_root}/boot/${name_uboot}\0" \
 	"burn_uboot_spi=sf probe; sf erase 0 0x100000; "		\
@@ -261,8 +259,7 @@
 	"get_fdt_ramfs=dhcp ${fdtaddr} ${tftp_root}/${name_fdt}\0"	\
 	"get_kern_ramfs=dhcp ${loadaddr} ${tftp_root}/${name_kern}\0"	\
 	"get_mon_ramfs=dhcp ${addr_mon} ${tftp_root}/${name_mon}\0"	\
-	"get_fit_ramfs=dhcp ${addr_fit} ${tftp_root}"			\
-						"/${fit_bootfile}\0"	\
+	"get_fit_ramfs=dhcp ${addr_fit} ${tftp_root}/${name_fit}\0"	\
 	"get_fs_ramfs=dhcp ${rdaddr} ${tftp_root}/${name_fs}\0"	\
 	"get_ubi_net=dhcp ${addr_ubi} ${tftp_root}/${name_ubi}\0"	\
 	"get_ubi_nfs=nfs ${addr_ubi} ${nfs_root}/boot/${name_ubi}\0"	\
-- 
2.17.1

[U-Boot] [PATCH 3/3] configs: Rename environment variable fit_bootfile to name_fit

[Tom Rini]

On Mon, Aug 12, 2019 at 03:59:55PM -0400, Andrew F. Davis wrote:

> Like we did with 'fit_loadaddr' to 'addr_fit', the variable
> 'fit_bootfile' contains a name and so should be prefixed with
> name_. Make this change here.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190821/300ae503/attachment.sig>

[U-Boot] [PATCH 1/3] configs: Remove unneeded overlay_files environment variable

[Tom Rini]

On Mon, Aug 12, 2019 at 03:59:53PM -0400, Andrew F. Davis wrote:

> The variable 'name_overlays' serves the same purpose. Remove
> 'overlay_files' and use 'name_overlays' everywhere.
> 
> Signed-off-by: Andrew F. Davis <afd@ti.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190821/0cd3fe92/attachment.sig>