| Message ID | 1398159826-29398-2-git-send-email-yamada.m@jp.panasonic.com |
|---|---|
| State | RFC |
| Delegated to: | Tom Rini |
| Headers | show |
Dear Masahiro, In message <1398159826-29398-2-git-send-email-yamada.m@jp.panasonic.com> you wrote: > CONFIG_ENV_VARS_UBOOT_CONFIG, if defined, sets environment > variables, "arch", "cpu", "board", etc. depending on > CONFIG_SYS_ARCH, CONFIG_SYS_CPU, CONFIG_SYS_BOARD, respectively. > > We are discussing the introduction of Kconfig. > In our discussion, we found boolean CONFIG macros are more useful > in Kconfig context. > > That is, > > CONFIG_ARM=y > CONFIG_CPU_ARMv7=y > CONFIG_BOARD_HARMONY=y > CONFIG_VENDOR_NVIDIA=y > > rather than > > CONFIG_SYS_ARCH="arm" > CONFIG_SYS_CPU="armv7" > CONFIG_SYS_BOARD="harmony" > CONFIG_SYS_VENDOR="nvidia" I understand your intention - but does this not mean that we lose all flexibility in assigning board and vendor names? So far, we allow any kind of names, lowercase and uppercase and mixed. Will we not lose this capability? Also, we have '-' characters in a number of board names - would this not also cause trouble? Finally, I don't see what your replacement code would be to create the set of environment settigns - and I think these are needed, as some user defined scripts are processing these? Best regards, Wolfgang Denk
Hi Wolfgang, On Tue, 22 Apr 2014 14:13:44 +0200 Wolfgang Denk <wd@denx.de> wrote: > Dear Masahiro, > > In message <1398159826-29398-2-git-send-email-yamada.m@jp.panasonic.com> you wrote: > > CONFIG_ENV_VARS_UBOOT_CONFIG, if defined, sets environment > > variables, "arch", "cpu", "board", etc. depending on > > CONFIG_SYS_ARCH, CONFIG_SYS_CPU, CONFIG_SYS_BOARD, respectively. > > > > We are discussing the introduction of Kconfig. > > In our discussion, we found boolean CONFIG macros are more useful > > in Kconfig context. > > > > That is, > > > > CONFIG_ARM=y > > CONFIG_CPU_ARMv7=y > > CONFIG_BOARD_HARMONY=y > > CONFIG_VENDOR_NVIDIA=y > > > > rather than > > > > CONFIG_SYS_ARCH="arm" > > CONFIG_SYS_CPU="armv7" > > CONFIG_SYS_BOARD="harmony" > > CONFIG_SYS_VENDOR="nvidia" > > I understand your intention - but does this not mean that we lose all > flexibility in assigning board and vendor names? So far, we allow any > kind of names, lowercase and uppercase and mixed. Will we not lose > this capability? Also, we have '-' characters in a number of board > names - would this not also cause trouble? > > Finally, I don't see what your replacement code would be to create the > set of environment settigns - and I think these are needed, as some > user defined scripts are processing these? The user who needs such environment setting can add them by using CONFIG_EXTRA_ENV_SETTINGS. For example, #define CONFIG_EXTRA_ENV_SETTINGS \ "arch=arm\0" \ "cpu=armv7\0" \ "soc=tegra20\0" I am not sure this is acceptable. Best Regards Masahiro Yamada
On 04/23/2014 06:03 AM, Masahiro Yamada wrote: > On Tue, 22 Apr 2014 14:13:44 +0200 > Wolfgang Denk <wd@denx.de> wrote: >> In message <1398159826-29398-2-git-send-email-yamada.m@jp.panasonic.com> you wrote: >>> CONFIG_ENV_VARS_UBOOT_CONFIG, if defined, sets environment >>> variables, "arch", "cpu", "board", etc. depending on >>> CONFIG_SYS_ARCH, CONFIG_SYS_CPU, CONFIG_SYS_BOARD, respectively. >>> >>> We are discussing the introduction of Kconfig. >>> In our discussion, we found boolean CONFIG macros are more useful >>> in Kconfig context. >>> >>> That is, >>> >>> CONFIG_ARM=y >>> CONFIG_CPU_ARMv7=y >>> CONFIG_BOARD_HARMONY=y >>> CONFIG_VENDOR_NVIDIA=y >>> >>> rather than >>> >>> CONFIG_SYS_ARCH="arm" >>> CONFIG_SYS_CPU="armv7" >>> CONFIG_SYS_BOARD="harmony" >>> CONFIG_SYS_VENDOR="nvidia" >> >> I understand your intention - but does this not mean that we lose all >> flexibility in assigning board and vendor names? So far, we allow any >> kind of names, lowercase and uppercase and mixed. Will we not lose >> this capability? Also, we have '-' characters in a number of board >> names - would this not also cause trouble? >> >> Finally, I don't see what your replacement code would be to create the >> set of environment settigns - and I think these are needed, as some >> user defined scripts are processing these? > > The user who needs such environment setting can > add them by using CONFIG_EXTRA_ENV_SETTINGS. > > For example, > > #define CONFIG_EXTRA_ENV_SETTINGS \ > "arch=arm\0" \ > "cpu=armv7\0" \ > "soc=tegra20\0" > > I am not sure this is acceptable. Right now, we get the values set up automatically for free. It seems like a regression to force the board maintainer to set these all up manually instead. Kconfig supports string variables. The Linux kernel stores the ARM debug_ll include filename in one for example. Perhaps using that technique would resolve the issue.
Dear Masahiro, In message <20140423210335.18EE.AA925319@jp.panasonic.com> you wrote: > > > Finally, I don't see what your replacement code would be to create the > > set of environment settigns - and I think these are needed, as some > > user defined scripts are processing these? > > The user who needs such environment setting can > add them by using CONFIG_EXTRA_ENV_SETTINGS. > > For example, > > #define CONFIG_EXTRA_ENV_SETTINGS \ > "arch=arm\0" \ > "cpu=armv7\0" \ > "soc=tegra20\0" > > I am not sure this is acceptable. Neither am I. What I can see is that this is less flexible - as is, we can easily derive these names from the make target name. Your implementation would either be static, or require #ifdefs ? Best regards, Wolfgang Denk
Hi Wolfgang, On Wed, 23 Apr 2014 21:13:44 +0200 Wolfgang Denk <wd@denx.de> wrote: > Dear Masahiro, > > In message <20140423210335.18EE.AA925319@jp.panasonic.com> you wrote: > > > > > Finally, I don't see what your replacement code would be to create the > > > set of environment settigns - and I think these are needed, as some > > > user defined scripts are processing these? > > > > The user who needs such environment setting can > > add them by using CONFIG_EXTRA_ENV_SETTINGS. > > > > For example, > > > > #define CONFIG_EXTRA_ENV_SETTINGS \ > > "arch=arm\0" \ > > "cpu=armv7\0" \ > > "soc=tegra20\0" > > > > I am not sure this is acceptable. > > Neither am I. What I can see is that this is less flexible - as is, > we can easily derive these names from the make target name. Your > implementation would either be static, or require #ifdefs ? I agree that people would be unconfortable with this approach. OK. Please reject this series. Best Regards Masahiro Yamada
Hi Stephen, On Wed, 23 Apr 2014 10:08:49 -0600 Stephen Warren <swarren@wwwdotorg.org> wrote: > On 04/23/2014 06:03 AM, Masahiro Yamada wrote: > > On Tue, 22 Apr 2014 14:13:44 +0200 > > Wolfgang Denk <wd@denx.de> wrote: > >> In message <1398159826-29398-2-git-send-email-yamada.m@jp.panasonic.com> you wrote: > >>> CONFIG_ENV_VARS_UBOOT_CONFIG, if defined, sets environment > >>> variables, "arch", "cpu", "board", etc. depending on > >>> CONFIG_SYS_ARCH, CONFIG_SYS_CPU, CONFIG_SYS_BOARD, respectively. > >>> > >>> We are discussing the introduction of Kconfig. > >>> In our discussion, we found boolean CONFIG macros are more useful > >>> in Kconfig context. > >>> > >>> That is, > >>> > >>> CONFIG_ARM=y > >>> CONFIG_CPU_ARMv7=y > >>> CONFIG_BOARD_HARMONY=y > >>> CONFIG_VENDOR_NVIDIA=y > >>> > >>> rather than > >>> > >>> CONFIG_SYS_ARCH="arm" > >>> CONFIG_SYS_CPU="armv7" > >>> CONFIG_SYS_BOARD="harmony" > >>> CONFIG_SYS_VENDOR="nvidia" > >> > >> I understand your intention - but does this not mean that we lose all > >> flexibility in assigning board and vendor names? So far, we allow any > >> kind of names, lowercase and uppercase and mixed. Will we not lose > >> this capability? Also, we have '-' characters in a number of board > >> names - would this not also cause trouble? > >> > >> Finally, I don't see what your replacement code would be to create the > >> set of environment settigns - and I think these are needed, as some > >> user defined scripts are processing these? > > > > The user who needs such environment setting can > > add them by using CONFIG_EXTRA_ENV_SETTINGS. > > > > For example, > > > > #define CONFIG_EXTRA_ENV_SETTINGS \ > > "arch=arm\0" \ > > "cpu=armv7\0" \ > > "soc=tegra20\0" > > > > I am not sure this is acceptable. > > Right now, we get the values set up automatically for free. It seems > like a regression to force the board maintainer to set these all up > manually instead. > > Kconfig supports string variables. The Linux kernel stores the ARM > debug_ll include filename in one for example. Perhaps using that > technique would resolve the issue. I think you mentioned the following config: config DEBUG_LL_INCLUDE string default "debug/8250.S" if DEBUG_LL_UART_8250 || DEBUG_UART_8250 default "debug/pl01x.S" if DEBUG_LL_UART_PL01X || DEBUG_UART_PL01X default "debug/exynos.S" if DEBUG_EXYNOS_UART default "debug/efm32.S" if DEBUG_LL_UART_EFM32 default "debug/icedcc.S" if DEBUG_ICEDCC default "debug/imx.S" if DEBUG_IMX1_UART || \ DEBUG_IMX25_UART || \ DEBUG_IMX21_IMX27_UART || \ DEBUG_IMX31_UART || \ DEBUG_IMX35_UART || \ DEBUG_IMX50_UART || \ DEBUG_IMX51_UART || \ DEBUG_IMX53_UART ||\ DEBUG_IMX6Q_UART || \ DEBUG_IMX6SL_UART default "debug/msm.S" if DEBUG_MSM_UART default "debug/omap2plus.S" if DEBUG_OMAP2PLUS_UART default "debug/sirf.S" if DEBUG_SIRFPRIMA2_UART1 || DEBUG_SIRFMARCO_UART1 default "debug/sti.S" if DEBUG_STI_UART default "debug/tegra.S" if DEBUG_TEGRA_UART default "debug/ux500.S" if DEBUG_UX500_UART default "debug/vexpress.S" if DEBUG_VEXPRESS_UART0_DETECT default "debug/vf.S" if DEBUG_VF_UART default "debug/vt8500.S" if DEBUG_VT8500_UART0 default "debug/zynq.S" if DEBUG_ZYNQ_UART0 || DEBUG_ZYNQ_UART1 default "mach/debug-macro.S" Actually I did this in my first RFC series. http://patchwork.ozlabs.org/patch/330796/ like this: config SYS_CPU default "mcf5227x" if MCF5227x default "mcf523x" if MCF523x default "mcf52x2" if MCF52x2 || MCF520x default "mcf532x" if MCF532x || MCF5301x default "mcf5445x" if MCF5441x || MCF5445x default "mcf547x_8x" if MCF547x_8x I think it works for sereval dozens of strings. But I hesitate to apply the same approach to CONFIG_SYS_BOARD. We have already hundreds of boards. It would be very dirty. Best Regards Masahiro Yamada
diff --git a/README b/README index f91e044..7fd47aa 100644 --- a/README +++ b/README @@ -2740,20 +2740,6 @@ CBFS (Coreboot Filesystem) support the environment like the "source" command or the boot command first. - CONFIG_ENV_VARS_UBOOT_CONFIG - - Define this in order to add variables describing the - U-Boot build configuration to the default environment. - These will be named arch, cpu, board, vendor, and soc. - - Enabling this option will cause the following to be defined: - - - CONFIG_SYS_ARCH - - CONFIG_SYS_CPU - - CONFIG_SYS_BOARD - - CONFIG_SYS_VENDOR - - CONFIG_SYS_SOC - CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG Define this in order to add variables describing certain diff --git a/include/configs/am335x_igep0033.h b/include/configs/am335x_igep0033.h index c17327f..8c84b7f 100644 --- a/include/configs/am335x_igep0033.h +++ b/include/configs/am335x_igep0033.h @@ -64,7 +64,6 @@ #define CONFIG_UBIFS_SILENCE_MSG #define CONFIG_BOOTDELAY 1 /* negative for no autoboot */ -#define CONFIG_ENV_VARS_UBOOT_CONFIG #define CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG #define CONFIG_EXTRA_ENV_SETTINGS \ "loadaddr=0x80F80000\0" \ diff --git a/include/configs/apf27.h b/include/configs/apf27.h index b10c48c..92b102a 100644 --- a/include/configs/apf27.h +++ b/include/configs/apf27.h @@ -157,7 +157,6 @@ #define CONFIG_CMDLINE_EDITING #define CONFIG_SYS_HUSH_PARSER /* enable the "hush" shell */ #define CONFIG_SYS_PROMPT_HUSH_PS2 "> " /* secondary prompt string */ -#define CONFIG_ENV_VARS_UBOOT_CONFIG #define CONFIG_PREBOOT "run check_flash check_env;" diff --git a/include/configs/pcm051.h b/include/configs/pcm051.h index 9af3efd..2375348 100644 --- a/include/configs/pcm051.h +++ b/include/configs/pcm051.h @@ -47,7 +47,6 @@ /* set to negative value for no autoboot */ #define CONFIG_BOOTDELAY 1 -#define CONFIG_ENV_VARS_UBOOT_CONFIG #define CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG #define CONFIG_EXTRA_ENV_SETTINGS \ "loadaddr=0x80007fc0\0" \ diff --git a/include/configs/rpi_b.h b/include/configs/rpi_b.h index ed8b4df..d5cd912 100644 --- a/include/configs/rpi_b.h +++ b/include/configs/rpi_b.h @@ -91,7 +91,6 @@ /* Environment */ #define CONFIG_ENV_SIZE SZ_16K #define CONFIG_ENV_IS_NOWHERE -#define CONFIG_ENV_VARS_UBOOT_CONFIG #define CONFIG_SYS_LOAD_ADDR 0x1000000 #define CONFIG_CONSOLE_MUX #define CONFIG_SYS_CONSOLE_IS_IN_ENV diff --git a/include/configs/s5p_goni.h b/include/configs/s5p_goni.h index 991c43e..66ee58b 100644 --- a/include/configs/s5p_goni.h +++ b/include/configs/s5p_goni.h @@ -119,7 +119,6 @@ #define CONFIG_ENV_OVERWRITE #define CONFIG_SYS_CONSOLE_IS_IN_ENV -#define CONFIG_ENV_VARS_UBOOT_CONFIG #define CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG #define CONFIG_EXTRA_ENV_SETTINGS \ CONFIG_UPDATEB \ diff --git a/include/configs/s5pc210_universal.h b/include/configs/s5pc210_universal.h index 2da8871..509bd06 100644 --- a/include/configs/s5pc210_universal.h +++ b/include/configs/s5pc210_universal.h @@ -99,7 +99,6 @@ #define CONFIG_ENV_COMMON_BOOT "${console} ${meminfo}" -#define CONFIG_ENV_VARS_UBOOT_CONFIG #define CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG #define CONFIG_EXTRA_ENV_SETTINGS \ diff --git a/include/configs/siemens-am33x-common.h b/include/configs/siemens-am33x-common.h index 721c4e6..41b1872 100644 --- a/include/configs/siemens-am33x-common.h +++ b/include/configs/siemens-am33x-common.h @@ -46,7 +46,6 @@ #define CONFIG_CMD_ECHO #define CONFIG_CMD_CACHE -#define CONFIG_ENV_VARS_UBOOT_CONFIG #ifndef CONFIG_SPL_BUILD #define CONFIG_ROOTPATH "/opt/eldk" #endif diff --git a/include/configs/tegra-common.h b/include/configs/tegra-common.h index ae786cf..65914fc 100644 --- a/include/configs/tegra-common.h +++ b/include/configs/tegra-common.h @@ -31,7 +31,6 @@ #define CONFIG_CMDLINE_TAG /* enable passing of ATAGs */ /* Environment */ -#define CONFIG_ENV_VARS_UBOOT_CONFIG #define CONFIG_ENV_SIZE 0x2000 /* Total Size Environment */ /* diff --git a/include/configs/ti814x_evm.h b/include/configs/ti814x_evm.h index b51400c..1b47e04 100644 --- a/include/configs/ti814x_evm.h +++ b/include/configs/ti814x_evm.h @@ -44,7 +44,6 @@ #define CONFIG_VERSION_VARIABLE #define CONFIG_BOOTDELAY 1 /* negative for no autoboot */ -#define CONFIG_ENV_VARS_UBOOT_CONFIG #define CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG #define CONFIG_EXTRA_ENV_SETTINGS \ "loadaddr=0x80200000\0" \ diff --git a/include/configs/ti_armv7_common.h b/include/configs/ti_armv7_common.h index 69d69a5..e083764 100644 --- a/include/configs/ti_armv7_common.h +++ b/include/configs/ti_armv7_common.h @@ -109,7 +109,6 @@ #define CONFIG_SYS_PROMPT "U-Boot# " #define CONFIG_SYS_CONSOLE_INFO_QUIET #define CONFIG_BAUDRATE 115200 -#define CONFIG_ENV_VARS_UBOOT_CONFIG /* Strongly encouraged */ #define CONFIG_ENV_OVERWRITE /* Overwrite ethaddr / serial# */ /* As stated above, the following choices are optional. */ diff --git a/include/configs/trats.h b/include/configs/trats.h index 5d8bd60..29cabab 100644 --- a/include/configs/trats.h +++ b/include/configs/trats.h @@ -78,7 +78,6 @@ #define CONFIG_ENV_OVERWRITE -#define CONFIG_ENV_VARS_UBOOT_CONFIG #define CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG /* Tizen - partitions definitions */ diff --git a/include/configs/trats2.h b/include/configs/trats2.h index 53d449c..f39a70b 100644 --- a/include/configs/trats2.h +++ b/include/configs/trats2.h @@ -68,7 +68,6 @@ #define CONFIG_ENV_OVERWRITE -#define CONFIG_ENV_VARS_UBOOT_CONFIG #define CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG /* Tizen - partitions definitions */ diff --git a/include/env_default.h b/include/env_default.h index 90431be..78c2267 100644 --- a/include/env_default.h +++ b/include/env_default.h @@ -106,18 +106,6 @@ const uchar default_environment[] = { #if defined(CONFIG_PCI_BOOTDELAY) && (CONFIG_PCI_BOOTDELAY > 0) "pcidelay=" __stringify(CONFIG_PCI_BOOTDELAY)"\0" #endif -#ifdef CONFIG_ENV_VARS_UBOOT_CONFIG - "arch=" CONFIG_SYS_ARCH "\0" - "cpu=" CONFIG_SYS_CPU "\0" - "board=" CONFIG_SYS_BOARD "\0" - "board_name=" CONFIG_SYS_BOARD "\0" -#ifdef CONFIG_SYS_VENDOR - "vendor=" CONFIG_SYS_VENDOR "\0" -#endif -#ifdef CONFIG_SYS_SOC - "soc=" CONFIG_SYS_SOC "\0" -#endif -#endif #ifdef CONFIG_EXTRA_ENV_SETTINGS CONFIG_EXTRA_ENV_SETTINGS #endif
CONFIG_ENV_VARS_UBOOT_CONFIG, if defined, sets environment variables, "arch", "cpu", "board", etc. depending on CONFIG_SYS_ARCH, CONFIG_SYS_CPU, CONFIG_SYS_BOARD, respectively. We are discussing the introduction of Kconfig. In our discussion, we found boolean CONFIG macros are more useful in Kconfig context. That is, CONFIG_ARM=y CONFIG_CPU_ARMv7=y CONFIG_BOARD_HARMONY=y CONFIG_VENDOR_NVIDIA=y rather than CONFIG_SYS_ARCH="arm" CONFIG_SYS_CPU="armv7" CONFIG_SYS_BOARD="harmony" CONFIG_SYS_VENDOR="nvidia" Using CONFIG_SYS_ARCH, CONFIG_SYS_CPU, etc. will be an obstacle for our future refactoring. Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com> --- README | 14 -------------- include/configs/am335x_igep0033.h | 1 - include/configs/apf27.h | 1 - include/configs/pcm051.h | 1 - include/configs/rpi_b.h | 1 - include/configs/s5p_goni.h | 1 - include/configs/s5pc210_universal.h | 1 - include/configs/siemens-am33x-common.h | 1 - include/configs/tegra-common.h | 1 - include/configs/ti814x_evm.h | 1 - include/configs/ti_armv7_common.h | 1 - include/configs/trats.h | 1 - include/configs/trats2.h | 1 - include/env_default.h | 12 ------------ 14 files changed, 38 deletions(-)