On 3/8/2020 9:29 PM, Peter Maydell wrote:
> On Thu, 5 Mar 2020 at 06:39, Pan Nengyuan <pannengy...@huawei.com> wrote:
>>
>> This patch fix a bug in mac_via where it failed to actually realize devices
>> it was using.
>> And move the init codes which inits the mos6522 objects and properties on
>> them from realize()
>> into init(). However, we keep qdev_set_parent_bus in realize(), otherwise it
>> will cause
>> device-introspect-test test fail. Then do the realize mos6522 device in the
>> mac_vir_realize.
>>
>> Signed-off-by: Pan Nengyuan <pannengy...@huawei.com>
>> ---
>> Cc: Laurent Vivier <laur...@vivier.eu>
>> Cc: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
>> ---
>
>> diff --git a/hw/misc/mac_via.c b/hw/misc/mac_via.c
>> index b7d0012794..4c5ad08805 100644
>> --- a/hw/misc/mac_via.c
>> +++ b/hw/misc/mac_via.c
>> @@ -868,24 +868,24 @@ static void mac_via_reset(DeviceState *dev)
>> static void mac_via_realize(DeviceState *dev, Error **errp)
>> {
>> MacVIAState *m = MAC_VIA(dev);
>> - MOS6522State *ms;
>> struct tm tm;
>> int ret;
>> + Error *err = NULL;
>>
>> - /* Init VIAs 1 and 2 */
>> - sysbus_init_child_obj(OBJECT(dev), "via1", &m->mos6522_via1,
>> - sizeof(m->mos6522_via1), TYPE_MOS6522_Q800_VIA1);
>> + qdev_set_parent_bus(DEVICE(&m->mos6522_via1), sysbus_get_default());
>> + qdev_set_parent_bus(DEVICE(&m->mos6522_via2), sysbus_get_default());
>
> Rather than manually setting the parent bus, you can use
> sysbus_init_child_obj() instead of object_initialize_child() --
> it is a convenience function that does both object_initialize_child()
> and qdev_set_parent_bus() for you.
Actually I used sysbus_init_child_obj() first, but it will fail to run
device-introspect-test.
Because qdev_set_parent_bus() will change 'info qtree' after we call
'device-list-properties'.
Thus, I do it in the realize.
>
>> - sysbus_init_child_obj(OBJECT(dev), "via2", &m->mos6522_via2,
>> - sizeof(m->mos6522_via2), TYPE_MOS6522_Q800_VIA2);
>> + object_property_set_bool(OBJECT(&m->mos6522_via1), true, "realized",
>> &err);
>> + if (err != NULL) {
>> + error_propagate(errp, err);
>> + return;
>> + }
>>
>> - /* 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]", &error_abort);
>> - ms = MOS6522(&m->mos6522_via2);
>> - object_property_add_alias(OBJECT(dev), "irq[1]", OBJECT(ms),
>> - SYSBUS_DEVICE_GPIO_IRQ "[0]", &error_abort);
>> + object_property_set_bool(OBJECT(&m->mos6522_via2), true, "realized",
>> &err);
>> + if (err != NULL) {
>> + error_propagate(errp, err);
>> + return;
>> + }
>>
>> /* Pass through mos6522 input IRQs */
>> qdev_pass_gpios(DEVICE(&m->mos6522_via1), dev, "via1-irq");
>> @@ -932,6 +932,7 @@ static void mac_via_init(Object *obj)
>> {
>> SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>> MacVIAState *m = MAC_VIA(obj);
>> + MOS6522State *ms;
>>
>> /* MMIO */
>> memory_region_init(&m->mmio, obj, "mac-via", 2 * VIA_SIZE);
>> @@ -948,6 +949,22 @@ static void mac_via_init(Object *obj)
>> /* ADB */
>> qbus_create_inplace((BusState *)&m->adb_bus, sizeof(m->adb_bus),
>> TYPE_ADB_BUS, DEVICE(obj), "adb.0");
>> +
>> + /* Init VIAs 1 and 2 */
>> + object_initialize_child(OBJECT(m), "via1", &m->mos6522_via1,
>> + sizeof(m->mos6522_via1), TYPE_MOS6522_Q800_VIA1,
>> + &error_abort, NULL);
>> + object_initialize_child(OBJECT(m), "via2", &m->mos6522_via2,
>> + sizeof(m->mos6522_via2), TYPE_MOS6522_Q800_VIA2,
>> + &error_abort, NULL);
>> +
>> + /* Pass through mos6522 output IRQs */
>> + ms = MOS6522(&m->mos6522_via1);
>> + object_property_add_alias(OBJECT(m), "irq[0]", OBJECT(ms),
>> + SYSBUS_DEVICE_GPIO_IRQ "[0]", &error_abort);
>
> There's no point in using the MOS6522() cast to get a MOS6522*,
> because the only thing you do with it is immediately cast it
> to an Object* with the OBJECT() cast. Just use the OBJECT cast directly.
Ok, will do it.
Thanks.
>
>> + ms = MOS6522(&m->mos6522_via2);
>> + object_property_add_alias(OBJECT(m), "irq[1]", OBJECT(ms),
>> + SYSBUS_DEVICE_GPIO_IRQ "[0]", &error_abort);
>> }
>>
>> static void postload_update_cb(void *opaque, int running, RunState state)
>
> thanks
> -- PMM
> .
>
