diff mbox

[v7,3/6] hw/arm/digic: add timer support

Message ID 1386887022-11532-4-git-send-email-antonynpavlov@gmail.com
State New
Headers show

Commit Message

Antony Pavlov Dec. 12, 2013, 10:23 p.m. UTC
Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/digic.c         |  28 ++++++++++
 hw/timer/Makefile.objs |   1 +
 hw/timer/digic-timer.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++++
 hw/timer/digic-timer.h |  36 +++++++++++++
 include/hw/arm/digic.h |   6 +++
 5 files changed, 211 insertions(+)
 create mode 100644 hw/timer/digic-timer.c
 create mode 100644 hw/timer/digic-timer.h

Comments

Peter Crosthwaite Dec. 12, 2013, 11:20 p.m. UTC | #1
On Fri, Dec 13, 2013 at 8:23 AM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/digic.c         |  28 ++++++++++
>  hw/timer/Makefile.objs |   1 +
>  hw/timer/digic-timer.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/timer/digic-timer.h |  36 +++++++++++++
>  include/hw/arm/digic.h |   6 +++
>  5 files changed, 211 insertions(+)
>  create mode 100644 hw/timer/digic-timer.c
>  create mode 100644 hw/timer/digic-timer.h
>
> diff --git a/hw/arm/digic.c b/hw/arm/digic.c
> index 2620262..e8eb0de 100644
> --- a/hw/arm/digic.c
> +++ b/hw/arm/digic.c
> @@ -22,18 +22,35 @@
>
>  #include "hw/arm/digic.h"
>
> +#define DIGIC4_TIMER_BASE(n)    (0xc0210000 + (n) * 0x100)
> +
>  static void digic_init(Object *obj)
>  {
>      DigicState *s = DIGIC(obj);
> +    DeviceState *dev;
> +    int i;
>
>      object_initialize(&s->cpu, sizeof(s->cpu), "arm946-" TYPE_ARM_CPU);
>      object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
> +
> +    for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
> +#define DIGIC_TIMER_NAME_MLEN    11
> +        char name[DIGIC_TIMER_NAME_MLEN];
> +
> +        object_initialize(&s->timer[i], sizeof(s->timer[i]), TYPE_DIGIC_TIMER);
> +        dev = DEVICE(&s->timer[i]);
> +        qdev_set_parent_bus(dev, sysbus_get_default());
> +        snprintf(name, DIGIC_TIMER_NAME_MLEN, "timer[%d]", i);
> +        object_property_add_child(obj, name, OBJECT(&s->timer[i]), NULL);
> +    }
>  }
>
>  static void digic_realize(DeviceState *dev, Error **errp)
>  {
>      DigicState *s = DIGIC(dev);
>      Error *err = NULL;
> +    SysBusDevice *sbd;
> +    int i;
>
>      object_property_set_bool(OBJECT(&s->cpu), true, "reset-hivecs", &err);
>      if (err != NULL) {
> @@ -46,6 +63,17 @@ static void digic_realize(DeviceState *dev, Error **errp)
>          error_propagate(errp, err);
>          return;
>      }
> +
> +    for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
> +        object_property_set_bool(OBJECT(&s->timer[i]), true, "realized", &err);
> +        if (err != NULL) {
> +            error_propagate(errp, err);
> +            return;
> +        }
> +
> +        sbd = SYS_BUS_DEVICE(&s->timer[i]);
> +        sysbus_mmio_map(sbd, 0, DIGIC4_TIMER_BASE(i));
> +    }
>  }
>
>  static void digic_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
> index 3ae091c..ea9f11f 100644
> --- a/hw/timer/Makefile.objs
> +++ b/hw/timer/Makefile.objs
> @@ -26,5 +26,6 @@ obj-$(CONFIG_OMAP) += omap_synctimer.o
>  obj-$(CONFIG_PXA2XX) += pxa2xx_timer.o
>  obj-$(CONFIG_SH4) += sh_timer.o
>  obj-$(CONFIG_TUSB6010) += tusb6010.o
> +obj-$(CONFIG_DIGIC) += digic-timer.o
>
>  obj-$(CONFIG_MC146818RTC) += mc146818rtc.o
> diff --git a/hw/timer/digic-timer.c b/hw/timer/digic-timer.c
> new file mode 100644
> index 0000000..974e588
> --- /dev/null
> +++ b/hw/timer/digic-timer.c
> @@ -0,0 +1,140 @@
> +/*
> + * QEMU model of the Canon DIGIC timer block.
> + *
> + * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
> + *
> + * This model is based on reverse engineering efforts
> + * made by CHDK (http://chdk.wikia.com) and
> + * Magic Lantern (http://www.magiclantern.fm) projects
> + * contributors.
> + *
> + * See "Timer/Clock Module" docs here:
> + *   http://magiclantern.wikia.com/wiki/Register_Map
> + *
> + * The QEMU model of the OSTimer in PKUnity SoC by Guan Xuetao
> + * is used as a template.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#include "hw/sysbus.h"
> +#include "hw/ptimer.h"
> +#include "qemu/main-loop.h"
> +
> +#include "hw/timer/digic-timer.h"
> +
> +#define DIGIC_TIMER_CONTROL 0x00
> +#define DIGIC_TIMER_VALUE 0x0c
> +
> +static const VMStateDescription vmstate_digic_timer = {
> +    .name = "digic.timer",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_PTIMER(ptimer, DigicTimerState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static uint64_t digic_timer_read(void *opaque, hwaddr offset, unsigned size)
> +{
> +    DigicTimerState *s = opaque;
> +    uint32_t ret = 0;
> +
> +    switch (offset) {
> +    case DIGIC_TIMER_VALUE:
> +        ret = (uint32_t)ptimer_get_count(s->ptimer);
> +        ret &= 0xffff;
> +        break;
> +    default:
> +        qemu_log_mask(LOG_UNIMP,
> +                      "digic-timer: read access to unknown register 0x"
> +                      TARGET_FMT_plx, offset);
> +    }
> +
> +    return ret;
> +}
> +
> +static void digic_timer_write(void *opaque, hwaddr offset,
> +                              uint64_t value, unsigned size)
> +{
> +    DigicTimerState *s = opaque;
> +
> +    /* FIXME: without documentation every write just starts timer */
> +    ptimer_set_limit(s->ptimer, 0x0000ffff, 1);
> +    ptimer_run(s->ptimer, 1);

You are chaining one-shot timers which is not the correct way to do
periodic timing. Can't you just start the timer as periodic here ...

> +}
> +
> +static const MemoryRegionOps digic_timer_ops = {
> +    .read = digic_timer_read,
> +    .write = digic_timer_write,
> +    .impl = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    },
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static void digic_timer_tick(void *opaque)
> +{
> +    DigicTimerState *s = opaque;
> +
> +    ptimer_run(s->ptimer, 1);

... and then this is just a nop?

Regards,
Peter

> +}
> +
> +static void digic_timer_init(Object *obj)
> +{
> +    DigicTimerState *s = DIGIC_TIMER(obj);
> +
> +    s->bh = qemu_bh_new(digic_timer_tick, s);
> +    s->ptimer = ptimer_init(s->bh);
> +
> +    /*
> +     * FIXME: there is no documentation on Digic timer
> +     * frequency setup so let it always run at 1 MHz
> +     */
> +    ptimer_set_freq(s->ptimer, 1 * 1000 * 1000);
> +
> +    memory_region_init_io(&s->iomem, OBJECT(s), &digic_timer_ops, s,
> +                          TYPE_DIGIC_TIMER, 0x100);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
> +}
> +
> +static void digic_timer_reset(DeviceState *dev)
> +{
> +    DigicTimerState *s = DIGIC_TIMER(dev);
> +
> +    ptimer_stop(s->ptimer);
> +}
> +
> +static void digic_timer_class_init(ObjectClass *klass, void *class_data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    dc->reset = digic_timer_reset;
> +    dc->vmsd = &vmstate_digic_timer;
> +}
> +
> +static const TypeInfo digic_timer_info = {
> +    .name = TYPE_DIGIC_TIMER,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(DigicTimerState),
> +    .instance_init = digic_timer_init,
> +    .class_init = digic_timer_class_init,
> +};
> +
> +static void digic_timer_register_type(void)
> +{
> +    type_register_static(&digic_timer_info);
> +}
> +
> +type_init(digic_timer_register_type)
> diff --git a/hw/timer/digic-timer.h b/hw/timer/digic-timer.h
> new file mode 100644
> index 0000000..daf271d
> --- /dev/null
> +++ b/hw/timer/digic-timer.h
> @@ -0,0 +1,36 @@
> +/*
> + * Canon DIGIC timer block declarations.
> + *
> + * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + */
> +
> +#ifndef HW_TIMER_DIGIC_TIMER_H
> +#define HW_TIMER_DIGIC_TIMER_H
> +
> +#include "hw/sysbus.h"
> +#include "qemu/typedefs.h"
> +#include "hw/ptimer.h"
> +
> +#define TYPE_DIGIC_TIMER "digic-timer"
> +#define DIGIC_TIMER(obj) OBJECT_CHECK(DigicTimerState, (obj), TYPE_DIGIC_TIMER)
> +
> +typedef struct DigicTimerState {
> +    SysBusDevice parent_obj;
> +
> +    MemoryRegion iomem;
> +    QEMUBH *bh;
> +    ptimer_state *ptimer;
> +} DigicTimerState;
> +
> +#endif /* HW_TIMER_DIGIC_TIMER_H */
> diff --git a/include/hw/arm/digic.h b/include/hw/arm/digic.h
> index b7d16fb..177a06d 100644
> --- a/include/hw/arm/digic.h
> +++ b/include/hw/arm/digic.h
> @@ -20,16 +20,22 @@
>
>  #include "cpu.h"
>
> +#include "hw/timer/digic-timer.h"
> +
>  #define TYPE_DIGIC "digic"
>
>  #define DIGIC(obj) OBJECT_CHECK(DigicState, (obj), TYPE_DIGIC)
>
> +#define DIGIC4_NB_TIMERS 3
> +
>  typedef struct DigicState {
>      /*< private >*/
>      DeviceState parent_obj;
>      /*< public >*/
>
>      ARMCPU cpu;
> +
> +    DigicTimerState timer[DIGIC4_NB_TIMERS];
>  } DigicState;
>
>  #endif /* HW_ARM_DIGIC_H */
> --
> 1.8.5
>
>
Antony Pavlov Dec. 13, 2013, 5:43 a.m. UTC | #2
On Fri, 13 Dec 2013 09:20:27 +1000
Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:

> On Fri, Dec 13, 2013 at 8:23 AM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >  hw/arm/digic.c         |  28 ++++++++++
> >  hw/timer/Makefile.objs |   1 +
> >  hw/timer/digic-timer.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  hw/timer/digic-timer.h |  36 +++++++++++++
> >  include/hw/arm/digic.h |   6 +++
> >  5 files changed, 211 insertions(+)
> >  create mode 100644 hw/timer/digic-timer.c
> >  create mode 100644 hw/timer/digic-timer.h
> >
> > diff --git a/hw/arm/digic.c b/hw/arm/digic.c
> > index 2620262..e8eb0de 100644
> > --- a/hw/arm/digic.c
> > +++ b/hw/arm/digic.c
> > @@ -22,18 +22,35 @@
> >
> >  #include "hw/arm/digic.h"
> >
> > +#define DIGIC4_TIMER_BASE(n)    (0xc0210000 + (n) * 0x100)
> > +
> >  static void digic_init(Object *obj)
> >  {
> >      DigicState *s = DIGIC(obj);
> > +    DeviceState *dev;
> > +    int i;
> >
> >      object_initialize(&s->cpu, sizeof(s->cpu), "arm946-" TYPE_ARM_CPU);
> >      object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
> > +
> > +    for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
> > +#define DIGIC_TIMER_NAME_MLEN    11
> > +        char name[DIGIC_TIMER_NAME_MLEN];
> > +
> > +        object_initialize(&s->timer[i], sizeof(s->timer[i]), TYPE_DIGIC_TIMER);
> > +        dev = DEVICE(&s->timer[i]);
> > +        qdev_set_parent_bus(dev, sysbus_get_default());
> > +        snprintf(name, DIGIC_TIMER_NAME_MLEN, "timer[%d]", i);
> > +        object_property_add_child(obj, name, OBJECT(&s->timer[i]), NULL);
> > +    }
> >  }
> >
> >  static void digic_realize(DeviceState *dev, Error **errp)
> >  {
> >      DigicState *s = DIGIC(dev);
> >      Error *err = NULL;
> > +    SysBusDevice *sbd;
> > +    int i;
> >
> >      object_property_set_bool(OBJECT(&s->cpu), true, "reset-hivecs", &err);
> >      if (err != NULL) {
> > @@ -46,6 +63,17 @@ static void digic_realize(DeviceState *dev, Error **errp)
> >          error_propagate(errp, err);
> >          return;
> >      }
> > +
> > +    for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
> > +        object_property_set_bool(OBJECT(&s->timer[i]), true, "realized", &err);
> > +        if (err != NULL) {
> > +            error_propagate(errp, err);
> > +            return;
> > +        }
> > +
> > +        sbd = SYS_BUS_DEVICE(&s->timer[i]);
> > +        sysbus_mmio_map(sbd, 0, DIGIC4_TIMER_BASE(i));
> > +    }
> >  }
> >
> >  static void digic_class_init(ObjectClass *oc, void *data)
> > diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
> > index 3ae091c..ea9f11f 100644
> > --- a/hw/timer/Makefile.objs
> > +++ b/hw/timer/Makefile.objs
> > @@ -26,5 +26,6 @@ obj-$(CONFIG_OMAP) += omap_synctimer.o
> >  obj-$(CONFIG_PXA2XX) += pxa2xx_timer.o
> >  obj-$(CONFIG_SH4) += sh_timer.o
> >  obj-$(CONFIG_TUSB6010) += tusb6010.o
> > +obj-$(CONFIG_DIGIC) += digic-timer.o
> >
> >  obj-$(CONFIG_MC146818RTC) += mc146818rtc.o
> > diff --git a/hw/timer/digic-timer.c b/hw/timer/digic-timer.c
> > new file mode 100644
> > index 0000000..974e588
> > --- /dev/null
> > +++ b/hw/timer/digic-timer.c
> > @@ -0,0 +1,140 @@
> > +/*
> > + * QEMU model of the Canon DIGIC timer block.
> > + *
> > + * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
> > + *
> > + * This model is based on reverse engineering efforts
> > + * made by CHDK (http://chdk.wikia.com) and
> > + * Magic Lantern (http://www.magiclantern.fm) projects
> > + * contributors.
> > + *
> > + * See "Timer/Clock Module" docs here:
> > + *   http://magiclantern.wikia.com/wiki/Register_Map
> > + *
> > + * The QEMU model of the OSTimer in PKUnity SoC by Guan Xuetao
> > + * is used as a template.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + */
> > +
> > +#include "hw/sysbus.h"
> > +#include "hw/ptimer.h"
> > +#include "qemu/main-loop.h"
> > +
> > +#include "hw/timer/digic-timer.h"
> > +
> > +#define DIGIC_TIMER_CONTROL 0x00
> > +#define DIGIC_TIMER_VALUE 0x0c
> > +
> > +static const VMStateDescription vmstate_digic_timer = {
> > +    .name = "digic.timer",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +    .minimum_version_id_old = 1,
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_PTIMER(ptimer, DigicTimerState),
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +
> > +static uint64_t digic_timer_read(void *opaque, hwaddr offset, unsigned size)
> > +{
> > +    DigicTimerState *s = opaque;
> > +    uint32_t ret = 0;
> > +
> > +    switch (offset) {
> > +    case DIGIC_TIMER_VALUE:
> > +        ret = (uint32_t)ptimer_get_count(s->ptimer);
> > +        ret &= 0xffff;
> > +        break;
> > +    default:
> > +        qemu_log_mask(LOG_UNIMP,
> > +                      "digic-timer: read access to unknown register 0x"
> > +                      TARGET_FMT_plx, offset);
> > +    }
> > +
> > +    return ret;
> > +}
> > +
> > +static void digic_timer_write(void *opaque, hwaddr offset,
> > +                              uint64_t value, unsigned size)
> > +{
> > +    DigicTimerState *s = opaque;
> > +
> > +    /* FIXME: without documentation every write just starts timer */
> > +    ptimer_set_limit(s->ptimer, 0x0000ffff, 1);
> > +    ptimer_run(s->ptimer, 1);
> 
> You are chaining one-shot timers which is not the correct way to do
> periodic timing. Can't you just start the timer as periodic here ...

I used hw/timer/lm32_timer.c as a model. Could you point to a better model please?


> > +}
> > +
> > +static const MemoryRegionOps digic_timer_ops = {
> > +    .read = digic_timer_read,
> > +    .write = digic_timer_write,
> > +    .impl = {
> > +        .min_access_size = 4,
> > +        .max_access_size = 4,
> > +    },
> > +    .endianness = DEVICE_NATIVE_ENDIAN,
> > +};
> > +
> > +static void digic_timer_tick(void *opaque)
> > +{
> > +    DigicTimerState *s = opaque;
> > +
> > +    ptimer_run(s->ptimer, 1);
> 
> ... and then this is just a nop?
> 
> Regards,
> Peter
> 
> > +}
> > +
> > +static void digic_timer_init(Object *obj)
> > +{
> > +    DigicTimerState *s = DIGIC_TIMER(obj);
> > +
> > +    s->bh = qemu_bh_new(digic_timer_tick, s);
> > +    s->ptimer = ptimer_init(s->bh);
> > +
> > +    /*
> > +     * FIXME: there is no documentation on Digic timer
> > +     * frequency setup so let it always run at 1 MHz
> > +     */
> > +    ptimer_set_freq(s->ptimer, 1 * 1000 * 1000);
> > +
> > +    memory_region_init_io(&s->iomem, OBJECT(s), &digic_timer_ops, s,
> > +                          TYPE_DIGIC_TIMER, 0x100);
> > +    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
> > +}
> > +
> > +static void digic_timer_reset(DeviceState *dev)
> > +{
> > +    DigicTimerState *s = DIGIC_TIMER(dev);
> > +
> > +    ptimer_stop(s->ptimer);
> > +}
> > +
> > +static void digic_timer_class_init(ObjectClass *klass, void *class_data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +    dc->reset = digic_timer_reset;
> > +    dc->vmsd = &vmstate_digic_timer;
> > +}
> > +
> > +static const TypeInfo digic_timer_info = {
> > +    .name = TYPE_DIGIC_TIMER,
> > +    .parent = TYPE_SYS_BUS_DEVICE,
> > +    .instance_size = sizeof(DigicTimerState),
> > +    .instance_init = digic_timer_init,
> > +    .class_init = digic_timer_class_init,
> > +};
> > +
> > +static void digic_timer_register_type(void)
> > +{
> > +    type_register_static(&digic_timer_info);
> > +}
> > +
> > +type_init(digic_timer_register_type)
> > diff --git a/hw/timer/digic-timer.h b/hw/timer/digic-timer.h
> > new file mode 100644
> > index 0000000..daf271d
> > --- /dev/null
> > +++ b/hw/timer/digic-timer.h
> > @@ -0,0 +1,36 @@
> > +/*
> > + * Canon DIGIC timer block declarations.
> > + *
> > + * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + */
> > +
> > +#ifndef HW_TIMER_DIGIC_TIMER_H
> > +#define HW_TIMER_DIGIC_TIMER_H
> > +
> > +#include "hw/sysbus.h"
> > +#include "qemu/typedefs.h"
> > +#include "hw/ptimer.h"
> > +
> > +#define TYPE_DIGIC_TIMER "digic-timer"
> > +#define DIGIC_TIMER(obj) OBJECT_CHECK(DigicTimerState, (obj), TYPE_DIGIC_TIMER)
> > +
> > +typedef struct DigicTimerState {
> > +    SysBusDevice parent_obj;
> > +
> > +    MemoryRegion iomem;
> > +    QEMUBH *bh;
> > +    ptimer_state *ptimer;
> > +} DigicTimerState;
> > +
> > +#endif /* HW_TIMER_DIGIC_TIMER_H */
> > diff --git a/include/hw/arm/digic.h b/include/hw/arm/digic.h
> > index b7d16fb..177a06d 100644
> > --- a/include/hw/arm/digic.h
> > +++ b/include/hw/arm/digic.h
> > @@ -20,16 +20,22 @@
> >
> >  #include "cpu.h"
> >
> > +#include "hw/timer/digic-timer.h"
> > +
> >  #define TYPE_DIGIC "digic"
> >
> >  #define DIGIC(obj) OBJECT_CHECK(DigicState, (obj), TYPE_DIGIC)
> >
> > +#define DIGIC4_NB_TIMERS 3
> > +
> >  typedef struct DigicState {
> >      /*< private >*/
> >      DeviceState parent_obj;
> >      /*< public >*/
> >
> >      ARMCPU cpu;
> > +
> > +    DigicTimerState timer[DIGIC4_NB_TIMERS];
> >  } DigicState;
> >
> >  #endif /* HW_ARM_DIGIC_H */
> > --
> > 1.8.5
> >
> >
Peter Crosthwaite Dec. 13, 2013, 7:19 a.m. UTC | #3
On Fri, Dec 13, 2013 at 3:43 PM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
> On Fri, 13 Dec 2013 09:20:27 +1000
> Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>
>> On Fri, Dec 13, 2013 at 8:23 AM, Antony Pavlov <antonynpavlov@gmail.com> wrote:
>> > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
>> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>> > ---
>> >  hw/arm/digic.c         |  28 ++++++++++
>> >  hw/timer/Makefile.objs |   1 +
>> >  hw/timer/digic-timer.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++++
>> >  hw/timer/digic-timer.h |  36 +++++++++++++
>> >  include/hw/arm/digic.h |   6 +++
>> >  5 files changed, 211 insertions(+)
>> >  create mode 100644 hw/timer/digic-timer.c
>> >  create mode 100644 hw/timer/digic-timer.h
>> >
>> > diff --git a/hw/arm/digic.c b/hw/arm/digic.c
>> > index 2620262..e8eb0de 100644
>> > --- a/hw/arm/digic.c
>> > +++ b/hw/arm/digic.c
>> > @@ -22,18 +22,35 @@
>> >
>> >  #include "hw/arm/digic.h"
>> >
>> > +#define DIGIC4_TIMER_BASE(n)    (0xc0210000 + (n) * 0x100)
>> > +
>> >  static void digic_init(Object *obj)
>> >  {
>> >      DigicState *s = DIGIC(obj);
>> > +    DeviceState *dev;
>> > +    int i;
>> >
>> >      object_initialize(&s->cpu, sizeof(s->cpu), "arm946-" TYPE_ARM_CPU);
>> >      object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
>> > +
>> > +    for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
>> > +#define DIGIC_TIMER_NAME_MLEN    11
>> > +        char name[DIGIC_TIMER_NAME_MLEN];
>> > +
>> > +        object_initialize(&s->timer[i], sizeof(s->timer[i]), TYPE_DIGIC_TIMER);
>> > +        dev = DEVICE(&s->timer[i]);
>> > +        qdev_set_parent_bus(dev, sysbus_get_default());
>> > +        snprintf(name, DIGIC_TIMER_NAME_MLEN, "timer[%d]", i);
>> > +        object_property_add_child(obj, name, OBJECT(&s->timer[i]), NULL);
>> > +    }
>> >  }
>> >
>> >  static void digic_realize(DeviceState *dev, Error **errp)
>> >  {
>> >      DigicState *s = DIGIC(dev);
>> >      Error *err = NULL;
>> > +    SysBusDevice *sbd;
>> > +    int i;
>> >
>> >      object_property_set_bool(OBJECT(&s->cpu), true, "reset-hivecs", &err);
>> >      if (err != NULL) {
>> > @@ -46,6 +63,17 @@ static void digic_realize(DeviceState *dev, Error **errp)
>> >          error_propagate(errp, err);
>> >          return;
>> >      }
>> > +
>> > +    for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
>> > +        object_property_set_bool(OBJECT(&s->timer[i]), true, "realized", &err);
>> > +        if (err != NULL) {
>> > +            error_propagate(errp, err);
>> > +            return;
>> > +        }
>> > +
>> > +        sbd = SYS_BUS_DEVICE(&s->timer[i]);
>> > +        sysbus_mmio_map(sbd, 0, DIGIC4_TIMER_BASE(i));
>> > +    }
>> >  }
>> >
>> >  static void digic_class_init(ObjectClass *oc, void *data)
>> > diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
>> > index 3ae091c..ea9f11f 100644
>> > --- a/hw/timer/Makefile.objs
>> > +++ b/hw/timer/Makefile.objs
>> > @@ -26,5 +26,6 @@ obj-$(CONFIG_OMAP) += omap_synctimer.o
>> >  obj-$(CONFIG_PXA2XX) += pxa2xx_timer.o
>> >  obj-$(CONFIG_SH4) += sh_timer.o
>> >  obj-$(CONFIG_TUSB6010) += tusb6010.o
>> > +obj-$(CONFIG_DIGIC) += digic-timer.o
>> >
>> >  obj-$(CONFIG_MC146818RTC) += mc146818rtc.o
>> > diff --git a/hw/timer/digic-timer.c b/hw/timer/digic-timer.c
>> > new file mode 100644
>> > index 0000000..974e588
>> > --- /dev/null
>> > +++ b/hw/timer/digic-timer.c
>> > @@ -0,0 +1,140 @@
>> > +/*
>> > + * QEMU model of the Canon DIGIC timer block.
>> > + *
>> > + * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
>> > + *
>> > + * This model is based on reverse engineering efforts
>> > + * made by CHDK (http://chdk.wikia.com) and
>> > + * Magic Lantern (http://www.magiclantern.fm) projects
>> > + * contributors.
>> > + *
>> > + * See "Timer/Clock Module" docs here:
>> > + *   http://magiclantern.wikia.com/wiki/Register_Map
>> > + *
>> > + * The QEMU model of the OSTimer in PKUnity SoC by Guan Xuetao
>> > + * is used as a template.
>> > + *
>> > + * This program is free software; you can redistribute it and/or modify
>> > + * it under the terms of the GNU General Public License as published by
>> > + * the Free Software Foundation; either version 2 of the License, or
>> > + * (at your option) any later version.
>> > + *
>> > + * This program is distributed in the hope that it will be useful,
>> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> > + * GNU General Public License for more details.
>> > + *
>> > + */
>> > +
>> > +#include "hw/sysbus.h"
>> > +#include "hw/ptimer.h"
>> > +#include "qemu/main-loop.h"
>> > +
>> > +#include "hw/timer/digic-timer.h"
>> > +
>> > +#define DIGIC_TIMER_CONTROL 0x00
>> > +#define DIGIC_TIMER_VALUE 0x0c
>> > +
>> > +static const VMStateDescription vmstate_digic_timer = {
>> > +    .name = "digic.timer",
>> > +    .version_id = 1,
>> > +    .minimum_version_id = 1,
>> > +    .minimum_version_id_old = 1,
>> > +    .fields = (VMStateField[]) {
>> > +        VMSTATE_PTIMER(ptimer, DigicTimerState),
>> > +        VMSTATE_END_OF_LIST()
>> > +    }
>> > +};
>> > +
>> > +static uint64_t digic_timer_read(void *opaque, hwaddr offset, unsigned size)
>> > +{
>> > +    DigicTimerState *s = opaque;
>> > +    uint32_t ret = 0;
>> > +
>> > +    switch (offset) {
>> > +    case DIGIC_TIMER_VALUE:
>> > +        ret = (uint32_t)ptimer_get_count(s->ptimer);
>> > +        ret &= 0xffff;
>> > +        break;
>> > +    default:
>> > +        qemu_log_mask(LOG_UNIMP,
>> > +                      "digic-timer: read access to unknown register 0x"
>> > +                      TARGET_FMT_plx, offset);
>> > +    }
>> > +
>> > +    return ret;
>> > +}
>> > +
>> > +static void digic_timer_write(void *opaque, hwaddr offset,
>> > +                              uint64_t value, unsigned size)
>> > +{
>> > +    DigicTimerState *s = opaque;
>> > +
>> > +    /* FIXME: without documentation every write just starts timer */
>> > +    ptimer_set_limit(s->ptimer, 0x0000ffff, 1);
>> > +    ptimer_run(s->ptimer, 1);
>>
>> You are chaining one-shot timers which is not the correct way to do
>> periodic timing. Can't you just start the timer as periodic here ...
>
> I used hw/timer/lm32_timer.c as a model. Could you point to a better model please?
>
>

Liguang's allwinner a10 timer (currently on list and in late stages of
review) is now the most modern timer in QEMU and this issue was
addressed here. Check v11 of that series for use of ptimer_run(foo,
0). There are examples in tree too, although most are stylistically
deprecated in some way or other. Your timer looks incredibly simple I
think you just need to start it periodic (ptimer_run() second arg =
0), and just make your hit callback a nop.

Regards,
peter
diff mbox

Patch

diff --git a/hw/arm/digic.c b/hw/arm/digic.c
index 2620262..e8eb0de 100644
--- a/hw/arm/digic.c
+++ b/hw/arm/digic.c
@@ -22,18 +22,35 @@ 
 
 #include "hw/arm/digic.h"
 
+#define DIGIC4_TIMER_BASE(n)    (0xc0210000 + (n) * 0x100)
+
 static void digic_init(Object *obj)
 {
     DigicState *s = DIGIC(obj);
+    DeviceState *dev;
+    int i;
 
     object_initialize(&s->cpu, sizeof(s->cpu), "arm946-" TYPE_ARM_CPU);
     object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
+
+    for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
+#define DIGIC_TIMER_NAME_MLEN    11
+        char name[DIGIC_TIMER_NAME_MLEN];
+
+        object_initialize(&s->timer[i], sizeof(s->timer[i]), TYPE_DIGIC_TIMER);
+        dev = DEVICE(&s->timer[i]);
+        qdev_set_parent_bus(dev, sysbus_get_default());
+        snprintf(name, DIGIC_TIMER_NAME_MLEN, "timer[%d]", i);
+        object_property_add_child(obj, name, OBJECT(&s->timer[i]), NULL);
+    }
 }
 
 static void digic_realize(DeviceState *dev, Error **errp)
 {
     DigicState *s = DIGIC(dev);
     Error *err = NULL;
+    SysBusDevice *sbd;
+    int i;
 
     object_property_set_bool(OBJECT(&s->cpu), true, "reset-hivecs", &err);
     if (err != NULL) {
@@ -46,6 +63,17 @@  static void digic_realize(DeviceState *dev, Error **errp)
         error_propagate(errp, err);
         return;
     }
+
+    for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
+        object_property_set_bool(OBJECT(&s->timer[i]), true, "realized", &err);
+        if (err != NULL) {
+            error_propagate(errp, err);
+            return;
+        }
+
+        sbd = SYS_BUS_DEVICE(&s->timer[i]);
+        sysbus_mmio_map(sbd, 0, DIGIC4_TIMER_BASE(i));
+    }
 }
 
 static void digic_class_init(ObjectClass *oc, void *data)
diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index 3ae091c..ea9f11f 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -26,5 +26,6 @@  obj-$(CONFIG_OMAP) += omap_synctimer.o
 obj-$(CONFIG_PXA2XX) += pxa2xx_timer.o
 obj-$(CONFIG_SH4) += sh_timer.o
 obj-$(CONFIG_TUSB6010) += tusb6010.o
+obj-$(CONFIG_DIGIC) += digic-timer.o
 
 obj-$(CONFIG_MC146818RTC) += mc146818rtc.o
diff --git a/hw/timer/digic-timer.c b/hw/timer/digic-timer.c
new file mode 100644
index 0000000..974e588
--- /dev/null
+++ b/hw/timer/digic-timer.c
@@ -0,0 +1,140 @@ 
+/*
+ * QEMU model of the Canon DIGIC timer block.
+ *
+ * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
+ *
+ * This model is based on reverse engineering efforts
+ * made by CHDK (http://chdk.wikia.com) and
+ * Magic Lantern (http://www.magiclantern.fm) projects
+ * contributors.
+ *
+ * See "Timer/Clock Module" docs here:
+ *   http://magiclantern.wikia.com/wiki/Register_Map
+ *
+ * The QEMU model of the OSTimer in PKUnity SoC by Guan Xuetao
+ * is used as a template.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include "hw/sysbus.h"
+#include "hw/ptimer.h"
+#include "qemu/main-loop.h"
+
+#include "hw/timer/digic-timer.h"
+
+#define DIGIC_TIMER_CONTROL 0x00
+#define DIGIC_TIMER_VALUE 0x0c
+
+static const VMStateDescription vmstate_digic_timer = {
+    .name = "digic.timer",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_PTIMER(ptimer, DigicTimerState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static uint64_t digic_timer_read(void *opaque, hwaddr offset, unsigned size)
+{
+    DigicTimerState *s = opaque;
+    uint32_t ret = 0;
+
+    switch (offset) {
+    case DIGIC_TIMER_VALUE:
+        ret = (uint32_t)ptimer_get_count(s->ptimer);
+        ret &= 0xffff;
+        break;
+    default:
+        qemu_log_mask(LOG_UNIMP,
+                      "digic-timer: read access to unknown register 0x"
+                      TARGET_FMT_plx, offset);
+    }
+
+    return ret;
+}
+
+static void digic_timer_write(void *opaque, hwaddr offset,
+                              uint64_t value, unsigned size)
+{
+    DigicTimerState *s = opaque;
+
+    /* FIXME: without documentation every write just starts timer */
+    ptimer_set_limit(s->ptimer, 0x0000ffff, 1);
+    ptimer_run(s->ptimer, 1);
+}
+
+static const MemoryRegionOps digic_timer_ops = {
+    .read = digic_timer_read,
+    .write = digic_timer_write,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void digic_timer_tick(void *opaque)
+{
+    DigicTimerState *s = opaque;
+
+    ptimer_run(s->ptimer, 1);
+}
+
+static void digic_timer_init(Object *obj)
+{
+    DigicTimerState *s = DIGIC_TIMER(obj);
+
+    s->bh = qemu_bh_new(digic_timer_tick, s);
+    s->ptimer = ptimer_init(s->bh);
+
+    /*
+     * FIXME: there is no documentation on Digic timer
+     * frequency setup so let it always run at 1 MHz
+     */
+    ptimer_set_freq(s->ptimer, 1 * 1000 * 1000);
+
+    memory_region_init_io(&s->iomem, OBJECT(s), &digic_timer_ops, s,
+                          TYPE_DIGIC_TIMER, 0x100);
+    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
+}
+
+static void digic_timer_reset(DeviceState *dev)
+{
+    DigicTimerState *s = DIGIC_TIMER(dev);
+
+    ptimer_stop(s->ptimer);
+}
+
+static void digic_timer_class_init(ObjectClass *klass, void *class_data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    dc->reset = digic_timer_reset;
+    dc->vmsd = &vmstate_digic_timer;
+}
+
+static const TypeInfo digic_timer_info = {
+    .name = TYPE_DIGIC_TIMER,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(DigicTimerState),
+    .instance_init = digic_timer_init,
+    .class_init = digic_timer_class_init,
+};
+
+static void digic_timer_register_type(void)
+{
+    type_register_static(&digic_timer_info);
+}
+
+type_init(digic_timer_register_type)
diff --git a/hw/timer/digic-timer.h b/hw/timer/digic-timer.h
new file mode 100644
index 0000000..daf271d
--- /dev/null
+++ b/hw/timer/digic-timer.h
@@ -0,0 +1,36 @@ 
+/*
+ * Canon DIGIC timer block declarations.
+ *
+ * Copyright (C) 2013 Antony Pavlov <antonynpavlov@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef HW_TIMER_DIGIC_TIMER_H
+#define HW_TIMER_DIGIC_TIMER_H
+
+#include "hw/sysbus.h"
+#include "qemu/typedefs.h"
+#include "hw/ptimer.h"
+
+#define TYPE_DIGIC_TIMER "digic-timer"
+#define DIGIC_TIMER(obj) OBJECT_CHECK(DigicTimerState, (obj), TYPE_DIGIC_TIMER)
+
+typedef struct DigicTimerState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion iomem;
+    QEMUBH *bh;
+    ptimer_state *ptimer;
+} DigicTimerState;
+
+#endif /* HW_TIMER_DIGIC_TIMER_H */
diff --git a/include/hw/arm/digic.h b/include/hw/arm/digic.h
index b7d16fb..177a06d 100644
--- a/include/hw/arm/digic.h
+++ b/include/hw/arm/digic.h
@@ -20,16 +20,22 @@ 
 
 #include "cpu.h"
 
+#include "hw/timer/digic-timer.h"
+
 #define TYPE_DIGIC "digic"
 
 #define DIGIC(obj) OBJECT_CHECK(DigicState, (obj), TYPE_DIGIC)
 
+#define DIGIC4_NB_TIMERS 3
+
 typedef struct DigicState {
     /*< private >*/
     DeviceState parent_obj;
     /*< public >*/
 
     ARMCPU cpu;
+
+    DigicTimerState timer[DIGIC4_NB_TIMERS];
 } DigicState;
 
 #endif /* HW_ARM_DIGIC_H */