On Wed, Nov 27, 2013 at 11:35 PM, Peter Crosthwaite <peter.crosthwa...@xilinx.com> wrote: > On Wed, Nov 27, 2013 at 6:29 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 | 1 + >> hw/timer/sunxi-pit.c | 254 >> +++++++++++++++++++++++++++++++++++++++ >> include/hw/timer/sunxi-pit.h | 56 +++++++++ >> 4 files changed, 313 insertions(+), 0 deletions(-) >> create mode 100644 hw/timer/sunxi-pit.c >> create mode 100644 include/hw/timer/sunxi-pit.h >> >> diff --git a/default-configs/arm-softmmu.mak >> b/default-configs/arm-softmmu.mak >> index a555eef..7bf5ad0 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_SUNXI_PIT=y >> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs >> index eca5905..f7888e9 100644 >> --- a/hw/timer/Makefile.objs >> +++ b/hw/timer/Makefile.objs >> @@ -27,3 +27,4 @@ obj-$(CONFIG_SH4) += sh_timer.o >> obj-$(CONFIG_TUSB6010) += tusb6010.o >> >> obj-$(CONFIG_MC146818RTC) += mc146818rtc.o >> +obj-$(CONFIG_SUNXI_PIT) += sunxi-pit.o >> diff --git a/hw/timer/sunxi-pit.c b/hw/timer/sunxi-pit.c >> new file mode 100644 >> index 0000000..19bc16c >> --- /dev/null >> +++ b/hw/timer/sunxi-pit.c >> @@ -0,0 +1,254 @@ >> +/* >> + * Allwinner sunxi timer device emulation > > May need to do global find/replace on sunxi, depending on outcoming of > the naming discussion. > >> + * >> + * 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 "hw/ptimer.h" >> +#include "sysemu/sysemu.h" >> +#include "hw/timer/sunxi-pit.h" >> + >> + >> +static uint64_t sunxi_pit_read(void *opaque, hwaddr offset, unsigned size) >> +{ >> + SunxiPITState *s = SUNXI_PIT(opaque); >> + uint8_t index; >> + >> + switch (offset) { >> + case SUNXI_TIMER_IRQ_EN: >> + return s->irq_enable; >> + case SUNXI_TIMER_IRQ_ST: >> + return s->irq_status; >> + case SUNXI_TIMER_BASE ... SUNXI_TIMER_BASE * 6 + SUNXI_TIMER_COUNT: >> + index = offset & 0xf0; >> + index >>= 4; >> + index -= 1; >> + switch (offset & 0x0f) { >> + case SUNXI_TIMER_CONTROL: >> + return s->control[index]; >> + case SUNXI_TIMER_INTERVAL: >> + return s->interval[index]; >> + case SUNXI_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 SUNXI_WDOG_CONTROL: >> + break; >> + case SUNXI_WDOG_MODE: >> + break; >> + case SUNXI_COUNT_LO: >> + return s->count_lo; >> + break; > > break after return. > > git diff | grep "return" -A 1 | grep "break" > > It's crude, but will find any you missed. > >> + case SUNXI_COUNT_HI: >> + return s->count_hi; >> + break; >> + case SUNXI_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 sunxi_pit_write(void *opaque, hwaddr offset, uint64_t value, >> + unsigned size) >> +{ >> + SunxiPITState *s = SUNXI_PIT(opaque); >> + uint8_t index; >> + >> + switch (offset) { >> + case SUNXI_TIMER_IRQ_EN: >> + s->irq_enable = value; >> + break; >> + case SUNXI_TIMER_IRQ_ST: >> + s->irq_status &= ~value; >> + break; >> + case SUNXI_TIMER_BASE ... SUNXI_TIMER_BASE * 6 + SUNXI_TIMER_COUNT: >> + index = offset & 0xf0; >> + index >>= 4; >> + index -= 1; >> + switch (offset & 0x0f) { >> + case SUNXI_TIMER_CONTROL: >> + s->control[index] = value; >> + if (s->control[index] & SUNXI_TIMER_RELOAD) { >> + ptimer_set_count(s->timer[index], s->interval[index]); >> + } >> + if (s->control[index] & SUNXI_TIMER_EN) { >> + ptimer_run(s->timer[index], 1); >> + } else { >> + ptimer_stop(s->timer[index]); >> + } >> + break; >> + case SUNXI_TIMER_INTERVAL: >> + s->interval[index] = value; >> + ptimer_set_count(s->timer[index], s->interval[index]); >> + break; >> + case SUNXI_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 SUNXI_WDOG_CONTROL: >> + s->watch_dog_control = value; >> + break; >> + case SUNXI_WDOG_MODE: >> + s->watch_dog_mode = value; >> + break; >> + case SUNXI_COUNT_LO: >> + s->count_lo = value; >> + break; >> + case SUNXI_COUNT_HI: >> + s->count_hi = value; >> + break; >> + case SUNXI_COUNT_CTL: >> + s->count_ctl = value; >> + if (s->count_ctl & SUNXI_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 &= ~SUNXI_COUNT_RL_EN; >> + } >> + if (s->count_ctl & SUNXI_COUNT_CLR_EN) { >> + s->count_lo =0; >> + s->count_hi =0; >> + s->count_ctl &= ~SUNXI_COUNT_CLR_EN; >> + } >> + break; >> + default: >> + qemu_log_mask(LOG_GUEST_ERROR, >> + "%s: Bad offset 0x%x\n", __func__, (int)offset); >> + break; >> + } >> +} >> + >> +static const MemoryRegionOps sunxi_pit_ops = { >> + .read = sunxi_pit_read, >> + .write = sunxi_pit_write, >> + .endianness = DEVICE_NATIVE_ENDIAN, >> +}; >> + >> +static const VMStateDescription vmstate_sunxi_pit = { >> + .name = "sunxi.pit", >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .minimum_version_id_old = 1, >> + .fields = (VMStateField[]) { >> + VMSTATE_UINT32(irq_enable, SunxiPITState), >> + VMSTATE_UINT32(irq_status, SunxiPITState), >> + VMSTATE_UINT32_ARRAY(control, SunxiPITState, SUNXI_TIMER_NR), >> + VMSTATE_UINT32_ARRAY(interval, SunxiPITState, SUNXI_TIMER_NR), >> + VMSTATE_UINT32_ARRAY(count, SunxiPITState, SUNXI_TIMER_NR), >> + VMSTATE_UINT32(watch_dog_mode, SunxiPITState), >> + VMSTATE_UINT32(watch_dog_control, SunxiPITState), >> + VMSTATE_UINT32(count_lo, SunxiPITState), >> + VMSTATE_UINT32(count_hi, SunxiPITState), >> + VMSTATE_UINT32(count_ctl, SunxiPITState), >> + VMSTATE_PTIMER_ARRAY(timer, SunxiPITState, SUNXI_TIMER_NR), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >> +static void sunxi_pit_reset(DeviceState *dev) >> +{ >> + SunxiPITState *s = SUNXI_PIT(dev); >> + uint8_t i; >> + >> + s->irq_enable = 0; >> + s->irq_status = 0; >> + for (i = 0; i < 6; i++) { >> + s->control[i] = SUNXI_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 sunxi_pit_timer_cb(void *opaque) >> +{ >> + SunxiPITState *s = SUNXI_PIT(opaque); >> + uint8_t i; >> + >> + for (i = 0; i < SUNXI_TIMER_NR; i++) { >> + if (s->control[i] & SUNXI_TIMER_EN && >> + ptimer_get_count(s->timer[i]) == 0) { >> + s->irq_status |= 1 << i; >> + if (!(s->control[i] & SUNXI_TIMER_MODE)) { >> + ptimer_set_count(s->timer[i], s->interval[i]); >> + ptimer_run(s->timer[i], 1);
So looking at this more closely, I see you are chaining one-shot ptimers. My understanding is that you shouldn't do this, as it leads to small slips in time, that over many hits integrate to a noticable slippage. You should refactor the intial ptimer_run to not one-shot when !SUNXI_TIMER_MODE. >> + } >> + qemu_irq_raise(s->irq[i]); > > I see that it's an edge triggered interrupt - makes a lot more sense > now. I think there is qemu_irq_pulse for this very purpose. But I also > think you are susceptible to spurious edges here however. If timer A > hits (and doesn't reload), and then timer B hits, I don't see what > stops this logic being reached for timer A again? > This issue is orthogonal and still at large even with one-shot fix. Regards, Peter > Regards, > Peter > >> + } >> + qemu_irq_lower(s->irq[i]); >> + } >> +} >> + >> +static void sunxi_pit_realize(DeviceState *dev, Error **errp) >> +{ >> + SunxiPITState *s = SUNXI_PIT(dev); >> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); >> + QEMUBH *bh[SUNXI_TIMER_NR]; >> + uint8_t i; >> + >> + for (i = 0; i < SUNXI_TIMER_NR; i++) { >> + sysbus_init_irq(sbd, &s->irq[i]); >> + } >> + memory_region_init_io(&s->iomem, OBJECT(s), &sunxi_pit_ops, s, >> + TYPE_SUNXI_PIT, 0x400); >> + sysbus_init_mmio(sbd, &s->iomem); >> + >> + for (i = 0; i < SUNXI_TIMER_NR; i++) { >> + bh[i] = qemu_bh_new(sunxi_pit_timer_cb, s); >> + s->timer[i] = ptimer_init(bh[i]); >> + ptimer_set_freq(s->timer[i], 240000); >> + } >> +} >> + >> +static void sunxi_pit_class_init(ObjectClass *klass, void *data) >> +{ >> + DeviceClass *dc = DEVICE_CLASS(klass); >> + >> + dc->realize = sunxi_pit_realize; >> + dc->reset = sunxi_pit_reset; >> + dc->desc = "sunxi timer"; >> + dc->vmsd = &vmstate_sunxi_pit; >> +} >> + >> +static const TypeInfo sunxi_pit_info = { >> + .name = TYPE_SUNXI_PIT, >> + .parent = TYPE_SYS_BUS_DEVICE, >> + .instance_size = sizeof(SunxiPITState), >> + .class_init = sunxi_pit_class_init, >> +}; >> + >> +static void sunxi_register_types(void) >> +{ >> + type_register_static(&sunxi_pit_info); >> +} >> + >> +type_init(sunxi_register_types); >> diff --git a/include/hw/timer/sunxi-pit.h b/include/hw/timer/sunxi-pit.h >> new file mode 100644 >> index 0000000..7c84946 >> --- /dev/null >> +++ b/include/hw/timer/sunxi-pit.h >> @@ -0,0 +1,56 @@ >> +#ifndef SUNXI_PIT_H >> +#define SUNXI_PIT_H >> + >> + >> +#define TYPE_SUNXI_PIT "sunxi-timer" >> +#define SUNXI_PIT(obj) OBJECT_CHECK(SunxiPITState, (obj), TYPE_SUNXI_PIT) >> + >> +#define SUNXI_TIMER_NR 6 >> +#define SUNXI_TIMER_IRQ 0x1 >> +#define SUNXI_WDOG_IRQ 0x100 >> + >> +#define SUNXI_TIMER_IRQ_EN 0 >> +#define SUNXI_TIMER_IRQ_ST 0x4 >> + >> +#define SUNXI_TIMER_CONTROL 0x0 >> +#define SUNXI_TIMER_EN 0x1 >> +#define SUNXI_TIMER_RELOAD 0x2 >> +#define SUNXI_TIMER_MODE 0x80 >> + >> +#define SUNXI_TIMER_INTERVAL 0x4 >> +#define SUNXI_TIMER_COUNT 0x8 >> +#define SUNXI_WDOG_CONTROL 0x90 >> +#define SUNXI_WDOG_MODE 0x94 >> + >> +#define SUNXI_COUNT_CTL 0xa0 >> +#define SUNXI_COUNT_RL_EN 0x2 >> +#define SUNXI_COUNT_CLR_EN 0x1 >> +#define SUNXI_COUNT_LO 0xa4 >> +#define SUNXI_COUNT_HI 0xa8 >> + >> +#define SUNXI_TIMER_BASE 0x10 >> + >> +#define SUNXI_DEFAULT_CLOCK 0x4 >> + >> +typedef struct SunxiPITState { >> + /*< private >*/ >> + SysBusDevice parent_obj; >> + /*< public >*/ >> + qemu_irq irq[SUNXI_TIMER_NR]; >> + ptimer_state *timer[SUNXI_TIMER_NR]; >> + MemoryRegion iomem; >> + >> + uint32_t irq_enable; >> + uint32_t irq_status; >> + uint32_t control[SUNXI_TIMER_NR]; >> + uint32_t interval[SUNXI_TIMER_NR]; >> + uint32_t count[SUNXI_TIMER_NR]; >> + uint32_t watch_dog_mode; >> + uint32_t watch_dog_control; >> + uint32_t count_lo; >> + uint32_t count_hi; >> + uint32_t count_ctl; >> +} SunxiPITState; >> + >> +#endif >> + >> -- >> 1.7.2.5 >> >>