On 2/2/19 3:48 PM, Andrew Jones wrote: > On Fri, Feb 01, 2019 at 11:16:37AM +0000, Alexandru Elisei wrote: >> Generate lib/config.h when configuring kvm-unit-tests. The file is empty >> for all architectures except for arm and arm64, where it is used to store >> the UART base address. This removes the hardcoded address from lib/arm/io.c >> and provides a mechanism for using different UART addresses in the future. >> >> Signed-off-by: Alexandru Elisei <alexandru.elisei@xxxxxxx> >> --- >> configure | 17 +++++++++++++++++ >> Makefile | 2 +- >> lib/arm/io.c | 5 ++--- >> .gitignore | 1 + >> 4 files changed, 21 insertions(+), 4 deletions(-) >> >> diff --git a/configure b/configure >> index df8581e3a906..44708b026422 100755 >> --- a/configure >> +++ b/configure >> @@ -198,3 +198,20 @@ ENVIRON_DEFAULT=$environ_default >> ERRATATXT=errata.txt >> U32_LONG_FMT=$u32_long >> EOF >> + >> +cat <<EOF > lib/config.h >> +#ifndef CONFIG_H >> +#define CONFIG_H 1 >> +/* >> + * Generated file. DO NOT MODIFY. >> + * >> + */ >> +EOF >> +if [ "$arch" = "arm" ] || [ "$arch" = "arm64" ]; then >> +cat <<EOF >> lib/config.h >> + >> +#define UART_EARLY_BASE (unsigned long)0x09000000 > The (unsigned long) cast should be in the assignment in lib/arm/io.c, > not here, i.e. > > static volatile u8 *uart0_base = (u8 *)(unsigned long)UART_EARLY_BASE; I know you have mentioned this before, I forgot to explain why I'm casting it here and not in io.c. The address UART_EARLY_BASE is used in 3 places in lib/arm/io.c: in the example above, and when a warning is displayed if the uart_base from the dtb doesn't match the expected uart address. I tried casting the address each time and the code looked a bit awkward to me. A had a earlier suggestion to make arm_uart_early_addr=0x09000000UL, but you disagreed with that because an user might want to supply it themselves with an 'UL' at the end, and we would end up with 0x09000000ULUL. But we assign that value to arm_uart_early_addr ourselves, I don't think that's an issue. In the end, it's your choice, I'm fine with casting it to unsigned long before each use in lib/arm/io.c. > > >> + >> +EOF >> +fi >> +echo "#endif" >> lib/config.h >> diff --git a/Makefile b/Makefile >> index e9f02272e156..643af05678ad 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -115,7 +115,7 @@ libfdt_clean: >> $(LIBFDT_objdir)/.*.d >> >> distclean: clean libfdt_clean >> - $(RM) lib/asm config.mak $(TEST_DIR)-run msr.out cscope.* build-head >> + $(RM) lib/asm lib/config.h config.mak $(TEST_DIR)-run msr.out cscope.* build-head >> $(RM) -r tests logs logs.old >> >> cscope: cscope_dirs = lib lib/libfdt lib/linux $(TEST_DIR) $(ARCH_LIBDIRS) lib/asm-generic >> diff --git a/lib/arm/io.c b/lib/arm/io.c >> index d2c1a07c19ee..0973885d19f5 100644 >> --- a/lib/arm/io.c >> +++ b/lib/arm/io.c >> @@ -11,6 +11,7 @@ >> #include <libcflat.h> >> #include <devicetree.h> >> #include <chr-testdev.h> >> +#include <config.h> >> #include <asm/spinlock.h> >> #include <asm/io.h> >> >> @@ -18,6 +19,7 @@ >> >> extern void halt(int code); >> >> +static struct spinlock uart_lock; >> /* >> * Use this guess for the pl011 base in order to make an attempt at >> * having earlier printf support. We'll overwrite it with the real >> @@ -25,9 +27,6 @@ extern void halt(int code); >> * the address we expect QEMU's mach-virt machine type to put in >> * its generated device tree. >> */ > The QEMU references in the above comment should be removed or modified to > also reference kvmtool. > >> -#define UART_EARLY_BASE 0x09000000UL >> - >> -static struct spinlock uart_lock; >> static volatile u8 *uart0_base = (u8 *)UART_EARLY_BASE; > As mentioned above, we should put the (unsigned long) cast here. > >> >> static void uart0_init(void) >> diff --git a/.gitignore b/.gitignore >> index 2405a8087ae5..483f7c7a09ea 100644 >> --- a/.gitignore >> +++ b/.gitignore >> @@ -10,6 +10,7 @@ patches >> cscope.* >> *.swp >> /lib/asm >> +/lib/config.h >> /config.mak >> /*-run >> /msr.out >> -- >> 2.17.0 >> > Thanks, > drew
