| Message ID | 1337189793-25367-1-git-send-email-swarren@wwwdotorg.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Wolfgang Denk |
| Headers | show |
On Wed, May 16, 2012 at 2:36 PM, Stephen Warren <swarren@wwwdotorg.org> wrote: > This can be useful for generic scripts. For example, rather than hard- > coding a script to ext2load tegra-harmony.dtb, it could load > ${board_soc}-${board_name}.dtb and hence not need adjustments to > run on multiple boards. I do like the idea; this moves to the right direction for generic scripts :)
Dear Stephen Warren, In message <1337189793-25367-1-git-send-email-swarren@wwwdotorg.org> you wrote: > From: Stephen Warren <swarren@nvidia.com> > > This can be useful for generic scripts. For example, rather than hard- > coding a script to ext2load tegra-harmony.dtb, it could load > ${board_soc}-${board_name}.dtb and hence not need adjustments to > run on multiple boards. > > Signed-off-by: Stephen Warren <swarren@nvidia.com> > --- > Wolfgang, does this seem like a reasonable feature? If so, can we merge > it through the Tegra tree, since the next patch enables this for Tegra. > Thanks! > > common/Makefile | 9 +++++++++ > common/env_common.c | 15 +++++++++++++++ > common/env_embedded.c | 15 +++++++++++++++ > 3 files changed, 39 insertions(+), 0 deletions(-) Please make this optional - I don't want to have this enforced on all boards, whether they want it or not. Best regards, Wolfgang Denk
On 05/21/2012 01:00 PM, Wolfgang Denk wrote: > Dear Stephen Warren, > > In message <1337189793-25367-1-git-send-email-swarren@wwwdotorg.org> you wrote: >> From: Stephen Warren <swarren@nvidia.com> >> >> This can be useful for generic scripts. For example, rather than hard- >> coding a script to ext2load tegra-harmony.dtb, it could load >> ${board_soc}-${board_name}.dtb and hence not need adjustments to >> run on multiple boards. >> >> Signed-off-by: Stephen Warren <swarren@nvidia.com> >> --- >> Wolfgang, does this seem like a reasonable feature? If so, can we merge >> it through the Tegra tree, since the next patch enables this for Tegra. >> Thanks! >> >> common/Makefile | 9 +++++++++ >> common/env_common.c | 15 +++++++++++++++ >> common/env_embedded.c | 15 +++++++++++++++ >> 3 files changed, 39 insertions(+), 0 deletions(-) > > Please make this optional - I don't want to have this enforced on all > boards, whether they want it or not. Wolfgang, Thanks for taking a look. The variables are already optional; you need to set CONFIG_ENV_VAR_* in order for common/env_*.c to add those variables to the default environment. Is that OK?
Dear Stephen Warren, In message <4FBA95F6.2020009@wwwdotorg.org> you wrote: > > > Please make this optional - I don't want to have this enforced on all > > boards, whether they want it or not. > > Thanks for taking a look. The variables are already optional; you need > to set CONFIG_ENV_VAR_* in order for common/env_*.c to add those > variables to the default environment. Is that OK? Ah, now I see what you mean. But this is awkward. Why do we need both CONFIG_ENV_VAR_<foo> and <foo> ? Also note that these BOARD_* names are kind of polluting the name space. I don't like that. Just use a single variable with an appropriate value for each of these settings. And don't pass these on the command line (which is already unreadable enough), but add it to the auto-generated config file. And make the whole feature user selectable - I think it makes no sense if you have to individually enable all the 5 settings. Hm... thinking about this, I also don't like the "board_*" prefix. What you actually have are names, i. e. it should probably read "soc_name" instead of "board_soc", etc. Best regards, Wolfgang Denk
diff --git a/common/Makefile b/common/Makefile index 6e23baa..e9fcd6a 100644 --- a/common/Makefile +++ b/common/Makefile @@ -44,6 +44,15 @@ COBJS-y += cmd_nvedit.o COBJS-y += cmd_version.o # environment +board_config_vars := \ + '-DBOARD_ARCH="$(ARCH)"' \ + '-DBOARD_CPU="$(CPU)"' \ + '-DBOARD_NAME="$(BOARD)"' \ + '-DBOARD_VENDOR="$(VENDOR)"' \ + '-DBOARD_SOC="$(SOC)"' +CFLAGS_common/env_common.o += $(board_config_vars) +CFLAGS_common/env_embedded.o += $(board_config_vars) + COBJS-y += env_common.o COBJS-$(CONFIG_ENV_IS_IN_DATAFLASH) += env_dataflash.o COBJS-$(CONFIG_ENV_IS_IN_EEPROM) += env_eeprom.o diff --git a/common/env_common.c b/common/env_common.c index c33d22d..4020ab9 100644 --- a/common/env_common.c +++ b/common/env_common.c @@ -116,6 +116,21 @@ const uchar default_environment[] = { #if defined(CONFIG_PCI_BOOTDELAY) && (CONFIG_PCI_BOOTDELAY > 0) "pcidelay=" MK_STR(CONFIG_PCI_BOOTDELAY) "\0" #endif +#ifdef CONFIG_ENV_VAR_BOARD_ARCH + "board_arch=" BOARD_ARCH "\0" +#endif +#ifdef CONFIG_ENV_VAR_BOARD_CPU + "board_cpu=" BOARD_CPU "\0" +#endif +#ifdef CONFIG_ENV_VAR_BOARD_NAME + "board_name=" BOARD_NAME "\0" +#endif +#ifdef CONFIG_ENV_VAR_BOARD_VENDOR + "board_vendor=" BOARD_VENDOR "\0" +#endif +#ifdef CONFIG_ENV_VAR_BOARD_SOC + "board_soc=" BOARD_SOC "\0" +#endif #ifdef CONFIG_EXTRA_ENV_SETTINGS CONFIG_EXTRA_ENV_SETTINGS #endif diff --git a/common/env_embedded.c b/common/env_embedded.c index 80fb29d..9ce2fc6 100644 --- a/common/env_embedded.c +++ b/common/env_embedded.c @@ -179,6 +179,21 @@ env_t environment __PPCENV__ = { #if defined(CONFIG_PCI_BOOTDELAY) && (CONFIG_PCI_BOOTDELAY > 0) "pcidelay=" MK_STR(CONFIG_PCI_BOOTDELAY) "\0" #endif +#ifdef CONFIG_ENV_VAR_BOARD_ARCH + "board_arch=" BOARD_ARCH "\0" +#endif +#ifdef CONFIG_ENV_VAR_BOARD_CPU + "board_cpu=" BOARD_CPU "\0" +#endif +#ifdef CONFIG_ENV_VAR_BOARD_NAME + "board_name=" BOARD_NAME "\0" +#endif +#ifdef CONFIG_ENV_VAR_BOARD_VENDOR + "board_vendor=" BOARD_VENDOR "\0" +#endif +#ifdef CONFIG_ENV_VAR_BOARD_SOC + "board_soc=" BOARD_SOC "\0" +#endif #ifdef CONFIG_EXTRA_ENV_SETTINGS CONFIG_EXTRA_ENV_SETTINGS #endif