From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56358) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VoMiW-0003pr-AV for qemu-devel@nongnu.org; Wed, 04 Dec 2013 19:23:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VoMiR-0001Hy-Mh for qemu-devel@nongnu.org; Wed, 04 Dec 2013 19:22:56 -0500 Received: from [222.73.24.84] (port=2039 helo=song.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VoMiQ-0001Fw-Mq for qemu-devel@nongnu.org; Wed, 04 Dec 2013 19:22:51 -0500 Message-ID: <529FC6F9.6040101@cn.fujitsu.com> Date: Thu, 05 Dec 2013 08:21:13 +0800 From: Li Guang MIME-Version: 1.0 References: <1386144561-23013-1-git-send-email-lig.fnst@cn.fujitsu.com> <1386144561-23013-5-git-send-email-lig.fnst@cn.fujitsu.com> In-Reply-To: Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset=ISO-8859-1; format=flowed Subject: Re: [Qemu-devel] [PATCH v8 4/5] hw/arm: add allwinner a10 SoC support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite Cc: Peter Maydell , "qemu-devel@nongnu.org Developers" , =?ISO-8859-1?Q?Andreas_F=E4rber?= Peter Crosthwaite wrote: > On Wed, Dec 4, 2013 at 6:09 PM, liguang wrote: > >> Signed-off-by: liguang >> --- >> hw/arm/Makefile.objs | 2 +- >> hw/arm/allwinner-a10.c | 77 ++++++++++++++++++++++++++++++++++++++++ >> include/hw/arm/allwinner-a10.h | 36 ++++++++++++++++++ >> 3 files changed, 114 insertions(+), 1 deletions(-) >> create mode 100644 hw/arm/allwinner-a10.c >> create mode 100644 include/hw/arm/allwinner-a10.h >> >> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs >> index 3671b42..b9e5983 100644 >> --- a/hw/arm/Makefile.objs >> +++ b/hw/arm/Makefile.objs >> @@ -4,4 +4,4 @@ obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o >> 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 += omap1.o omap2.o strongarm.o allwinner-a10.o >> diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c >> new file mode 100644 >> index 0000000..c4699b7 >> --- /dev/null >> +++ b/hw/arm/allwinner-a10.c >> @@ -0,0 +1,77 @@ >> +#include "hw/sysbus.h" >> +#include "hw/devices.h" >> +#include "hw/arm/allwinner-a10.h" >> + >> + >> +static void aw_a10_init(Object *obj) >> +{ >> + AwA10State *s = AW_A10(obj); >> + DeviceState *dev; >> + >> + object_initialize(&s->cpu, sizeof(s->cpu), "cortex-a8-" TYPE_ARM_CPU); >> + object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL); >> + >> + object_initialize(&s->intc, sizeof(s->timer), TYPE_AW_A10_PIC); >> + dev = DEVICE(&s->intc); >> > You could just use DEVICE(foo) inline here and below and drop the dev > variable to reduce verbosity. > > >> + qdev_set_parent_bus(dev, sysbus_get_default()); >> + >> + object_initialize(&s->timer, sizeof(s->timer), TYPE_AW_A10_PIT); >> + dev = DEVICE(&s->timer); >> + qdev_set_parent_bus(dev, sysbus_get_default()); >> +} >> + >> +static void aw_a10_realize(DeviceState *dev, Error **errp) >> +{ >> + AwA10State *s = AW_A10(dev); >> + SysBusDevice *sysbusdev; >> + uint8_t i; >> + Error *err = NULL; >> + >> + object_property_set_bool(OBJECT(&s->cpu), true, "realized",&err); >> + s->cpu_irq[0] = qdev_get_gpio_in(DEVICE(&s->cpu), ARM_CPU_IRQ); >> + s->cpu_irq[1] = qdev_get_gpio_in(DEVICE(&s->cpu), ARM_CPU_FIQ); >> + >> + object_property_set_bool(OBJECT(&s->intc), true, "realized",&err); >> > You neither assert nor propagate the error. So the repeated usage > below will cause a delayed (and somewhat obscure) assertion below if > both happen to error for some reason. > > >> + sysbusdev = SYS_BUS_DEVICE(&s->intc); >> + sysbus_mmio_map(sysbusdev, 0, AW_A10_PIC_REG_BASE); >> + sysbus_connect_irq(sysbusdev, 0, s->cpu_irq[0]); >> + sysbus_connect_irq(sysbusdev, 1, s->cpu_irq[1]); >> + for (i = 0; i< AW_A10_PIC_INT_NR; i++) { >> + s->irq[i] = qdev_get_gpio_in(DEVICE(&s->intc), i); >> + } >> + >> + object_property_set_bool(OBJECT(&s->timer), true, "realized",&err); >> + sysbusdev = SYS_BUS_DEVICE(&s->timer); >> + sysbus_mmio_map(sysbusdev, 0, AW_A10_PIT_REG_BASE); >> + sysbus_connect_irq(sysbusdev, 0, s->irq[22]); >> + sysbus_connect_irq(sysbusdev, 1, s->irq[23]); >> + sysbus_connect_irq(sysbusdev, 2, s->irq[24]); >> + sysbus_connect_irq(sysbusdev, 3, s->irq[25]); >> + sysbus_connect_irq(sysbusdev, 4, s->irq[67]); >> + sysbus_connect_irq(sysbusdev, 5, s->irq[68]); >> + >> + serial_mm_init(get_system_memory(), AW_A10_UART0_REG_BASE, 2, s->irq[1], >> + 115200, serial_hds[0], DEVICE_NATIVE_ENDIAN); >> +} >> + >> +static void aw_a10_class_init(ObjectClass *oc, void *data) >> +{ >> + DeviceClass *dc = DEVICE_CLASS(oc); >> + >> + dc->realize = aw_a10_realize; >> +} >> + >> +static const TypeInfo aw_a10_type_info = { >> + .name = TYPE_AW_A10, >> + .parent = TYPE_DEVICE, >> + .instance_size = sizeof(AwA10State), >> + .instance_init = aw_a10_init, >> + .class_init = aw_a10_class_init, >> +}; >> + >> +static void aw_a10_register_types(void) >> +{ >> + type_register_static(&aw_a10_type_info); >> +} >> + >> +type_init(aw_a10_register_types) >> diff --git a/include/hw/arm/allwinner-a10.h b/include/hw/arm/allwinner-a10.h >> new file mode 100644 >> index 0000000..a3e7b77 >> --- /dev/null >> +++ b/include/hw/arm/allwinner-a10.h >> @@ -0,0 +1,36 @@ >> +#ifndef ALLWINNER_H_ >> + >> +#include "qemu-common.h" >> +#include "qemu/error-report.h" >> +#include "hw/char/serial.h" >> +#include "hw/arm/arm.h" >> +#include "hw/timer/allwinner-a10-pit.h" >> +#include "hw/intc/allwinner-a10-pic.h" >> + >> +#include "sysemu/sysemu.h" >> +#include "exec/address-spaces.h" >> + >> + >> +#define AW_A10_PIC_REG_BASE 0x01c20400 >> +#define AW_A10_PIT_REG_BASE 0x01c20c00 >> +#define AW_A10_UART0_REG_BASE 0x01c28000 >> + >> +#define AW_A10_SDRAM_BASE 0x40000000 >> + >> +#define TYPE_AW_A10 "allwiner-a10" >> > "allwinner" > > >> +#define AW_A10(obj) OBJECT_CHECK(AwA10State, (obj), TYPE_AW_A10) >> + >> +typedef struct AwA10State { >> + /*< private>*/ >> + DeviceState parent_obj; >> + /*< public>*/ >> + >> + ARMCPU cpu; >> + qemu_irq irq[AW_A10_PIC_INT_NR]; >> + qemu_irq cpu_irq[2]; >> > I dont see the need to keep these as device state. They appear to be > just local variables to realize(). > > >> + AwA10PITState timer; >> + AwA10PICState intc; >> +} AwA10State; >> + >> +#define ALLWINNER_H_ >> +#endif >> -- >> 1.7.2.5 >> >> >> > OK, will fix. Thanks! Li Guang