[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 at ti.com>

> 
> Signed-off-by: Andrew F. Davis <afd at 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
>