[PATCH] common/board_f.c: use #ifdefs a little more consistently

Rasmus Villemoes rasmus.villemoes at prevas.dk
Fri Feb 28 18:24:58 CET 2020


On 28/02/2020 16.46, Tom Rini wrote:
> On Fri, Feb 28, 2020 at 08:42:21AM +0000, Rasmus Villemoes wrote:
>> On 28/02/2020 00.40, Simon Glass wrote:

>>> Using if() is preferable to #if if there is no cost.
>>
>> Completely agree, and I also prefer to have the linker eliminate unused
>> functions rather than cluttering the C code with #ifdefs. But that can't
>> be used in this case.
>>
>> Anyway, this wasn't primarily to save 112 bytes or whatnot from the
>> U-Boot image, just to use one style a little more consistently.
> 
> Perhaps we could come up with a little more macro-magic?  In
> psuedo-code:
> #define RESERVE_INIT_SEQ_F_ENTRY(fn) \
> #if CONFIG_IS_ENABLED(toupper(fn))
> reserve_##fn
> #endif
> #endif

I'm afraid that's rather far from something that can be implemented;
macros cannot expand to other preprocessor directives, and toupper is
also pretty hard to do.

What we could do if we want to reduce #ifdefs while still eliminating
the no-op functions is to replace

#ifdef CONFIG_FOO
static int foo_init(void) {
  blabla;
  return 0;
}
#endif
...
init_sequence_f[] = { ...
#ifdef CONFIG_FOO
  foo_init,
#endif
...
}

by

static int foo_init(void) { /* always defined */
  blabla;
  return 0;
}

init_sequence_f[] = { ...
  IS_ENABLED(CONFIG_FOO) ? foo_init : NULL,
...
}

and teach the iterator to ignore NULL entries (I don't remember if we
use a NULL terminator or use ARRAY_SIZE; if the former one should switch
to the latter). It will still cost sizeof(void*) for the NULL entries,
but the function bodies (and on powerpc the .fixups) should be
eliminated, and there's not an #ifdef in sight.

If one also wants to get rid of those NULL entries, I did
https://lore.kernel.org/lkml/1444165543-2209-1-git-send-email-linux@rasmusvillemoes.dk/
a long time ago (see the COND_INITIALIZER()), but whether that can be
tweaked to still automatically avoid a "defined but not used" warning I
don't know.

> And then a little harder thinking about negative-case (OF_EMBED) ?  Or
> just have those be the few ifndef's?

That can be done in the same manner by just switching second and third
operand.

The problem is the function bodies which rely on symbols (for example
CONFIG_* values) that only exist if CONFIG_FOO.

Rasmus


More information about the U-Boot mailing list