From patchwork Tue Feb 26 16:10:54 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 223278 X-Patchwork-Delegate: trini@ti.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id 7D9FE2C0077 for ; Wed, 27 Feb 2013 03:13:34 +1100 (EST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 7AC934A140; Tue, 26 Feb 2013 17:13:26 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id cVBUZSrY+8tr; Tue, 26 Feb 2013 17:13:26 +0100 (CET) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 333F64A0C3; Tue, 26 Feb 2013 17:13:12 +0100 (CET) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 0A57B4A0C7 for ; Tue, 26 Feb 2013 17:13:05 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id LGJAE2CW-rGk for ; Tue, 26 Feb 2013 17:13:03 +0100 (CET) X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 NOT_IN_BL_NJABL=-1.5 (only DNSBL check requested) Received: from mail-ve0-f202.google.com (mail-ve0-f202.google.com [209.85.128.202]) by theia.denx.de (Postfix) with ESMTPS id 7410B4A0A1 for ; Tue, 26 Feb 2013 17:13:01 +0100 (CET) Received: by mail-ve0-f202.google.com with SMTP id m1so458619ves.1 for ; Tue, 26 Feb 2013 08:13:00 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references:x-gm-message-state; bh=HBJwRxQ3HMjvwGA8Wwdxj670G7QEDcIKn+yxBYiKiq8=; b=WuXjaOuvjYl3Rl6xMe5tMeKpKq8J+hx6rsobaMptEIbNLcKMd3nuEmFxraFu1xo8D/ Tf/wtSo9idbmDUK1mRq8ajARWiUq9+Cc+h6dRrFX8s5cNpOKQ+qx6pztgHQO0KQQOQXH BKxRm9GRXyAMpf2DDYwa8KZtkM+/pohR/94kxCX2zYJ7VSy45y+QKcnwnGQvyepezCuX cFCpF4TcwccgRxakJDZc/GNGA9AyuwLF/OZ6NekWXpweQhwwPTijGep6AC1KdC4+G3vD fxLh7irNQZaMMp8wSGVXqvt6QZz/nL/+JUL6pmDBOO00793ctFs7SEU8neYhAygcQAsJ 12+Q== X-Received: by 10.236.93.20 with SMTP id k20mr9662058yhf.4.1361895180321; Tue, 26 Feb 2013 08:13:00 -0800 (PST) Received: from corp2gmr1-2.hot.corp.google.com (corp2gmr1-2.hot.corp.google.com [172.24.189.93]) by gmr-mx.google.com with ESMTPS id t30si64571yhi.6.2013.02.26.08.13.00 (version=TLSv1.1 cipher=AES128-SHA bits=128/128); Tue, 26 Feb 2013 08:13:00 -0800 (PST) Received: from kaka.mtv.corp.google.com (kaka.mtv.corp.google.com [172.22.73.79]) by corp2gmr1-2.hot.corp.google.com (Postfix) with ESMTP id 029195A40A5; Tue, 26 Feb 2013 08:13:00 -0800 (PST) Received: by kaka.mtv.corp.google.com (Postfix, from userid 121222) id 99EF116051A; Tue, 26 Feb 2013 08:12:59 -0800 (PST) From: Simon Glass To: U-Boot Mailing List Date: Tue, 26 Feb 2013 08:10:54 -0800 Message-Id: <1361895069-7343-2-git-send-email-sjg@chromium.org> X-Mailer: git-send-email 1.8.1.3 In-Reply-To: <1361895069-7343-1-git-send-email-sjg@chromium.org> References: <1361895069-7343-1-git-send-email-sjg@chromium.org> X-Gm-Message-State: ALoCoQlKeumfJpImkd6twzFRuRpeLofqsQGWNWX1lXXHoiqIdorX84KNs7ltazUnhaSsET3iD0MBhN0UlKmckt5A5bOWENEfhmJTgS14HPFJwS3CVSRAiQhGNceFNMIDxQVTdDUWEyanKWnXps4v4FaV69YXLlCdyhxI7wlLQyxq12ZQBzXlPyub6OrldJyqJ1SwiMmRYXd/ Cc: David Hendrix , Graeme Russ , Joe Hershberger , Tom Rini , Vadim Bendebury Subject: [U-Boot] [PATCH v3 01/16] Implement autoconf header file X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.11 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: u-boot-bounces@lists.denx.de Errors-To: u-boot-bounces@lists.denx.de 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 --- 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 +#ifndef DO_DEPS_ONLY +#include +#endif #include #include #include 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 +}