Remove CONFIG_SYS_{ARCH, CPU, SOC, VENDOR, BOARD}

When I posted the RFC version of Kconfig series,
I defined
CONFIG_SYS_ARCH, CONFIG_SYS_CPU, CONFIG_SYS_SOC, CONFIG_SYS_VENDOR, CONFIG_SYS_BOARD in Kconfig.

For example,
configs/harmony_defconfig was like this:
CONFIG_SPL=y
CONFIG_ARM=y
CONFIG_SYS_CPU="armv7"
CONFIG_SOC_DIR=y
CONFIG_SYS_SOC="tegra20" CONFIG_SYS_BOARD="harmony"
CONFIG_VENDOR_DIR=y
CONFIG_SYS_VENDOR="nvidia" CONFIG_SYS_CONFIG_NAME="harmony" CONFIG_BOARD_MAINTAINER="Tom Warren <[email protected]>"

But the most reviewers involved did not like that. (See http://patchwork.ozlabs.org/patch/330915/)

Instead, Daniel Schwierzeck suggested to select a board by using boolean macro: something like this, CONFIG_BOARD_HARMONY, CONFIG_VENDOR_NVIDIA, ...

I agree this is a good idea.
But string macros are used everywhere in U-Boot.

$(CPU), $(SOC), $(VENDOR), $(BOARD) here and there in makefiles and CONFIG_SYS_{ARCH, CPU, SOC, VENDOR, BOARD} in C source files.

I took an action little by little to reduce the number of such variables in makefiles
For example, these patches:
http://patchwork.ozlabs.org/patch/335202/
http://patchwork.ozlabs.org/patch/335201/

I want to do something with CONFIG_SYS_{ARCH,...} in this series.

Masahiro Yamada (3):
env: drop CONFIG_ENV_VARS_UBOOT_CONFIG support
cmd_pxe: remove SoC, ARCH path from pxe_default_path
mkconfig: Do not define CONFIG_SYS_{ARCH,CPU,SOC,VENDOR,BOARD} in
config.h.

 README                                     | 14 --------------
 arch/arm/include/asm/arch-tegra114/tegra.h |  1 +
 arch/arm/include/asm/arch-tegra124/tegra.h |  1 +
 arch/arm/include/asm/arch-tegra20/tegra.h  |  1 +
 arch/arm/include/asm/arch-tegra30/tegra.h  |  1 +
 arch/blackfin/lib/Makefile                 |  3 ---
 common/cmd_pxe.c                           |  4 ----
 include/configs/am335x_igep0033.h          |  1 -
 include/configs/apf27.h                    |  1 -
 include/configs/exynos4-dt.h               |  2 +-
 include/configs/omap4_panda.h              |  1 +
 include/configs/pcm051.h                   |  1 -
 include/configs/rpi_b.h                    |  1 -
 include/configs/s5p_goni.h                 |  1 -
 include/configs/s5pc210_universal.h        |  3 ++-
 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                    |  2 +-
 include/configs/trats2.h                   |  3 ++-
 include/env_default.h                      | 12 ------------
 mkconfig                                   |  8 --------
 23 files changed, 11 insertions(+), 54 deletions(-)

1.8.3.2

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.

---

 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(-)

index f91e044..7fd47aa 100644
@@ -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

index c17327f..8c84b7f 100644
@@ -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" \

index b10c48c..92b102a 100644
@@ -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;"
 
 

index 9af3efd..2375348 100644
@@ -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" \

index ed8b4df..d5cd912 100644
@@ -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

index 991c43e..66ee58b 100644
@@ -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 \

index 2da8871..509bd06 100644
@@ -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					\

index 721c4e6..41b1872 100644
@@ -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

index ae786cf..65914fc 100644
@@ -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 */
 
 /*

index b51400c..1b47e04 100644
@@ -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" \

index 69d69a5..e083764 100644
@@ -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. */

index 5d8bd60..29cabab 100644
@@ -78,7 +78,6 @@
 
 #define CONFIG_ENV_OVERWRITE
 
-#define CONFIG_ENV_VARS_UBOOT_CONFIG
 #define CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
 
 /* Tizen - partitions definitions */

index 53d449c..f39a70b 100644
@@ -68,7 +68,6 @@
 
 #define CONFIG_ENV_OVERWRITE
 
-#define CONFIG_ENV_VARS_UBOOT_CONFIG
 #define CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
 
 /* Tizen - partitions definitions */

index 90431be..78c2267 100644
@@ -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

1.8.3.2

Dear Masahiro,

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

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
You can only live once, but if you do it right, once is enough.

Hi Wolfgang,

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

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.

Hi Stephen,

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

Dear Masahiro,

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

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If you want strict real-time behavior, run in the real  time  schedu-
ling class.  But there are no seatbelts or airbags;  main(){for(;;);}
can hard hang your system.                          -- Bart Smaalders

Hi Wolfgang,

I agree that people would be unconfortable with this approach.

OK. Please reject this series.

Best Regards
Masahiro Yamada

Drop CONFIG_SYS_ARCH and CONFIG_SYS_SOC from the code.

---

 common/cmd_pxe.c | 4 ----
 1 file changed, 4 deletions(-)

index 3483328..4b4a954 100644
@@ -18,10 +18,6 @@
 #define MAX_TFTP_PATH_LEN 127
 
 const char *pxe_default_paths[] = {
-#ifdef CONFIG_SYS_SOC
-	"default-" CONFIG_SYS_ARCH "-" CONFIG_SYS_SOC,
-#endif
-	"default-" CONFIG_SYS_ARCH,
 	"default",
 	NULL
 };

1.8.3.2

That is clear from the diff, but you are not stating why you need this change.

Rob

This seems like something that will be annoying to people who rely on this behaviour; the patch removes functionality for no purpose.

This commit modifies mkconfig not to define CONFIG_SYS_ARCH, CONFIG_SYS_CPU, CONFIG_SYS_SOC, CONFIG_SYS_VENDOR, CONFIG_SYS_BOARD.

They are still used in some board files. Tegra family, OMAP-Panda board, some Samsung boards.

Add CONFIG_SYS_SOC, CONFIG_SYS_BOARD definition to their header files to keep the same behavior.

---

 arch/arm/include/asm/arch-tegra114/tegra.h | 1 +
 arch/arm/include/asm/arch-tegra124/tegra.h | 1 +
 arch/arm/include/asm/arch-tegra20/tegra.h  | 1 +
 arch/arm/include/asm/arch-tegra30/tegra.h  | 1 +
 arch/blackfin/lib/Makefile                 | 3 ---
 include/configs/exynos4-dt.h               | 2 +-
 include/configs/omap4_panda.h              | 1 +
 include/configs/s5pc210_universal.h        | 2 ++
 include/configs/trats.h                    | 1 +
 include/configs/trats2.h                   | 2 ++
 mkconfig                                   | 8 --------
 11 files changed, 11 insertions(+), 12 deletions(-)

index 705ca57..e6b56c6 100644
@@ -18,6 +18,7 @@
 #define _TEGRA114_H_
 
 #define CONFIG_TEGRA114
+#define CONFIG_SYS_SOC		"tegra114"
 
 #define NV_PA_SDRAM_BASE	0x80000000	/* 0x80000000 for real T114 */
 #define NV_PA_TSC_BASE		0x700F0000	/* System Counter TSC regs */

index 86ebd19..9f455a1 100644
@@ -9,6 +9,7 @@
 #define _TEGRA124_H_
 
 #define CONFIG_TEGRA124
+#define CONFIG_SYS_SOC		"tegra124"
 
 #define NV_PA_SDRAM_BASE	0x80000000
 #define NV_PA_TSC_BASE		0x700F0000	/* System Counter TSC regs */

index 6a4b40e..8d50b80 100644
@@ -9,6 +9,7 @@
 #define _TEGRA20_H_
 
 #define CONFIG_TEGRA20
+#define CONFIG_SYS_SOC		"tegra20"
 
 #define NV_PA_SDRAM_BASE	0x00000000
 

index 4ad8b1c..5ddb1c0 100644
@@ -18,6 +18,7 @@
 #define _TEGRA30_H_
 
 #define CONFIG_TEGRA30
+#define CONFIG_SYS_SOC		"tegra30"
 
 #define NV_PA_SDRAM_BASE	0x80000000	/* 0x80000000 for real T30 */
 

index 4ba7bf6..f99dfc7 100644
@@ -9,9 +9,6 @@
 # SPDX-License-Identifier:	GPL-2.0+
 #
 
-# Unnecessary.
-# Use CONFIG_SYS_BOARD instead of BFIN_BOARD_NAME
-# and delete this.
 ccflags-y += -DBFIN_BOARD_NAME='"$(BOARD)"'
 
 obj-y	+= ins.o

index 2040bf7..2063887 100644
@@ -13,7 +13,7 @@
 #define CONFIG_SAMSUNG			/* in a SAMSUNG core */
 #define CONFIG_S5P			/* S5P Family */
 #define CONFIG_EXYNOS4			/* which is in a Exynos4 Family */
-
+#define CONFIG_SYS_SOC	"exynos"
 #include <asm/arch/cpu.h>		/* get chip and board defs */
 
 #define CONFIG_ARCH_CPU_INIT

index 7378acd..4a844a1 100644
@@ -15,6 +15,7 @@
 /*
  * High Level Configuration Options
  */
+#define CONFIG_SYS_BOARD    "panda"
 
 /* USB UHH support options */
 #define CONFIG_CMD_USB

index 509bd06..ac914f9 100644
@@ -14,6 +14,8 @@
 
 #define CONFIG_SYS_PROMPT	"Universal # "	/* Monitor Command Prompt */
 
+#define CONFIG_SYS_BOARD	"universal_c210"
+
 #undef CONFIG_DEFAULT_DEVICE_TREE
 #define CONFIG_DEFAULT_DEVICE_TREE	exynos4210-universal_c210
 

index 29cabab..19160ac 100644
@@ -15,6 +15,7 @@
 #define CONFIG_SYS_PROMPT	"Trats # "	/* Monitor Command Prompt */
 
 #define CONFIG_TRATS
+#define CONFIG_SYS_BOARD	"trats"
 
 #undef CONFIG_DEFAULT_DEVICE_TREE
 #define CONFIG_DEFAULT_DEVICE_TREE	exynos4210-trats

index f39a70b..7c6221e 100644
@@ -15,6 +15,8 @@
 
 #define CONFIG_SYS_PROMPT	"Trats2 # "	/* Monitor Command Prompt */
 
+#define CONFIG_SYS_BOARD	"trats2"
+
 #undef CONFIG_DEFAULT_DEVICE_TREE
 #define CONFIG_DEFAULT_DEVICE_TREE	exynos4412-trats2
 

index cd911a9..ccdc61c 100755
@@ -171,14 +171,6 @@ for i in ${TARGETS} ; do
 	echo "#define CONFIG_${i}" >>config.h ;
 done
 
-echo "#define CONFIG_SYS_ARCH  \"${arch}\""  >> config.h
-echo "#define CONFIG_SYS_CPU   \"${cpu}\""   >> config.h
-echo "#define CONFIG_SYS_BOARD \"${board}\"" >> config.h
-
-[ "${vendor}" ] && echo "#define CONFIG_SYS_VENDOR \"${vendor}\"" >> config.h
-
-[ "${soc}"    ] && echo "#define CONFIG_SYS_SOC    \"${soc}\""    >> config.h
-
 [ "${board}"  ] && echo "#define CONFIG_BOARDDIR board/$BOARDDIR" >> config.h
 cat << EOF >> config.h
 #include <config_cmd_defaults.h>

1.8.3.2

NAK. These are required by include/env_default.h, which creates environment variables which cross-board boot.scr may use e.g. to create DTB filenames. These variables need to continue to exist.

Hi Masahiro,

For trats board:

Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group