On Tue, Dec 3, 2013 at 7:11 PM, liguang <lig.f...@cn.fujitsu.com> wrote:
> Signed-off-by: liguang <lig.f...@cn.fujitsu.com>
> ---
> default-configs/arm-softmmu.mak | 2 +
> hw/timer/Makefile.objs | 2 +
> hw/timer/allwinner-a10_pit.c | 253
> ++++++++++++++++++++++++++++++++++
Mix of _ and - in filename is ugly. I think all - in filename is best.
> include/hw/timer/allwinner-a10_pit.h | 57 ++++++++
> 4 files changed, 314 insertions(+), 0 deletions(-)
> create mode 100644 hw/timer/allwinner-a10_pit.c
> create mode 100644 include/hw/timer/allwinner-a10_pit.h
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index a555eef..0029596 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -81,3 +81,5 @@ CONFIG_VERSATILE_I2C=y
>
> CONFIG_SDHCI=y
> CONFIG_INTEGRATOR_DEBUG=y
> +
> +CONFIG_ALLWINNER_A10=y
> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
> index eca5905..4ff2e1f 100644
> --- a/hw/timer/Makefile.objs
> +++ b/hw/timer/Makefile.objs
> @@ -27,3 +27,5 @@ obj-$(CONFIG_SH4) += sh_timer.o
> obj-$(CONFIG_TUSB6010) += tusb6010.o
>
> obj-$(CONFIG_MC146818RTC) += mc146818rtc.o
> +
> +obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10_pit.o
> diff --git a/hw/timer/allwinner-a10_pit.c b/hw/timer/allwinner-a10_pit.c
> new file mode 100644
> index 0000000..2f9b458
> --- /dev/null
> +++ b/hw/timer/allwinner-a10_pit.c
> @@ -0,0 +1,253 @@
> +/*
> + * Allwinner A10 timer device emulation
> + *
> + * Copyright (C) 2013 Li Guang
> + * Written by Li Guang <lig.f...@cn.fujitsu.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.
> + */
> +
> +#include "hw/sysbus.h"
> +#include "sysemu/sysemu.h"
> +#include "hw/timer/allwinner-a10_pit.h"
> +
> +
> +static uint64_t a10_pit_read(void *opaque, hwaddr offset, unsigned size)
> +{
> + A10PITState *s = A10_PIT(opaque);
> + uint8_t index;
> +
> + switch (offset) {
> + case A10PIT_TIMER_IRQ_EN:
Its odd to have no _ between A10 and PIT. Does that match your documentation?
> + return s->irq_enable;
> + case A10PIT_TIMER_IRQ_ST:
> + return s->irq_status;
> + case A10PIT_TIMER_BASE ... A10PIT_TIMER_BASE * 6 + A10PIT_TIMER_COUNT:
> + index = offset & 0xf0;
> + index >>= 4;
> + index -= 1;
> + switch (offset & 0x0f) {
> + case A10PIT_TIMER_CONTROL:
> + return s->control[index];
> + case A10PIT_TIMER_INTERVAL:
> + return s->interval[index];
> + case A10PIT_TIMER_COUNT:
> + s->count[index] = ptimer_get_count(s->timer[index]);
> + return s->count[index];
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Bad offset 0x%x\n", __func__, (int)offset);
> + break;
> + }
> + case A10PIT_WDOG_CONTROL:
> + break;
> + case A10PIT_WDOG_MODE:
> + break;
> + case A10PIT_COUNT_LO:
> + return s->count_lo;
> + case A10PIT_COUNT_HI:
> + return s->count_hi;
> + case A10PIT_COUNT_CTL:
> + return s->count_ctl;
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Bad offset 0x%x\n", __func__, (int)offset);
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static void a10_pit_write(void *opaque, hwaddr offset, uint64_t value,
> + unsigned size)
> +{
> + A10PITState *s = A10_PIT(opaque);
> + uint8_t index;
> +
> + switch (offset) {
> + case A10PIT_TIMER_IRQ_EN:
> + s->irq_enable = value;
> + break;
> + case A10PIT_TIMER_IRQ_ST:
> + s->irq_status &= ~value;
> + break;
> + case A10PIT_TIMER_BASE ... A10PIT_TIMER_BASE * 6 + A10PIT_TIMER_COUNT:
> + index = offset & 0xf0;
> + index >>= 4;
> + index -= 1;
> + switch (offset & 0x0f) {
> + case A10PIT_TIMER_CONTROL:
> + s->control[index] = value;
> + if (s->control[index] & A10PIT_TIMER_RELOAD) {
> + ptimer_set_count(s->timer[index], s->interval[index]);
> + }
Why? Doesn't the count just always stay up to date when writing TIMER_INTERVAL?
> + if (s->control[index] & A10PIT_TIMER_EN) {
> + ptimer_run(s->timer[index], 1);
> + } else {
> + ptimer_stop(s->timer[index]);
> + }
> + break;
> + case A10PIT_TIMER_INTERVAL:
> + s->interval[index] = value;
> + ptimer_set_count(s->timer[index], s->interval[index]);
> + break;
> + case A10PIT_TIMER_COUNT:
> + s->count[index] = value;
> + break;
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Bad offset 0x%x\n", __func__, (int)offset);
> + }
> + break;
> + case A10PIT_WDOG_CONTROL:
> + s->watch_dog_control = value;
> + break;
> + case A10PIT_WDOG_MODE:
> + s->watch_dog_mode = value;
> + break;
> + case A10PIT_COUNT_LO:
> + s->count_lo = value;
> + break;
> + case A10PIT_COUNT_HI:
> + s->count_hi = value;
> + break;
> + case A10PIT_COUNT_CTL:
> + s->count_ctl = value;
> + if (s->count_ctl & A10PIT_COUNT_RL_EN) {
> + s->count_lo = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> + s->count_hi = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) >> 32;
> + s->count_ctl &= ~A10PIT_COUNT_RL_EN;
> + }
> + if (s->count_ctl & A10PIT_COUNT_CLR_EN) {
> + s->count_lo = 0;
> + s->count_hi = 0;
> + s->count_ctl &= ~A10PIT_COUNT_CLR_EN;
> + }
> + break;
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Bad offset 0x%x\n", __func__, (int)offset);
> + break;
> + }
> +}
> +
> +static const MemoryRegionOps a10_pit_ops = {
> + .read = a10_pit_read,
> + .write = a10_pit_write,
> + .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static const VMStateDescription vmstate_a10_pit = {
> + .name = "a10.pit",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .minimum_version_id_old = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32(irq_enable, A10PITState),
> + VMSTATE_UINT32(irq_status, A10PITState),
> + VMSTATE_UINT32_ARRAY(control, A10PITState, A10PIT_TIMER_NR),
> + VMSTATE_UINT32_ARRAY(interval, A10PITState, A10PIT_TIMER_NR),
> + VMSTATE_UINT32_ARRAY(count, A10PITState, A10PIT_TIMER_NR),
> + VMSTATE_UINT32(watch_dog_mode, A10PITState),
> + VMSTATE_UINT32(watch_dog_control, A10PITState),
> + VMSTATE_UINT32(count_lo, A10PITState),
> + VMSTATE_UINT32(count_hi, A10PITState),
> + VMSTATE_UINT32(count_ctl, A10PITState),
> + VMSTATE_PTIMER_ARRAY(timer, A10PITState, A10PIT_TIMER_NR),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static void a10_pit_reset(DeviceState *dev)
> +{
> + A10PITState *s = A10_PIT(dev);
> + uint8_t i;
> +
> + s->irq_enable = 0;
> + s->irq_status = 0;
> + for (i = 0; i < 6; i++) {
> + s->control[i] = A10PIT_DEFAULT_CLOCK;
> + s->interval[i] = 0;
> + s->count[i] = 0;
> + ptimer_stop(s->timer[i]);
> + }
> + s->watch_dog_mode = 0;
> + s->watch_dog_control = 0;
> + s->count_lo = 0;
> + s->count_hi = 0;
> + s->count_ctl = 0;
> +}
> +
> +static void a10_pit_timer_cb(void *opaque)
> +{
> + A10PITState *s = A10_PIT(opaque);
> + uint8_t i;
> +
> + for (i = 0; i < A10PIT_TIMER_NR; i++) {
> + if (s->control[i] & A10PIT_TIMER_EN &&
> + ptimer_get_count(s->timer[i]) == 0) {
> + s->irq_status |= 1 << i;
> + if (!(s->control[i] & A10PIT_TIMER_MODE)) {
> + ptimer_set_count(s->timer[i], s->interval[i]);
> + ptimer_run(s->timer[i], 1);
You are still chaining oneshot timers together here to form a periodic
timer. Just run the timer as periodic in the first place when
!A10PIT_TIMER_MODE.
> + s->control[i] |= A10PIT_TIMER_EN;
Dead assignment. TIMER_EN is already set.
> + } else {
> + s->control[i] &= ~A10PIT_TIMER_EN;
So the spurious interrupts are gone now, which is good news
> + }
> + qemu_irq_pulse(s->irq[i]);
> + }
> + }
> +}
> +
> +static void a10_pit_realize(DeviceState *dev, Error **errp)
> +{
> + A10PITState *s = A10_PIT(dev);
> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> + QEMUBH * bh[A10PIT_TIMER_NR];
> + uint8_t i;
> +
> + for (i = 0; i < A10PIT_TIMER_NR; i++) {
> + sysbus_init_irq(sbd, &s->irq[i]);
> + }
> + memory_region_init_io(&s->iomem, OBJECT(s), &a10_pit_ops, s,
> + TYPE_A10_PIT, 0x400);
> + sysbus_init_mmio(sbd, &s->iomem);
> +
> + for (i = 0; i < A10PIT_TIMER_NR; i++) {
> + bh[i] = qemu_bh_new(a10_pit_timer_cb, s);
> + s->timer[i] = ptimer_init(bh[i]);
> + ptimer_set_freq(s->timer[i], 240000);
> + }
> +}
> +
> +static void a10_pit_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->realize = a10_pit_realize;
> + dc->reset = a10_pit_reset;
> + dc->desc = "allwinner a10 timer";
> + dc->vmsd = &vmstate_a10_pit;
> +}
> +
> +static const TypeInfo a10_pit_info = {
> + .name = TYPE_A10_PIT,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(A10PITState),
> + .class_init = a10_pit_class_init,
> +};
> +
> +static void a10_register_types(void)
> +{
> + type_register_static(&a10_pit_info);
> +}
> +
> +type_init(a10_register_types);
> diff --git a/include/hw/timer/allwinner-a10_pit.h
> b/include/hw/timer/allwinner-a10_pit.h
> new file mode 100644
> index 0000000..e061cc2
> --- /dev/null
> +++ b/include/hw/timer/allwinner-a10_pit.h
> @@ -0,0 +1,57 @@
> +#ifndef A10_PIT_H
> +#define A10_PIT_H
> +
> +#include "hw/ptimer.h"
> +
> +#define TYPE_A10_PIT "a10-timer"
> +#define A10_PIT(obj) OBJECT_CHECK(A10PITState, (obj), TYPE_A10_PIT)
> +
> +#define A10PIT_TIMER_NR 6
> +#define A10PIT_TIMER_IRQ 0x1
> +#define A10PIT_WDOG_IRQ 0x100
> +
So "A10" as the macro prefix is a little vague. We could easily have
confusion with ARMs own usage of Axx. Is there a contraction of
"allwinner" we could prefix in there?
If you plan to use this timer in the future for devices other than
A10, you could just drop the "A10" labelling here now. That suggestion
to use the specific SoC name is more for the SoC container object, not
the peripheral devices.
Regards,
Peter
> +#define A10PIT_TIMER_IRQ_EN 0
> +#define A10PIT_TIMER_IRQ_ST 0x4
> +
> +#define A10PIT_TIMER_CONTROL 0x0
> +#define A10PIT_TIMER_EN 0x1
> +#define A10PIT_TIMER_RELOAD 0x2
> +#define A10PIT_TIMER_MODE 0x80
> +
> +#define A10PIT_TIMER_INTERVAL 0x4
> +#define A10PIT_TIMER_COUNT 0x8
> +#define A10PIT_WDOG_CONTROL 0x90
> +#define A10PIT_WDOG_MODE 0x94
> +
> +#define A10PIT_COUNT_CTL 0xa0
> +#define A10PIT_COUNT_RL_EN 0x2
> +#define A10PIT_COUNT_CLR_EN 0x1
> +#define A10PIT_COUNT_LO 0xa4
> +#define A10PIT_COUNT_HI 0xa8
> +
> +#define A10PIT_TIMER_BASE 0x10
> +
> +#define A10PIT_DEFAULT_CLOCK 0x4
> +
> +typedef struct A10PITState {
> + /*< private >*/
> + SysBusDevice parent_obj;
> + /*< public >*/
> + qemu_irq irq[A10PIT_TIMER_NR];
> + ptimer_state *timer[A10PIT_TIMER_NR];
> + MemoryRegion iomem;
> +
> + uint32_t irq_enable;
> + uint32_t irq_status;
> + uint32_t control[A10PIT_TIMER_NR];
> + uint32_t interval[A10PIT_TIMER_NR];
> + uint32_t count[A10PIT_TIMER_NR];
> + uint32_t watch_dog_mode;
> + uint32_t watch_dog_control;
> + uint32_t count_lo;
> + uint32_t count_hi;
> + uint32_t count_ctl;
> +} A10PITState;
> +
> +#endif
> +
> --
> 1.7.2.5
>
>
