[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 01/14] hw/timer: QOM'ify arm_timer
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH v2 01/14] hw/timer: QOM'ify arm_timer |
Date: |
Mon, 15 Feb 2016 18:06:47 +0000 |
On 27 January 2016 at 02:54, xiaoqiang zhao <address@hidden> 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 <address@hidden>
> ---
> 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
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v2 01/14] hw/timer: QOM'ify arm_timer,
Peter Maydell <=