Re: [Qemu-devel] [PATCH 4/5] hw/arm: add allwinner a10 SoC support

[Peter Crosthwaite]

On Tue, Dec 3, 2013 at 7:11 PM, liguang <address@hidden> wrote:
> Signed-off-by: liguang <address@hidden>
> ---
>  hw/arm/Makefile.objs           |    2 +-
>  hw/arm/allwinner-a10.c         |   39 +++++++++++++++++++++++++++++++++++++++
>  include/hw/arm/allwinner-a10.h |   27 +++++++++++++++++++++++++++
>  3 files changed, 67 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..cbc6db4
> --- /dev/null
> +++ b/hw/arm/allwinner-a10.c
> @@ -0,0 +1,39 @@
> +#include "hw/sysbus.h"
> +#include "hw/devices.h"
> +#include "hw/arm/allwinner-a10.h"
> +
> +
> +A10State *a10_init(MemoryRegion *system_mem, unsigned long ram_size)
> +{
> +    A10State *s = (A10State *)g_malloc0(sizeof(A10State));
> +    MemoryRegion *address_space_mem = system_mem;
> +    qemu_irq pic[A10_PIC_INT_NR];
> +    DeviceState *dev;
> +    uint8_t i;
> +
> +    s->cpu = cpu_arm_init("cortex-a8");
> +    if (!s->cpu) {
> +        fprintf(stderr, "Unable to find CPU definition\n");

error_report is encouraged in situations like this I think.

> +        exit(1);
> +    }
> +
> +    memory_region_init_ram(&s->sdram, NULL, "a10.ram", ram_size);
> +    vmstate_register_ram_global(&s->sdram);
> +    memory_region_add_subregion(address_space_mem, A10_SDRAM_BASE, 
> &s->sdram);
> +
> +    dev = sysbus_create_varargs(TYPE_A10_PIC, A10_PIC_REG_BASE,
> +                                qdev_get_gpio_in(DEVICE(s->cpu), 
> ARM_CPU_IRQ),
> +                                qdev_get_gpio_in(DEVICE(s->cpu), 
> ARM_CPU_FIQ),
> +                                NULL);

sysbus create is the old API. Object initialise is the new correct API for this.

> +    for (i = 0; i < A10_PIC_INT_NR; i++) {
> +        pic[i] = qdev_get_gpio_in(dev, i);
> +    }
> +
> +    sysbus_create_varargs(TYPE_A10_PIT, A10_PIT_REG_BASE, pic[22], pic[23],
> +                          pic[24], pic[25], pic[67], pic[68], NULL);
> +
> +    serial_mm_init(address_space_mem, A10_UART0_REG_BASE, 2, pic[1], 115200,
> +                    serial_hds[0], DEVICE_NATIVE_ENDIAN);
> +

what do you do with the sysbus devs once you you create them? arent
they just lost?

> +    return s;
> +}
> diff --git a/include/hw/arm/allwinner-a10.h b/include/hw/arm/allwinner-a10.h
> new file mode 100644
> index 0000000..32a8cb5
> --- /dev/null
> +++ b/include/hw/arm/allwinner-a10.h
> @@ -0,0 +1,27 @@
> +#ifndef ALLWINNER_H_
> +
> +#include "qemu-common.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 A10_PIC_REG_BASE 0x01c20400
> +#define A10_PIT_REG_BASE 0x01c20c00
> +#define A10_UART0_REG_BASE 0x01c28000
> +
> +#define A10_SDRAM_BASE 0x40000000
> +
> +typedef struct A10State {
> +    ARMCPU *cpu;
> +    MemoryRegion sdram;
> +} A10State;
> +

This needs to be QOMified as a device. There should be a type
definition for it. Check the Digic series for a good example (Patch 1
V6).

> +A10State *a10_init(MemoryRegion *system_mem, unsigned long ram_size);
> +

And this just becomes object-init or device-realize as appropriate.

Regards,
Peter

> +#define ALLWINNER_H_
> +#endif
> --
> 1.7.2.5
>
>