Add support for generating an autoconf.h header file that can be used in the source instead of #ifdef.

For example, instead of:

#ifdef CONFIG_VERSION_VARIABLE setenv("ver", version_string); /* set version variable */ #endif

you can do:

if (autoconf_version_variable()) setenv("ver", version_string); /* set version variable */

The compiler will ensure that the dead code is eliminated, so the result is the same.

Where the value of the CONFIG define is 0, you can use the autoconf_has...() form. For example CONFIG_BOOTDELAY can be -ve, 0 or +ve, but if it is defined at all, it affects behaviour:

#if defined(CONFIG_BOOTDELAY) && (CONFIG_BOOTDELAY >= 0) s = getenv ("bootdelay"); #endif

So we use:

if (autoconf_has_bootdelay() && autoconf_bootdelay() >= 0) s = getenv ("bootdelay");

This later form should only be used for such 'difficult' defines where a zero value still means that the CONFIG should be considered to be defined.

Signed-off-by: Simon Glass sjg@chromium.org --- Changes in v3: - Update config_xxx() to autoconf_xxx() in comments/README/sed - Update config_xxx_enabled() to autoconf_has_xxx() in comments/README/sed - Add comment as to why we use [A-Za-z0-9_][A-Za-z0-9_]* - Rename sed scripts to more useful names

Changes in v2: - Split out changes to main.c into separate patches - Fix up a few errors and comments in the original RFC - Use autoconf_...() instead of config_...() - Use autoconf_has_...() instead of config_..._enabled() - Add a grep to the sed/sort pipe to speed up processing

Makefile | 43 ++++++++++++++++++++- README | 87 ++++++++++++++++++++++++++++++++++++++++-- include/common.h | 3 ++ include/config_drop.h | 17 +++++++++ tools/scripts/define2value.sed | 37 ++++++++++++++++++ tools/scripts/define2zero.sed | 32 ++++++++++++++++ 6 files changed, 215 insertions(+), 4 deletions(-) create mode 100644 include/config_drop.h create mode 100644 tools/scripts/define2value.sed create mode 100644 tools/scripts/define2zero.sed

diff --git a/Makefile b/Makefile index fc18dd4..8ba068d 100644 --- a/Makefile +++ b/Makefile @@ -614,6 +614,7 @@ updater: # parallel sub-makes creating .depend files simultaneously. depend dep: $(TIMESTAMP_FILE) $(VERSION_FILE) \ $(obj)include/autoconf.mk \ + $(obj)include/generated/autoconf.h \ $(obj)include/generated/generic-asm-offsets.h \ $(obj)include/generated/asm-offsets.h for dir in $(SUBDIRS) $(CPUDIR) $(LDSCRIPT_MAKEFILE_DIR) ; do \ @@ -688,6 +689,45 @@ $(obj)include/autoconf.mk: $(obj)include/config.h sed -n -f tools/scripts/define2mk.sed > $@.tmp && \ mv $@.tmp $@

+# Create a C header file where every '#define CONFIG_XXX value' becomes +# '#define autoconf_xxx() value', or '#define autoconf_xxx() 0' where the +# CONFIG is not used by this board configuration. This allows C code to do +# things like 'if (autoconf_xxx())' and have the compiler remove the dead code, +# instead of using '#ifdef CONFIG_XXX...#endif'. Note that in most cases +# if the autoconf_...() returns 0 then the option is not enabled. In some rare +# cases such as CONFIG_BOOTDELAY, the config can be enabled but still have a +# a value of 0. So in addition we a #define autoconf_has_xxx(), setting the +# value to 0 if the option is disabled, 1 if enabled. This last feature will +# hopefully be deprecated soon. +# The file is regenerated when any U-Boot header file changes. +$(obj)include/generated/autoconf.h: $(obj)include/config.h + @$(XECHO) Generating $@ ; \ + set -e ; \ + : Extract the config macros to a C header file ; \ + $(CPP) $(CFLAGS) -DDO_DEPS_ONLY -dM include/common.h | \ + sed -n -f tools/scripts/define2value.sed > $@.tmp; \ + : Regenerate our list of all config macros if neeed ; \ + if [ ! -f $@-all.tmp ] || \ + find $(src) -name '*.h' -type f -newer $@-all.tmp | \ + egrep -qv 'include/(autoconf.h|generated|config.h)'; \ + then \ + : Extract all config macros from all C header files ; \ + : We can grep for CONFIG since the value will be dropped ; \ + ( \ + find ${src} -name "*.h" -type f | xargs \ + cat | grep CONFIG | \ + sed -n -f tools/scripts/define2zero.sed \ + ) | sort | uniq > $@-all.tmp; \ + fi; \ + : Extract the enabled config macros to a C header file ; \ + $(CPP) $(CFLAGS) -DDO_DEPS_ONLY -dM include/common.h | \ + sed -n -f tools/scripts/define2zero.sed | \ + sort > $@-enabled.tmp; \ + set -e ; \ + : Find CONFIGs that are not enabled ; \ + comm -13 $@-enabled.tmp $@-all.tmp >>$@.tmp && \ + mv $@.tmp $@ + $(obj)include/generated/generic-asm-offsets.h: $(obj)include/autoconf.mk.dep \ $(obj)lib/asm-offsets.s @$(XECHO) Generating $@ @@ -770,7 +810,8 @@ include/license.h: tools/bin2header COPYING unconfig: @rm -f $(obj)include/config.h $(obj)include/config.mk \ $(obj)board/*/config.tmp $(obj)board/*/*/config.tmp \ - $(obj)include/autoconf.mk $(obj)include/autoconf.mk.dep + $(obj)include/autoconf.mk $(obj)include/autoconf.mk.dep \ + $(obj)include/generated/autoconf.h

%_config:: unconfig @$(MKCONFIG) -A $(@:_config=) diff --git a/README b/README index d8cb394..e6f3c04 100644 --- a/README +++ b/README @@ -5434,11 +5434,92 @@ Notes: * If you modify existing code, make sure that your new code does not add to the memory footprint of the code ;-) Small is beautiful! When adding new features, these should compile conditionally only - (using #ifdef), and the resulting code with the new feature - disabled must not need more memory than the old code without your - modification. + (avoiding #ifdef where at all possible), and the resulting code with + the new feature disabled must not need more memory than the old code + without your modification.

* Remember that there is a size limit of 100 kB per message on the u-boot mailing list. Bigger patches will be moderated. If they are reasonable and not too big, they will be acknowledged. But patches bigger than the size limit should be avoided. + + +Use of #ifdef: +-------------- +Many parts of the U-Boot code base are sprinkled with #ifdefs. This makes +different boards compile different versions of the source code, meaning +that we must build all boards to check for failures. It is easy to misspell +an #ifdef and there is not as much checking of this by the compiler. For +someone coming new into the code base, #ifdefs are a big turn-off. Multiple +dependent #ifdefs are harder to do than with if..then..else. Variable +declarations must be #idefed as well as the code that uses them, often much +later in the file/function. #ifdef indents don't match code indents and +have their own separate indent feature. Overall, excessive use of #idef +hurts readability and makes the code harder to modify and refactor. + +In an effort to reduce the use of #ifdef in U-Boot, without requiring lots +of special static inlines all over the header files, a single autoconf.h +header file with lower-case function-type macros has been made available. + +This file has either: + +# #define autoconf_xxx() value + +for enabled options, or: + +# #define autoconf_xxx() 0 + +for disabled options. You can therefore generally change code like this: + + #ifdef CONFIG_XXX + do_something + #else + do_something_else + #endif + +to this: + + if (autoconf_xxx()) + do_something; + else + do_something_else; + +The compiler will see that autoconf_xxx() evalutes to a constant and will +eliminate the dead code. The resulting code (and code size) is the same. + +Multiple #ifdefs can be converted also: + + #if defined(CONFIG_XXX) && !defined(CONFIG_YYY) + do_something + #endif + + if (autoconf_xxx() && !autoconf_yyy()) + do_something; + +Where the macro evaluates to a string, it will be non-NULL, so the above +will work whether the macro is a string or a number. + +This takes care of almost all CONFIG macros. Unfortunately there are a few +cases where a value of 0 does not mean the option is disabled. For example +CONFIG_BOOTDELAY can be defined to 0, which means that the bootdelay +code should be used, but with a value of 0. To get around this and other +sticky cases, an addition macro with an 'has_' prefix is provided, where +the value is always either 0 or 1: + + // Will work even if boaard config has '#define CONFIG_BOOTDELAY 0' + if (autoconf_has_bootdelay()) + do_something; + +(Probably such config options should be deprecated and then we can remove +this feature) + +U-Boot already has a Makefile scheme to permit files to be easily included +based on CONFIG. This can be used where the code to be compiled exists in +its own source file. So the following rules apply: + + 1. Use #ifdef to conditionally compile an exported function or variable + 2. Use ordinary C code with autoconf_xxx() everywhere else + 3. Mark your functions and data structures static where possible + 4. Use the autoconf_has_xxx() variants only if essential + 5. When changing existing code, first create a new patch to replace + #ifdefs in the surrounding area diff --git a/include/common.h b/include/common.h index 4ad17ea..491783b 100644 --- a/include/common.h +++ b/include/common.h @@ -35,6 +35,9 @@ typedef volatile unsigned short vu_short; typedef volatile unsigned char vu_char;

#include <config.h> +#ifndef DO_DEPS_ONLY +#include <generated/autoconf.h> +#endif #include <asm-offsets.h> #include <linux/bitops.h> #include <linux/types.h> diff --git a/include/config_drop.h b/include/config_drop.h new file mode 100644 index 0000000..bf68b50 --- /dev/null +++ b/include/config_drop.h @@ -0,0 +1,17 @@ +/* + * Copyright 2013 Google, Inc + * + * This file is licensed under the terms of the GNU General Public + * License Version 2. This file is licensed "as is" without any + * warranty of any kind, whether express or implied. + */ + +#ifndef _CONFIG_DROP_H +#define _CONFIG_DROP_H + +/* Options which don't seem to be defined by any header in U-Boot */ +#define CONFIG_MENUPROMPT "Auto-boot prompt" +#define CONFIG_MENUKEY +#define CONFIG_UPDATE_TFTP + +#endif diff --git a/tools/scripts/define2value.sed b/tools/scripts/define2value.sed new file mode 100644 index 0000000..205f9fe --- /dev/null +++ b/tools/scripts/define2value.sed @@ -0,0 +1,37 @@ +# +# Sed script to parse CPP macros and generate a list of CONFIG macros +# +# This converts: +# #define CONFIG_XXX value +#into: +# #define autoconf_xxx() value +# #define autoconf_has_xxx() 1 + +# Macros with parameters are ignored. +# (Note we avoid + since it doesn't appear to work) +/^#define CONFIG_[A-Za-z0-9_][A-Za-z0-9_]*(/ { + d +} + +# Only process values prefixed with #define CONFIG_ +/^#define CONFIG_[A-Za-z0-9_][A-Za-z0-9_]*/ { + # Strip the #define prefix + s/#define[ \t]*CONFIG_/autoconf_/; + # Change to form CONFIG_*=VALUE + s/[\t ][\t ]*/=/; + # Handle lines with no value + s/^([^=]*)$/\1=/; + # Drop trailing spaces + s/ *$//; + # Change empty values to '1' + s/=$/=1/; + # Add #define at the start + s/^([^=]*)=/#define \L\1() / + # print the line + p + # Create autoconf_has_...(), value 1 + s/().*/() 1/ + s/(autoconf_)/\1has_/ + # print the line + p +} diff --git a/tools/scripts/define2zero.sed b/tools/scripts/define2zero.sed new file mode 100644 index 0000000..95e6860 --- /dev/null +++ b/tools/scripts/define2zero.sed @@ -0,0 +1,32 @@ +# +# Sed script to parse CPP macros and generate a list of autoconf macros +# +# This converts: +# #define CONFIG_XXX value +#into: +# #define autoconf_xxx() 0 +# #define autoconf_has_xxx() 0 + +# Macros with parameters are ignored. +# (Note we avoid + since it doesn't appear to work) +/^#define CONFIG_[A-Za-z0-9_][A-Za-z0-9_]*(/ { + s/.*// +} + +# Only process values prefixed with #define CONFIG_ +/^#define CONFIG_[A-Za-z0-9_][A-Za-z0-9_]*/ { + # Strip the #define prefix + s/#define *//; + # Remove the value + s/[ \t].*//; + # Convert to lower case, prepend #define + s/CONFIG_(.*)/#define autoconf_\L\1/ + # Append 0 + s/$/() 0/ + # print the line + p + # Create autoconf_has_...(), value 0 + s/(autoconf_)/\1has_/ + # print the line + p +}

This function should be declared in net.h. At the same time, let's use autoconf instead of #ifdef for its inclusion.

Signed-off-by: Simon Glass sjg@chromium.org Acked-by: Joe Hershberger joe.hershberger@ni.com --- Changes in v3: None Changes in v2: None

common/cmd_fitupd.c | 3 +-- common/main.c | 9 ++------- common/update.c | 24 ++++++++---------------- include/net.h | 3 +++ 4 files changed, 14 insertions(+), 25 deletions(-)

diff --git a/common/cmd_fitupd.c b/common/cmd_fitupd.c index 7a3789e..618ff7c 100644 --- a/common/cmd_fitupd.c +++ b/common/cmd_fitupd.c @@ -8,13 +8,12 @@

#include <common.h> #include <command.h> +#include <net.h>

#if !defined(CONFIG_UPDATE_TFTP) #error "CONFIG_UPDATE_TFTP required" #endif

-extern int update_tftp(ulong addr); - static int do_fitupd(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { ulong addr = 0UL; diff --git a/common/main.c b/common/main.c index e2d2e09..2b8af2c 100644 --- a/common/main.c +++ b/common/main.c @@ -61,10 +61,6 @@ DECLARE_GLOBAL_DATA_PTR; void inline __show_boot_progress (int val) {} void show_boot_progress (int val) __attribute__((weak, alias("__show_boot_progress")));

-#if defined(CONFIG_UPDATE_TFTP) -int update_tftp (ulong addr); -#endif /* CONFIG_UPDATE_TFTP */ - #define MAX_DELAY_STOP_STR 32

#undef DEBUG_PARSER @@ -427,9 +423,8 @@ void main_loop (void) } #endif /* CONFIG_PREBOOT */

-#if defined(CONFIG_UPDATE_TFTP) - update_tftp (0UL); -#endif /* CONFIG_UPDATE_TFTP */ + if (autoconf_update_tftp()) + update_tftp(0UL);

#if defined(CONFIG_BOOTDELAY) && (CONFIG_BOOTDELAY >= 0) s = getenv ("bootdelay"); diff --git a/common/update.c b/common/update.c index 94d6a82..9cd9ca2 100644 --- a/common/update.c +++ b/common/update.c @@ -43,19 +43,6 @@ /* env variable holding the location of the update file */ #define UPDATE_FILE_ENV "updatefile"

-/* set configuration defaults if needed */ -#ifndef CONFIG_UPDATE_LOAD_ADDR -#define CONFIG_UPDATE_LOAD_ADDR 0x100000 -#endif - -#ifndef CONFIG_UPDATE_TFTP_MSEC_MAX -#define CONFIG_UPDATE_TFTP_MSEC_MAX 100 -#endif - -#ifndef CONFIG_UPDATE_TFTP_CNT_MAX -#define CONFIG_UPDATE_TFTP_CNT_MAX 0 -#endif - extern ulong TftpRRQTimeoutMSecs; extern int TftpRRQTimeoutCountMax; extern flash_info_t flash_info[]; @@ -244,6 +231,7 @@ int update_tftp(ulong addr) char *filename, *env_addr; int images_noffset, ndepth, noffset; ulong update_addr, update_fladdr, update_size; + int msec_max; void *fit; int ret = 0;

@@ -266,12 +254,16 @@ int update_tftp(ulong addr) /* get load address of downloaded update file */ if ((env_addr = getenv("loadaddr")) != NULL) addr = simple_strtoul(env_addr, NULL, 16); + else if (autoconf_has_update_load_addr()) + addr = autoconf_update_load_addr(); else - addr = CONFIG_UPDATE_LOAD_ADDR; + addr = 0x100000;

+ msec_max = autoconf_has_update_tftp_msec_max() ? + autoconf_update_tftp_msec_max() : 100;

- if (update_load(filename, CONFIG_UPDATE_TFTP_MSEC_MAX, - CONFIG_UPDATE_TFTP_CNT_MAX, addr)) { + if (update_load(filename, msec_max, autoconf_update_tftp_cnt_max(), + addr)) { printf("Can't load update file, aborting auto-update\n"); return 1; } diff --git a/include/net.h b/include/net.h index 970d4d1..23fb947 100644 --- a/include/net.h +++ b/include/net.h @@ -695,6 +695,9 @@ extern void copy_filename(char *dst, const char *src, int size); /* get a random source port */ extern unsigned int random_port(void);

+/* Update U-Boot over TFTP */ +extern int update_tftp(ulong addr); + /**********************************************************************/

#endif /* __NET_H__ */

Move this code into its own function, since it clutters up main_loop().

Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Joe Hershberger joe.hershberger@ni.com --- Changes in v3: None Changes in v2: None

common/main.c | 155 ++++++++++++++++++++++++++++++---------------------------- 1 file changed, 79 insertions(+), 76 deletions(-)

diff --git a/common/main.c b/common/main.c index 1e12e55..0df7992 100644 --- a/common/main.c +++ b/common/main.c @@ -91,7 +91,6 @@ extern void mdm_init(void); /* defined in board.c */ * Watch for 'delay' seconds for autoboot stop or autoboot delay string. * returns: 0 - no key string, allow autoboot 1 - got key string, abort */ -#if defined(CONFIG_BOOTDELAY) && (CONFIG_BOOTDELAY >= 0) static int abortboot_keyed(int bootdelay) { int abort = 0; @@ -273,7 +272,6 @@ static int abortboot(int bootdelay) else return abortboot_normal(bootdelay); } -#endif /* CONFIG_BOOTDELAY >= 0 */

/* * Runs the given boot command securely. Specifically: @@ -289,8 +287,7 @@ static int abortboot(int bootdelay) * printing the error message to console. */

-#if defined(CONFIG_BOOTDELAY) && (CONFIG_BOOTDELAY >= 0) && \ - defined(CONFIG_OF_CONTROL) +#ifdef CONFIG_OF_CONTROL static void secure_boot_cmd(char *cmd) { cmd_tbl_t *cmdtp; @@ -331,46 +328,33 @@ static void process_fdt_options(const void *blob)

/* Add an env variable to point to a kernel payload, if available */ addr = fdtdec_get_config_int(gd->fdt_blob, "kernel-offset", 0); - if (addr) - setenv_addr("kernaddr", (void *)(CONFIG_SYS_TEXT_BASE + addr)); + if (addr) { + setenv_addr("kernaddr", + (void *)(autoconf_sys_text_base() + addr)); + }

/* Add an env variable to point to a root disk, if available */ addr = fdtdec_get_config_int(gd->fdt_blob, "rootdisk-offset", 0); - if (addr) - setenv_addr("rootaddr", (void *)(CONFIG_SYS_TEXT_BASE + addr)); + if (addr) { + setenv_addr("rootaddr", + (void *)(autoconf_sys_text_base() + addr)); + } } #endif /* CONFIG_OF_CONTROL */

- -/****************************************************************************/ - -void main_loop (void) +static void process_boot_delay(void) { -#ifndef CONFIG_SYS_HUSH_PARSER - static char lastcommand[CONFIG_SYS_CBSIZE] = { 0, }; - int len; - int rc = 1; - int flag; -#endif -#if defined(CONFIG_BOOTDELAY) && (CONFIG_BOOTDELAY >= 0) && \ - defined(CONFIG_OF_CONTROL) - char *env; -#endif -#if defined(CONFIG_BOOTDELAY) && (CONFIG_BOOTDELAY >= 0) - char *s; - int bootdelay; -#endif -#ifdef CONFIG_PREBOOT - char *p; -#endif #ifdef CONFIG_BOOTCOUNT_LIMIT unsigned long bootcount = 0; unsigned long bootlimit = 0; char *bcs; char bcs_set[16]; #endif /* CONFIG_BOOTCOUNT_LIMIT */ - - bootstage_mark_name(BOOTSTAGE_ID_MAIN_LOOP, "main_loop"); +#ifdef CONFIG_OF_CONTROL + char *env; +#endif + char *s; + int bootdelay;

#ifdef CONFIG_BOOTCOUNT_LIMIT bootcount = bootcount_load(); @@ -382,51 +366,8 @@ void main_loop (void) bootlimit = bcs ? simple_strtoul (bcs, NULL, 10) : 0; #endif /* CONFIG_BOOTCOUNT_LIMIT */

-#ifdef CONFIG_MODEM_SUPPORT - debug ("DEBUG: main_loop: do_mdm_init=%d\n", do_mdm_init); - if (do_mdm_init) { - char *str = strdup(getenv("mdm_cmd")); - setenv ("preboot", str); /* set or delete definition */ - if (str != NULL) - free (str); - mdm_init(); /* wait for modem connection */ - } -#endif /* CONFIG_MODEM_SUPPORT */ - -#ifdef CONFIG_VERSION_VARIABLE - { - setenv ("ver", version_string); /* set version variable */ - } -#endif /* CONFIG_VERSION_VARIABLE */ - -#ifdef CONFIG_SYS_HUSH_PARSER - u_boot_hush_start (); -#endif - -#if defined(CONFIG_HUSH_INIT_VAR) - hush_init_var (); -#endif - -#ifdef CONFIG_PREBOOT - if ((p = getenv ("preboot")) != NULL) { -# ifdef CONFIG_AUTOBOOT_KEYED - int prev = disable_ctrlc(1); /* disable Control C checking */ -# endif - - run_command_list(p, -1, 0); - -# ifdef CONFIG_AUTOBOOT_KEYED - disable_ctrlc(prev); /* restore Control C checking */ -# endif - } -#endif /* CONFIG_PREBOOT */ - - if (autoconf_update_tftp()) - update_tftp(0UL); - -#if defined(CONFIG_BOOTDELAY) && (CONFIG_BOOTDELAY >= 0) s = getenv ("bootdelay"); - bootdelay = s ? (int)simple_strtol(s, NULL, 10) : CONFIG_BOOTDELAY; + bootdelay = s ? (int)simple_strtol(s, NULL, 10) : autoconf_bootdelay();

debug ("### main_loop entered: bootdelay=%d\n\n", bootdelay);

@@ -491,7 +432,69 @@ void main_loop (void) run_command_list(s, -1, 0); } #endif /* CONFIG_MENUKEY */ -#endif /* CONFIG_BOOTDELAY */ +} + +/****************************************************************************/ + +void main_loop(void) +{ +#ifndef CONFIG_SYS_HUSH_PARSER + static char lastcommand[CONFIG_SYS_CBSIZE] = { 0, }; + int len; + int rc = 1; + int flag; +#endif +#ifdef CONFIG_PREBOOT + char *p; +#endif + + bootstage_mark_name(BOOTSTAGE_ID_MAIN_LOOP, "main_loop"); + +#ifdef CONFIG_MODEM_SUPPORT + debug("DEBUG: main_loop: do_mdm_init=%d\n", do_mdm_init); + if (do_mdm_init) { + char *str = strdup(getenv("mdm_cmd")); + setenv("preboot", str); /* set or delete definition */ + if (str != NULL) + free(str); + mdm_init(); /* wait for modem connection */ + } +#endif /* CONFIG_MODEM_SUPPORT */ + +#ifdef CONFIG_VERSION_VARIABLE + { + setenv("ver", version_string); /* set version variable */ + } +#endif /* CONFIG_VERSION_VARIABLE */ + +#ifdef CONFIG_SYS_HUSH_PARSER + u_boot_hush_start(); +#endif + +#if defined(CONFIG_HUSH_INIT_VAR) + hush_init_var(); +#endif + +#ifdef CONFIG_PREBOOT + p = getenv("preboot"); + if (p) { +# ifdef CONFIG_AUTOBOOT_KEYED + int prev = disable_ctrlc(1); /* disable Control C checking */ +# endif + + run_command_list(p, -1, 0); + +# ifdef CONFIG_AUTOBOOT_KEYED + disable_ctrlc(prev); /* restore Control C checking */ +# endif + } +#endif /* CONFIG_PREBOOT */ + + if (autoconf_update_tftp()) + update_tftp(0UL); + + if (autoconf_has_bootdelay() && autoconf_bootdelay() >= 0) + process_boot_delay();

#if defined CONFIG_OF_CONTROL set_working_fdt_addr((void *)gd->fdt_blob);

Move this code over to using autoconf. We can add the autoconf values to the delaykey[] array, and move the code that checks for autoconf values into the loop.

Also change to using ARRAY_SIZE on delaykey[].

Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Joe Hershberger joe.hershberger@ni.com --- Changes in v3: None Changes in v2: None

common/main.c | 86 ++++++++++++++++++++++------------------------------------- 1 file changed, 32 insertions(+), 54 deletions(-)

diff --git a/common/main.c b/common/main.c index 36d8048..e2eb9ec 100644 --- a/common/main.c +++ b/common/main.c @@ -90,15 +90,20 @@ static int abortboot_keyed(int bootdelay) int abort = 0; uint64_t etime = endtick(bootdelay); struct { - char* str; + const char *str; u_int len; int retry; + const char *conf; /* Configuration value */ } delaykey [] = { - { str: getenv ("bootdelaykey"), retry: 1 }, - { str: getenv ("bootdelaykey2"), retry: 1 }, - { str: getenv ("bootstopkey"), retry: 0 }, - { str: getenv ("bootstopkey2"), retry: 0 }, + { str: getenv("bootdelaykey"), retry: 1, + conf: autoconf_autoboot_delay_str() }, + { str: getenv("bootdelaykey2"), retry: 1, + conf: autoconf_autoboot_delay_str2() }, + { str: getenv("bootstopkey"), retry: 0, + conf: autoconf_autoboot_stop_str() }, + { str: getenv("bootstopkey2"), retry: 0, + conf: autoconf_autoboot_stop_str2() }, };

char presskey [MAX_DELAY_STOP_STR]; @@ -106,33 +111,15 @@ static int abortboot_keyed(int bootdelay) u_int presskey_max = 0; u_int i;

-#ifndef CONFIG_ZERO_BOOTDELAY_CHECK - if (bootdelay == 0) + if (!autoconf_zero_bootdelay_check() && bootdelay == 0) return 0; -#endif - -# ifdef CONFIG_AUTOBOOT_PROMPT - printf(CONFIG_AUTOBOOT_PROMPT); -# endif

-# ifdef CONFIG_AUTOBOOT_DELAY_STR - if (delaykey[0].str == NULL) - delaykey[0].str = CONFIG_AUTOBOOT_DELAY_STR; -# endif -# ifdef CONFIG_AUTOBOOT_DELAY_STR2 - if (delaykey[1].str == NULL) - delaykey[1].str = CONFIG_AUTOBOOT_DELAY_STR2; -# endif -# ifdef CONFIG_AUTOBOOT_STOP_STR - if (delaykey[2].str == NULL) - delaykey[2].str = CONFIG_AUTOBOOT_STOP_STR; -# endif -# ifdef CONFIG_AUTOBOOT_STOP_STR2 - if (delaykey[3].str == NULL) - delaykey[3].str = CONFIG_AUTOBOOT_STOP_STR2; -# endif + if (autoconf_has_autoboot_prompt()) + printf(autoconf_autoboot_prompt());

- for (i = 0; i < sizeof(delaykey) / sizeof(delaykey[0]); i ++) { + for (i = 0; i < ARRAY_SIZE(delaykey); i++) { + if (delaykey[i].conf && !delaykey[i].str) + delaykey[i].str = delaykey[i].conf; delaykey[i].len = delaykey[i].str == NULL ? 0 : strlen (delaykey[i].str); delaykey[i].len = delaykey[i].len > MAX_DELAY_STOP_STR ? @@ -164,7 +151,7 @@ static int abortboot_keyed(int bootdelay) } }

- for (i = 0; i < sizeof(delaykey) / sizeof(delaykey[0]); i ++) { + for (i = 0; i < ARRAY_SIZE(delaykey); i++) { if (delaykey[i].len > 0 && presskey_len >= delaykey[i].len && memcmp (presskey + presskey_len - delaykey[i].len, @@ -189,43 +176,37 @@ static int abortboot_keyed(int bootdelay) puts("key timeout\n"); # endif

-#ifdef CONFIG_SILENT_CONSOLE - if (abort) + if (autoconf_silent_console() && abort) gd->flags &= ~GD_FLG_SILENT; -#endif

return abort; }

-#ifdef CONFIG_MENUKEY static int menukey = 0; -#endif

static int abortboot_normal(int bootdelay) { int abort = 0; unsigned long ts;

-#ifdef CONFIG_MENUPROMPT - printf(CONFIG_MENUPROMPT); -#else - if (bootdelay >= 0) + if (autoconf_menuprompt()) + printf(autoconf_menuprompt()); + else if (bootdelay >= 0) printf("Hit any key to stop autoboot: %2d ", bootdelay); -#endif

-#if defined CONFIG_ZERO_BOOTDELAY_CHECK /* - * Check if key already pressed - * Don't check if bootdelay < 0 + * If we need to do a bootdelay check even if bootdelay is 0, do + * it here, since the loop below will be skipped in this case. + * We don't do this check if bootdelay < 0. */ - if (bootdelay >= 0) { - if (tstc()) { /* we got a key press */ + if (autoconf_zero_bootdelay_check() && bootdelay >= 0) { + /* Check if key already pressed */ + if (tstc()) { /* we got a key press */ (void) getc(); /* consume input */ puts ("\b\b\b 0"); abort = 1; /* don't auto boot */ } } -#endif

while ((bootdelay > 0) && (!abort)) { --bootdelay; @@ -235,11 +216,10 @@ static int abortboot_normal(int bootdelay) if (tstc()) { /* we got a key press */ abort = 1; /* don't auto boot */ bootdelay = 0; /* no more delay */ -# ifdef CONFIG_MENUKEY - menukey = getc(); -# else - (void) getc(); /* consume input */ -# endif + if (autoconf_menukey()) + menukey = getc(); + else + (void) getc(); /* consume input */ break; } udelay(10000); @@ -250,10 +230,8 @@ static int abortboot_normal(int bootdelay)

putc('\n');

-#ifdef CONFIG_SILENT_CONSOLE - if (abort) + if (autoconf_silent_console() && abort) gd->flags &= ~GD_FLG_SILENT; -#endif

return abort; }

Convert this function and its children to use autoconf instead of #ifdef.

Some header files must now be included unconditionally, so remove some of the #ifdefs from the header section, and put these header files into the right order.

Signed-off-by: Simon Glass sjg@chromium.org --- Changes in v3: - Simplify code for finding out bootdelay from config or environment

Changes in v2: None

common/main.c | 107 ++++++++++++++++++++++----------------------------------- include/menu.h | 2 -- 2 files changed, 42 insertions(+), 67 deletions(-)

diff --git a/common/main.c b/common/main.c index 8987444..974fc82 100644 --- a/common/main.c +++ b/common/main.c @@ -31,27 +31,17 @@ #include <watchdog.h> #include <command.h> #include <fdtdec.h> +#include <fdt_support.h> #include <malloc.h> +#include <menu.h> #include <version.h> -#ifdef CONFIG_MODEM_SUPPORT -#include <malloc.h> /* for free() prototype */ -#endif

#ifdef CONFIG_SYS_HUSH_PARSER #include <hush.h> #endif

-#ifdef CONFIG_OF_CONTROL -#include <fdtdec.h> -#endif - -#ifdef CONFIG_OF_LIBFDT -#include <fdt_support.h> -#endif /* CONFIG_OF_LIBFDT */ - #include <post.h> #include <linux/ctype.h> -#include <menu.h>

DECLARE_GLOBAL_DATA_PTR;

@@ -258,7 +248,6 @@ static int abortboot(int bootdelay) * printing the error message to console. */

-#ifdef CONFIG_OF_CONTROL static void secure_boot_cmd(char *cmd) { cmd_tbl_t *cmdtp; @@ -311,93 +300,81 @@ static void process_fdt_options(const void *blob) (void *)(autoconf_sys_text_base() + addr)); } } -#endif /* CONFIG_OF_CONTROL */

static void process_boot_delay(void) { -#ifdef CONFIG_BOOTCOUNT_LIMIT unsigned long bootcount = 0; unsigned long bootlimit = 0; -#endif /* CONFIG_BOOTCOUNT_LIMIT */ -#ifdef CONFIG_OF_CONTROL - char *env; -#endif - char *s; + const char *s; int bootdelay;

-#ifdef CONFIG_BOOTCOUNT_LIMIT - bootcount = bootcount_load(); - bootcount++; - bootcount_store (bootcount); - setenv_ulong("bootcount", bootcount); - bootlimit = getenv_ulong("bootlimit", 10, 0); -#endif /* CONFIG_BOOTCOUNT_LIMIT */ + if (autoconf_bootcount_limit()) { + bootcount = bootcount_load(); + bootcount++; + bootcount_store(bootcount); + setenv_ulong("bootcount", bootcount); + bootlimit = getenv_ulong("bootlimit", 10, 0); + }

- s = getenv ("bootdelay"); - bootdelay = s ? (int)simple_strtol(s, NULL, 10) : autoconf_bootdelay(); + bootdelay = getenv_ulong("bootdelay", 10, autoconf_bootdelay());

debug ("### main_loop entered: bootdelay=%d\n\n", bootdelay);

-#if defined(CONFIG_MENU_SHOW) - bootdelay = menu_show(bootdelay); -#endif + if (autoconf_menu_show()) + bootdelay = menu_show(bootdelay); if (autoconf_boot_retry_time()) init_cmd_timeout();

-#ifdef CONFIG_POST - if (gd->flags & GD_FLG_POSTFAIL) { + if (autoconf_post() && (gd->flags & GD_FLG_POSTFAIL)) { s = getenv("failbootcmd"); - } - else -#endif /* CONFIG_POST */ -#ifdef CONFIG_BOOTCOUNT_LIMIT - if (bootlimit && (bootcount > bootlimit)) { + } else if (autoconf_bootcount_limit() && bootlimit && + (bootcount > bootlimit)) { printf ("Warning: Bootlimit (%u) exceeded. Using altbootcmd.\n", (unsigned)bootlimit); s = getenv ("altbootcmd"); - } - else -#endif /* CONFIG_BOOTCOUNT_LIMIT */ + } else { s = getenv ("bootcmd"); -#ifdef CONFIG_OF_CONTROL - /* Allow the fdt to override the boot command */ - env = fdtdec_get_config_string(gd->fdt_blob, "bootcmd"); - if (env) - s = env; + } + if (autoconf_of_control()) { + char *env;

- process_fdt_options(gd->fdt_blob); + /* Allow the fdt to override the boot command */ + env = fdtdec_get_config_string(gd->fdt_blob, "bootcmd"); + if (env) + s = env;

- /* - * If the bootsecure option was chosen, use secure_boot_cmd(). - * Always use 'env' in this case, since bootsecure requres that the - * bootcmd was specified in the FDT too. - */ - if (fdtdec_get_config_int(gd->fdt_blob, "bootsecure", 0)) - secure_boot_cmd(env); + process_fdt_options(gd->fdt_blob);

-#endif /* CONFIG_OF_CONTROL */ + /* + * If the bootsecure option was chosen, use secure_boot_cmd(). + * Always use 'env' in this case, since bootsecure requres that + * the bootcmd was specified in the FDT too. + */ + if (fdtdec_get_config_int(gd->fdt_blob, "bootsecure", 0)) + secure_boot_cmd(env); + }

debug ("### main_loop: bootcmd="%s"\n", s ? s : "<UNDEFINED>");

if (bootdelay != -1 && s && !abortboot(bootdelay)) { -# ifdef CONFIG_AUTOBOOT_KEYED - int prev = disable_ctrlc(1); /* disable Control C checking */ -# endif + int prev; + + /* disable Control C checking */ + if (autoconf_autoboot_keyed()) + prev = disable_ctrlc(1);

run_command_list(s, -1, 0);

-# ifdef CONFIG_AUTOBOOT_KEYED - disable_ctrlc(prev); /* restore Control C checking */ -# endif + /* restore Control C checking */ + if (autoconf_autoboot_keyed()) + disable_ctrlc(prev); }

-# ifdef CONFIG_MENUKEY - if (menukey == CONFIG_MENUKEY) { + if (autoconf_menukey() && menukey == autoconf_menukey()) { s = getenv("menucmd"); if (s) run_command_list(s, -1, 0); } -#endif /* CONFIG_MENUKEY */ }

/****************************************************************************/ diff --git a/include/menu.h b/include/menu.h index 7af5fdb..1cdcb88 100644 --- a/include/menu.h +++ b/include/menu.h @@ -28,7 +28,5 @@ int menu_item_add(struct menu *m, char *item_key, void *item_data); int menu_destroy(struct menu *m); void menu_display_statusline(struct menu *m);

-#if defined(CONFIG_MENU_SHOW) int menu_show(int bootdelay); -#endif #endif /* __MENU_H__ */

Allow parser selection to make use of autoconf instead of #ifdefs. This requires us to make header includes unconditional, but this is simpler anyway.

Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Joe Hershberger joe.hershberger@ni.com --- Changes in v3: None Changes in v2: None

common/main.c | 92 +++++++++++++++++++++++++++------------------------------- include/hush.h | 2 -- 2 files changed, 42 insertions(+), 52 deletions(-)

diff --git a/common/main.c b/common/main.c index 974fc82..d49692d 100644 --- a/common/main.c +++ b/common/main.c @@ -32,14 +32,11 @@ #include <command.h> #include <fdtdec.h> #include <fdt_support.h> +#include <hush.h> #include <malloc.h> #include <menu.h> #include <version.h>

-#ifdef CONFIG_SYS_HUSH_PARSER -#include <hush.h> -#endif - #include <post.h> #include <linux/ctype.h>

@@ -381,12 +378,10 @@ static void process_boot_delay(void)

void main_loop(void) { -#ifndef CONFIG_SYS_HUSH_PARSER static char lastcommand[CONFIG_SYS_CBSIZE] = { 0, }; int len; int rc = 1; int flag; -#endif #ifdef CONFIG_PREBOOT char *p; #endif @@ -446,12 +441,11 @@ void main_loop(void) /* * Main Loop for Monitor Command Processing */ -#ifdef CONFIG_SYS_HUSH_PARSER - parse_file_outer(); - /* This point is never reached */ - for (;;); -#else - for (;;) { + if (autoconf_sys_hush_parser()) { + parse_file_outer(); + /* This point is never reached */ + for (;;); + } else { if (autoconf_boot_retry_time() && rc >= 0) { /* Saw enough of a valid command to * restart the timeout. @@ -486,7 +480,6 @@ void main_loop(void) lastcommand[0] = 0; } } -#endif /*CONFIG_SYS_HUSH_PARSER*/ }

/* @@ -1184,7 +1177,6 @@ int parse_line (char *line, char *argv[])

/****************************************************************************/

-#ifndef CONFIG_SYS_HUSH_PARSER static void process_macros (const char *input, char *output) { char c, prev; @@ -1400,7 +1392,6 @@ static int builtin_run_command(const char *cmd, int flag)

return rc ? rc : repeatable; } -#endif

/* * Run a command using the selected parser. @@ -1411,22 +1402,21 @@ static int builtin_run_command(const char *cmd, int flag) */ int run_command(const char *cmd, int flag) { -#ifndef CONFIG_SYS_HUSH_PARSER - /* - * builtin_run_command can return 0 or 1 for success, so clean up - * its result. - */ - if (builtin_run_command(cmd, flag) == -1) - return 1; - - return 0; -#else - return parse_string_outer(cmd, + if (autoconf_sys_hush_parser()) { + return parse_string_outer(cmd, FLAG_PARSE_SEMICOLON | FLAG_EXIT_FROM_LOOP); -#endif + } else { + /* + * builtin_run_command can return 0 or 1 for success, so + * clean up its result. + */ + if (builtin_run_command(cmd, flag) == -1) + return 1; + + return 0; + } }

-#ifndef CONFIG_SYS_HUSH_PARSER /** * Execute a list of command separated by ; or \n using the built-in parser. * @@ -1467,7 +1457,6 @@ static int builtin_run_command_list(char *cmd, int flag)

return rcode; } -#endif

int run_command_list(const char *cmd, int len, int flag) { @@ -1477,13 +1466,16 @@ int run_command_list(const char *cmd, int len, int flag)

if (len == -1) { len = strlen(cmd); -#ifdef CONFIG_SYS_HUSH_PARSER - /* hush will never change our string */ - need_buff = 0; -#else - /* the built-in parser will change our string if it sees \n */ - need_buff = strchr(cmd, '\n') != NULL; -#endif + if (autoconf_sys_hush_parser()) { + /* hush will never change our string */ + need_buff = 0; + } else { + /* + * the built-in parser will change our string if it + * sees \n + */ + need_buff = strchr(cmd, '\n') != NULL; + } } if (need_buff) { buff = malloc(len + 1); @@ -1492,20 +1484,20 @@ int run_command_list(const char *cmd, int len, int flag) memcpy(buff, cmd, len); buff[len] = '\0'; } -#ifdef CONFIG_SYS_HUSH_PARSER - rcode = parse_string_outer(buff, FLAG_PARSE_SEMICOLON); -#else - /* - * This function will overwrite any \n it sees with a \0, which - * is why it can't work with a const char *. Here we are making - * using of internal knowledge of this function, to avoid always - * doing a malloc() which is actually required only in a case that - * is pretty rare. - */ - rcode = builtin_run_command_list(buff, flag); - if (need_buff) - free(buff); -#endif + if (autoconf_sys_hush_parser()) { + rcode = parse_string_outer(buff, FLAG_PARSE_SEMICOLON); + } else { + /* + * This function will overwrite any \n it sees with a \0, which + * is why it can't work with a const char *. Here we are making + * using of internal knowledge of this function, to avoid always + * doing a malloc() which is actually required only in a case + * that is pretty rare. + */ + rcode = builtin_run_command_list(buff, flag); + if (need_buff) + free(buff); + }

return rcode; } diff --git a/include/hush.h b/include/hush.h index ecf9222..12c55f4 100644 --- a/include/hush.h +++ b/include/hush.h @@ -36,7 +36,5 @@ int set_local_var(const char *s, int flg_export); void unset_local_var(const char *name); char *get_local_var(const char *s);

-#if defined(CONFIG_HUSH_INIT_VAR) extern int hush_init_var (void); #endif -#endif

Define a simple debug condition at the top of the file, to avoid using lots of #ifdefs later on.

Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Joe Hershberger joe.hershberger@ni.com --- Changes in v3: None Changes in v2: None

common/main.c | 58 +++++++++++++++++++++++----------------------------------- 1 file changed, 23 insertions(+), 35 deletions(-)

diff --git a/common/main.c b/common/main.c index a464620..8ea9475 100644 --- a/common/main.c +++ b/common/main.c @@ -49,7 +49,11 @@ void show_boot_progress (int val) __attribute__((weak, alias("__show_boot_progre

#define MAX_DELAY_STOP_STR 32

-#undef DEBUG_PARSER +#define DEBUG_PARSER 0 /* set to 1 to debug */ + +#define debug_parser(fmt, args...) \ + debug_cond(DEBUG_PARSER, fmt, ##args) +

char console_buffer[CONFIG_SYS_CBSIZE + 1]; /* console I/O buffer */

@@ -1105,9 +1109,7 @@ int parse_line (char *line, char *argv[]) { int nargs = 0;

-#ifdef DEBUG_PARSER - printf ("parse_line: "%s"\n", line); -#endif + debug_parser("parse_line: "%s"\n", line); while (nargs < CONFIG_SYS_MAXARGS) {

/* skip any white space */ @@ -1116,10 +1118,8 @@ int parse_line (char *line, char *argv[])

if (*line == '\0') { /* end of line, no more args */ argv[nargs] = NULL; -#ifdef DEBUG_PARSER - printf ("parse_line: nargs=%d\n", nargs); -#endif - return (nargs); + debug_parser("parse_line: nargs=%d\n", nargs); + return nargs; }

argv[nargs++] = line; /* begin of argument string */ @@ -1130,10 +1130,8 @@ int parse_line (char *line, char *argv[])

if (*line == '\0') { /* end of line, no more args */ argv[nargs] = NULL; -#ifdef DEBUG_PARSER - printf ("parse_line: nargs=%d\n", nargs); -#endif - return (nargs); + debug_parser("parse_line: nargs=%d\n", nargs); + return nargs; }

*line++ = '\0'; /* terminate current arg */ @@ -1141,9 +1139,7 @@ int parse_line (char *line, char *argv[])

printf ("** Too many args (max. %d) **\n", CONFIG_SYS_MAXARGS);

-#ifdef DEBUG_PARSER - printf ("parse_line: nargs=%d\n", nargs); -#endif + debug_parser("parse_line: nargs=%d\n", nargs); return (nargs); }

@@ -1160,12 +1156,10 @@ static void process_macros (const char *input, char *output) /* 1 = waiting for '(' or '{' */ /* 2 = waiting for ')' or '}' */ /* 3 = waiting for ''' */ -#ifdef DEBUG_PARSER char *output_start = output;

- printf ("[PROCESS_MACROS] INPUT len %d: "%s"\n", strlen (input), - input); -#endif + debug_parser("[PROCESS_MACROS] INPUT len %zd: "%s"\n", strlen(input), + input);

prev = '\0'; /* previous character */

@@ -1253,10 +1247,8 @@ static void process_macros (const char *input, char *output) else *(output - 1) = 0;

-#ifdef DEBUG_PARSER - printf ("[PROCESS_MACROS] OUTPUT len %d: "%s"\n", - strlen (output_start), output_start); -#endif + debug_parser("[PROCESS_MACROS] OUTPUT len %zd: "%s"\n", + strlen(output_start), output_start); }

/**************************************************************************** @@ -1287,12 +1279,12 @@ static int builtin_run_command(const char *cmd, int flag) int repeatable = 1; int rc = 0;

-#ifdef DEBUG_PARSER - printf ("[RUN_COMMAND] cmd[%p]="", cmd); - puts (cmd ? cmd : "NULL"); /* use puts - string may be loooong */ - puts (""\n"); -#endif - + debug_parser("[RUN_COMMAND] cmd[%p]="", cmd); + if (DEBUG_PARSER) { + /* use puts - string may be loooong */ + puts(cmd ? cmd : "NULL"); + puts(""\n"); + } clear_ctrlc(); /* forget any previous Control C */

if (!cmd || !*cmd) { @@ -1310,9 +1302,7 @@ static int builtin_run_command(const char *cmd, int flag) * repeatable commands */

-#ifdef DEBUG_PARSER - printf ("[PROCESS_SEPARATORS] %s\n", cmd); -#endif + debug_parser("[PROCESS_SEPARATORS] %s\n", cmd); while (*str) {

/* @@ -1341,9 +1331,7 @@ static int builtin_run_command(const char *cmd, int flag) } else str = sep; /* no more commands for next pass */ -#ifdef DEBUG_PARSER - printf ("token: "%s"\n", token); -#endif + debug_parser("token: "%s"\n", token);

/* find macros in this token and replace them */ process_macros (token, finaltoken);