diff mbox

[v2,01/14] hw/timer: QOM'ify arm_timer

Message ID 1453863283-7562-2-git-send-email-zxq_yx_007@163.com (mailing list archive)
State New, archived
Headers show

Commit Message

zhao xiao qiang Jan. 27, 2016, 2:54 a.m. UTC 
* assign icp_pit_init to icp_pit_info.instance_init
* split sp804_init into sp804_info.instance_init (sp804_init) and sp804_realize
* use DeviceClass::realize instead of SysBusDeviceClass::init

Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
---
 hw/timer/arm_timer.c | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

Comments

Peter Maydell Feb. 15, 2016, 6:06 p.m. UTC | #1 
On 27 January 2016 at 02:54, xiaoqiang zhao <zxq_yx_007@163.com> wrote:
> * assign icp_pit_init to icp_pit_info.instance_init
> * split sp804_init into sp804_info.instance_init (sp804_init) and sp804_realize
> * use DeviceClass::realize instead of SysBusDeviceClass::init
>
> Signed-off-by: xiaoqiang zhao <zxq_yx_007@163.com>
> ---
>  hw/timer/arm_timer.c | 38 +++++++++++++++++++++-----------------
>  1 file changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/hw/timer/arm_timer.c b/hw/timer/arm_timer.c
> index d53f39a..cce1abc 100644
> --- a/hw/timer/arm_timer.c
> +++ b/hw/timer/arm_timer.c
> @@ -276,21 +276,26 @@ static const VMStateDescription vmstate_sp804 = {
>      }
>  };
>
> -static int sp804_init(SysBusDevice *sbd)
> +static void sp804_init(Object *obj)
>  {
> -    DeviceState *dev = DEVICE(sbd);
> -    SP804State *s = SP804(dev);
> +    SP804State *s = SP804(obj);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>
>      sysbus_init_irq(sbd, &s->irq);
> +    memory_region_init_io(&s->iomem, OBJECT(s), &sp804_ops, s,
> +                          "sp804", 0x1000);

You already have an Object* here, 'obj'; you don't
need to cast s back via OBJECT() again.

> +    sysbus_init_mmio(sbd, &s->iomem);
> +}
> +
> +static void sp804_realize(DeviceState *dev, Error **errp)
> +{
> +    SP804State *s = SP804(dev);
> +
>      s->timer[0] = arm_timer_init(s->freq0);
>      s->timer[1] = arm_timer_init(s->freq1);
>      s->timer[0]->irq = qemu_allocate_irq(sp804_set_irq, s, 0);
>      s->timer[1]->irq = qemu_allocate_irq(sp804_set_irq, s, 1);
> -    memory_region_init_io(&s->iomem, OBJECT(s), &sp804_ops, s,
> -                          "sp804", 0x1000);
> -    sysbus_init_mmio(sbd, &s->iomem);
>      vmstate_register(dev, -1, &vmstate_sp804, s);

It would be nice to register the VMState by setting the
DeviceClass vmsd member in the class init, rather than
vmstate_register(). (You can do that in a separate patch
if you prefer.)

> -    return 0;
>  }
>
>  /* Integrator/CP timer module.  */
> @@ -343,9 +348,10 @@ static const MemoryRegionOps icp_pit_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>
> -static int icp_pit_init(SysBusDevice *dev)
> +static void icp_pit_init(Object *obj)
>  {
> -    icp_pit_state *s = INTEGRATOR_PIT(dev);
> +    icp_pit_state *s = INTEGRATOR_PIT(obj);
> +    SysBusDevice *dev = SYS_BUS_DEVICE(obj);
>
>      /* Timer 0 runs at the system clock speed (40MHz).  */
>      s->timer[0] = arm_timer_init(40000000);
> @@ -362,20 +368,18 @@ static int icp_pit_init(SysBusDevice *dev)
>      sysbus_init_mmio(dev, &s->iomem);
>      /* This device has no state to save/restore.  The component timers will
>         save themselves.  */
> -    return 0;
>  }
>
>  static void icp_pit_class_init(ObjectClass *klass, void *data)
>  {
> -    SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
> -
> -    sdc->init = icp_pit_init;
> +    /* do nothing */

If there's no need for the class init function to do anything,
just delete it from the TypeInfo rather than leaving it but
with an empty function.

>  }
>
>  static const TypeInfo icp_pit_info = {
>      .name          = TYPE_INTEGRATOR_PIT,
>      .parent        = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(icp_pit_state),
> +    .instance_init = icp_pit_init,
>      .class_init    = icp_pit_class_init,
>  };
>
> @@ -387,17 +391,17 @@ static Property sp804_properties[] = {
>
>  static void sp804_class_init(ObjectClass *klass, void *data)
>  {
> -    SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
> -    DeviceClass *k = DEVICE_CLASS(klass);
> +    DeviceClass *dc = DEVICE_CLASS(klass);

Doesn't seem necessary to change the name of the DeviceClass* variable here.

>
> -    sdc->init = sp804_init;
> -    k->props = sp804_properties;
> +    dc->realize = sp804_realize;
> +    dc->props = sp804_properties;
>  }
>
>  static const TypeInfo sp804_info = {
>      .name          = TYPE_SP804,
>      .parent        = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(SP804State),
> +    .instance_init = sp804_init,
>      .class_init    = sp804_class_init,
>  };
>
> --
> 2.1.4

These devices are also missing reset handling, though that should
be added in a different patch.

thanks
-- PMM
diff mbox

Patch

diff --git a/hw/timer/arm_timer.c b/hw/timer/arm_timer.c
index d53f39a..cce1abc 100644
--- a/hw/timer/arm_timer.c
+++ b/hw/timer/arm_timer.c
@@ -276,21 +276,26 @@  static const VMStateDescription vmstate_sp804 = {
     }
 };
 
-static int sp804_init(SysBusDevice *sbd)
+static void sp804_init(Object *obj)
 {
-    DeviceState *dev = DEVICE(sbd);
-    SP804State *s = SP804(dev);
+    SP804State *s = SP804(obj);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 
     sysbus_init_irq(sbd, &s->irq);
+    memory_region_init_io(&s->iomem, OBJECT(s), &sp804_ops, s,
+                          "sp804", 0x1000);
+    sysbus_init_mmio(sbd, &s->iomem);
+}
+
+static void sp804_realize(DeviceState *dev, Error **errp)
+{
+    SP804State *s = SP804(dev);
+
     s->timer[0] = arm_timer_init(s->freq0);
     s->timer[1] = arm_timer_init(s->freq1);
     s->timer[0]->irq = qemu_allocate_irq(sp804_set_irq, s, 0);
     s->timer[1]->irq = qemu_allocate_irq(sp804_set_irq, s, 1);
-    memory_region_init_io(&s->iomem, OBJECT(s), &sp804_ops, s,
-                          "sp804", 0x1000);
-    sysbus_init_mmio(sbd, &s->iomem);
     vmstate_register(dev, -1, &vmstate_sp804, s);
-    return 0;
 }
 
 /* Integrator/CP timer module.  */
@@ -343,9 +348,10 @@  static const MemoryRegionOps icp_pit_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static int icp_pit_init(SysBusDevice *dev)
+static void icp_pit_init(Object *obj)
 {
-    icp_pit_state *s = INTEGRATOR_PIT(dev);
+    icp_pit_state *s = INTEGRATOR_PIT(obj);
+    SysBusDevice *dev = SYS_BUS_DEVICE(obj);
 
     /* Timer 0 runs at the system clock speed (40MHz).  */
     s->timer[0] = arm_timer_init(40000000);
@@ -362,20 +368,18 @@  static int icp_pit_init(SysBusDevice *dev)
     sysbus_init_mmio(dev, &s->iomem);
     /* This device has no state to save/restore.  The component timers will
        save themselves.  */
-    return 0;
 }
 
 static void icp_pit_class_init(ObjectClass *klass, void *data)
 {
-    SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
-
-    sdc->init = icp_pit_init;
+    /* do nothing */
 }
 
 static const TypeInfo icp_pit_info = {
     .name          = TYPE_INTEGRATOR_PIT,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(icp_pit_state),
+    .instance_init = icp_pit_init,
     .class_init    = icp_pit_class_init,
 };
 
@@ -387,17 +391,17 @@  static Property sp804_properties[] = {
 
 static void sp804_class_init(ObjectClass *klass, void *data)
 {
-    SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
-    DeviceClass *k = DEVICE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
 
-    sdc->init = sp804_init;
-    k->props = sp804_properties;
+    dc->realize = sp804_realize;
+    dc->props = sp804_properties;
 }
 
 static const TypeInfo sp804_info = {
     .name          = TYPE_SP804,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(SP804State),
+    .instance_init = sp804_init,
     .class_init    = sp804_class_init,
 };