Re: [PATCH 27/36] next-cube: QOMify NeXTRTC

[Thomas Huth <huth@tuxfamily.org>]

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