Re: [Qemu-devel] [PATCH] hw/misc/macio: Add the nvram as child of the MacIO south bridge

[Mark Cave-Ayland]

On 01/07/2019 13:34, Philippe Mathieu-Daudé wrote:

> Avoid another QOM orphean, link it back to its parent.
> 
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
>  hw/misc/macio/macio.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index 82492096f8..e02882be28 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -243,7 +243,8 @@ static void macio_oldworld_init(Object *obj)
>  
>      macio_init_child_obj(s, "cuda", &s->cuda, sizeof(s->cuda), TYPE_CUDA);
>  
> -    object_initialize(&os->nvram, sizeof(os->nvram), TYPE_MACIO_NVRAM);
> +    object_initialize_child(s, "nvram",  &os->nvram, sizeof(os->nvram),
> +                            TYPE_MACIO_NVRAM, &error_abort, NULL);
>      dev = DEVICE(&os->nvram);
>      qdev_prop_set_uint32(dev, "size", 0x2000);
>      qdev_prop_set_uint32(dev, "it_shift", 4);

Hmmm actually I think this should be using macio_init_child_obj() which does 
the same
as you have done above, plus it sets the qbus correctly too. And also a minor 
nit on
the subject line: you should mention that this just for Old World Macs, since 
for New
World Macs the NVRAM is located elsewhere.

In fact it looks like there are also a few cases in macio_newworld_realize() 
where
this function should be used too, if you fancy touching those up whilst you're 
in the
area.


ATB,

Mark.