Dear coreboot folks,
using latest master
commit 3cc0d1eb3f611cb7bf0e45d8ccdb0c84f54f54dc Author: David Hendricks dhendrix@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] […]
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.
Thanks,
Paul
Hello Paul,
Am 27.03.2013 14:04, schrieb Paul Menzel:
Dear coreboot folks,
using latest master
commit 3cc0d1eb3f611cb7bf0e45d8ccdb0c84f54f54dc Author: David Hendricks <dhendrix@chromium.org> Date: Tue Mar 26 16:28:21 2013 -0700 exynos5250: assign RAM resources in cpu_init() […] Reviewed-on: http://review.coreboot.org/2923it 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 1If 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
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@chromium.org> Date: Tue Mar 26 16:28:21 2013 -0700 exynos5250: assign RAM resources in cpu_init() […] Reviewed-on: http://review.coreboot.org/2923it 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 1If 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
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@chromium.org> Date: Tue Mar 26 16:28:21 2013 -0700 exynos5250: assign RAM resources in cpu_init() […] Reviewed-on: http://review.coreboot.org/2923it 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 1If 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
-
Nico Huber -
Paul Menzel