Re: [PATCH-for-5.1 v3 03/24] hw/arm/allwinner-a10: Move some code from realize() to init()

[Philippe Mathieu-Daudé]

On 4/13/20 12:35 AM, Philippe Mathieu-Daudé wrote:
> Coccinelle reported:
> 
>   $ spatch ... --timeout 60 --sp-file \
>     scripts/coccinelle/simplify-init-realize-error_propagate.cocci
>   HANDLING: ./hw/arm/allwinner-a10.c
>   >>> possible moves from aw_a10_init() to aw_a10_realize() in 
> ./hw/arm/allwinner-a10.c:77
> 
> Move the calls using &error_fatal which don't depend on input
> updated before realize() to init().
> 
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
> v3: Typo 'depend of' -> 'depend on' (eblake)
> ---
>  hw/arm/allwinner-a10.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
> index 62a67a3e1a..f41395734a 100644
> --- a/hw/arm/allwinner-a10.c
> +++ b/hw/arm/allwinner-a10.c
> @@ -60,10 +60,18 @@ static void aw_a10_init(Object *obj)
>          int i;
>  
>          for (i = 0; i < AW_A10_NUM_USB; i++) {
> +            char bus[16];
> +
>              sysbus_init_child_obj(obj, "ehci[*]", OBJECT(&s->ehci[i]),
>                                    sizeof(s->ehci[i]), TYPE_PLATFORM_EHCI);
>              sysbus_init_child_obj(obj, "ohci[*]", OBJECT(&s->ohci[i]),
>                                    sizeof(s->ohci[i]), TYPE_SYSBUS_OHCI);
> +            object_property_set_bool(OBJECT(&s->ehci[i]), true,
> +                                     "companion-enable", &error_fatal);
> +
> +            sprintf(bus, "usb-bus.%d", i);
> +            object_property_set_str(OBJECT(&s->ohci[i]), bus, "masterbus",
> +                                    &error_fatal);
>          }
>      }
>  
> @@ -72,6 +80,11 @@ static void aw_a10_init(Object *obj)
>  
>      sysbus_init_child_obj(obj, "rtc", &s->rtc, sizeof(s->rtc),
>                            TYPE_AW_RTC_SUN4I);
> +
> +    memory_region_init_ram(&s->sram_a, obj, "sram A", 48 * KiB,
> +                           &error_fatal);
> +    memory_region_add_subregion(get_system_memory(), 0x00000000, &s->sram_a);

Unfortunately this makes qtest/device-introspect-test fail:

(qemu) device_add allwinner-a10,help
(qemu) device_add allwinner-a10,help
RAMBlock "sram A" already registered, abort!
Aborted (core dumped)

(gdb) bt
#1  0x00007ffff5089895 in abort () at /lib64/libc.so.6
#2  0x0000555557324445 in qemu_ram_set_idstr (new_block=0x614000026a40,
name=0x602000231a50 "sram A", dev=0x633000038800) at exec.c:2023
#3  0x00005555595ed050 in vmstate_register_ram (mr=0x633000049610,
dev=0x633000038800) at migration/savevm.c:2921
#4  0x0000555557569a85 in memory_region_init_ram (mr=0x633000049610,
owner=0x633000038800, name=0x55555a4da6a0 <str> "sram A", size=49152,
errp=0x55555c908d00 <error_fatal>)
    at memory.c:3170
#5  0x0000555557b7cadf in aw_a10_init (obj=0x633000038800) at
hw/arm/allwinner-a10.c:84
#6  0x00005555599b0c09 in object_init_with_type (obj=0x633000038800,
ti=0x61300001da80) at qom/object.c:376
#7  0x0000555559994350 in object_initialize_with_type
(data=0x633000038800, size=97024, type=0x61300001da80) at qom/object.c:516
#8  0x0000555559996756 in object_new_with_type (type=0x61300001da80) at
qom/object.c:684
#9  0x0000555559996832 in object_new (typename=0x60200022e8b0
"allwinner-a10") at qom/object.c:699
#10 0x000055555979e809 in qmp_device_list_properties
(typename=0x60200022e8b0 "allwinner-a10", errp=0x7fffffffafc0) at
qom/qom-qmp-cmds.c:153
#11 0x00005555582b68cd in qdev_device_help (opts=0x607000120130) at
qdev-monitor.c:282


> +    create_unimplemented_device("a10-sram-ctrl", 0x01c00000, 4 * KiB);
>  }
>  
>  static void aw_a10_realize(DeviceState *dev, Error **errp)
> @@ -113,11 +126,6 @@ static void aw_a10_realize(DeviceState *dev, Error 
> **errp)
>      sysbus_connect_irq(sysbusdev, 4, qdev_get_gpio_in(dev, 67));
>      sysbus_connect_irq(sysbusdev, 5, qdev_get_gpio_in(dev, 68));
>  
> -    memory_region_init_ram(&s->sram_a, OBJECT(dev), "sram A", 48 * KiB,
> -                           &error_fatal);
> -    memory_region_add_subregion(get_system_memory(), 0x00000000, &s->sram_a);
> -    create_unimplemented_device("a10-sram-ctrl", 0x01c00000, 4 * KiB);
> -
>      /* FIXME use qdev NIC properties instead of nd_table[] */
>      if (nd_table[0].used) {
>          qemu_check_nic_model(&nd_table[0], TYPE_AW_EMAC);
> @@ -149,12 +157,6 @@ static void aw_a10_realize(DeviceState *dev, Error 
> **errp)
>          int i;
>  
>          for (i = 0; i < AW_A10_NUM_USB; i++) {
> -            char bus[16];
> -
> -            sprintf(bus, "usb-bus.%d", i);
> -
> -            object_property_set_bool(OBJECT(&s->ehci[i]), true,
> -                                     "companion-enable", &error_fatal);
>              object_property_set_bool(OBJECT(&s->ehci[i]), true, "realized",
>                                       &error_fatal);
>              sysbus_mmio_map(SYS_BUS_DEVICE(&s->ehci[i]), 0,
> @@ -162,8 +164,6 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
>              sysbus_connect_irq(SYS_BUS_DEVICE(&s->ehci[i]), 0,
>                                 qdev_get_gpio_in(dev, 39 + i));
>  
> -            object_property_set_str(OBJECT(&s->ohci[i]), bus, "masterbus",
> -                                    &error_fatal);
>              object_property_set_bool(OBJECT(&s->ohci[i]), true, "realized",
>                                       &error_fatal);
>              sysbus_mmio_map(SYS_BUS_DEVICE(&s->ohci[i]), 0,
>