* [Qemu-devel] [PATCH v4 1/4] hw/timer: add sunxi timer device
2013-11-26 7:22 [Qemu-devel] [PATCH v4 0/4] add sunxi machine type liguang
@ 2013-11-26 7:22 ` liguang
2013-11-26 8:40 ` Peter Crosthwaite
2013-11-26 7:22 ` [Qemu-devel] [PATCH v4 2/4] hw/intc: add sunxi interrupt controller device liguang
` (2 subsequent siblings)
3 siblings, 1 reply; 33+ messages in thread
From: liguang @ 2013-11-26 7:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, liguang
Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
---
default-configs/arm-softmmu.mak | 2 +
hw/timer/Makefile.objs | 1 +
hw/timer/sunxi-pit.c | 295 +++++++++++++++++++++++++++++++++++++++
include/hw/timer/sunxi-pit.h | 37 +++++
4 files changed, 335 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..39b84ab
--- /dev/null
+++ b/hw/timer/sunxi-pit.c
@@ -0,0 +1,295 @@
+/*
+ * Allwinner sunxi timer device emulation
+ *
+ * Copyright (C) 2013 Li Guang
+ * Written by Li Guang <lig.fnst@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"
+
+
+typedef struct SunxiTimer {
+ ptimer_state *timer;
+} SunxiTimer;
+
+typedef struct SunxiPITState {
+ /*< private >*/
+ SysBusDevice parent_obj;
+ /*< public >*/
+ qemu_irq irq[SUNXI_TIMER_NR];
+ SunxiTimer 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;
+
+static uint64_t sunxi_pit_read(void *opaque, hwaddr offset, unsigned size)
+{
+ SunxiPITState *s = SUNXI_PIT(opaque);
+ uint8_t index = 0;
+
+ switch (offset) {
+ case SUNXI_TIMER_IRQ_EN:
+ return s->irq_enable;
+ break;
+ case SUNXI_TIMER_IRQ_ST:
+ return s->irq_status;
+ 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:
+ return s->control[index];
+ break;
+ case SUNXI_TIMER_INTERVAL:
+ return s->interval[index];
+ break;
+ case SUNXI_TIMER_COUNT: {
+ SunxiTimer *t = &s->timer[index];
+ s->count[index] = ptimer_get_count(t->timer);
+ return s->count[index];
+ }
+ default:
+ break;
+ }
+ break;
+ case SUNXI_WDOG_CONTROL:
+ break;
+ case SUNXI_WDOG_MODE:
+ break;
+ case SUNXI_COUNT_LO:
+ return s->count_lo;
+ break;
+ case SUNXI_COUNT_HI:
+ return s->count_hi;
+ break;
+ case SUNXI_COUNT_CTL:
+ return s->count_ctl;
+ default:
+ 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 = 0;
+
+ 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: {
+ SunxiTimer *t = &s->timer[index];
+ s->control[index] = value;
+ if (s->control[index] & SUNXI_TIMER_RELOAD) {
+ ptimer_set_count(t->timer, s->interval[index]);
+ }
+ if (s->control[index] & SUNXI_TIMER_EN) {
+ ptimer_run(t->timer, 1);
+ } else {
+ ptimer_stop(t->timer);
+ }
+ break;
+ }
+ case SUNXI_TIMER_INTERVAL: {
+ SunxiTimer *t = &s->timer[index];
+ s->interval[index] = value;
+ ptimer_set_count(t->timer, s->interval[index]);
+ break;
+ }
+ case SUNXI_TIMER_COUNT:
+ s->count[index] = value;
+ default:
+ break;
+ }
+ 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;
+ }
+ default:
+ break;
+ }
+}
+
+static const MemoryRegionOps sunxi_pit_ops = {
+ .read = sunxi_pit_read,
+ .write = sunxi_pit_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static const VMStateDescription vmstate_sunxi_timer = {
+ .name = "sunxi.timer",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .minimum_version_id_old = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_PTIMER(timer, SunxiTimer),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+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_STRUCT_ARRAY(timer, SunxiPITState, SUNXI_TIMER_NR, 1,
+ vmstate_sunxi_timer, SunxiTimer),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static void sunxi_pit_reset(DeviceState *dev)
+{
+ SunxiPITState *s = SUNXI_PIT(dev);
+ uint8_t i = 0;
+
+ s->irq_enable = 0;
+ s->irq_status = 0;
+ for (i = 0; i < 6; i++) {
+ SunxiTimer *t = &s->timer[i];
+ s->control[i] = 0x4;
+ s->interval[i] = 0;
+ s->count[i] = 0;
+ ptimer_stop(t->timer);
+ }
+ 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 = 0;
+
+ for (i = 0; i < SUNXI_TIMER_NR; i++) {
+ SunxiTimer *t = &s->timer[i];
+ if (s->control[i] & SUNXI_TIMER_EN &&
+ ptimer_get_count(t->timer) == 0) {
+ s->irq_status |= 1 << i;
+ if (!(s->control[i] & SUNXI_TIMER_MODE)) {
+ ptimer_set_count(t->timer, s->interval[i]);
+ ptimer_run(t->timer, 1);
+ }
+ qemu_set_irq(s->irq[i],
+ (s->irq_status & s->irq_enable & 1 << i) != 0);
+ }
+ }
+}
+
+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 = 0;
+
+ 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++) {
+ SunxiTimer *t = &s->timer[i];
+ bh[i] = qemu_bh_new(sunxi_pit_timer_cb, s);
+ t->timer = ptimer_init(bh[i]);
+ ptimer_set_freq(t->timer, 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..260d36f
--- /dev/null
+++ b/include/hw/timer/sunxi-pit.h
@@ -0,0 +1,37 @@
+#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
+
+
+#endif
+
--
1.7.2.5
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 1/4] hw/timer: add sunxi timer device
2013-11-26 7:22 ` [Qemu-devel] [PATCH v4 1/4] hw/timer: add sunxi timer device liguang
@ 2013-11-26 8:40 ` Peter Crosthwaite
2013-11-26 8:59 ` Li Guang
0 siblings, 1 reply; 33+ messages in thread
From: Peter Crosthwaite @ 2013-11-26 8:40 UTC (permalink / raw)
To: liguang; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Tue, Nov 26, 2013 at 5:22 PM, liguang <lig.fnst@cn.fujitsu.com> wrote:
> Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> ---
> default-configs/arm-softmmu.mak | 2 +
> hw/timer/Makefile.objs | 1 +
> hw/timer/sunxi-pit.c | 295 +++++++++++++++++++++++++++++++++++++++
> include/hw/timer/sunxi-pit.h | 37 +++++
> 4 files changed, 335 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..39b84ab
> --- /dev/null
> +++ b/hw/timer/sunxi-pit.c
> @@ -0,0 +1,295 @@
> +/*
> + * Allwinner sunxi timer device emulation
> + *
> + * Copyright (C) 2013 Li Guang
> + * Written by Li Guang <lig.fnst@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"
> +
> +
> +typedef struct SunxiTimer {
> + ptimer_state *timer;
> +} SunxiTimer;
> +
I don't understand the need for this struct. What was wrong with the
direct array of ptimers you had before?
> +typedef struct SunxiPITState {
> + /*< private >*/
> + SysBusDevice parent_obj;
> + /*< public >*/
> + qemu_irq irq[SUNXI_TIMER_NR];
> + SunxiTimer 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;
> +
> +static uint64_t sunxi_pit_read(void *opaque, hwaddr offset, unsigned size)
> +{
> + SunxiPITState *s = SUNXI_PIT(opaque);
> + uint8_t index = 0;
initializer to 0 un-needed.
> +
> + switch (offset) {
> + case SUNXI_TIMER_IRQ_EN:
> + return s->irq_enable;
> + break;
> + case SUNXI_TIMER_IRQ_ST:
> + return s->irq_status;
> + 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:
> + return s->control[index];
> + break;
> + case SUNXI_TIMER_INTERVAL:
> + return s->interval[index];
> + break;
> + case SUNXI_TIMER_COUNT: {
> + SunxiTimer *t = &s->timer[index];
> + s->count[index] = ptimer_get_count(t->timer);
> + return s->count[index];
> + }
> + default:
> + break;
> + }
> + break;
> + case SUNXI_WDOG_CONTROL:
> + break;
> + case SUNXI_WDOG_MODE:
> + break;
> + case SUNXI_COUNT_LO:
> + return s->count_lo;
> + break;
> + case SUNXI_COUNT_HI:
> + return s->count_hi;
> + break;
> + case SUNXI_COUNT_CTL:
> + return s->count_ctl;
> + default:
> + break;
Usual to do a log_guest error here. Same for writes below.
> + }
> +
> + return 0;
> +}
> +
> +static void sunxi_pit_write(void *opaque, hwaddr offset, uint64_t value,
> + unsigned size)
> +{
> + SunxiPITState *s = SUNXI_PIT(opaque);
> + uint8_t index = 0;
> +
> + switch (offset) {
> + case SUNXI_TIMER_IRQ_EN:
> + s->irq_enable = value;
> + break;
> + case SUNXI_TIMER_IRQ_ST:
> + s->irq_status &= value;
Are you missing a ~ ? This is a clear-to-clear semantic rather than
write-to-clear.
Also shouldn't this de-assert the relevant interrupt lines?
> + 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: {
> + SunxiTimer *t = &s->timer[index];
> + s->control[index] = value;
> + if (s->control[index] & SUNXI_TIMER_RELOAD) {
> + ptimer_set_count(t->timer, s->interval[index]);
> + }
> + if (s->control[index] & SUNXI_TIMER_EN) {
> + ptimer_run(t->timer, 1);
> + } else {
> + ptimer_stop(t->timer);
> + }
> + break;
> + }
> + case SUNXI_TIMER_INTERVAL: {
> + SunxiTimer *t = &s->timer[index];
> + s->interval[index] = value;
> + ptimer_set_count(t->timer, s->interval[index]);
> + break;
> + }
> + case SUNXI_TIMER_COUNT:
> + s->count[index] = value;
> + default:
> + break;
> + }
> + 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;
> + }
> + default:
> + break;
> + }
> +}
> +
> +static const MemoryRegionOps sunxi_pit_ops = {
> + .read = sunxi_pit_read,
> + .write = sunxi_pit_write,
> + .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static const VMStateDescription vmstate_sunxi_timer = {
> + .name = "sunxi.timer",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .minimum_version_id_old = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_PTIMER(timer, SunxiTimer),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +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_STRUCT_ARRAY(timer, SunxiPITState, SUNXI_TIMER_NR, 1,
> + vmstate_sunxi_timer, SunxiTimer),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static void sunxi_pit_reset(DeviceState *dev)
> +{
> + SunxiPITState *s = SUNXI_PIT(dev);
> + uint8_t i = 0;
> +
> + s->irq_enable = 0;
> + s->irq_status = 0;
> + for (i = 0; i < 6; i++) {
> + SunxiTimer *t = &s->timer[i];
> + s->control[i] = 0x4;
Magic number.
Regards,
Peter
> + s->interval[i] = 0;
> + s->count[i] = 0;
> + ptimer_stop(t->timer);
> + }
> + 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 = 0;
> +
> + for (i = 0; i < SUNXI_TIMER_NR; i++) {
> + SunxiTimer *t = &s->timer[i];
> + if (s->control[i] & SUNXI_TIMER_EN &&
> + ptimer_get_count(t->timer) == 0) {
> + s->irq_status |= 1 << i;
> + if (!(s->control[i] & SUNXI_TIMER_MODE)) {
> + ptimer_set_count(t->timer, s->interval[i]);
> + ptimer_run(t->timer, 1);
> + }
> + qemu_set_irq(s->irq[i],
> + (s->irq_status & s->irq_enable & 1 << i) != 0);
> + }
> + }
> +}
> +
> +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 = 0;
> +
> + 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++) {
> + SunxiTimer *t = &s->timer[i];
> + bh[i] = qemu_bh_new(sunxi_pit_timer_cb, s);
> + t->timer = ptimer_init(bh[i]);
> + ptimer_set_freq(t->timer, 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..260d36f
> --- /dev/null
> +++ b/include/hw/timer/sunxi-pit.h
> @@ -0,0 +1,37 @@
> +#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
> +
> +
> +#endif
> +
> --
> 1.7.2.5
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 1/4] hw/timer: add sunxi timer device
2013-11-26 8:40 ` Peter Crosthwaite
@ 2013-11-26 8:59 ` Li Guang
2013-11-26 9:14 ` Peter Crosthwaite
0 siblings, 1 reply; 33+ messages in thread
From: Li Guang @ 2013-11-26 8:59 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
Peter Crosthwaite wrote:
> On Tue, Nov 26, 2013 at 5:22 PM, liguang<lig.fnst@cn.fujitsu.com> wrote:
>
>> Signed-off-by: liguang<lig.fnst@cn.fujitsu.com>
>> ---
>> default-configs/arm-softmmu.mak | 2 +
>> hw/timer/Makefile.objs | 1 +
>> hw/timer/sunxi-pit.c | 295 +++++++++++++++++++++++++++++++++++++++
>> include/hw/timer/sunxi-pit.h | 37 +++++
>> 4 files changed, 335 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..39b84ab
>> --- /dev/null
>> +++ b/hw/timer/sunxi-pit.c
>> @@ -0,0 +1,295 @@
>> +/*
>> + * Allwinner sunxi timer device emulation
>> + *
>> + * Copyright (C) 2013 Li Guang
>> + * Written by Li Guang<lig.fnst@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"
>> +
>> +
>> +typedef struct SunxiTimer {
>> + ptimer_state *timer;
>> +} SunxiTimer;
>> +
>>
> I don't understand the need for this struct. What was wrong with the
> direct array of ptimers you had before?
>
because I have to pack timer array into VMSD,
and there's no VMSTATE_PTIMER_ARRAY for ptimer_state.
>
>> +typedef struct SunxiPITState {
>> + /*< private>*/
>> + SysBusDevice parent_obj;
>> + /*< public>*/
>> + qemu_irq irq[SUNXI_TIMER_NR];
>> + SunxiTimer 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;
>> +
>> +static uint64_t sunxi_pit_read(void *opaque, hwaddr offset, unsigned size)
>> +{
>> + SunxiPITState *s = SUNXI_PIT(opaque);
>> + uint8_t index = 0;
>>
> initializer to 0 un-needed.
>
>
OK.
>> +
>> + switch (offset) {
>> + case SUNXI_TIMER_IRQ_EN:
>> + return s->irq_enable;
>> + break;
>> + case SUNXI_TIMER_IRQ_ST:
>> + return s->irq_status;
>> + 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:
>> + return s->control[index];
>> + break;
>> + case SUNXI_TIMER_INTERVAL:
>> + return s->interval[index];
>> + break;
>> + case SUNXI_TIMER_COUNT: {
>> + SunxiTimer *t =&s->timer[index];
>> + s->count[index] = ptimer_get_count(t->timer);
>> + return s->count[index];
>> + }
>> + default:
>> + break;
>> + }
>> + break;
>> + case SUNXI_WDOG_CONTROL:
>> + break;
>> + case SUNXI_WDOG_MODE:
>> + break;
>> + case SUNXI_COUNT_LO:
>> + return s->count_lo;
>> + break;
>> + case SUNXI_COUNT_HI:
>> + return s->count_hi;
>> + break;
>> + case SUNXI_COUNT_CTL:
>> + return s->count_ctl;
>> + default:
>> + break;
>>
> Usual to do a log_guest error here. Same for writes below.
>
OK.
>
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void sunxi_pit_write(void *opaque, hwaddr offset, uint64_t value,
>> + unsigned size)
>> +{
>> + SunxiPITState *s = SUNXI_PIT(opaque);
>> + uint8_t index = 0;
>> +
>> + switch (offset) {
>> + case SUNXI_TIMER_IRQ_EN:
>> + s->irq_enable = value;
>> + break;
>> + case SUNXI_TIMER_IRQ_ST:
>> + s->irq_status&= value;
>>
> Are you missing a ~ ? This is a clear-to-clear semantic rather than
> write-to-clear.
>
yes
> Also shouldn't this de-assert the relevant interrupt lines?
>
>
there's no related tips in sunxi SoC data-sheet,
and test is fine until now.
>> + 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: {
>> + SunxiTimer *t =&s->timer[index];
>> + s->control[index] = value;
>> + if (s->control[index]& SUNXI_TIMER_RELOAD) {
>> + ptimer_set_count(t->timer, s->interval[index]);
>> + }
>> + if (s->control[index]& SUNXI_TIMER_EN) {
>> + ptimer_run(t->timer, 1);
>> + } else {
>> + ptimer_stop(t->timer);
>> + }
>> + break;
>> + }
>> + case SUNXI_TIMER_INTERVAL: {
>> + SunxiTimer *t =&s->timer[index];
>> + s->interval[index] = value;
>> + ptimer_set_count(t->timer, s->interval[index]);
>> + break;
>> + }
>> + case SUNXI_TIMER_COUNT:
>> + s->count[index] = value;
>> + default:
>> + break;
>> + }
>> + 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;
>> + }
>> + default:
>> + break;
>> + }
>> +}
>> +
>> +static const MemoryRegionOps sunxi_pit_ops = {
>> + .read = sunxi_pit_read,
>> + .write = sunxi_pit_write,
>> + .endianness = DEVICE_NATIVE_ENDIAN,
>> +};
>> +
>> +static const VMStateDescription vmstate_sunxi_timer = {
>> + .name = "sunxi.timer",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .minimum_version_id_old = 1,
>> + .fields = (VMStateField[]) {
>> + VMSTATE_PTIMER(timer, SunxiTimer),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> +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_STRUCT_ARRAY(timer, SunxiPITState, SUNXI_TIMER_NR, 1,
>> + vmstate_sunxi_timer, SunxiTimer),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> +static void sunxi_pit_reset(DeviceState *dev)
>> +{
>> + SunxiPITState *s = SUNXI_PIT(dev);
>> + uint8_t i = 0;
>> +
>> + s->irq_enable = 0;
>> + s->irq_status = 0;
>> + for (i = 0; i< 6; i++) {
>> + SunxiTimer *t =&s->timer[i];
>> + s->control[i] = 0x4;
>>
> Magic number.
>
>
OK, should be SUNXI_DEFAULT_CLOCK,
Thanks!
>> + s->interval[i] = 0;
>> + s->count[i] = 0;
>> + ptimer_stop(t->timer);
>> + }
>> + 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 = 0;
>> +
>> + for (i = 0; i< SUNXI_TIMER_NR; i++) {
>> + SunxiTimer *t =&s->timer[i];
>> + if (s->control[i]& SUNXI_TIMER_EN&&
>> + ptimer_get_count(t->timer) == 0) {
>> + s->irq_status |= 1<< i;
>> + if (!(s->control[i]& SUNXI_TIMER_MODE)) {
>> + ptimer_set_count(t->timer, s->interval[i]);
>> + ptimer_run(t->timer, 1);
>> + }
>> + qemu_set_irq(s->irq[i],
>> + (s->irq_status& s->irq_enable& 1<< i) != 0);
>> + }
>> + }
>> +}
>> +
>> +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 = 0;
>> +
>> + 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++) {
>> + SunxiTimer *t =&s->timer[i];
>> + bh[i] = qemu_bh_new(sunxi_pit_timer_cb, s);
>> + t->timer = ptimer_init(bh[i]);
>> + ptimer_set_freq(t->timer, 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..260d36f
>> --- /dev/null
>> +++ b/include/hw/timer/sunxi-pit.h
>> @@ -0,0 +1,37 @@
>> +#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
>> +
>> +
>> +#endif
>> +
>> --
>> 1.7.2.5
>>
>>
>>
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 1/4] hw/timer: add sunxi timer device
2013-11-26 8:59 ` Li Guang
@ 2013-11-26 9:14 ` Peter Crosthwaite
2013-11-26 9:19 ` Li Guang
0 siblings, 1 reply; 33+ messages in thread
From: Peter Crosthwaite @ 2013-11-26 9:14 UTC (permalink / raw)
To: Li Guang; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Tue, Nov 26, 2013 at 6:59 PM, Li Guang <lig.fnst@cn.fujitsu.com> wrote:
> Peter Crosthwaite wrote:
>>
>> On Tue, Nov 26, 2013 at 5:22 PM, liguang<lig.fnst@cn.fujitsu.com> wrote:
>>
>>>
>>> Signed-off-by: liguang<lig.fnst@cn.fujitsu.com>
>>> ---
>>> default-configs/arm-softmmu.mak | 2 +
>>> hw/timer/Makefile.objs | 1 +
>>> hw/timer/sunxi-pit.c | 295
>>> +++++++++++++++++++++++++++++++++++++++
>>> include/hw/timer/sunxi-pit.h | 37 +++++
>>> 4 files changed, 335 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..39b84ab
>>> --- /dev/null
>>> +++ b/hw/timer/sunxi-pit.c
>>> @@ -0,0 +1,295 @@
>>> +/*
>>> + * Allwinner sunxi timer device emulation
>>> + *
>>> + * Copyright (C) 2013 Li Guang
>>> + * Written by Li Guang<lig.fnst@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"
>>> +
>>> +
>>> +typedef struct SunxiTimer {
>>> + ptimer_state *timer;
>>> +} SunxiTimer;
>>> +
>>>
>>
>> I don't understand the need for this struct. What was wrong with the
>> direct array of ptimers you had before?
>>
>
>
> because I have to pack timer array into VMSD,
> and there's no VMSTATE_PTIMER_ARRAY for ptimer_state.
>
Anyway you can just use VMSTATE_STRUCT_ARRAY on ptimers own VMSD definition?
If you cant, then I think you make a reasonable case for moving the
relevant bits and pieces to headers so they are visible. That or
implement VMSTATE_PTIMER_ARRAY.
>
>>
>>>
>>> +typedef struct SunxiPITState {
>>> + /*< private>*/
>>> + SysBusDevice parent_obj;
>>> + /*< public>*/
>>> + qemu_irq irq[SUNXI_TIMER_NR];
>>> + SunxiTimer 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;
>>> +
>>> +static uint64_t sunxi_pit_read(void *opaque, hwaddr offset, unsigned
>>> size)
>>> +{
>>> + SunxiPITState *s = SUNXI_PIT(opaque);
>>> + uint8_t index = 0;
>>>
>>
>> initializer to 0 un-needed.
>>
>>
>
>
> OK.
>>>
>>> +
>>> + switch (offset) {
>>> + case SUNXI_TIMER_IRQ_EN:
>>> + return s->irq_enable;
>>> + break;
>>> + case SUNXI_TIMER_IRQ_ST:
>>> + return s->irq_status;
>>> + 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:
>>> + return s->control[index];
>>> + break;
>>> + case SUNXI_TIMER_INTERVAL:
>>> + return s->interval[index];
>>> + break;
>>> + case SUNXI_TIMER_COUNT: {
>>> + SunxiTimer *t =&s->timer[index];
>>>
>>> + s->count[index] = ptimer_get_count(t->timer);
>>> + return s->count[index];
>>> + }
>>> + default:
>>> + break;
>>> + }
>>> + break;
>>> + case SUNXI_WDOG_CONTROL:
>>> + break;
>>> + case SUNXI_WDOG_MODE:
>>> + break;
>>> + case SUNXI_COUNT_LO:
>>> + return s->count_lo;
>>> + break;
>>> + case SUNXI_COUNT_HI:
>>> + return s->count_hi;
>>> + break;
>>> + case SUNXI_COUNT_CTL:
>>> + return s->count_ctl;
>>> + default:
>>> + break;
>>>
>>
>> Usual to do a log_guest error here. Same for writes below.
>>
>
>
> OK.
>>
>>
>>>
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void sunxi_pit_write(void *opaque, hwaddr offset, uint64_t value,
>>> + unsigned size)
>>> +{
>>> + SunxiPITState *s = SUNXI_PIT(opaque);
>>> + uint8_t index = 0;
>>> +
>>> + switch (offset) {
>>> + case SUNXI_TIMER_IRQ_EN:
>>> + s->irq_enable = value;
>>> + break;
>>> + case SUNXI_TIMER_IRQ_ST:
>>> + s->irq_status&= value;
>>>
>>
>> Are you missing a ~ ? This is a clear-to-clear semantic rather than
>> write-to-clear.
>>
>
>
> yes
>
>> Also shouldn't this de-assert the relevant interrupt lines?
>>
>>
>
>
> there's no related tips in sunxi SoC data-sheet,
> and test is fine until now.
>
But AFAICT, there is no way for software to lower an interrupt in an
immediate timeframe, which is very un-usual. Usually the WTC of an ISR
bring down the IRQ pin. There is only one call to qemu_set_irq and
thats in the timer callback so the only way to lower interrupts is for
the timer to hit again with the IRQ masked.
Regards,
Peter
>>> + 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: {
>>> + SunxiTimer *t =&s->timer[index];
>>>
>>> + s->control[index] = value;
>>> + if (s->control[index]& SUNXI_TIMER_RELOAD) {
>>> + ptimer_set_count(t->timer, s->interval[index]);
>>> + }
>>> + if (s->control[index]& SUNXI_TIMER_EN) {
>>> + ptimer_run(t->timer, 1);
>>> + } else {
>>> + ptimer_stop(t->timer);
>>> + }
>>> + break;
>>> + }
>>> + case SUNXI_TIMER_INTERVAL: {
>>> + SunxiTimer *t =&s->timer[index];
>>>
>>> + s->interval[index] = value;
>>> + ptimer_set_count(t->timer, s->interval[index]);
>>> + break;
>>> + }
>>> + case SUNXI_TIMER_COUNT:
>>> + s->count[index] = value;
>>> + default:
>>> + break;
>>> + }
>>> + 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;
>>>
>>> + }
>>> + default:
>>> + break;
>>> + }
>>> +}
>>> +
>>> +static const MemoryRegionOps sunxi_pit_ops = {
>>> + .read = sunxi_pit_read,
>>> + .write = sunxi_pit_write,
>>> + .endianness = DEVICE_NATIVE_ENDIAN,
>>> +};
>>> +
>>> +static const VMStateDescription vmstate_sunxi_timer = {
>>> + .name = "sunxi.timer",
>>> + .version_id = 1,
>>> + .minimum_version_id = 1,
>>> + .minimum_version_id_old = 1,
>>> + .fields = (VMStateField[]) {
>>> + VMSTATE_PTIMER(timer, SunxiTimer),
>>> + VMSTATE_END_OF_LIST()
>>> + }
>>> +};
>>> +
>>> +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_STRUCT_ARRAY(timer, SunxiPITState, SUNXI_TIMER_NR, 1,
>>> + vmstate_sunxi_timer, SunxiTimer),
>>> + VMSTATE_END_OF_LIST()
>>> + }
>>> +};
>>> +
>>> +static void sunxi_pit_reset(DeviceState *dev)
>>> +{
>>> + SunxiPITState *s = SUNXI_PIT(dev);
>>> + uint8_t i = 0;
>>> +
>>> + s->irq_enable = 0;
>>> + s->irq_status = 0;
>>> + for (i = 0; i< 6; i++) {
>>> + SunxiTimer *t =&s->timer[i];
>>>
>>> + s->control[i] = 0x4;
>>>
>>
>> Magic number.
>>
>>
>
> OK, should be SUNXI_DEFAULT_CLOCK,
>
> Thanks!
>
>>> + s->interval[i] = 0;
>>> + s->count[i] = 0;
>>> + ptimer_stop(t->timer);
>>> + }
>>> + 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 = 0;
>>> +
>>> + for (i = 0; i< SUNXI_TIMER_NR; i++) {
>>> + SunxiTimer *t =&s->timer[i];
>>>
>>> + if (s->control[i]& SUNXI_TIMER_EN&&
>>> + ptimer_get_count(t->timer) == 0) {
>>> + s->irq_status |= 1<< i;
>>> + if (!(s->control[i]& SUNXI_TIMER_MODE)) {
>>> + ptimer_set_count(t->timer, s->interval[i]);
>>> + ptimer_run(t->timer, 1);
>>> + }
>>> + qemu_set_irq(s->irq[i],
>>> + (s->irq_status& s->irq_enable& 1<< i) != 0);
>>>
>>> + }
>>> + }
>>> +}
>>> +
>>> +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 = 0;
>>> +
>>> + 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++) {
>>> + SunxiTimer *t =&s->timer[i];
>>>
>>> + bh[i] = qemu_bh_new(sunxi_pit_timer_cb, s);
>>> + t->timer = ptimer_init(bh[i]);
>>> + ptimer_set_freq(t->timer, 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..260d36f
>>> --- /dev/null
>>> +++ b/include/hw/timer/sunxi-pit.h
>>> @@ -0,0 +1,37 @@
>>> +#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
>>> +
>>> +
>>> +#endif
>>> +
>>> --
>>> 1.7.2.5
>>>
>>>
>>>
>>
>>
>
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 1/4] hw/timer: add sunxi timer device
2013-11-26 9:14 ` Peter Crosthwaite
@ 2013-11-26 9:19 ` Li Guang
2013-11-26 9:25 ` Peter Crosthwaite
0 siblings, 1 reply; 33+ messages in thread
From: Li Guang @ 2013-11-26 9:19 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
Peter Crosthwaite wrote:
> On Tue, Nov 26, 2013 at 6:59 PM, Li Guang<lig.fnst@cn.fujitsu.com> wrote:
>
>> Peter Crosthwaite wrote:
>>
>>> On Tue, Nov 26, 2013 at 5:22 PM, liguang<lig.fnst@cn.fujitsu.com> wrote:
>>>
>>>
>>>> Signed-off-by: liguang<lig.fnst@cn.fujitsu.com>
>>>> ---
>>>> default-configs/arm-softmmu.mak | 2 +
>>>> hw/timer/Makefile.objs | 1 +
>>>> hw/timer/sunxi-pit.c | 295
>>>> +++++++++++++++++++++++++++++++++++++++
>>>> include/hw/timer/sunxi-pit.h | 37 +++++
>>>> 4 files changed, 335 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..39b84ab
>>>> --- /dev/null
>>>> +++ b/hw/timer/sunxi-pit.c
>>>> @@ -0,0 +1,295 @@
>>>> +/*
>>>> + * Allwinner sunxi timer device emulation
>>>> + *
>>>> + * Copyright (C) 2013 Li Guang
>>>> + * Written by Li Guang<lig.fnst@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"
>>>> +
>>>> +
>>>> +typedef struct SunxiTimer {
>>>> + ptimer_state *timer;
>>>> +} SunxiTimer;
>>>> +
>>>>
>>>>
>>> I don't understand the need for this struct. What was wrong with the
>>> direct array of ptimers you had before?
>>>
>>>
>>
>> because I have to pack timer array into VMSD,
>> and there's no VMSTATE_PTIMER_ARRAY for ptimer_state.
>>
>>
> Anyway you can just use VMSTATE_STRUCT_ARRAY on ptimers own VMSD definition?
>
> If you cant, then I think you make a reasonable case for moving the
> relevant bits and pieces to headers so they are visible. That or
> implement VMSTATE_PTIMER_ARRAY.
>
>
right, but that seems be in a separated patch,
I consider to use current way, can I?
>>
>>>
>>>> +typedef struct SunxiPITState {
>>>> + /*< private>*/
>>>> + SysBusDevice parent_obj;
>>>> + /*< public>*/
>>>> + qemu_irq irq[SUNXI_TIMER_NR];
>>>> + SunxiTimer 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;
>>>> +
>>>> +static uint64_t sunxi_pit_read(void *opaque, hwaddr offset, unsigned
>>>> size)
>>>> +{
>>>> + SunxiPITState *s = SUNXI_PIT(opaque);
>>>> + uint8_t index = 0;
>>>>
>>>>
>>> initializer to 0 un-needed.
>>>
>>>
>>>
>>
>> OK.
>>
>>>> +
>>>> + switch (offset) {
>>>> + case SUNXI_TIMER_IRQ_EN:
>>>> + return s->irq_enable;
>>>> + break;
>>>> + case SUNXI_TIMER_IRQ_ST:
>>>> + return s->irq_status;
>>>> + 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:
>>>> + return s->control[index];
>>>> + break;
>>>> + case SUNXI_TIMER_INTERVAL:
>>>> + return s->interval[index];
>>>> + break;
>>>> + case SUNXI_TIMER_COUNT: {
>>>> + SunxiTimer *t =&s->timer[index];
>>>>
>>>> + s->count[index] = ptimer_get_count(t->timer);
>>>> + return s->count[index];
>>>> + }
>>>> + default:
>>>> + break;
>>>> + }
>>>> + break;
>>>> + case SUNXI_WDOG_CONTROL:
>>>> + break;
>>>> + case SUNXI_WDOG_MODE:
>>>> + break;
>>>> + case SUNXI_COUNT_LO:
>>>> + return s->count_lo;
>>>> + break;
>>>> + case SUNXI_COUNT_HI:
>>>> + return s->count_hi;
>>>> + break;
>>>> + case SUNXI_COUNT_CTL:
>>>> + return s->count_ctl;
>>>> + default:
>>>> + break;
>>>>
>>>>
>>> Usual to do a log_guest error here. Same for writes below.
>>>
>>>
>>
>> OK.
>>
>>>
>>>
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void sunxi_pit_write(void *opaque, hwaddr offset, uint64_t value,
>>>> + unsigned size)
>>>> +{
>>>> + SunxiPITState *s = SUNXI_PIT(opaque);
>>>> + uint8_t index = 0;
>>>> +
>>>> + switch (offset) {
>>>> + case SUNXI_TIMER_IRQ_EN:
>>>> + s->irq_enable = value;
>>>> + break;
>>>> + case SUNXI_TIMER_IRQ_ST:
>>>> + s->irq_status&= value;
>>>>
>>>>
>>> Are you missing a ~ ? This is a clear-to-clear semantic rather than
>>> write-to-clear.
>>>
>>>
>>
>> yes
>>
>>
>>> Also shouldn't this de-assert the relevant interrupt lines?
>>>
>>>
>>>
>>
>> there's no related tips in sunxi SoC data-sheet,
>> and test is fine until now.
>>
>>
> But AFAICT, there is no way for software to lower an interrupt in an
> immediate timeframe, which is very un-usual. Usually the WTC of an ISR
> bring down the IRQ pin. There is only one call to qemu_set_irq and
> thats in the timer callback so the only way to lower interrupts is for
> the timer to hit again with the IRQ masked.
>
>
>
I will consider how to de-asset irq line correctly in this case,
Thanks!
>>>> + 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: {
>>>> + SunxiTimer *t =&s->timer[index];
>>>>
>>>> + s->control[index] = value;
>>>> + if (s->control[index]& SUNXI_TIMER_RELOAD) {
>>>> + ptimer_set_count(t->timer, s->interval[index]);
>>>> + }
>>>> + if (s->control[index]& SUNXI_TIMER_EN) {
>>>> + ptimer_run(t->timer, 1);
>>>> + } else {
>>>> + ptimer_stop(t->timer);
>>>> + }
>>>> + break;
>>>> + }
>>>> + case SUNXI_TIMER_INTERVAL: {
>>>> + SunxiTimer *t =&s->timer[index];
>>>>
>>>> + s->interval[index] = value;
>>>> + ptimer_set_count(t->timer, s->interval[index]);
>>>> + break;
>>>> + }
>>>> + case SUNXI_TIMER_COUNT:
>>>> + s->count[index] = value;
>>>> + default:
>>>> + break;
>>>> + }
>>>> + 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;
>>>>
>>>> + }
>>>> + default:
>>>> + break;
>>>> + }
>>>> +}
>>>> +
>>>> +static const MemoryRegionOps sunxi_pit_ops = {
>>>> + .read = sunxi_pit_read,
>>>> + .write = sunxi_pit_write,
>>>> + .endianness = DEVICE_NATIVE_ENDIAN,
>>>> +};
>>>> +
>>>> +static const VMStateDescription vmstate_sunxi_timer = {
>>>> + .name = "sunxi.timer",
>>>> + .version_id = 1,
>>>> + .minimum_version_id = 1,
>>>> + .minimum_version_id_old = 1,
>>>> + .fields = (VMStateField[]) {
>>>> + VMSTATE_PTIMER(timer, SunxiTimer),
>>>> + VMSTATE_END_OF_LIST()
>>>> + }
>>>> +};
>>>> +
>>>> +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_STRUCT_ARRAY(timer, SunxiPITState, SUNXI_TIMER_NR, 1,
>>>> + vmstate_sunxi_timer, SunxiTimer),
>>>> + VMSTATE_END_OF_LIST()
>>>> + }
>>>> +};
>>>> +
>>>> +static void sunxi_pit_reset(DeviceState *dev)
>>>> +{
>>>> + SunxiPITState *s = SUNXI_PIT(dev);
>>>> + uint8_t i = 0;
>>>> +
>>>> + s->irq_enable = 0;
>>>> + s->irq_status = 0;
>>>> + for (i = 0; i< 6; i++) {
>>>> + SunxiTimer *t =&s->timer[i];
>>>>
>>>> + s->control[i] = 0x4;
>>>>
>>>>
>>> Magic number.
>>>
>>>
>>>
>> OK, should be SUNXI_DEFAULT_CLOCK,
>>
>> Thanks!
>>
>>
>>>> + s->interval[i] = 0;
>>>> + s->count[i] = 0;
>>>> + ptimer_stop(t->timer);
>>>> + }
>>>> + 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 = 0;
>>>> +
>>>> + for (i = 0; i< SUNXI_TIMER_NR; i++) {
>>>> + SunxiTimer *t =&s->timer[i];
>>>>
>>>> + if (s->control[i]& SUNXI_TIMER_EN&&
>>>> + ptimer_get_count(t->timer) == 0) {
>>>> + s->irq_status |= 1<< i;
>>>> + if (!(s->control[i]& SUNXI_TIMER_MODE)) {
>>>> + ptimer_set_count(t->timer, s->interval[i]);
>>>> + ptimer_run(t->timer, 1);
>>>> + }
>>>> + qemu_set_irq(s->irq[i],
>>>> + (s->irq_status& s->irq_enable& 1<< i) != 0);
>>>>
>>>> + }
>>>> + }
>>>> +}
>>>> +
>>>> +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 = 0;
>>>> +
>>>> + 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++) {
>>>> + SunxiTimer *t =&s->timer[i];
>>>>
>>>> + bh[i] = qemu_bh_new(sunxi_pit_timer_cb, s);
>>>> + t->timer = ptimer_init(bh[i]);
>>>> + ptimer_set_freq(t->timer, 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..260d36f
>>>> --- /dev/null
>>>> +++ b/include/hw/timer/sunxi-pit.h
>>>> @@ -0,0 +1,37 @@
>>>> +#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
>>>> +
>>>> +
>>>> +#endif
>>>> +
>>>> --
>>>> 1.7.2.5
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>>
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 1/4] hw/timer: add sunxi timer device
2013-11-26 9:19 ` Li Guang
@ 2013-11-26 9:25 ` Peter Crosthwaite
0 siblings, 0 replies; 33+ messages in thread
From: Peter Crosthwaite @ 2013-11-26 9:25 UTC (permalink / raw)
To: Li Guang; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Tue, Nov 26, 2013 at 7:19 PM, Li Guang <lig.fnst@cn.fujitsu.com> wrote:
> Peter Crosthwaite wrote:
>>
>> On Tue, Nov 26, 2013 at 6:59 PM, Li Guang<lig.fnst@cn.fujitsu.com> wrote:
>>
>>>
>>> Peter Crosthwaite wrote:
>>>
>>>>
>>>> On Tue, Nov 26, 2013 at 5:22 PM, liguang<lig.fnst@cn.fujitsu.com>
>>>> wrote:
>>>>
>>>>
>>>>>
>>>>> Signed-off-by: liguang<lig.fnst@cn.fujitsu.com>
>>>>> ---
>>>>> default-configs/arm-softmmu.mak | 2 +
>>>>> hw/timer/Makefile.objs | 1 +
>>>>> hw/timer/sunxi-pit.c | 295
>>>>> +++++++++++++++++++++++++++++++++++++++
>>>>> include/hw/timer/sunxi-pit.h | 37 +++++
>>>>> 4 files changed, 335 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..39b84ab
>>>>> --- /dev/null
>>>>> +++ b/hw/timer/sunxi-pit.c
>>>>> @@ -0,0 +1,295 @@
>>>>> +/*
>>>>> + * Allwinner sunxi timer device emulation
>>>>> + *
>>>>> + * Copyright (C) 2013 Li Guang
>>>>> + * Written by Li Guang<lig.fnst@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"
>>>>> +
>>>>> +
>>>>> +typedef struct SunxiTimer {
>>>>> + ptimer_state *timer;
>>>>> +} SunxiTimer;
>>>>> +
>>>>>
>>>>>
>>>>
>>>> I don't understand the need for this struct. What was wrong with the
>>>> direct array of ptimers you had before?
>>>>
>>>>
>>>
>>>
>>> because I have to pack timer array into VMSD,
>>> and there's no VMSTATE_PTIMER_ARRAY for ptimer_state.
>>>
>>>
>>
>> Anyway you can just use VMSTATE_STRUCT_ARRAY on ptimers own VMSD
>> definition?
>>
>> If you cant, then I think you make a reasonable case for moving the
>> relevant bits and pieces to headers so they are visible. That or
>> implement VMSTATE_PTIMER_ARRAY.
>>
>>
>
>
> right, but that seems be in a separated patch,
> I consider to use current way, can I?
>
Well you have effectively implemented VMSTATE_PTIMER_ARRAY here
locally. Its probably best to just fix ptimer as first patch in this
series and it keeps your device model cleaner from the start. You may
have already done the hard work.
Regards,
Peter
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH v4 2/4] hw/intc: add sunxi interrupt controller device
2013-11-26 7:22 [Qemu-devel] [PATCH v4 0/4] add sunxi machine type liguang
2013-11-26 7:22 ` [Qemu-devel] [PATCH v4 1/4] hw/timer: add sunxi timer device liguang
@ 2013-11-26 7:22 ` liguang
2013-11-26 9:02 ` Peter Crosthwaite
2013-11-26 7:22 ` [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type liguang
2013-11-26 7:22 ` [Qemu-devel] [PATCH v4 4/4] MAINTAINERS: add myself to maintain sunxi machine liguang
3 siblings, 1 reply; 33+ messages in thread
From: liguang @ 2013-11-26 7:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, liguang
Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
---
default-configs/arm-softmmu.mak | 1 +
hw/intc/Makefile.objs | 1 +
hw/intc/sunxi-pic.c | 238 +++++++++++++++++++++++++++++++++++++++
include/hw/intc/sunxi-pic.h | 20 ++++
4 files changed, 260 insertions(+), 0 deletions(-)
create mode 100644 hw/intc/sunxi-pic.c
create mode 100644 include/hw/intc/sunxi-pic.h
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 7bf5ad0..bbe00e4 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -83,3 +83,4 @@ CONFIG_SDHCI=y
CONFIG_INTEGRATOR_DEBUG=y
CONFIG_SUNXI_PIT=y
+CONFIG_SUNXI_PIC=y
diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index 47ac442..dad8c43 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -12,6 +12,7 @@ common-obj-$(CONFIG_IOAPIC) += ioapic_common.o
common-obj-$(CONFIG_ARM_GIC) += arm_gic_common.o
common-obj-$(CONFIG_ARM_GIC) += arm_gic.o
common-obj-$(CONFIG_OPENPIC) += openpic.o
+common-obj-$(CONFIG_SUNXI_PIC) += sunxi-pic.o
obj-$(CONFIG_APIC) += apic.o apic_common.o
obj-$(CONFIG_ARM_GIC_KVM) += arm_gic_kvm.o
diff --git a/hw/intc/sunxi-pic.c b/hw/intc/sunxi-pic.c
new file mode 100644
index 0000000..e84fc55
--- /dev/null
+++ b/hw/intc/sunxi-pic.c
@@ -0,0 +1,238 @@
+/*
+ * Allwinner sunxi interrupt controller device emulation
+ *
+ * Copyright (C) 2013 Li Guang
+ * Written by Li Guang <lig.fnst@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/devices.h"
+#include "sysemu/sysemu.h"
+#include "hw/intc/sunxi-pic.h"
+
+
+typedef struct SunxiPICState {
+ /*< private >*/
+ SysBusDevice parent_obj;
+ /*< public >*/
+ MemoryRegion iomem;
+ qemu_irq parent_fiq;
+ qemu_irq parent_irq;
+ uint32_t vector;
+ uint32_t base_addr;
+ uint32_t protect;
+ uint32_t nmi;
+ uint32_t irq_pending[SUNXI_PIC_REG_IDX];
+ uint32_t fiq_pending[SUNXI_PIC_REG_IDX];
+ uint32_t select[SUNXI_PIC_REG_IDX];
+ uint32_t enable[SUNXI_PIC_REG_IDX];
+ uint32_t mask[SUNXI_PIC_REG_IDX];
+ /*priority setting here*/
+} SunxiPICState;
+
+static void sunxi_pic_update(SunxiPICState *s)
+{
+ uint8_t i = 0, j = 0;
+ bool irq = false;
+
+ for (i = 0, j = 0; i < SUNXI_PIC_REG_IDX; i++) {
+ for (j = 0; j < 32; j++) {
+ if (!test_bit(j, (void *)&s->mask[i])) {
+ if (test_bit(j, (void *)&s->irq_pending[i])) {
+ qemu_set_irq(s->parent_irq, 1);
+ irq = true;
+ }
+ if (test_bit(j, (void *)&s->fiq_pending[i]) &&
+ test_bit(j, (void *)&s->select[i])) {
+ qemu_set_irq(s->parent_fiq, 1);
+ irq = true;
+ }
+ if (irq) {
+ goto out;
+ }
+ }
+ }
+ }
+out:
+ return;
+}
+
+static void sunxi_pic_set_irq(void *opaque, int irq, int level)
+{
+ SunxiPICState *s = opaque;
+
+ if (level) {
+ set_bit(irq, (void *)&s->irq_pending[irq/32]);
+ }
+ sunxi_pic_update(s);
+}
+
+static uint64_t sunxi_pic_read(void *opaque, hwaddr offset, unsigned size)
+{
+ SunxiPICState *s = opaque;
+ uint8_t index = (offset & 0xc)/4;
+
+ switch (offset) {
+ case SUNXI_PIC_VECTOR:
+ return s->vector;
+ break;
+ case SUNXI_PIC_BASE_ADDR:
+ return s->base_addr;
+ break;
+ case SUNXI_PIC_PROTECT:
+ return s->protect;
+ break;
+ case SUNXI_PIC_NMI:
+ return s->nmi;
+ break;
+ case SUNXI_PIC_IRQ_PENDING ... SUNXI_PIC_IRQ_PENDING + 8:
+ return s->irq_pending[index];
+ break;
+ case SUNXI_PIC_FIQ_PENDING ... SUNXI_PIC_FIQ_PENDING + 8:
+ return s->fiq_pending[index];
+ break;
+ case SUNXI_PIC_SELECT ... SUNXI_PIC_SELECT + 8:
+ return s->select[index];
+ break;
+ case SUNXI_PIC_ENABLE ... SUNXI_PIC_ENABLE + 8:
+ return s->enable[index];
+ break;
+ case SUNXI_PIC_MASK ... SUNXI_PIC_MASK + 8:
+ return s->mask[index];
+ break;
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static void sunxi_pic_write(void *opaque, hwaddr offset, uint64_t value,
+ unsigned size)
+{
+ SunxiPICState *s = opaque;
+ uint8_t index = (offset & 0xc)/4;
+
+ switch (offset) {
+ case SUNXI_PIC_VECTOR:
+ s->vector = value & ~0x3;
+ break;
+ case SUNXI_PIC_BASE_ADDR:
+ s->base_addr = value & ~0x3;
+ case SUNXI_PIC_PROTECT:
+ s->protect = value;
+ break;
+ case SUNXI_PIC_NMI:
+ s->nmi = value;
+ break;
+ case SUNXI_PIC_IRQ_PENDING ... SUNXI_PIC_IRQ_PENDING + 8:
+ s->irq_pending[index] &= ~value;
+ break;
+ case SUNXI_PIC_FIQ_PENDING ... SUNXI_PIC_FIQ_PENDING + 8:
+ s->fiq_pending[index] &= ~value;
+ break;
+ case SUNXI_PIC_SELECT ... SUNXI_PIC_SELECT + 8:
+ s->select[index] = value;
+ break;
+ case SUNXI_PIC_ENABLE ... SUNXI_PIC_ENABLE + 8:
+ s->enable[index] = value;
+ break;
+ case SUNXI_PIC_MASK ... SUNXI_PIC_MASK + 8:
+ s->mask[index] = value;
+ break;
+ default:
+ break;
+ }
+
+ sunxi_pic_update(s);
+}
+
+static const MemoryRegionOps sunxi_pic_ops = {
+ .read = sunxi_pic_read,
+ .write = sunxi_pic_write,
+ .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static const VMStateDescription vmstate_sunxi_pic = {
+ .name = "sunxi.pic",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .minimum_version_id_old = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32(vector, SunxiPICState),
+ VMSTATE_UINT32(base_addr, SunxiPICState),
+ VMSTATE_UINT32(protect, SunxiPICState),
+ VMSTATE_UINT32(nmi, SunxiPICState),
+ VMSTATE_UINT32_ARRAY(irq_pending, SunxiPICState, SUNXI_PIC_REG_IDX),
+ VMSTATE_UINT32_ARRAY(fiq_pending, SunxiPICState, SUNXI_PIC_REG_IDX),
+ VMSTATE_UINT32_ARRAY(enable, SunxiPICState, SUNXI_PIC_REG_IDX),
+ VMSTATE_UINT32_ARRAY(select, SunxiPICState, SUNXI_PIC_REG_IDX),
+ VMSTATE_UINT32_ARRAY(mask, SunxiPICState, SUNXI_PIC_REG_IDX),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static void sunxi_pic_realize(DeviceState *ds, Error **errp)
+{
+ SunxiPICState *s = SUNXI_PIC(ds);
+ SysBusDevice *dev = SYS_BUS_DEVICE(ds);
+
+ qdev_init_gpio_in(DEVICE(dev), sunxi_pic_set_irq, SUNXI_PIC_INT_NR);
+ sysbus_init_irq(dev, &s->parent_irq);
+ sysbus_init_irq(dev, &s->parent_fiq);
+ memory_region_init_io(&s->iomem, OBJECT(s), &sunxi_pic_ops, s,
+ "sunxi-pic", 0x400);
+ sysbus_init_mmio(dev, &s->iomem);
+}
+
+static void sunxi_pic_reset(DeviceState *d)
+{
+ SunxiPICState *s = SUNXI_PIC(d);
+ uint8_t i;
+
+ s->base_addr = 0;
+ s->protect = 0;
+ s->nmi = 0;
+ s->vector = 0;
+ for (i = 0; i < SUNXI_PIC_REG_IDX; i++) {
+ s->irq_pending[i] = 0;
+ s->fiq_pending[i] = 0;
+ s->select[i] = 0;
+ s->enable[i] = 0;
+ s->mask[i] = 0;
+ }
+}
+
+static void sunxi_pic_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+
+ dc->realize = sunxi_pic_realize;
+ dc->reset = sunxi_pic_reset;
+ dc->desc = "sunxi pic";
+ dc->vmsd = &vmstate_sunxi_pic;
+ }
+
+static const TypeInfo sunxi_pic_info = {
+ .name = TYPE_SUNXI_PIC,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(SunxiPICState),
+ .class_init = sunxi_pic_class_init,
+};
+
+static void sunxi_register_types(void)
+{
+ type_register_static(&sunxi_pic_info);
+}
+
+type_init(sunxi_register_types);
diff --git a/include/hw/intc/sunxi-pic.h b/include/hw/intc/sunxi-pic.h
new file mode 100644
index 0000000..d52b53c
--- /dev/null
+++ b/include/hw/intc/sunxi-pic.h
@@ -0,0 +1,20 @@
+#ifndef SUNXI_PIC_H
+#define SUNXI_PIC_H
+
+#define TYPE_SUNXI_PIC "sunxi-pic"
+#define SUNXI_PIC(obj) OBJECT_CHECK(SunxiPICState, (obj), TYPE_SUNXI_PIC)
+
+#define SUNXI_PIC_VECTOR 0
+#define SUNXI_PIC_BASE_ADDR 4
+#define SUNXI_PIC_PROTECT 8
+#define SUNXI_PIC_NMI 0xc
+#define SUNXI_PIC_IRQ_PENDING 0x10
+#define SUNXI_PIC_FIQ_PENDING 0x20
+#define SUNXI_PIC_SELECT 0x30
+#define SUNXI_PIC_ENABLE 0x40
+#define SUNXI_PIC_MASK 0x50
+
+#define SUNXI_PIC_INT_NR 95
+#define SUNXI_PIC_REG_IDX (SUNXI_PIC_INT_NR / 32 + 1)
+
+#endif
--
1.7.2.5
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 2/4] hw/intc: add sunxi interrupt controller device
2013-11-26 7:22 ` [Qemu-devel] [PATCH v4 2/4] hw/intc: add sunxi interrupt controller device liguang
@ 2013-11-26 9:02 ` Peter Crosthwaite
2013-11-26 9:11 ` Li Guang
0 siblings, 1 reply; 33+ messages in thread
From: Peter Crosthwaite @ 2013-11-26 9:02 UTC (permalink / raw)
To: liguang; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Tue, Nov 26, 2013 at 5:22 PM, liguang <lig.fnst@cn.fujitsu.com> wrote:
> Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> ---
> default-configs/arm-softmmu.mak | 1 +
> hw/intc/Makefile.objs | 1 +
> hw/intc/sunxi-pic.c | 238 +++++++++++++++++++++++++++++++++++++++
> include/hw/intc/sunxi-pic.h | 20 ++++
> 4 files changed, 260 insertions(+), 0 deletions(-)
> create mode 100644 hw/intc/sunxi-pic.c
> create mode 100644 include/hw/intc/sunxi-pic.h
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index 7bf5ad0..bbe00e4 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -83,3 +83,4 @@ CONFIG_SDHCI=y
> CONFIG_INTEGRATOR_DEBUG=y
>
> CONFIG_SUNXI_PIT=y
> +CONFIG_SUNXI_PIC=y
> diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
> index 47ac442..dad8c43 100644
> --- a/hw/intc/Makefile.objs
> +++ b/hw/intc/Makefile.objs
> @@ -12,6 +12,7 @@ common-obj-$(CONFIG_IOAPIC) += ioapic_common.o
> common-obj-$(CONFIG_ARM_GIC) += arm_gic_common.o
> common-obj-$(CONFIG_ARM_GIC) += arm_gic.o
> common-obj-$(CONFIG_OPENPIC) += openpic.o
> +common-obj-$(CONFIG_SUNXI_PIC) += sunxi-pic.o
>
> obj-$(CONFIG_APIC) += apic.o apic_common.o
> obj-$(CONFIG_ARM_GIC_KVM) += arm_gic_kvm.o
> diff --git a/hw/intc/sunxi-pic.c b/hw/intc/sunxi-pic.c
> new file mode 100644
> index 0000000..e84fc55
> --- /dev/null
> +++ b/hw/intc/sunxi-pic.c
> @@ -0,0 +1,238 @@
> +/*
> + * Allwinner sunxi interrupt controller device emulation
> + *
> + * Copyright (C) 2013 Li Guang
> + * Written by Li Guang <lig.fnst@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/devices.h"
> +#include "sysemu/sysemu.h"
> +#include "hw/intc/sunxi-pic.h"
> +
> +
> +typedef struct SunxiPICState {
> + /*< private >*/
> + SysBusDevice parent_obj;
> + /*< public >*/
> + MemoryRegion iomem;
> + qemu_irq parent_fiq;
> + qemu_irq parent_irq;
Blank line here for readability.
> + uint32_t vector;
> + uint32_t base_addr;
> + uint32_t protect;
> + uint32_t nmi;
> + uint32_t irq_pending[SUNXI_PIC_REG_IDX];
> + uint32_t fiq_pending[SUNXI_PIC_REG_IDX];
this appears to be constant 0. I cant find anywhere that sets fiq_pending.
> + uint32_t select[SUNXI_PIC_REG_IDX];
> + uint32_t enable[SUNXI_PIC_REG_IDX];
> + uint32_t mask[SUNXI_PIC_REG_IDX];
> + /*priority setting here*/
> +} SunxiPICState;
> +
> +static void sunxi_pic_update(SunxiPICState *s)
> +{
> + uint8_t i = 0, j = 0;
Initialisers un-needed.
> + bool irq = false;
> +
> + for (i = 0, j = 0; i < SUNXI_PIC_REG_IDX; i++) {
> + for (j = 0; j < 32; j++) {
> + if (!test_bit(j, (void *)&s->mask[i])) {
You can save on a level of indentation here with:
if (test_bit(j, (void *)&s->mask[i])) {
continue;
}
> + if (test_bit(j, (void *)&s->irq_pending[i])) {
> + qemu_set_irq(s->parent_irq, 1);
qemu_irq_raise() is simpler.
I can't find anywhere in the code where the interrupts are lowered.
How do they come down, once they are up?
> + irq = true;
> + }
> + if (test_bit(j, (void *)&s->fiq_pending[i]) &&
> + test_bit(j, (void *)&s->select[i])) {
> + qemu_set_irq(s->parent_fiq, 1);
> + irq = true;
> + }
> + if (irq) {
> + goto out;
What happens if two interrupts are both active, the first setting IRQ
the second FIQ? Wont this escape logic cause FIQ raising to
potentionally be missed?
> + }
> + }
> + }
> + }
> +out:
> + return;
> +}
> +
> +static void sunxi_pic_set_irq(void *opaque, int irq, int level)
> +{
> + SunxiPICState *s = opaque;
> +
> + if (level) {
> + set_bit(irq, (void *)&s->irq_pending[irq/32]);
set_bit(irq % 32, ...)
> + }
Is this supposed to set either fiq_pending or irq_pending depending on
the select bit?
> + sunxi_pic_update(s);
> +}
> +
> +static uint64_t sunxi_pic_read(void *opaque, hwaddr offset, unsigned size)
> +{
> + SunxiPICState *s = opaque;
> + uint8_t index = (offset & 0xc)/4;
> +
> + switch (offset) {
> + case SUNXI_PIC_VECTOR:
> + return s->vector;
> + break;
> + case SUNXI_PIC_BASE_ADDR:
> + return s->base_addr;
> + break;
> + case SUNXI_PIC_PROTECT:
> + return s->protect;
> + break;
> + case SUNXI_PIC_NMI:
> + return s->nmi;
> + break;
> + case SUNXI_PIC_IRQ_PENDING ... SUNXI_PIC_IRQ_PENDING + 8:
> + return s->irq_pending[index];
> + break;
> + case SUNXI_PIC_FIQ_PENDING ... SUNXI_PIC_FIQ_PENDING + 8:
> + return s->fiq_pending[index];
> + break;
> + case SUNXI_PIC_SELECT ... SUNXI_PIC_SELECT + 8:
> + return s->select[index];
> + break;
> + case SUNXI_PIC_ENABLE ... SUNXI_PIC_ENABLE + 8:
> + return s->enable[index];
> + break;
> + case SUNXI_PIC_MASK ... SUNXI_PIC_MASK + 8:
> + return s->mask[index];
> + break;
> + default:
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static void sunxi_pic_write(void *opaque, hwaddr offset, uint64_t value,
> + unsigned size)
> +{
> + SunxiPICState *s = opaque;
> + uint8_t index = (offset & 0xc)/4;
> +
> + switch (offset) {
> + case SUNXI_PIC_VECTOR:
> + s->vector = value & ~0x3;
> + break;
> + case SUNXI_PIC_BASE_ADDR:
> + s->base_addr = value & ~0x3;
> + case SUNXI_PIC_PROTECT:
> + s->protect = value;
> + break;
> + case SUNXI_PIC_NMI:
> + s->nmi = value;
> + break;
> + case SUNXI_PIC_IRQ_PENDING ... SUNXI_PIC_IRQ_PENDING + 8:
> + s->irq_pending[index] &= ~value;
> + break;
> + case SUNXI_PIC_FIQ_PENDING ... SUNXI_PIC_FIQ_PENDING + 8:
> + s->fiq_pending[index] &= ~value;
> + break;
> + case SUNXI_PIC_SELECT ... SUNXI_PIC_SELECT + 8:
> + s->select[index] = value;
> + break;
> + case SUNXI_PIC_ENABLE ... SUNXI_PIC_ENABLE + 8:
> + s->enable[index] = value;
> + break;
> + case SUNXI_PIC_MASK ... SUNXI_PIC_MASK + 8:
> + s->mask[index] = value;
> + break;
> + default:
> + break;
> + }
> +
> + sunxi_pic_update(s);
> +}
> +
> +static const MemoryRegionOps sunxi_pic_ops = {
> + .read = sunxi_pic_read,
> + .write = sunxi_pic_write,
> + .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static const VMStateDescription vmstate_sunxi_pic = {
> + .name = "sunxi.pic",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .minimum_version_id_old = 1,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32(vector, SunxiPICState),
> + VMSTATE_UINT32(base_addr, SunxiPICState),
> + VMSTATE_UINT32(protect, SunxiPICState),
> + VMSTATE_UINT32(nmi, SunxiPICState),
> + VMSTATE_UINT32_ARRAY(irq_pending, SunxiPICState, SUNXI_PIC_REG_IDX),
> + VMSTATE_UINT32_ARRAY(fiq_pending, SunxiPICState, SUNXI_PIC_REG_IDX),
> + VMSTATE_UINT32_ARRAY(enable, SunxiPICState, SUNXI_PIC_REG_IDX),
> + VMSTATE_UINT32_ARRAY(select, SunxiPICState, SUNXI_PIC_REG_IDX),
> + VMSTATE_UINT32_ARRAY(mask, SunxiPICState, SUNXI_PIC_REG_IDX),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static void sunxi_pic_realize(DeviceState *ds, Error **errp)
> +{
> + SunxiPICState *s = SUNXI_PIC(ds);
> + SysBusDevice *dev = SYS_BUS_DEVICE(ds);
> +
> + qdev_init_gpio_in(DEVICE(dev), sunxi_pic_set_irq, SUNXI_PIC_INT_NR);
> + sysbus_init_irq(dev, &s->parent_irq);
> + sysbus_init_irq(dev, &s->parent_fiq);
> + memory_region_init_io(&s->iomem, OBJECT(s), &sunxi_pic_ops, s,
> + "sunxi-pic", 0x400);
> + sysbus_init_mmio(dev, &s->iomem);
> +}
> +
> +static void sunxi_pic_reset(DeviceState *d)
> +{
> + SunxiPICState *s = SUNXI_PIC(d);
> + uint8_t i;
> +
> + s->base_addr = 0;
> + s->protect = 0;
> + s->nmi = 0;
> + s->vector = 0;
> + for (i = 0; i < SUNXI_PIC_REG_IDX; i++) {
> + s->irq_pending[i] = 0;
> + s->fiq_pending[i] = 0;
> + s->select[i] = 0;
> + s->enable[i] = 0;
> + s->mask[i] = 0;
> + }
> +}
> +
> +static void sunxi_pic_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->realize = sunxi_pic_realize;
> + dc->reset = sunxi_pic_reset;
> + dc->desc = "sunxi pic";
> + dc->vmsd = &vmstate_sunxi_pic;
> + }
> +
> +static const TypeInfo sunxi_pic_info = {
> + .name = TYPE_SUNXI_PIC,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(SunxiPICState),
> + .class_init = sunxi_pic_class_init,
> +};
> +
> +static void sunxi_register_types(void)
> +{
> + type_register_static(&sunxi_pic_info);
> +}
> +
> +type_init(sunxi_register_types);
> diff --git a/include/hw/intc/sunxi-pic.h b/include/hw/intc/sunxi-pic.h
> new file mode 100644
> index 0000000..d52b53c
> --- /dev/null
> +++ b/include/hw/intc/sunxi-pic.h
> @@ -0,0 +1,20 @@
> +#ifndef SUNXI_PIC_H
> +#define SUNXI_PIC_H
> +
> +#define TYPE_SUNXI_PIC "sunxi-pic"
> +#define SUNXI_PIC(obj) OBJECT_CHECK(SunxiPICState, (obj), TYPE_SUNXI_PIC)
> +
> +#define SUNXI_PIC_VECTOR 0
> +#define SUNXI_PIC_BASE_ADDR 4
> +#define SUNXI_PIC_PROTECT 8
> +#define SUNXI_PIC_NMI 0xc
> +#define SUNXI_PIC_IRQ_PENDING 0x10
> +#define SUNXI_PIC_FIQ_PENDING 0x20
> +#define SUNXI_PIC_SELECT 0x30
> +#define SUNXI_PIC_ENABLE 0x40
> +#define SUNXI_PIC_MASK 0x50
> +
> +#define SUNXI_PIC_INT_NR 95
Is this constant or configurable? Should it perhaps be a static prop?
> +#define SUNXI_PIC_REG_IDX (SUNXI_PIC_INT_NR / 32 + 1)
> +
DIV_ROUND_UP is probably the intention here.
Regards,
Peter
> +#endif
> --
> 1.7.2.5
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 2/4] hw/intc: add sunxi interrupt controller device
2013-11-26 9:02 ` Peter Crosthwaite
@ 2013-11-26 9:11 ` Li Guang
2013-11-27 3:36 ` Li Guang
0 siblings, 1 reply; 33+ messages in thread
From: Li Guang @ 2013-11-26 9:11 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
Peter Crosthwaite wrote:
> On Tue, Nov 26, 2013 at 5:22 PM, liguang<lig.fnst@cn.fujitsu.com> wrote:
>
>> Signed-off-by: liguang<lig.fnst@cn.fujitsu.com>
>> ---
>> default-configs/arm-softmmu.mak | 1 +
>> hw/intc/Makefile.objs | 1 +
>> hw/intc/sunxi-pic.c | 238 +++++++++++++++++++++++++++++++++++++++
>> include/hw/intc/sunxi-pic.h | 20 ++++
>> 4 files changed, 260 insertions(+), 0 deletions(-)
>> create mode 100644 hw/intc/sunxi-pic.c
>> create mode 100644 include/hw/intc/sunxi-pic.h
>>
>> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
>> index 7bf5ad0..bbe00e4 100644
>> --- a/default-configs/arm-softmmu.mak
>> +++ b/default-configs/arm-softmmu.mak
>> @@ -83,3 +83,4 @@ CONFIG_SDHCI=y
>> CONFIG_INTEGRATOR_DEBUG=y
>>
>> CONFIG_SUNXI_PIT=y
>> +CONFIG_SUNXI_PIC=y
>> diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
>> index 47ac442..dad8c43 100644
>> --- a/hw/intc/Makefile.objs
>> +++ b/hw/intc/Makefile.objs
>> @@ -12,6 +12,7 @@ common-obj-$(CONFIG_IOAPIC) += ioapic_common.o
>> common-obj-$(CONFIG_ARM_GIC) += arm_gic_common.o
>> common-obj-$(CONFIG_ARM_GIC) += arm_gic.o
>> common-obj-$(CONFIG_OPENPIC) += openpic.o
>> +common-obj-$(CONFIG_SUNXI_PIC) += sunxi-pic.o
>>
>> obj-$(CONFIG_APIC) += apic.o apic_common.o
>> obj-$(CONFIG_ARM_GIC_KVM) += arm_gic_kvm.o
>> diff --git a/hw/intc/sunxi-pic.c b/hw/intc/sunxi-pic.c
>> new file mode 100644
>> index 0000000..e84fc55
>> --- /dev/null
>> +++ b/hw/intc/sunxi-pic.c
>> @@ -0,0 +1,238 @@
>> +/*
>> + * Allwinner sunxi interrupt controller device emulation
>> + *
>> + * Copyright (C) 2013 Li Guang
>> + * Written by Li Guang<lig.fnst@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/devices.h"
>> +#include "sysemu/sysemu.h"
>> +#include "hw/intc/sunxi-pic.h"
>> +
>> +
>> +typedef struct SunxiPICState {
>> + /*< private>*/
>> + SysBusDevice parent_obj;
>> + /*< public>*/
>> + MemoryRegion iomem;
>> + qemu_irq parent_fiq;
>> + qemu_irq parent_irq;
>>
> Blank line here for readability.
>
>
OK
>> + uint32_t vector;
>> + uint32_t base_addr;
>> + uint32_t protect;
>> + uint32_t nmi;
>> + uint32_t irq_pending[SUNXI_PIC_REG_IDX];
>> + uint32_t fiq_pending[SUNXI_PIC_REG_IDX];
>>
> this appears to be constant 0. I cant find anywhere that sets fiq_pending.
>
>
here, only NMI can generate fiq,
and I did take care NMI now,
so there's no real case to set fiq.
>> + uint32_t select[SUNXI_PIC_REG_IDX];
>> + uint32_t enable[SUNXI_PIC_REG_IDX];
>> + uint32_t mask[SUNXI_PIC_REG_IDX];
>> + /*priority setting here*/
>> +} SunxiPICState;
>> +
>> +static void sunxi_pic_update(SunxiPICState *s)
>> +{
>> + uint8_t i = 0, j = 0;
>>
> Initialisers un-needed.
>
>
OK
>> + bool irq = false;
>> +
>> + for (i = 0, j = 0; i< SUNXI_PIC_REG_IDX; i++) {
>> + for (j = 0; j< 32; j++) {
>> + if (!test_bit(j, (void *)&s->mask[i])) {
>>
> You can save on a level of indentation here with:
>
> if (test_bit(j, (void *)&s->mask[i])) {
> continue;
> }
>
>
OK
>> + if (test_bit(j, (void *)&s->irq_pending[i])) {
>> + qemu_set_irq(s->parent_irq, 1);
>>
> qemu_irq_raise() is simpler.
>
> I can't find anywhere in the code where the interrupts are lowered.
> How do they come down, once they are up?
>
>
>> + irq = true;
>> + }
>> + if (test_bit(j, (void *)&s->fiq_pending[i])&&
>> + test_bit(j, (void *)&s->select[i])) {
>> + qemu_set_irq(s->parent_fiq, 1);
>> + irq = true;
>> + }
>> + if (irq) {
>> + goto out;
>>
> What happens if two interrupts are both active, the first setting IRQ
> the second FIQ? Wont this escape logic cause FIQ raising to
> potentionally be missed?
>
>
>> + }
>> + }
>> + }
>> + }
>> +out:
>> + return;
>> +}
>> +
>> +static void sunxi_pic_set_irq(void *opaque, int irq, int level)
>> +{
>> + SunxiPICState *s = opaque;
>> +
>> + if (level) {
>> + set_bit(irq, (void *)&s->irq_pending[irq/32]);
>>
> set_bit(irq % 32, ...)
>
>
OK
>> + }
>>
> Is this supposed to set either fiq_pending or irq_pending depending on
> the select bit?
>
>
Yes, but, I as I stated before, maybe I will take NMI later.
>> + sunxi_pic_update(s);
>> +}
>> +
>> +static uint64_t sunxi_pic_read(void *opaque, hwaddr offset, unsigned size)
>> +{
>> + SunxiPICState *s = opaque;
>> + uint8_t index = (offset& 0xc)/4;
>> +
>> + switch (offset) {
>> + case SUNXI_PIC_VECTOR:
>> + return s->vector;
>> + break;
>> + case SUNXI_PIC_BASE_ADDR:
>> + return s->base_addr;
>> + break;
>> + case SUNXI_PIC_PROTECT:
>> + return s->protect;
>> + break;
>> + case SUNXI_PIC_NMI:
>> + return s->nmi;
>> + break;
>> + case SUNXI_PIC_IRQ_PENDING ... SUNXI_PIC_IRQ_PENDING + 8:
>> + return s->irq_pending[index];
>> + break;
>> + case SUNXI_PIC_FIQ_PENDING ... SUNXI_PIC_FIQ_PENDING + 8:
>> + return s->fiq_pending[index];
>> + break;
>> + case SUNXI_PIC_SELECT ... SUNXI_PIC_SELECT + 8:
>> + return s->select[index];
>> + break;
>> + case SUNXI_PIC_ENABLE ... SUNXI_PIC_ENABLE + 8:
>> + return s->enable[index];
>> + break;
>> + case SUNXI_PIC_MASK ... SUNXI_PIC_MASK + 8:
>> + return s->mask[index];
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void sunxi_pic_write(void *opaque, hwaddr offset, uint64_t value,
>> + unsigned size)
>> +{
>> + SunxiPICState *s = opaque;
>> + uint8_t index = (offset& 0xc)/4;
>> +
>> + switch (offset) {
>> + case SUNXI_PIC_VECTOR:
>> + s->vector = value& ~0x3;
>> + break;
>> + case SUNXI_PIC_BASE_ADDR:
>> + s->base_addr = value& ~0x3;
>> + case SUNXI_PIC_PROTECT:
>> + s->protect = value;
>> + break;
>> + case SUNXI_PIC_NMI:
>> + s->nmi = value;
>> + break;
>> + case SUNXI_PIC_IRQ_PENDING ... SUNXI_PIC_IRQ_PENDING + 8:
>> + s->irq_pending[index]&= ~value;
>> + break;
>> + case SUNXI_PIC_FIQ_PENDING ... SUNXI_PIC_FIQ_PENDING + 8:
>> + s->fiq_pending[index]&= ~value;
>> + break;
>> + case SUNXI_PIC_SELECT ... SUNXI_PIC_SELECT + 8:
>> + s->select[index] = value;
>> + break;
>> + case SUNXI_PIC_ENABLE ... SUNXI_PIC_ENABLE + 8:
>> + s->enable[index] = value;
>> + break;
>> + case SUNXI_PIC_MASK ... SUNXI_PIC_MASK + 8:
>> + s->mask[index] = value;
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> + sunxi_pic_update(s);
>> +}
>> +
>> +static const MemoryRegionOps sunxi_pic_ops = {
>> + .read = sunxi_pic_read,
>> + .write = sunxi_pic_write,
>> + .endianness = DEVICE_NATIVE_ENDIAN,
>> +};
>> +
>> +static const VMStateDescription vmstate_sunxi_pic = {
>> + .name = "sunxi.pic",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .minimum_version_id_old = 1,
>> + .fields = (VMStateField[]) {
>> + VMSTATE_UINT32(vector, SunxiPICState),
>> + VMSTATE_UINT32(base_addr, SunxiPICState),
>> + VMSTATE_UINT32(protect, SunxiPICState),
>> + VMSTATE_UINT32(nmi, SunxiPICState),
>> + VMSTATE_UINT32_ARRAY(irq_pending, SunxiPICState, SUNXI_PIC_REG_IDX),
>> + VMSTATE_UINT32_ARRAY(fiq_pending, SunxiPICState, SUNXI_PIC_REG_IDX),
>> + VMSTATE_UINT32_ARRAY(enable, SunxiPICState, SUNXI_PIC_REG_IDX),
>> + VMSTATE_UINT32_ARRAY(select, SunxiPICState, SUNXI_PIC_REG_IDX),
>> + VMSTATE_UINT32_ARRAY(mask, SunxiPICState, SUNXI_PIC_REG_IDX),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> +static void sunxi_pic_realize(DeviceState *ds, Error **errp)
>> +{
>> + SunxiPICState *s = SUNXI_PIC(ds);
>> + SysBusDevice *dev = SYS_BUS_DEVICE(ds);
>> +
>> + qdev_init_gpio_in(DEVICE(dev), sunxi_pic_set_irq, SUNXI_PIC_INT_NR);
>> + sysbus_init_irq(dev,&s->parent_irq);
>> + sysbus_init_irq(dev,&s->parent_fiq);
>> + memory_region_init_io(&s->iomem, OBJECT(s),&sunxi_pic_ops, s,
>> + "sunxi-pic", 0x400);
>> + sysbus_init_mmio(dev,&s->iomem);
>> +}
>> +
>> +static void sunxi_pic_reset(DeviceState *d)
>> +{
>> + SunxiPICState *s = SUNXI_PIC(d);
>> + uint8_t i;
>> +
>> + s->base_addr = 0;
>> + s->protect = 0;
>> + s->nmi = 0;
>> + s->vector = 0;
>> + for (i = 0; i< SUNXI_PIC_REG_IDX; i++) {
>> + s->irq_pending[i] = 0;
>> + s->fiq_pending[i] = 0;
>> + s->select[i] = 0;
>> + s->enable[i] = 0;
>> + s->mask[i] = 0;
>> + }
>> +}
>> +
>> +static void sunxi_pic_class_init(ObjectClass *klass, void *data)
>> +{
>> + DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> + dc->realize = sunxi_pic_realize;
>> + dc->reset = sunxi_pic_reset;
>> + dc->desc = "sunxi pic";
>> + dc->vmsd =&vmstate_sunxi_pic;
>> + }
>> +
>> +static const TypeInfo sunxi_pic_info = {
>> + .name = TYPE_SUNXI_PIC,
>> + .parent = TYPE_SYS_BUS_DEVICE,
>> + .instance_size = sizeof(SunxiPICState),
>> + .class_init = sunxi_pic_class_init,
>> +};
>> +
>> +static void sunxi_register_types(void)
>> +{
>> + type_register_static(&sunxi_pic_info);
>> +}
>> +
>> +type_init(sunxi_register_types);
>> diff --git a/include/hw/intc/sunxi-pic.h b/include/hw/intc/sunxi-pic.h
>> new file mode 100644
>> index 0000000..d52b53c
>> --- /dev/null
>> +++ b/include/hw/intc/sunxi-pic.h
>> @@ -0,0 +1,20 @@
>> +#ifndef SUNXI_PIC_H
>> +#define SUNXI_PIC_H
>> +
>> +#define TYPE_SUNXI_PIC "sunxi-pic"
>> +#define SUNXI_PIC(obj) OBJECT_CHECK(SunxiPICState, (obj), TYPE_SUNXI_PIC)
>> +
>> +#define SUNXI_PIC_VECTOR 0
>> +#define SUNXI_PIC_BASE_ADDR 4
>> +#define SUNXI_PIC_PROTECT 8
>> +#define SUNXI_PIC_NMI 0xc
>> +#define SUNXI_PIC_IRQ_PENDING 0x10
>> +#define SUNXI_PIC_FIQ_PENDING 0x20
>> +#define SUNXI_PIC_SELECT 0x30
>> +#define SUNXI_PIC_ENABLE 0x40
>> +#define SUNXI_PIC_MASK 0x50
>> +
>> +#define SUNXI_PIC_INT_NR 95
>>
> Is this constant or configurable? Should it perhaps be a static prop?
>
NO, it's constant
>
>> +#define SUNXI_PIC_REG_IDX (SUNXI_PIC_INT_NR / 32 + 1)
>> +
>>
> DIV_ROUND_UP is probably the intention here.
>
>
>
right, will fix
thank you so much!
>
>> +#endif
>> --
>> 1.7.2.5
>>
>>
>>
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 2/4] hw/intc: add sunxi interrupt controller device
2013-11-26 9:11 ` Li Guang
@ 2013-11-27 3:36 ` Li Guang
2013-11-27 5:31 ` Peter Crosthwaite
0 siblings, 1 reply; 33+ messages in thread
From: Li Guang @ 2013-11-27 3:36 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
Li Guang wrote:
> Peter Crosthwaite wrote:
>> On Tue, Nov 26, 2013 at 5:22 PM, liguang<lig.fnst@cn.fujitsu.com>
>> wrote:
>>> Signed-off-by: liguang<lig.fnst@cn.fujitsu.com>
>>> ---
>>> default-configs/arm-softmmu.mak | 1 +
>>> hw/intc/Makefile.objs | 1 +
>>> hw/intc/sunxi-pic.c | 238
>>> +++++++++++++++++++++++++++++++++++++++
>>> include/hw/intc/sunxi-pic.h | 20 ++++
>>> 4 files changed, 260 insertions(+), 0 deletions(-)
>>> create mode 100644 hw/intc/sunxi-pic.c
>>> create mode 100644 include/hw/intc/sunxi-pic.h
>>>
>>> diff --git a/default-configs/arm-softmmu.mak
>>> b/default-configs/arm-softmmu.mak
>>> index 7bf5ad0..bbe00e4 100644
>>> --- a/default-configs/arm-softmmu.mak
>>> +++ b/default-configs/arm-softmmu.mak
>>> @@ -83,3 +83,4 @@ CONFIG_SDHCI=y
>>> CONFIG_INTEGRATOR_DEBUG=y
>>>
>>> CONFIG_SUNXI_PIT=y
>>> +CONFIG_SUNXI_PIC=y
>>> diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
>>> index 47ac442..dad8c43 100644
>>> --- a/hw/intc/Makefile.objs
>>> +++ b/hw/intc/Makefile.objs
>>> @@ -12,6 +12,7 @@ common-obj-$(CONFIG_IOAPIC) += ioapic_common.o
>>> common-obj-$(CONFIG_ARM_GIC) += arm_gic_common.o
>>> common-obj-$(CONFIG_ARM_GIC) += arm_gic.o
>>> common-obj-$(CONFIG_OPENPIC) += openpic.o
>>> +common-obj-$(CONFIG_SUNXI_PIC) += sunxi-pic.o
>>>
>>> obj-$(CONFIG_APIC) += apic.o apic_common.o
>>> obj-$(CONFIG_ARM_GIC_KVM) += arm_gic_kvm.o
>>> diff --git a/hw/intc/sunxi-pic.c b/hw/intc/sunxi-pic.c
>>> new file mode 100644
>>> index 0000000..e84fc55
>>> --- /dev/null
>>> +++ b/hw/intc/sunxi-pic.c
>>> @@ -0,0 +1,238 @@
>>> +/*
>>> + * Allwinner sunxi interrupt controller device emulation
>>> + *
>>> + * Copyright (C) 2013 Li Guang
>>> + * Written by Li Guang<lig.fnst@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/devices.h"
>>> +#include "sysemu/sysemu.h"
>>> +#include "hw/intc/sunxi-pic.h"
>>> +
>>> +
>>> +typedef struct SunxiPICState {
>>> + /*< private>*/
>>> + SysBusDevice parent_obj;
>>> + /*< public>*/
>>> + MemoryRegion iomem;
>>> + qemu_irq parent_fiq;
>>> + qemu_irq parent_irq;
>> Blank line here for readability.
>>
>
> OK
>>> + uint32_t vector;
>>> + uint32_t base_addr;
>>> + uint32_t protect;
>>> + uint32_t nmi;
>>> + uint32_t irq_pending[SUNXI_PIC_REG_IDX];
>>> + uint32_t fiq_pending[SUNXI_PIC_REG_IDX];
>> this appears to be constant 0. I cant find anywhere that sets
>> fiq_pending.
>>
>
> here, only NMI can generate fiq,
> and I did take care NMI now,
> so there's no real case to set fiq.
>
>>> + uint32_t select[SUNXI_PIC_REG_IDX];
>>> + uint32_t enable[SUNXI_PIC_REG_IDX];
>>> + uint32_t mask[SUNXI_PIC_REG_IDX];
>>> + /*priority setting here*/
>>> +} SunxiPICState;
>>> +
>>> +static void sunxi_pic_update(SunxiPICState *s)
>>> +{
>>> + uint8_t i = 0, j = 0;
>> Initialisers un-needed.
>>
> OK
>>> + bool irq = false;
>>> +
>>> + for (i = 0, j = 0; i< SUNXI_PIC_REG_IDX; i++) {
>>> + for (j = 0; j< 32; j++) {
>>> + if (!test_bit(j, (void *)&s->mask[i])) {
>> You can save on a level of indentation here with:
>>
>> if (test_bit(j, (void *)&s->mask[i])) {
>> continue;
>> }
>>
> OK
>>> + if (test_bit(j, (void *)&s->irq_pending[i])) {
>>> + qemu_set_irq(s->parent_irq, 1);
>> qemu_irq_raise() is simpler.
>>
>> I can't find anywhere in the code where the interrupts are lowered.
>> How do they come down, once they are up?
>>
>
>
>
>>> + irq = true;
>>> + }
>>> + if (test_bit(j, (void *)&s->fiq_pending[i])&&
>>> + test_bit(j, (void *)&s->select[i])) {
>>> + qemu_set_irq(s->parent_fiq, 1);
>>> + irq = true;
>>> + }
>>> + if (irq) {
>>> + goto out;
>> What happens if two interrupts are both active, the first setting IRQ
>> the second FIQ? Wont this escape logic cause FIQ raising to
>> potentionally be missed?
>>
>>> + }
>>> + }
>>> + }
>>> + }
>>> +out:
>>> + return;
>>> +}
>>> +
>>> +static void sunxi_pic_set_irq(void *opaque, int irq, int level)
>>> +{
>>> + SunxiPICState *s = opaque;
>>> +
>>> + if (level) {
>>> + set_bit(irq, (void *)&s->irq_pending[irq/32]);
>> set_bit(irq % 32, ...)
>>
>
> OK
No, it is wrong,
irq/32 is right.
>
>>> + }
>> Is this supposed to set either fiq_pending or irq_pending depending on
>> the select bit?
>>
>
> Yes, but, I as I stated before, maybe I will take NMI later.
>>> + sunxi_pic_update(s);
>>> +}
>>> +
>>> +static uint64_t sunxi_pic_read(void *opaque, hwaddr offset,
>>> unsigned size)
>>> +{
>>> + SunxiPICState *s = opaque;
>>> + uint8_t index = (offset& 0xc)/4;
>>> +
>>> + switch (offset) {
>>> + case SUNXI_PIC_VECTOR:
>>> + return s->vector;
>>> + break;
>>> + case SUNXI_PIC_BASE_ADDR:
>>> + return s->base_addr;
>>> + break;
>>> + case SUNXI_PIC_PROTECT:
>>> + return s->protect;
>>> + break;
>>> + case SUNXI_PIC_NMI:
>>> + return s->nmi;
>>> + break;
>>> + case SUNXI_PIC_IRQ_PENDING ... SUNXI_PIC_IRQ_PENDING + 8:
>>> + return s->irq_pending[index];
>>> + break;
>>> + case SUNXI_PIC_FIQ_PENDING ... SUNXI_PIC_FIQ_PENDING + 8:
>>> + return s->fiq_pending[index];
>>> + break;
>>> + case SUNXI_PIC_SELECT ... SUNXI_PIC_SELECT + 8:
>>> + return s->select[index];
>>> + break;
>>> + case SUNXI_PIC_ENABLE ... SUNXI_PIC_ENABLE + 8:
>>> + return s->enable[index];
>>> + break;
>>> + case SUNXI_PIC_MASK ... SUNXI_PIC_MASK + 8:
>>> + return s->mask[index];
>>> + break;
>>> + default:
>>> + break;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void sunxi_pic_write(void *opaque, hwaddr offset, uint64_t
>>> value,
>>> + unsigned size)
>>> +{
>>> + SunxiPICState *s = opaque;
>>> + uint8_t index = (offset& 0xc)/4;
>>> +
>>> + switch (offset) {
>>> + case SUNXI_PIC_VECTOR:
>>> + s->vector = value& ~0x3;
>>> + break;
>>> + case SUNXI_PIC_BASE_ADDR:
>>> + s->base_addr = value& ~0x3;
>>> + case SUNXI_PIC_PROTECT:
>>> + s->protect = value;
>>> + break;
>>> + case SUNXI_PIC_NMI:
>>> + s->nmi = value;
>>> + break;
>>> + case SUNXI_PIC_IRQ_PENDING ... SUNXI_PIC_IRQ_PENDING + 8:
>>> + s->irq_pending[index]&= ~value;
>>> + break;
>>> + case SUNXI_PIC_FIQ_PENDING ... SUNXI_PIC_FIQ_PENDING + 8:
>>> + s->fiq_pending[index]&= ~value;
>>> + break;
>>> + case SUNXI_PIC_SELECT ... SUNXI_PIC_SELECT + 8:
>>> + s->select[index] = value;
>>> + break;
>>> + case SUNXI_PIC_ENABLE ... SUNXI_PIC_ENABLE + 8:
>>> + s->enable[index] = value;
>>> + break;
>>> + case SUNXI_PIC_MASK ... SUNXI_PIC_MASK + 8:
>>> + s->mask[index] = value;
>>> + break;
>>> + default:
>>> + break;
>>> + }
>>> +
>>> + sunxi_pic_update(s);
>>> +}
>>> +
>>> +static const MemoryRegionOps sunxi_pic_ops = {
>>> + .read = sunxi_pic_read,
>>> + .write = sunxi_pic_write,
>>> + .endianness = DEVICE_NATIVE_ENDIAN,
>>> +};
>>> +
>>> +static const VMStateDescription vmstate_sunxi_pic = {
>>> + .name = "sunxi.pic",
>>> + .version_id = 1,
>>> + .minimum_version_id = 1,
>>> + .minimum_version_id_old = 1,
>>> + .fields = (VMStateField[]) {
>>> + VMSTATE_UINT32(vector, SunxiPICState),
>>> + VMSTATE_UINT32(base_addr, SunxiPICState),
>>> + VMSTATE_UINT32(protect, SunxiPICState),
>>> + VMSTATE_UINT32(nmi, SunxiPICState),
>>> + VMSTATE_UINT32_ARRAY(irq_pending, SunxiPICState,
>>> SUNXI_PIC_REG_IDX),
>>> + VMSTATE_UINT32_ARRAY(fiq_pending, SunxiPICState,
>>> SUNXI_PIC_REG_IDX),
>>> + VMSTATE_UINT32_ARRAY(enable, SunxiPICState,
>>> SUNXI_PIC_REG_IDX),
>>> + VMSTATE_UINT32_ARRAY(select, SunxiPICState,
>>> SUNXI_PIC_REG_IDX),
>>> + VMSTATE_UINT32_ARRAY(mask, SunxiPICState, SUNXI_PIC_REG_IDX),
>>> + VMSTATE_END_OF_LIST()
>>> + }
>>> +};
>>> +
>>> +static void sunxi_pic_realize(DeviceState *ds, Error **errp)
>>> +{
>>> + SunxiPICState *s = SUNXI_PIC(ds);
>>> + SysBusDevice *dev = SYS_BUS_DEVICE(ds);
>>> +
>>> + qdev_init_gpio_in(DEVICE(dev), sunxi_pic_set_irq,
>>> SUNXI_PIC_INT_NR);
>>> + sysbus_init_irq(dev,&s->parent_irq);
>>> + sysbus_init_irq(dev,&s->parent_fiq);
>>> + memory_region_init_io(&s->iomem, OBJECT(s),&sunxi_pic_ops, s,
>>> + "sunxi-pic", 0x400);
>>> + sysbus_init_mmio(dev,&s->iomem);
>>> +}
>>> +
>>> +static void sunxi_pic_reset(DeviceState *d)
>>> +{
>>> + SunxiPICState *s = SUNXI_PIC(d);
>>> + uint8_t i;
>>> +
>>> + s->base_addr = 0;
>>> + s->protect = 0;
>>> + s->nmi = 0;
>>> + s->vector = 0;
>>> + for (i = 0; i< SUNXI_PIC_REG_IDX; i++) {
>>> + s->irq_pending[i] = 0;
>>> + s->fiq_pending[i] = 0;
>>> + s->select[i] = 0;
>>> + s->enable[i] = 0;
>>> + s->mask[i] = 0;
>>> + }
>>> +}
>>> +
>>> +static void sunxi_pic_class_init(ObjectClass *klass, void *data)
>>> +{
>>> + DeviceClass *dc = DEVICE_CLASS(klass);
>>> +
>>> + dc->realize = sunxi_pic_realize;
>>> + dc->reset = sunxi_pic_reset;
>>> + dc->desc = "sunxi pic";
>>> + dc->vmsd =&vmstate_sunxi_pic;
>>> + }
>>> +
>>> +static const TypeInfo sunxi_pic_info = {
>>> + .name = TYPE_SUNXI_PIC,
>>> + .parent = TYPE_SYS_BUS_DEVICE,
>>> + .instance_size = sizeof(SunxiPICState),
>>> + .class_init = sunxi_pic_class_init,
>>> +};
>>> +
>>> +static void sunxi_register_types(void)
>>> +{
>>> + type_register_static(&sunxi_pic_info);
>>> +}
>>> +
>>> +type_init(sunxi_register_types);
>>> diff --git a/include/hw/intc/sunxi-pic.h b/include/hw/intc/sunxi-pic.h
>>> new file mode 100644
>>> index 0000000..d52b53c
>>> --- /dev/null
>>> +++ b/include/hw/intc/sunxi-pic.h
>>> @@ -0,0 +1,20 @@
>>> +#ifndef SUNXI_PIC_H
>>> +#define SUNXI_PIC_H
>>> +
>>> +#define TYPE_SUNXI_PIC "sunxi-pic"
>>> +#define SUNXI_PIC(obj) OBJECT_CHECK(SunxiPICState, (obj),
>>> TYPE_SUNXI_PIC)
>>> +
>>> +#define SUNXI_PIC_VECTOR 0
>>> +#define SUNXI_PIC_BASE_ADDR 4
>>> +#define SUNXI_PIC_PROTECT 8
>>> +#define SUNXI_PIC_NMI 0xc
>>> +#define SUNXI_PIC_IRQ_PENDING 0x10
>>> +#define SUNXI_PIC_FIQ_PENDING 0x20
>>> +#define SUNXI_PIC_SELECT 0x30
>>> +#define SUNXI_PIC_ENABLE 0x40
>>> +#define SUNXI_PIC_MASK 0x50
>>> +
>>> +#define SUNXI_PIC_INT_NR 95
>> Is this constant or configurable? Should it perhaps be a static prop?
>
> NO, it's constant
>>> +#define SUNXI_PIC_REG_IDX (SUNXI_PIC_INT_NR / 32 + 1)
>>> +
>> DIV_ROUND_UP is probably the intention here.
>>
>>
> right, will fix
>
> thank you so much!
>
>>> +#endif
>>> --
>>> 1.7.2.5
>>>
>>>
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 2/4] hw/intc: add sunxi interrupt controller device
2013-11-27 3:36 ` Li Guang
@ 2013-11-27 5:31 ` Peter Crosthwaite
2013-11-27 5:44 ` Li Guang
0 siblings, 1 reply; 33+ messages in thread
From: Peter Crosthwaite @ 2013-11-27 5:31 UTC (permalink / raw)
To: Li Guang; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Wed, Nov 27, 2013 at 1:36 PM, Li Guang <lig.fnst@cn.fujitsu.com> wrote:
> Li Guang wrote:
>>
>> Peter Crosthwaite wrote:
>>>
>>> On Tue, Nov 26, 2013 at 5:22 PM, liguang<lig.fnst@cn.fujitsu.com> wrote:
>>>>
>>>> Signed-off-by: liguang<lig.fnst@cn.fujitsu.com>
>>>> ---
>>>> default-configs/arm-softmmu.mak | 1 +
>>>> hw/intc/Makefile.objs | 1 +
>>>> hw/intc/sunxi-pic.c | 238
>>>> +++++++++++++++++++++++++++++++++++++++
>>>> include/hw/intc/sunxi-pic.h | 20 ++++
>>>> +
>>>> +static void sunxi_pic_set_irq(void *opaque, int irq, int level)
>>>> +{
>>>> + SunxiPICState *s = opaque;
>>>> +
>>>> + if (level) {
>>>> + set_bit(irq, (void *)&s->irq_pending[irq/32]);
>>>
>>> set_bit(irq % 32, ...)
>>>
>>
>> OK
>
>
> No, it is wrong,
> irq/32 is right.
>
The irq/32 is right I agree. This issue is the first arugment.
Shouln't the whole thing be:
set_bit(irq%32, (void *)&s->irq_pending[irq/32]);
Regards,
Peter
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 2/4] hw/intc: add sunxi interrupt controller device
2013-11-27 5:31 ` Peter Crosthwaite
@ 2013-11-27 5:44 ` Li Guang
0 siblings, 0 replies; 33+ messages in thread
From: Li Guang @ 2013-11-27 5:44 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: Peter Maydell, qemu-devel@nongnu.org Developers
Peter Crosthwaite wrote:
> On Wed, Nov 27, 2013 at 1:36 PM, Li Guang<lig.fnst@cn.fujitsu.com> wrote:
>
>> Li Guang wrote:
>>
>>> Peter Crosthwaite wrote:
>>>
>>>> On Tue, Nov 26, 2013 at 5:22 PM, liguang<lig.fnst@cn.fujitsu.com> wrote:
>>>>
>>>>> Signed-off-by: liguang<lig.fnst@cn.fujitsu.com>
>>>>> ---
>>>>> default-configs/arm-softmmu.mak | 1 +
>>>>> hw/intc/Makefile.objs | 1 +
>>>>> hw/intc/sunxi-pic.c | 238
>>>>> +++++++++++++++++++++++++++++++++++++++
>>>>> include/hw/intc/sunxi-pic.h | 20 ++++
>>>>>
>
>>>>> +
>>>>> +static void sunxi_pic_set_irq(void *opaque, int irq, int level)
>>>>> +{
>>>>> + SunxiPICState *s = opaque;
>>>>> +
>>>>> + if (level) {
>>>>> + set_bit(irq, (void *)&s->irq_pending[irq/32]);
>>>>>
>>>> set_bit(irq % 32, ...)
>>>>
>>>>
>>> OK
>>>
>>
>> No, it is wrong,
>> irq/32 is right.
>>
>>
> The irq/32 is right I agree. This issue is the first arugment.
> Shouln't the whole thing be:
>
> set_bit(irq%32, (void *)&s->irq_pending[irq/32]);
>
>
>
OK, fix like this
diff --git a/hw/intc/sunxi-pic.c b/hw/intc/sunxi-pic.c
index 5fd86f9..ea75f84 100644
--- a/hw/intc/sunxi-pic.c
+++ b/hw/intc/sunxi-pic.c
@@ -77,7 +77,7 @@ static void sunxi_pic_set_irq(void *opaque, int irq,
int level)
SunxiPICState *s = opaque;
if (level) {
- set_bit(irq, (void *)&s->irq_pending[irq/32]);
+ set_bit(irq%32, (void *)&s->irq_pending[irq/32]);
}
sunxi_pic_update(s);
thanks!
Li Guang
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type
2013-11-26 7:22 [Qemu-devel] [PATCH v4 0/4] add sunxi machine type liguang
2013-11-26 7:22 ` [Qemu-devel] [PATCH v4 1/4] hw/timer: add sunxi timer device liguang
2013-11-26 7:22 ` [Qemu-devel] [PATCH v4 2/4] hw/intc: add sunxi interrupt controller device liguang
@ 2013-11-26 7:22 ` liguang
2013-11-26 9:22 ` Peter Crosthwaite
2013-11-26 7:22 ` [Qemu-devel] [PATCH v4 4/4] MAINTAINERS: add myself to maintain sunxi machine liguang
3 siblings, 1 reply; 33+ messages in thread
From: liguang @ 2013-11-26 7:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, liguang
Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
---
hw/arm/Makefile.objs | 1 +
hw/arm/sunxi-soc.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 99 insertions(+), 0 deletions(-)
create mode 100644 hw/arm/sunxi-soc.c
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 3671b42..f9f3071 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -5,3 +5,4 @@ obj-y += tosa.o versatilepb.o vexpress.o xilinx_zynq.o z2.o
obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
obj-y += omap1.o omap2.o strongarm.o
+obj-y += sunxi-soc.o
diff --git a/hw/arm/sunxi-soc.c b/hw/arm/sunxi-soc.c
new file mode 100644
index 0000000..b45af6d
--- /dev/null
+++ b/hw/arm/sunxi-soc.c
@@ -0,0 +1,98 @@
+/*
+ * Allwinner sunxi series SoC emulation
+ *
+ * Copyright (C) 2013 Li Guang
+ * Written by Li Guang <lig.fnst@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/devices.h"
+#include "hw/boards.h"
+#include "hw/arm/arm.h"
+#include "hw/ptimer.h"
+#include "hw/char/serial.h"
+#include "hw/timer/sunxi-pit.h"
+#include "hw/intc/sunxi-pic.h"
+
+#include "sysemu/sysemu.h"
+#include "exec/address-spaces.h"
+
+
+#define SUNXI_PIC_REG_BASE 0x01c20400
+#define SUNXI_PIT_REG_BASE 0x01c20c00
+#define SUNXI_UART0_REG_BASE 0x01c28000
+
+static struct arm_boot_info sunxi_binfo = {
+ .loader_start = 0x40000000,
+ .board_id = 0x1008,
+};
+
+static void sunxi_init(QEMUMachineInitArgs *args)
+{
+ ram_addr_t ram_size = args->ram_size;
+ const char *cpu_model = args->cpu_model;
+ const char *kernel_filename = args->kernel_filename;
+ const char *kernel_cmdline = args->kernel_cmdline;
+ ARMCPU *cpu;
+ MemoryRegion *address_space_mem = get_system_memory();
+ MemoryRegion *ram = g_new(MemoryRegion, 1);
+ MemoryRegion *ram_alias = g_new(MemoryRegion, 1);
+ qemu_irq pic[95];
+ DeviceState *dev;
+ uint8_t i;
+
+ /*here we currently support sunxi-4i*/
+ cpu_model = "cortex-a8";
+ cpu = cpu_arm_init(cpu_model);
+ if (!cpu) {
+ fprintf(stderr, "Unable to find CPU definition\n");
+ exit(1);
+ }
+
+ memory_region_init_ram(ram, NULL, "sunxi-soc.ram", ram_size);
+ memory_region_add_subregion(address_space_mem, 0, ram);
+ memory_region_init_alias(ram_alias, NULL, "ram.alias", ram, 0, ram_size);
+ memory_region_add_subregion(address_space_mem, 0x40000000, ram_alias);
+
+ dev = sysbus_create_varargs(TYPE_SUNXI_PIC, SUNXI_PIC_REG_BASE,
+ qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ),
+ qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_FIQ),
+ NULL);
+ for (i = 0; i < SUNXI_PIC_INT_NR; i++) {
+ pic[i] = qdev_get_gpio_in(dev, i);
+ }
+
+ sysbus_create_varargs(TYPE_SUNXI_PIT, SUNXI_PIT_REG_BASE, pic[22], pic[23],
+ pic[24], pic[25], pic[67], pic[68], NULL);
+
+ serial_mm_init(address_space_mem, SUNXI_UART0_REG_BASE, 2, pic[1], 115200,
+ serial_hds[0], DEVICE_NATIVE_ENDIAN);
+
+ sunxi_binfo.ram_size = ram_size;
+ sunxi_binfo.kernel_filename = kernel_filename;
+ sunxi_binfo.kernel_cmdline = kernel_cmdline;
+ arm_load_kernel(cpu, &sunxi_binfo);
+}
+
+static QEMUMachine sunxi_machine = {
+ .name = "sunxi",
+ .desc = "Allwinner's SoC (sunxi series)",
+ .init = sunxi_init,
+};
+
+static void sunxi_machine_init(void)
+{
+ qemu_register_machine(&sunxi_machine);
+}
+
+machine_init(sunxi_machine_init);
--
1.7.2.5
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type
2013-11-26 7:22 ` [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type liguang
@ 2013-11-26 9:22 ` Peter Crosthwaite
2013-11-27 0:22 ` Li Guang
2013-11-27 9:22 ` Andreas Färber
0 siblings, 2 replies; 33+ messages in thread
From: Peter Crosthwaite @ 2013-11-26 9:22 UTC (permalink / raw)
To: liguang, Andreas Färber
Cc: Peter Maydell, qemu-devel@nongnu.org Developers
On Tue, Nov 26, 2013 at 5:22 PM, liguang <lig.fnst@cn.fujitsu.com> wrote:
> Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> ---
> hw/arm/Makefile.objs | 1 +
> hw/arm/sunxi-soc.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 99 insertions(+), 0 deletions(-)
> create mode 100644 hw/arm/sunxi-soc.c
>
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index 3671b42..f9f3071 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -5,3 +5,4 @@ obj-y += tosa.o versatilepb.o vexpress.o xilinx_zynq.o z2.o
>
> obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
> obj-y += omap1.o omap2.o strongarm.o
> +obj-y += sunxi-soc.o
> diff --git a/hw/arm/sunxi-soc.c b/hw/arm/sunxi-soc.c
> new file mode 100644
> index 0000000..b45af6d
> --- /dev/null
> +++ b/hw/arm/sunxi-soc.c
> @@ -0,0 +1,98 @@
> +/*
> + * Allwinner sunxi series SoC emulation
> + *
> + * Copyright (C) 2013 Li Guang
> + * Written by Li Guang <lig.fnst@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/devices.h"
> +#include "hw/boards.h"
> +#include "hw/arm/arm.h"
> +#include "hw/ptimer.h"
> +#include "hw/char/serial.h"
> +#include "hw/timer/sunxi-pit.h"
> +#include "hw/intc/sunxi-pic.h"
> +
> +#include "sysemu/sysemu.h"
> +#include "exec/address-spaces.h"
> +
> +
> +#define SUNXI_PIC_REG_BASE 0x01c20400
> +#define SUNXI_PIT_REG_BASE 0x01c20c00
> +#define SUNXI_UART0_REG_BASE 0x01c28000
> +
> +static struct arm_boot_info sunxi_binfo = {
> + .loader_start = 0x40000000,
> + .board_id = 0x1008,
> +};
> +
> +static void sunxi_init(QEMUMachineInitArgs *args)
I would check with Andreas/PMM on what the go is with SoCs regarding
container devices and boards. My (vague) understanding is that SoCs
should be container devices and boards instantiate those containers
with off-chip connectivity. This seems flat to me, with everything on
board level.
> +{
> + ram_addr_t ram_size = args->ram_size;
> + const char *cpu_model = args->cpu_model;
> + const char *kernel_filename = args->kernel_filename;
> + const char *kernel_cmdline = args->kernel_cmdline;
> + ARMCPU *cpu;
> + MemoryRegion *address_space_mem = get_system_memory();
> + MemoryRegion *ram = g_new(MemoryRegion, 1);
> + MemoryRegion *ram_alias = g_new(MemoryRegion, 1);
> + qemu_irq pic[95];
[SUNXI_PIC_INT_NR]
Regards,
Peter
> + DeviceState *dev;
> + uint8_t i;
> +
> + /*here we currently support sunxi-4i*/
> + cpu_model = "cortex-a8";
> + cpu = cpu_arm_init(cpu_model);
> + if (!cpu) {
> + fprintf(stderr, "Unable to find CPU definition\n");
> + exit(1);
> + }
> +
> + memory_region_init_ram(ram, NULL, "sunxi-soc.ram", ram_size);
> + memory_region_add_subregion(address_space_mem, 0, ram);
> + memory_region_init_alias(ram_alias, NULL, "ram.alias", ram, 0, ram_size);
> + memory_region_add_subregion(address_space_mem, 0x40000000, ram_alias);
> +
> + dev = sysbus_create_varargs(TYPE_SUNXI_PIC, SUNXI_PIC_REG_BASE,
> + qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ),
> + qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_FIQ),
> + NULL);
> + for (i = 0; i < SUNXI_PIC_INT_NR; i++) {
> + pic[i] = qdev_get_gpio_in(dev, i);
> + }
> +
> + sysbus_create_varargs(TYPE_SUNXI_PIT, SUNXI_PIT_REG_BASE, pic[22], pic[23],
> + pic[24], pic[25], pic[67], pic[68], NULL);
> +
> + serial_mm_init(address_space_mem, SUNXI_UART0_REG_BASE, 2, pic[1], 115200,
> + serial_hds[0], DEVICE_NATIVE_ENDIAN);
> +
> + sunxi_binfo.ram_size = ram_size;
> + sunxi_binfo.kernel_filename = kernel_filename;
> + sunxi_binfo.kernel_cmdline = kernel_cmdline;
> + arm_load_kernel(cpu, &sunxi_binfo);
> +}
> +
> +static QEMUMachine sunxi_machine = {
> + .name = "sunxi",
> + .desc = "Allwinner's SoC (sunxi series)",
> + .init = sunxi_init,
> +};
> +
> +static void sunxi_machine_init(void)
> +{
> + qemu_register_machine(&sunxi_machine);
> +}
> +
> +machine_init(sunxi_machine_init);
> --
> 1.7.2.5
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type
2013-11-26 9:22 ` Peter Crosthwaite
@ 2013-11-27 0:22 ` Li Guang
2013-11-27 9:22 ` Andreas Färber
1 sibling, 0 replies; 33+ messages in thread
From: Li Guang @ 2013-11-27 0:22 UTC (permalink / raw)
To: Peter Crosthwaite
Cc: Peter Maydell, Andreas Färber,
qemu-devel@nongnu.org Developers
Peter Crosthwaite wrote:
> On Tue, Nov 26, 2013 at 5:22 PM, liguang<lig.fnst@cn.fujitsu.com> wrote:
>
>> Signed-off-by: liguang<lig.fnst@cn.fujitsu.com>
>> ---
>> hw/arm/Makefile.objs | 1 +
>> hw/arm/sunxi-soc.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 99 insertions(+), 0 deletions(-)
>> create mode 100644 hw/arm/sunxi-soc.c
>>
>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>> index 3671b42..f9f3071 100644
>> --- a/hw/arm/Makefile.objs
>> +++ b/hw/arm/Makefile.objs
>> @@ -5,3 +5,4 @@ obj-y += tosa.o versatilepb.o vexpress.o xilinx_zynq.o z2.o
>>
>> obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
>> obj-y += omap1.o omap2.o strongarm.o
>> +obj-y += sunxi-soc.o
>> diff --git a/hw/arm/sunxi-soc.c b/hw/arm/sunxi-soc.c
>> new file mode 100644
>> index 0000000..b45af6d
>> --- /dev/null
>> +++ b/hw/arm/sunxi-soc.c
>> @@ -0,0 +1,98 @@
>> +/*
>> + * Allwinner sunxi series SoC emulation
>> + *
>> + * Copyright (C) 2013 Li Guang
>> + * Written by Li Guang<lig.fnst@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/devices.h"
>> +#include "hw/boards.h"
>> +#include "hw/arm/arm.h"
>> +#include "hw/ptimer.h"
>> +#include "hw/char/serial.h"
>> +#include "hw/timer/sunxi-pit.h"
>> +#include "hw/intc/sunxi-pic.h"
>> +
>> +#include "sysemu/sysemu.h"
>> +#include "exec/address-spaces.h"
>> +
>> +
>> +#define SUNXI_PIC_REG_BASE 0x01c20400
>> +#define SUNXI_PIT_REG_BASE 0x01c20c00
>> +#define SUNXI_UART0_REG_BASE 0x01c28000
>> +
>> +static struct arm_boot_info sunxi_binfo = {
>> + .loader_start = 0x40000000,
>> + .board_id = 0x1008,
>> +};
>> +
>> +static void sunxi_init(QEMUMachineInitArgs *args)
>>
> I would check with Andreas/PMM on what the go is with SoCs regarding
> container devices and boards. My (vague) understanding is that SoCs
> should be container devices and boards instantiate those containers
> with off-chip connectivity. This seems flat to me, with everything on
> board level.
>
>
well, interesting thought.
IMO, SoC is a board.
>> +{
>> + ram_addr_t ram_size = args->ram_size;
>> + const char *cpu_model = args->cpu_model;
>> + const char *kernel_filename = args->kernel_filename;
>> + const char *kernel_cmdline = args->kernel_cmdline;
>> + ARMCPU *cpu;
>> + MemoryRegion *address_space_mem = get_system_memory();
>> + MemoryRegion *ram = g_new(MemoryRegion, 1);
>> + MemoryRegion *ram_alias = g_new(MemoryRegion, 1);
>> + qemu_irq pic[95];
>>
> [SUNXI_PIC_INT_NR]
>
>
>
yes, will fix.
Thanks!
Li Guang
>
>> + DeviceState *dev;
>> + uint8_t i;
>> +
>> + /*here we currently support sunxi-4i*/
>> + cpu_model = "cortex-a8";
>> + cpu = cpu_arm_init(cpu_model);
>> + if (!cpu) {
>> + fprintf(stderr, "Unable to find CPU definition\n");
>> + exit(1);
>> + }
>> +
>> + memory_region_init_ram(ram, NULL, "sunxi-soc.ram", ram_size);
>> + memory_region_add_subregion(address_space_mem, 0, ram);
>> + memory_region_init_alias(ram_alias, NULL, "ram.alias", ram, 0, ram_size);
>> + memory_region_add_subregion(address_space_mem, 0x40000000, ram_alias);
>> +
>> + dev = sysbus_create_varargs(TYPE_SUNXI_PIC, SUNXI_PIC_REG_BASE,
>> + qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_IRQ),
>> + qdev_get_gpio_in(DEVICE(cpu), ARM_CPU_FIQ),
>> + NULL);
>> + for (i = 0; i< SUNXI_PIC_INT_NR; i++) {
>> + pic[i] = qdev_get_gpio_in(dev, i);
>> + }
>> +
>> + sysbus_create_varargs(TYPE_SUNXI_PIT, SUNXI_PIT_REG_BASE, pic[22], pic[23],
>> + pic[24], pic[25], pic[67], pic[68], NULL);
>> +
>> + serial_mm_init(address_space_mem, SUNXI_UART0_REG_BASE, 2, pic[1], 115200,
>> + serial_hds[0], DEVICE_NATIVE_ENDIAN);
>> +
>> + sunxi_binfo.ram_size = ram_size;
>> + sunxi_binfo.kernel_filename = kernel_filename;
>> + sunxi_binfo.kernel_cmdline = kernel_cmdline;
>> + arm_load_kernel(cpu,&sunxi_binfo);
>> +}
>> +
>> +static QEMUMachine sunxi_machine = {
>> + .name = "sunxi",
>> + .desc = "Allwinner's SoC (sunxi series)",
>> + .init = sunxi_init,
>> +};
>> +
>> +static void sunxi_machine_init(void)
>> +{
>> + qemu_register_machine(&sunxi_machine);
>> +}
>> +
>> +machine_init(sunxi_machine_init);
>> --
>> 1.7.2.5
>>
>>
>>
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type
2013-11-26 9:22 ` Peter Crosthwaite
2013-11-27 0:22 ` Li Guang
@ 2013-11-27 9:22 ` Andreas Färber
2013-11-27 9:27 ` Andreas Färber
1 sibling, 1 reply; 33+ messages in thread
From: Andreas Färber @ 2013-11-27 9:22 UTC (permalink / raw)
To: liguang; +Cc: Peter Maydell, Peter Crosthwaite, qemu-devel, Bamvor Jian Zhang
Hi,
Am 26.11.2013 10:22, schrieb Peter Crosthwaite:
> On Tue, Nov 26, 2013 at 5:22 PM, liguang <lig.fnst@cn.fujitsu.com> wrote:
>> Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
>> ---
>> hw/arm/Makefile.objs | 1 +
>> hw/arm/sunxi-soc.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 99 insertions(+), 0 deletions(-)
>> create mode 100644 hw/arm/sunxi-soc.c
>>
>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>> index 3671b42..f9f3071 100644
>> --- a/hw/arm/Makefile.objs
>> +++ b/hw/arm/Makefile.objs
>> @@ -5,3 +5,4 @@ obj-y += tosa.o versatilepb.o vexpress.o xilinx_zynq.o z2.o
>>
>> obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
>> obj-y += omap1.o omap2.o strongarm.o
>> +obj-y += sunxi-soc.o
>> diff --git a/hw/arm/sunxi-soc.c b/hw/arm/sunxi-soc.c
>> new file mode 100644
>> index 0000000..b45af6d
>> --- /dev/null
>> +++ b/hw/arm/sunxi-soc.c
>> @@ -0,0 +1,98 @@
>> +/*
>> + * Allwinner sunxi series SoC emulation
>> + *
>> + * Copyright (C) 2013 Li Guang
>> + * Written by Li Guang <lig.fnst@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/devices.h"
>> +#include "hw/boards.h"
>> +#include "hw/arm/arm.h"
>> +#include "hw/ptimer.h"
>> +#include "hw/char/serial.h"
>> +#include "hw/timer/sunxi-pit.h"
>> +#include "hw/intc/sunxi-pic.h"
>> +
>> +#include "sysemu/sysemu.h"
>> +#include "exec/address-spaces.h"
>> +
>> +
>> +#define SUNXI_PIC_REG_BASE 0x01c20400
>> +#define SUNXI_PIT_REG_BASE 0x01c20c00
>> +#define SUNXI_UART0_REG_BASE 0x01c28000
>> +
>> +static struct arm_boot_info sunxi_binfo = {
>> + .loader_start = 0x40000000,
>> + .board_id = 0x1008,
>> +};
>> +
>> +static void sunxi_init(QEMUMachineInitArgs *args)
>
> I would check with Andreas/PMM on what the go is with SoCs regarding
> container devices and boards. My (vague) understanding is that SoCs
> should be container devices and boards instantiate those containers
> with off-chip connectivity. This seems flat to me, with everything on
> board level.
Yes, thanks, that matches what I was going to comment. But I think it's
even more complicated: To my understanding, "sunxi" is the name of a
community effort [1] to clean up and upstream the BSP kernels from
Allwinner, so it sounds as if this was an attempt to write an emulation
for that kernel family while naming everything "sunxi" when in fact the
SoCs are called Axx [2] (with A1x = sun4i, A2x = sun5i, A3x = sun6i but
no literal "sunxi" AFAIK) and boards include Cubieboard, Cubieboard2,
Cubieboard3/Cubietruck [3] and whatever tablets etc. are out there.
(CC'ing Bamvor)
That's a lesson we learned from the old "prep" machine: Please name
things after real hardware, only then can it later be verified whether
the modeling is actually correct or which changes need to be performed.
A practical aspect of modeling SoCs correctly is that they can more
easily be reused across boards or modules, and you don't need to mess
with machine-level cpu_model if you have a fixed SoC-CPU mapping.
You may want to consult the recent DIGIC or earlier Faraday series or my
Tegra2 repository for examples of how to implement this paradigm.
I believe the composition tree naming wrt "cortex" and the MPCore was
still open, hopefully PMM can comment on his current preferences.
And thanks for your efforts, from a distribution viewpoint I am looking
forward to testing our kernels and images with this.
Regards,
Andreas
[1] http://linux-sunxi.org/Main_Page
[2] http://www.allwinnertech.com/en/product/A-Serial.html
[3] http://cubieboard.org/
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type
2013-11-27 9:22 ` Andreas Färber
@ 2013-11-27 9:27 ` Andreas Färber
2013-11-29 0:46 ` Li Guang
0 siblings, 1 reply; 33+ messages in thread
From: Andreas Färber @ 2013-11-27 9:27 UTC (permalink / raw)
To: liguang; +Cc: Peter Maydell, Peter Crosthwaite, qemu-devel, Bamvor Jian Zhang
Am 27.11.2013 10:22, schrieb Andreas Färber:
> Hi,
>
> Am 26.11.2013 10:22, schrieb Peter Crosthwaite:
>> On Tue, Nov 26, 2013 at 5:22 PM, liguang <lig.fnst@cn.fujitsu.com> wrote:
>>> Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
>>> ---
>>> hw/arm/Makefile.objs | 1 +
>>> hw/arm/sunxi-soc.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 99 insertions(+), 0 deletions(-)
>>> create mode 100644 hw/arm/sunxi-soc.c
>>>
>>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>>> index 3671b42..f9f3071 100644
>>> --- a/hw/arm/Makefile.objs
>>> +++ b/hw/arm/Makefile.objs
>>> @@ -5,3 +5,4 @@ obj-y += tosa.o versatilepb.o vexpress.o xilinx_zynq.o z2.o
>>>
>>> obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
>>> obj-y += omap1.o omap2.o strongarm.o
>>> +obj-y += sunxi-soc.o
>>> diff --git a/hw/arm/sunxi-soc.c b/hw/arm/sunxi-soc.c
>>> new file mode 100644
>>> index 0000000..b45af6d
>>> --- /dev/null
>>> +++ b/hw/arm/sunxi-soc.c
>>> @@ -0,0 +1,98 @@
>>> +/*
>>> + * Allwinner sunxi series SoC emulation
>>> + *
>>> + * Copyright (C) 2013 Li Guang
>>> + * Written by Li Guang <lig.fnst@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/devices.h"
>>> +#include "hw/boards.h"
>>> +#include "hw/arm/arm.h"
>>> +#include "hw/ptimer.h"
>>> +#include "hw/char/serial.h"
>>> +#include "hw/timer/sunxi-pit.h"
>>> +#include "hw/intc/sunxi-pic.h"
>>> +
>>> +#include "sysemu/sysemu.h"
>>> +#include "exec/address-spaces.h"
>>> +
>>> +
>>> +#define SUNXI_PIC_REG_BASE 0x01c20400
>>> +#define SUNXI_PIT_REG_BASE 0x01c20c00
>>> +#define SUNXI_UART0_REG_BASE 0x01c28000
>>> +
>>> +static struct arm_boot_info sunxi_binfo = {
>>> + .loader_start = 0x40000000,
>>> + .board_id = 0x1008,
>>> +};
>>> +
>>> +static void sunxi_init(QEMUMachineInitArgs *args)
>>
>> I would check with Andreas/PMM on what the go is with SoCs regarding
>> container devices and boards. My (vague) understanding is that SoCs
>> should be container devices and boards instantiate those containers
>> with off-chip connectivity. This seems flat to me, with everything on
>> board level.
>
> Yes, thanks, that matches what I was going to comment. But I think it's
> even more complicated: To my understanding, "sunxi" is the name of a
> community effort [1] to clean up and upstream the BSP kernels from
> Allwinner, so it sounds as if this was an attempt to write an emulation
> for that kernel family while naming everything "sunxi" when in fact the
> SoCs are called Axx [2] (with A1x = sun4i, A2x = sun5i, A3x = sun6i but
My interpolation was incorrect: A10 = sun4i, A13 = sun5i, A3x = sun6i,
A20 = sun7i
Andreas
> no literal "sunxi" AFAIK) and boards include Cubieboard, Cubieboard2,
> Cubieboard3/Cubietruck [3] and whatever tablets etc. are out there.
> (CC'ing Bamvor)
>
> That's a lesson we learned from the old "prep" machine: Please name
> things after real hardware, only then can it later be verified whether
> the modeling is actually correct or which changes need to be performed.
>
> A practical aspect of modeling SoCs correctly is that they can more
> easily be reused across boards or modules, and you don't need to mess
> with machine-level cpu_model if you have a fixed SoC-CPU mapping.
>
> You may want to consult the recent DIGIC or earlier Faraday series or my
> Tegra2 repository for examples of how to implement this paradigm.
> I believe the composition tree naming wrt "cortex" and the MPCore was
> still open, hopefully PMM can comment on his current preferences.
>
> And thanks for your efforts, from a distribution viewpoint I am looking
> forward to testing our kernels and images with this.
>
> Regards,
> Andreas
>
> [1] http://linux-sunxi.org/Main_Page
> [2] http://www.allwinnertech.com/en/product/A-Serial.html
> [3] http://cubieboard.org/
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type
2013-11-27 9:27 ` Andreas Färber
@ 2013-11-29 0:46 ` Li Guang
2013-11-29 0:53 ` Peter Crosthwaite
2013-11-29 3:27 ` Andreas Färber
0 siblings, 2 replies; 33+ messages in thread
From: Li Guang @ 2013-11-29 0:46 UTC (permalink / raw)
To: Andreas Färber
Cc: Peter Maydell, Peter Crosthwaite, qemu-devel, Bamvor Jian Zhang
Andreas Färber wrote:
> Am 27.11.2013 10:22, schrieb Andreas Färber:
>
>> Hi,
>>
>> Am 26.11.2013 10:22, schrieb Peter Crosthwaite:
>>
>>> On Tue, Nov 26, 2013 at 5:22 PM, liguang<lig.fnst@cn.fujitsu.com> wrote:
>>>
>>>> Signed-off-by: liguang<lig.fnst@cn.fujitsu.com>
>>>> ---
>>>> hw/arm/Makefile.objs | 1 +
>>>> hw/arm/sunxi-soc.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>> 2 files changed, 99 insertions(+), 0 deletions(-)
>>>> create mode 100644 hw/arm/sunxi-soc.c
>>>>
>>>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>>>> index 3671b42..f9f3071 100644
>>>> --- a/hw/arm/Makefile.objs
>>>> +++ b/hw/arm/Makefile.objs
>>>> @@ -5,3 +5,4 @@ obj-y += tosa.o versatilepb.o vexpress.o xilinx_zynq.o z2.o
>>>>
>>>> obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
>>>> obj-y += omap1.o omap2.o strongarm.o
>>>> +obj-y += sunxi-soc.o
>>>> diff --git a/hw/arm/sunxi-soc.c b/hw/arm/sunxi-soc.c
>>>> new file mode 100644
>>>> index 0000000..b45af6d
>>>> --- /dev/null
>>>> +++ b/hw/arm/sunxi-soc.c
>>>> @@ -0,0 +1,98 @@
>>>> +/*
>>>> + * Allwinner sunxi series SoC emulation
>>>> + *
>>>> + * Copyright (C) 2013 Li Guang
>>>> + * Written by Li Guang<lig.fnst@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/devices.h"
>>>> +#include "hw/boards.h"
>>>> +#include "hw/arm/arm.h"
>>>> +#include "hw/ptimer.h"
>>>> +#include "hw/char/serial.h"
>>>> +#include "hw/timer/sunxi-pit.h"
>>>> +#include "hw/intc/sunxi-pic.h"
>>>> +
>>>> +#include "sysemu/sysemu.h"
>>>> +#include "exec/address-spaces.h"
>>>> +
>>>> +
>>>> +#define SUNXI_PIC_REG_BASE 0x01c20400
>>>> +#define SUNXI_PIT_REG_BASE 0x01c20c00
>>>> +#define SUNXI_UART0_REG_BASE 0x01c28000
>>>> +
>>>> +static struct arm_boot_info sunxi_binfo = {
>>>> + .loader_start = 0x40000000,
>>>> + .board_id = 0x1008,
>>>> +};
>>>> +
>>>> +static void sunxi_init(QEMUMachineInitArgs *args)
>>>>
>>> I would check with Andreas/PMM on what the go is with SoCs regarding
>>> container devices and boards. My (vague) understanding is that SoCs
>>> should be container devices and boards instantiate those containers
>>> with off-chip connectivity. This seems flat to me, with everything on
>>> board level.
>>>
>> Yes, thanks, that matches what I was going to comment. But I think it's
>> even more complicated: To my understanding, "sunxi" is the name of a
>> community effort [1] to clean up and upstream the BSP kernels from
>> Allwinner, so it sounds as if this was an attempt to write an emulation
>> for that kernel family while naming everything "sunxi" when in fact the
>> SoCs are called Axx [2] (with A1x = sun4i, A2x = sun5i, A3x = sun6i but
>>
> My interpolation was incorrect: A10 = sun4i, A13 = sun5i, A3x = sun6i,
> A20 = sun7i
>
> Andreas
>
>
>> no literal "sunxi" AFAIK) and boards include Cubieboard, Cubieboard2,
>> Cubieboard3/Cubietruck [3] and whatever tablets etc. are out there.
>> (CC'ing Bamvor)
>>
>> That's a lesson we learned from the old "prep" machine: Please name
>> things after real hardware, only then can it later be verified whether
>> the modeling is actually correct or which changes need to be performed.
>>
>>
well, sunxi maybe be representation of Axx series,
but, what's wrong?
we can't track Axx hardware changes? why?
and also, this patch-set is also community effort just like
sunxi in linux kernel.
>> A practical aspect of modeling SoCs correctly is that they can more
>> easily be reused across boards or modules, and you don't need to mess
>> with machine-level cpu_model if you have a fixed SoC-CPU mapping.
>>
modeling SoC is good, but
sorry, I can't assure that fixed mapping.
>> You may want to consult the recent DIGIC or earlier Faraday series or my
>> Tegra2 repository for examples of how to implement this paradigm.
>> I believe the composition tree naming wrt "cortex" and the MPCore was
>> still open, hopefully PMM can comment on his current preferences.
>>
>> And thanks for your efforts, from a distribution viewpoint I am looking
>> forward to testing our kernels and images with this.
>>
currently, I can only provide linux kernel build for sunxi-4i,
where I can up-load it to?
>> Regards,
>> Andreas
>>
>> [1] http://linux-sunxi.org/Main_Page
>> [2] http://www.allwinnertech.com/en/product/A-Serial.html
>>
this page is can't accessed for me.
Thanks for your comment!
Li Guang
>> [3] http://cubieboard.org/
>>
>>
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type
2013-11-29 0:46 ` Li Guang
@ 2013-11-29 0:53 ` Peter Crosthwaite
2013-11-29 3:27 ` Andreas Färber
1 sibling, 0 replies; 33+ messages in thread
From: Peter Crosthwaite @ 2013-11-29 0:53 UTC (permalink / raw)
To: Li Guang
Cc: Peter Maydell, Bamvor Jian Zhang, Andreas Färber, qemu-devel
On Fri, Nov 29, 2013 at 10:46 AM, Li Guang <lig.fnst@cn.fujitsu.com> wrote:
> Andreas Färber wrote:
>>
>> Am 27.11.2013 10:22, schrieb Andreas Färber:
>>
>>>
>>> Hi,
>>>
>>> Am 26.11.2013 10:22, schrieb Peter Crosthwaite:
>>>
>>>>
>>>> On Tue, Nov 26, 2013 at 5:22 PM, liguang<lig.fnst@cn.fujitsu.com>
>>>> wrote:
>>>>
>>>>>
>>>>> Signed-off-by: liguang<lig.fnst@cn.fujitsu.com>
>>>>> ---
>>>>> hw/arm/Makefile.objs | 1 +
>>>>> hw/arm/sunxi-soc.c | 98
>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>> 2 files changed, 99 insertions(+), 0 deletions(-)
>>>>> create mode 100644 hw/arm/sunxi-soc.c
>>>>>
>>>>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>>>>> index 3671b42..f9f3071 100644
>>>>> --- a/hw/arm/Makefile.objs
>>>>> +++ b/hw/arm/Makefile.objs
>>>>> @@ -5,3 +5,4 @@ obj-y += tosa.o versatilepb.o vexpress.o xilinx_zynq.o
>>>>> z2.o
>>>>>
>>>>> obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
>>>>> obj-y += omap1.o omap2.o strongarm.o
>>>>> +obj-y += sunxi-soc.o
>>>>> diff --git a/hw/arm/sunxi-soc.c b/hw/arm/sunxi-soc.c
>>>>> new file mode 100644
>>>>> index 0000000..b45af6d
>>>>> --- /dev/null
>>>>> +++ b/hw/arm/sunxi-soc.c
>>>>> @@ -0,0 +1,98 @@
>>>>> +/*
>>>>> + * Allwinner sunxi series SoC emulation
>>>>> + *
>>>>> + * Copyright (C) 2013 Li Guang
>>>>> + * Written by Li Guang<lig.fnst@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/devices.h"
>>>>> +#include "hw/boards.h"
>>>>> +#include "hw/arm/arm.h"
>>>>> +#include "hw/ptimer.h"
>>>>> +#include "hw/char/serial.h"
>>>>> +#include "hw/timer/sunxi-pit.h"
>>>>> +#include "hw/intc/sunxi-pic.h"
>>>>> +
>>>>> +#include "sysemu/sysemu.h"
>>>>> +#include "exec/address-spaces.h"
>>>>> +
>>>>> +
>>>>> +#define SUNXI_PIC_REG_BASE 0x01c20400
>>>>> +#define SUNXI_PIT_REG_BASE 0x01c20c00
>>>>> +#define SUNXI_UART0_REG_BASE 0x01c28000
>>>>> +
>>>>> +static struct arm_boot_info sunxi_binfo = {
>>>>> + .loader_start = 0x40000000,
>>>>> + .board_id = 0x1008,
>>>>> +};
>>>>> +
>>>>> +static void sunxi_init(QEMUMachineInitArgs *args)
>>>>>
>>>>
>>>> I would check with Andreas/PMM on what the go is with SoCs regarding
>>>> container devices and boards. My (vague) understanding is that SoCs
>>>> should be container devices and boards instantiate those containers
>>>> with off-chip connectivity. This seems flat to me, with everything on
>>>> board level.
>>>>
>>>
>>> Yes, thanks, that matches what I was going to comment. But I think it's
>>> even more complicated: To my understanding, "sunxi" is the name of a
>>> community effort [1] to clean up and upstream the BSP kernels from
>>> Allwinner, so it sounds as if this was an attempt to write an emulation
>>> for that kernel family while naming everything "sunxi" when in fact the
>>> SoCs are called Axx [2] (with A1x = sun4i, A2x = sun5i, A3x = sun6i but
>>>
>>
>> My interpolation was incorrect: A10 = sun4i, A13 = sun5i, A3x = sun6i,
>> A20 = sun7i
>>
>> Andreas
>>
>>
>>>
>>> no literal "sunxi" AFAIK) and boards include Cubieboard, Cubieboard2,
>>> Cubieboard3/Cubietruck [3] and whatever tablets etc. are out there.
>>> (CC'ing Bamvor)
>>>
>>> That's a lesson we learned from the old "prep" machine: Please name
>>> things after real hardware, only then can it later be verified whether
>>> the modeling is actually correct or which changes need to be performed.
>>>
>>>
>
>
> well, sunxi maybe be representation of Axx series,
> but, what's wrong?
> we can't track Axx hardware changes? why?
> and also, this patch-set is also community effort just like
> sunxi in linux kernel.
>
>
>>> A practical aspect of modeling SoCs correctly is that they can more
>>> easily be reused across boards or modules, and you don't need to mess
>>> with machine-level cpu_model if you have a fixed SoC-CPU mapping.
>>>
>
>
> modeling SoC is good, but
> sorry, I can't assure that fixed mapping.
>
How does such variation occur, is it between different SoCs or board
configurable (tieoffs, bootrom etc)?
Regards,
Peter
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type
2013-11-29 0:46 ` Li Guang
2013-11-29 0:53 ` Peter Crosthwaite
@ 2013-11-29 3:27 ` Andreas Färber
2013-11-29 8:06 ` Li Guang
1 sibling, 1 reply; 33+ messages in thread
From: Andreas Färber @ 2013-11-29 3:27 UTC (permalink / raw)
To: Li Guang
Cc: Peter Maydell, Peter Crosthwaite, Hervé Poussineau,
qemu-devel, Bamvor Jian Zhang
Am 29.11.2013 01:46, schrieb Li Guang:
> Andreas Färber wrote:
>> Am 27.11.2013 10:22, schrieb Andreas Färber:
>>
>>> Hi,
>>>
>>> Am 26.11.2013 10:22, schrieb Peter Crosthwaite:
>>>
>>>> On Tue, Nov 26, 2013 at 5:22 PM, liguang<lig.fnst@cn.fujitsu.com>
>>>> wrote:
>>>>
>>>>> Signed-off-by: liguang<lig.fnst@cn.fujitsu.com>
>>>>> ---
>>>>> hw/arm/Makefile.objs | 1 +
>>>>> hw/arm/sunxi-soc.c | 98
>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>> 2 files changed, 99 insertions(+), 0 deletions(-)
>>>>> create mode 100644 hw/arm/sunxi-soc.c
>>>>>
>>>>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>>>>> index 3671b42..f9f3071 100644
>>>>> --- a/hw/arm/Makefile.objs
>>>>> +++ b/hw/arm/Makefile.objs
>>>>> @@ -5,3 +5,4 @@ obj-y += tosa.o versatilepb.o vexpress.o
>>>>> xilinx_zynq.o z2.o
>>>>>
>>>>> obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
>>>>> obj-y += omap1.o omap2.o strongarm.o
>>>>> +obj-y += sunxi-soc.o
>>>>> diff --git a/hw/arm/sunxi-soc.c b/hw/arm/sunxi-soc.c
>>>>> new file mode 100644
>>>>> index 0000000..b45af6d
>>>>> --- /dev/null
>>>>> +++ b/hw/arm/sunxi-soc.c
>>>>> @@ -0,0 +1,98 @@
>>>>> +/*
>>>>> + * Allwinner sunxi series SoC emulation
>>>>> + *
>>>>> + * Copyright (C) 2013 Li Guang
>>>>> + * Written by Li Guang<lig.fnst@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/devices.h"
>>>>> +#include "hw/boards.h"
>>>>> +#include "hw/arm/arm.h"
>>>>> +#include "hw/ptimer.h"
>>>>> +#include "hw/char/serial.h"
>>>>> +#include "hw/timer/sunxi-pit.h"
>>>>> +#include "hw/intc/sunxi-pic.h"
>>>>> +
>>>>> +#include "sysemu/sysemu.h"
>>>>> +#include "exec/address-spaces.h"
>>>>> +
>>>>> +
>>>>> +#define SUNXI_PIC_REG_BASE 0x01c20400
>>>>> +#define SUNXI_PIT_REG_BASE 0x01c20c00
>>>>> +#define SUNXI_UART0_REG_BASE 0x01c28000
>>>>> +
>>>>> +static struct arm_boot_info sunxi_binfo = {
>>>>> + .loader_start = 0x40000000,
>>>>> + .board_id = 0x1008,
>>>>> +};
>>>>> +
>>>>> +static void sunxi_init(QEMUMachineInitArgs *args)
>>>>>
>>>> I would check with Andreas/PMM on what the go is with SoCs regarding
>>>> container devices and boards. My (vague) understanding is that SoCs
>>>> should be container devices and boards instantiate those containers
>>>> with off-chip connectivity. This seems flat to me, with everything on
>>>> board level.
>>>>
>>> Yes, thanks, that matches what I was going to comment. But I think it's
>>> even more complicated: To my understanding, "sunxi" is the name of a
>>> community effort [1] to clean up and upstream the BSP kernels from
>>> Allwinner, so it sounds as if this was an attempt to write an emulation
>>> for that kernel family while naming everything "sunxi" when in fact the
>>> SoCs are called Axx [2] (with A1x = sun4i, A2x = sun5i, A3x = sun6i but
>>>
>> My interpolation was incorrect: A10 = sun4i, A13 = sun5i, A3x = sun6i,
>> A20 = sun7i
>>
>> Andreas
>>
>>
>>> no literal "sunxi" AFAIK) and boards include Cubieboard, Cubieboard2,
>>> Cubieboard3/Cubietruck [3] and whatever tablets etc. are out there.
>>> (CC'ing Bamvor)
>>>
>>> That's a lesson we learned from the old "prep" machine: Please name
>>> things after real hardware, only then can it later be verified whether
>>> the modeling is actually correct or which changes need to be performed.
>>>
>>>
>
> well, sunxi maybe be representation of Axx series,
> but, what's wrong?
You're modeling too general IMO and thereby you're creating a
virtual-only machine (despite parallel efforts by Linaro to introduce
mach-virt for that purpose). Please model an actual piece of hardware -
SoC and board - and not something random that happens to run with the
"sunxi" kernel flavor but will leave us puzzled in the future. Should be
pretty easy to avoid.
My example was qemu-system-ppc -M prep. Today no one knows what hardware
that was supposed to match (possibly none) because there are a number of
different PReP based machines from IBM and Motorola out there; switching
from OpenHack'Ware to OpenBIOS became difficult because among other
things we don't have a device tree dump from a physical machine to
compare to, and Hervé thus set out to create new machines such as 40P
where we actually know which components the hardware contains rather
than which drivers are available in the kernel and happened to have
matching QEMU device implementations at the time.
A slightly similar problem occurred with -M pc, where we now have an
i440fx based one and the new q35 based one. It's easier to abstract
commonalities and share code between different devices/machines than
turning a generic machine/device into a less generic one, in particular
for backwards compatibility for guests, command line and QMP.
When the difference between two devices is just a value or an offset,
then you can use static properties to set them and have the realize
function take them into account. If the composition tree differs
significantly or if you want to facilitate reuse, then different types
will be needed. Multiple machines can call a shared helper function with
some parameter; examples include PC, Versatile Express and DIGIC.
> we can't track Axx hardware changes? why?
Sorry, I don't get that? The Sunxi, Allwinner and Wikipedia pages all
document some key differences, in particular Cortex-A8 in A10/A13 vs.
Cortex-A7 in A20/A31. Cortex-A7 has MPCore, which drags along some key
differences that cannot easily fit in a single SunxiState SoC device.
At least from my understanding of Cortex-A9 and Cortex-A15 being much
closer than Cortex-A8, that is. For example, you have your own PIC for
the Cortex-A8 in this series whereas Cortex-A7 will use ARM's GIC and
may be able to reuse the "a15mpcore_priv" composite device.
http://en.wikipedia.org/wiki/List_of_ARM_microprocessor_cores#Designed_by_ARM
> and also, this patch-set is also community effort just like
> sunxi in linux kernel.
My whole point is, try to design the model forward from hardware and
less backwards from kernel. Whether it's sun4i or A10 is less relevant.
Kernels may contain bugs. Hardware doesn't change except for new revs,
but definitely not depending on who writes a kernel to run on it. :)
>>> A practical aspect of modeling SoCs correctly is that they can more
>>> easily be reused across boards or modules, and you don't need to mess
>>> with machine-level cpu_model if you have a fixed SoC-CPU mapping.
>>>
>
> modeling SoC is good, but
> sorry, I can't assure that fixed mapping.
See above. A10 / sun4i => Cortex-A8, that's fixed, and then you can
properly embed the ARMCPU in an A10State/Sun4iState without pointer and
using object_initialize().
It is your approach of a single "sunxi" machine and SunxiState that's
interfering with a fixed mapping AFAICT. Otherwise you'll need to
explain more verbose why the mapping is not assured, please.
QOM uses a strict composition model. If you choose the physical board
you have, say a Gooseberry board, then modeling should be so that we use
qemu-system-arm -M gooseberry (without -cpu cortex-a8)
and /machine has-a child<allwinner-a10> "a10"
which in turn has-a child<cortex-a8-arm-cpu> "cpu".
-M cubieboard and -M marsboard can then all reuse the allwinner-a10 SoC
device, and in the future you can then tweak CPU properties via QMP
after TypeInfo::instance_init and before DeviceClass::realize.
-M cubieboard2 /machine by contrast has-a child<allwinner-a20> "a20"
which has-a child<cortex-a7-arm-cpu> "cpu[0]",
has-a child<cortex-a7-arm-cpu> "cpu[1]".
Like I said below, Peter Maydell should be able to guide you in more
detail for the exact naming and composition.
>>> You may want to consult the recent DIGIC or earlier Faraday series or my
>>> Tegra2 repository for examples of how to implement this paradigm.
>>> I believe the composition tree naming wrt "cortex" and the MPCore was
>>> still open, hopefully PMM can comment on his current preferences.
>>>
>>> And thanks for your efforts, from a distribution viewpoint I am looking
>>> forward to testing our kernels and images with this.
>>>
>
> currently, I can only provide linux kernel build for sunxi-4i,
> where I can up-load it to?
I recall Faraday using Google Drive, for instance.
openSUSE seems to provide some sun4i and sun5i kernel RPMs here:
https://build.opensuse.org/project/show/devel:ARM:12.3:Contrib:sunxi
http://download.opensuse.org/repositories/devel:/ARM:/12.3:/Contrib:/sunxi/ports/armv7hl/
>>> [1] http://linux-sunxi.org/Main_Page
>>> [2] http://www.allwinnertech.com/en/product/A-Serial.html
>>>
>
> this page is can't accessed for me.
Works for me ATM, so either a temporary problem or firewall issue...
It provides a table of the SoCs, mapping names to CPU, GPU, etc.
Cf. http://en.wikipedia.org/wiki/Allwinner_Technology#A-Series
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type
2013-11-29 3:27 ` Andreas Färber
@ 2013-11-29 8:06 ` Li Guang
2013-11-29 8:31 ` Peter Maydell
` (2 more replies)
0 siblings, 3 replies; 33+ messages in thread
From: Li Guang @ 2013-11-29 8:06 UTC (permalink / raw)
To: Andreas Färber
Cc: Peter Maydell, Peter Crosthwaite, Hervé Poussineau,
qemu-devel, Bamvor Jian Zhang
Andreas Färber wrote:
> Am 29.11.2013 01:46, schrieb Li Guang:
>
>> Andreas Färber wrote:
>>
>>> Am 27.11.2013 10:22, schrieb Andreas Färber:
>>>
>>>
>>>> Hi,
>>>>
>>>> Am 26.11.2013 10:22, schrieb Peter Crosthwaite:
>>>>
>>>>
>>>>> On Tue, Nov 26, 2013 at 5:22 PM, liguang<lig.fnst@cn.fujitsu.com>
>>>>> wrote:
>>>>>
>>>>>
>>>>>> Signed-off-by: liguang<lig.fnst@cn.fujitsu.com>
>>>>>> ---
>>>>>> hw/arm/Makefile.objs | 1 +
>>>>>> hw/arm/sunxi-soc.c | 98
>>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>> 2 files changed, 99 insertions(+), 0 deletions(-)
>>>>>> create mode 100644 hw/arm/sunxi-soc.c
>>>>>>
>>>>>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>>>>>> index 3671b42..f9f3071 100644
>>>>>> --- a/hw/arm/Makefile.objs
>>>>>> +++ b/hw/arm/Makefile.objs
>>>>>> @@ -5,3 +5,4 @@ obj-y += tosa.o versatilepb.o vexpress.o
>>>>>> xilinx_zynq.o z2.o
>>>>>>
>>>>>> obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
>>>>>> obj-y += omap1.o omap2.o strongarm.o
>>>>>> +obj-y += sunxi-soc.o
>>>>>> diff --git a/hw/arm/sunxi-soc.c b/hw/arm/sunxi-soc.c
>>>>>> new file mode 100644
>>>>>> index 0000000..b45af6d
>>>>>> --- /dev/null
>>>>>> +++ b/hw/arm/sunxi-soc.c
>>>>>> @@ -0,0 +1,98 @@
>>>>>> +/*
>>>>>> + * Allwinner sunxi series SoC emulation
>>>>>> + *
>>>>>> + * Copyright (C) 2013 Li Guang
>>>>>> + * Written by Li Guang<lig.fnst@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/devices.h"
>>>>>> +#include "hw/boards.h"
>>>>>> +#include "hw/arm/arm.h"
>>>>>> +#include "hw/ptimer.h"
>>>>>> +#include "hw/char/serial.h"
>>>>>> +#include "hw/timer/sunxi-pit.h"
>>>>>> +#include "hw/intc/sunxi-pic.h"
>>>>>> +
>>>>>> +#include "sysemu/sysemu.h"
>>>>>> +#include "exec/address-spaces.h"
>>>>>> +
>>>>>> +
>>>>>> +#define SUNXI_PIC_REG_BASE 0x01c20400
>>>>>> +#define SUNXI_PIT_REG_BASE 0x01c20c00
>>>>>> +#define SUNXI_UART0_REG_BASE 0x01c28000
>>>>>> +
>>>>>> +static struct arm_boot_info sunxi_binfo = {
>>>>>> + .loader_start = 0x40000000,
>>>>>> + .board_id = 0x1008,
>>>>>> +};
>>>>>> +
>>>>>> +static void sunxi_init(QEMUMachineInitArgs *args)
>>>>>>
>>>>>>
>>>>> I would check with Andreas/PMM on what the go is with SoCs regarding
>>>>> container devices and boards. My (vague) understanding is that SoCs
>>>>> should be container devices and boards instantiate those containers
>>>>> with off-chip connectivity. This seems flat to me, with everything on
>>>>> board level.
>>>>>
>>>>>
>>>> Yes, thanks, that matches what I was going to comment. But I think it's
>>>> even more complicated: To my understanding, "sunxi" is the name of a
>>>> community effort [1] to clean up and upstream the BSP kernels from
>>>> Allwinner, so it sounds as if this was an attempt to write an emulation
>>>> for that kernel family while naming everything "sunxi" when in fact the
>>>> SoCs are called Axx [2] (with A1x = sun4i, A2x = sun5i, A3x = sun6i but
>>>>
>>>>
>>> My interpolation was incorrect: A10 = sun4i, A13 = sun5i, A3x = sun6i,
>>> A20 = sun7i
>>>
>>> Andreas
>>>
>>>
>>>
>>>> no literal "sunxi" AFAIK) and boards include Cubieboard, Cubieboard2,
>>>> Cubieboard3/Cubietruck [3] and whatever tablets etc. are out there.
>>>> (CC'ing Bamvor)
>>>>
>>>> That's a lesson we learned from the old "prep" machine: Please name
>>>> things after real hardware, only then can it later be verified whether
>>>> the modeling is actually correct or which changes need to be performed.
>>>>
>>>>
>>>>
>> well, sunxi maybe be representation of Axx series,
>> but, what's wrong?
>>
> You're modeling too general IMO and thereby you're creating a
> virtual-only machine (despite parallel efforts by Linaro to introduce
> mach-virt for that purpose). Please model an actual piece of hardware -
> SoC and board - and not something random that happens to run with the
> "sunxi" kernel flavor but will leave us puzzled in the future. Should be
> pretty easy to avoid.
>
> My example was qemu-system-ppc -M prep. Today no one knows what hardware
> that was supposed to match (possibly none) because there are a number of
> different PReP based machines from IBM and Motorola out there; switching
> from OpenHack'Ware to OpenBIOS became difficult because among other
> things we don't have a device tree dump from a physical machine to
> compare to, and Hervé thus set out to create new machines such as 40P
> where we actually know which components the hardware contains rather
> than which drivers are available in the kernel and happened to have
> matching QEMU device implementations at the time.
> A slightly similar problem occurred with -M pc, where we now have an
> i440fx based one and the new q35 based one. It's easier to abstract
> commonalities and share code between different devices/machines than
> turning a generic machine/device into a less generic one, in particular
> for backwards compatibility for guests, command line and QMP.
>
> When the difference between two devices is just a value or an offset,
> then you can use static properties to set them and have the realize
> function take them into account. If the composition tree differs
> significantly or if you want to facilitate reuse, then different types
> will be needed. Multiple machines can call a shared helper function with
> some parameter; examples include PC, Versatile Express and DIGIC.
>
>
>> we can't track Axx hardware changes? why?
>>
> Sorry, I don't get that? The Sunxi, Allwinner and Wikipedia pages all
> document some key differences, in particular Cortex-A8 in A10/A13 vs.
> Cortex-A7 in A20/A31. Cortex-A7 has MPCore, which drags along some key
> differences that cannot easily fit in a single SunxiState SoC device.
>
right, A10/20... seem have similar devices except CPU
> At least from my understanding of Cortex-A9 and Cortex-A15 being much
> closer than Cortex-A8, that is. For example, you have your own PIC for
> the Cortex-A8 in this series whereas Cortex-A7 will use ARM's GIC and
> may be able to reuse the "a15mpcore_priv" composite device.
> http://en.wikipedia.org/wiki/List_of_ARM_microprocessor_cores#Designed_by_ARM
>
>
>> and also, this patch-set is also community effort just like
>> sunxi in linux kernel.
>>
> My whole point is, try to design the model forward from hardware and
> less backwards from kernel. Whether it's sun4i or A10 is less relevant.
> Kernels may contain bugs. Hardware doesn't change except for new revs,
> but definitely not depending on who writes a kernel to run on it. :)
>
>
of course, I am aiming to emulate the real hardware,
so name is not the problem, right?
>>>> A practical aspect of modeling SoCs correctly is that they can more
>>>> easily be reused across boards or modules, and you don't need to mess
>>>> with machine-level cpu_model if you have a fixed SoC-CPU mapping.
>>>>
>>>>
>> modeling SoC is good, but
>> sorry, I can't assure that fixed mapping.
>>
> See above. A10 / sun4i => Cortex-A8, that's fixed, and then you can
> properly embed the ARMCPU in an A10State/Sun4iState without pointer and
> using object_initialize().
>
> It is your approach of a single "sunxi" machine and SunxiState that's
> interfering with a fixed mapping AFAICT. Otherwise you'll need to
> explain more verbose why the mapping is not assured, please.
>
I mean, e.g. A10 and A13 are different only on HDMI-transmitter and
SATA-controller,
but we have to have Sun4iState, and Sun5iState, I think.
what I design is:
we have a sunxi series as a machine, then
for sunx4i, we specify -M sunxi -cpu cortex-a8 -device x1 ...
for sunx5i, we specify -M sunxi -cpu cortex-a8 -device x2 ...
for sunx7i, we specify -M sunxi -cpu cortex-a7 -devcie x3 ...
for cubieboard, we specify -M sunxi -cpu -cortex-a8 -device x1 -device
p1 ...
> QOM uses a strict composition model. If you choose the physical board
> you have, say a Gooseberry board, then modeling should be so that we use
> qemu-system-arm -M gooseberry (without -cpu cortex-a8)
> and /machine has-a child<allwinner-a10> "a10"
> which in turn has-a child<cortex-a8-arm-cpu> "cpu".
> -M cubieboard and -M marsboard can then all reuse the allwinner-a10 SoC
> device, and in the future you can then tweak CPU properties via QMP
> after TypeInfo::instance_init and before DeviceClass::realize.
> -M cubieboard2 /machine by contrast has-a child<allwinner-a20> "a20"
> which has-a child<cortex-a7-arm-cpu> "cpu[0]",
> has-a child<cortex-a7-arm-cpu> "cpu[1]".
>
> Like I said below, Peter Maydell should be able to guide you in more
> detail for the exact naming and composition.
>
>
>>>> You may want to consult the recent DIGIC or earlier Faraday series or my
>>>> Tegra2 repository for examples of how to implement this paradigm.
>>>> I believe the composition tree naming wrt "cortex" and the MPCore was
>>>> still open, hopefully PMM can comment on his current preferences.
>>>>
>>>> And thanks for your efforts, from a distribution viewpoint I am looking
>>>> forward to testing our kernels and images with this.
>>>>
>>>>
>> currently, I can only provide linux kernel build for sunxi-4i,
>> where I can up-load it to?
>>
> I recall Faraday using Google Drive, for instance.
>
> openSUSE seems to provide some sun4i and sun5i kernel RPMs here:
> https://build.opensuse.org/project/show/devel:ARM:12.3:Contrib:sunxi
> http://download.opensuse.org/repositories/devel:/ARM:/12.3:/Contrib:/sunxi/ports/armv7hl/
>
>
tried to attach zImage in mail,
but, seems failed.
I will try other ways like google drive.
>>>> [1] http://linux-sunxi.org/Main_Page
>>>> [2] http://www.allwinnertech.com/en/product/A-Serial.html
>>>>
>>>>
>> this page is can't accessed for me.
>>
> Works for me ATM, so either a temporary problem or firewall issue...
> It provides a table of the SoCs, mapping names to CPU, GPU, etc.
>
> Cf. http://en.wikipedia.org/wiki/Allwinner_Technology#A-Series
>
>
>
OK.
Thanks!
Li Guang
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type
2013-11-29 8:06 ` Li Guang
@ 2013-11-29 8:31 ` Peter Maydell
2013-11-29 8:49 ` Bamvor Jian Zhang
2013-11-29 8:56 ` Li Guang
2013-11-29 8:41 ` Bamvor Jian Zhang
2013-11-29 13:04 ` Andreas Färber
2 siblings, 2 replies; 33+ messages in thread
From: Peter Maydell @ 2013-11-29 8:31 UTC (permalink / raw)
To: Li Guang
Cc: Peter Crosthwaite, Bamvor Jian Zhang, Hervé Poussineau,
Andreas Färber, qemu-devel
On 29 November 2013 08:06, Li Guang <lig.fnst@cn.fujitsu.com> wrote:
> what I design is:
> we have a sunxi series as a machine, then
> for sunx4i, we specify -M sunxi -cpu cortex-a8 -device x1 ...
> for sunx5i, we specify -M sunxi -cpu cortex-a8 -device x2 ...
> for sunx7i, we specify -M sunxi -cpu cortex-a7 -devcie x3 ...
> for cubieboard, we specify -M sunxi -cpu -cortex-a8 -device x1 -device p1
No, QEMU doesn't work this way. "-M whatever" specifies a board
model, so in this example it should be "-M cubieboard" and so on.
That then gives you a particular CPU and set of devices. Obviously
where we have several board models that share a single SoC they
share implementation (by instantiating the same SoC object).
If we have several SoCs that share common subcomponents like
a UART, then they share implementation by having all those SoCs
instantiate the same UART object.
-cpu is really only intended where you have a situation like the
PC where just the CPU can be plugged and unplugged into a
board; it doesn't fit for SoC-based systems.
Similarly, -device is really (currently) for pluggable devices like
ISA or PCI cards -- where the device is a non-removable
part of the SoC it doesn't work.
As Andreas says, we need to model real actual hardware,
not some abstraction that kind of matches the kernel's
abstractions.
Is "sunxi" what the hardware is actually called, or only
what the kernel port has been called? More information
about where this name comes from might make it easier
to tell if it is the correct one for the QEMU SoC models.
thanks
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type
2013-11-29 8:31 ` Peter Maydell
@ 2013-11-29 8:49 ` Bamvor Jian Zhang
2013-11-29 13:51 ` Andreas Färber
2013-11-29 8:56 ` Li Guang
1 sibling, 1 reply; 33+ messages in thread
From: Bamvor Jian Zhang @ 2013-11-29 8:49 UTC (permalink / raw)
To: Li Guang, Peter Maydell; +Cc: Peter Crosthwaite, hpoussin, qemu-devel, afaerber
Hi,
>>>Peter Maydell <peter.maydell@linaro.org> wrote:
> On 29 November 2013 08:06, Li Guang <lig.fnst@cn.fujitsu.com> wrote:
> > what I design is:
> > we have a sunxi series as a machine, then
> > for sunx4i, we specify -M sunxi -cpu cortex-a8 -device x1 ...
> > for sunx5i, we specify -M sunxi -cpu cortex-a8 -device x2 ...
> > for sunx7i, we specify -M sunxi -cpu cortex-a7 -devcie x3 ...
> > for cubieboard, we specify -M sunxi -cpu -cortex-a8 -device x1 -device p1
>
> No, QEMU doesn't work this way. "-M whatever" specifies a board
> model, so in this example it should be "-M cubieboard" and so on.
> That then gives you a particular CPU and set of devices. Obviously
> where we have several board models that share a single SoC they
> share implementation (by instantiating the same SoC object).
> If we have several SoCs that share common subcomponents like
> a UART, then they share implementation by having all those SoCs
> instantiate the same UART object.
>
> -cpu is really only intended where you have a situation like the
> PC where just the CPU can be plugged and unplugged into a
> board; it doesn't fit for SoC-based systems.
> Similarly, -device is really (currently) for pluggable devices like
> ISA or PCI cards -- where the device is a non-removable
> part of the SoC it doesn't work.
>
> As Andreas says, we need to model real actual hardware,
> not some abstraction that kind of matches the kernel's
> abstractions.
>
> Is "sunxi" what the hardware is actually called, or only
> what the kernel port has been called? More information
> about where this name comes from might make it easier
> to tell if it is the correct one for the QEMU SoC models.
just FYI:
in the kernel source code, it usually use the sunxi-axx format, e.g. sun7i-a20, sun5i-a10s, sun5i-a13...
ref: linux/drivers/clk/sunxi/clk-sunxi.c
linux/arch/arm/boot/dts/sun*.dts
> thanks
> -- PMM
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type
2013-11-29 8:49 ` Bamvor Jian Zhang
@ 2013-11-29 13:51 ` Andreas Färber
0 siblings, 0 replies; 33+ messages in thread
From: Andreas Färber @ 2013-11-29 13:51 UTC (permalink / raw)
To: Bamvor Jian Zhang, Li Guang; +Cc: Peter Maydell, Peter Crosthwaite, qemu-devel
Hi Bamvor and Guang,
Am 29.11.2013 09:49, schrieb Bamvor Jian Zhang:
> >>>Peter Maydell <peter.maydell@linaro.org> wrote:
>> Is "sunxi" what the hardware is actually called, or only
>> what the kernel port has been called? More information
>> about where this name comes from might make it easier
>> to tell if it is the correct one for the QEMU SoC models.
> just FYI:
> in the kernel source code, it usually use the sunxi-axx format, e.g. sun7i-a20, sun5i-a10s, sun5i-a13...
> ref: linux/drivers/clk/sunxi/clk-sunxi.c
> linux/arch/arm/boot/dts/sun*.dts
I'm curious, "sun" surely has nothing to do with Sun Microsystems.
Chinese "sun" means bamboo sprout (笋) among others, right? Is there
some meaning hidden in the combination with "xi" pronounced [she] then,
or do you pronounce it [ex eye]? 西 means Western I believe, but is
usually placed before rather than after the word it describes AFAICT? :)
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type
2013-11-29 8:31 ` Peter Maydell
2013-11-29 8:49 ` Bamvor Jian Zhang
@ 2013-11-29 8:56 ` Li Guang
2013-11-29 9:11 ` Peter Maydell
1 sibling, 1 reply; 33+ messages in thread
From: Li Guang @ 2013-11-29 8:56 UTC (permalink / raw)
To: Peter Maydell
Cc: qemu-devel, Peter Crosthwaite, Hervé Poussineau,
Andreas Färber, Bamvor Jian Zhang
Peter Maydell wrote:
> On 29 November 2013 08:06, Li Guang<lig.fnst@cn.fujitsu.com> wrote:
>
>> what I design is:
>> we have a sunxi series as a machine, then
>> for sunx4i, we specify -M sunxi -cpu cortex-a8 -device x1 ...
>> for sunx5i, we specify -M sunxi -cpu cortex-a8 -device x2 ...
>> for sunx7i, we specify -M sunxi -cpu cortex-a7 -devcie x3 ...
>> for cubieboard, we specify -M sunxi -cpu -cortex-a8 -device x1 -device p1
>>
> No, QEMU doesn't work this way. "-M whatever" specifies a board
> model, so in this example it should be "-M cubieboard" and so on.
> That then gives you a particular CPU and set of devices. Obviously
> where we have several board models that share a single SoC they
> share implementation (by instantiating the same SoC object).
> If we have several SoCs that share common subcomponents like
> a UART, then they share implementation by having all those SoCs
> instantiate the same UART object.
>
> -cpu is really only intended where you have a situation like the
> PC where just the CPU can be plugged and unplugged into a
> board; it doesn't fit for SoC-based systems.
> Similarly, -device is really (currently) for pluggable devices like
> ISA or PCI cards -- where the device is a non-removable
> part of the SoC it doesn't work.
>
why not just say this SoC is a board?
and other board like cubieboard are only
this SoC + several devices,
I think is reasonable, at least in this case.
A10 and A13 both have a cortex-a8, different in HDMI and SATA,
suppose we modeled A10, A10State,
if we add cubieboard, we realize A10,
then we have a board called demoboard based on A13,
what we will do here?
also realize A10? unlucky, we miss HDMI and SATA difference,
model A13? new a A13State?
but we have most devices the same for A10 & A13.
> As Andreas says, we need to model real actual hardware,
> not some abstraction that kind of matches the kernel's
> abstractions.
>
I never aimed to do what you said abstraction,
I just specified a represented of real hardware.
> Is "sunxi" what the hardware is actually called, or only
> what the kernel port has been called? More information
> about where this name comes from might make it easier
> to tell if it is the correct one for the QEMU SoC models.
>
>
>
I tried to contact Allwinner's engineer,
no response until now.
Thanks!
Li Guang
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type
2013-11-29 8:56 ` Li Guang
@ 2013-11-29 9:11 ` Peter Maydell
0 siblings, 0 replies; 33+ messages in thread
From: Peter Maydell @ 2013-11-29 9:11 UTC (permalink / raw)
To: Li Guang
Cc: qemu-devel, Peter Crosthwaite, Hervé Poussineau,
Andreas Färber, Bamvor Jian Zhang
On 29 November 2013 08:56, Li Guang <lig.fnst@cn.fujitsu.com> wrote:
> why not just say this SoC is a board?
> and other board like cubieboard are only
> this SoC + several devices,
> I think is reasonable, at least in this case.
Because it's not in general how we prefer to model
things in QEMU. When something goes into upstream
it means we then have to maintain it and keeping things
consistent with everything else is an important part of that.
> A10 and A13 both have a cortex-a8, different in HDMI and SATA,
> suppose we modeled A10, A10State,
> if we add cubieboard, we realize A10,
> then we have a board called demoboard based on A13,
> what we will do here?
> also realize A10? unlucky, we miss HDMI and SATA difference,
> model A13? new a A13State?
> but we have most devices the same for A10 & A13.
Yes, in that case you'd have board models for cubieboard
and demoboard, SoC models for A10 and A13, and
device models for all the components of A10 and A13.
That seems an entirely straightforward approach. If
A10 and A13 hardware share the same devices mostly
then the QEMU models of them would also share the
same device models: the top level SoC model generally
just creates a bunch of device models and wires them up.
>> Is "sunxi" what the hardware is actually called, or only
>> what the kernel port has been called? More information
>> about where this name comes from might make it easier
>> to tell if it is the correct one for the QEMU SoC models.
> I tried to contact Allwinner's engineer,
> no response until now.
Well, at the moment I have no idea if "sunxi" is
a name that comes from within Allwinner or if it's
just some codename you guys have picked.
-- PMM
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type
2013-11-29 8:06 ` Li Guang
2013-11-29 8:31 ` Peter Maydell
@ 2013-11-29 8:41 ` Bamvor Jian Zhang
2013-11-29 9:01 ` Li Guang
2013-11-29 13:04 ` Andreas Färber
2 siblings, 1 reply; 33+ messages in thread
From: Bamvor Jian Zhang @ 2013-11-29 8:41 UTC (permalink / raw)
To: Li Guang, afaerber; +Cc: Peter Maydell, Peter Crosthwaite, hpoussin, qemu-devel
Hi,
>>>Li Guang <lig.fnst@cn.fujitsu.com> wrote:
> Andreas Färber wrote:
> > Am 29.11.2013 01:46, schrieb Li Guang:
> >
> >> Andreas Färber wrote:
> >>
> >>> Am 27.11.2013 10:22, schrieb Andreas Färber:
> >>>
> >>>
> >>>> Hi,
> >>>>
> >>>> Am 26.11.2013 10:22, schrieb Peter Crosthwaite:
> >>>>
> >>>>
> >>>>> On Tue, Nov 26, 2013 at 5:22 PM, liguang<lig.fnst@cn.fujitsu.com>
> >>>>> wrote:
> >>>>>
> >>>>>
> >>>>>> Signed-off-by: liguang<lig.fnst@cn.fujitsu.com>
> >>>>>> ---
> >>>>>> hw/arm/Makefile.objs | 1 +
> >>>>>> hw/arm/sunxi-soc.c | 98
> >>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++
> >>>>>> 2 files changed, 99 insertions(+), 0 deletions(-)
> >>>>>> create mode 100644 hw/arm/sunxi-soc.c
> >>>>>>
> >>>>>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> >>>>>> index 3671b42..f9f3071 100644
> >>>>>> --- a/hw/arm/Makefile.objs
> >>>>>> +++ b/hw/arm/Makefile.objs
> >>>>>> @@ -5,3 +5,4 @@ obj-y += tosa.o versatilepb.o vexpress.o
> >>>>>> xilinx_zynq.o z2.o
> >>>>>>
> >>>>>> obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
> >>>>>> obj-y += omap1.o omap2.o strongarm.o
> >>>>>> +obj-y += sunxi-soc.o
> >>>>>> diff --git a/hw/arm/sunxi-soc.c b/hw/arm/sunxi-soc.c
> >>>>>> new file mode 100644
> >>>>>> index 0000000..b45af6d
> >>>>>> --- /dev/null
> >>>>>> +++ b/hw/arm/sunxi-soc.c
> >>>>>> @@ -0,0 +1,98 @@
> >>>>>> +/*
> >>>>>> + * Allwinner sunxi series SoC emulation
> >>>>>> + *
> >>>>>> + * Copyright (C) 2013 Li Guang
> >>>>>> + * Written by Li Guang<lig.fnst@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/devices.h"
> >>>>>> +#include "hw/boards.h"
> >>>>>> +#include "hw/arm/arm.h"
> >>>>>> +#include "hw/ptimer.h"
> >>>>>> +#include "hw/char/serial.h"
> >>>>>> +#include "hw/timer/sunxi-pit.h"
> >>>>>> +#include "hw/intc/sunxi-pic.h"
> >>>>>> +
> >>>>>> +#include "sysemu/sysemu.h"
> >>>>>> +#include "exec/address-spaces.h"
> >>>>>> +
> >>>>>> +
> >>>>>> +#define SUNXI_PIC_REG_BASE 0x01c20400
> >>>>>> +#define SUNXI_PIT_REG_BASE 0x01c20c00
> >>>>>> +#define SUNXI_UART0_REG_BASE 0x01c28000
> >>>>>> +
> >>>>>> +static struct arm_boot_info sunxi_binfo = {
> >>>>>> + .loader_start = 0x40000000,
> >>>>>> + .board_id = 0x1008,
> >>>>>> +};
> >>>>>> +
> >>>>>> +static void sunxi_init(QEMUMachineInitArgs *args)
> >>>>>>
> >>>>>>
> >>>>> I would check with Andreas/PMM on what the go is with SoCs regarding
> >>>>> container devices and boards. My (vague) understanding is that SoCs
> >>>>> should be container devices and boards instantiate those containers
> >>>>> with off-chip connectivity. This seems flat to me, with everything on
> >>>>> board level.
> >>>>>
> >>>>>
> >>>> Yes, thanks, that matches what I was going to comment. But I think it's
> >>>> even more complicated: To my understanding, "sunxi" is the name of a
> >>>> community effort [1] to clean up and upstream the BSP kernels from
> >>>> Allwinner, so it sounds as if this was an attempt to write an emulation
> >>>> for that kernel family while naming everything "sunxi" when in fact the
>
>>>> SoCs are called Axx [2] (with A1x = sun4i, A2x = sun5i, A3x = sun6i but
> >>>>
> >>>>
> >>> My interpolation was incorrect: A10 = sun4i, A13 = sun5i, A3x = sun6i,
> >>> A20 = sun7i
> >>>
> >>> Andreas
> >>>
> >>>
> >>>
> >>>> no literal "sunxi" AFAIK) and boards include Cubieboard, Cubieboard2,
> >>>> Cubieboard3/Cubietruck [3] and whatever tablets etc. are out there.
> >>>> (CC'ing Bamvor)
> >>>>
> >>>> That's a lesson we learned from the old "prep" machine: Please name
> >>>> things after real hardware, only then can it later be verified whether
> >>>> the modeling is actually correct or which changes need to be performed.
> >>>>
> >>>>
> >>>>
> >> well, sunxi maybe be representation of Axx series,
> >> but, what's wrong?
> >>
> > You're modeling too general IMO and thereby you're creating a
> > virtual-only machine (despite parallel efforts by Linaro to introduce
> > mach-virt for that purpose). Please model an actual piece of hardware -
> > SoC and board - and not something random that happens to run with the
> > "sunxi" kernel flavor but will leave us puzzled in the future. Should be
> > pretty easy to avoid.
> >
> > My example was qemu-system-ppc -M prep. Today no one knows what hardware
> > that was supposed to match (possibly none) because there are a number of
> > different PReP based machines from IBM and Motorola out there; switching
> > from OpenHack'Ware to OpenBIOS became difficult because among other
> > things we don't have a device tree dump from a physical machine to
> > compare to, and Hervé thus set out to create new machines such as 40P
> > where we actually know which components the hardware contains rather
> > than which drivers are available in the kernel and happened to have
> > matching QEMU device implementations at the time.
> > A slightly similar problem occurred with -M pc, where we now have an
> > i440fx based one and the new q35 based one. It's easier to abstract
> > commonalities and share code between different devices/machines than
> > turning a generic machine/device into a less generic one, in particular
> > for backwards compatibility for guests, command line and QMP.
> >
> > When the difference between two devices is just a value or an offset,
> > then you can use static properties to set them and have the realize
> > function take them into account. If the composition tree differs
> > significantly or if you want to facilitate reuse, then different types
> > will be needed. Multiple machines can call a shared helper function with
> > some parameter; examples include PC, Versatile Express and DIGIC.
> >
> >
> >> we can't track Axx hardware changes? why?
> >>
> > Sorry, I don't get that? The Sunxi, Allwinner and Wikipedia pages all
> > document some key differences, in particular Cortex-A8 in A10/A13 vs.
> > Cortex-A7 in A20/A31. Cortex-A7 has MPCore, which drags along some key
> > differences that cannot easily fit in a single SunxiState SoC device.
> >
>
> right, A10/20... seem have similar devices except CPU
>
> > At least from my understanding of Cortex-A9 and Cortex-A15 being much
> > closer than Cortex-A8, that is. For example, you have your own PIC for
> > the Cortex-A8 in this series whereas Cortex-A7 will use ARM's GIC and
> > may be able to reuse the "a15mpcore_priv" composite device.
> >
> http://en.wikipedia.org/wiki/List_of_ARM_microprocessor_cores#Designed_by_ARM
> >
> >
> >> and also, this patch-set is also community effort just like
> >> sunxi in linux kernel.
> >>
> > My whole point is, try to design the model forward from hardware and
> > less backwards from kernel. Whether it's sun4i or A10 is less relevant.
> > Kernels may contain bugs. Hardware doesn't change except for new revs,
> > but definitely not depending on who writes a kernel to run on it. :)
> >
> >
>
> of course, I am aiming to emulate the real hardware,
> so name is no
t the problem, right?
>
> >>>> A practical aspect of modeling SoCs correctly is that they can more
> >>>> easily be reused across boards or modules, and you don't need to mess
> >>>> with machine-level cpu_model if you have a fixed SoC-CPU mapping.
> >>>>
> >>>>
> >> modeling SoC is good, but
> >> sorry, I can't assure that fixed mapping.
> >>
> > See above. A10 / sun4i => Cortex-A8, that's fixed, and then you can
> > properly embed the ARMCPU in an A10State/Sun4iState without pointer and
> > using object_initialize().
> >
> > It is your approach of a single "sunxi" machine and SunxiState that's
> > interfering with a fixed mapping AFAICT. Otherwise you'll need to
> > explain more verbose why the mapping is not assured, please.
> >
>
> I mean, e.g. A10 and A13 are different only on HDMI-transmitter and
> SATA-controller,
> but we have to have Sun4iState, and Sun5iState, I think.
>
> what I design is:
> we have a sunxi series as a machine, then
> for sunx4i, we specify -M sunxi -cpu cortex-a8 -device x1 ...
> for sunx5i, we specify -M sunxi -cpu cortex-a8 -device x2 ...
> for sunx7i, we specify -M sunxi -cpu cortex-a7 -devcie x3 ...
> for cubieboard, we specify -M sunxi -cpu -cortex-a8 -device x1 -device
> p1 ...
>
> > QOM uses a strict composition model. If you choose the physical board
> > you have, say a Gooseberry board, then modeling should be so that we use
> > qemu-system-arm -M gooseberry (without -cpu cortex-a8)
> > and /machine has-a child<allwinner-a10> "a10"
> > which in turn has-a child<cortex-a8-arm-cpu> "cpu".
> > -M cubieboard and -M marsboard can then all reuse the allwinner-a10 SoC
> > device, and in the future you can then tweak CPU properties via QMP
> > after TypeInfo::instance_init and before DeviceClass::realize.
> > -M cubieboard2 /machine by contrast has-a child<allwinner-a20> "a20"
> > which has-a child<cortex-a7-arm-cpu> "cpu[0]",
> > has-a child<cortex-a7-arm-cpu> "cpu[1]".
> >
> > Like I said below, Peter Maydell should be able to guide you in more
> > detail for the exact naming and composition.
> >
> >
> >>>> You may want to consult the recent DIGIC or earlier Faraday series or my
> >>>> Tegra2 repository for examples of how to implement this paradigm.
> >>>> I believe the composition tree naming wrt "cortex" and the MPCore was
> >>>> still open, hopefully PMM can comment on his current preferences.
> >>>>
> >>>> And thanks for your efforts, from a distribution viewpoint I am looking
> >>>> forward to testing our kernels and images with this.
> >>>>
> >>>>
> >> currently, I can only provide linux kernel build for sunxi-4i,
> >> where I can up-load it to?
> >>
> > I recall Faraday using Google Drive, for instance.
> >
> > openSUSE seems to provide some sun4i and sun5i kernel RPMs here:
> > https://build.opensuse.org/project/show/devel:ARM:12.3:Contrib:sunxi
> >
> http://download.opensuse.org/repositories/devel:/ARM:/12.3:/Contrib:/sunxi/po
> rts/armv7hl/
> >
> >
>
> tried to attach zImage in mail,
> but, seems failed.
>
> I will try other ways like google drive.
you could also try the nightly build at http://dl.linux-sunxi.org/nightly/linux-sunxi/
>
> >>>> [1] http://linux-sunxi.org/Main_Page
> >>>> [2] http://www.allwinnertech.com/en/product/A-Serial.html
> >>>>
> >>>>
> >> this page is can't accessed for me.
> >>
> > Works for me ATM, so either a temporary problem or firewall issue...
> > It provides a table of the SoCs, mapping names to CPU, GPU, etc.
> >
> > Cf. http://en.wikipedia.org/wiki/Allwinner_Technology#A-Series
> >
> >
> >
> OK.
>
> Thanks!
> Li Guang
>
>
>
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type
2013-11-29 8:41 ` Bamvor Jian Zhang
@ 2013-11-29 9:01 ` Li Guang
0 siblings, 0 replies; 33+ messages in thread
From: Li Guang @ 2013-11-29 9:01 UTC (permalink / raw)
To: Bamvor Jian Zhang
Cc: Peter Maydell, Peter Crosthwaite, hpoussin, afaerber, qemu-devel
Bamvor Jian Zhang wrote:
> Hi,
>
> >>>Li Guang<lig.fnst@cn.fujitsu.com> wrote:
>
>> Andreas Färber wrote:
>>
>>> Am 29.11.2013 01:46, schrieb Li Guang:
>>>
>>>
>>>> Andreas Färber wrote:
>>>>
>>>>
>>>>> Am 27.11.2013 10:22, schrieb Andreas Färber:
>>>>>
>>>>>
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Am 26.11.2013 10:22, schrieb Peter Crosthwaite:
>>>>>>
>>>>>>
>>>>>>
>>>>>>> On Tue, Nov 26, 2013 at 5:22 PM, liguang<lig.fnst@cn.fujitsu.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>> Signed-off-by: liguang<lig.fnst@cn.fujitsu.com>
>>>>>>>> ---
>>>>>>>> hw/arm/Makefile.objs | 1 +
>>>>>>>> hw/arm/sunxi-soc.c | 98
>>>>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>>> 2 files changed, 99 insertions(+), 0 deletions(-)
>>>>>>>> create mode 100644 hw/arm/sunxi-soc.c
>>>>>>>>
>>>>>>>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>>>>>>>> index 3671b42..f9f3071 100644
>>>>>>>> --- a/hw/arm/Makefile.objs
>>>>>>>> +++ b/hw/arm/Makefile.objs
>>>>>>>> @@ -5,3 +5,4 @@ obj-y += tosa.o versatilepb.o vexpress.o
>>>>>>>> xilinx_zynq.o z2.o
>>>>>>>>
>>>>>>>> obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
>>>>>>>> obj-y += omap1.o omap2.o strongarm.o
>>>>>>>> +obj-y += sunxi-soc.o
>>>>>>>> diff --git a/hw/arm/sunxi-soc.c b/hw/arm/sunxi-soc.c
>>>>>>>> new file mode 100644
>>>>>>>> index 0000000..b45af6d
>>>>>>>> --- /dev/null
>>>>>>>> +++ b/hw/arm/sunxi-soc.c
>>>>>>>> @@ -0,0 +1,98 @@
>>>>>>>> +/*
>>>>>>>> + * Allwinner sunxi series SoC emulation
>>>>>>>> + *
>>>>>>>> + * Copyright (C) 2013 Li Guang
>>>>>>>> + * Written by Li Guang<lig.fnst@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/devices.h"
>>>>>>>> +#include "hw/boards.h"
>>>>>>>> +#include "hw/arm/arm.h"
>>>>>>>> +#include "hw/ptimer.h"
>>>>>>>> +#include "hw/char/serial.h"
>>>>>>>> +#include "hw/timer/sunxi-pit.h"
>>>>>>>> +#include "hw/intc/sunxi-pic.h"
>>>>>>>> +
>>>>>>>> +#include "sysemu/sysemu.h"
>>>>>>>> +#include "exec/address-spaces.h"
>>>>>>>> +
>>>>>>>> +
>>>>>>>> +#define SUNXI_PIC_REG_BASE 0x01c20400
>>>>>>>> +#define SUNXI_PIT_REG_BASE 0x01c20c00
>>>>>>>> +#define SUNXI_UART0_REG_BASE 0x01c28000
>>>>>>>> +
>>>>>>>> +static struct arm_boot_info sunxi_binfo = {
>>>>>>>> + .loader_start = 0x40000000,
>>>>>>>> + .board_id = 0x1008,
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +static void sunxi_init(QEMUMachineInitArgs *args)
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>> I would check with Andreas/PMM on what the go is with SoCs regarding
>>>>>>> container devices and boards. My (vague) understanding is that SoCs
>>>>>>> should be container devices and boards instantiate those containers
>>>>>>> with off-chip connectivity. This seems flat to me, with everything on
>>>>>>> board level.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> Yes, thanks, that matches what I was going to comment. But I think it's
>>>>>> even more complicated: To my understanding, "sunxi" is the name of a
>>>>>> community effort [1] to clean up and upstream the BSP kernels from
>>>>>> Allwinner, so it sounds as if this was an attempt to write an emulation
>>>>>> for that kernel family while naming everything "sunxi" when in fact the
>>>>>> SoCs are called Axx [2] (with A1x = sun4i, A2x = sun5i, A3x = sun6i but
>>>>>>
>>>>>>
>>>>>>
>>>>> My interpolation was incorrect: A10 = sun4i, A13 = sun5i, A3x = sun6i,
>>>>> A20 = sun7i
>>>>>
>>>>> Andreas
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> no literal "sunxi" AFAIK) and boards include Cubieboard, Cubieboard2,
>>>>>> Cubieboard3/Cubietruck [3] and whatever tablets etc. are out there.
>>>>>> (CC'ing Bamvor)
>>>>>>
>>>>>> That's a lesson we learned from the old "prep" machine: Please name
>>>>>> things after real hardware, only then can it later be verified whether
>>>>>> the modeling is actually correct or which changes need to be performed.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>> well, sunxi maybe be representation of Axx series,
>>>> but, what's wrong?
>>>>
>>>>
>>> You're modeling too general IMO and thereby you're creating a
>>> virtual-only machine (despite parallel efforts by Linaro to introduce
>>> mach-virt for that purpose). Please model an actual piece of hardware -
>>> SoC and board - and not something random that happens to run with the
>>> "sunxi" kernel flavor but will leave us puzzled in the future. Should be
>>> pretty easy to avoid.
>>>
>>> My example was qemu-system-ppc -M prep. Today no one knows what hardware
>>> that was supposed to match (possibly none) because there are a number of
>>> different PReP based machines from IBM and Motorola out there; switching
>>> from OpenHack'Ware to OpenBIOS became difficult because among other
>>> things we don't have a device tree dump from a physical machine to
>>> compare to, and Hervé thus set out to create new machines such as 40P
>>> where we actually know which components the hardware contains rather
>>> than which drivers are available in the kernel and happened to have
>>> matching QEMU device implementations at the time.
>>> A slightly similar problem occurred with -M pc, where we now have an
>>> i440fx based one and the new q35 based one. It's easier to abstract
>>> commonalities and share code between different devices/machines than
>>> turning a generic machine/device into a less generic one, in particular
>>> for backwards compatibility for guests, command line and QMP.
>>>
>>> When the difference between two devices is just a value or an offset,
>>> then you can use static properties to set them and have the realize
>>> function take them into account. If the composition tree differs
>>> significantly or if you want to facilitate reuse, then different types
>>> will be needed. Multiple machines can call a shared helper function with
>>> some parameter; examples include PC, Versatile Express and DIGIC.
>>>
>>>
>>>
>>>> we can't track Axx hardware changes? why?
>>>>
>>>>
>>> Sorry, I don't get that? The Sunxi, Allwinner and Wikipedia pages all
>>> document some key differences, in particular Cortex-A8 in A10/A13 vs.
>>> Cortex-A7 in A20/A31. Cortex-A7 has MPCore, which drags along some key
>>> differences that cannot easily fit in a single SunxiState SoC device.
>>>
>>>
>>
>> right, A10/20... seem have similar devices except CPU
>>
>>
>>> At least from my understanding of Cortex-A9 and Cortex-A15 being much
>>> closer than Cortex-A8, that is. For example, you have your own PIC for
>>> the Cortex-A8 in this series whereas Cortex-A7 will use ARM's GIC and
>>> may be able to reuse the "a15mpcore_priv" composite device.
>>>
>>>
>> http://en.wikipedia.org/wiki/List_of_ARM_microprocessor_cores#Designed_by_ARM
>>
>>>
>>>
>>>> and also, this patch-set is also community effort just like
>>>> sunxi in linux kernel.
>>>>
>>>>
>>> My whole point is, try to design the model forward from hardware and
>>> less backwards from kernel. Whether it's sun4i or A10 is less relevant.
>>> Kernels may contain bugs. Hardware doesn't change except for new revs,
>>> but definitely not depending on who writes a kernel to run on it. :)
>>>
>>>
>>>
>>
>> of course, I am aiming to emulate the real hardware,
>> so name is not the problem, right?
>>
>>
>>>>>> A practical aspect of modeling SoCs correctly is that they can more
>>>>>> easily be reused across boards or modules, and you don't need to mess
>>>>>> with machine-level cpu_model if you have a fixed SoC-CPU mapping.
>>>>>>
>>>>>>
>>>>>>
>>>> modeling SoC is good, but
>>>> sorry, I can't assure that fixed mapping.
>>>>
>>>>
>>> See above. A10 / sun4i => Cortex-A8, that's fixed, and then you can
>>> properly embed the ARMCPU in an A10State/Sun4iState without pointer and
>>> using object_initialize().
>>>
>>> It is your approach of a single "sunxi" machine and SunxiState that's
>>> interfering with a fixed mapping AFAICT. Otherwise you'll need to
>>> explain more verbose why the mapping is not assured, please.
>>>
>>>
>>
>> I mean, e.g. A10 and A13 are different only on HDMI-transmitter and
>> SATA-controller,
>> but we have to have Sun4iState, and Sun5iState, I think.
>>
>> what I design is:
>> we have a sunxi series as a machine, then
>> for sunx4i, we specify -M sunxi -cpu cortex-a8 -device x1 ...
>> for sunx5i, we specify -M sunxi -cpu cortex-a8 -device x2 ...
>> for sunx7i, we specify -M sunxi -cpu cortex-a7 -devcie x3 ...
>> for cubieboard, we specify -M sunxi -cpu -cortex-a8 -device x1 -device
>> p1 ...
>>
>>
>>> QOM uses a strict composition model. If you choose the physical board
>>> you have, say a Gooseberry board, then modeling should be so that we use
>>> qemu-system-arm -M gooseberry (without -cpu cortex-a8)
>>> and /machine has-a child<allwinner-a10> "a10"
>>> which in turn has-a child<cortex-a8-arm-cpu> "cpu".
>>> -M cubieboard and -M marsboard can then all reuse the allwinner-a10 SoC
>>> device, and in the future you can then tweak CPU properties via QMP
>>> after TypeInfo::instance_init and before DeviceClass::realize.
>>> -M cubieboard2 /machine by contrast has-a child<allwinner-a20> "a20"
>>> which has-a child<cortex-a7-arm-cpu> "cpu[0]",
>>> has-a child<cortex-a7-arm-cpu> "cpu[1]".
>>>
>>> Like I said below, Peter Maydell should be able to guide you in more
>>> detail for the exact naming and composition.
>>>
>>>
>>>
>>>>>> You may want to consult the recent DIGIC or earlier Faraday series or my
>>>>>> Tegra2 repository for examples of how to implement this paradigm.
>>>>>> I believe the composition tree naming wrt "cortex" and the MPCore was
>>>>>> still open, hopefully PMM can comment on his current preferences.
>>>>>>
>>>>>> And thanks for your efforts, from a distribution viewpoint I am looking
>>>>>> forward to testing our kernels and images with this.
>>>>>>
>>>>>>
>>>>>>
>>>> currently, I can only provide linux kernel build for sunxi-4i,
>>>> where I can up-load it to?
>>>>
>>>>
>>> I recall Faraday using Google Drive, for instance.
>>>
>>> openSUSE seems to provide some sun4i and sun5i kernel RPMs here:
>>> https://build.opensuse.org/project/show/devel:ARM:12.3:Contrib:sunxi
>>>
>>>
>> http://download.opensuse.org/repositories/devel:/ARM:/12.3:/Contrib:/sunxi/po
>> rts/armv7hl/
>>
>>>
>>>
>>
>> tried to attach zImage in mail,
>> but, seems failed.
>>
>> I will try other ways like google drive.
>>
> you could also try the nightly build at http://dl.linux-sunxi.org/nightly/linux-sunxi/
>
you mean we should directly use kernel image from there?
for me, it's not OK,
currently, I only add several base devices,
and I didn't do BROM and PLL controller emulation,
there maybe some panics for that build,
we have to disable PLL config to silent kernel panics.
>>
>>
>>>>>> [1] http://linux-sunxi.org/Main_Page
>>>>>> [2] http://www.allwinnertech.com/en/product/A-Serial.html
>>>>>>
>>>>>>
>>>>>>
>>>> this page is can't accessed for me.
>>>>
>>>>
>>> Works for me ATM, so either a temporary problem or firewall issue...
>>> It provides a table of the SoCs, mapping names to CPU, GPU, etc.
>>>
>>> Cf. http://en.wikipedia.org/wiki/Allwinner_Technology#A-Series
>>>
>>>
>>>
>>>
>> OK.
>>
>> Thanks!
>> Li Guang
>>
>>
>>
>>
>>
>>
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type
2013-11-29 8:06 ` Li Guang
2013-11-29 8:31 ` Peter Maydell
2013-11-29 8:41 ` Bamvor Jian Zhang
@ 2013-11-29 13:04 ` Andreas Färber
2013-12-01 11:48 ` Peter Crosthwaite
2013-12-02 4:30 ` Li Guang
2 siblings, 2 replies; 33+ messages in thread
From: Andreas Färber @ 2013-11-29 13:04 UTC (permalink / raw)
To: Li Guang
Cc: Alexey Kardashevskiy, Peter Maydell, Peter Crosthwaite,
qemu-devel, Bamvor Jian Zhang
Am 29.11.2013 09:06, schrieb Li Guang:
> Andreas Färber wrote:
>> Am 29.11.2013 01:46, schrieb Li Guang:
>>
>>> Andreas Färber wrote:
>>>
>>>> Am 27.11.2013 10:22, schrieb Andreas Färber:
>>>>
>>>>> [...] To my understanding, "sunxi" is the name of a
>>>>> community effort [1] to clean up and upstream the BSP kernels from
>>>>> Allwinner, so it sounds as if this was an attempt to write an
>>>>> emulation
>>>>> for that kernel family while naming everything "sunxi" when in fact
>>>>> the
>>>>> SoCs are called Axx [2] (with A1x = sun4i, A2x = sun5i, A3x = sun6i
>>>>> but
>>>>>
>>>>>
>>>> My interpolation was incorrect: A10 = sun4i, A13 = sun5i, A3x = sun6i,
>>>> A20 = sun7i
>>>>
>>>>> no literal "sunxi" AFAIK) and boards include Cubieboard, Cubieboard2,
>>>>> Cubieboard3/Cubietruck [3] and whatever tablets etc. are out there.
>>>>> (CC'ing Bamvor)
>>>>>
>>>>> That's a lesson we learned from the old "prep" machine: Please name
>>>>> things after real hardware, only then can it later be verified whether
>>>>> the modeling is actually correct or which changes need to be
>>>>> performed.
>>>>>
>>>>>
>>>>>
>>> well, sunxi maybe be representation of Axx series,
>>> but, what's wrong?
>>>
>> You're modeling too general IMO and thereby you're creating a
>> virtual-only machine (despite parallel efforts by Linaro to introduce
>> mach-virt for that purpose). Please model an actual piece of hardware -
>> SoC and board - and not something random that happens to run with the
>> "sunxi" kernel flavor but will leave us puzzled in the future. Should be
>> pretty easy to avoid.
>>
>> My example was qemu-system-ppc -M prep. Today no one knows what hardware
>> that was supposed to match (possibly none) because there are a number of
>> different PReP based machines from IBM and Motorola out there; switching
>> from OpenHack'Ware to OpenBIOS became difficult because among other
>> things we don't have a device tree dump from a physical machine to
>> compare to, and Hervé thus set out to create new machines such as 40P
>> where we actually know which components the hardware contains rather
>> than which drivers are available in the kernel and happened to have
>> matching QEMU device implementations at the time.
>> A slightly similar problem occurred with -M pc, where we now have an
>> i440fx based one and the new q35 based one. It's easier to abstract
>> commonalities and share code between different devices/machines than
>> turning a generic machine/device into a less generic one, in particular
>> for backwards compatibility for guests, command line and QMP.
>>
>> When the difference between two devices is just a value or an offset,
>> then you can use static properties to set them and have the realize
>> function take them into account. If the composition tree differs
>> significantly or if you want to facilitate reuse, then different types
>> will be needed. Multiple machines can call a shared helper function with
>> some parameter; examples include PC, Versatile Express and DIGIC.
>>
>>
>>> we can't track Axx hardware changes? why?
>>>
>> Sorry, I don't get that? The Sunxi, Allwinner and Wikipedia pages all
>> document some key differences, in particular Cortex-A8 in A10/A13 vs.
>> Cortex-A7 in A20/A31. Cortex-A7 has MPCore, which drags along some key
>> differences that cannot easily fit in a single SunxiState SoC device.
>>
>
> right, A10/20... seem have similar devices except CPU
>
>> At least from my understanding of Cortex-A9 and Cortex-A15 being much
>> closer than Cortex-A8, that is. For example, you have your own PIC for
>> the Cortex-A8 in this series whereas Cortex-A7 will use ARM's GIC and
>> may be able to reuse the "a15mpcore_priv" composite device.
>> http://en.wikipedia.org/wiki/List_of_ARM_microprocessor_cores#Designed_by_ARM
>>
>>
>>
>>> and also, this patch-set is also community effort just like
>>> sunxi in linux kernel.
>>>
>> My whole point is, try to design the model forward from hardware and
>> less backwards from kernel. Whether it's sun4i or A10 is less relevant.
>> Kernels may contain bugs. Hardware doesn't change except for new revs,
>> but definitely not depending on who writes a kernel to run on it. :)
>>
>>
>
> of course, I am aiming to emulate the real hardware,
> so name is not the problem, right?
It is. The x in sunxi appears to be a wildcard.
Quoting http://linux-sunxi.org/Main_Page:
"sunxi represents the family of ARM SoC [...] made by Allwinner Tech."
The Boxship F20 is named as "sun3i", so it's even ARM9, Cortex-A8 and
Cortex-A7 all within that family. That goes beyond what we can model by
some revision property on a "sunxi" device or with -cpu, and we cannot
today create some deep detail device such as MPCore and wire that up to
containing devices. You can only instantiate devices from the command
line that sit on a bus that supports automatic wiring-up based on device
properties and knowledge of peers on the bus. In particular you cannot
initialize IRQs or map MMIO MemoryRegions from -device for
SysBusDevices, that's a repeating topic really, and we already had one
KVM conference call on that topic with no solution emerging. Otherwise
you could use -M none. I'm not writing lengthy replies here for fun!
Please replace "sunxi" with a concrete board name on machine level
(e.g., "gooseberry", "cubieboard") and with a concrete SoC name on SoC
level, whether "sun4i", "sun4i-a10", "allwinner-a13" or anything unique,
so that your series can later be extended with additional SoC family
members and/or boards with more than just the SoC on it.
>>>>> A practical aspect of modeling SoCs correctly is that they can more
>>>>> easily be reused across boards or modules, and you don't need to mess
>>>>> with machine-level cpu_model if you have a fixed SoC-CPU mapping.
>>>>>
>>>>>
>>> modeling SoC is good, but
>>> sorry, I can't assure that fixed mapping.
>>>
>> See above. A10 / sun4i => Cortex-A8, that's fixed, and then you can
>> properly embed the ARMCPU in an A10State/Sun4iState without pointer and
>> using object_initialize().
>>
>> It is your approach of a single "sunxi" machine and SunxiState that's
>> interfering with a fixed mapping AFAICT. Otherwise you'll need to
>> explain more verbose why the mapping is not assured, please.
>>
>
> I mean, e.g. A10 and A13 are different only on HDMI-transmitter and
> SATA-controller,
> but we have to have Sun4iState, and Sun5iState, I think.
Without knowing the hardware details, that sounds okay to me.
Alternatively name it after the one that's used on the board (A10) and
when someone actually needs the A13 then they can just derive a new type
with no functional changes. If they have, e.g., different MIDR values
then different types would be good for lack of property to set it. But
type name and struct name obviously don't need to match; you could even
use multi-level inheritance to model such a tree with an abstract
"sun4i" device and non-abstract A10 and A13 devices.
> what I design is:
> we have a sunxi series as a machine, then
> for sunx4i, we specify -M sunxi -cpu cortex-a8 -device x1 ...
> for sunx5i, we specify -M sunxi -cpu cortex-a8 -device x2 ...
> for sunx7i, we specify -M sunxi -cpu cortex-a7 -devcie x3 ...
> for cubieboard, we specify -M sunxi -cpu -cortex-a8 -device x1 -device
> p1 ...
And that is exactly what I am objecting to. For the Midway board we
asked the same change (there "Highbank" is a codename but it is unique
in referring to ECX-1000 model with Cortex-A9, with "Midway" being
ECX-2000 with Cortex-A15 [*] and thus -cpu cortex-a15 not working well,
cf. list archives).
Your prescribed use of -cpu argument interferes with my QOM/CPU
refactorings, with board vs. SoC layering and makes it more difficult
for the user. Your modeling seems centered on testing flavors of the
sunxi kernel that you possibly work on, whereas I am asking you to model
a board and then test that the intended kernel flavor runs on it.
The cpu_model string determines the type of the object to be
instantiated, plus possibly optional properties if we manage to go with
some form of generalization as proposed by Alexey. You cannot easily
pass all that through from machine to device level. Therefore the
recommendation is to have a SoC device where the CPU does not change
except for setting properties to enable/disable features or set reset
values etc. and to ignore -cpu on the command line. If we need to
instantiate the CPU during realization due to a typename property, then
the user will have no chance to inspect or tweak the CPU cores via QMP.
If someone wants to volunteer to summarize or link this on the
QOMConventions Wiki page that would be appreciated BTW. :)
[*] http://www.calxeda.com/products/
>> QOM uses a strict composition model. If you choose the physical board
>> you have, say a Gooseberry board, then modeling should be so that we use
>> qemu-system-arm -M gooseberry (without -cpu cortex-a8)
>> and /machine has-a child<allwinner-a10> "a10"
>> which in turn has-a child<cortex-a8-arm-cpu> "cpu".
>> -M cubieboard and -M marsboard can then all reuse the allwinner-a10 SoC
>> device, and in the future you can then tweak CPU properties via QMP
>> after TypeInfo::instance_init and before DeviceClass::realize.
>> -M cubieboard2 /machine by contrast has-a child<allwinner-a20> "a20"
>> which has-a child<cortex-a7-arm-cpu> "cpu[0]",
>> has-a child<cortex-a7-arm-cpu> "cpu[1]".
>>
>> Like I said below, Peter Maydell should be able to guide you in more
>> detail for the exact naming and composition.
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type
2013-11-29 13:04 ` Andreas Färber
@ 2013-12-01 11:48 ` Peter Crosthwaite
2013-12-02 4:30 ` Li Guang
1 sibling, 0 replies; 33+ messages in thread
From: Peter Crosthwaite @ 2013-12-01 11:48 UTC (permalink / raw)
To: Andreas Färber
Cc: Alexey Kardashevskiy, Peter Maydell, qemu-devel, Li Guang,
Bamvor Jian Zhang
On Fri, Nov 29, 2013 at 11:04 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 29.11.2013 09:06, schrieb Li Guang:
>> Andreas Färber wrote:
>>> Am 29.11.2013 01:46, schrieb Li Guang:
>>>
>>>> Andreas Färber wrote:
>>>>
>>>>> Am 27.11.2013 10:22, schrieb Andreas Färber:
>>>>>
>>>>>> [...] To my understanding, "sunxi" is the name of a
>>>>>> community effort [1] to clean up and upstream the BSP kernels from
>>>>>> Allwinner, so it sounds as if this was an attempt to write an
>>>>>> emulation
>>>>>> for that kernel family while naming everything "sunxi" when in fact
>>>>>> the
>>>>>> SoCs are called Axx [2] (with A1x = sun4i, A2x = sun5i, A3x = sun6i
>>>>>> but
>>>>>>
>>>>>>
>>>>> My interpolation was incorrect: A10 = sun4i, A13 = sun5i, A3x = sun6i,
>>>>> A20 = sun7i
>>>>>
>>>>>> no literal "sunxi" AFAIK) and boards include Cubieboard, Cubieboard2,
>>>>>> Cubieboard3/Cubietruck [3] and whatever tablets etc. are out there.
>>>>>> (CC'ing Bamvor)
>>>>>>
>>>>>> That's a lesson we learned from the old "prep" machine: Please name
>>>>>> things after real hardware, only then can it later be verified whether
>>>>>> the modeling is actually correct or which changes need to be
>>>>>> performed.
>>>>>>
>>>>>>
>>>>>>
>>>> well, sunxi maybe be representation of Axx series,
>>>> but, what's wrong?
>>>>
>>> You're modeling too general IMO and thereby you're creating a
>>> virtual-only machine (despite parallel efforts by Linaro to introduce
>>> mach-virt for that purpose). Please model an actual piece of hardware -
>>> SoC and board - and not something random that happens to run with the
>>> "sunxi" kernel flavor but will leave us puzzled in the future. Should be
>>> pretty easy to avoid.
>>>
>>> My example was qemu-system-ppc -M prep. Today no one knows what hardware
>>> that was supposed to match (possibly none) because there are a number of
>>> different PReP based machines from IBM and Motorola out there; switching
>>> from OpenHack'Ware to OpenBIOS became difficult because among other
>>> things we don't have a device tree dump from a physical machine to
>>> compare to, and Hervé thus set out to create new machines such as 40P
>>> where we actually know which components the hardware contains rather
>>> than which drivers are available in the kernel and happened to have
>>> matching QEMU device implementations at the time.
>>> A slightly similar problem occurred with -M pc, where we now have an
>>> i440fx based one and the new q35 based one. It's easier to abstract
>>> commonalities and share code between different devices/machines than
>>> turning a generic machine/device into a less generic one, in particular
>>> for backwards compatibility for guests, command line and QMP.
>>>
>>> When the difference between two devices is just a value or an offset,
>>> then you can use static properties to set them and have the realize
>>> function take them into account. If the composition tree differs
>>> significantly or if you want to facilitate reuse, then different types
>>> will be needed. Multiple machines can call a shared helper function with
>>> some parameter; examples include PC, Versatile Express and DIGIC.
>>>
>>>
>>>> we can't track Axx hardware changes? why?
>>>>
>>> Sorry, I don't get that? The Sunxi, Allwinner and Wikipedia pages all
>>> document some key differences, in particular Cortex-A8 in A10/A13 vs.
>>> Cortex-A7 in A20/A31. Cortex-A7 has MPCore, which drags along some key
>>> differences that cannot easily fit in a single SunxiState SoC device.
>>>
>>
>> right, A10/20... seem have similar devices except CPU
>>
>>> At least from my understanding of Cortex-A9 and Cortex-A15 being much
>>> closer than Cortex-A8, that is. For example, you have your own PIC for
>>> the Cortex-A8 in this series whereas Cortex-A7 will use ARM's GIC and
>>> may be able to reuse the "a15mpcore_priv" composite device.
>>> http://en.wikipedia.org/wiki/List_of_ARM_microprocessor_cores#Designed_by_ARM
>>>
>>>
>>>
>>>> and also, this patch-set is also community effort just like
>>>> sunxi in linux kernel.
>>>>
>>> My whole point is, try to design the model forward from hardware and
>>> less backwards from kernel. Whether it's sun4i or A10 is less relevant.
>>> Kernels may contain bugs. Hardware doesn't change except for new revs,
>>> but definitely not depending on who writes a kernel to run on it. :)
>>>
>>>
>>
>> of course, I am aiming to emulate the real hardware,
>> so name is not the problem, right?
>
> It is. The x in sunxi appears to be a wildcard.
>
> Quoting http://linux-sunxi.org/Main_Page:
> "sunxi represents the family of ARM SoC [...] made by Allwinner Tech."
>
> The Boxship F20 is named as "sun3i", so it's even ARM9, Cortex-A8 and
> Cortex-A7 all within that family. That goes beyond what we can model by
> some revision property on a "sunxi" device or with -cpu, and we cannot
> today create some deep detail device such as MPCore and wire that up to
> containing devices. You can only instantiate devices from the command
> line that sit on a bus that supports automatic wiring-up based on device
> properties and knowledge of peers on the bus. In particular you cannot
> initialize IRQs or map MMIO MemoryRegions from -device for
> SysBusDevices, that's a repeating topic really, and we already had one
> KVM conference call on that topic with no solution emerging. Otherwise
> you could use -M none. I'm not writing lengthy replies here for fun!
>
> Please replace "sunxi" with a concrete board name on machine level
> (e.g., "gooseberry", "cubieboard") and with a concrete SoC name on SoC
> level, whether "sun4i", "sun4i-a10", "allwinner-a13" or anything unique,
> so that your series can later be extended with additional SoC family
> members and/or boards with more than just the SoC on it.
>
>>>>>> A practical aspect of modeling SoCs correctly is that they can more
>>>>>> easily be reused across boards or modules, and you don't need to mess
>>>>>> with machine-level cpu_model if you have a fixed SoC-CPU mapping.
>>>>>>
>>>>>>
>>>> modeling SoC is good, but
>>>> sorry, I can't assure that fixed mapping.
>>>>
>>> See above. A10 / sun4i => Cortex-A8, that's fixed, and then you can
>>> properly embed the ARMCPU in an A10State/Sun4iState without pointer and
>>> using object_initialize().
>>>
>>> It is your approach of a single "sunxi" machine and SunxiState that's
>>> interfering with a fixed mapping AFAICT. Otherwise you'll need to
>>> explain more verbose why the mapping is not assured, please.
>>>
>>
>> I mean, e.g. A10 and A13 are different only on HDMI-transmitter and
>> SATA-controller,
>> but we have to have Sun4iState, and Sun5iState, I think.
>
> Without knowing the hardware details, that sounds okay to me.
>
> Alternatively name it after the one that's used on the board (A10) and
> when someone actually needs the A13 then they can just derive a new type
> with no functional changes. If they have, e.g., different MIDR values
> then different types would be good for lack of property to set it. But
> type name and struct name obviously don't need to match; you could even
> use multi-level inheritance to model such a tree with an abstract
> "sun4i" device and non-abstract A10 and A13 devices.
>
>> what I design is:
>> we have a sunxi series as a machine, then
>> for sunx4i, we specify -M sunxi -cpu cortex-a8 -device x1 ...
>> for sunx5i, we specify -M sunxi -cpu cortex-a8 -device x2 ...
>> for sunx7i, we specify -M sunxi -cpu cortex-a7 -devcie x3 ...
>> for cubieboard, we specify -M sunxi -cpu -cortex-a8 -device x1 -device
>> p1 ...
>
> And that is exactly what I am objecting to. For the Midway board we
> asked the same change (there "Highbank" is a codename but it is unique
> in referring to ECX-1000 model with Cortex-A9, with "Midway" being
> ECX-2000 with Cortex-A15 [*] and thus -cpu cortex-a15 not working well,
> cf. list archives).
A bit of searching suggests no sunxi branding by allwinner themselves.
I would suggest the correct name for the soc container is
"allwinner-axx". If you can parameterise then use the literal
"allwinner-axx" to mean multiple devices. But iw ould suggest its
easiest in your first series to just pick you favorite SoC variant
(e.g. A13) and your favorite board and go from there. Generalise to
the wider sunxi supported hardware in a second series.
Regards,
Peter
> Your prescribed use of -cpu argument interferes with my QOM/CPU
> refactorings, with board vs. SoC layering and makes it more difficult
> for the user. Your modeling seems centered on testing flavors of the
> sunxi kernel that you possibly work on, whereas I am asking you to model
> a board and then test that the intended kernel flavor runs on it.
>
> The cpu_model string determines the type of the object to be
> instantiated, plus possibly optional properties if we manage to go with
> some form of generalization as proposed by Alexey. You cannot easily
> pass all that through from machine to device level. Therefore the
> recommendation is to have a SoC device where the CPU does not change
> except for setting properties to enable/disable features or set reset
> values etc. and to ignore -cpu on the command line. If we need to
> instantiate the CPU during realization due to a typename property, then
> the user will have no chance to inspect or tweak the CPU cores via QMP.
>
> If someone wants to volunteer to summarize or link this on the
> QOMConventions Wiki page that would be appreciated BTW. :)
>
How about:
"If its branded, sold or soldered as one piece, it should be a
container device".
Simplistic but captures the idea.
We could add some stuff about naming conventions too.
> [*] http://www.calxeda.com/products/
>
>>> QOM uses a strict composition model. If you choose the physical board
>>> you have, say a Gooseberry board, then modeling should be so that we use
>>> qemu-system-arm -M gooseberry (without -cpu cortex-a8)
>>> and /machine has-a child<allwinner-a10> "a10"
>>> which in turn has-a child<cortex-a8-arm-cpu> "cpu".
>>> -M cubieboard and -M marsboard can then all reuse the allwinner-a10 SoC
>>> device, and in the future you can then tweak CPU properties via QMP
>>> after TypeInfo::instance_init and before DeviceClass::realize.
>>> -M cubieboard2 /machine by contrast has-a child<allwinner-a20> "a20"
>>> which has-a child<cortex-a7-arm-cpu> "cpu[0]",
>>> has-a child<cortex-a7-arm-cpu> "cpu[1]".
>>>
>>> Like I said below, Peter Maydell should be able to guide you in more
>>> detail for the exact naming and composition.
>
could we standardise on a "vendor-product" convention for all of
boards / containers and devices? E.g:
Some SoCs would be:
hw/arm/calxeda-ecx.c
hw/arm/allwinner-axx.c
hw/arm/xilinx-zynq.c
Some boards would be:
hw/arm/arm-vexpress.c
hw/arm/xilinx-zc70x.c
And with peripheral devices, the preference should be the IP vendor
name NOT the SoC manufacturer. E.G:
hw/char/cadence-uart.c
hw/dma/arm-pl330.c
Vendor names are simply omitted when the IP follows a standardized HCI:
hw/sd/shdci.c
hw/usb/hci-ehci.c
Consequently, Zynq and Highbank do need a makeover, or at least be
marked as bad examples of how not to do an ARM SoC with the new rules.
Regards,
Peter
> Regards,
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type
2013-11-29 13:04 ` Andreas Färber
2013-12-01 11:48 ` Peter Crosthwaite
@ 2013-12-02 4:30 ` Li Guang
1 sibling, 0 replies; 33+ messages in thread
From: Li Guang @ 2013-12-02 4:30 UTC (permalink / raw)
To: Andreas Färber
Cc: Alexey Kardashevskiy, Peter Maydell, Peter Crosthwaite,
qemu-devel, Bamvor Jian Zhang
Andreas Färber wrote:
> Am 29.11.2013 09:06, schrieb Li Guang:
>
>> Andreas Färber wrote:
>>
>>> Am 29.11.2013 01:46, schrieb Li Guang:
>>>
>>>
>>>> Andreas Färber wrote:
>>>>
>>>>
>>>>> Am 27.11.2013 10:22, schrieb Andreas Färber:
>>>>>
>>>>>
>>>>>> [...] To my understanding, "sunxi" is the name of a
>>>>>> community effort [1] to clean up and upstream the BSP kernels from
>>>>>> Allwinner, so it sounds as if this was an attempt to write an
>>>>>> emulation
>>>>>> for that kernel family while naming everything "sunxi" when in fact
>>>>>> the
>>>>>> SoCs are called Axx [2] (with A1x = sun4i, A2x = sun5i, A3x = sun6i
>>>>>> but
>>>>>>
>>>>>>
>>>>>>
>>>>> My interpolation was incorrect: A10 = sun4i, A13 = sun5i, A3x = sun6i,
>>>>> A20 = sun7i
>>>>>
>>>>>
>>>>>> no literal "sunxi" AFAIK) and boards include Cubieboard, Cubieboard2,
>>>>>> Cubieboard3/Cubietruck [3] and whatever tablets etc. are out there.
>>>>>> (CC'ing Bamvor)
>>>>>>
>>>>>> That's a lesson we learned from the old "prep" machine: Please name
>>>>>> things after real hardware, only then can it later be verified whether
>>>>>> the modeling is actually correct or which changes need to be
>>>>>> performed.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>> well, sunxi maybe be representation of Axx series,
>>>> but, what's wrong?
>>>>
>>>>
>>> You're modeling too general IMO and thereby you're creating a
>>> virtual-only machine (despite parallel efforts by Linaro to introduce
>>> mach-virt for that purpose). Please model an actual piece of hardware -
>>> SoC and board - and not something random that happens to run with the
>>> "sunxi" kernel flavor but will leave us puzzled in the future. Should be
>>> pretty easy to avoid.
>>>
>>> My example was qemu-system-ppc -M prep. Today no one knows what hardware
>>> that was supposed to match (possibly none) because there are a number of
>>> different PReP based machines from IBM and Motorola out there; switching
>>> from OpenHack'Ware to OpenBIOS became difficult because among other
>>> things we don't have a device tree dump from a physical machine to
>>> compare to, and Hervé thus set out to create new machines such as 40P
>>> where we actually know which components the hardware contains rather
>>> than which drivers are available in the kernel and happened to have
>>> matching QEMU device implementations at the time.
>>> A slightly similar problem occurred with -M pc, where we now have an
>>> i440fx based one and the new q35 based one. It's easier to abstract
>>> commonalities and share code between different devices/machines than
>>> turning a generic machine/device into a less generic one, in particular
>>> for backwards compatibility for guests, command line and QMP.
>>>
>>> When the difference between two devices is just a value or an offset,
>>> then you can use static properties to set them and have the realize
>>> function take them into account. If the composition tree differs
>>> significantly or if you want to facilitate reuse, then different types
>>> will be needed. Multiple machines can call a shared helper function with
>>> some parameter; examples include PC, Versatile Express and DIGIC.
>>>
>>>
>>>
>>>> we can't track Axx hardware changes? why?
>>>>
>>>>
>>> Sorry, I don't get that? The Sunxi, Allwinner and Wikipedia pages all
>>> document some key differences, in particular Cortex-A8 in A10/A13 vs.
>>> Cortex-A7 in A20/A31. Cortex-A7 has MPCore, which drags along some key
>>> differences that cannot easily fit in a single SunxiState SoC device.
>>>
>>>
>> right, A10/20... seem have similar devices except CPU
>>
>>
>>> At least from my understanding of Cortex-A9 and Cortex-A15 being much
>>> closer than Cortex-A8, that is. For example, you have your own PIC for
>>> the Cortex-A8 in this series whereas Cortex-A7 will use ARM's GIC and
>>> may be able to reuse the "a15mpcore_priv" composite device.
>>> http://en.wikipedia.org/wiki/List_of_ARM_microprocessor_cores#Designed_by_ARM
>>>
>>>
>>>
>>>
>>>> and also, this patch-set is also community effort just like
>>>> sunxi in linux kernel.
>>>>
>>>>
>>> My whole point is, try to design the model forward from hardware and
>>> less backwards from kernel. Whether it's sun4i or A10 is less relevant.
>>> Kernels may contain bugs. Hardware doesn't change except for new revs,
>>> but definitely not depending on who writes a kernel to run on it. :)
>>>
>>>
>>>
>> of course, I am aiming to emulate the real hardware,
>> so name is not the problem, right?
>>
> It is. The x in sunxi appears to be a wildcard.
>
> Quoting http://linux-sunxi.org/Main_Page:
> "sunxi represents the family of ARM SoC [...] made by Allwinner Tech."
>
> The Boxship F20 is named as "sun3i", so it's even ARM9, Cortex-A8 and
> Cortex-A7 all within that family. That goes beyond what we can model by
> some revision property on a "sunxi" device or with -cpu, and we cannot
> today create some deep detail device such as MPCore and wire that up to
> containing devices. You can only instantiate devices from the command
> line that sit on a bus that supports automatic wiring-up based on device
> properties and knowledge of peers on the bus. In particular you cannot
> initialize IRQs or map MMIO MemoryRegions from -device for
> SysBusDevices, that's a repeating topic really, and we already had one
> KVM conference call on that topic with no solution emerging. Otherwise
> you could use -M none. I'm not writing lengthy replies here for fun!
>
> Please replace "sunxi" with a concrete board name on machine level
> (e.g., "gooseberry", "cubieboard") and with a concrete SoC name on SoC
> level, whether "sun4i", "sun4i-a10", "allwinner-a13" or anything unique,
> so that your series can later be extended with additional SoC family
> members and/or boards with more than just the SoC on it.
>
>
OK, let me change and re-post.
Thanks!
>>>>>> A practical aspect of modeling SoCs correctly is that they can more
>>>>>> easily be reused across boards or modules, and you don't need to mess
>>>>>> with machine-level cpu_model if you have a fixed SoC-CPU mapping.
>>>>>>
>>>>>>
>>>>>>
>>>> modeling SoC is good, but
>>>> sorry, I can't assure that fixed mapping.
>>>>
>>>>
>>> See above. A10 / sun4i => Cortex-A8, that's fixed, and then you can
>>> properly embed the ARMCPU in an A10State/Sun4iState without pointer and
>>> using object_initialize().
>>>
>>> It is your approach of a single "sunxi" machine and SunxiState that's
>>> interfering with a fixed mapping AFAICT. Otherwise you'll need to
>>> explain more verbose why the mapping is not assured, please.
>>>
>>>
>> I mean, e.g. A10 and A13 are different only on HDMI-transmitter and
>> SATA-controller,
>> but we have to have Sun4iState, and Sun5iState, I think.
>>
> Without knowing the hardware details, that sounds okay to me.
>
> Alternatively name it after the one that's used on the board (A10) and
> when someone actually needs the A13 then they can just derive a new type
> with no functional changes. If they have, e.g., different MIDR values
> then different types would be good for lack of property to set it. But
> type name and struct name obviously don't need to match; you could even
> use multi-level inheritance to model such a tree with an abstract
> "sun4i" device and non-abstract A10 and A13 devices.
>
>
>> what I design is:
>> we have a sunxi series as a machine, then
>> for sunx4i, we specify -M sunxi -cpu cortex-a8 -device x1 ...
>> for sunx5i, we specify -M sunxi -cpu cortex-a8 -device x2 ...
>> for sunx7i, we specify -M sunxi -cpu cortex-a7 -devcie x3 ...
>> for cubieboard, we specify -M sunxi -cpu -cortex-a8 -device x1 -device
>> p1 ...
>>
> And that is exactly what I am objecting to. For the Midway board we
> asked the same change (there "Highbank" is a codename but it is unique
> in referring to ECX-1000 model with Cortex-A9, with "Midway" being
> ECX-2000 with Cortex-A15 [*] and thus -cpu cortex-a15 not working well,
> cf. list archives).
> Your prescribed use of -cpu argument interferes with my QOM/CPU
> refactorings, with board vs. SoC layering and makes it more difficult
> for the user. Your modeling seems centered on testing flavors of the
> sunxi kernel that you possibly work on, whereas I am asking you to model
> a board and then test that the intended kernel flavor runs on it.
>
> The cpu_model string determines the type of the object to be
> instantiated, plus possibly optional properties if we manage to go with
> some form of generalization as proposed by Alexey. You cannot easily
> pass all that through from machine to device level. Therefore the
> recommendation is to have a SoC device where the CPU does not change
> except for setting properties to enable/disable features or set reset
> values etc. and to ignore -cpu on the command line. If we need to
> instantiate the CPU during realization due to a typename property, then
> the user will have no chance to inspect or tweak the CPU cores via QMP.
>
> If someone wants to volunteer to summarize or link this on the
> QOMConventions Wiki page that would be appreciated BTW. :)
>
> [*] http://www.calxeda.com/products/
>
>
>>> QOM uses a strict composition model. If you choose the physical board
>>> you have, say a Gooseberry board, then modeling should be so that we use
>>> qemu-system-arm -M gooseberry (without -cpu cortex-a8)
>>> and /machine has-a child<allwinner-a10> "a10"
>>> which in turn has-a child<cortex-a8-arm-cpu> "cpu".
>>> -M cubieboard and -M marsboard can then all reuse the allwinner-a10 SoC
>>> device, and in the future you can then tweak CPU properties via QMP
>>> after TypeInfo::instance_init and before DeviceClass::realize.
>>> -M cubieboard2 /machine by contrast has-a child<allwinner-a20> "a20"
>>> which has-a child<cortex-a7-arm-cpu> "cpu[0]",
>>> has-a child<cortex-a7-arm-cpu> "cpu[1]".
>>>
>>> Like I said below, Peter Maydell should be able to guide you in more
>>> detail for the exact naming and composition.
>>>
> Regards,
> Andreas
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH v4 4/4] MAINTAINERS: add myself to maintain sunxi machine
2013-11-26 7:22 [Qemu-devel] [PATCH v4 0/4] add sunxi machine type liguang
` (2 preceding siblings ...)
2013-11-26 7:22 ` [Qemu-devel] [PATCH v4 3/4] hw/arm: add sunxi machine type liguang
@ 2013-11-26 7:22 ` liguang
3 siblings, 0 replies; 33+ messages in thread
From: liguang @ 2013-11-26 7:22 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, liguang
Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
---
MAINTAINERS | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 77edacf..232e1a1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -284,6 +284,15 @@ M: Peter Maydell <peter.maydell@linaro.org>
S: Maintained
F: hw/*/stellaris*
+Sunxi
+M: Li Guang <lig.fnst@cn.fujitsu.com>
+S: Maintained
+F: hw/arm/sunxi-soc.c
+F: hw/intc/sunxi-pic.c
+F: hw/timer/sunxi-pit.c
+F: include/hw/intc/sunxi-pic.h
+F: include/hw/timer/sunxi-pit.h
+
Versatile PB
M: Peter Maydell <peter.maydell@linaro.org>
S: Maintained
--
1.7.2.5
^ permalink raw reply related [flat|nested] 33+ messages in thread