* [PATCH] mac_via: fix init() and realize() behaviour
@ 2020-10-13 16:26 Mark Cave-Ayland
2020-10-13 18:21 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 2+ messages in thread
From: Mark Cave-Ayland @ 2020-10-13 16:26 UTC (permalink / raw)
To: ehabkost, laurent, qemu-devel
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(-)
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;
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);
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);
--
2.20.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] mac_via: fix init() and realize() behaviour
2020-10-13 16:26 [PATCH] mac_via: fix init() and realize() behaviour Mark Cave-Ayland
@ 2020-10-13 18:21 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 2+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-13 18:21 UTC (permalink / raw)
To: Mark Cave-Ayland, ehabkost, laurent, qemu-devel
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);
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-10-13 18:22 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-13 16:26 [PATCH] mac_via: fix init() and realize() behaviour Mark Cave-Ayland
2020-10-13 18:21 ` Philippe Mathieu-Daudé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.