Re: [PATCH 1/1] ARM: vexpress_ca9x4: Reintroduce board in order to use with QEMU.

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 5035 bytes --]

On Tue, Sep 07, 2021 at 08:37:51AM +0200, Kristian Amlie wrote:

> vexpress_ca9x4 is seemingly the only board except for qemu_arm which
> is able to run U-Boot correctly, using the `-M vexpress-a9` option to
> QEMU. Building for qemu_arm and running qemu-system-arm with the `-M
> virt` argument has a number of downsides, most importantly that it
> only supports virtio storage drivers. This significantly reduces its
> usefulness in testing memory card and Flash solutions, especially when
> the tested images are from a third party source.
> 
> So therefore we reintroduce the vexpress_ca9x4 board in this commit,
> with the explicit goal of using it with QEMU.
> 
> A number of differences to note from the original:
> 
> * Since the board was apparently unmaintained, I have now set myself
>   as the maintainer.
> 
> * The board has been converted to use the driver model, which was the
>   reason it was removed in the first place.
> 
> * The vexpress_ca15_tc2 and vexpress_ca5x2 boards, which were removed
>   in the same commit, are not necessary for the QEMU use case, and
>   have been omitted.
> 
> * An `mmc0` alias was introduced in the dts file. The mmc is not
>   detected correctly without this, now that it's based on the device
>   tree instead of the board's init function.
> 
> * A couple of other nodes were removed because they were problematic
>   when trying to run the UEFI bootmgr. Once again, the primary use
>   case here is QEMU, and these nodes are not needed for that to work.
> 
> * Unnecessary board init code has been removed, thanks to driver model
>   and device tree.
> 
> * `CONFIG_OF_EMBED` has been enabled. I know this goes against
>   recommended practice, but there doesn't seem to be any other way to
>   pass the dtb to U-Boot in the QEMU scenario. Using the -dtb argument
>   does not work, I suppose because U-Boot doesn't use the same
>   mechanics as the kernel when it's booting.

This is something that should get looked at and figured out, but is
separate.  I thought this did work on Pi for example.

> * Load addresses have been changed to fit QEMU use case.
> 
> People wanting to get a more detailed, yet somewhat isolated, diff
> between this and the original, can run this command:
> 
>   git diff c6c26a05b89f25a06e7562f8c2071b60fd0c9eac~1 -- \
>       $( git diff-tree --diff-filter=A -r --name-only HEAD~1 HEAD)
> 
> (Make sure to either check out this commit first, or replace HEAD with
> the commit ID of this commit)
> 
> Signed-off-by: Kristian Amlie <kristian.amlie@northern.tech>

I'm glad to see this come back.  A request:

[snip]
> diff --git a/include/configs/vexpress_ca9x4.h b/include/configs/vexpress_ca9x4.h
> new file mode 100644
> index 0000000000..8157a5868d
> --- /dev/null
> +++ b/include/configs/vexpress_ca9x4.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * (C) Copyright 2011 Linaro
> + * Ryan Harkin, <ryan.harkin@linaro.org>
> + *
> + * Configuration for Versatile Express. Parts were derived from other ARM
> + *   configurations.
> + */
> +
> +#ifndef __VEXPRESS_CA9X4_H
> +#define __VEXPRESS_CA9X4_H
> +
> +#define CONFIG_VEXPRESS_ORIGINAL_MEMORY_MAP
> +#include "vexpress_common.h"

CONFIG_VEXPRESS_ORIGINAL_MEMORY_MAP looks to just be polluting the
CONFIG namespace, it's only then used..

> +
> +#endif /* VEXPRESS_CA9X4_H */
> diff --git a/include/configs/vexpress_common.h b/include/configs/vexpress_common.h
> index b131480e5b..99a5dd064a 100644
> --- a/include/configs/vexpress_common.h
> +++ b/include/configs/vexpress_common.h
> @@ -169,29 +169,10 @@
>          func(DHCP, dhcp, na)
>  #include <config_distro_bootcmd.h>
>  
> -#ifdef CONFIG_VEXPRESS_ORIGINAL_MEMORY_MAP
> -#define CONFIG_PLATFORM_ENV_SETTINGS \
> -		"loadaddr=0x80008000\0" \
> -		"ramdisk_addr_r=0x61000000\0" \
> -		"kernel_addr=0x44100000\0" \
> -		"ramdisk_addr=0x44800000\0" \
> -		"maxramdisk=0x1800000\0" \
> -		"pxefile_addr_r=0x88000000\0" \
> -		"scriptaddr=0x88000000\0" \
> -		"kernel_addr_r=0x80008000\0"
> -#elif defined(CONFIG_VEXPRESS_EXTENDED_MEMORY_MAP)
> -#define CONFIG_PLATFORM_ENV_SETTINGS \
> -		"loadaddr=0xa0008000\0" \
> -		"ramdisk_addr_r=0x81000000\0" \
> -		"kernel_addr=0x0c100000\0" \
> -		"ramdisk_addr=0x0c800000\0" \
> -		"maxramdisk=0x1800000\0" \
> -		"pxefile_addr_r=0xa8000000\0" \
> -		"scriptaddr=0xa8000000\0" \
> -		"kernel_addr_r=0xa0008000\0"
> -#endif
>  #define CONFIG_EXTRA_ENV_SETTINGS \
> -		CONFIG_PLATFORM_ENV_SETTINGS \
> +                "kernel_addr_r=0x60100000\0" \
> +                "fdt_addr_r=0x60000000\0" \
> +                "bootargs=console=tty0 console=ttyAMA0,38400n8\0" \
>                  BOOTENV \
>  		"console=ttyAMA0,38400n8\0" \
>  		"dram=1024M\0" \

around here, to modify what the default environment is.  Can you please
do a patch (a follow-up to this is fine) to rename it to just
VEXPRESS_ORIGINAL_MEMORY_MAP ?  Thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]