Re: [Qemu-devel] [PATCH v8 2/3] hw/arm/boot: arm_load_kernel implemented as a machine init done notifier

[Peter Maydell <peter.maydell@linaro.org>]

On 5 January 2015 at 16:14, Eric Auger <eric.auger@linaro.org> wrote:
> Device tree nodes for the platform bus and its children dynamic sysbus
> devices are added in a machine init done notifier. To load the dtb once,
> after those latter nodes are built and before ROM freeze, the actual
> arm_load_kernel existing code is moved into a notifier notify function,
> arm_load_kernel_notify. arm_load_kernel now only registers the
> corresponding notifier.
>
> Machine files that do not support platform bus stay unchanged. Machine
> files willing to support dynamic sysbus devices must call arm_load_kernel
> before sysbus-fdt arm_register_platform_bus_fdt_creator to make sure
> dynamic sysbus device nodes are integrated in the dtb.

> --- a/include/hw/arm/arm.h
> +++ b/include/hw/arm/arm.h
> @@ -19,6 +19,16 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory,
>                        int flash_size, int sram_size,
>                        const char *kernel_filename, const char *cpu_model);
>
> +/*
> + * struct used as a parameter of the arm_load_kernel machine init
> + * done notifier
> + */
> +typedef struct {
> +    ARMCPU *cpu; /* handle to the first cpu object */
> +    Notifier notifier; /* actual notifier */

This is an odd order -- why not put 'notifier' first?

> +} ArmLoadKernelNotifier;
> +
> +

This breaks building of the linux-user targets:

In file included from
/home/petmay01/linaro/qemu-from-laptop/qemu/target-arm/cpu.c:29:0:
/home/petmay01/linaro/qemu-from-laptop/qemu/include/hw/arm/arm.h:28:5:
error: unknown type name ‘Notifier’
     Notifier notifier; /* actual notifier */
     ^
make[1]: *** [target-arm/cpu.o] Error 1

Including "qemu/notify.h" in this file fixes it.

Also, stray blank line.

Otherwise looks OK. (It seems a slight shame to have to drag
notify.h into everything that includes arm.h when really the
notifier is just a detail of the implementation of boot.c,
but I can't see a nice way round that, so never mind.)

-- PMM