From patchwork Sat Oct 26 15:14:10 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 286286 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 1840A2C0199 for ; Sun, 27 Oct 2013 02:17:00 +1100 (EST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 1AD334A1F7; Sat, 26 Oct 2013 17:16:57 +0200 (CEST) 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 4T6FjIgiaROD; Sat, 26 Oct 2013 17:16:56 +0200 (CEST) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 5ACAC4A24B; Sat, 26 Oct 2013 17:16:32 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 41C554A20D for ; Sat, 26 Oct 2013 17:16:24 +0200 (CEST) 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 j59AVIqs7Rfr for ; Sat, 26 Oct 2013 17:16:17 +0200 (CEST) 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-pa0-f74.google.com (mail-pa0-f74.google.com [209.85.220.74]) by theia.denx.de (Postfix) with ESMTPS id A66184A1FD for ; Sat, 26 Oct 2013 17:16:10 +0200 (CEST) Received: by mail-pa0-f74.google.com with SMTP id fb1so491285pad.5 for ; Sat, 26 Oct 2013 08:16:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=c7/o5u3LRxKPfWbOQmPMyEyX1TmquqfGzCJuNjyohXY=; b=a3vahPNMeKGYr3V/VyD0B7Wqq0+puvXBVl1i9U4xSfDwTtmOCIpwcz+g18uRG0Hv9u sSm7Yh4MyD2wytFunMpl7jjujbuV7QryOzsx/KTNTlsbOXBs7D2bIveWzvpVK2RhkNnP uSFXoqinimHLkbszfy66VmCiB0Elr7CvAnd0oe2KYpHhj2FObemgvJXNGQ51Wz04GJB9 qx99KhVCDY/Gs5pk90Sb9Hvgyab44Jsrd279rU/0EggdmYs8rB/g+xe0qybYvmpWKp57 Q6tHucDlHOUaP0s9uOAnxKxyWGkonR/AzWiIj/si6v/QN+VsQZwOKmizmk5ZdKuJjhyc Q3oQ== X-Gm-Message-State: ALoCoQmZZqN4A9f+NIdCzvyeTi+W2vG4PIUVG9kw/M2qM5jEp2drX39VdWUPb5si55QCd2lKF9Aea7xNrIb4Y/i7hCZjcnxpvO3ZJLcmpRjrzG0s+r2Bj+jthefy9qno1z1iwblMmUN3ZCFyHoNVXUzHkWR2hgcIjPeroehR+U4GkAoiic7Q0fdZcYRpSBLaPAolqAaek+N9 X-Received: by 10.66.20.100 with SMTP id m4mr5491328pae.36.1382800568292; Sat, 26 Oct 2013 08:16:08 -0700 (PDT) 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 m48si674132yho.6.2013.10.26.08.16.08 for (version=TLSv1.1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Sat, 26 Oct 2013 08:16:08 -0700 (PDT) Received: from kaki.bld.corp.google.com (kaki.bld.corp.google.com [172.29.216.32]) by corp2gmr1-2.hot.corp.google.com (Postfix) with ESMTP id ED7175A429B; Sat, 26 Oct 2013 08:16:07 -0700 (PDT) Received: by kaki.bld.corp.google.com (Postfix, from userid 121222) id 6558F220836; Sat, 26 Oct 2013 09:15:23 -0600 (MDT) From: Simon Glass To: U-Boot Mailing List Date: Sat, 26 Oct 2013 09:14:10 -0600 Message-Id: <1382800457-26608-2-git-send-email-sjg@chromium.org> X-Mailer: git-send-email 1.8.4.1 In-Reply-To: <1382800457-26608-1-git-send-email-sjg@chromium.org> References: <1382800457-26608-1-git-send-email-sjg@chromium.org> Cc: David Hendrix , Joe Hershberger , Tom Rini , Vadim Bendebury Subject: [U-Boot] [PATCH v4 1/8] 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 v4: - Rebase on current master Changes in v3: - Add comment as to why we use [A-Za-z0-9_][A-Za-z0-9_]* - Rename sed scripts to more useful names - Update config_xxx() to autoconf_xxx() in comments/README/sed - Update config_xxx_enabled() to autoconf_has_xxx() in comments/README/sed Changes in v2: - Add a grep to the sed/sort pipe to speed up processing - Fix up a few errors and comments in the original RFC - Split out changes to main.c into separate patches - Use autoconf_...() instead of config_...() - Use autoconf_has_...() instead of config_..._enabled() 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 2d18d27..3cc6db0 100644 --- a/Makefile +++ b/Makefile @@ -636,6 +636,7 @@ depend dep: $(TIMESTAMP_FILE) $(VERSION_FILE) \ $(obj)include/spl-autoconf.mk \ $(obj)include/tpl-autoconf.mk \ $(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 \ @@ -734,6 +735,45 @@ $(obj)include/spl-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)include/spl-autoconf.mk \ $(obj)include/tpl-autoconf.mk \ @@ -833,7 +873,8 @@ unconfig: $(obj)board/*/config.tmp $(obj)board/*/*/config.tmp \ $(obj)include/autoconf.mk $(obj)include/autoconf.mk.dep \ $(obj)include/spl-autoconf.mk \ - $(obj)include/tpl-autoconf.mk + $(obj)include/tpl-autoconf.mk \ + $(obj)include/generated/autoconf.h %_config:: unconfig @$(MKCONFIG) -A $(@:_config=) diff --git a/README b/README index f0eedbb..428332c 100644 --- a/README +++ b/README @@ -5912,11 +5912,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 409515f..f07f740 100644 --- a/include/common.h +++ b/include/common.h @@ -19,6 +19,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 +}