[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v8 3/6] hw/arm/digic: add timer support
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-devel] [PATCH v8 3/6] hw/arm/digic: add timer support |
Date: |
Sat, 14 Dec 2013 16:41:44 +1000 |
On Sat, Dec 14, 2013 at 7:42 AM, Antony Pavlov <address@hidden> wrote:
> Signed-off-by: Antony Pavlov <address@hidden>
> ---
> hw/arm/digic.c | 28 +++++++++
> hw/timer/Makefile.objs | 1 +
> hw/timer/digic-timer.c | 168
> +++++++++++++++++++++++++++++++++++++++++++++++++
> hw/timer/digic-timer.h | 38 +++++++++++
> include/hw/arm/digic.h | 6 ++
> 5 files changed, 241 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..14cd352
> --- /dev/null
> +++ b/hw/timer/digic-timer.c
> @@ -0,0 +1,168 @@
> +/*
> + * QEMU model of the Canon DIGIC timer block.
> + *
> + * Copyright (C) 2013 Antony Pavlov <address@hidden>
> + *
> + * 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_CONTROL_RST 0x80000000
> +#define DIGIC_TIMER_CONTROL_EN 0x00000001
> +#define DIGIC_TIMER_RELVALUE 0x08
> +#define DIGIC_TIMER_VALUE 0x0c
> +
Programmer model #defines should go in the timer/digic-timer.h header
to allow for easy generation of test stimulus - other code can
exercise your device using your already written full macro suite.
> +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_UINT32(control, DigicTimerState),
> + VMSTATE_UINT32(relvalue, DigicTimerState),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static void digic_timer_reset(DeviceState *dev)
> +{
> + DigicTimerState *s = DIGIC_TIMER(dev);
> +
> + ptimer_stop(s->ptimer);
> + s->control = 0;
> + s->relvalue = 0;
> +}
> +
> +static uint64_t digic_timer_read(void *opaque, hwaddr offset, unsigned size)
> +{
> + DigicTimerState *s = opaque;
> + uint64_t ret = 0;
> +
> + switch (offset) {
> + case DIGIC_TIMER_CONTROL:
> + ret = s->control;
> + break;
> + case DIGIC_TIMER_RELVALUE:
> + ret = s->relvalue;
> + break;
> + case DIGIC_TIMER_VALUE:
> + ret = ptimer_get_count(s->ptimer) & 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;
> +
> + switch (offset) {
> + case DIGIC_TIMER_CONTROL:
> + if (value & DIGIC_TIMER_CONTROL_RST) {
> + digic_timer_reset((DeviceState *)s);
> + break;
> + }
> +
> + if (value & DIGIC_TIMER_CONTROL_EN) {
> + ptimer_run(s->ptimer, 0);
> + }
So is there definately no stop function on de-assertion of this bit? I
couldnt find it in your documentation but it seem quite natural to me
that,
} else {
ptimer_stop(s->ptimer);
}
Would probably be the implementation of an enable bit, rather than the
only way to stop the timer being soft reset. I wont block on it
however.
> +
> + s->control = (uint32_t)value;
> + break;
> +
> + case DIGIC_TIMER_RELVALUE:
> + s->relvalue = (uint32_t)(value & 0xffff);
extract32 is preferable over & >> logic.
> + ptimer_set_limit(s->ptimer, s->relvalue, 1);
> + break;
> +
> + case DIGIC_TIMER_VALUE:
> + break;
> +
> + default:
> + qemu_log_mask(LOG_UNIMP,
> + "digic-timer: read access to unknown register 0x"
> + TARGET_FMT_plx, offset);
> + }
> +}
> +
> +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_init(Object *obj)
> +{
> + DigicTimerState *s = DIGIC_TIMER(obj);
> +
> + s->ptimer = ptimer_init(NULL);
> +
> + /*
> + * 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_class_init(ObjectClass *klass, void *class_data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
Add a blank line here.
> + 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..a1e526e
> --- /dev/null
> +++ b/hw/timer/digic-timer.h
> @@ -0,0 +1,38 @@
> +/*
> + * Canon DIGIC timer block declarations.
> + *
> + * Copyright (C) 2013 Antony Pavlov <address@hidden>
> + *
> + * 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 {
/*< private >*/
> + SysBusDevice parent_obj;
/*< public >*/
This is all trivial stuff. So if you make the changes (with the
stop-on-no-en being optional),
Reviewed-by: Peter Crosthwaite <address@hidden>
> +
> + MemoryRegion iomem;
> + ptimer_state *ptimer;
> +
> + uint32_t control;
> + uint32_t relvalue;
> +} 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
>
>
- [Qemu-devel] [PATCH v8 0/6] hw/arm: add initial support for Canon DIGIC SoC, Antony Pavlov, 2013/12/13
- [Qemu-devel] [PATCH v8 1/6] hw/arm: add very initial support for Canon DIGIC SoC, Antony Pavlov, 2013/12/13
- [Qemu-devel] [PATCH v8 2/6] hw/arm/digic: prepare DIGIC-based boards support, Antony Pavlov, 2013/12/13
- [Qemu-devel] [PATCH v8 3/6] hw/arm/digic: add timer support, Antony Pavlov, 2013/12/13
- Re: [Qemu-devel] [PATCH v8 3/6] hw/arm/digic: add timer support,
Peter Crosthwaite <=
- [Qemu-devel] [PATCH v8 4/6] hw/arm/digic: add UART support, Antony Pavlov, 2013/12/13
- [Qemu-devel] [PATCH v8 6/6] MAINTAINERS: Document 'Canon DIGIC' machine, Antony Pavlov, 2013/12/13
- [Qemu-devel] [PATCH v8 5/6] hw/arm/digic: add NOR ROM support, Antony Pavlov, 2013/12/13