[coreboot] libpayload: Missing inclusion of `libpayload-config.h`

[Nico Huber]

Dear Paul,

Am 28.03.2013 12:37, schrieb Paul Menzel:
> 
> Am Mittwoch, den 27.03.2013, 20:49 +0100 schrieb Nico Huber:
> 
>> Am 27.03.2013 14:04, schrieb Paul Menzel:
>>> Dear coreboot folks,
>>>
>>>
>>> using latest master
>>>
>>>         commit 3cc0d1eb3f611cb7bf0e45d8ccdb0c84f54f54dc
>>>         Author: David Hendricks <dhendrix at chromium.org>
>>>         Date:   Tue Mar 26 16:28:21 2013 -0700
>>>         
>>>             exynos5250: assign RAM resources in cpu_init()
>>>
>>>         […]
>>>
>>>             Reviewed-on: http://review.coreboot.org/2923
>>>
>>> it looks like libpayload’s PDCurses backend is not including
>>> `libpayload-config.h` in some files so the configuration is ignored,
>>> right? I noticed this due to the following warning.
>>>
>>>         /src/coreboot/payloads/libpayload(master) $ make clean
>>>         /src/coreboot/payloads/libpayload(master) $ make
>>>         […]
>>>             CC         curses/pdcurses-backend/pdcscrn.libcurses.o
>>>         curses/pdcurses-backend/pdcscrn.c: In function 'PDC_scr_open':
>>>         curses/pdcurses-backend/pdcscrn.c:75:5: warning: "CONFIG_SPEAKER" is not defined [-Wundef]
>>>         […]
>>>
>> I don't see the include missing: `pdcscrn.c` includes `lppdc.h` which,
>> in turn, includes `libpayload-config.h`.
>>
>>> No warnings are printed for the other files, because `#ifdef` is used
>>> there. But these macros will never be defined, when
>>> `libpayload-config.h` is not included, if I am not mistaken.
>>>
>>> So is the solution to just include `config.h` or `libpayload-config.h`
>>> everywhere?
>>>
>>>         /src/coreboot/payloads/libpayload(master) $ grep -R CONFIG_SPEA .
>>>         ./configs/defconfig:CONFIG_SPEAKER=y
>>>         ./curses/pdcurses-backend/pdcscrn.c:#ifdef CONFIG_SPEAKER
>>>         ./curses/pdcurses-backend/pdcutil.c:#ifdef CONFIG_SPEAKER
>>>         ./curses/tinycurses.c:#ifdef CONFIG_SPEAKER
>>>         ./drivers/Makefile.inc:libc-$(CONFIG_SPEAKER) += speaker.c
>>>         ./build/config.h:#define CONFIG_SPEAKER 1
>>>         ./build/auto.conf:CONFIG_SPEAKER=y
>>>         ./build/libpayload-config.h:#define CONFIG_SPEAKER 1
>>>         ./.config:CONFIG_SPEAKER=y
>>>         ./tests/libpayload-config.h:#define CONFIG_SPEAKER 1
>>>
>>> If you tell me the preferred solution, I am going to submit a patch.
>>>
>> Libpayload's config mechanism doesn't define unset options (unlike the one
>> coreboot uses).
> 
> My problem, as seen in my paste above, is that `CONFIG_SPEAKER` is
> defined and I still get the error.
> 
I can't reproduce the warning with CONFIG_SPEAKER being set. I've tested the
following with the reference toolchain (plus current toolchain of ArchLinux).
Starting with Stefan's patch [1] for the size_t issue:

$ git fetch http://review.coreboot.org/coreboot refs/changes/33/2933/1
Von http://review.coreboot.org/coreboot
 * branch            refs/changes/33/2933/1 -> FETCH_HEAD

$ git checkout FETCH_HEAD
[...]
Zweigspitze (HEAD) ist jetzt bei c33a40b... libpayload: drop size_t and ssize_t

$ make distclean

$ rm -f .xcompile

$ yes '' | make oldconfig
[...]

$ # check used toolchain:
$ cat .xcompile
[...]
# elf32-i386 toolchain (/home/icoN/workspace/coreboot/coreboot/payloads/libpayload/../../util/crossgcc/xgcc/bin/i386-elf-gcc)
[...]

$ # make sure CONFIG_SPEAKER is unset:
$ sed -i -e 's,CONFIG_SPEAKER=y,# CONFIG_SPEAKER is not set,' .config

$ make oldconfig
[...]

$ make build/curses/pdcurses-backend/pdcscrn.libcurses.o
    CC         curses/pdcurses-backend/pdcscrn.libcurses.o
curses/pdcurses-backend/pdcscrn.c: In function 'PDC_scr_open':
curses/pdcurses-backend/pdcscrn.c:75:5: warning: "CONFIG_SPEAKER" is not defined [-Wundef]

$ # now set CONFIG_SPEAKER:
$ sed -i -e 's,# CONFIG_SPEAKER is not set,CONFIG_SPEAKER=y,' .config

$ make oldconfig
[...]

$ grep CONFIG_SPEAKER build/libpayload-config.h
#define CONFIG_SPEAKER 1

$ make build/curses/pdcurses-backend/pdcscrn.libcurses.o
build/libpayload-config.h build/config.h differ: byte 99, line 4
    CC         curses/pdcurses-backend/pdcscrn.libcurses.o

The warning disappears with CONFIG_SPEAKER being set.

Regards,
Nico

[1] http://review.coreboot.org/2933