| Message ID | 20241023085852.1061031-28-mark.cave-ayland@ilande.co.uk (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | next-cube: more tidy-ups and improvements | expand |
Hi Mark, On 23/10/24 05:58, Mark Cave-Ayland wrote: > This is to allow the RTC functionality to be maintained within its own separate > device. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > hw/m68k/next-cube.c | 66 ++++++++++++++++++++++++++++++++------------- > 1 file changed, 48 insertions(+), 18 deletions(-) > > diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c > index e4d0083eb0..6b574d39cf 100644 > --- a/hw/m68k/next-cube.c > +++ b/hw/m68k/next-cube.c > @@ -42,7 +42,13 @@ > #define RAM_SIZE 0x4000000 > #define ROM_FILE "Rev_2.5_v66.bin" > > -typedef struct NeXTRTC { > + > +#define TYPE_NEXT_RTC "next-rtc" > +OBJECT_DECLARE_SIMPLE_TYPE(NeXTRTC, NEXT_RTC) > + > +struct NeXTRTC { > + SysBusDevice parent_obj; Since it was not explicitly reset, maybe QDev parent is enough? > int8_t phase; > uint8_t ram[32]; > uint8_t command; > @@ -50,7 +56,7 @@ typedef struct NeXTRTC { > uint8_t status; > uint8_t control; > uint8_t retval; > -} NeXTRTC; > +};
On 24/10/2024 03:44, Philippe Mathieu-Daudé wrote: > Hi Mark, > > On 23/10/24 05:58, Mark Cave-Ayland wrote: >> This is to allow the RTC functionality to be maintained within its own separate >> device. >> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> --- >> hw/m68k/next-cube.c | 66 ++++++++++++++++++++++++++++++++------------- >> 1 file changed, 48 insertions(+), 18 deletions(-) >> >> diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c >> index e4d0083eb0..6b574d39cf 100644 >> --- a/hw/m68k/next-cube.c >> +++ b/hw/m68k/next-cube.c >> @@ -42,7 +42,13 @@ >> #define RAM_SIZE 0x4000000 >> #define ROM_FILE "Rev_2.5_v66.bin" >> -typedef struct NeXTRTC { >> + >> +#define TYPE_NEXT_RTC "next-rtc" >> +OBJECT_DECLARE_SIMPLE_TYPE(NeXTRTC, NEXT_RTC) >> + >> +struct NeXTRTC { >> + SysBusDevice parent_obj; > > Since it was not explicitly reset, maybe QDev parent is enough? Hi Phil, This is deliberate, since the next-pc device resets a couple of fields directly in the NeXTRTC struct in next_pc_reset_hold(), and these are moved to a separate next_rtc_reset_hold() in the next patch. >> int8_t phase; >> uint8_t ram[32]; >> uint8_t command; >> @@ -50,7 +56,7 @@ typedef struct NeXTRTC { >> uint8_t status; >> uint8_t control; >> uint8_t retval; >> -} NeXTRTC; >> +}; ATB, Mark.
Am Wed, 23 Oct 2024 09:58:43 +0100 schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>: > This is to allow the RTC functionality to be maintained within its own separate > device. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > hw/m68k/next-cube.c | 66 ++++++++++++++++++++++++++++++++------------- > 1 file changed, 48 insertions(+), 18 deletions(-) > > diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c > index e4d0083eb0..6b574d39cf 100644 > --- a/hw/m68k/next-cube.c > +++ b/hw/m68k/next-cube.c > @@ -42,7 +42,13 @@ > #define RAM_SIZE 0x4000000 > #define ROM_FILE "Rev_2.5_v66.bin" > > -typedef struct NeXTRTC { > + > +#define TYPE_NEXT_RTC "next-rtc" > +OBJECT_DECLARE_SIMPLE_TYPE(NeXTRTC, NEXT_RTC) > + > +struct NeXTRTC { > + SysBusDevice parent_obj; > + > int8_t phase; > uint8_t ram[32]; > uint8_t command; > @@ -50,7 +56,7 @@ typedef struct NeXTRTC { > uint8_t status; > uint8_t control; > uint8_t retval; > -} NeXTRTC; > +}; > > #define TYPE_NEXT_SCSI "next-scsi" > OBJECT_DECLARE_SIMPLE_TYPE(NeXTSCSI, NEXT_SCSI) > @@ -1012,6 +1018,37 @@ static const MemoryRegionOps next_dummy_en_ops = { > .endianness = DEVICE_BIG_ENDIAN, > }; > > +static const VMStateDescription next_rtc_vmstate = { > + .name = "next-rtc", > + .version_id = 3, > + .minimum_version_id = 3, > + .fields = (const VMStateField[]) { > + VMSTATE_INT8(phase, NeXTRTC), > + VMSTATE_UINT8_ARRAY(ram, NeXTRTC, 32), > + VMSTATE_UINT8(command, NeXTRTC), > + VMSTATE_UINT8(value, NeXTRTC), > + VMSTATE_UINT8(status, NeXTRTC), > + VMSTATE_UINT8(control, NeXTRTC), > + VMSTATE_UINT8(retval, NeXTRTC), > + VMSTATE_END_OF_LIST() > + }, > +}; > + > +static void next_rtc_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + > + dc->desc = "NeXT RTC"; > + dc->vmsd = &next_rtc_vmstate; > +} > + > +static const TypeInfo next_rtc_info = { > + .name = TYPE_NEXT_RTC, > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_size = sizeof(NeXTRTC), > + .class_init = next_rtc_class_init, > +}; > + > static void next_pc_rtc_data_in_irq(void *opaque, int n, int level) > { > NeXTPC *s = NEXT_PC(opaque); > @@ -1078,6 +1115,12 @@ static void next_pc_realize(DeviceState *dev, Error **errp) > } > sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(dev, NEXT_SCC_I)); > sysbus_connect_irq(sbd, 1, qdev_get_gpio_in(dev, NEXT_SCC_DMA_I)); > + > + /* RTC */ > + d = DEVICE(object_resolve_path_component(OBJECT(dev), "rtc")); > + if (!sysbus_realize(SYS_BUS_DEVICE(d), errp)) { > + return; > + } > } > > static void next_pc_init(Object *obj) > @@ -1111,6 +1154,8 @@ static void next_pc_init(Object *obj) > "next.timer", 4); > sysbus_init_mmio(sbd, &s->timer_mem); > > + object_initialize_child(obj, "rtc", &s->rtc, TYPE_NEXT_RTC); > + > s->rtc_power_irq = qdev_get_gpio_in(DEVICE(obj), NEXT_PWR_I); > qdev_init_gpio_in_named(DEVICE(obj), next_pc_rtc_data_in_irq, > "pc-rtc-data-in", 1); > @@ -1129,22 +1174,6 @@ static Property next_pc_properties[] = { > DEFINE_PROP_END_OF_LIST(), > }; > > -static const VMStateDescription next_rtc_vmstate = { > - .name = "next-rtc", > - .version_id = 2, > - .minimum_version_id = 2, > - .fields = (const VMStateField[]) { > - VMSTATE_INT8(phase, NeXTRTC), > - VMSTATE_UINT8_ARRAY(ram, NeXTRTC, 32), > - VMSTATE_UINT8(command, NeXTRTC), > - VMSTATE_UINT8(value, NeXTRTC), > - VMSTATE_UINT8(status, NeXTRTC), > - VMSTATE_UINT8(control, NeXTRTC), > - VMSTATE_UINT8(retval, NeXTRTC), > - VMSTATE_END_OF_LIST() > - }, > -}; > - > static const VMStateDescription next_pc_vmstate = { > .name = "next-pc", > .version_id = 3, > @@ -1297,6 +1326,7 @@ static void next_register_type(void) > type_register_static(&next_typeinfo); > type_register_static(&next_pc_info); > type_register_static(&next_scsi_info); > + type_register_static(&next_rtc_info); > } > > type_init(next_register_type) Shouldn't the next_rtc_vmstate get removed from next_pc_vmstate now? Also, should we finally move the RTC code to a separate file? Thomas
On 09/11/2024 08:14, Thomas Huth wrote: > Am Wed, 23 Oct 2024 09:58:43 +0100 > schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>: > >> This is to allow the RTC functionality to be maintained within its own separate >> device. >> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> --- >> hw/m68k/next-cube.c | 66 ++++++++++++++++++++++++++++++++------------- >> 1 file changed, 48 insertions(+), 18 deletions(-) >> >> diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c >> index e4d0083eb0..6b574d39cf 100644 >> --- a/hw/m68k/next-cube.c >> +++ b/hw/m68k/next-cube.c >> @@ -42,7 +42,13 @@ >> #define RAM_SIZE 0x4000000 >> #define ROM_FILE "Rev_2.5_v66.bin" >> >> -typedef struct NeXTRTC { >> + >> +#define TYPE_NEXT_RTC "next-rtc" >> +OBJECT_DECLARE_SIMPLE_TYPE(NeXTRTC, NEXT_RTC) >> + >> +struct NeXTRTC { >> + SysBusDevice parent_obj; >> + >> int8_t phase; >> uint8_t ram[32]; >> uint8_t command; >> @@ -50,7 +56,7 @@ typedef struct NeXTRTC { >> uint8_t status; >> uint8_t control; >> uint8_t retval; >> -} NeXTRTC; >> +}; >> >> #define TYPE_NEXT_SCSI "next-scsi" >> OBJECT_DECLARE_SIMPLE_TYPE(NeXTSCSI, NEXT_SCSI) >> @@ -1012,6 +1018,37 @@ static const MemoryRegionOps next_dummy_en_ops = { >> .endianness = DEVICE_BIG_ENDIAN, >> }; >> >> +static const VMStateDescription next_rtc_vmstate = { >> + .name = "next-rtc", >> + .version_id = 3, >> + .minimum_version_id = 3, >> + .fields = (const VMStateField[]) { >> + VMSTATE_INT8(phase, NeXTRTC), >> + VMSTATE_UINT8_ARRAY(ram, NeXTRTC, 32), >> + VMSTATE_UINT8(command, NeXTRTC), >> + VMSTATE_UINT8(value, NeXTRTC), >> + VMSTATE_UINT8(status, NeXTRTC), >> + VMSTATE_UINT8(control, NeXTRTC), >> + VMSTATE_UINT8(retval, NeXTRTC), >> + VMSTATE_END_OF_LIST() >> + }, >> +}; >> + >> +static void next_rtc_class_init(ObjectClass *klass, void *data) >> +{ >> + DeviceClass *dc = DEVICE_CLASS(klass); >> + >> + dc->desc = "NeXT RTC"; >> + dc->vmsd = &next_rtc_vmstate; >> +} >> + >> +static const TypeInfo next_rtc_info = { >> + .name = TYPE_NEXT_RTC, >> + .parent = TYPE_SYS_BUS_DEVICE, >> + .instance_size = sizeof(NeXTRTC), >> + .class_init = next_rtc_class_init, >> +}; >> + >> static void next_pc_rtc_data_in_irq(void *opaque, int n, int level) >> { >> NeXTPC *s = NEXT_PC(opaque); >> @@ -1078,6 +1115,12 @@ static void next_pc_realize(DeviceState *dev, Error **errp) >> } >> sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(dev, NEXT_SCC_I)); >> sysbus_connect_irq(sbd, 1, qdev_get_gpio_in(dev, NEXT_SCC_DMA_I)); >> + >> + /* RTC */ >> + d = DEVICE(object_resolve_path_component(OBJECT(dev), "rtc")); >> + if (!sysbus_realize(SYS_BUS_DEVICE(d), errp)) { >> + return; >> + } >> } >> >> static void next_pc_init(Object *obj) >> @@ -1111,6 +1154,8 @@ static void next_pc_init(Object *obj) >> "next.timer", 4); >> sysbus_init_mmio(sbd, &s->timer_mem); >> >> + object_initialize_child(obj, "rtc", &s->rtc, TYPE_NEXT_RTC); >> + >> s->rtc_power_irq = qdev_get_gpio_in(DEVICE(obj), NEXT_PWR_I); >> qdev_init_gpio_in_named(DEVICE(obj), next_pc_rtc_data_in_irq, >> "pc-rtc-data-in", 1); >> @@ -1129,22 +1174,6 @@ static Property next_pc_properties[] = { >> DEFINE_PROP_END_OF_LIST(), >> }; >> >> -static const VMStateDescription next_rtc_vmstate = { >> - .name = "next-rtc", >> - .version_id = 2, >> - .minimum_version_id = 2, >> - .fields = (const VMStateField[]) { >> - VMSTATE_INT8(phase, NeXTRTC), >> - VMSTATE_UINT8_ARRAY(ram, NeXTRTC, 32), >> - VMSTATE_UINT8(command, NeXTRTC), >> - VMSTATE_UINT8(value, NeXTRTC), >> - VMSTATE_UINT8(status, NeXTRTC), >> - VMSTATE_UINT8(control, NeXTRTC), >> - VMSTATE_UINT8(retval, NeXTRTC), >> - VMSTATE_END_OF_LIST() >> - }, >> -}; >> - >> static const VMStateDescription next_pc_vmstate = { >> .name = "next-pc", >> .version_id = 3, >> @@ -1297,6 +1326,7 @@ static void next_register_type(void) >> type_register_static(&next_typeinfo); >> type_register_static(&next_pc_info); >> type_register_static(&next_scsi_info); >> + type_register_static(&next_rtc_info); >> } >> >> type_init(next_register_type) > > Shouldn't the next_rtc_vmstate get removed from next_pc_vmstate now? Indeed, yes it should. I'll fix that up for v2. > Also, should we finally move the RTC code to a separate file? I was thinking about whether it was worth splitting up next-cube.c whilst writing this series, but I didn't come up with a solution I was completely happy with. It is certainly something to reconsider in future though. ATB, Mark.
diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c index e4d0083eb0..6b574d39cf 100644 --- a/hw/m68k/next-cube.c +++ b/hw/m68k/next-cube.c @@ -42,7 +42,13 @@ #define RAM_SIZE 0x4000000 #define ROM_FILE "Rev_2.5_v66.bin" -typedef struct NeXTRTC { + +#define TYPE_NEXT_RTC "next-rtc" +OBJECT_DECLARE_SIMPLE_TYPE(NeXTRTC, NEXT_RTC) + +struct NeXTRTC { + SysBusDevice parent_obj; + int8_t phase; uint8_t ram[32]; uint8_t command; @@ -50,7 +56,7 @@ typedef struct NeXTRTC { uint8_t status; uint8_t control; uint8_t retval; -} NeXTRTC; +}; #define TYPE_NEXT_SCSI "next-scsi" OBJECT_DECLARE_SIMPLE_TYPE(NeXTSCSI, NEXT_SCSI) @@ -1012,6 +1018,37 @@ static const MemoryRegionOps next_dummy_en_ops = { .endianness = DEVICE_BIG_ENDIAN, }; +static const VMStateDescription next_rtc_vmstate = { + .name = "next-rtc", + .version_id = 3, + .minimum_version_id = 3, + .fields = (const VMStateField[]) { + VMSTATE_INT8(phase, NeXTRTC), + VMSTATE_UINT8_ARRAY(ram, NeXTRTC, 32), + VMSTATE_UINT8(command, NeXTRTC), + VMSTATE_UINT8(value, NeXTRTC), + VMSTATE_UINT8(status, NeXTRTC), + VMSTATE_UINT8(control, NeXTRTC), + VMSTATE_UINT8(retval, NeXTRTC), + VMSTATE_END_OF_LIST() + }, +}; + +static void next_rtc_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + + dc->desc = "NeXT RTC"; + dc->vmsd = &next_rtc_vmstate; +} + +static const TypeInfo next_rtc_info = { + .name = TYPE_NEXT_RTC, + .parent = TYPE_SYS_BUS_DEVICE, + .instance_size = sizeof(NeXTRTC), + .class_init = next_rtc_class_init, +}; + static void next_pc_rtc_data_in_irq(void *opaque, int n, int level) { NeXTPC *s = NEXT_PC(opaque); @@ -1078,6 +1115,12 @@ static void next_pc_realize(DeviceState *dev, Error **errp) } sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(dev, NEXT_SCC_I)); sysbus_connect_irq(sbd, 1, qdev_get_gpio_in(dev, NEXT_SCC_DMA_I)); + + /* RTC */ + d = DEVICE(object_resolve_path_component(OBJECT(dev), "rtc")); + if (!sysbus_realize(SYS_BUS_DEVICE(d), errp)) { + return; + } } static void next_pc_init(Object *obj) @@ -1111,6 +1154,8 @@ static void next_pc_init(Object *obj) "next.timer", 4); sysbus_init_mmio(sbd, &s->timer_mem); + object_initialize_child(obj, "rtc", &s->rtc, TYPE_NEXT_RTC); + s->rtc_power_irq = qdev_get_gpio_in(DEVICE(obj), NEXT_PWR_I); qdev_init_gpio_in_named(DEVICE(obj), next_pc_rtc_data_in_irq, "pc-rtc-data-in", 1); @@ -1129,22 +1174,6 @@ static Property next_pc_properties[] = { DEFINE_PROP_END_OF_LIST(), }; -static const VMStateDescription next_rtc_vmstate = { - .name = "next-rtc", - .version_id = 2, - .minimum_version_id = 2, - .fields = (const VMStateField[]) { - VMSTATE_INT8(phase, NeXTRTC), - VMSTATE_UINT8_ARRAY(ram, NeXTRTC, 32), - VMSTATE_UINT8(command, NeXTRTC), - VMSTATE_UINT8(value, NeXTRTC), - VMSTATE_UINT8(status, NeXTRTC), - VMSTATE_UINT8(control, NeXTRTC), - VMSTATE_UINT8(retval, NeXTRTC), - VMSTATE_END_OF_LIST() - }, -}; - static const VMStateDescription next_pc_vmstate = { .name = "next-pc", .version_id = 3, @@ -1297,6 +1326,7 @@ static void next_register_type(void) type_register_static(&next_typeinfo); type_register_static(&next_pc_info); type_register_static(&next_scsi_info); + type_register_static(&next_rtc_info); } type_init(next_register_type)
This is to allow the RTC functionality to be maintained within its own separate device. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> --- hw/m68k/next-cube.c | 66 ++++++++++++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 18 deletions(-)