No subject
Thu Oct 11 07:38:59 CEST 2012
interpreting wrong) it lists all things NOT enabled. But I guess you
are wanting exact text with which to prune the -all.tmp. Could you
craft it such that you can use .tmp directly to prune -all.tmp instead
of having to generate this -enable.tmp file (which is misleading and
potentially a waste of sed time)? Please rename define2list.sed and
define2conf.sed to something which has meaning. This is quite hard to
read as it.
> + 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 +809,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..3e89551 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
Isn't it true that dead code stripping doesn't strip variables only
used by that dead code? So stack usage is wasted everywhere in this
new model?
> +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 config_xxx() value
Use the new names here...
> +
> +for enabled options, or:
> +
> +# #define config_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 (config_xxx())
> + do_something;
> + else
> + do_something_else;
> +
> +The compiler will see that config_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 (config_xxx() && !config_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 '_enabled' suffix is provided, where
> +the value is always either 0 or 1:
> +
> + // Will work even if boaard config has '#define CONFIG_BOOTDELAY 0'
> + if (config_bootdelay_enabled())
> + 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 config_xxx() everywhere else
> + 3. Mark your functions and data structures static where possible
> + 4. Use the config_xxx_enabled() variants only if essential
Fix names here.
> + 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..bf2beaa
> --- /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 referred to anywhere in U-Boot */
You mean not referred to by any config header?
> +#define CONFIG_MENUPROMPT "Auto-boot prompt"
> +#define CONFIG_MENUKEY
> +#define CONFIG_UPDATE_TFTP
> +
> +#endif
> diff --git a/tools/scripts/define2conf.sed b/tools/scripts/define2conf.sed
> new file mode 100644
> index 0000000..2c4a2ef
> --- /dev/null
> +++ b/tools/scripts/define2conf.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 config_xxx() value
> +# #define config_xxx_enabled() 1
Fix the names here.
> +#
> +
> +# Macros with parameters are ignored.
> +/^#define CONFIG_[A-Za-z0-9_][A-Za-z0-9_]*(/ {
> + d
Any reason not to use "[A-Za-z0-9_]+" instead of "[A-Za-z0-9_][A-Za-z0-9_]*"?
> +}
> +
> +# 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/define2list.sed b/tools/scripts/define2list.sed
Please change the name to be more clear as to its purpose. "list"
doesn't convey to me that it is all configs that should be set as
undefined.
> new file mode 100644
> index 0000000..152280d
> --- /dev/null
> +++ b/tools/scripts/define2list.sed
> @@ -0,0 +1,31 @@
> +#
> +# Sed script to parse CPP macros and generate a list of CONFIG macros
> +#
> +# This converts:
> +# #define CONFIG_XXX value
> +#into:
> +# #define config_xxx() 0
> +# #define config_xxx_enabled() 0
Fix names here.
> +
> +# Macros with parameters are ignored.
> +/^#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
> +}
> --
> 1.8.1.3
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
More information about the U-Boot
mailing list