mac_via: fix init() and realize() behaviour

The mac_via device does not currently follow the rules for init() and realize() in
regard to the mos6522 child devices. These child devices must be initialised using
object_initialize_child() within the mac_via init() function and then realized as
part of the mac_via realize() function. Move object_initialize_child() from
realize() to init() which is where the iniitalisation of child devices should occur.

Similarly the realize() function creates alias properties to allow the VIA input
and output IRQs to be wired up to the interrupt controller during machine init, but
realize() should never alter object properties. Remove these aliases and instead
use object_resolve_path_component() to access the child objects from the mac_via
device.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/m68k/q800.c    | 12 ++++++++----
 hw/misc/mac_via.c | 36 ++++++++++++------------------------
 2 files changed, 20 insertions(+), 28 deletions(-)

On 10/13/20 6:26 PM, Mark Cave-Ayland wrote:
> The mac_via device does not currently follow the rules for init() and realize() in
> regard to the mos6522 child devices. These child devices must be initialised using
> object_initialize_child() within the mac_via init() function and then realized as
> part of the mac_via realize() function. Move object_initialize_child() from
> realize() to init() which is where the iniitalisation of child devices should occur.

Typo "initialisation".

> 
> Similarly the realize() function creates alias properties to allow the VIA input
> and output IRQs to be wired up to the interrupt controller during machine init, but
> realize() should never alter object properties. Remove these aliases and instead
> use object_resolve_path_component() to access the child objects from the mac_via
> device.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>   hw/m68k/q800.c    | 12 ++++++++----
>   hw/misc/mac_via.c | 36 ++++++++++++------------------------
>   2 files changed, 20 insertions(+), 28 deletions(-)
> 
> diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
> index ce4b47c3e3..773d75c1f8 100644
> --- a/hw/m68k/q800.c
> +++ b/hw/m68k/q800.c
> @@ -174,6 +174,7 @@ static void q800_init(MachineState *machine)
>       SysBusESPState *sysbus_esp;
>       ESPState *esp;
>       SysBusDevice *sysbus;
> +    MOS6522State *ms;

As we often use 'ms' for MachineState* variable, maybe name
this 'mos' or 'via'?
Also you can use simply a Object*.

>       BusState *adb_bus;
>       NubusBus *nubus;
>       GLUEState *irq;
> @@ -226,9 +227,11 @@ static void q800_init(MachineState *machine)
>       sysbus = SYS_BUS_DEVICE(via_dev);
>       sysbus_realize_and_unref(sysbus, &error_fatal);
>       sysbus_mmio_map(sysbus, 0, VIA_BASE);
> -    qdev_connect_gpio_out_named(DEVICE(sysbus), "irq", 0, pic[0]);
> -    qdev_connect_gpio_out_named(DEVICE(sysbus), "irq", 1, pic[1]);
>   
> +    ms = MOS6522(object_resolve_path_component(OBJECT(via_dev), "via1"));
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ms), 0, pic[0]);
> +    ms = MOS6522(object_resolve_path_component(OBJECT(via_dev), "via2"));
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ms), 0, pic[1]);
>   
>       adb_bus = qdev_get_child_bus(via_dev, "adb.0");
>       dev = qdev_new(TYPE_ADB_KEYBOARD);
> @@ -300,11 +303,12 @@ static void q800_init(MachineState *machine)
>   
>       sysbus = SYS_BUS_DEVICE(dev);
>       sysbus_realize_and_unref(sysbus, &error_fatal);
> -    sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in_named(via_dev,
> +    ms = MOS6522(object_resolve_path_component(OBJECT(via_dev), "via2"));
> +    sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in_named(DEVICE(ms),
>                                                            "via2-irq",
>                                                            VIA2_IRQ_SCSI_BIT));
>       sysbus_connect_irq(sysbus, 1,
> -                       qdev_get_gpio_in_named(via_dev, "via2-irq",
> +                       qdev_get_gpio_in_named(DEVICE(ms), "via2-irq",
>                                                 VIA2_IRQ_SCSI_DATA_BIT));
>       sysbus_mmio_map(sysbus, 0, ESP_BASE);
>       sysbus_mmio_map(sysbus, 1, ESP_PDMA);
> diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
> index 6db62dab7d..7c2c943d3f 100644
> --- a/hw/misc/mac_via.c
> +++ b/hw/misc/mac_via.c
> @@ -1016,40 +1016,21 @@ static void mac_via_realize(DeviceState *dev, Error **errp)
>       struct tm tm;
>       int ret;
>   
> -    /* Init VIAs 1 and 2 */
> -    object_initialize_child(OBJECT(dev), "via1", &m->mos6522_via1,
> -                            TYPE_MOS6522_Q800_VIA1);
> -
> -    object_initialize_child(OBJECT(dev), "via2", &m->mos6522_via2,
> -                            TYPE_MOS6522_Q800_VIA2);
> -
> -    /* Pass through mos6522 output IRQs */
> -    ms = MOS6522(&m->mos6522_via1);
> -    object_property_add_alias(OBJECT(dev), "irq[0]", OBJECT(ms),
> -                              SYSBUS_DEVICE_GPIO_IRQ "[0]");
> -    ms = MOS6522(&m->mos6522_via2);
> -    object_property_add_alias(OBJECT(dev), "irq[1]", OBJECT(ms),
> -                              SYSBUS_DEVICE_GPIO_IRQ "[0]");
> -
> +    /* Realize VIAs */
>       sysbus_realize(SYS_BUS_DEVICE(&m->mos6522_via1), &error_abort);
>       sysbus_realize(SYS_BUS_DEVICE(&m->mos6522_via2), &error_abort);
>   
> -    /* Pass through mos6522 input IRQs */
> -    qdev_pass_gpios(DEVICE(&m->mos6522_via1), dev, "via1-irq");
> -    qdev_pass_gpios(DEVICE(&m->mos6522_via2), dev, "via2-irq");
> -
>       /* VIA 1 */
> +    ms = MOS6522(&m->mos6522_via1);
>       m->mos6522_via1.one_second_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL,
> -                                                     via1_one_second,
> -                                                     &m->mos6522_via1);
> -    m->mos6522_via1.VBL_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, via1_VBL,
> -                                              &m->mos6522_via1);
> +                                                     via1_one_second, ms);
> +    m->mos6522_via1.VBL_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, via1_VBL, ms);

Unrelated to your patch, but doesn't this belong
to mos6522_q800_via1_init()?

>   
>       qemu_get_timedate(&tm, 0);
>       m->tick_offset = (uint32_t)mktimegm(&tm) + RTC_OFFSET;
>   
>       adb_register_autopoll_callback(adb_bus, adb_via_poll, m);
> -    m->adb_data_ready = qdev_get_gpio_in_named(dev, "via1-irq",
> +    m->adb_data_ready = qdev_get_gpio_in_named(DEVICE(ms), "via1-irq",
>                                                  VIA1_IRQ_ADB_READY_BIT);
>   
>       if (m->blk) {
> @@ -1080,6 +1061,13 @@ static void mac_via_init(Object *obj)
>       SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>       MacVIAState *m = MAC_VIA(obj);
>   
> +    /* Init VIAs 1 and 2 */
> +    object_initialize_child(obj, "via1", &m->mos6522_via1,
> +                            TYPE_MOS6522_Q800_VIA1);
> +
> +    object_initialize_child(obj, "via2", &m->mos6522_via2,
> +                            TYPE_MOS6522_Q800_VIA2);
> +
>       /* MMIO */
>       memory_region_init(&m->mmio, obj, "mac-via", 2 * VIA_SIZE);
>       sysbus_init_mmio(sbd, &m->mmio);
>