Asami Doi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33287 )
Change subject: payloads/libpayload: Update a Makefile and a wrapper of a compiler in libpayload in order to build sample payload for QEMU/ARM. ......................................................................
Patch Set 6:
(5 comments)
https://review.coreboot.org/#/c/33287/6/payloads/libpayload/README File payloads/libpayload/README:
https://review.coreboot.org/#/c/33287/6/payloads/libpayload/README@28 PS6, Line 28: you might have to install the 32bit libgcc version, otherwise
This doesn't really make sense, the issue this is talking about is x86 specific. […]
Thank you for your comment. I will apply your comment in the next update.
https://review.coreboot.org/#/c/33287/6/payloads/libpayload/bin/lpgcc File payloads/libpayload/bin/lpgcc:
https://review.coreboot.org/#/c/33287/6/payloads/libpayload/bin/lpgcc@203 PS6, Line 203: $DEFAULT_CC $_LDFLAGS $HEAD_O $CMDLINE $_CFLAGS -lpayload $_LIBGCC -lc
Why do you need -lc?
This was my mistake. I didn't notice that 'libpayload.a' already has 'libc.a'. I will revert this change in next update. Thank you for your comment!
https://review.coreboot.org/#/c/33287/6/payloads/libpayload/drivers/Makefile... File payloads/libpayload/drivers/Makefile.inc:
https://review.coreboot.org/#/c/33287/6/payloads/libpayload/drivers/Makefile... PS6, Line 53:
Why are you removing the ifneq?
I removed it because 'libc/time.c', which is included as a default, uses timer_hz() and timer_raw_value() but the implementations of these are not included as a default. So I thought a timer is necessary to build.
However, I understood this change doesn't make sense. So I will revert it in next update.
Is it ok just to set up CONFIG_LP_TIMER_GENERIC_HZ to 1M or some value above it in order to avoid build errors?
https://review.coreboot.org/#/c/33287/6/payloads/libpayload/include/arm/arch... File payloads/libpayload/include/arm/arch/io.h:
https://review.coreboot.org/#/c/33287/6/payloads/libpayload/include/arm/arch... PS6, Line 37: a
ARM doesn't support in/out. […]
Do you mean ARM only supports memory mapped I/O?
https://review.coreboot.org/#/c/33287/6/payloads/libpayload/sample/Makefile File payloads/libpayload/sample/Makefile:
https://review.coreboot.org/#/c/33287/6/payloads/libpayload/sample/Makefile@... PS6, Line 35:
POWERPC support has been removed a long time ago so I believe this is correct.
Thank you for your comment, Julius.
Currently, libpayload only has ARM, X86, ARM64, and MIPS as a target architecture. Also, the variable name of x86 changed from i386 to x86_32. It is defined at .xcompile.