[U-Boot] [patch] socfpga: move configuration options to config file

Setting configuration options in header file leads to incosistency between .config user sees, and .config he has. What is worse, a lot of compile warnings is presented for any such config option user sets in .config.
Signed-off-by: Pavel Machek pavel@denx.de
diff --git a/configs/socfpga_cyclone5_defconfig b/configs/socfpga_cyclone5_defconfig index 0ebfbfc..762b937 100644 --- a/configs/socfpga_cyclone5_defconfig +++ b/configs/socfpga_cyclone5_defconfig @@ -6,3 +6,19 @@ CONFIG_DEFAULT_DEVICE_TREE="socfpga_cyclone5_socdk" CONFIG_DM=y CONFIG_DM_SPI=y CONFIG_DM_SPI_FLASH=y +CONFIG_CMD_ASKENV=y +CONFIG_CMD_BOOTZ=y +CONFIG_CMD_CACHE=y +CONFIG_CMD_DFU=y +CONFIG_CMD_DHCP=y +CONFIG_CMD_EXT4=y +CONFIG_CMD_EXT4_WRITE=y +CONFIG_CMD_FAT=y +CONFIG_CMD_FS_GENERIC=y +CONFIG_CMD_GREPENV=y +CONFIG_CMD_MII=y +CONFIG_CMD_MMC=y +CONFIG_CMD_PING=y +CONFIG_CMD_SETEXPR=y +CONFIG_CMD_USB=y +CONFIG_CMD_USB_MASS_STORAGE=y diff --git a/include/configs/socfpga_cyclone5.h b/include/configs/socfpga_cyclone5.h index 2e3a8b6..171ddf4 100644 --- a/include/configs/socfpga_cyclone5.h +++ b/include/configs/socfpga_cyclone5.h @@ -18,24 +18,6 @@ #define CONFIG_FAT_WRITE #define CONFIG_HW_WATCHDOG
-#define CONFIG_CMD_ASKENV -#define CONFIG_CMD_BOOTZ -#define CONFIG_CMD_CACHE -#define CONFIG_CMD_DFU -#define CONFIG_CMD_DHCP -#define CONFIG_CMD_EXT4 -#define CONFIG_CMD_EXT4_WRITE -#define CONFIG_CMD_FAT -#define CONFIG_CMD_FPGA -#define CONFIG_CMD_FS_GENERIC -#define CONFIG_CMD_GREPENV -#define CONFIG_CMD_MII -#define CONFIG_CMD_MMC -#define CONFIG_CMD_NET -#define CONFIG_CMD_PING -#define CONFIG_CMD_SETEXPR -#define CONFIG_CMD_USB -#define CONFIG_CMD_USB_MASS_STORAGE
#define CONFIG_REGEX /* Enable regular expression support */

On Monday, April 20, 2015 at 02:30:48 PM, Pavel Machek wrote:
Setting configuration options in header file leads to incosistency between .config user sees, and .config he has. What is worse, a lot of compile warnings is presented for any such config option user sets in .config.
Can you please elaborate on such warnings ?
Best regards, Marek Vasut

On Mon 2015-04-20 21:23:23, Marek Vasut wrote:
On Monday, April 20, 2015 at 02:30:48 PM, Pavel Machek wrote:
Setting configuration options in header file leads to incosistency between .config user sees, and .config he has. What is worse, a lot of compile warnings is presented for any such config option user sets in .config.
Can you please elaborate on such warnings ?
Lets remove CONFIG_CMD_RUN from .config, then select it:
run (CMD_RUN) [N/y/?] (NEW) y
Now you warning for most C files:
CC arch/arm/lib/asm-offsets.s In file included from include/configs/socfpga_cyclone5.h:16:0, from include/config.h:5, from include/common.h:18, from arch/arm/lib/asm-offsets.c:15: include/config_cmd_default.h:38:0: warning: "CONFIG_CMD_RUN" redefined [enabled by default] #define CONFIG_CMD_RUN /* run command in env variable */ ^ In file included from ././include/linux/kconfig.h:4:0, from <command-line>:0: include/generated/autoconf.h:35:0: note: this is the location of the previous definition #define CONFIG_CMD_RUN 1 ^ In file included from include/configs/socfpga_cyclone5.h:16:0, from include/config.h:5, from include/common.h:18, from lib/asm-offsets.c:15: include/config_cmd_default.h:38:0: warning: "CONFIG_CMD_RUN" redefined [enabled by default] #define CONFIG_CMD_RUN /* run command in env variable */ ^ In file included from ././include/linux/kconfig.h:4:0, from <command-line>:0: include/generated/autoconf.h:35:0: note: this is the location of the previous definition #define CONFIG_CMD_RUN 1 ^ CHK include/generated/asm-offsets.h CHK include/generated/generic-asm-offsets.h LDS u-boot.lds In file included from include/configs/socfpga_cyclone5.h:16:0, from include/config.h:5, from arch/arm/cpu/u-boot.lds:10: include/config_cmd_default.h:38:0: warning: "CONFIG_CMD_RUN" redefined [enabled by default] #define CONFIG_CMD_RUN /* run command in env variable */ ^ In file included from ././include/linux/kconfig.h:4:0, from <command-line>:0: include/generated/autoconf.h:35:0: note: this is the location of the previous definition #define CONFIG_CMD_RUN 1 ^ HOSTCC tools/mkenvimage.o Pavel

On Monday, April 20, 2015 at 10:27:02 PM, Pavel Machek wrote:
On Mon 2015-04-20 21:23:23, Marek Vasut wrote:
On Monday, April 20, 2015 at 02:30:48 PM, Pavel Machek wrote:
Setting configuration options in header file leads to incosistency between .config user sees, and .config he has. What is worse, a lot of compile warnings is presented for any such config option user sets in .config.
Can you please elaborate on such warnings ?
Lets remove CONFIG_CMD_RUN from .config, then select it:
run (CMD_RUN) [N/y/?] (NEW) y
Now you warning for most C files:
CC arch/arm/lib/asm-offsets.s In file included from include/configs/socfpga_cyclone5.h:16:0, from include/config.h:5, from include/common.h:18, from arch/arm/lib/asm-offsets.c:15: include/config_cmd_default.h:38:0: warning: "CONFIG_CMD_RUN" redefined [enabled by default] #define CONFIG_CMD_RUN /* run command in env variable */ ^ In file included from ././include/linux/kconfig.h:4:0, from <command-line>:0: include/generated/autoconf.h:35:0: note: this is
That meant CONFIG_CMD_RUN is defined by default, yes? In which case, this patch would just paper over some bug (?) instead of fixing the root cause ? The correct fix would probably be to zap those macros, which are defined by default from the socfpga_cyclone5.h file, no ?
Best regards, Marek Vasut

On Mon 2015-04-20 23:32:33, Marek Vasut wrote:
On Monday, April 20, 2015 at 10:27:02 PM, Pavel Machek wrote:
On Mon 2015-04-20 21:23:23, Marek Vasut wrote:
On Monday, April 20, 2015 at 02:30:48 PM, Pavel Machek wrote:
Setting configuration options in header file leads to incosistency between .config user sees, and .config he has. What is worse, a lot of compile warnings is presented for any such config option user sets in .config.
Can you please elaborate on such warnings ?
Lets remove CONFIG_CMD_RUN from .config, then select it:
run (CMD_RUN) [N/y/?] (NEW) y
Now you warning for most C files:
CC arch/arm/lib/asm-offsets.s In file included from include/configs/socfpga_cyclone5.h:16:0, from include/config.h:5, from include/common.h:18, from arch/arm/lib/asm-offsets.c:15: include/config_cmd_default.h:38:0: warning: "CONFIG_CMD_RUN" redefined [enabled by default] #define CONFIG_CMD_RUN /* run command in env variable */ ^ In file included from ././include/linux/kconfig.h:4:0, from <command-line>:0: include/generated/autoconf.h:35:0: note: this is
That meant CONFIG_CMD_RUN is defined by default, yes? In which case, this patch would just paper over some bug (?) instead of fixing the root cause ? The correct fix would probably be to zap those macros, which are defined by default from the socfpga_cyclone5.h file, no ?
CONFIG_CMD_RUN is defined in socfpga_cyclone5.h, but it is set to N by .config. Take a look. If you set it to Y, you'll get the ugly warnings. Try that.
Apply the patch. See that .config now corresponds to real configuration and warnings are gone. Pavel

On Monday, April 20, 2015 at 11:54:34 PM, Pavel Machek wrote:
On Mon 2015-04-20 23:32:33, Marek Vasut wrote:
On Monday, April 20, 2015 at 10:27:02 PM, Pavel Machek wrote:
On Mon 2015-04-20 21:23:23, Marek Vasut wrote:
On Monday, April 20, 2015 at 02:30:48 PM, Pavel Machek wrote:
Setting configuration options in header file leads to incosistency between .config user sees, and .config he has. What is worse, a lot of compile warnings is presented for any such config option user sets in .config.
Can you please elaborate on such warnings ?
Lets remove CONFIG_CMD_RUN from .config, then select it:
run (CMD_RUN) [N/y/?] (NEW) y
Now you warning for most C files: CC arch/arm/lib/asm-offsets.s In file included from include/configs/socfpga_cyclone5.h:16:0,
from include/config.h:5, from include/common.h:18, from
arch/arm/lib/asm-offsets.c:15: include/config_cmd_default.h:38:0: warning: "CONFIG_CMD_RUN" redefined [enabled by default]
#define CONFIG_CMD_RUN /* run command in env variable */
^ In file included from ././include/linux/kconfig.h:4:0, from <command-line>:0: include/generated/autoconf.h:35:0: note: this is
That meant CONFIG_CMD_RUN is defined by default, yes? In which case, this patch would just paper over some bug (?) instead of fixing the root cause ? The correct fix would probably be to zap those macros, which are defined by default from the socfpga_cyclone5.h file, no ?
CONFIG_CMD_RUN is defined in socfpga_cyclone5.h, but it is set to N by .config. Take a look. If you set it to Y, you'll get the ugly warnings. Try that.
I also looked into include/config_cmd_default.h , where the CONFIG_CMD_RUN is defined. It should therefore be safe to remove CONFIG_CMD_RUN from socfpga_cyclone5.h .
Apply the patch. See that .config now corresponds to real configuration and warnings are gone.
Pavel
Best regards, Marek Vasut

2015-04-21 6:32 GMT+09:00 Marek Vasut marex@denx.de:
On Monday, April 20, 2015 at 10:27:02 PM, Pavel Machek wrote:
On Mon 2015-04-20 21:23:23, Marek Vasut wrote:
On Monday, April 20, 2015 at 02:30:48 PM, Pavel Machek wrote:
Setting configuration options in header file leads to incosistency between .config user sees, and .config he has. What is worse, a lot of compile warnings is presented for any such config option user sets in .config.
Can you please elaborate on such warnings ?
Lets remove CONFIG_CMD_RUN from .config, then select it:
run (CMD_RUN) [N/y/?] (NEW) y
Now you warning for most C files:
CC arch/arm/lib/asm-offsets.s In file included from include/configs/socfpga_cyclone5.h:16:0, from include/config.h:5, from include/common.h:18, from arch/arm/lib/asm-offsets.c:15: include/config_cmd_default.h:38:0: warning: "CONFIG_CMD_RUN" redefined [enabled by default] #define CONFIG_CMD_RUN /* run command in env variable */ ^ In file included from ././include/linux/kconfig.h:4:0, from <command-line>:0: include/generated/autoconf.h:35:0: note: this is
That meant CONFIG_CMD_RUN is defined by default, yes? In which case, this patch would just paper over some bug (?) instead of fixing the root cause ? The correct fix would probably be to zap those macros, which are defined by default from the socfpga_cyclone5.h file, no ?
We usually define boolean macros without values, like
#define CONFIG_CMD_ASKENV #define CONFIG_CMD_BOOTZ #define CONFIG_CMD_CACHE
On the other hand, Kconfig define boolean macros as 1 (see include/generated/autoconf.h)
That is why the compiler spits tons of warnings.

2015-04-20 21:30 GMT+09:00 Pavel Machek pavel@denx.de:
Setting configuration options in header file leads to incosistency between .config user sees, and .config he has. What is worse, a lot of compile warnings is presented for any such config option user sets in .config.
Signed-off-by: Pavel Machek pavel@denx.de
diff --git a/configs/socfpga_cyclone5_defconfig b/configs/socfpga_cyclone5_defconfig index 0ebfbfc..762b937 100644 --- a/configs/socfpga_cyclone5_defconfig +++ b/configs/socfpga_cyclone5_defconfig @@ -6,3 +6,19 @@ CONFIG_DEFAULT_DEVICE_TREE="socfpga_cyclone5_socdk" CONFIG_DM=y CONFIG_DM_SPI=y CONFIG_DM_SPI_FLASH=y +CONFIG_CMD_ASKENV=y +CONFIG_CMD_BOOTZ=y +CONFIG_CMD_CACHE=y +CONFIG_CMD_DFU=y +CONFIG_CMD_DHCP=y +CONFIG_CMD_EXT4=y +CONFIG_CMD_EXT4_WRITE=y +CONFIG_CMD_FAT=y +CONFIG_CMD_FS_GENERIC=y +CONFIG_CMD_GREPENV=y +CONFIG_CMD_MII=y +CONFIG_CMD_MMC=y +CONFIG_CMD_PING=y +CONFIG_CMD_SETEXPR=y +CONFIG_CMD_USB=y +CONFIG_CMD_USB_MASS_STORAGE=y
You need to add these to common/Kconfig as well as to your defconfig.
Otherwise, they do not appear in the .config file.
There exist CMD_RUN, CMD_PING, CMD_USB in common/Kconfig, but most of the others are missing.

Setting configuration options in header file leads to incosistency between .config user sees, and .config he has. What is worse, a lot of compile warnings is presented for any such config option user sets in .config.
Signed-off-by: Pavel Machek pavel@denx.de
--- v2:
You need to add these to common/Kconfig as well as to your defconfig.
Otherwise, they do not appear in the .config file.
There exist CMD_RUN, CMD_PING, CMD_USB in common/Kconfig, but most of the others are missing.
Ok, I did conversion only for those present in Kconfig.
diff --git a/configs/socfpga_cyclone5_defconfig b/configs/socfpga_cyclone5_defconfig index 0ebfbfc..5cfc56c 100644 --- a/configs/socfpga_cyclone5_defconfig +++ b/configs/socfpga_cyclone5_defconfig @@ -6,3 +6,8 @@ CONFIG_DEFAULT_DEVICE_TREE="socfpga_cyclone5_socdk" CONFIG_DM=y CONFIG_DM_SPI=y CONFIG_DM_SPI_FLASH=y +CONFIG_CMD_DHCP=y +CONFIG_CMD_FPGA=y +CONFIG_CMD_NET=y +CONFIG_CMD_PING=y +CONFIG_CMD_USB=y diff --git a/configs/socfpga_socrates_defconfig b/configs/socfpga_socrates_defconfig index 873b721..0cde0e9 100644 --- a/configs/socfpga_socrates_defconfig +++ b/configs/socfpga_socrates_defconfig @@ -6,3 +6,8 @@ CONFIG_DEFAULT_DEVICE_TREE="socfpga_cyclone5_socrates" CONFIG_DM=y CONFIG_DM_SPI=y CONFIG_DM_SPI_FLASH=y +CONFIG_CMD_DHCP=y +CONFIG_CMD_FPGA=y +CONFIG_CMD_NET=y +CONFIG_CMD_PING=y +CONFIG_CMD_USB=y diff --git a/include/configs/socfpga_cyclone5.h b/include/configs/socfpga_cyclone5.h index 2e3a8b6..93a0572 100644 --- a/include/configs/socfpga_cyclone5.h +++ b/include/configs/socfpga_cyclone5.h @@ -22,19 +22,14 @@ #define CONFIG_CMD_BOOTZ #define CONFIG_CMD_CACHE #define CONFIG_CMD_DFU -#define CONFIG_CMD_DHCP #define CONFIG_CMD_EXT4 #define CONFIG_CMD_EXT4_WRITE #define CONFIG_CMD_FAT -#define CONFIG_CMD_FPGA #define CONFIG_CMD_FS_GENERIC #define CONFIG_CMD_GREPENV #define CONFIG_CMD_MII #define CONFIG_CMD_MMC -#define CONFIG_CMD_NET -#define CONFIG_CMD_PING #define CONFIG_CMD_SETEXPR -#define CONFIG_CMD_USB #define CONFIG_CMD_USB_MASS_STORAGE
#define CONFIG_REGEX /* Enable regular expression support */

On Tue 2015-04-21 12:23:07, Pavel Machek wrote:
Setting configuration options in header file leads to incosistency between .config user sees, and .config he has. What is worse, a lot of compile warnings is presented for any such config option user sets in .config.
Signed-off-by: Pavel Machek pavel@denx.de
Ping? Please apply.
v2:
You need to add these to common/Kconfig as well as to your defconfig.
Otherwise, they do not appear in the .config file.
There exist CMD_RUN, CMD_PING, CMD_USB in common/Kconfig, but most of the others are missing.
Ok, I did conversion only for those present in Kconfig.
diff --git a/configs/socfpga_cyclone5_defconfig b/configs/socfpga_cyclone5_defconfig index 0ebfbfc..5cfc56c 100644 --- a/configs/socfpga_cyclone5_defconfig +++ b/configs/socfpga_cyclone5_defconfig @@ -6,3 +6,8 @@ CONFIG_DEFAULT_DEVICE_TREE="socfpga_cyclone5_socdk" CONFIG_DM=y CONFIG_DM_SPI=y CONFIG_DM_SPI_FLASH=y +CONFIG_CMD_DHCP=y +CONFIG_CMD_FPGA=y +CONFIG_CMD_NET=y +CONFIG_CMD_PING=y +CONFIG_CMD_USB=y diff --git a/configs/socfpga_socrates_defconfig b/configs/socfpga_socrates_defconfig index 873b721..0cde0e9 100644 --- a/configs/socfpga_socrates_defconfig +++ b/configs/socfpga_socrates_defconfig @@ -6,3 +6,8 @@ CONFIG_DEFAULT_DEVICE_TREE="socfpga_cyclone5_socrates" CONFIG_DM=y CONFIG_DM_SPI=y CONFIG_DM_SPI_FLASH=y +CONFIG_CMD_DHCP=y +CONFIG_CMD_FPGA=y +CONFIG_CMD_NET=y +CONFIG_CMD_PING=y +CONFIG_CMD_USB=y diff --git a/include/configs/socfpga_cyclone5.h b/include/configs/socfpga_cyclone5.h index 2e3a8b6..93a0572 100644 --- a/include/configs/socfpga_cyclone5.h +++ b/include/configs/socfpga_cyclone5.h @@ -22,19 +22,14 @@ #define CONFIG_CMD_BOOTZ #define CONFIG_CMD_CACHE #define CONFIG_CMD_DFU -#define CONFIG_CMD_DHCP #define CONFIG_CMD_EXT4 #define CONFIG_CMD_EXT4_WRITE #define CONFIG_CMD_FAT -#define CONFIG_CMD_FPGA #define CONFIG_CMD_FS_GENERIC #define CONFIG_CMD_GREPENV #define CONFIG_CMD_MII #define CONFIG_CMD_MMC -#define CONFIG_CMD_NET -#define CONFIG_CMD_PING #define CONFIG_CMD_SETEXPR -#define CONFIG_CMD_USB #define CONFIG_CMD_USB_MASS_STORAGE
#define CONFIG_REGEX /* Enable regular expression support */

2015-05-15 17:31 GMT+09:00 Pavel Machek pavel@denx.de:
On Tue 2015-04-21 12:23:07, Pavel Machek wrote:
Setting configuration options in header file leads to incosistency between .config user sees, and .config he has. What is worse, a lot of compile warnings is presented for any such config option user sets in .config.
Signed-off-by: Pavel Machek pavel@denx.de
Ping? Please apply.
This message was sent to me, but I assume it was addressed to Marek because it is only related to SOCFPGA.

On Fri 2015-05-15 17:36:32, Masahiro Yamada wrote:
2015-05-15 17:31 GMT+09:00 Pavel Machek pavel@denx.de:
On Tue 2015-04-21 12:23:07, Pavel Machek wrote:
Setting configuration options in header file leads to incosistency between .config user sees, and .config he has. What is worse, a lot of compile warnings is presented for any such config option user sets in .config.
Signed-off-by: Pavel Machek pavel@denx.de
Ping? Please apply.
This message was sent to me, but I assume it was addressed to Marek because it is only related to SOCFPGA.
Yes, I'd like Marek to apply it. And Marek wanted your confirmation it looks reasonable...
Thanks, Pavel

Hi.
2015-05-15 17:48 GMT+09:00 Pavel Machek pavel@denx.de:
On Fri 2015-05-15 17:36:32, Masahiro Yamada wrote:
2015-05-15 17:31 GMT+09:00 Pavel Machek pavel@denx.de:
On Tue 2015-04-21 12:23:07, Pavel Machek wrote:
Setting configuration options in header file leads to incosistency between .config user sees, and .config he has. What is worse, a lot of compile warnings is presented for any such config option user sets in .config.
Signed-off-by: Pavel Machek pavel@denx.de
Ping? Please apply.
This message was sent to me, but I assume it was addressed to Marek because it is only related to SOCFPGA.
Yes, I'd like Marek to apply it. And Marek wanted your confirmation it looks reasonable...
The defconfig files in u-boot/master have been all cleaned up by
commit bd328eb38274ffaf04caaa8a6ecc09b7e19a650e Author: Joe Hershberger joe.hershberger@ni.com Date: Tue May 12 14:46:24 2015 -0500
Clean all defconfigs with savedefconfig
If this patch is applied onto u-boot-socfpga now and then a pull-req is issued, it will cause a conflict.
So, I recommend Marek to sync u-boot-socfpga/master with the mainline, and then v3 should be applied.
Please use savedefconfig rather than adding entries to the bottom of the files.

On Friday, May 15, 2015 at 11:00:37 AM, Masahiro Yamada wrote:
Hi.
Hi!
2015-05-15 17:48 GMT+09:00 Pavel Machek pavel@denx.de:
On Fri 2015-05-15 17:36:32, Masahiro Yamada wrote:
2015-05-15 17:31 GMT+09:00 Pavel Machek pavel@denx.de:
On Tue 2015-04-21 12:23:07, Pavel Machek wrote:
Setting configuration options in header file leads to incosistency between .config user sees, and .config he has. What is worse, a lot of compile warnings is presented for any such config option user sets in .config.
Signed-off-by: Pavel Machek pavel@denx.de
Ping? Please apply.
This message was sent to me, but I assume it was addressed to Marek because it is only related to SOCFPGA.
Yes, I'd like Marek to apply it. And Marek wanted your confirmation it looks reasonable...
The defconfig files in u-boot/master have been all cleaned up by
commit bd328eb38274ffaf04caaa8a6ecc09b7e19a650e Author: Joe Hershberger joe.hershberger@ni.com Date: Tue May 12 14:46:24 2015 -0500
Clean all defconfigs with savedefconfig
If this patch is applied onto u-boot-socfpga now and then a pull-req is issued, it will cause a conflict.
So, I recommend Marek to sync u-boot-socfpga/master with the mainline, and then v3 should be applied.
Yes Sir, synched!
Please use savedefconfig rather than adding entries to the bottom of the files.
Best regards, Marek Vasut
participants (3)
-
Marek Vasut
-
Masahiro Yamada
-
Pavel Machek