From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Date: Sat, 11 Jun 2016 16:38:47 +0200 Subject: [U-Boot] [PATCH v2 4/5] autoboot: fix a bunch of misconversion of CONFIG_BOOTDELAY In-Reply-To: <20160611124115.GM11619@bill-the-cat> References: <1465638252-13196-1-git-send-email-yamada.masahiro@socionext.com> <1465638252-13196-5-git-send-email-yamada.masahiro@socionext.com> <20160611124115.GM11619@bill-the-cat> Message-ID: <1d786bba-9c6e-d0dc-dab3-b60d03e47367@redhat.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, On 11-06-16 14:43, Tom Rini wrote: > On Sat, Jun 11, 2016 at 01:31:29PM +0200, Hans de Goede wrote: >> Hi, >> >> On 11-06-16 11:44, Masahiro Yamada wrote: >>> Commit bb597c0eeb7e ("common: bootdelay: move CONFIG_BOOTDELAY into >>> a Kconfig option") made a number of misconversion. >>> >>> [1] CONFIG_BOOTDELAY=-1 all gone >>> [2] CONFIG_BOOTDELAY=1 all gone >>> [3] CONFIG_BOOTDELAY=2 all gone >>> [4] Conditionally defined CONFIG_BOOTDELAY all gone >>> >>> All of the misconverted boards now use the default value, >>> CONFIG_BOOTDELAY=0, which came from the Kconfig entry. >>> >>> I am imagining some reasons for this. >>> >>> For [1], due to the bug of tools/scripts/define2mk.sed (now fixed), >>> #define CONFIG_BOOTDELAY -1 >>> was converted to >>> CONFIG_BOOTDELAY="-1" >>> in the include/autoconf.mk, so the tools/moveconfig.py regarded it >>> as a string type option, and failed to move it. >>> >>> For [2], as you see in the comment block in the define2mk.sed, >>> #define CONFIG_BOOTDELAY 1 >>> is converted to >>> CONFIG_BOOTDELAY=y >>> in the include/autoconf.mk. This needs a special care because we do >>> not know whether we are moving a bool option with value y or an >>> integer option with value 1. A recently-sent patch fixes this issue. >>> >>> I do not understand the reason for [3]. >>> >>> [4] is another case the current moveconfig cannot handle correctly. >>> If the define is surrounded by #ifndef CONFIG_BOOTDELAY like follows, >>> the default value from Kconfig entry beats the define in C header. >>> >>> #ifndef CONFIG_BOOTDELAY >>> #define CONFIG_BOOTDELAY 3 >>> #endif >>> >>> Joe's patch can solve this issue. >>> >>> Anyway, I ran the newest moveconfig tool based on commit 3191d8408053 >>> (=immediately prior to the bad commit) to generate this patch. >>> >>> Signed-off-by: Masahiro Yamada >>> --- >>> >>> Changes in v2: >>> - Fix case [4] > [snip] >> >> No, just no NACK at least for all the sunxi boards. >> >> This madness where everytime we convert something to Kconfig >> we end up bloating ALL the defconfigs needs to stop and it >> needs to stop now. >> >> I'm 100% done with fixing up things everytime after someone >> thinks this is a good idea. >> >> This is not a good idea, it is a REALLY REALLY bad idea. >> >> The defconfig files should contain board specific settings, >> we had the same nonsense when some of the CONFIG_CMD_FOO >> things were converted, and all of a sudden we had the >> same 5 extra lines in every defconfig for every board >> which used to use config_distro_bootcmd.h >> >> Apparently no one was smart enough to realize that if >> we were moving stuff from a common config header to >> Kconfig, we would need something like >> >> CONFIG_DISTRO_BOOTCMD in Kconfig which would then select >> all CONFIG_CMD_FOO options which were moved, and then >> board / soc Kconfig snippets could simply do: >> >> select CONFIG_DISTRO_BOOTCMD >> >> I've hacked around this for sunxi: >> >> config ARCH_SUNXI >> bool "Support sunxi (Allwinner) SoCs" >> select CMD_BOOTZ >> select CMD_DHCP >> select CMD_EXT2 >> select CMD_EXT4 >> select CMD_FAT >> select CMD_FS_GENERIC >> select CMD_GPIO >> select CMD_MII >> select CMD_MMC if MMC >> select CMD_PING >> select CMD_USB >> ... >> >> But I'm sick and tired of this nonsense repeating over >> and over again. > > OK. You make some good points. Can you please write the help text for > DISTRO_DEFAULTS and DISTRO_BOOTCMD and do at least the initial selects > for things that have been converted? That will help out a lot, thanks! When I said DISTRO_BOOTCMD above I meant DISTRO_DEFAULTS, DISTRO_BOOTCMD does not make sense / is not necessary for now. I've prepated a patch adding DISTRO_DEFAULTS I will send it right after this mail. Regards, Hans