[coreboot] libpayload: Missing inclusion of `libpayload-config.h`
Nico Huber
nico.h at gmx.de
Thu Mar 28 15:44:45 CET 2013
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
More information about the coreboot
mailing list