[PATCH 01/14] qemu: arm: Use the generated DTB only when CONGIG_OF_BOARD is defined

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

On 11/26/20 7:40 PM, Sughosh Ganu wrote:
> The Qemu platform emulator generates a device tree blob and places it
> at the start of the dram, which is then used by u-boot. Use this dtb
> only if CONFIG_OF_BOARD is defined. This allows using a different
> device tree, using the CONFIG_OF_SEPARATE option. This dtb is attached
> to the u-boot binary as a u-boot-fdt.bin file

Dear Sughosh,

thank your for this series which will allow us to better demonstrate and
test capsule updates.

I am not sure if the approach that you take at device-trees here is the
right one.

On QEMU the device-tree is generated on the fly by the QEMU binary
depending on which devices the user has specified.

Your idea is to replace this device-tree completely to be able to add
extra elements (the EFI signature list, see patch 2/14). Thus a
device-tree might be loaded that does not match the user selected devices.

An alternative approach would be to apply all additions to the
device-tree as an FDT overlay (or fixup). This would allow the dynamic
parts of the QEMU device-tree still to be passed through.

Could you, please, assess the pros and cons of the two approaches with
respect to:

* usability for capsule updates
* applicability for non-QEMU systems
* integration of DTB overlays with FIT images for other use cases

Best regards

Heinrich


>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
>   board/emulation/qemu-arm/qemu-arm.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
> index f18f2ed7da..e146d1cc50 100644
> --- a/board/emulation/qemu-arm/qemu-arm.c
> +++ b/board/emulation/qemu-arm/qemu-arm.c
> @@ -89,11 +89,13 @@ int dram_init_banksize(void)
>   	return 0;
>   }
>
> +#if defined(CONFIG_OF_BOARD)
>   void *board_fdt_blob_setup(void)
>   {
>   	/* QEMU loads a generated DTB for us at the start of RAM. */
>   	return (void *)CONFIG_SYS_SDRAM_BASE;
>   }
> +#endif
>
>   void enable_caches(void)
>   {
>