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

[Paul Menzel]

Dear Nico,


thank you for your reply.


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.

> Therefore, the use of `#ifdef` is correct. You can change `#if
> CONFIG_...` to `#ifdef` where appropriate.

I (accidentally) pushed such a patch to Gerrit [1].


Thanks,

Paul


[1] http://review.coreboot.org/#/c/2934/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20130328/294c27e3/attachment.sig>