[U-Boot] [PATCH v4 0/8] Provide a mechanism to avoid using #ifdef everywhere
Simon Glass
sjg at chromium.org
Sat Oct 26 17:14:09 CEST 2013
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. 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. For
people coming newly into the code base, #ifdefs can be a big barrier.
The use of #ifdef in U-Boot has possibly got a little out of hand. In an
attempt to turn the tide, this series includes a patch which provides a way
to make CONFIG macros available to C code without using the preprocessor.
This makes it possible to use standard C conditional features such as
if/then instead of #ifdef. A README update exhorts compliance.
As an example of how to use this, this series replaces all but two #ifdefs
from the main code body of common/main.c, which is one of the largest users
of #ifdef, even after a recent cleanup:
$ for f in $(find . -name *.c); do echo $(grep -c "ifdef" $f) $f; \
done |sort -nr |head
82 ./common/board_r.c
57 ./common/board_f.c
57 ./arch/powerpc/cpu/mpc83xx/cpu_init.c
49 ./common/main.c
49 ./arch/powerpc/lib/board.c
45 ./drivers/video/cfb_console.c
40 ./drivers/mtd/cfi_flash.c
38 ./net/tftp.c
37 ./drivers/usb/host/ohci-hcd.c
36 ./drivers/fpga/ivm_core.c
Code size for this series seems to be roughly neutral (below numbers are
average change in byte size for each region:
blackfin: (for 21/35 boards) all -13.7 text -13.7
x86: (for 1/1 boards) bss +4.0 data +4.0 text -8.0
avr32: (for 10/10 boards) all -7.2 data -2.0 text -5.2
m68k: (for 35/50 boards) all -30.9 text -30.9
powerpc: (for 673/675 boards) all -20.8 bss +0.4 data +0.0 rodata -0.7
spl/u-boot-spl:all +0.1 spl/u-boot-spl:data +0.1 text -20.5
sandbox: (for 1/1 boards) all +16.0 bss +16.0
sh: (for 16/21 boards) all -76.2 bss +3.2 rodata -15.5 text -64.0
nios2: (for 3/3 boards) all +18.7 bss +1.3 data -1.3 text +18.7
microblaze: (for 1/1 boards) all -864.0 bss +8.0 rodata -216.0 text -656.0
arm: (for 286/344 boards) all -51.7 bss -5.7 data +0.2 rodata -3.9
spl/u-boot-spl:all +0.2 spl/u-boot-spl:bss +0.2 text -42.4
nds32: (for 3/3 boards) all -52.0 text -52.0
Note that a config_drop.h file is added - this defines all the CONFIGs
which are not used in any board config file. Without this, autoconf cannot
define the macros for this CONFIGs.
Compile time for main.c does not seem to be any different in my tests. The
time to perform the 'dep' step (which now creates autoconf.h) increases,
from about 2.8s to about 4.6s. This additional time is used to grep, sed
and sort the contents of all the header file in U-Boot. The time for an
incremental build is not affected.
It would be much more efficient to maintain a list of all available CONFIG
defines, but no such list exists at present.
Buildman output shows no additional failures from mainline (commit 02 is
a bug in buildman and commit 11 will be sent separately):
$ ./tools/buildman/buildman -b us-config7 -s
Summary of 10 commits for 1178 boards (32 threads, 1 job per thread)
01: usb: rename board_usb_init_type to usb_init_type
blackfin: + bf561-acvilon cm-bf561 blackstamp br4 bct-brettl2 cm-bf527 dnp5370 bf506f-ezkit ip04 bf527-sdp bf609-ezkit bf537-stamp bf527-ezkit-v2 cm-bf537e tcm-bf518 cm-bf537u bf537-pnav cm-bf533 pr1 bf533-ezkit ibf-dsp561 bf537-srv1 cm-bf548 bf537-minotaur bf538f-ezkit bf548-ezkit bf525-ucr2 blackvme bf527-ezkit tcm-bf537 bf533-stamp bf518f-ezbrd bf527-ad7160-eval bf526-ezbrd bf561-ezkit
m68k: + M54455EVB_a66 M5329AFEE M5249EVB idmr M5208EVBE eb_cpu5282 M5475FFE M54451EVB astro_mcf5373l M54418TWR_serial_rmii M54455EVB_intel M5282EVB M54455EVB_i66 M5475GFE M5253DEMO M54455EVB_stm33 M5485BFE M5485DFE TASREG M5329BFEE M52277EVB M5475EFE M5475CFE cobra5272 M5485AFE M53017EVB M5485HFE M5235EVB M5253EVBE M54418TWR_nand_mii M54418TWR_nand_rmii_lowfreq M5475BFE M5475DFE M5275EVB M52277EVB_stmicro eb_cpu5282_internal M54451EVB_stmicro M5271EVB M5485GFE M5485EFE M5485FFE M54418TWR M5235EVB_Flash32 M5373EVB M54418TWR_nand_rmii M54418TWR_serial_mii M5485CFE M54455EVB M5475AFE M5272C3
powerpc: + MVBLM7 MVSMR lcd4_lwmon5
sparc: + grsim grsim_leon2 gr_cpci_ax2000 gr_xc3s_1500 gr_ep2s60
sh: + rsk7269 rsk7264 sh7752evb rsk7203 sh7757lcr
microblaze: + microblaze-generic
openrisc: + openrisc-generic
arm: + pm9g45 qong palmtc zipitz2 omap3_zoom1 omap3_overo goflexhome davinci_sonata VCMA9 iconnect km_kirkwood_pci ib62x0 lubbock ethernut5 zynq_dcc vpac270_nor_128 colibri_pxa270 sheevaplug kzm9g am3517_crane zynq tnetv107x_evm xaeniax magnesium palmtreo680 kmsuv31 polaris omap3_sdp3430 imx27lite mgcoge3un vpac270_nor_256 pxa255_idp kmnusa kmcoge5un am3517_evm nhk8815_onenand openrd_client openrd_base nhk8815 km_kirkwood dns325 mcx lp8x4x vpac270_ond_256 smdk2410 h2200 jornada balloon3 omap3_evm omap3_logic dockstar portl2 palmld openrd_ultimate trizepsiv pogo_e02 pm9263 devkit8000
02: Signed-off-by: Troy Kisky <troy.kisky at boundarydevices.com>
arm: + am43xx_evm dra7xx_evm
powerpc: + T1040QDS
03: Implement autoconf header file
arm: am43xx_evm dra7xx_evm
powerpc: T1040QDS
04: main: Use autoconf for boot retry feature
05: main: Remove CONFIG #ifdefs from the abortboot() code
06: main: Use autoconf to remove #ifdefs around process_boot_delay()
07: main: Use autoconf for boot_delay code
08: main: Use autoconf for parser selection
09: main: Use autoconf in command line reading
10: main: Use autoconf in main_loop()
11: fix Cover-letter-cc
Changes in v4:
- Rebase on current master
- Split out new patch to remove #ifdefs around process_boot_delay()
- Tidy up code style nits with new checkpatch
Changes in v3:
- Add comment as to why we use [A-Za-z0-9_][A-Za-z0-9_]*
- Fix missing && in if() statement
- Remove the extra config_of_libfdt() condition in main_loop()
- Remove unneeded retry_min variable
- Rename sed scripts to more useful names
- Simplify code for finding out bootdelay from config or environment
- 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()
Simon Glass (8):
Implement autoconf header file
main: Use autoconf for boot retry feature
main: Remove CONFIG #ifdefs from the abortboot() code
main: Use autoconf to remove #ifdefs around process_boot_delay()
main: Use autoconf for boot_delay code
main: Use autoconf for parser selection
main: Use autoconf in command line reading
main: Use autoconf in main_loop()
Makefile | 43 ++-
README | 87 +++++-
common/main.c | 629 ++++++++++++++++++-----------------------
include/command.h | 2 -
include/common.h | 6 +-
include/config_drop.h | 17 ++
include/fdt_support.h | 4 +-
include/hush.h | 2 -
include/menu.h | 2 -
tools/scripts/define2value.sed | 37 +++
tools/scripts/define2zero.sed | 32 +++
11 files changed, 492 insertions(+), 369 deletions(-)
create mode 100644 include/config_drop.h
create mode 100644 tools/scripts/define2value.sed
create mode 100644 tools/scripts/define2zero.sed
--
1.8.4.1
More information about the U-Boot
mailing list