[Qemu-devel] [PATCH qom-next for-1.6 v2] fdc: Fix inheritence for SUNW, fdtwo

[Qemu-devel] [PATCH qom-next for-1.6 v2] fdc: Fix inheritence for SUNW, fdtwo

[Andreas Färber]

Since commit dd3be7420774f7dc8f37a96ca24d07f0b6f31b3b SUNW,fdtwo's
initfn (realizefn since 940194c2369e50d91d1abf6f36d43853eea5e539)
was using SYSBUS_FDC() cast. This uses type sysbus-fdc rather than
SUNW,fdtwo.

Fix this by letting SUNW,fdtwo and sysbus-fdc both inherit from an
abstract type base-sysbus-fdc.

This allows to consolidate realizefns by using instance_init functions.
Clean up variable names and variable order while at it.

Reported-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: Hu Tao <hutao@cn.fujitsu.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hw/block/fdc.c | 86 +++++++++++++++++++++++++++++-----------------------------
 1 file changed, 43 insertions(+), 43 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index d32f6ba..723d2c7 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -544,7 +544,7 @@ struct FDCtrl {
     uint8_t timer1;
 };
 
-#define TYPE_SYSBUS_FDC "sysbus-fdc"
+#define TYPE_SYSBUS_FDC "base-sysbus-fdc"
 #define SYSBUS_FDC(obj) OBJECT_CHECK(FDCtrlSysBus, (obj), TYPE_SYSBUS_FDC)
 
 typedef struct FDCtrlSysBus {
@@ -2153,60 +2153,49 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp)
 
 static void sysbus_fdc_initfn(Object *obj)
 {
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
     FDCtrlSysBus *sys = SYSBUS_FDC(obj);
     FDCtrl *fdctrl = &sys->state;
 
+    fdctrl->dma_chann = -1;
+
     memory_region_init_io(&fdctrl->iomem, obj, &fdctrl_mem_ops, fdctrl,
                           "fdc", 0x08);
+    sysbus_init_mmio(sbd, &fdctrl->iomem);
 }
 
-static void sysbus_fdc_realize(DeviceState *dev, Error **errp)
+static void sun4m_fdc_initfn(Object *obj)
 {
-    FDCtrlSysBus *sys = SYSBUS_FDC(dev);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    FDCtrlSysBus *sys = SYSBUS_FDC(obj);
     FDCtrl *fdctrl = &sys->state;
-    SysBusDevice *b = SYS_BUS_DEVICE(dev);
-    Error *err = NULL;
 
-    sysbus_init_mmio(b, &fdctrl->iomem);
-    sysbus_init_irq(b, &fdctrl->irq);
-    qdev_init_gpio_in(dev, fdctrl_handle_tc, 1);
-    fdctrl->dma_chann = -1;
+    fdctrl->sun4m = 1;
 
-    qdev_set_legacy_instance_id(dev, 0 /* io */, 2); /* FIXME */
-    fdctrl_realize_common(fdctrl, &err);
-    if (err != NULL) {
-        error_propagate(errp, err);
-        return;
-    }
+    memory_region_init_io(&fdctrl->iomem, obj, &fdctrl_mem_strict_ops,
+                          fdctrl, "fdctrl", 0x08);
+    sysbus_init_mmio(sbd, &fdctrl->iomem);
 }
 
-static void sun4m_fdc_initfn(Object *obj)
+static void sysbus_fdc_common_initfn(Object *obj)
 {
+    DeviceState *dev = DEVICE(obj);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
     FDCtrlSysBus *sys = SYSBUS_FDC(obj);
     FDCtrl *fdctrl = &sys->state;
 
-    memory_region_init_io(&fdctrl->iomem, obj, &fdctrl_mem_strict_ops,
-                          fdctrl, "fdctrl", 0x08);
+    qdev_set_legacy_instance_id(dev, 0 /* io */, 2); /* FIXME */
+
+    sysbus_init_irq(sbd, &fdctrl->irq);
+    qdev_init_gpio_in(dev, fdctrl_handle_tc, 1);
 }
 
-static void sun4m_fdc_realize(DeviceState *dev, Error **errp)
+static void sysbus_fdc_common_realize(DeviceState *dev, Error **errp)
 {
     FDCtrlSysBus *sys = SYSBUS_FDC(dev);
     FDCtrl *fdctrl = &sys->state;
-    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
-    Error *err = NULL;
 
-    sysbus_init_mmio(sbd, &fdctrl->iomem);
-    sysbus_init_irq(sbd, &fdctrl->irq);
-    qdev_init_gpio_in(dev, fdctrl_handle_tc, 1);
-
-    fdctrl->sun4m = 1;
-    qdev_set_legacy_instance_id(dev, 0 /* io */, 2); /* FIXME */
-    fdctrl_realize_common(fdctrl, &err);
-    if (err != NULL) {
-        error_propagate(errp, err);
-        return;
-    }
+    fdctrl_realize_common(fdctrl, errp);
 }
 
 FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i)
@@ -2278,16 +2267,12 @@ static void sysbus_fdc_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    dc->realize = sysbus_fdc_realize;
-    dc->reset = fdctrl_external_reset_sysbus;
-    dc->vmsd = &vmstate_sysbus_fdc;
     dc->props = sysbus_fdc_properties;
 }
 
 static const TypeInfo sysbus_fdc_info = {
-    .name          = TYPE_SYSBUS_FDC,
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(FDCtrlSysBus),
+    .name          = "sysbus-fdc",
+    .parent        = TYPE_SYSBUS_FDC,
     .instance_init = sysbus_fdc_initfn,
     .class_init    = sysbus_fdc_class_init,
 };
@@ -2301,23 +2286,38 @@ static void sun4m_fdc_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    dc->realize = sun4m_fdc_realize;
-    dc->reset = fdctrl_external_reset_sysbus;
-    dc->vmsd = &vmstate_sysbus_fdc;
     dc->props = sun4m_fdc_properties;
 }
 
 static const TypeInfo sun4m_fdc_info = {
     .name          = "SUNW,fdtwo",
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(FDCtrlSysBus),
+    .parent        = TYPE_SYSBUS_FDC,
     .instance_init = sun4m_fdc_initfn,
     .class_init    = sun4m_fdc_class_init,
 };
 
+static void sysbus_fdc_common_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = sysbus_fdc_common_realize;
+    dc->reset = fdctrl_external_reset_sysbus;
+    dc->vmsd = &vmstate_sysbus_fdc;
+}
+
+static const TypeInfo sysbus_fdc_type_info = {
+    .name          = TYPE_SYSBUS_FDC,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(FDCtrlSysBus),
+    .instance_init = sysbus_fdc_common_initfn,
+    .abstract      = true,
+    .class_init    = sysbus_fdc_common_class_init,
+};
+
 static void fdc_register_types(void)
 {
     type_register_static(&isa_fdc_info);
+    type_register_static(&sysbus_fdc_type_info);
     type_register_static(&sysbus_fdc_info);
     type_register_static(&sun4m_fdc_info);
 }
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH qom-next for-1.6 v2] fdc: Fix inheritence for SUNW, fdtwo

[Andreas Färber]

Am 27.07.2013 03:50, schrieb Andreas Färber:
> Since commit dd3be7420774f7dc8f37a96ca24d07f0b6f31b3b SUNW,fdtwo's
> initfn (realizefn since 940194c2369e50d91d1abf6f36d43853eea5e539)
> was using SYSBUS_FDC() cast. This uses type sysbus-fdc rather than
> SUNW,fdtwo.
> 
> Fix this by letting SUNW,fdtwo and sysbus-fdc both inherit from an
> abstract type base-sysbus-fdc.
> 
> This allows to consolidate realizefns by using instance_init functions.
> Clean up variable names and variable order while at it.
> 
> Reported-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Cc: Hu Tao <hutao@cn.fujitsu.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  hw/block/fdc.c | 86 +++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 43 insertions(+), 43 deletions(-)
> 
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index d32f6ba..723d2c7 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -544,7 +544,7 @@ struct FDCtrl {
>      uint8_t timer1;
>  };
>  
> -#define TYPE_SYSBUS_FDC "sysbus-fdc"
> +#define TYPE_SYSBUS_FDC "base-sysbus-fdc"
>  #define SYSBUS_FDC(obj) OBJECT_CHECK(FDCtrlSysBus, (obj), TYPE_SYSBUS_FDC)
>  
>  typedef struct FDCtrlSysBus {
> @@ -2153,60 +2153,49 @@ static void isabus_fdc_realize(DeviceState *dev, Error **errp)
>  
>  static void sysbus_fdc_initfn(Object *obj)
>  {
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>      FDCtrlSysBus *sys = SYSBUS_FDC(obj);
>      FDCtrl *fdctrl = &sys->state;
>  
> +    fdctrl->dma_chann = -1;
> +
>      memory_region_init_io(&fdctrl->iomem, obj, &fdctrl_mem_ops, fdctrl,
>                            "fdc", 0x08);
> +    sysbus_init_mmio(sbd, &fdctrl->iomem);
>  }
>  
> -static void sysbus_fdc_realize(DeviceState *dev, Error **errp)
> +static void sun4m_fdc_initfn(Object *obj)
>  {
> -    FDCtrlSysBus *sys = SYSBUS_FDC(dev);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> +    FDCtrlSysBus *sys = SYSBUS_FDC(obj);
>      FDCtrl *fdctrl = &sys->state;
> -    SysBusDevice *b = SYS_BUS_DEVICE(dev);
> -    Error *err = NULL;
>  
> -    sysbus_init_mmio(b, &fdctrl->iomem);
> -    sysbus_init_irq(b, &fdctrl->irq);
> -    qdev_init_gpio_in(dev, fdctrl_handle_tc, 1);
> -    fdctrl->dma_chann = -1;
> +    fdctrl->sun4m = 1;
>  
> -    qdev_set_legacy_instance_id(dev, 0 /* io */, 2); /* FIXME */
> -    fdctrl_realize_common(fdctrl, &err);
> -    if (err != NULL) {
> -        error_propagate(errp, err);
> -        return;
> -    }
> +    memory_region_init_io(&fdctrl->iomem, obj, &fdctrl_mem_strict_ops,
> +                          fdctrl, "fdctrl", 0x08);
> +    sysbus_init_mmio(sbd, &fdctrl->iomem);
>  }
>  
> -static void sun4m_fdc_initfn(Object *obj)
> +static void sysbus_fdc_common_initfn(Object *obj)
>  {
> +    DeviceState *dev = DEVICE(obj);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>      FDCtrlSysBus *sys = SYSBUS_FDC(obj);
>      FDCtrl *fdctrl = &sys->state;
>  
> -    memory_region_init_io(&fdctrl->iomem, obj, &fdctrl_mem_strict_ops,
> -                          fdctrl, "fdctrl", 0x08);
> +    qdev_set_legacy_instance_id(dev, 0 /* io */, 2); /* FIXME */
> +
> +    sysbus_init_irq(sbd, &fdctrl->irq);
> +    qdev_init_gpio_in(dev, fdctrl_handle_tc, 1);
>  }
>  
> -static void sun4m_fdc_realize(DeviceState *dev, Error **errp)
> +static void sysbus_fdc_common_realize(DeviceState *dev, Error **errp)
>  {
>      FDCtrlSysBus *sys = SYSBUS_FDC(dev);
>      FDCtrl *fdctrl = &sys->state;
> -    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> -    Error *err = NULL;
>  
> -    sysbus_init_mmio(sbd, &fdctrl->iomem);
> -    sysbus_init_irq(sbd, &fdctrl->irq);
> -    qdev_init_gpio_in(dev, fdctrl_handle_tc, 1);
> -
> -    fdctrl->sun4m = 1;
> -    qdev_set_legacy_instance_id(dev, 0 /* io */, 2); /* FIXME */
> -    fdctrl_realize_common(fdctrl, &err);
> -    if (err != NULL) {
> -        error_propagate(errp, err);
> -        return;
> -    }
> +    fdctrl_realize_common(fdctrl, errp);
>  }
>  
>  FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i)
> @@ -2278,16 +2267,12 @@ static void sysbus_fdc_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
> -    dc->realize = sysbus_fdc_realize;
> -    dc->reset = fdctrl_external_reset_sysbus;
> -    dc->vmsd = &vmstate_sysbus_fdc;
>      dc->props = sysbus_fdc_properties;
>  }
>  
>  static const TypeInfo sysbus_fdc_info = {
> -    .name          = TYPE_SYSBUS_FDC,
> -    .parent        = TYPE_SYS_BUS_DEVICE,
> -    .instance_size = sizeof(FDCtrlSysBus),
> +    .name          = "sysbus-fdc",
> +    .parent        = TYPE_SYSBUS_FDC,
>      .instance_init = sysbus_fdc_initfn,
>      .class_init    = sysbus_fdc_class_init,
>  };
> @@ -2301,23 +2286,38 @@ static void sun4m_fdc_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>  
> -    dc->realize = sun4m_fdc_realize;
> -    dc->reset = fdctrl_external_reset_sysbus;
> -    dc->vmsd = &vmstate_sysbus_fdc;
>      dc->props = sun4m_fdc_properties;
>  }
>  
>  static const TypeInfo sun4m_fdc_info = {
>      .name          = "SUNW,fdtwo",
> -    .parent        = TYPE_SYS_BUS_DEVICE,
> -    .instance_size = sizeof(FDCtrlSysBus),
> +    .parent        = TYPE_SYSBUS_FDC,
>      .instance_init = sun4m_fdc_initfn,
>      .class_init    = sun4m_fdc_class_init,
>  };
>  
> +static void sysbus_fdc_common_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = sysbus_fdc_common_realize;
> +    dc->reset = fdctrl_external_reset_sysbus;
> +    dc->vmsd = &vmstate_sysbus_fdc;
> +}
> +
> +static const TypeInfo sysbus_fdc_type_info = {
> +    .name          = TYPE_SYSBUS_FDC,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(FDCtrlSysBus),
> +    .instance_init = sysbus_fdc_common_initfn,
> +    .abstract      = true,
> +    .class_init    = sysbus_fdc_common_class_init,
> +};
> +
>  static void fdc_register_types(void)
>  {
>      type_register_static(&isa_fdc_info);
> +    type_register_static(&sysbus_fdc_type_info);
>      type_register_static(&sysbus_fdc_info);
>      type_register_static(&sun4m_fdc_info);
>  }

Plus the following hunk to fix mips64:

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 723d2c7..28266c2 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2055,7 +2055,7 @@ void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
     SysBusDevice *sbd;
     FDCtrlSysBus *sys;

-    dev = qdev_create(NULL, TYPE_SYSBUS_FDC);
+    dev = qdev_create(NULL, "sysbus-fdc");
     sys = SYSBUS_FDC(dev);
     fdctrl = &sys->state;
     fdctrl->dma_chann = dma_chann; /* FIXME */

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH qom-next for-1.6 v2] fdc: Fix inheritence for SUNW, fdtwo

[Mark Cave-Ayland]

On 27/07/13 02:50, Andreas Färber wrote:

> Since commit dd3be7420774f7dc8f37a96ca24d07f0b6f31b3b SUNW,fdtwo's
> initfn (realizefn since 940194c2369e50d91d1abf6f36d43853eea5e539)
> was using SYSBUS_FDC() cast. This uses type sysbus-fdc rather than
> SUNW,fdtwo.
>
> Fix this by letting SUNW,fdtwo and sysbus-fdc both inherit from an
> abstract type base-sysbus-fdc.
>
> This allows to consolidate realizefns by using instance_init functions.
> Clean up variable names and variable order while at it.
>
> Reported-by: Mark Cave-Ayland<mark.cave-ayland@ilande.co.uk>
> Cc: Hu Tao<hutao@cn.fujitsu.com>
> Signed-off-by: Andreas Färber<afaerber@suse.de>

Hi Andreas,

I can confirm that this fixes SPARC32 for me - thanks!

Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.

Re: [Qemu-devel] [PATCH qom-next for-1.6 v2] fdc: Fix inheritence for SUNW, fdtwo

[Andreas Färber]

Am 29.07.2013 13:55, schrieb Mark Cave-Ayland:
> On 27/07/13 02:50, Andreas Färber wrote:
> 
>> Since commit dd3be7420774f7dc8f37a96ca24d07f0b6f31b3b SUNW,fdtwo's
>> initfn (realizefn since 940194c2369e50d91d1abf6f36d43853eea5e539)
>> was using SYSBUS_FDC() cast. This uses type sysbus-fdc rather than
>> SUNW,fdtwo.
>>
>> Fix this by letting SUNW,fdtwo and sysbus-fdc both inherit from an
>> abstract type base-sysbus-fdc.
>>
>> This allows to consolidate realizefns by using instance_init functions.
>> Clean up variable names and variable order while at it.
>>
>> Reported-by: Mark Cave-Ayland<mark.cave-ayland@ilande.co.uk>
>> Cc: Hu Tao<hutao@cn.fujitsu.com>
>> Signed-off-by: Andreas Färber<afaerber@suse.de>
> 
> Hi Andreas,
> 
> I can confirm that this fixes SPARC32 for me - thanks!
> 
> Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Thanks. Can you give my latest qom-devices branch a whirl, too?
I started adding trivial qtests for machines, but only covering the
default machines without -fda etc.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg