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

Nico Huber nico.h at gmx.de
Wed Mar 27 20:49:18 CET 2013


Hello Paul,

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). Therefore, the use of `#ifdef` is correct. You can change
`#if CONFIG_...` to `#ifdef` where appropriate.

Regards,
Nico



More information about the coreboot mailing list