[v2,01/20] arm: add Faraday a36x SoC platform support

Message ID	1359101996-11667-2-git-send-email-dantesu@gmail.com
State	New
Headers	show Return-Path: <qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org> From: Kuo-Jung Su <dantesu@gmail.com> To: qemu-devel@nongnu.org Date: Fri, 25 Jan 2013 16:19:37 +0800 Message-Id: <1359101996-11667-2-git-send-email-dantesu@gmail.com> In-Reply-To: <1359101996-11667-1-git-send-email-dantesu@gmail.com> References: <1359101996-11667-1-git-send-email-dantesu@gmail.com> In-Reply-To: <1358490506-18281-1-git-send-email-dantesu@faraday-tech.com> References: <1358490506-18281-1-git-send-email-dantesu@faraday-tech.com> Cc: peter.maydell@linaro.org, blauwirbel@gmail.com, paul@codesourcery.com, Kuo-Jung Su <dantesu@faraday-tech.com>, afaerber@suse.de, fred.konrad@greensocs.com Subject: [Qemu-devel] [PATCH v2 01/20] arm: add Faraday a36x SoC platform support Precedence: list Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org

Kuo-Jung Su Jan. 25, 2013, 8:19 a.m. UTC

From: Kuo-Jung Su <dantesu@faraday-tech.com>

The Faraday A360/A369 EVB is a Faraday platform main board used for the
Faraday IP functional verification based on the well-known ARM AMBA 2.0
architecture. This main board provides a fully verified microprocessor
platform, ATA-II, OTG 2.0 and USB 2.0 host connectivity.

Faraday A360 EVB provides the on-board DDR2-SODIMM (256 MB),
NAND Flash (128 MB), USB2.0 interface, two PCI Express interfaces,
two I2C interfaces, two SSP ports, Secure Digital interface, and a 12-bit
A/D converter in addition to many other features.

Faraday A369 EVB provides the on-board DDR2-SODIMM (512 MB),
NAND Flash (256 MB), SPI Flash ROM (16 MB), I2C EPROM, Giga-bit Ethernet,
IDE, SATA host, OTG 2.0 and USB 2.0 host connectors, video output interface,
TV encoder for video output, LCD controller interface, SD/MMC card reader
interface, I2S codec, UART(X2), Multi-ICE, 80 ports, and an AHB extension bus.

Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
---
 hw/a360.c            |  271 +++++++++++++++++++++++
 hw/a369.c            |  581 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/arm/Makefile.objs |    6 +
 hw/faraday.h         |   21 ++
 4 files changed, 879 insertions(+)
 create mode 100644 hw/a360.c
 create mode 100644 hw/a369.c
 create mode 100644 hw/faraday.h

Blue Swirl Jan. 25, 2013, 8:19 p.m. UTC | #1

On Fri, Jan 25, 2013 at 8:19 AM, Kuo-Jung Su <dantesu@gmail.com> wrote:
> From: Kuo-Jung Su <dantesu@faraday-tech.com>
>
> The Faraday A360/A369 EVB is a Faraday platform main board used for the
> Faraday IP functional verification based on the well-known ARM AMBA 2.0
> architecture. This main board provides a fully verified microprocessor
> platform, ATA-II, OTG 2.0 and USB 2.0 host connectivity.
>
> Faraday A360 EVB provides the on-board DDR2-SODIMM (256 MB),
> NAND Flash (128 MB), USB2.0 interface, two PCI Express interfaces,
> two I2C interfaces, two SSP ports, Secure Digital interface, and a 12-bit
> A/D converter in addition to many other features.
>
> Faraday A369 EVB provides the on-board DDR2-SODIMM (512 MB),
> NAND Flash (256 MB), SPI Flash ROM (16 MB), I2C EPROM, Giga-bit Ethernet,
> IDE, SATA host, OTG 2.0 and USB 2.0 host connectors, video output interface,
> TV encoder for video output, LCD controller interface, SD/MMC card reader
> interface, I2S codec, UART(X2), Multi-ICE, 80 ports, and an AHB extension bus.
>
> Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
> ---
>  hw/a360.c            |  271 +++++++++++++++++++++++
>  hw/a369.c            |  581 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/arm/Makefile.objs |    6 +
>  hw/faraday.h         |   21 ++
>  4 files changed, 879 insertions(+)
>  create mode 100644 hw/a360.c
>  create mode 100644 hw/a369.c
>  create mode 100644 hw/faraday.h
>
> diff --git a/hw/a360.c b/hw/a360.c
> new file mode 100644
> index 0000000..cb0a588
> --- /dev/null
> +++ b/hw/a360.c
> @@ -0,0 +1,271 @@
> +/*
> + * Faraday A360 Evalution Board
> + *
> + * Copyright (c) 2012 Faraday Technology
> + * Written by Dante Su <dantesu@faraday-tech.com>
> + *
> + * This code is licensed under GNU GPL v2.
> + */
> +
> +#include "sysbus.h"
> +#include "arm-misc.h"
> +#include "devices.h"
> +#include "net/net.h"
> +#include "sysemu/sysemu.h"
> +#include "sysemu/blockdev.h"
> +#include "exec/address-spaces.h"
> +#include "i2c.h"
> +#include "boards.h"
> +#include "flash.h"
> +#include "serial.h"
> +#include "ssi.h"
> +#include "faraday.h"
> +
> +typedef struct A360State {
> +    ARMCPU       *cpu;
> +    DeviceState  *hdma;    /* AHB DMA */
> +    DeviceState  *pdma;    /* APB DMA */
> +
> +    MemoryRegion *as;
> +    MemoryRegion *ram;
> +
> +    i2c_bus      *i2c[2];
> +
> +    struct {
> +        MemoryRegion iomem;
> +    } pmu;
> +} a360_state;

The typedef names should be also in CamelCase.

> +
> +/* PMU block */
> +static uint64_t
> +a360_pmu_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    uint64_t ret = 0;
> +
> +    switch (addr) {
> +    case 0x30:
> +        ret = 27 << 3;
> +        break;
> +    }
> +
> +    return ret;
> +}
> +
> +static void
> +a360_pmu_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
> +{
> +}
> +
> +static const MemoryRegionOps a360_pmu_ops = {
> +    .read  = a360_pmu_read,
> +    .write = a360_pmu_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +static int
> +a360_pmu_init(a360_state *s, hwaddr base)
> +{
> +    MemoryRegion *iomem = &s->pmu.iomem;
> +    memory_region_init_io(iomem, &a360_pmu_ops, s, "a360_pmu", 0x1000);
> +    memory_region_add_subregion(s->as, base, iomem);
> +    return 0;
> +}
> +
> +/* Board init. */
> +static void
> +a360_device_init(a360_state *s)
> +{
> +    qemu_irq *pic;
> +    qemu_irq ack, req;
> +    qemu_irq cs_line;
> +    DeviceState *ds;
> +    int i, done_nic = 0, nr_flash = 1;
> +    SSIBus *spi;
> +    DeviceState *fl;
> +
> +    /* Interrupt Controller */
> +    pic = ftintc020_init(0x98800000, s->cpu);
> +
> +    /* Timer */
> +    ds = qdev_create(NULL, "fttmr010");
> +    qdev_prop_set_uint32(ds, "freq", 66 * 1000000);
> +    qdev_init_nofail(ds);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(ds), 0, 0x98400000);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 0, pic[42]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 1, pic[19]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 2, pic[14]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 3, pic[15]);
> +
> +    /* Serial (FTUART010 which is 16550A compatible) */
> +    if (serial_hds[0]) {
> +        serial_mm_init(s->as,
> +                       0x98200000,
> +                       2,
> +                       pic[10],
> +                       18432000 / 16,
> +                       serial_hds[0],
> +                       DEVICE_LITTLE_ENDIAN);
> +    }
> +    if (serial_hds[1]) {
> +        serial_mm_init(s->as,
> +                       0x98300000,
> +                       2,
> +                       pic[11],
> +                       18432000 / 16,
> +                       serial_hds[1],
> +                       DEVICE_LITTLE_ENDIAN);
> +    }
> +
> +    /* pmu */
> +    a360_pmu_init(s, 0x98100000);
> +
> +    /* ftdmac020 */
> +    s->hdma = sysbus_create_varargs("ftdmac020",
> +                                    0x90400000,
> +                                    pic[21], /* ALL */
> +                                    pic[40], /* TC */
> +                                    pic[41], /* ERR */
> +                                    NULL);
> +
> +    /* ftapbbrg020 */
> +    s->pdma = sysbus_create_simple("ftapbbrg020", 0x90500000, pic[24]);
> +
> +    /* ftsdc010 */
> +    ds = sysbus_create_simple("ftsdc010", 0x90700000, pic[5]);
> +    ack = qdev_get_gpio_in(ds, 0);
> +    req = qdev_get_gpio_in(s->hdma, 0);
> +    qdev_connect_gpio_out(s->hdma, 0, ack);
> +    qdev_connect_gpio_out(ds, 0, req);
> +
> +    /* fusbh200 */
> +    sysbus_create_simple("faraday-ehci-usb", 0x90A00000, pic[23]);
> +
> +    /* fotg210 */
> +    sysbus_create_simple("faraday-ehci-usb", 0x90B00000, pic[26]);
> +
> +    /* ftmac110 */
> +    for (i = 0; i < nb_nics; i++) {
> +        NICInfo *nd = &nd_table[i];
> +        if (!done_nic && (!nd->model || strcmp(nd->model, "ftmac110") == 0)) {
> +            ftmac110_init(nd, 0x90900000, pic[25]);
> +            done_nic = 1;
> +        }
> +    }
> +
> +    /* ftwdt010 */
> +    sysbus_create_simple("ftwdt010", 0x98500000, pic[16]);
> +
> +    /* ftlcdc200 */
> +    sysbus_create_varargs("ftlcdc200",
> +                          0x90600000,
> +                          pic[27],  /* Global */
> +                          NULL);
> +
> +    /* fti2c010 */
> +    ds = sysbus_create_simple("fti2c010", 0x98A00000, pic[3]);
> +    s->i2c[0] = (i2c_bus *)qdev_get_child_bus(ds, "i2c");
> +    ds = sysbus_create_simple("fti2c010", 0x98C00000, pic[22]);
> +    s->i2c[1] = (i2c_bus *)qdev_get_child_bus(ds, "i2c");
> +
> +    /* ftssp010 */
> +    ds = qdev_create(NULL, "ftssp010");
> +
> +    /* i2s */
> +    qdev_prop_set_ptr(ds, "codec_i2c", s->i2c[0]);
> +    qdev_init_nofail(ds);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(ds), 0, 0x98B00000);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 0, pic[2]);
> +
> +    /* spi */
> +    spi = (SSIBus *)qdev_get_child_bus(ds, "spi");
> +    for (i = 0; i < nr_flash; i++) {
> +        fl = ssi_create_slave_no_init(spi, "m25p80");
> +        qdev_prop_set_string(fl, "partname", "w25q64");
> +        qdev_init_nofail(fl);
> +        cs_line = qdev_get_gpio_in(fl, 0);
> +        sysbus_connect_irq(SYS_BUS_DEVICE(ds), i+1, cs_line);
> +    }
> +
> +    /* DMA (Tx) */
> +    ack = qdev_get_gpio_in(ds, 0);
> +    req = qdev_get_gpio_in(s->pdma, 1);
> +    qdev_connect_gpio_out(s->pdma, 1, ack);
> +    qdev_connect_gpio_out(ds, 0, req);
> +
> +    /* DMA (Rx) */
> +    ack = qdev_get_gpio_in(ds, 1);
> +    req = qdev_get_gpio_in(s->pdma, 2);
> +    qdev_connect_gpio_out(s->pdma, 2, ack);
> +    qdev_connect_gpio_out(ds, 1, req);
> +}
> +
> +static void
> +a360_board_reset(void *opaque)
> +{
> +    a360_state *s = opaque;
> +
> +    cpu_reset(CPU(s->cpu));
> +}
> +
> +static void
> +a360_board_init(QEMUMachineInitArgs *args)
> +{
> +    struct arm_boot_info *bi = NULL;
> +    a360_state *s = g_new(a360_state, 1);
> +
> +    s->as = get_system_memory();
> +    s->ram  = g_new(MemoryRegion, 1);
> +
> +    /* CPU */
> +    if (!args->cpu_model) {
> +        args->cpu_model = "fa626te";
> +    }
> +
> +    s->cpu = cpu_arm_init(args->cpu_model);
> +    if (!s->cpu) {
> +        fprintf(stderr, "Unable to find CPU definition\n");
> +        exit(1);
> +    }
> +
> +    /* A360 supports upto 1GB ram space */
> +    if (args->ram_size > 0x40000000) {
> +        args->ram_size = 0x40000000;
> +    }
> +
> +    printf("qemu: faraday a360 with %dMB ram.\n", args->ram_size >> 20);

Please turn printfs into debug versions or tracepoints.

> +
> +    /* RAM Init */
> +    memory_region_init_ram(s->ram, "a360.ram", args->ram_size);
> +    vmstate_register_ram_global(s->ram);
> +
> +    a360_device_init(s);
> +    qemu_register_reset(a360_board_reset, s);
> +
> +    /* Prepare for direct boot from linux kernel or u-boot elf */
> +    bi = g_new0(struct arm_boot_info, 1);
> +
> +    /* RAM Address Binding */
> +    memory_region_add_subregion(s->as, 0x00000000, s->ram);
> +
> +    /* Boot Info */
> +    bi->ram_size = args->ram_size;
> +    bi->kernel_filename = args->kernel_filename;
> +    bi->kernel_cmdline = args->kernel_cmdline;
> +    bi->initrd_filename = args->initrd_filename;
> +    bi->board_id = 0xa360;
> +    arm_load_kernel(s->cpu, bi);
> +}
> +
> +static QEMUMachine a360_machine = {
> +    .name = "a360",
> +    .desc = "Faraday A360 (fa626te)",
> +    .init = a360_board_init,
> +};
> +
> +static void
> +a360_machine_init(void)
> +{
> +    qemu_register_machine(&a360_machine);
> +}
> +
> +machine_init(a360_machine_init);
> diff --git a/hw/a369.c b/hw/a369.c
> new file mode 100644
> index 0000000..646888f
> --- /dev/null
> +++ b/hw/a369.c
> @@ -0,0 +1,581 @@
> +/*
> + * Faraday A369 Evalution Board
> + *
> + * Copyright (c) 2012 Faraday Technology
> + * Written by Dante Su <dantesu@faraday-tech.com>
> + *
> + * This code is licensed under GNU GPL v2.
> + */
> +
> +#include "sysbus.h"
> +#include "arm-misc.h"
> +#include "devices.h"
> +#include "net/net.h"
> +#include "sysemu/sysemu.h"
> +#include "sysemu/blockdev.h"
> +#include "exec/address-spaces.h"
> +#include "i2c.h"
> +#include "boards.h"
> +#include "flash.h"
> +#include "serial.h"
> +#include "ssi.h"
> +#include "faraday.h"
> +
> +#define A369_NOR_FLASH_ADDR         0x20000000
> +#define A369_NOR_FLASH_SIZE         (16 * 1024 * 1024)
> +#define A369_NOR_FLASH_SECT_SIZE    (128 * 1024)
> +
> +typedef struct A369State {
> +    ARMCPU       *cpu;
> +    DeviceState  *rom;
> +    DeviceState  *hdma[2];    /* AHB DMA */
> +    DeviceState  *pdma;    /* APB DMA */
> +
> +    MemoryRegion *as;
> +    MemoryRegion *ram;
> +    MemoryRegion *ram_alias;
> +    MemoryRegion *sram;
> +
> +    i2c_bus      *i2c[2];
> +
> +    struct {
> +        MemoryRegion iomem;
> +        /* HW register cache */
> +        uint32_t general_cfg;
> +        uint32_t sclk_cfg0;
> +        uint32_t sclk_cfg1;
> +        uint32_t mfpsr0;
> +        uint32_t mfpsr1;
> +    } scu;
> +
> +    struct {
> +        MemoryRegion iomem;
> +        /* HW register cache */
> +        uint32_t cr;
> +    } ahbc;
> +
> +    struct {
> +        MemoryRegion iomem;
> +        /* HW register cache */
> +        uint32_t mcr;
> +        uint32_t msr;
> +    } ddrc;
> +
> +} a369_state;
> +
> +/* SCU block */
> +
> +static uint64_t
> +a369_scu_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    a369_state *s = opaque;
> +
> +    switch (addr) {
> +    case 0x000: return 0x00003369;
> +    case 0x004: return 0x00010000;
> +    case 0x008: return 0x00000c10;
> +    case 0x00c: return 0x00000230;
> +    case 0x010: return 0x00000083;
> +    case 0x014: return 0x00000100;
> +    case 0x01C: return 0x00000003;
> +    case 0x020: return 0x20010003;
> +    case 0x024: return 0x00000003;
> +    case 0x060: return 0x00280028;
> +    case 0x200: return s->scu.general_cfg;
> +    case 0x204: return 0x00001cc8;
> +    case 0x228: return s->scu.sclk_cfg0;
> +    case 0x22c: return s->scu.sclk_cfg1;
> +    case 0x230: return 0x00003fff;
> +    case 0x238: return s->scu.mfpsr0;
> +    case 0x23c: return s->scu.mfpsr1;
> +    case 0x240: return 0x11111111;
> +    case 0x244: return 0x11111111;
> +    case 0x254: return 0x00000303;
> +    case 0x258: return 0x8000007f;
> +    }
> +
> +    return 0;
> +}
> +
> +static void
> +a369_scu_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
> +{
> +    a369_state *s = opaque;
> +
> +    switch (addr) {
> +    case 0x200:
> +        s->scu.general_cfg = (uint32_t)val;
> +        break;
> +    case 0x228:
> +        s->scu.sclk_cfg0 = (uint32_t)val;
> +        break;
> +    case 0x22c:
> +        s->scu.sclk_cfg1 = (uint32_t)val;
> +        break;
> +    case 0x238:
> +        s->scu.mfpsr0 = (uint32_t)val;
> +        break;
> +    case 0x23c:
> +        s->scu.mfpsr1 = (uint32_t)val;
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps a369_scu_ops = {
> +    .read  = a369_scu_read,
> +    .write = a369_scu_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +    .valid = {
> +        .min_access_size = 4,
> +        .max_access_size = 4
> +    }
> +};
> +
> +static int
> +a369_scu_init(a369_state *s, hwaddr base)
> +{
> +    MemoryRegion *iomem = &s->scu.iomem;
> +
> +    memory_region_init_io(iomem, &a369_scu_ops, s, "a369_scu", 0x1000);
> +    memory_region_add_subregion(s->as, base, iomem);
> +
> +    s->scu.general_cfg = 0x00001078;
> +    s->scu.sclk_cfg0   = 0x26877330;
> +    s->scu.sclk_cfg1   = 0x000a0a0a;
> +    s->scu.mfpsr0      = 0x00000241;
> +    s->scu.mfpsr1      = 0x00000000;
> +
> +    return 0;
> +}
> +
> +/* AHB controller block */
> +
> +static uint64_t
> +a369_ahbc_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    a369_state *s = opaque;
> +
> +    switch (addr) {
> +    case 0x00: return 0x94050000;
> +    case 0x04: return 0x96040000;
> +    case 0x08: return 0x90f00000;
> +    case 0x0c: return 0x92050000;
> +    case 0x10: return 0x20080000;
> +    case 0x14: return 0xc0080000;
> +    case 0x18: return 0x00090000;
> +    case 0x1c: return 0x90000000;
> +    case 0x20: return 0x90100000;
> +    case 0x24: return 0x90200000;
> +    case 0x28: return 0x90300000;
> +    case 0x2c: return 0x90400000;
> +    case 0x30: return 0x90500000;
> +    case 0x34: return 0x90600000;
> +    case 0x38: return 0x90700000;
> +    case 0x3c: return 0x90800000;
> +    case 0x40: return 0x90900000;
> +    case 0x44: return 0x90a00000;
> +    case 0x48: return 0x90b00000;
> +    case 0x4c: return 0x90c00000;
> +    case 0x50: return 0x90d00000;
> +    case 0x54: return 0x90e00000;
> +    case 0x58: return 0x40080000;
> +    case 0x5c: return 0x60080000;
> +    case 0x60: return 0xa0000000;
> +    case 0x84: return 0x00000001;
> +    case 0x88: return s->ahbc.cr;
> +    case 0x8c: return 0x00010301;
> +    }
> +
> +    return 0;
> +}
> +
> +static void
> +a369_ahbc_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
> +{
> +    a369_state *s = opaque;
> +
> +    switch (addr) {
> +    case 0x88:
> +        if (!(s->ahbc.cr & 0x01) && (val & 0x01)) {
> +            /* AHB remap */
> +            sysbus_mmio_map(SYS_BUS_DEVICE(s->rom), 0, 0x40000000);
> +            memory_region_del_subregion(s->as, s->ram_alias);
> +            memory_region_add_subregion(s->as, 0x00000000, s->ram);
> +        }
> +        s->ahbc.cr = (uint32_t)val;
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps a369_ahbc_ops = {
> +    .read  = a369_ahbc_read,
> +    .write = a369_ahbc_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +static int
> +a369_ahbc_init(a369_state *s, hwaddr base)
> +{
> +    MemoryRegion *iomem = &s->ahbc.iomem;
> +
> +    memory_region_init_io(iomem, &a369_ahbc_ops, s, "a369_ahbc", 0x1000);
> +    memory_region_add_subregion(s->as, base, iomem);
> +
> +    s->ahbc.cr = 0;
> +
> +    return 0;
> +}
> +
> +/* DDRII controller block */
> +
> +static uint64_t
> +a369_ddrc_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    a369_state *s = opaque;
> +
> +    switch (addr) {
> +    case 0x00:  return s->ddrc.mcr;
> +    case 0x04:  return s->ddrc.msr;
> +    }
> +
> +    return 0;
> +}
> +
> +static void
> +a369_ddrc_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
> +{
> +    a369_state *s = opaque;
> +
> +    switch (addr) {
> +    case 0x00:
> +        s->ddrc.mcr = (uint32_t)val & 0xffff;
> +        break;
> +    case 0x04:
> +        val = (val & 0x3f) | (s->ddrc.msr & 0x100);
> +        if (!(s->ddrc.msr & 0x100) && (val & 0x01)) {
> +            val &= 0xfffffffe;
> +            val |= 0x100;
> +            memory_region_add_subregion(s->as, 0x10000000, s->ram_alias);
> +        }
> +        s->ddrc.msr = (uint32_t)val;
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps a369_ddrc_ops = {
> +    .read  = a369_ddrc_read,
> +    .write = a369_ddrc_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +static int
> +a369_ddrc_init(a369_state *s, hwaddr base)
> +{
> +    MemoryRegion *iomem = &s->ddrc.iomem;
> +
> +    memory_region_init_io(iomem, &a369_ddrc_ops, s, "a369_ddrc", 0x1000);
> +    memory_region_add_subregion(s->as, base, iomem);
> +
> +    s->ddrc.mcr = 0;
> +    s->ddrc.msr = 0;
> +
> +    return 0;
> +}
> +
> +/* Board init.  */
> +
> +static void
> +a369_device_init(a369_state *s)
> +{
> +    qemu_irq *pic;
> +    qemu_irq ack, req;
> +    qemu_irq cs_line;
> +    DeviceState *ds;
> +    int i, done_nic = 0, nr_flash = 1;
> +    SSIBus *spi;
> +    DeviceState *fl;
> +    DriveInfo *dinfo;
> +
> +    /* Interrupt Controller */
> +    pic = ftintc020_init(0x90100000, s->cpu);
> +
> +    /* Timer */
> +    ds = qdev_create(NULL, "ftpwmtmr010");
> +    qdev_prop_set_uint32(ds, "freq", 66 * 1000000);
> +    qdev_init_nofail(ds);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(ds), 0, 0x92300000);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 0, pic[8]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 1, pic[9]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 2, pic[10]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 3, pic[11]);
> +
> +    /* Serial (FTUART010 which is 16550A compatible) */
> +    if (serial_hds[0]) {
> +        serial_mm_init(s->as,
> +                       0x92b00000,
> +                       2,
> +                       pic[53],
> +                       18432000 / 16,
> +                       serial_hds[0],
> +                       DEVICE_LITTLE_ENDIAN);
> +    }
> +    if (serial_hds[1]) {
> +        serial_mm_init(s->as,
> +                       0x92c00000,
> +                       2,
> +                       pic[54],
> +                       18432000 / 16,
> +                       serial_hds[1],
> +                       DEVICE_LITTLE_ENDIAN);
> +    }
> +
> +    /* ftscu010 */
> +    a369_scu_init(s, 0x92000000);
> +
> +    /* ftddrII030 */
> +    a369_ddrc_init(s, 0x93100000);
> +
> +    /* ftahbc020 */
> +    a369_ahbc_init(s, 0x94000000);
> +
> +    /* ftdmac020 */
> +    s->hdma[0] = sysbus_create_varargs("ftdmac020",
> +                                       0x90300000,
> +                                       pic[0],  /* ALL (NC in A369) */
> +                                       pic[15], /* TC */
> +                                       pic[16], /* ERR */
> +                                       NULL);
> +    s->hdma[1] = sysbus_create_varargs("ftdmac020",
> +                                       0x96100000,
> +                                       pic[0],  /* ALL (NC in A369) */
> +                                       pic[17], /* TC */
> +                                       pic[18], /* ERR */
> +                                       NULL);
> +
> +    /* ftapbbrg020 */
> +    s->pdma = sysbus_create_simple("ftapbbrg020", 0x90f00000, pic[14]);
> +
> +    /* ftnandc021 */
> +    ds = sysbus_create_simple("ftnandc021", 0x90200000, pic[30]);
> +    ack = qdev_get_gpio_in(ds, 0);
> +    req = qdev_get_gpio_in(s->hdma[0], 15);
> +    qdev_connect_gpio_out(s->hdma[0], 15, ack);
> +    qdev_connect_gpio_out(ds, 0, req);
> +
> +    /* ftsdc010 */
> +#if 0
> +    ds = sysbus_create_simple("ftsdc010", 0x90500000, pic[38]);
> +    ack = qdev_get_gpio_in(ds, 0);
> +    req = qdev_get_gpio_in(s->hdma[0], 14);
> +    qdev_connect_gpio_out(s->hdma[0], 14, ack);
> +    qdev_connect_gpio_out(ds, 0, req);
> +#endif

Remove disabled code.

> +
> +    ds = sysbus_create_simple("ftsdc010", 0x90600000, pic[39]);
> +    ack = qdev_get_gpio_in(ds, 0);
> +    req = qdev_get_gpio_in(s->hdma[0], 13);
> +    qdev_connect_gpio_out(s->hdma[0], 13, ack);
> +    qdev_connect_gpio_out(ds, 0, req);
> +
> +    /* fusbh200 */
> +    sysbus_create_simple("faraday-ehci-usb", 0x90800000, pic[36]);
> +
> +    /* fotg210 */
> +    sysbus_create_simple("faraday-ehci-usb", 0x90900000, pic[37]);
> +
> +    /* ftgmac100 */
> +    for (i = 0; i < nb_nics; i++) {
> +        NICInfo *nd = &nd_table[i];
> +        if (!done_nic && (!nd->model || strcmp(nd->model, "ftgmac100") == 0)) {
> +            ftgmac100_init(nd, 0x90c00000, pic[32]);
> +            done_nic = 1;
> +        }
> +    }
> +
> +    /* ftrtc011 (only alarm interrupt is connected) */
> +    sysbus_create_varargs("ftrtc011",
> +                          0x92100000,
> +                          pic[0],     /* Alarm (Level): NC in A369 */
> +                          pic[42],    /* Alarm (Edge) */
> +                          pic[43],    /* Second (Edge) */
> +                          pic[44],    /* Minute (Edge) */
> +                          pic[45],    /* Hour (Edge) */
> +                          NULL);
> +
> +    /* ftwdt010 */
> +    sysbus_create_simple("ftwdt010", 0x92200000, pic[46]);
> +
> +    /* fttsc010 */
> +    sysbus_create_simple("fttsc010", 0x92400000, pic[19]);
> +
> +    /* ftkbc010 */
> +    sysbus_create_simple("ftkbc010", 0x92f00000, pic[21]);
> +
> +    /* ftlcdc200 */
> +    sysbus_create_varargs("ftlcdc200",
> +                          0x94a00000,
> +                          pic[0],  /* ALL (NC in A369) */
> +                          pic[25], /* VSTATUS */
> +                          pic[24], /* Base Address Update */
> +                          pic[23], /* FIFO Under-Run */
> +                          pic[22], /* AHB Bus Error */
> +                          NULL);
> +
> +    /* fti2c010 */
> +    ds = sysbus_create_simple("fti2c010", 0x92900000, pic[51]);
> +    s->i2c[0] = (i2c_bus *)qdev_get_child_bus(ds, "i2c");
> +    ds = sysbus_create_simple("fti2c010", 0x92A00000, pic[52]);
> +    s->i2c[1] = (i2c_bus *)qdev_get_child_bus(ds, "i2c");
> +
> +    /* ftssp010 */
> +    ds = qdev_create(NULL, "ftssp010");
> +
> +    /* i2s */
> +    qdev_prop_set_ptr(ds, "codec_i2c", s->i2c[0]);
> +    qdev_init_nofail(ds);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(ds), 0, 0x92700000);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 0, pic[49]);
> +
> +    /* spi */
> +    spi = (SSIBus *)qdev_get_child_bus(ds, "spi");
> +    for (i = 0; i < nr_flash; i++) {
> +        fl = ssi_create_slave_no_init(spi, "m25p80");
> +        qdev_prop_set_string(fl, "partname", "w25q64");
> +        qdev_init_nofail(fl);
> +        cs_line = qdev_get_gpio_in(fl, 0);
> +        sysbus_connect_irq(SYS_BUS_DEVICE(ds), i+1, cs_line);
> +    }
> +
> +    /* DMA (Tx) */
> +    ack = qdev_get_gpio_in(ds, 0);
> +    req = qdev_get_gpio_in(s->pdma, 7);
> +    qdev_connect_gpio_out(s->pdma, 7, ack);
> +    qdev_connect_gpio_out(ds, 0, req);
> +
> +    /* DMA (Rx) */
> +    ack = qdev_get_gpio_in(ds, 1);
> +    req = qdev_get_gpio_in(s->pdma, 8);
> +    qdev_connect_gpio_out(s->pdma, 8, ack);
> +    qdev_connect_gpio_out(ds, 1, req);
> +
> +    /* Parallel NOR Flash */
> +    dinfo = drive_get_next(IF_PFLASH);
> +    if (!pflash_cfi01_register(
> +                    A369_NOR_FLASH_ADDR,
> +                    NULL,
> +                    "a369.pflash",
> +                    A369_NOR_FLASH_SIZE,
> +                    dinfo ? dinfo->bdrv : NULL,
> +                    A369_NOR_FLASH_SECT_SIZE,
> +                    A369_NOR_FLASH_SIZE / A369_NOR_FLASH_SECT_SIZE,
> +                    2, 0x0001, 0x227E, 0x2101, 0x0, 0)) {
> +        hw_error("qemu: Error registering flash memory.\n");
> +        exit(1);
> +    }
> +}
> +
> +static void
> +a369_board_reset(void *opaque)
> +{
> +    a369_state *s = opaque;
> +
> +    if (s->ddrc.msr) {
> +        s->ddrc.mcr = 0;
> +        s->ddrc.msr = 0;
> +        if (s->ahbc.cr) {
> +            /* AHB remapped */
> +            sysbus_mmio_map(SYS_BUS_DEVICE(s->rom), 0, 0x00000000);
> +            memory_region_del_subregion(s->as, s->ram);
> +        } else {
> +            /* AHB is not yet remapped, but SDRAM is ready */
> +            memory_region_del_subregion(s->as, s->ram_alias);
> +        }
> +        s->ahbc.cr = 0;
> +    }
> +
> +    cpu_reset(CPU(s->cpu));
> +}
> +
> +static void
> +a369_board_init(QEMUMachineInitArgs *args)
> +{
> +    struct arm_boot_info *bi = NULL;
> +    a369_state *s = g_new(a369_state, 1);
> +
> +    s->as = get_system_memory();
> +    s->ram  = g_new(MemoryRegion, 1);
> +    s->sram = g_new(MemoryRegion, 1);
> +
> +    /* CPU */
> +    if (!args->cpu_model) {
> +        args->cpu_model = "fa626te";
> +    }
> +
> +    s->cpu = cpu_arm_init(args->cpu_model);
> +    if (!s->cpu) {
> +        fprintf(stderr, "Unable to find CPU definition\n");
> +        exit(1);
> +    }
> +
> +    /* A369 supports upto 1GB ram space */
> +    if (args->ram_size > 0x40000000) {
> +        args->ram_size = 0x40000000;
> +    }
> +
> +    printf("qemu: faraday a369 with %dMB ram.\n", args->ram_size >> 20);
> +
> +    /* Embedded ROM Init */
> +    s->rom = qdev_create(NULL, "rom");
> +    qdev_prop_set_uint32(s->rom, "size", 8192);
> +    qdev_init_nofail(s->rom);
> +
> +    /* Embedded RAM Init */
> +    memory_region_init_ram(s->sram, "a369.sram", 0x4000);
> +    vmstate_register_ram_global(s->sram);
> +    memory_region_add_subregion(s->as, 0xA0000000, s->sram);
> +
> +    /* RAM Init */
> +    memory_region_init_ram(s->ram, "a369.ram", args->ram_size);
> +    vmstate_register_ram_global(s->ram);
> +
> +    a369_device_init(s);
> +    qemu_register_reset(a369_board_reset, s);
> +
> +    if (args->kernel_filename) {
> +        bi = g_new0(struct arm_boot_info, 1);
> +
> +        /* RAM Address Binding */
> +        memory_region_add_subregion(s->as, 0x00000000, s->ram);
> +
> +        /* Boot Info */
> +        bi->ram_size = args->ram_size;
> +        bi->kernel_filename = args->kernel_filename;
> +        bi->kernel_cmdline = args->kernel_cmdline;
> +        bi->initrd_filename = args->initrd_filename;
> +        bi->board_id = 0xa369;
> +        arm_load_kernel(s->cpu, bi);
> +    } else {
> +        /* ROM Address Binding */
> +        sysbus_mmio_map(SYS_BUS_DEVICE(s->rom), 0, 0x00000000);
> +        /* Partial RAM (before ahb remapped) Address Binding */
> +        s->ram_alias = g_new(MemoryRegion, 1);
> +        memory_region_init_alias(s->ram_alias, "a369.ram_alias",
> +                                 s->ram,
> +                                 0,
> +                                 MIN(0x10000000, args->ram_size));
> +    }
> +}
> +
> +static QEMUMachine a369_machine = {
> +    .name = "a369",
> +    .desc = "Faraday A369 (fa626te)",
> +    .init = a369_board_init,
> +};
> +
> +static void
> +a369_machine_init(void)
> +{
> +    qemu_register_machine(&a369_machine);
> +}
> +
> +machine_init(a369_machine_init);
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index 6d049e7..c7bb10e 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -1,4 +1,10 @@
>  obj-y = integratorcp.o versatilepb.o arm_pic.o
> +obj-y += a360.o a369.o \
> +                               rom.o ftdmac020.o ftapbbrg020.o \
> +                               ftintc020.o fttmr010.o ftpwmtmr010.o \
> +                               ftspi020.o ftssp010.o fti2c010.o \
> +                               ftrtc011.o ftwdt010.o ftmac110.o ftgmac100.o ftlcdc200.o \
> +                               fttsc010.o ftkbc010.o ftnandc021.o ftsdc010.o
>  obj-y += arm_boot.o
>  obj-y += xilinx_zynq.o zynq_slcr.o
>  obj-y += xilinx_spips.o
> diff --git a/hw/faraday.h b/hw/faraday.h
> new file mode 100644
> index 0000000..f4fe0cc
> --- /dev/null
> +++ b/hw/faraday.h
> @@ -0,0 +1,21 @@
> +/*
> + * Faraday SoC platform support.
> + *
> + * Copyright (c) 2013 Faraday Technology
> + * Written by Kuo-Jung Su <dantesu@gmail.com>
> + *
> + * This code is licensed under the GNU GPL v2.
> + */
> +#ifndef FARADAY_H
> +#define FARADAY_H
> +
> +/* ftintc020.c */
> +qemu_irq *ftintc020_init(hwaddr base, ARMCPU *cpu);
> +
> +/* ftgmac100.c */
> +void ftgmac100_init(NICInfo *, uint32_t, qemu_irq);

Please add the parameter names, they also document how to use the function.

> +
> +/* ftmac110.c */
> +void ftmac110_init(NICInfo *, uint32_t, qemu_irq);
> +
> +#endif
> --
> 1.7.9.5
>

Mitsyanko Igor Jan. 31, 2013, 7:12 p.m. UTC | #2

Hi, Kuo-Jung, please see several comments bellow.


On 01/25/2013 12:19 PM, Kuo-Jung Su wrote:
> From: Kuo-Jung Su <dantesu@faraday-tech.com>
>
> The Faraday A360/A369 EVB is a Faraday platform main board used for the
> Faraday IP functional verification based on the well-known ARM AMBA 2.0
> architecture. This main board provides a fully verified microprocessor
> platform, ATA-II, OTG 2.0 and USB 2.0 host connectivity.
>
> Faraday A360 EVB provides the on-board DDR2-SODIMM (256 MB),
> NAND Flash (128 MB), USB2.0 interface, two PCI Express interfaces,
> two I2C interfaces, two SSP ports, Secure Digital interface, and a 12-bit
> A/D converter in addition to many other features.
>
> Faraday A369 EVB provides the on-board DDR2-SODIMM (512 MB),
> NAND Flash (256 MB), SPI Flash ROM (16 MB), I2C EPROM, Giga-bit Ethernet,
> IDE, SATA host, OTG 2.0 and USB 2.0 host connectors, video output interface,
> TV encoder for video output, LCD controller interface, SD/MMC card reader
> interface, I2S codec, UART(X2), Multi-ICE, 80 ports, and an AHB extension bus.
>
> Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
> ---
>   hw/a360.c            |  271 +++++++++++++++++++++++
>   hw/a369.c            |  581 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   hw/arm/Makefile.objs |    6 +
>   hw/faraday.h         |   21 ++
>   4 files changed, 879 insertions(+)
>   create mode 100644 hw/a360.c
>   create mode 100644 hw/a369.c
>   create mode 100644 hw/faraday.h
>
> diff --git a/hw/a360.c b/hw/a360.c
> new file mode 100644
> index 0000000..cb0a588
> --- /dev/null
> +++ b/hw/a360.c
> @@ -0,0 +1,271 @@
> +/*
> + * Faraday A360 Evalution Board
> + *
> + * Copyright (c) 2012 Faraday Technology
> + * Written by Dante Su <dantesu@faraday-tech.com>
> + *
> + * This code is licensed under GNU GPL v2.
> + */
> +
> +#include "sysbus.h"
> +#include "arm-misc.h"
> +#include "devices.h"
> +#include "net/net.h"
> +#include "sysemu/sysemu.h"
> +#include "sysemu/blockdev.h"
> +#include "exec/address-spaces.h"
> +#include "i2c.h"
> +#include "boards.h"
> +#include "flash.h"
> +#include "serial.h"
> +#include "ssi.h"
> +#include "faraday.h"
> +
> +typedef struct A360State {
> +    ARMCPU       *cpu;
> +    DeviceState  *hdma;    /* AHB DMA */
> +    DeviceState  *pdma;    /* APB DMA */
> +
> +    MemoryRegion *as;
> +    MemoryRegion *ram;
> +
> +    i2c_bus      *i2c[2];
> +
> +    struct {
> +        MemoryRegion iomem;
> +    } pmu;
> +} a360_state;
> +
> +/* PMU block */
> +static uint64_t
> +a360_pmu_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    uint64_t ret = 0;
> +
> +    switch (addr) {
> +    case 0x30:
> +        ret = 27 << 3;

Since we do not have documentation, it would be nice at least to have 
descriptive defines for all these numbers.


> +        break;
> +    }
> +
> +    return ret;
> +}
> +
> +static void
> +a360_pmu_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
> +{
> +}
> +
> +static const MemoryRegionOps a360_pmu_ops = {
> +    .read  = a360_pmu_read,
> +    .write = a360_pmu_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +static int
> +a360_pmu_init(a360_state *s, hwaddr base)
> +{
> +    MemoryRegion *iomem = &s->pmu.iomem;
> +    memory_region_init_io(iomem, &a360_pmu_ops, s, "a360_pmu", 0x1000);
> +    memory_region_add_subregion(s->as, base, iomem);
> +    return 0;
> +}
> +
> +/* Board init. */
> +static void
> +a360_device_init(a360_state *s)
> +{
> +    qemu_irq *pic;
> +    qemu_irq ack, req;
> +    qemu_irq cs_line;
> +    DeviceState *ds;
> +    int i, done_nic = 0, nr_flash = 1;
> +    SSIBus *spi;
> +    DeviceState *fl;
> +
> +    /* Interrupt Controller */
> +    pic = ftintc020_init(0x98800000, s->cpu);


You haven't introduced this interrupt controller yet, patches should be 
arranged in such an order that they at least wouldn't break a build.
Same goes for ftintc020_init and ftgmac100_init.


> +
> +    /* Timer */
> +    ds = qdev_create(NULL, "fttmr010");
> +    qdev_prop_set_uint32(ds, "freq", 66 * 1000000);
> +    qdev_init_nofail(ds);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(ds), 0, 0x98400000);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 0, pic[42]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 1, pic[19]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 2, pic[14]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 3, pic[15]);
> +
> +    /* Serial (FTUART010 which is 16550A compatible) */
> +    if (serial_hds[0]) {
> +        serial_mm_init(s->as,
> +                       0x98200000,
> +                       2,
> +                       pic[10],
> +                       18432000 / 16,
> +                       serial_hds[0],
> +                       DEVICE_LITTLE_ENDIAN);
> +    }
> +    if (serial_hds[1]) {
> +        serial_mm_init(s->as,
> +                       0x98300000,
> +                       2,
> +                       pic[11],
> +                       18432000 / 16,
> +                       serial_hds[1],
> +                       DEVICE_LITTLE_ENDIAN);
> +    }
> +
> +    /* pmu */
> +    a360_pmu_init(s, 0x98100000);
> +
> +    /* ftdmac020 */
> +    s->hdma = sysbus_create_varargs("ftdmac020",
> +                                    0x90400000,
> +                                    pic[21], /* ALL */
> +                                    pic[40], /* TC */
> +                                    pic[41], /* ERR */
> +                                    NULL);
> +
> +    /* ftapbbrg020 */
> +    s->pdma = sysbus_create_simple("ftapbbrg020", 0x90500000, pic[24]);
> +
> +    /* ftsdc010 */
> +    ds = sysbus_create_simple("ftsdc010", 0x90700000, pic[5]);
> +    ack = qdev_get_gpio_in(ds, 0);
> +    req = qdev_get_gpio_in(s->hdma, 0);
> +    qdev_connect_gpio_out(s->hdma, 0, ack);
> +    qdev_connect_gpio_out(ds, 0, req);
> +
> +    /* fusbh200 */
> +    sysbus_create_simple("faraday-ehci-usb", 0x90A00000, pic[23]);
> +
> +    /* fotg210 */
> +    sysbus_create_simple("faraday-ehci-usb", 0x90B00000, pic[26]);
> +
> +    /* ftmac110 */
> +    for (i = 0; i < nb_nics; i++) {
> +        NICInfo *nd = &nd_table[i];
> +        if (!done_nic && (!nd->model || strcmp(nd->model, "ftmac110") == 0)) {
> +            ftmac110_init(nd, 0x90900000, pic[25]);
> +            done_nic = 1;

What is it for? It never checked anywhere.


> +        }
> +    }
> +
> +    /* ftwdt010 */
> +    sysbus_create_simple("ftwdt010", 0x98500000, pic[16]);
> +
> +    /* ftlcdc200 */
> +    sysbus_create_varargs("ftlcdc200",
> +                          0x90600000,
> +                          pic[27],  /* Global */
> +                          NULL);


Why sysbus_create_simple() before but sysbus_create_varargs() here? 
Also, this comment is not very useful, what is it trying to say?


> +
> +    /* fti2c010 */
> +    ds = sysbus_create_simple("fti2c010", 0x98A00000, pic[3]);
> +    s->i2c[0] = (i2c_bus *)qdev_get_child_bus(ds, "i2c");
> +    ds = sysbus_create_simple("fti2c010", 0x98C00000, pic[22]);
> +    s->i2c[1] = (i2c_bus *)qdev_get_child_bus(ds, "i2c");
> +
> +    /* ftssp010 */
> +    ds = qdev_create(NULL, "ftssp010");
> +
> +    /* i2s */
> +    qdev_prop_set_ptr(ds, "codec_i2c", s->i2c[0]);
> +    qdev_init_nofail(ds);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(ds), 0, 0x98B00000);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 0, pic[2]);
> +
> +    /* spi */
> +    spi = (SSIBus *)qdev_get_child_bus(ds, "spi");
> +    for (i = 0; i < nr_flash; i++) {
> +        fl = ssi_create_slave_no_init(spi, "m25p80");
> +        qdev_prop_set_string(fl, "partname", "w25q64");
> +        qdev_init_nofail(fl);
> +        cs_line = qdev_get_gpio_in(fl, 0);
> +        sysbus_connect_irq(SYS_BUS_DEVICE(ds), i+1, cs_line);
> +    }
> +
> +    /* DMA (Tx) */
> +    ack = qdev_get_gpio_in(ds, 0);
> +    req = qdev_get_gpio_in(s->pdma, 1);
> +    qdev_connect_gpio_out(s->pdma, 1, ack);
> +    qdev_connect_gpio_out(ds, 0, req);
> +
> +    /* DMA (Rx) */
> +    ack = qdev_get_gpio_in(ds, 1);
> +    req = qdev_get_gpio_in(s->pdma, 2);
> +    qdev_connect_gpio_out(s->pdma, 2, ack);
> +    qdev_connect_gpio_out(ds, 1, req);
> +}
> +
> +static void
> +a360_board_reset(void *opaque)
> +{
> +    a360_state *s = opaque;
> +
> +    cpu_reset(CPU(s->cpu));
> +}
> +


Why is this necessary? CPU reset handler is registered in arm_load_kernel().


> +static void
> +a360_board_init(QEMUMachineInitArgs *args)
> +{
> +    struct arm_boot_info *bi = NULL;
> +    a360_state *s = g_new(a360_state, 1);
> +
> +    s->as = get_system_memory();
> +    s->ram  = g_new(MemoryRegion, 1);
> +
> +    /* CPU */
> +    if (!args->cpu_model) {
> +        args->cpu_model = "fa626te";
> +    }
> +
> +    s->cpu = cpu_arm_init(args->cpu_model);
> +    if (!s->cpu) {
> +        fprintf(stderr, "Unable to find CPU definition\n");
> +        exit(1);
> +    }
> +
> +    /* A360 supports upto 1GB ram space */
> +    if (args->ram_size > 0x40000000) {
> +        args->ram_size = 0x40000000;
> +    }
> +
> +    printf("qemu: faraday a360 with %dMB ram.\n", args->ram_size >> 20);

You should use RAM_ADDR_FMT for ram_addr_t formatting or else it would 
break build.
But usually QEMU doesn't use such messages when initialising devices, 
maybe just drop it?

> +
> +    /* RAM Init */
> +    memory_region_init_ram(s->ram, "a360.ram", args->ram_size);
> +    vmstate_register_ram_global(s->ram);
> +
> +    a360_device_init(s);
> +    qemu_register_reset(a360_board_reset, s);
> +
> +    /* Prepare for direct boot from linux kernel or u-boot elf */
> +    bi = g_new0(struct arm_boot_info, 1);
> +
> +    /* RAM Address Binding */
> +    memory_region_add_subregion(s->as, 0x00000000, s->ram);
> +
> +    /* Boot Info */
> +    bi->ram_size = args->ram_size;
> +    bi->kernel_filename = args->kernel_filename;
> +    bi->kernel_cmdline = args->kernel_cmdline;
> +    bi->initrd_filename = args->initrd_filename;
> +    bi->board_id = 0xa360;
> +    arm_load_kernel(s->cpu, bi);
> +}
> +
> +static QEMUMachine a360_machine = {
> +    .name = "a360",
> +    .desc = "Faraday A360 (fa626te)",
> +    .init = a360_board_init,


Not sure if its mandatory, but looks like you need to add 
DEFAULT_MACHINE_OPTIONS here.


> +};
> +
> +static void
> +a360_machine_init(void)
> +{
> +    qemu_register_machine(&a360_machine);
> +}
> +
> +machine_init(a360_machine_init);
> diff --git a/hw/a369.c b/hw/a369.c
> new file mode 100644
> index 0000000..646888f
> --- /dev/null
> +++ b/hw/a369.c
> @@ -0,0 +1,581 @@
> +/*
> + * Faraday A369 Evalution Board
> + *
> + * Copyright (c) 2012 Faraday Technology
> + * Written by Dante Su <dantesu@faraday-tech.com>
> + *
> + * This code is licensed under GNU GPL v2.
> + */
> +
> +#include "sysbus.h"
> +#include "arm-misc.h"
> +#include "devices.h"
> +#include "net/net.h"
> +#include "sysemu/sysemu.h"
> +#include "sysemu/blockdev.h"
> +#include "exec/address-spaces.h"
> +#include "i2c.h"
> +#include "boards.h"
> +#include "flash.h"
> +#include "serial.h"
> +#include "ssi.h"
> +#include "faraday.h"
> +
> +#define A369_NOR_FLASH_ADDR         0x20000000
> +#define A369_NOR_FLASH_SIZE         (16 * 1024 * 1024)
> +#define A369_NOR_FLASH_SECT_SIZE    (128 * 1024)
> +
> +typedef struct A369State {
> +    ARMCPU       *cpu;
> +    DeviceState  *rom;
> +    DeviceState  *hdma[2];    /* AHB DMA */
> +    DeviceState  *pdma;    /* APB DMA */
> +
> +    MemoryRegion *as;
> +    MemoryRegion *ram;
> +    MemoryRegion *ram_alias;
> +    MemoryRegion *sram;
> +
> +    i2c_bus      *i2c[2];
> +
> +    struct {
> +        MemoryRegion iomem;
> +        /* HW register cache */
> +        uint32_t general_cfg;
> +        uint32_t sclk_cfg0;
> +        uint32_t sclk_cfg1;
> +        uint32_t mfpsr0;
> +        uint32_t mfpsr1;
> +    } scu;
> +
> +    struct {
> +        MemoryRegion iomem;
> +        /* HW register cache */
> +        uint32_t cr;
> +    } ahbc;
> +
> +    struct {
> +        MemoryRegion iomem;
> +        /* HW register cache */
> +        uint32_t mcr;
> +        uint32_t msr;
> +    } ddrc;
> +
> +} a369_state;
> +
> +/* SCU block */
> +
> +static uint64_t
> +a369_scu_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    a369_state *s = opaque;
> +
> +    switch (addr) {
> +    case 0x000: return 0x00003369;
> +    case 0x004: return 0x00010000;
> +    case 0x008: return 0x00000c10;
> +    case 0x00c: return 0x00000230;
> +    case 0x010: return 0x00000083;
> +    case 0x014: return 0x00000100;
> +    case 0x01C: return 0x00000003;
> +    case 0x020: return 0x20010003;
> +    case 0x024: return 0x00000003;
> +    case 0x060: return 0x00280028;
> +    case 0x200: return s->scu.general_cfg;
> +    case 0x204: return 0x00001cc8;
> +    case 0x228: return s->scu.sclk_cfg0;
> +    case 0x22c: return s->scu.sclk_cfg1;
> +    case 0x230: return 0x00003fff;
> +    case 0x238: return s->scu.mfpsr0;
> +    case 0x23c: return s->scu.mfpsr1;
> +    case 0x240: return 0x11111111;
> +    case 0x244: return 0x11111111;
> +    case 0x254: return 0x00000303;
> +    case 0x258: return 0x8000007f;
> +    }
> +
> +    return 0;
> +}
> +
> +static void
> +a369_scu_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
> +{
> +    a369_state *s = opaque;
> +
> +    switch (addr) {
> +    case 0x200:
> +        s->scu.general_cfg = (uint32_t)val;
> +        break;
> +    case 0x228:
> +        s->scu.sclk_cfg0 = (uint32_t)val;
> +        break;
> +    case 0x22c:
> +        s->scu.sclk_cfg1 = (uint32_t)val;
> +        break;
> +    case 0x238:
> +        s->scu.mfpsr0 = (uint32_t)val;
> +        break;
> +    case 0x23c:
> +        s->scu.mfpsr1 = (uint32_t)val;
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps a369_scu_ops = {
> +    .read  = a369_scu_read,
> +    .write = a369_scu_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +    .valid = {
> +        .min_access_size = 4,
> +        .max_access_size = 4
> +    }
> +};
> +
> +static int
> +a369_scu_init(a369_state *s, hwaddr base)
> +{
> +    MemoryRegion *iomem = &s->scu.iomem;
> +
> +    memory_region_init_io(iomem, &a369_scu_ops, s, "a369_scu", 0x1000);
> +    memory_region_add_subregion(s->as, base, iomem);
> +
> +    s->scu.general_cfg = 0x00001078;
> +    s->scu.sclk_cfg0   = 0x26877330;
> +    s->scu.sclk_cfg1   = 0x000a0a0a;
> +    s->scu.mfpsr0      = 0x00000241;
> +    s->scu.mfpsr1      = 0x00000000;
> +
> +    return 0;
> +}
> +
> +/* AHB controller block */
> +
> +static uint64_t
> +a369_ahbc_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    a369_state *s = opaque;
> +
> +    switch (addr) {
> +    case 0x00: return 0x94050000;
> +    case 0x04: return 0x96040000;
> +    case 0x08: return 0x90f00000;
> +    case 0x0c: return 0x92050000;
> +    case 0x10: return 0x20080000;
> +    case 0x14: return 0xc0080000;
> +    case 0x18: return 0x00090000;
> +    case 0x1c: return 0x90000000;
> +    case 0x20: return 0x90100000;
> +    case 0x24: return 0x90200000;
> +    case 0x28: return 0x90300000;
> +    case 0x2c: return 0x90400000;
> +    case 0x30: return 0x90500000;
> +    case 0x34: return 0x90600000;
> +    case 0x38: return 0x90700000;
> +    case 0x3c: return 0x90800000;
> +    case 0x40: return 0x90900000;
> +    case 0x44: return 0x90a00000;
> +    case 0x48: return 0x90b00000;
> +    case 0x4c: return 0x90c00000;
> +    case 0x50: return 0x90d00000;
> +    case 0x54: return 0x90e00000;
> +    case 0x58: return 0x40080000;
> +    case 0x5c: return 0x60080000;
> +    case 0x60: return 0xa0000000;
> +    case 0x84: return 0x00000001;
> +    case 0x88: return s->ahbc.cr;
> +    case 0x8c: return 0x00010301;
> +    }
> +
> +    return 0;
> +}
> +
> +static void
> +a369_ahbc_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
> +{
> +    a369_state *s = opaque;
> +
> +    switch (addr) {
> +    case 0x88:
> +        if (!(s->ahbc.cr & 0x01) && (val & 0x01)) {
> +            /* AHB remap */
> +            sysbus_mmio_map(SYS_BUS_DEVICE(s->rom), 0, 0x40000000);
> +            memory_region_del_subregion(s->as, s->ram_alias);
> +            memory_region_add_subregion(s->as, 0x00000000, s->ram);
> +        }
> +        s->ahbc.cr = (uint32_t)val;
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps a369_ahbc_ops = {
> +    .read  = a369_ahbc_read,
> +    .write = a369_ahbc_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +static int
> +a369_ahbc_init(a369_state *s, hwaddr base)
> +{
> +    MemoryRegion *iomem = &s->ahbc.iomem;
> +
> +    memory_region_init_io(iomem, &a369_ahbc_ops, s, "a369_ahbc", 0x1000);
> +    memory_region_add_subregion(s->as, base, iomem);
> +
> +    s->ahbc.cr = 0;
> +
> +    return 0;
> +}
> +
> +/* DDRII controller block */
> +
> +static uint64_t
> +a369_ddrc_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    a369_state *s = opaque;
> +
> +    switch (addr) {
> +    case 0x00:  return s->ddrc.mcr;
> +    case 0x04:  return s->ddrc.msr;
> +    }
> +
> +    return 0;
> +}
> +
> +static void
> +a369_ddrc_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
> +{
> +    a369_state *s = opaque;
> +
> +    switch (addr) {
> +    case 0x00:
> +        s->ddrc.mcr = (uint32_t)val & 0xffff;
> +        break;
> +    case 0x04:
> +        val = (val & 0x3f) | (s->ddrc.msr & 0x100);
> +        if (!(s->ddrc.msr & 0x100) && (val & 0x01)) {
> +            val &= 0xfffffffe;
> +            val |= 0x100;
> +            memory_region_add_subregion(s->as, 0x10000000, s->ram_alias);
> +        }
> +        s->ddrc.msr = (uint32_t)val;
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps a369_ddrc_ops = {
> +    .read  = a369_ddrc_read,
> +    .write = a369_ddrc_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +static int
> +a369_ddrc_init(a369_state *s, hwaddr base)
> +{
> +    MemoryRegion *iomem = &s->ddrc.iomem;
> +
> +    memory_region_init_io(iomem, &a369_ddrc_ops, s, "a369_ddrc", 0x1000);
> +    memory_region_add_subregion(s->as, base, iomem);
> +
> +    s->ddrc.mcr = 0;
> +    s->ddrc.msr = 0;
> +
> +    return 0;
> +}
> +
> +/* Board init.  */
> +
> +static void
> +a369_device_init(a369_state *s)
> +{
> +    qemu_irq *pic;
> +    qemu_irq ack, req;
> +    qemu_irq cs_line;
> +    DeviceState *ds;
> +    int i, done_nic = 0, nr_flash = 1;
> +    SSIBus *spi;
> +    DeviceState *fl;
> +    DriveInfo *dinfo;
> +
> +    /* Interrupt Controller */
> +    pic = ftintc020_init(0x90100000, s->cpu);
> +
> +    /* Timer */
> +    ds = qdev_create(NULL, "ftpwmtmr010");
> +    qdev_prop_set_uint32(ds, "freq", 66 * 1000000);
> +    qdev_init_nofail(ds);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(ds), 0, 0x92300000);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 0, pic[8]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 1, pic[9]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 2, pic[10]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 3, pic[11]);
> +
> +    /* Serial (FTUART010 which is 16550A compatible) */
> +    if (serial_hds[0]) {
> +        serial_mm_init(s->as,
> +                       0x92b00000,
> +                       2,
> +                       pic[53],
> +                       18432000 / 16,
> +                       serial_hds[0],
> +                       DEVICE_LITTLE_ENDIAN);
> +    }
> +    if (serial_hds[1]) {
> +        serial_mm_init(s->as,
> +                       0x92c00000,
> +                       2,
> +                       pic[54],
> +                       18432000 / 16,
> +                       serial_hds[1],
> +                       DEVICE_LITTLE_ENDIAN);
> +    }
> +
> +    /* ftscu010 */
> +    a369_scu_init(s, 0x92000000);
> +
> +    /* ftddrII030 */
> +    a369_ddrc_init(s, 0x93100000);
> +
> +    /* ftahbc020 */
> +    a369_ahbc_init(s, 0x94000000);
> +
> +    /* ftdmac020 */
> +    s->hdma[0] = sysbus_create_varargs("ftdmac020",
> +                                       0x90300000,
> +                                       pic[0],  /* ALL (NC in A369) */
> +                                       pic[15], /* TC */
> +                                       pic[16], /* ERR */
> +                                       NULL);
> +    s->hdma[1] = sysbus_create_varargs("ftdmac020",
> +                                       0x96100000,
> +                                       pic[0],  /* ALL (NC in A369) */
> +                                       pic[17], /* TC */
> +                                       pic[18], /* ERR */
> +                                       NULL);
> +
> +    /* ftapbbrg020 */
> +    s->pdma = sysbus_create_simple("ftapbbrg020", 0x90f00000, pic[14]);
> +
> +    /* ftnandc021 */
> +    ds = sysbus_create_simple("ftnandc021", 0x90200000, pic[30]);
> +    ack = qdev_get_gpio_in(ds, 0);
> +    req = qdev_get_gpio_in(s->hdma[0], 15);
> +    qdev_connect_gpio_out(s->hdma[0], 15, ack);
> +    qdev_connect_gpio_out(ds, 0, req);
> +
> +    /* ftsdc010 */
> +#if 0
> +    ds = sysbus_create_simple("ftsdc010", 0x90500000, pic[38]);
> +    ack = qdev_get_gpio_in(ds, 0);
> +    req = qdev_get_gpio_in(s->hdma[0], 14);
> +    qdev_connect_gpio_out(s->hdma[0], 14, ack);
> +    qdev_connect_gpio_out(ds, 0, req);
> +#endif
> +
> +    ds = sysbus_create_simple("ftsdc010", 0x90600000, pic[39]);
> +    ack = qdev_get_gpio_in(ds, 0);
> +    req = qdev_get_gpio_in(s->hdma[0], 13);
> +    qdev_connect_gpio_out(s->hdma[0], 13, ack);
> +    qdev_connect_gpio_out(ds, 0, req);
> +
> +    /* fusbh200 */
> +    sysbus_create_simple("faraday-ehci-usb", 0x90800000, pic[36]);
> +
> +    /* fotg210 */
> +    sysbus_create_simple("faraday-ehci-usb", 0x90900000, pic[37]);
> +
> +    /* ftgmac100 */
> +    for (i = 0; i < nb_nics; i++) {
> +        NICInfo *nd = &nd_table[i];
> +        if (!done_nic && (!nd->model || strcmp(nd->model, "ftgmac100") == 0)) {
> +            ftgmac100_init(nd, 0x90c00000, pic[32]);
> +            done_nic = 1;
> +        }
> +    }
> +
> +    /* ftrtc011 (only alarm interrupt is connected) */
> +    sysbus_create_varargs("ftrtc011",
> +                          0x92100000,
> +                          pic[0],     /* Alarm (Level): NC in A369 */
> +                          pic[42],    /* Alarm (Edge) */
> +                          pic[43],    /* Second (Edge) */
> +                          pic[44],    /* Minute (Edge) */
> +                          pic[45],    /* Hour (Edge) */
> +                          NULL);
> +
> +    /* ftwdt010 */
> +    sysbus_create_simple("ftwdt010", 0x92200000, pic[46]);
> +
> +    /* fttsc010 */
> +    sysbus_create_simple("fttsc010", 0x92400000, pic[19]);
> +
> +    /* ftkbc010 */
> +    sysbus_create_simple("ftkbc010", 0x92f00000, pic[21]);
> +
> +    /* ftlcdc200 */
> +    sysbus_create_varargs("ftlcdc200",
> +                          0x94a00000,
> +                          pic[0],  /* ALL (NC in A369) */
> +                          pic[25], /* VSTATUS */
> +                          pic[24], /* Base Address Update */
> +                          pic[23], /* FIFO Under-Run */
> +                          pic[22], /* AHB Bus Error */
> +                          NULL);
> +
> +    /* fti2c010 */
> +    ds = sysbus_create_simple("fti2c010", 0x92900000, pic[51]);
> +    s->i2c[0] = (i2c_bus *)qdev_get_child_bus(ds, "i2c");
> +    ds = sysbus_create_simple("fti2c010", 0x92A00000, pic[52]);
> +    s->i2c[1] = (i2c_bus *)qdev_get_child_bus(ds, "i2c");
> +
> +    /* ftssp010 */
> +    ds = qdev_create(NULL, "ftssp010");
> +
> +    /* i2s */
> +    qdev_prop_set_ptr(ds, "codec_i2c", s->i2c[0]);
> +    qdev_init_nofail(ds);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(ds), 0, 0x92700000);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 0, pic[49]);
> +
> +    /* spi */
> +    spi = (SSIBus *)qdev_get_child_bus(ds, "spi");
> +    for (i = 0; i < nr_flash; i++) {
> +        fl = ssi_create_slave_no_init(spi, "m25p80");
> +        qdev_prop_set_string(fl, "partname", "w25q64");
> +        qdev_init_nofail(fl);
> +        cs_line = qdev_get_gpio_in(fl, 0);
> +        sysbus_connect_irq(SYS_BUS_DEVICE(ds), i+1, cs_line);
> +    }
> +
> +    /* DMA (Tx) */
> +    ack = qdev_get_gpio_in(ds, 0);
> +    req = qdev_get_gpio_in(s->pdma, 7);
> +    qdev_connect_gpio_out(s->pdma, 7, ack);
> +    qdev_connect_gpio_out(ds, 0, req);
> +
> +    /* DMA (Rx) */
> +    ack = qdev_get_gpio_in(ds, 1);
> +    req = qdev_get_gpio_in(s->pdma, 8);
> +    qdev_connect_gpio_out(s->pdma, 8, ack);
> +    qdev_connect_gpio_out(ds, 1, req);
> +
> +    /* Parallel NOR Flash */
> +    dinfo = drive_get_next(IF_PFLASH);
> +    if (!pflash_cfi01_register(
> +                    A369_NOR_FLASH_ADDR,
> +                    NULL,
> +                    "a369.pflash",
> +                    A369_NOR_FLASH_SIZE,
> +                    dinfo ? dinfo->bdrv : NULL,
> +                    A369_NOR_FLASH_SECT_SIZE,
> +                    A369_NOR_FLASH_SIZE / A369_NOR_FLASH_SECT_SIZE,
> +                    2, 0x0001, 0x227E, 0x2101, 0x0, 0)) {
> +        hw_error("qemu: Error registering flash memory.\n");
> +        exit(1);
> +    }
> +}
> +
> +static void
> +a369_board_reset(void *opaque)
> +{
> +    a369_state *s = opaque;
> +
> +    if (s->ddrc.msr) {
> +        s->ddrc.mcr = 0;
> +        s->ddrc.msr = 0;
> +        if (s->ahbc.cr) {
> +            /* AHB remapped */
> +            sysbus_mmio_map(SYS_BUS_DEVICE(s->rom), 0, 0x00000000);
> +            memory_region_del_subregion(s->as, s->ram);
> +        } else {
> +            /* AHB is not yet remapped, but SDRAM is ready */
> +            memory_region_del_subregion(s->as, s->ram_alias);
> +        }
> +        s->ahbc.cr = 0;
> +    }
> +
> +    cpu_reset(CPU(s->cpu));
> +}
> +
> +static void
> +a369_board_init(QEMUMachineInitArgs *args)
> +{
> +    struct arm_boot_info *bi = NULL;
> +    a369_state *s = g_new(a369_state, 1);
> +
> +    s->as = get_system_memory();
> +    s->ram  = g_new(MemoryRegion, 1);
> +    s->sram = g_new(MemoryRegion, 1);
> +
> +    /* CPU */
> +    if (!args->cpu_model) {
> +        args->cpu_model = "fa626te";
> +    }
> +
> +    s->cpu = cpu_arm_init(args->cpu_model);
> +    if (!s->cpu) {
> +        fprintf(stderr, "Unable to find CPU definition\n");
> +        exit(1);
> +    }
> +
> +    /* A369 supports upto 1GB ram space */
> +    if (args->ram_size > 0x40000000) {
> +        args->ram_size = 0x40000000;
> +    }
> +
> +    printf("qemu: faraday a369 with %dMB ram.\n", args->ram_size >> 20);
> +
> +    /* Embedded ROM Init */
> +    s->rom = qdev_create(NULL, "rom");
> +    qdev_prop_set_uint32(s->rom, "size", 8192);
> +    qdev_init_nofail(s->rom);
> +
> +    /* Embedded RAM Init */
> +    memory_region_init_ram(s->sram, "a369.sram", 0x4000);
> +    vmstate_register_ram_global(s->sram);
> +    memory_region_add_subregion(s->as, 0xA0000000, s->sram);
> +
> +    /* RAM Init */
> +    memory_region_init_ram(s->ram, "a369.ram", args->ram_size);
> +    vmstate_register_ram_global(s->ram);
> +
> +    a369_device_init(s);
> +    qemu_register_reset(a369_board_reset, s);
> +
> +    if (args->kernel_filename) {
> +        bi = g_new0(struct arm_boot_info, 1);
> +
> +        /* RAM Address Binding */
> +        memory_region_add_subregion(s->as, 0x00000000, s->ram);
> +
> +        /* Boot Info */
> +        bi->ram_size = args->ram_size;
> +        bi->kernel_filename = args->kernel_filename;
> +        bi->kernel_cmdline = args->kernel_cmdline;
> +        bi->initrd_filename = args->initrd_filename;
> +        bi->board_id = 0xa369;
> +        arm_load_kernel(s->cpu, bi);
> +    } else {
> +        /* ROM Address Binding */
> +        sysbus_mmio_map(SYS_BUS_DEVICE(s->rom), 0, 0x00000000);
> +        /* Partial RAM (before ahb remapped) Address Binding */
> +        s->ram_alias = g_new(MemoryRegion, 1);
> +        memory_region_init_alias(s->ram_alias, "a369.ram_alias",
> +                                 s->ram,
> +                                 0,
> +                                 MIN(0x10000000, args->ram_size));
> +    }


Sorry, could you please explain what will happen when we do not specify 
-kernel argument?



> +}
> +
> +static QEMUMachine a369_machine = {
> +    .name = "a369",
> +    .desc = "Faraday A369 (fa626te)",
> +    .init = a369_board_init,
> +};
> +
> +static void
> +a369_machine_init(void)
> +{
> +    qemu_register_machine(&a369_machine);
> +}
> +
> +machine_init(a369_machine_init);
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index 6d049e7..c7bb10e 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -1,4 +1,10 @@
>   obj-y = integratorcp.o versatilepb.o arm_pic.o
> +obj-y += a360.o a369.o \
> +				rom.o ftdmac020.o ftapbbrg020.o \
> +				ftintc020.o fttmr010.o ftpwmtmr010.o \
> +				ftspi020.o ftssp010.o fti2c010.o \
> +				ftrtc011.o ftwdt010.o ftmac110.o ftgmac100.o ftlcdc200.o \
> +				fttsc010.o ftkbc010.o ftnandc021.o ftsdc010.o

No such files exist at this point, you should add them here one by one 
in a corresponding patch.
And tabs should be replaced with spaces.


>   obj-y += arm_boot.o
>   obj-y += xilinx_zynq.o zynq_slcr.o
>   obj-y += xilinx_spips.o
> diff --git a/hw/faraday.h b/hw/faraday.h
> new file mode 100644
> index 0000000..f4fe0cc
> --- /dev/null
> +++ b/hw/faraday.h


None of three function prototyped in this file exists at this point, I 
think you should add this file later in the patch set.


> @@ -0,0 +1,21 @@
> +/*
> + * Faraday SoC platform support.
> + *
> + * Copyright (c) 2013 Faraday Technology
> + * Written by Kuo-Jung Su <dantesu@gmail.com>
> + *
> + * This code is licensed under the GNU GPL v2.
> + */
> +#ifndef FARADAY_H
> +#define FARADAY_H
> +
> +/* ftintc020.c */
> +qemu_irq *ftintc020_init(hwaddr base, ARMCPU *cpu);
> +
> +/* ftgmac100.c */
> +void ftgmac100_init(NICInfo *, uint32_t, qemu_irq);
> +
> +/* ftmac110.c */
> +void ftmac110_init(NICInfo *, uint32_t, qemu_irq);
> +
> +#endif
>

Kuo-Jung Su Feb. 1, 2013, 1:39 a.m. UTC | #3

2013/2/1 Igor Mitsyanko <i.mitsyanko@samsung.com>
>
> Hi, Kuo-Jung, please see several comments bellow.
>
>
>
> On 01/25/2013 12:19 PM, Kuo-Jung Su wrote:
>>
>> From: Kuo-Jung Su <dantesu@faraday-tech.com>
>>
>> The Faraday A360/A369 EVB is a Faraday platform main board used for the
>> Faraday IP functional verification based on the well-known ARM AMBA 2.0
>> architecture. This main board provides a fully verified microprocessor
>> platform, ATA-II, OTG 2.0 and USB 2.0 host connectivity.
>>
>> Faraday A360 EVB provides the on-board DDR2-SODIMM (256 MB),
>> NAND Flash (128 MB), USB2.0 interface, two PCI Express interfaces,
>> two I2C interfaces, two SSP ports, Secure Digital interface, and a 12-bit
>> A/D converter in addition to many other features.
>>
>> Faraday A369 EVB provides the on-board DDR2-SODIMM (512 MB),
>> NAND Flash (256 MB), SPI Flash ROM (16 MB), I2C EPROM, Giga-bit Ethernet,
>> IDE, SATA host, OTG 2.0 and USB 2.0 host connectors, video output interface,
>> TV encoder for video output, LCD controller interface, SD/MMC card reader
>> interface, I2S codec, UART(X2), Multi-ICE, 80 ports, and an AHB extension bus.
>>
>> Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
>> ---
>>   hw/a360.c            |  271 +++++++++++++++++++++++
>>   hw/a369.c            |  581 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>   hw/arm/Makefile.objs |    6 +
>>   hw/faraday.h         |   21 ++
>>   4 files changed, 879 insertions(+)
>>   create mode 100644 hw/a360.c
>>   create mode 100644 hw/a369.c
>>   create mode 100644 hw/faraday.h
>>
>> diff --git a/hw/a360.c b/hw/a360.c
>> new file mode 100644
>> index 0000000..cb0a588
>> --- /dev/null
>> +++ b/hw/a360.c
>> @@ -0,0 +1,271 @@
>> +/*
>> + * Faraday A360 Evalution Board
>> + *
>> + * Copyright (c) 2012 Faraday Technology
>> + * Written by Dante Su <dantesu@faraday-tech.com>
>> + *
>> + * This code is licensed under GNU GPL v2.
>> + */
>> +
>> +#include "sysbus.h"
>> +#include "arm-misc.h"
>> +#include "devices.h"
>> +#include "net/net.h"
>> +#include "sysemu/sysemu.h"
>> +#include "sysemu/blockdev.h"
>> +#include "exec/address-spaces.h"
>> +#include "i2c.h"
>> +#include "boards.h"
>> +#include "flash.h"
>> +#include "serial.h"
>> +#include "ssi.h"
>> +#include "faraday.h"
>> +
>> +typedef struct A360State {
>> +    ARMCPU       *cpu;
>> +    DeviceState  *hdma;    /* AHB DMA */
>> +    DeviceState  *pdma;    /* APB DMA */
>> +
>> +    MemoryRegion *as;
>> +    MemoryRegion *ram;
>> +
>> +    i2c_bus      *i2c[2];
>> +
>> +    struct {
>> +        MemoryRegion iomem;
>> +    } pmu;
>> +} a360_state;
>> +
>> +/* PMU block */
>> +static uint64_t
>> +a360_pmu_read(void *opaque, hwaddr addr, unsigned size)
>> +{
>> +    uint64_t ret = 0;
>> +
>> +    switch (addr) {
>> +    case 0x30:
>> +        ret = 27 << 3;
>
>
> Since we do not have documentation, it would be nice at least to have descriptive defines for all these numbers.

I'll do that in the incoming new patch set, but because most of these registers
are simply useless in QEMU model, so I might prefer add descriptions.

Here is the short description to SCU/PMU, DDRC and AHBC:

1. SCU/PMU:
    Its registers ranges from 0x000 to 0x25C, and 90% of them were clock
    and pinmux settings, and only 5 of them would be read by u-boot/linux
    to detect current clock rate, and apply pinmux change.
    (i.e. Enable ICE support, which is totally meanless to QEMU).
2. DDRC:
    It stands for DDR controller, it is responsible for DDRII/DDRIII SDRAM init,
    the only one thing we care in QEMU is when the SDRAM become accessible.
    Obviously 90% of the registers are meanless to QEMU.

3. AHBC:
    It stands for AHB controller, it simply describe each slave device settings
    (i.e. address and size), in my newer bootstrap code, I'll try to find out
    the base address and size for slave 4 and 6, those would be 'swapped'
    after AHB remapping. Obviously 90% of the registers are meanless to QEMU.

>
>
>
>> +        break;
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +static void
>> +a360_pmu_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
>> +{
>> +}
>> +
>> +static const MemoryRegionOps a360_pmu_ops = {
>> +    .read  = a360_pmu_read,
>> +    .write = a360_pmu_write,
>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>> +};
>> +
>> +static int
>> +a360_pmu_init(a360_state *s, hwaddr base)
>> +{
>> +    MemoryRegion *iomem = &s->pmu.iomem;
>> +    memory_region_init_io(iomem, &a360_pmu_ops, s, "a360_pmu", 0x1000);
>> +    memory_region_add_subregion(s->as, base, iomem);
>> +    return 0;
>> +}
>> +
>> +/* Board init. */
>> +static void
>> +a360_device_init(a360_state *s)
>> +{
>> +    qemu_irq *pic;
>> +    qemu_irq ack, req;
>> +    qemu_irq cs_line;
>> +    DeviceState *ds;
>> +    int i, done_nic = 0, nr_flash = 1;
>> +    SSIBus *spi;
>> +    DeviceState *fl;
>> +
>> +    /* Interrupt Controller */
>> +    pic = ftintc020_init(0x98800000, s->cpu);
>
>
>
> You haven't introduced this interrupt controller yet, patches should be arranged in such an order that they at least wouldn't break a build.
> Same goes for ftintc020_init and ftgmac100_init.


I thought that's why patch set is designed for.
And I susposed to split up each component and send out patches one by one?

>
>
>
>> +
>> +    /* Timer */
>> +    ds = qdev_create(NULL, "fttmr010");
>> +    qdev_prop_set_uint32(ds, "freq", 66 * 1000000);
>> +    qdev_init_nofail(ds);
>> +    sysbus_mmio_map(SYS_BUS_DEVICE(ds), 0, 0x98400000);
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 0, pic[42]);
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 1, pic[19]);
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 2, pic[14]);
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 3, pic[15]);
>> +
>> +    /* Serial (FTUART010 which is 16550A compatible) */
>> +    if (serial_hds[0]) {
>> +        serial_mm_init(s->as,
>> +                       0x98200000,
>> +                       2,
>> +                       pic[10],
>> +                       18432000 / 16,
>> +                       serial_hds[0],
>> +                       DEVICE_LITTLE_ENDIAN);
>> +    }
>> +    if (serial_hds[1]) {
>> +        serial_mm_init(s->as,
>> +                       0x98300000,
>> +                       2,
>> +                       pic[11],
>> +                       18432000 / 16,
>> +                       serial_hds[1],
>> +                       DEVICE_LITTLE_ENDIAN);
>> +    }
>> +
>> +    /* pmu */
>> +    a360_pmu_init(s, 0x98100000);
>> +
>> +    /* ftdmac020 */
>> +    s->hdma = sysbus_create_varargs("ftdmac020",
>> +                                    0x90400000,
>> +                                    pic[21], /* ALL */
>> +                                    pic[40], /* TC */
>> +                                    pic[41], /* ERR */
>> +                                    NULL);
>> +
>> +    /* ftapbbrg020 */
>> +    s->pdma = sysbus_create_simple("ftapbbrg020", 0x90500000, pic[24]);
>> +
>> +    /* ftsdc010 */
>> +    ds = sysbus_create_simple("ftsdc010", 0x90700000, pic[5]);
>> +    ack = qdev_get_gpio_in(ds, 0);
>> +    req = qdev_get_gpio_in(s->hdma, 0);
>> +    qdev_connect_gpio_out(s->hdma, 0, ack);
>> +    qdev_connect_gpio_out(ds, 0, req);
>> +
>> +    /* fusbh200 */
>> +    sysbus_create_simple("faraday-ehci-usb", 0x90A00000, pic[23]);
>> +
>> +    /* fotg210 */
>> +    sysbus_create_simple("faraday-ehci-usb", 0x90B00000, pic[26]);
>> +
>> +    /* ftmac110 */
>> +    for (i = 0; i < nb_nics; i++) {
>> +        NICInfo *nd = &nd_table[i];
>> +        if (!done_nic && (!nd->model || strcmp(nd->model, "ftmac110") == 0)) {
>> +            ftmac110_init(nd, 0x90900000, pic[25]);
>> +            done_nic = 1;
>
>
> What is it for? It never checked anywhere.

Did you mean 'done_nic' ?
It's used to avoid initializing 'ftmac110' twice, and frankly
speaking, it's copied from versatilepb.c.

>
>
>
>> +        }
>> +    }
>> +
>> +    /* ftwdt010 */
>> +    sysbus_create_simple("ftwdt010", 0x98500000, pic[16]);
>> +
>> +    /* ftlcdc200 */
>> +    sysbus_create_varargs("ftlcdc200",
>> +                          0x90600000,
>> +                          pic[27],  /* Global */
>> +                          NULL);
>
>
>
> Why sysbus_create_simple() before but sysbus_create_varargs() here? Also, this comment is not very useful, what is it trying to say?

Sorry, it's a typo.
There is 5 interrupt pin from FTLCDC200, however only one of them
connected with A360.
I simply forget to change the function while the interrupt has cut
down to 1 only.

>
>
>
>> +
>> +    /* fti2c010 */
>> +    ds = sysbus_create_simple("fti2c010", 0x98A00000, pic[3]);
>> +    s->i2c[0] = (i2c_bus *)qdev_get_child_bus(ds, "i2c");
>> +    ds = sysbus_create_simple("fti2c010", 0x98C00000, pic[22]);
>> +    s->i2c[1] = (i2c_bus *)qdev_get_child_bus(ds, "i2c");
>> +
>> +    /* ftssp010 */
>> +    ds = qdev_create(NULL, "ftssp010");
>> +
>> +    /* i2s */
>> +    qdev_prop_set_ptr(ds, "codec_i2c", s->i2c[0]);
>> +    qdev_init_nofail(ds);
>> +    sysbus_mmio_map(SYS_BUS_DEVICE(ds), 0, 0x98B00000);
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 0, pic[2]);
>> +
>> +    /* spi */
>> +    spi = (SSIBus *)qdev_get_child_bus(ds, "spi");
>> +    for (i = 0; i < nr_flash; i++) {
>> +        fl = ssi_create_slave_no_init(spi, "m25p80");
>> +        qdev_prop_set_string(fl, "partname", "w25q64");
>> +        qdev_init_nofail(fl);
>> +        cs_line = qdev_get_gpio_in(fl, 0);
>> +        sysbus_connect_irq(SYS_BUS_DEVICE(ds), i+1, cs_line);
>> +    }
>> +
>> +    /* DMA (Tx) */
>> +    ack = qdev_get_gpio_in(ds, 0);
>> +    req = qdev_get_gpio_in(s->pdma, 1);
>> +    qdev_connect_gpio_out(s->pdma, 1, ack);
>> +    qdev_connect_gpio_out(ds, 0, req);
>> +
>> +    /* DMA (Rx) */
>> +    ack = qdev_get_gpio_in(ds, 1);
>> +    req = qdev_get_gpio_in(s->pdma, 2);
>> +    qdev_connect_gpio_out(s->pdma, 2, ack);
>> +    qdev_connect_gpio_out(ds, 1, req);
>> +}
>> +
>> +static void
>> +a360_board_reset(void *opaque)
>> +{
>> +    a360_state *s = opaque;
>> +
>> +    cpu_reset(CPU(s->cpu));
>> +}
>> +
>
>
>
> Why is this necessary? CPU reset handler is registered in arm_load_kernel().

I planed to implement AHB remap (ROM emulation) to A360, and that's why
I need a board reset here. At the end, this function is cancled because the only
reason the A360 is created to QEMU is now for FTMAC110 test only.

So it would be removed later.

>
>
>
>> +static void
>> +a360_board_init(QEMUMachineInitArgs *args)
>> +{
>> +    struct arm_boot_info *bi = NULL;
>> +    a360_state *s = g_new(a360_state, 1);
>> +
>> +    s->as = get_system_memory();
>> +    s->ram  = g_new(MemoryRegion, 1);
>> +
>> +    /* CPU */
>> +    if (!args->cpu_model) {
>> +        args->cpu_model = "fa626te";
>> +    }
>> +
>> +    s->cpu = cpu_arm_init(args->cpu_model);
>> +    if (!s->cpu) {
>> +        fprintf(stderr, "Unable to find CPU definition\n");
>> +        exit(1);
>> +    }
>> +
>> +    /* A360 supports upto 1GB ram space */
>> +    if (args->ram_size > 0x40000000) {
>> +        args->ram_size = 0x40000000;
>> +    }
>> +
>> +    printf("qemu: faraday a360 with %dMB ram.\n", args->ram_size >> 20);
>
>
> You should use RAM_ADDR_FMT for ram_addr_t formatting or else it would break build.
> But usually QEMU doesn't use such messages when initialising devices, maybe just drop it?

It has been drop now.

>
>
>> +
>> +    /* RAM Init */
>> +    memory_region_init_ram(s->ram, "a360.ram", args->ram_size);
>> +    vmstate_register_ram_global(s->ram);
>> +
>> +    a360_device_init(s);
>> +    qemu_register_reset(a360_board_reset, s);
>> +
>> +    /* Prepare for direct boot from linux kernel or u-boot elf */
>> +    bi = g_new0(struct arm_boot_info, 1);
>> +
>> +    /* RAM Address Binding */
>> +    memory_region_add_subregion(s->as, 0x00000000, s->ram);
>> +
>> +    /* Boot Info */
>> +    bi->ram_size = args->ram_size;
>> +    bi->kernel_filename = args->kernel_filename;
>> +    bi->kernel_cmdline = args->kernel_cmdline;
>> +    bi->initrd_filename = args->initrd_filename;
>> +    bi->board_id = 0xa360;
>> +    arm_load_kernel(s->cpu, bi);
>> +}
>> +
>> +static QEMUMachine a360_machine = {
>> +    .name = "a360",
>> +    .desc = "Faraday A360 (fa626te)",
>> +    .init = a360_board_init,
>
>
>
> Not sure if its mandatory, but looks like you need to add DEFAULT_MACHINE_OPTIONS here.

It looks like a new item in the master, while I copied this example
code from realview.c
in qemu-1.3.0, there is no such thing.
Thanks for reminding me of it.

>
>
>
>> +};
>> +
>> +static void
>> +a360_machine_init(void)
>> +{
>> +    qemu_register_machine(&a360_machine);
>> +}
>> +
>> +machine_init(a360_machine_init);
>> diff --git a/hw/a369.c b/hw/a369.c
>> new file mode 100644
>> index 0000000..646888f
>> --- /dev/null
>> +++ b/hw/a369.c
>> @@ -0,0 +1,581 @@
>> +/*
>> + * Faraday A369 Evalution Board
>> + *
>> + * Copyright (c) 2012 Faraday Technology
>> + * Written by Dante Su <dantesu@faraday-tech.com>
>> + *
>> + * This code is licensed under GNU GPL v2.
>> + */
>> +
>> +#include "sysbus.h"
>> +#include "arm-misc.h"
>> +#include "devices.h"
>> +#include "net/net.h"
>> +#include "sysemu/sysemu.h"
>> +#include "sysemu/blockdev.h"
>> +#include "exec/address-spaces.h"
>> +#include "i2c.h"
>> +#include "boards.h"
>> +#include "flash.h"
>> +#include "serial.h"
>> +#include "ssi.h"
>> +#include "faraday.h"
>> +
>> +#define A369_NOR_FLASH_ADDR         0x20000000
>> +#define A369_NOR_FLASH_SIZE         (16 * 1024 * 1024)
>> +#define A369_NOR_FLASH_SECT_SIZE    (128 * 1024)
>> +
>> +typedef struct A369State {
>> +    ARMCPU       *cpu;
>> +    DeviceState  *rom;
>> +    DeviceState  *hdma[2];    /* AHB DMA */
>> +    DeviceState  *pdma;    /* APB DMA */
>> +
>> +    MemoryRegion *as;
>> +    MemoryRegion *ram;
>> +    MemoryRegion *ram_alias;
>> +    MemoryRegion *sram;
>> +
>> +    i2c_bus      *i2c[2];
>> +
>> +    struct {
>> +        MemoryRegion iomem;
>> +        /* HW register cache */
>> +        uint32_t general_cfg;
>> +        uint32_t sclk_cfg0;
>> +        uint32_t sclk_cfg1;
>> +        uint32_t mfpsr0;
>> +        uint32_t mfpsr1;
>> +    } scu;
>> +
>> +    struct {
>> +        MemoryRegion iomem;
>> +        /* HW register cache */
>> +        uint32_t cr;
>> +    } ahbc;
>> +
>> +    struct {
>> +        MemoryRegion iomem;
>> +        /* HW register cache */
>> +        uint32_t mcr;
>> +        uint32_t msr;
>> +    } ddrc;
>> +
>> +} a369_state;
>> +
>> +/* SCU block */
>> +
>> +static uint64_t
>> +a369_scu_read(void *opaque, hwaddr addr, unsigned size)
>> +{
>> +    a369_state *s = opaque;
>> +
>> +    switch (addr) {
>> +    case 0x000: return 0x00003369;
>> +    case 0x004: return 0x00010000;
>> +    case 0x008: return 0x00000c10;
>> +    case 0x00c: return 0x00000230;
>> +    case 0x010: return 0x00000083;
>> +    case 0x014: return 0x00000100;
>> +    case 0x01C: return 0x00000003;
>> +    case 0x020: return 0x20010003;
>> +    case 0x024: return 0x00000003;
>> +    case 0x060: return 0x00280028;
>> +    case 0x200: return s->scu.general_cfg;
>> +    case 0x204: return 0x00001cc8;
>> +    case 0x228: return s->scu.sclk_cfg0;
>> +    case 0x22c: return s->scu.sclk_cfg1;
>> +    case 0x230: return 0x00003fff;
>> +    case 0x238: return s->scu.mfpsr0;
>> +    case 0x23c: return s->scu.mfpsr1;
>> +    case 0x240: return 0x11111111;
>> +    case 0x244: return 0x11111111;
>> +    case 0x254: return 0x00000303;
>> +    case 0x258: return 0x8000007f;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static void
>> +a369_scu_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
>> +{
>> +    a369_state *s = opaque;
>> +
>> +    switch (addr) {
>> +    case 0x200:
>> +        s->scu.general_cfg = (uint32_t)val;
>> +        break;
>> +    case 0x228:
>> +        s->scu.sclk_cfg0 = (uint32_t)val;
>> +        break;
>> +    case 0x22c:
>> +        s->scu.sclk_cfg1 = (uint32_t)val;
>> +        break;
>> +    case 0x238:
>> +        s->scu.mfpsr0 = (uint32_t)val;
>> +        break;
>> +    case 0x23c:
>> +        s->scu.mfpsr1 = (uint32_t)val;
>> +        break;
>> +    }
>> +}
>> +
>> +static const MemoryRegionOps a369_scu_ops = {
>> +    .read  = a369_scu_read,
>> +    .write = a369_scu_write,
>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>> +    .valid = {
>> +        .min_access_size = 4,
>> +        .max_access_size = 4
>> +    }
>> +};
>> +
>> +static int
>> +a369_scu_init(a369_state *s, hwaddr base)
>> +{
>> +    MemoryRegion *iomem = &s->scu.iomem;
>> +
>> +    memory_region_init_io(iomem, &a369_scu_ops, s, "a369_scu", 0x1000);
>> +    memory_region_add_subregion(s->as, base, iomem);
>> +
>> +    s->scu.general_cfg = 0x00001078;
>> +    s->scu.sclk_cfg0   = 0x26877330;
>> +    s->scu.sclk_cfg1   = 0x000a0a0a;
>> +    s->scu.mfpsr0      = 0x00000241;
>> +    s->scu.mfpsr1      = 0x00000000;
>> +
>> +    return 0;
>> +}
>> +
>> +/* AHB controller block */
>> +
>> +static uint64_t
>> +a369_ahbc_read(void *opaque, hwaddr addr, unsigned size)
>> +{
>> +    a369_state *s = opaque;
>> +
>> +    switch (addr) {
>> +    case 0x00: return 0x94050000;
>> +    case 0x04: return 0x96040000;
>> +    case 0x08: return 0x90f00000;
>> +    case 0x0c: return 0x92050000;
>> +    case 0x10: return 0x20080000;
>> +    case 0x14: return 0xc0080000;
>> +    case 0x18: return 0x00090000;
>> +    case 0x1c: return 0x90000000;
>> +    case 0x20: return 0x90100000;
>> +    case 0x24: return 0x90200000;
>> +    case 0x28: return 0x90300000;
>> +    case 0x2c: return 0x90400000;
>> +    case 0x30: return 0x90500000;
>> +    case 0x34: return 0x90600000;
>> +    case 0x38: return 0x90700000;
>> +    case 0x3c: return 0x90800000;
>> +    case 0x40: return 0x90900000;
>> +    case 0x44: return 0x90a00000;
>> +    case 0x48: return 0x90b00000;
>> +    case 0x4c: return 0x90c00000;
>> +    case 0x50: return 0x90d00000;
>> +    case 0x54: return 0x90e00000;
>> +    case 0x58: return 0x40080000;
>> +    case 0x5c: return 0x60080000;
>> +    case 0x60: return 0xa0000000;
>> +    case 0x84: return 0x00000001;
>> +    case 0x88: return s->ahbc.cr;
>> +    case 0x8c: return 0x00010301;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static void
>> +a369_ahbc_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
>> +{
>> +    a369_state *s = opaque;
>> +
>> +    switch (addr) {
>> +    case 0x88:
>> +        if (!(s->ahbc.cr & 0x01) && (val & 0x01)) {
>> +            /* AHB remap */
>> +            sysbus_mmio_map(SYS_BUS_DEVICE(s->rom), 0, 0x40000000);
>> +            memory_region_del_subregion(s->as, s->ram_alias);
>> +            memory_region_add_subregion(s->as, 0x00000000, s->ram);
>> +        }
>> +        s->ahbc.cr = (uint32_t)val;
>> +        break;
>> +    }
>> +}
>> +
>> +static const MemoryRegionOps a369_ahbc_ops = {
>> +    .read  = a369_ahbc_read,
>> +    .write = a369_ahbc_write,
>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>> +};
>> +
>> +static int
>> +a369_ahbc_init(a369_state *s, hwaddr base)
>> +{
>> +    MemoryRegion *iomem = &s->ahbc.iomem;
>> +
>> +    memory_region_init_io(iomem, &a369_ahbc_ops, s, "a369_ahbc", 0x1000);
>> +    memory_region_add_subregion(s->as, base, iomem);
>> +
>> +    s->ahbc.cr = 0;
>> +
>> +    return 0;
>> +}
>> +
>> +/* DDRII controller block */
>> +
>> +static uint64_t
>> +a369_ddrc_read(void *opaque, hwaddr addr, unsigned size)
>> +{
>> +    a369_state *s = opaque;
>> +
>> +    switch (addr) {
>> +    case 0x00:  return s->ddrc.mcr;
>> +    case 0x04:  return s->ddrc.msr;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static void
>> +a369_ddrc_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
>> +{
>> +    a369_state *s = opaque;
>> +
>> +    switch (addr) {
>> +    case 0x00:
>> +        s->ddrc.mcr = (uint32_t)val & 0xffff;
>> +        break;
>> +    case 0x04:
>> +        val = (val & 0x3f) | (s->ddrc.msr & 0x100);
>> +        if (!(s->ddrc.msr & 0x100) && (val & 0x01)) {
>> +            val &= 0xfffffffe;
>> +            val |= 0x100;
>> +            memory_region_add_subregion(s->as, 0x10000000, s->ram_alias);
>> +        }
>> +        s->ddrc.msr = (uint32_t)val;
>> +        break;
>> +    }
>> +}
>> +
>> +static const MemoryRegionOps a369_ddrc_ops = {
>> +    .read  = a369_ddrc_read,
>> +    .write = a369_ddrc_write,
>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>> +};
>> +
>> +static int
>> +a369_ddrc_init(a369_state *s, hwaddr base)
>> +{
>> +    MemoryRegion *iomem = &s->ddrc.iomem;
>> +
>> +    memory_region_init_io(iomem, &a369_ddrc_ops, s, "a369_ddrc", 0x1000);
>> +    memory_region_add_subregion(s->as, base, iomem);
>> +
>> +    s->ddrc.mcr = 0;
>> +    s->ddrc.msr = 0;
>> +
>> +    return 0;
>> +}
>> +
>> +/* Board init.  */
>> +
>> +static void
>> +a369_device_init(a369_state *s)
>> +{
>> +    qemu_irq *pic;
>> +    qemu_irq ack, req;
>> +    qemu_irq cs_line;
>> +    DeviceState *ds;
>> +    int i, done_nic = 0, nr_flash = 1;
>> +    SSIBus *spi;
>> +    DeviceState *fl;
>> +    DriveInfo *dinfo;
>> +
>> +    /* Interrupt Controller */
>> +    pic = ftintc020_init(0x90100000, s->cpu);
>> +
>> +    /* Timer */
>> +    ds = qdev_create(NULL, "ftpwmtmr010");
>> +    qdev_prop_set_uint32(ds, "freq", 66 * 1000000);
>> +    qdev_init_nofail(ds);
>> +    sysbus_mmio_map(SYS_BUS_DEVICE(ds), 0, 0x92300000);
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 0, pic[8]);
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 1, pic[9]);
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 2, pic[10]);
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 3, pic[11]);
>> +
>> +    /* Serial (FTUART010 which is 16550A compatible) */
>> +    if (serial_hds[0]) {
>> +        serial_mm_init(s->as,
>> +                       0x92b00000,
>> +                       2,
>> +                       pic[53],
>> +                       18432000 / 16,
>> +                       serial_hds[0],
>> +                       DEVICE_LITTLE_ENDIAN);
>> +    }
>> +    if (serial_hds[1]) {
>> +        serial_mm_init(s->as,
>> +                       0x92c00000,
>> +                       2,
>> +                       pic[54],
>> +                       18432000 / 16,
>> +                       serial_hds[1],
>> +                       DEVICE_LITTLE_ENDIAN);
>> +    }
>> +
>> +    /* ftscu010 */
>> +    a369_scu_init(s, 0x92000000);
>> +
>> +    /* ftddrII030 */
>> +    a369_ddrc_init(s, 0x93100000);
>> +
>> +    /* ftahbc020 */
>> +    a369_ahbc_init(s, 0x94000000);
>> +
>> +    /* ftdmac020 */
>> +    s->hdma[0] = sysbus_create_varargs("ftdmac020",
>> +                                       0x90300000,
>> +                                       pic[0],  /* ALL (NC in A369) */
>> +                                       pic[15], /* TC */
>> +                                       pic[16], /* ERR */
>> +                                       NULL);
>> +    s->hdma[1] = sysbus_create_varargs("ftdmac020",
>> +                                       0x96100000,
>> +                                       pic[0],  /* ALL (NC in A369) */
>> +                                       pic[17], /* TC */
>> +                                       pic[18], /* ERR */
>> +                                       NULL);
>> +
>> +    /* ftapbbrg020 */
>> +    s->pdma = sysbus_create_simple("ftapbbrg020", 0x90f00000, pic[14]);
>> +
>> +    /* ftnandc021 */
>> +    ds = sysbus_create_simple("ftnandc021", 0x90200000, pic[30]);
>> +    ack = qdev_get_gpio_in(ds, 0);
>> +    req = qdev_get_gpio_in(s->hdma[0], 15);
>> +    qdev_connect_gpio_out(s->hdma[0], 15, ack);
>> +    qdev_connect_gpio_out(ds, 0, req);
>> +
>> +    /* ftsdc010 */
>> +#if 0
>> +    ds = sysbus_create_simple("ftsdc010", 0x90500000, pic[38]);
>> +    ack = qdev_get_gpio_in(ds, 0);
>> +    req = qdev_get_gpio_in(s->hdma[0], 14);
>> +    qdev_connect_gpio_out(s->hdma[0], 14, ack);
>> +    qdev_connect_gpio_out(ds, 0, req);
>> +#endif
>> +
>> +    ds = sysbus_create_simple("ftsdc010", 0x90600000, pic[39]);
>> +    ack = qdev_get_gpio_in(ds, 0);
>> +    req = qdev_get_gpio_in(s->hdma[0], 13);
>> +    qdev_connect_gpio_out(s->hdma[0], 13, ack);
>> +    qdev_connect_gpio_out(ds, 0, req);
>> +
>> +    /* fusbh200 */
>> +    sysbus_create_simple("faraday-ehci-usb", 0x90800000, pic[36]);
>> +
>> +    /* fotg210 */
>> +    sysbus_create_simple("faraday-ehci-usb", 0x90900000, pic[37]);
>> +
>> +    /* ftgmac100 */
>> +    for (i = 0; i < nb_nics; i++) {
>> +        NICInfo *nd = &nd_table[i];
>> +        if (!done_nic && (!nd->model || strcmp(nd->model, "ftgmac100") == 0)) {
>> +            ftgmac100_init(nd, 0x90c00000, pic[32]);
>> +            done_nic = 1;
>> +        }
>> +    }
>> +
>> +    /* ftrtc011 (only alarm interrupt is connected) */
>> +    sysbus_create_varargs("ftrtc011",
>> +                          0x92100000,
>> +                          pic[0],     /* Alarm (Level): NC in A369 */
>> +                          pic[42],    /* Alarm (Edge) */
>> +                          pic[43],    /* Second (Edge) */
>> +                          pic[44],    /* Minute (Edge) */
>> +                          pic[45],    /* Hour (Edge) */
>> +                          NULL);
>> +
>> +    /* ftwdt010 */
>> +    sysbus_create_simple("ftwdt010", 0x92200000, pic[46]);
>> +
>> +    /* fttsc010 */
>> +    sysbus_create_simple("fttsc010", 0x92400000, pic[19]);
>> +
>> +    /* ftkbc010 */
>> +    sysbus_create_simple("ftkbc010", 0x92f00000, pic[21]);
>> +
>> +    /* ftlcdc200 */
>> +    sysbus_create_varargs("ftlcdc200",
>> +                          0x94a00000,
>> +                          pic[0],  /* ALL (NC in A369) */
>> +                          pic[25], /* VSTATUS */
>> +                          pic[24], /* Base Address Update */
>> +                          pic[23], /* FIFO Under-Run */
>> +                          pic[22], /* AHB Bus Error */
>> +                          NULL);
>> +
>> +    /* fti2c010 */
>> +    ds = sysbus_create_simple("fti2c010", 0x92900000, pic[51]);
>> +    s->i2c[0] = (i2c_bus *)qdev_get_child_bus(ds, "i2c");
>> +    ds = sysbus_create_simple("fti2c010", 0x92A00000, pic[52]);
>> +    s->i2c[1] = (i2c_bus *)qdev_get_child_bus(ds, "i2c");
>> +
>> +    /* ftssp010 */
>> +    ds = qdev_create(NULL, "ftssp010");
>> +
>> +    /* i2s */
>> +    qdev_prop_set_ptr(ds, "codec_i2c", s->i2c[0]);
>> +    qdev_init_nofail(ds);
>> +    sysbus_mmio_map(SYS_BUS_DEVICE(ds), 0, 0x92700000);
>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 0, pic[49]);
>> +
>> +    /* spi */
>> +    spi = (SSIBus *)qdev_get_child_bus(ds, "spi");
>> +    for (i = 0; i < nr_flash; i++) {
>> +        fl = ssi_create_slave_no_init(spi, "m25p80");
>> +        qdev_prop_set_string(fl, "partname", "w25q64");
>> +        qdev_init_nofail(fl);
>> +        cs_line = qdev_get_gpio_in(fl, 0);
>> +        sysbus_connect_irq(SYS_BUS_DEVICE(ds), i+1, cs_line);
>> +    }
>> +
>> +    /* DMA (Tx) */
>> +    ack = qdev_get_gpio_in(ds, 0);
>> +    req = qdev_get_gpio_in(s->pdma, 7);
>> +    qdev_connect_gpio_out(s->pdma, 7, ack);
>> +    qdev_connect_gpio_out(ds, 0, req);
>> +
>> +    /* DMA (Rx) */
>> +    ack = qdev_get_gpio_in(ds, 1);
>> +    req = qdev_get_gpio_in(s->pdma, 8);
>> +    qdev_connect_gpio_out(s->pdma, 8, ack);
>> +    qdev_connect_gpio_out(ds, 1, req);
>> +
>> +    /* Parallel NOR Flash */
>> +    dinfo = drive_get_next(IF_PFLASH);
>> +    if (!pflash_cfi01_register(
>> +                    A369_NOR_FLASH_ADDR,
>> +                    NULL,
>> +                    "a369.pflash",
>> +                    A369_NOR_FLASH_SIZE,
>> +                    dinfo ? dinfo->bdrv : NULL,
>> +                    A369_NOR_FLASH_SECT_SIZE,
>> +                    A369_NOR_FLASH_SIZE / A369_NOR_FLASH_SECT_SIZE,
>> +                    2, 0x0001, 0x227E, 0x2101, 0x0, 0)) {
>> +        hw_error("qemu: Error registering flash memory.\n");
>> +        exit(1);
>> +    }
>> +}
>> +
>> +static void
>> +a369_board_reset(void *opaque)
>> +{
>> +    a369_state *s = opaque;
>> +
>> +    if (s->ddrc.msr) {
>> +        s->ddrc.mcr = 0;
>> +        s->ddrc.msr = 0;
>> +        if (s->ahbc.cr) {
>> +            /* AHB remapped */
>> +            sysbus_mmio_map(SYS_BUS_DEVICE(s->rom), 0, 0x00000000);
>> +            memory_region_del_subregion(s->as, s->ram);
>> +        } else {
>> +            /* AHB is not yet remapped, but SDRAM is ready */
>> +            memory_region_del_subregion(s->as, s->ram_alias);
>> +        }
>> +        s->ahbc.cr = 0;
>> +    }
>> +
>> +    cpu_reset(CPU(s->cpu));
>> +}
>> +
>> +static void
>> +a369_board_init(QEMUMachineInitArgs *args)
>> +{
>> +    struct arm_boot_info *bi = NULL;
>> +    a369_state *s = g_new(a369_state, 1);
>> +
>> +    s->as = get_system_memory();
>> +    s->ram  = g_new(MemoryRegion, 1);
>> +    s->sram = g_new(MemoryRegion, 1);
>> +
>> +    /* CPU */
>> +    if (!args->cpu_model) {
>> +        args->cpu_model = "fa626te";
>> +    }
>> +
>> +    s->cpu = cpu_arm_init(args->cpu_model);
>> +    if (!s->cpu) {
>> +        fprintf(stderr, "Unable to find CPU definition\n");
>> +        exit(1);
>> +    }
>> +
>> +    /* A369 supports upto 1GB ram space */
>> +    if (args->ram_size > 0x40000000) {
>> +        args->ram_size = 0x40000000;
>> +    }
>> +
>> +    printf("qemu: faraday a369 with %dMB ram.\n", args->ram_size >> 20);
>> +
>> +    /* Embedded ROM Init */
>> +    s->rom = qdev_create(NULL, "rom");
>> +    qdev_prop_set_uint32(s->rom, "size", 8192);
>> +    qdev_init_nofail(s->rom);
>> +
>> +    /* Embedded RAM Init */
>> +    memory_region_init_ram(s->sram, "a369.sram", 0x4000);
>> +    vmstate_register_ram_global(s->sram);
>> +    memory_region_add_subregion(s->as, 0xA0000000, s->sram);
>> +
>> +    /* RAM Init */
>> +    memory_region_init_ram(s->ram, "a369.ram", args->ram_size);
>> +    vmstate_register_ram_global(s->ram);
>> +
>> +    a369_device_init(s);
>> +    qemu_register_reset(a369_board_reset, s);
>> +
>> +    if (args->kernel_filename) {
>> +        bi = g_new0(struct arm_boot_info, 1);
>> +
>> +        /* RAM Address Binding */
>> +        memory_region_add_subregion(s->as, 0x00000000, s->ram);
>> +
>> +        /* Boot Info */
>> +        bi->ram_size = args->ram_size;
>> +        bi->kernel_filename = args->kernel_filename;
>> +        bi->kernel_cmdline = args->kernel_cmdline;
>> +        bi->initrd_filename = args->initrd_filename;
>> +        bi->board_id = 0xa369;
>> +        arm_load_kernel(s->cpu, bi);
>> +    } else {
>> +        /* ROM Address Binding */
>> +        sysbus_mmio_map(SYS_BUS_DEVICE(s->rom), 0, 0x00000000);
>> +        /* Partial RAM (before ahb remapped) Address Binding */
>> +        s->ram_alias = g_new(MemoryRegion, 1);
>> +        memory_region_init_alias(s->ram_alias, "a369.ram_alias",
>> +                                 s->ram,
>> +                                 0,
>> +                                 MIN(0x10000000, args->ram_size));
>> +    }
>
>
>
> Sorry, could you please explain what will happen when we do not specify -kernel argument?
>
>
>
>
>> +}
>> +
>> +static QEMUMachine a369_machine = {
>> +    .name = "a369",
>> +    .desc = "Faraday A369 (fa626te)",
>> +    .init = a369_board_init,
>> +};
>> +
>> +static void
>> +a369_machine_init(void)
>> +{
>> +    qemu_register_machine(&a369_machine);
>> +}
>> +
>> +machine_init(a369_machine_init);
>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>> index 6d049e7..c7bb10e 100644
>> --- a/hw/arm/Makefile.objs
>> +++ b/hw/arm/Makefile.objs
>> @@ -1,4 +1,10 @@
>>   obj-y = integratorcp.o versatilepb.o arm_pic.o
>> +obj-y += a360.o a369.o \
>> +                               rom.o ftdmac020.o ftapbbrg020.o \
>> +                               ftintc020.o fttmr010.o ftpwmtmr010.o \
>> +                               ftspi020.o ftssp010.o fti2c010.o \
>> +                               ftrtc011.o ftwdt010.o ftmac110.o ftgmac100.o ftlcdc200.o \
>> +                               fttsc010.o ftkbc010.o ftnandc021.o ftsdc010.o
>
>
> No such files exist at this point, you should add them here one by one in a corresponding patch.
> And tabs should be replaced with spaces.

It looks like that I really have to split up each of one by one.
That's good to me, because I don't have any common sense about patch process,
Each component in one patch would be much more easier to me.

>
>
>
>>   obj-y += arm_boot.o
>>   obj-y += xilinx_zynq.o zynq_slcr.o
>>   obj-y += xilinx_spips.o
>> diff --git a/hw/faraday.h b/hw/faraday.h
>> new file mode 100644
>> index 0000000..f4fe0cc
>> --- /dev/null
>> +++ b/hw/faraday.h
>
>
>
> None of three function prototyped in this file exists at this point, I think you should add this file later in the patch set.

Same issue.
I'm studying patchwork now, because I don't want to ack like a idiot,
I'll send out the new patch when I 'though' I'm ready.
It might take few days to me, I just have patchwork installed in my PC today.

>
>
>
>> @@ -0,0 +1,21 @@
>> +/*
>> + * Faraday SoC platform support.
>> + *
>> + * Copyright (c) 2013 Faraday Technology
>> + * Written by Kuo-Jung Su <dantesu@gmail.com>
>> + *
>> + * This code is licensed under the GNU GPL v2.
>> + */
>> +#ifndef FARADAY_H
>> +#define FARADAY_H
>> +
>> +/* ftintc020.c */
>> +qemu_irq *ftintc020_init(hwaddr base, ARMCPU *cpu);
>> +
>> +/* ftgmac100.c */
>> +void ftgmac100_init(NICInfo *, uint32_t, qemu_irq);
>> +
>> +/* ftmac110.c */
>> +void ftmac110_init(NICInfo *, uint32_t, qemu_irq);
>> +
>> +#endif
>>
>
>
> --
> Mitsyanko Igor
> ASWG, Moscow R&D center, Samsung Electronics
> email: i.mitsyanko@samsung.com




--
Best wishes,
Kuo-Jung Su

Kuo-Jung Su Feb. 1, 2013, 2:22 a.m. UTC | #4

2013/2/1 Kuo-Jung Su <dantesu@gmail.com>:
> 2013/2/1 Igor Mitsyanko <i.mitsyanko@samsung.com>
>>
>> Hi, Kuo-Jung, please see several comments bellow.
>>
>>
>>
>> On 01/25/2013 12:19 PM, Kuo-Jung Su wrote:
>>>
>>> From: Kuo-Jung Su <dantesu@faraday-tech.com>
>>>
>>> The Faraday A360/A369 EVB is a Faraday platform main board used for the
>>> Faraday IP functional verification based on the well-known ARM AMBA 2.0
>>> architecture. This main board provides a fully verified microprocessor
>>> platform, ATA-II, OTG 2.0 and USB 2.0 host connectivity.
>>>
>>> Faraday A360 EVB provides the on-board DDR2-SODIMM (256 MB),
>>> NAND Flash (128 MB), USB2.0 interface, two PCI Express interfaces,
>>> two I2C interfaces, two SSP ports, Secure Digital interface, and a 12-bit
>>> A/D converter in addition to many other features.
>>>
>>> Faraday A369 EVB provides the on-board DDR2-SODIMM (512 MB),
>>> NAND Flash (256 MB), SPI Flash ROM (16 MB), I2C EPROM, Giga-bit Ethernet,
>>> IDE, SATA host, OTG 2.0 and USB 2.0 host connectors, video output interface,
>>> TV encoder for video output, LCD controller interface, SD/MMC card reader
>>> interface, I2S codec, UART(X2), Multi-ICE, 80 ports, and an AHB extension bus.
>>>
>>> Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
>>> ---
>>>   hw/a360.c            |  271 +++++++++++++++++++++++
>>>   hw/a369.c            |  581 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>>   hw/arm/Makefile.objs |    6 +
>>>   hw/faraday.h         |   21 ++
>>>   4 files changed, 879 insertions(+)
>>>   create mode 100644 hw/a360.c
>>>   create mode 100644 hw/a369.c
>>>   create mode 100644 hw/faraday.h
>>>
>>> diff --git a/hw/a360.c b/hw/a360.c
>>> new file mode 100644
>>> index 0000000..cb0a588
>>> --- /dev/null
>>> +++ b/hw/a360.c
>>> @@ -0,0 +1,271 @@
>>> +/*
>>> + * Faraday A360 Evalution Board
>>> + *
>>> + * Copyright (c) 2012 Faraday Technology
>>> + * Written by Dante Su <dantesu@faraday-tech.com>
>>> + *
>>> + * This code is licensed under GNU GPL v2.
>>> + */
>>> +
>>> +#include "sysbus.h"
>>> +#include "arm-misc.h"
>>> +#include "devices.h"
>>> +#include "net/net.h"
>>> +#include "sysemu/sysemu.h"
>>> +#include "sysemu/blockdev.h"
>>> +#include "exec/address-spaces.h"
>>> +#include "i2c.h"
>>> +#include "boards.h"
>>> +#include "flash.h"
>>> +#include "serial.h"
>>> +#include "ssi.h"
>>> +#include "faraday.h"
>>> +
>>> +typedef struct A360State {
>>> +    ARMCPU       *cpu;
>>> +    DeviceState  *hdma;    /* AHB DMA */
>>> +    DeviceState  *pdma;    /* APB DMA */
>>> +
>>> +    MemoryRegion *as;
>>> +    MemoryRegion *ram;
>>> +
>>> +    i2c_bus      *i2c[2];
>>> +
>>> +    struct {
>>> +        MemoryRegion iomem;
>>> +    } pmu;
>>> +} a360_state;
>>> +
>>> +/* PMU block */
>>> +static uint64_t
>>> +a360_pmu_read(void *opaque, hwaddr addr, unsigned size)
>>> +{
>>> +    uint64_t ret = 0;
>>> +
>>> +    switch (addr) {
>>> +    case 0x30:
>>> +        ret = 27 << 3;
>>
>>
>> Since we do not have documentation, it would be nice at least to have descriptive defines for all these numbers.
>
> I'll do that in the incoming new patch set, but because most of these registers
> are simply useless in QEMU model, so I might prefer add descriptions.
>
> Here is the short description to SCU/PMU, DDRC and AHBC:
>
> 1. SCU/PMU:
>     Its registers ranges from 0x000 to 0x25C, and 90% of them were clock
>     and pinmux settings, and only 5 of them would be read by u-boot/linux
>     to detect current clock rate, and apply pinmux change.
>     (i.e. Enable ICE support, which is totally meanless to QEMU).
> 2. DDRC:
>     It stands for DDR controller, it is responsible for DDRII/DDRIII SDRAM init,
>     the only one thing we care in QEMU is when the SDRAM become accessible.
>     Obviously 90% of the registers are meanless to QEMU.
>
> 3. AHBC:
>     It stands for AHB controller, it simply describe each slave device settings
>     (i.e. address and size), in my newer bootstrap code, I'll try to find out
>     the base address and size for slave 4 and 6, those would be 'swapped'
>     after AHB remapping. Obviously 90% of the registers are meanless to QEMU.
>
>>
>>
>>
>>> +        break;
>>> +    }
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +static void
>>> +a360_pmu_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
>>> +{
>>> +}
>>> +
>>> +static const MemoryRegionOps a360_pmu_ops = {
>>> +    .read  = a360_pmu_read,
>>> +    .write = a360_pmu_write,
>>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>>> +};
>>> +
>>> +static int
>>> +a360_pmu_init(a360_state *s, hwaddr base)
>>> +{
>>> +    MemoryRegion *iomem = &s->pmu.iomem;
>>> +    memory_region_init_io(iomem, &a360_pmu_ops, s, "a360_pmu", 0x1000);
>>> +    memory_region_add_subregion(s->as, base, iomem);
>>> +    return 0;
>>> +}
>>> +
>>> +/* Board init. */
>>> +static void
>>> +a360_device_init(a360_state *s)
>>> +{
>>> +    qemu_irq *pic;
>>> +    qemu_irq ack, req;
>>> +    qemu_irq cs_line;
>>> +    DeviceState *ds;
>>> +    int i, done_nic = 0, nr_flash = 1;
>>> +    SSIBus *spi;
>>> +    DeviceState *fl;
>>> +
>>> +    /* Interrupt Controller */
>>> +    pic = ftintc020_init(0x98800000, s->cpu);
>>
>>
>>
>> You haven't introduced this interrupt controller yet, patches should be arranged in such an order that they at least wouldn't break a build.
>> Same goes for ftintc020_init and ftgmac100_init.
>
>
> I thought that's why patch set is designed for.
> And I susposed to split up each component and send out patches one by one?
>
>>
>>
>>
>>> +
>>> +    /* Timer */
>>> +    ds = qdev_create(NULL, "fttmr010");
>>> +    qdev_prop_set_uint32(ds, "freq", 66 * 1000000);
>>> +    qdev_init_nofail(ds);
>>> +    sysbus_mmio_map(SYS_BUS_DEVICE(ds), 0, 0x98400000);
>>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 0, pic[42]);
>>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 1, pic[19]);
>>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 2, pic[14]);
>>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 3, pic[15]);
>>> +
>>> +    /* Serial (FTUART010 which is 16550A compatible) */
>>> +    if (serial_hds[0]) {
>>> +        serial_mm_init(s->as,
>>> +                       0x98200000,
>>> +                       2,
>>> +                       pic[10],
>>> +                       18432000 / 16,
>>> +                       serial_hds[0],
>>> +                       DEVICE_LITTLE_ENDIAN);
>>> +    }
>>> +    if (serial_hds[1]) {
>>> +        serial_mm_init(s->as,
>>> +                       0x98300000,
>>> +                       2,
>>> +                       pic[11],
>>> +                       18432000 / 16,
>>> +                       serial_hds[1],
>>> +                       DEVICE_LITTLE_ENDIAN);
>>> +    }
>>> +
>>> +    /* pmu */
>>> +    a360_pmu_init(s, 0x98100000);
>>> +
>>> +    /* ftdmac020 */
>>> +    s->hdma = sysbus_create_varargs("ftdmac020",
>>> +                                    0x90400000,
>>> +                                    pic[21], /* ALL */
>>> +                                    pic[40], /* TC */
>>> +                                    pic[41], /* ERR */
>>> +                                    NULL);
>>> +
>>> +    /* ftapbbrg020 */
>>> +    s->pdma = sysbus_create_simple("ftapbbrg020", 0x90500000, pic[24]);
>>> +
>>> +    /* ftsdc010 */
>>> +    ds = sysbus_create_simple("ftsdc010", 0x90700000, pic[5]);
>>> +    ack = qdev_get_gpio_in(ds, 0);
>>> +    req = qdev_get_gpio_in(s->hdma, 0);
>>> +    qdev_connect_gpio_out(s->hdma, 0, ack);
>>> +    qdev_connect_gpio_out(ds, 0, req);
>>> +
>>> +    /* fusbh200 */
>>> +    sysbus_create_simple("faraday-ehci-usb", 0x90A00000, pic[23]);
>>> +
>>> +    /* fotg210 */
>>> +    sysbus_create_simple("faraday-ehci-usb", 0x90B00000, pic[26]);
>>> +
>>> +    /* ftmac110 */
>>> +    for (i = 0; i < nb_nics; i++) {
>>> +        NICInfo *nd = &nd_table[i];
>>> +        if (!done_nic && (!nd->model || strcmp(nd->model, "ftmac110") == 0)) {
>>> +            ftmac110_init(nd, 0x90900000, pic[25]);
>>> +            done_nic = 1;
>>
>>
>> What is it for? It never checked anywhere.
>
> Did you mean 'done_nic' ?
> It's used to avoid initializing 'ftmac110' twice, and frankly
> speaking, it's copied from versatilepb.c.
>
>>
>>
>>
>>> +        }
>>> +    }
>>> +
>>> +    /* ftwdt010 */
>>> +    sysbus_create_simple("ftwdt010", 0x98500000, pic[16]);
>>> +
>>> +    /* ftlcdc200 */
>>> +    sysbus_create_varargs("ftlcdc200",
>>> +                          0x90600000,
>>> +                          pic[27],  /* Global */
>>> +                          NULL);
>>
>>
>>
>> Why sysbus_create_simple() before but sysbus_create_varargs() here? Also, this comment is not very useful, what is it trying to say?
>
> Sorry, it's a typo.
> There is 5 interrupt pin from FTLCDC200, however only one of them
> connected with A360.
> I simply forget to change the function while the interrupt has cut
> down to 1 only.
>
>>
>>
>>
>>> +
>>> +    /* fti2c010 */
>>> +    ds = sysbus_create_simple("fti2c010", 0x98A00000, pic[3]);
>>> +    s->i2c[0] = (i2c_bus *)qdev_get_child_bus(ds, "i2c");
>>> +    ds = sysbus_create_simple("fti2c010", 0x98C00000, pic[22]);
>>> +    s->i2c[1] = (i2c_bus *)qdev_get_child_bus(ds, "i2c");
>>> +
>>> +    /* ftssp010 */
>>> +    ds = qdev_create(NULL, "ftssp010");
>>> +
>>> +    /* i2s */
>>> +    qdev_prop_set_ptr(ds, "codec_i2c", s->i2c[0]);
>>> +    qdev_init_nofail(ds);
>>> +    sysbus_mmio_map(SYS_BUS_DEVICE(ds), 0, 0x98B00000);
>>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 0, pic[2]);
>>> +
>>> +    /* spi */
>>> +    spi = (SSIBus *)qdev_get_child_bus(ds, "spi");
>>> +    for (i = 0; i < nr_flash; i++) {
>>> +        fl = ssi_create_slave_no_init(spi, "m25p80");
>>> +        qdev_prop_set_string(fl, "partname", "w25q64");
>>> +        qdev_init_nofail(fl);
>>> +        cs_line = qdev_get_gpio_in(fl, 0);
>>> +        sysbus_connect_irq(SYS_BUS_DEVICE(ds), i+1, cs_line);
>>> +    }
>>> +
>>> +    /* DMA (Tx) */
>>> +    ack = qdev_get_gpio_in(ds, 0);
>>> +    req = qdev_get_gpio_in(s->pdma, 1);
>>> +    qdev_connect_gpio_out(s->pdma, 1, ack);
>>> +    qdev_connect_gpio_out(ds, 0, req);
>>> +
>>> +    /* DMA (Rx) */
>>> +    ack = qdev_get_gpio_in(ds, 1);
>>> +    req = qdev_get_gpio_in(s->pdma, 2);
>>> +    qdev_connect_gpio_out(s->pdma, 2, ack);
>>> +    qdev_connect_gpio_out(ds, 1, req);
>>> +}
>>> +
>>> +static void
>>> +a360_board_reset(void *opaque)
>>> +{
>>> +    a360_state *s = opaque;
>>> +
>>> +    cpu_reset(CPU(s->cpu));
>>> +}
>>> +
>>
>>
>>
>> Why is this necessary? CPU reset handler is registered in arm_load_kernel().
>
> I planed to implement AHB remap (ROM emulation) to A360, and that's why
> I need a board reset here. At the end, this function is cancled because the only
> reason the A360 is created to QEMU is now for FTMAC110 test only.
>
> So it would be removed later.
>
>>
>>
>>
>>> +static void
>>> +a360_board_init(QEMUMachineInitArgs *args)
>>> +{
>>> +    struct arm_boot_info *bi = NULL;
>>> +    a360_state *s = g_new(a360_state, 1);
>>> +
>>> +    s->as = get_system_memory();
>>> +    s->ram  = g_new(MemoryRegion, 1);
>>> +
>>> +    /* CPU */
>>> +    if (!args->cpu_model) {
>>> +        args->cpu_model = "fa626te";
>>> +    }
>>> +
>>> +    s->cpu = cpu_arm_init(args->cpu_model);
>>> +    if (!s->cpu) {
>>> +        fprintf(stderr, "Unable to find CPU definition\n");
>>> +        exit(1);
>>> +    }
>>> +
>>> +    /* A360 supports upto 1GB ram space */
>>> +    if (args->ram_size > 0x40000000) {
>>> +        args->ram_size = 0x40000000;
>>> +    }
>>> +
>>> +    printf("qemu: faraday a360 with %dMB ram.\n", args->ram_size >> 20);
>>
>>
>> You should use RAM_ADDR_FMT for ram_addr_t formatting or else it would break build.
>> But usually QEMU doesn't use such messages when initialising devices, maybe just drop it?
>
> It has been drop now.
>
>>
>>
>>> +
>>> +    /* RAM Init */
>>> +    memory_region_init_ram(s->ram, "a360.ram", args->ram_size);
>>> +    vmstate_register_ram_global(s->ram);
>>> +
>>> +    a360_device_init(s);
>>> +    qemu_register_reset(a360_board_reset, s);
>>> +
>>> +    /* Prepare for direct boot from linux kernel or u-boot elf */
>>> +    bi = g_new0(struct arm_boot_info, 1);
>>> +
>>> +    /* RAM Address Binding */
>>> +    memory_region_add_subregion(s->as, 0x00000000, s->ram);
>>> +
>>> +    /* Boot Info */
>>> +    bi->ram_size = args->ram_size;
>>> +    bi->kernel_filename = args->kernel_filename;
>>> +    bi->kernel_cmdline = args->kernel_cmdline;
>>> +    bi->initrd_filename = args->initrd_filename;
>>> +    bi->board_id = 0xa360;
>>> +    arm_load_kernel(s->cpu, bi);
>>> +}
>>> +
>>> +static QEMUMachine a360_machine = {
>>> +    .name = "a360",
>>> +    .desc = "Faraday A360 (fa626te)",
>>> +    .init = a360_board_init,
>>
>>
>>
>> Not sure if its mandatory, but looks like you need to add DEFAULT_MACHINE_OPTIONS here.
>
> It looks like a new item in the master, while I copied this example
> code from realview.c
> in qemu-1.3.0, there is no such thing.
> Thanks for reminding me of it.
>
>>
>>
>>
>>> +};
>>> +
>>> +static void
>>> +a360_machine_init(void)
>>> +{
>>> +    qemu_register_machine(&a360_machine);
>>> +}
>>> +
>>> +machine_init(a360_machine_init);
>>> diff --git a/hw/a369.c b/hw/a369.c
>>> new file mode 100644
>>> index 0000000..646888f
>>> --- /dev/null
>>> +++ b/hw/a369.c
>>> @@ -0,0 +1,581 @@
>>> +/*
>>> + * Faraday A369 Evalution Board
>>> + *
>>> + * Copyright (c) 2012 Faraday Technology
>>> + * Written by Dante Su <dantesu@faraday-tech.com>
>>> + *
>>> + * This code is licensed under GNU GPL v2.
>>> + */
>>> +
>>> +#include "sysbus.h"
>>> +#include "arm-misc.h"
>>> +#include "devices.h"
>>> +#include "net/net.h"
>>> +#include "sysemu/sysemu.h"
>>> +#include "sysemu/blockdev.h"
>>> +#include "exec/address-spaces.h"
>>> +#include "i2c.h"
>>> +#include "boards.h"
>>> +#include "flash.h"
>>> +#include "serial.h"
>>> +#include "ssi.h"
>>> +#include "faraday.h"
>>> +
>>> +#define A369_NOR_FLASH_ADDR         0x20000000
>>> +#define A369_NOR_FLASH_SIZE         (16 * 1024 * 1024)
>>> +#define A369_NOR_FLASH_SECT_SIZE    (128 * 1024)
>>> +
>>> +typedef struct A369State {
>>> +    ARMCPU       *cpu;
>>> +    DeviceState  *rom;
>>> +    DeviceState  *hdma[2];    /* AHB DMA */
>>> +    DeviceState  *pdma;    /* APB DMA */
>>> +
>>> +    MemoryRegion *as;
>>> +    MemoryRegion *ram;
>>> +    MemoryRegion *ram_alias;
>>> +    MemoryRegion *sram;
>>> +
>>> +    i2c_bus      *i2c[2];
>>> +
>>> +    struct {
>>> +        MemoryRegion iomem;
>>> +        /* HW register cache */
>>> +        uint32_t general_cfg;
>>> +        uint32_t sclk_cfg0;
>>> +        uint32_t sclk_cfg1;
>>> +        uint32_t mfpsr0;
>>> +        uint32_t mfpsr1;
>>> +    } scu;
>>> +
>>> +    struct {
>>> +        MemoryRegion iomem;
>>> +        /* HW register cache */
>>> +        uint32_t cr;
>>> +    } ahbc;
>>> +
>>> +    struct {
>>> +        MemoryRegion iomem;
>>> +        /* HW register cache */
>>> +        uint32_t mcr;
>>> +        uint32_t msr;
>>> +    } ddrc;
>>> +
>>> +} a369_state;
>>> +
>>> +/* SCU block */
>>> +
>>> +static uint64_t
>>> +a369_scu_read(void *opaque, hwaddr addr, unsigned size)
>>> +{
>>> +    a369_state *s = opaque;
>>> +
>>> +    switch (addr) {
>>> +    case 0x000: return 0x00003369;
>>> +    case 0x004: return 0x00010000;
>>> +    case 0x008: return 0x00000c10;
>>> +    case 0x00c: return 0x00000230;
>>> +    case 0x010: return 0x00000083;
>>> +    case 0x014: return 0x00000100;
>>> +    case 0x01C: return 0x00000003;
>>> +    case 0x020: return 0x20010003;
>>> +    case 0x024: return 0x00000003;
>>> +    case 0x060: return 0x00280028;
>>> +    case 0x200: return s->scu.general_cfg;
>>> +    case 0x204: return 0x00001cc8;
>>> +    case 0x228: return s->scu.sclk_cfg0;
>>> +    case 0x22c: return s->scu.sclk_cfg1;
>>> +    case 0x230: return 0x00003fff;
>>> +    case 0x238: return s->scu.mfpsr0;
>>> +    case 0x23c: return s->scu.mfpsr1;
>>> +    case 0x240: return 0x11111111;
>>> +    case 0x244: return 0x11111111;
>>> +    case 0x254: return 0x00000303;
>>> +    case 0x258: return 0x8000007f;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static void
>>> +a369_scu_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
>>> +{
>>> +    a369_state *s = opaque;
>>> +
>>> +    switch (addr) {
>>> +    case 0x200:
>>> +        s->scu.general_cfg = (uint32_t)val;
>>> +        break;
>>> +    case 0x228:
>>> +        s->scu.sclk_cfg0 = (uint32_t)val;
>>> +        break;
>>> +    case 0x22c:
>>> +        s->scu.sclk_cfg1 = (uint32_t)val;
>>> +        break;
>>> +    case 0x238:
>>> +        s->scu.mfpsr0 = (uint32_t)val;
>>> +        break;
>>> +    case 0x23c:
>>> +        s->scu.mfpsr1 = (uint32_t)val;
>>> +        break;
>>> +    }
>>> +}
>>> +
>>> +static const MemoryRegionOps a369_scu_ops = {
>>> +    .read  = a369_scu_read,
>>> +    .write = a369_scu_write,
>>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>>> +    .valid = {
>>> +        .min_access_size = 4,
>>> +        .max_access_size = 4
>>> +    }
>>> +};
>>> +
>>> +static int
>>> +a369_scu_init(a369_state *s, hwaddr base)
>>> +{
>>> +    MemoryRegion *iomem = &s->scu.iomem;
>>> +
>>> +    memory_region_init_io(iomem, &a369_scu_ops, s, "a369_scu", 0x1000);
>>> +    memory_region_add_subregion(s->as, base, iomem);
>>> +
>>> +    s->scu.general_cfg = 0x00001078;
>>> +    s->scu.sclk_cfg0   = 0x26877330;
>>> +    s->scu.sclk_cfg1   = 0x000a0a0a;
>>> +    s->scu.mfpsr0      = 0x00000241;
>>> +    s->scu.mfpsr1      = 0x00000000;
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +/* AHB controller block */
>>> +
>>> +static uint64_t
>>> +a369_ahbc_read(void *opaque, hwaddr addr, unsigned size)
>>> +{
>>> +    a369_state *s = opaque;
>>> +
>>> +    switch (addr) {
>>> +    case 0x00: return 0x94050000;
>>> +    case 0x04: return 0x96040000;
>>> +    case 0x08: return 0x90f00000;
>>> +    case 0x0c: return 0x92050000;
>>> +    case 0x10: return 0x20080000;
>>> +    case 0x14: return 0xc0080000;
>>> +    case 0x18: return 0x00090000;
>>> +    case 0x1c: return 0x90000000;
>>> +    case 0x20: return 0x90100000;
>>> +    case 0x24: return 0x90200000;
>>> +    case 0x28: return 0x90300000;
>>> +    case 0x2c: return 0x90400000;
>>> +    case 0x30: return 0x90500000;
>>> +    case 0x34: return 0x90600000;
>>> +    case 0x38: return 0x90700000;
>>> +    case 0x3c: return 0x90800000;
>>> +    case 0x40: return 0x90900000;
>>> +    case 0x44: return 0x90a00000;
>>> +    case 0x48: return 0x90b00000;
>>> +    case 0x4c: return 0x90c00000;
>>> +    case 0x50: return 0x90d00000;
>>> +    case 0x54: return 0x90e00000;
>>> +    case 0x58: return 0x40080000;
>>> +    case 0x5c: return 0x60080000;
>>> +    case 0x60: return 0xa0000000;
>>> +    case 0x84: return 0x00000001;
>>> +    case 0x88: return s->ahbc.cr;
>>> +    case 0x8c: return 0x00010301;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static void
>>> +a369_ahbc_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
>>> +{
>>> +    a369_state *s = opaque;
>>> +
>>> +    switch (addr) {
>>> +    case 0x88:
>>> +        if (!(s->ahbc.cr & 0x01) && (val & 0x01)) {
>>> +            /* AHB remap */
>>> +            sysbus_mmio_map(SYS_BUS_DEVICE(s->rom), 0, 0x40000000);
>>> +            memory_region_del_subregion(s->as, s->ram_alias);
>>> +            memory_region_add_subregion(s->as, 0x00000000, s->ram);
>>> +        }
>>> +        s->ahbc.cr = (uint32_t)val;
>>> +        break;
>>> +    }
>>> +}
>>> +
>>> +static const MemoryRegionOps a369_ahbc_ops = {
>>> +    .read  = a369_ahbc_read,
>>> +    .write = a369_ahbc_write,
>>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>>> +};
>>> +
>>> +static int
>>> +a369_ahbc_init(a369_state *s, hwaddr base)
>>> +{
>>> +    MemoryRegion *iomem = &s->ahbc.iomem;
>>> +
>>> +    memory_region_init_io(iomem, &a369_ahbc_ops, s, "a369_ahbc", 0x1000);
>>> +    memory_region_add_subregion(s->as, base, iomem);
>>> +
>>> +    s->ahbc.cr = 0;
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +/* DDRII controller block */
>>> +
>>> +static uint64_t
>>> +a369_ddrc_read(void *opaque, hwaddr addr, unsigned size)
>>> +{
>>> +    a369_state *s = opaque;
>>> +
>>> +    switch (addr) {
>>> +    case 0x00:  return s->ddrc.mcr;
>>> +    case 0x04:  return s->ddrc.msr;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static void
>>> +a369_ddrc_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
>>> +{
>>> +    a369_state *s = opaque;
>>> +
>>> +    switch (addr) {
>>> +    case 0x00:
>>> +        s->ddrc.mcr = (uint32_t)val & 0xffff;
>>> +        break;
>>> +    case 0x04:
>>> +        val = (val & 0x3f) | (s->ddrc.msr & 0x100);
>>> +        if (!(s->ddrc.msr & 0x100) && (val & 0x01)) {
>>> +            val &= 0xfffffffe;
>>> +            val |= 0x100;
>>> +            memory_region_add_subregion(s->as, 0x10000000, s->ram_alias);
>>> +        }
>>> +        s->ddrc.msr = (uint32_t)val;
>>> +        break;
>>> +    }
>>> +}
>>> +
>>> +static const MemoryRegionOps a369_ddrc_ops = {
>>> +    .read  = a369_ddrc_read,
>>> +    .write = a369_ddrc_write,
>>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>>> +};
>>> +
>>> +static int
>>> +a369_ddrc_init(a369_state *s, hwaddr base)
>>> +{
>>> +    MemoryRegion *iomem = &s->ddrc.iomem;
>>> +
>>> +    memory_region_init_io(iomem, &a369_ddrc_ops, s, "a369_ddrc", 0x1000);
>>> +    memory_region_add_subregion(s->as, base, iomem);
>>> +
>>> +    s->ddrc.mcr = 0;
>>> +    s->ddrc.msr = 0;
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +/* Board init.  */
>>> +
>>> +static void
>>> +a369_device_init(a369_state *s)
>>> +{
>>> +    qemu_irq *pic;
>>> +    qemu_irq ack, req;
>>> +    qemu_irq cs_line;
>>> +    DeviceState *ds;
>>> +    int i, done_nic = 0, nr_flash = 1;
>>> +    SSIBus *spi;
>>> +    DeviceState *fl;
>>> +    DriveInfo *dinfo;
>>> +
>>> +    /* Interrupt Controller */
>>> +    pic = ftintc020_init(0x90100000, s->cpu);
>>> +
>>> +    /* Timer */
>>> +    ds = qdev_create(NULL, "ftpwmtmr010");
>>> +    qdev_prop_set_uint32(ds, "freq", 66 * 1000000);
>>> +    qdev_init_nofail(ds);
>>> +    sysbus_mmio_map(SYS_BUS_DEVICE(ds), 0, 0x92300000);
>>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 0, pic[8]);
>>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 1, pic[9]);
>>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 2, pic[10]);
>>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 3, pic[11]);
>>> +
>>> +    /* Serial (FTUART010 which is 16550A compatible) */
>>> +    if (serial_hds[0]) {
>>> +        serial_mm_init(s->as,
>>> +                       0x92b00000,
>>> +                       2,
>>> +                       pic[53],
>>> +                       18432000 / 16,
>>> +                       serial_hds[0],
>>> +                       DEVICE_LITTLE_ENDIAN);
>>> +    }
>>> +    if (serial_hds[1]) {
>>> +        serial_mm_init(s->as,
>>> +                       0x92c00000,
>>> +                       2,
>>> +                       pic[54],
>>> +                       18432000 / 16,
>>> +                       serial_hds[1],
>>> +                       DEVICE_LITTLE_ENDIAN);
>>> +    }
>>> +
>>> +    /* ftscu010 */
>>> +    a369_scu_init(s, 0x92000000);
>>> +
>>> +    /* ftddrII030 */
>>> +    a369_ddrc_init(s, 0x93100000);
>>> +
>>> +    /* ftahbc020 */
>>> +    a369_ahbc_init(s, 0x94000000);
>>> +
>>> +    /* ftdmac020 */
>>> +    s->hdma[0] = sysbus_create_varargs("ftdmac020",
>>> +                                       0x90300000,
>>> +                                       pic[0],  /* ALL (NC in A369) */
>>> +                                       pic[15], /* TC */
>>> +                                       pic[16], /* ERR */
>>> +                                       NULL);
>>> +    s->hdma[1] = sysbus_create_varargs("ftdmac020",
>>> +                                       0x96100000,
>>> +                                       pic[0],  /* ALL (NC in A369) */
>>> +                                       pic[17], /* TC */
>>> +                                       pic[18], /* ERR */
>>> +                                       NULL);
>>> +
>>> +    /* ftapbbrg020 */
>>> +    s->pdma = sysbus_create_simple("ftapbbrg020", 0x90f00000, pic[14]);
>>> +
>>> +    /* ftnandc021 */
>>> +    ds = sysbus_create_simple("ftnandc021", 0x90200000, pic[30]);
>>> +    ack = qdev_get_gpio_in(ds, 0);
>>> +    req = qdev_get_gpio_in(s->hdma[0], 15);
>>> +    qdev_connect_gpio_out(s->hdma[0], 15, ack);
>>> +    qdev_connect_gpio_out(ds, 0, req);
>>> +
>>> +    /* ftsdc010 */
>>> +#if 0
>>> +    ds = sysbus_create_simple("ftsdc010", 0x90500000, pic[38]);
>>> +    ack = qdev_get_gpio_in(ds, 0);
>>> +    req = qdev_get_gpio_in(s->hdma[0], 14);
>>> +    qdev_connect_gpio_out(s->hdma[0], 14, ack);
>>> +    qdev_connect_gpio_out(ds, 0, req);
>>> +#endif
>>> +
>>> +    ds = sysbus_create_simple("ftsdc010", 0x90600000, pic[39]);
>>> +    ack = qdev_get_gpio_in(ds, 0);
>>> +    req = qdev_get_gpio_in(s->hdma[0], 13);
>>> +    qdev_connect_gpio_out(s->hdma[0], 13, ack);
>>> +    qdev_connect_gpio_out(ds, 0, req);
>>> +
>>> +    /* fusbh200 */
>>> +    sysbus_create_simple("faraday-ehci-usb", 0x90800000, pic[36]);
>>> +
>>> +    /* fotg210 */
>>> +    sysbus_create_simple("faraday-ehci-usb", 0x90900000, pic[37]);
>>> +
>>> +    /* ftgmac100 */
>>> +    for (i = 0; i < nb_nics; i++) {
>>> +        NICInfo *nd = &nd_table[i];
>>> +        if (!done_nic && (!nd->model || strcmp(nd->model, "ftgmac100") == 0)) {
>>> +            ftgmac100_init(nd, 0x90c00000, pic[32]);
>>> +            done_nic = 1;
>>> +        }
>>> +    }
>>> +
>>> +    /* ftrtc011 (only alarm interrupt is connected) */
>>> +    sysbus_create_varargs("ftrtc011",
>>> +                          0x92100000,
>>> +                          pic[0],     /* Alarm (Level): NC in A369 */
>>> +                          pic[42],    /* Alarm (Edge) */
>>> +                          pic[43],    /* Second (Edge) */
>>> +                          pic[44],    /* Minute (Edge) */
>>> +                          pic[45],    /* Hour (Edge) */
>>> +                          NULL);
>>> +
>>> +    /* ftwdt010 */
>>> +    sysbus_create_simple("ftwdt010", 0x92200000, pic[46]);
>>> +
>>> +    /* fttsc010 */
>>> +    sysbus_create_simple("fttsc010", 0x92400000, pic[19]);
>>> +
>>> +    /* ftkbc010 */
>>> +    sysbus_create_simple("ftkbc010", 0x92f00000, pic[21]);
>>> +
>>> +    /* ftlcdc200 */
>>> +    sysbus_create_varargs("ftlcdc200",
>>> +                          0x94a00000,
>>> +                          pic[0],  /* ALL (NC in A369) */
>>> +                          pic[25], /* VSTATUS */
>>> +                          pic[24], /* Base Address Update */
>>> +                          pic[23], /* FIFO Under-Run */
>>> +                          pic[22], /* AHB Bus Error */
>>> +                          NULL);
>>> +
>>> +    /* fti2c010 */
>>> +    ds = sysbus_create_simple("fti2c010", 0x92900000, pic[51]);
>>> +    s->i2c[0] = (i2c_bus *)qdev_get_child_bus(ds, "i2c");
>>> +    ds = sysbus_create_simple("fti2c010", 0x92A00000, pic[52]);
>>> +    s->i2c[1] = (i2c_bus *)qdev_get_child_bus(ds, "i2c");
>>> +
>>> +    /* ftssp010 */
>>> +    ds = qdev_create(NULL, "ftssp010");
>>> +
>>> +    /* i2s */
>>> +    qdev_prop_set_ptr(ds, "codec_i2c", s->i2c[0]);
>>> +    qdev_init_nofail(ds);
>>> +    sysbus_mmio_map(SYS_BUS_DEVICE(ds), 0, 0x92700000);
>>> +    sysbus_connect_irq(SYS_BUS_DEVICE(ds), 0, pic[49]);
>>> +
>>> +    /* spi */
>>> +    spi = (SSIBus *)qdev_get_child_bus(ds, "spi");
>>> +    for (i = 0; i < nr_flash; i++) {
>>> +        fl = ssi_create_slave_no_init(spi, "m25p80");
>>> +        qdev_prop_set_string(fl, "partname", "w25q64");
>>> +        qdev_init_nofail(fl);
>>> +        cs_line = qdev_get_gpio_in(fl, 0);
>>> +        sysbus_connect_irq(SYS_BUS_DEVICE(ds), i+1, cs_line);
>>> +    }
>>> +
>>> +    /* DMA (Tx) */
>>> +    ack = qdev_get_gpio_in(ds, 0);
>>> +    req = qdev_get_gpio_in(s->pdma, 7);
>>> +    qdev_connect_gpio_out(s->pdma, 7, ack);
>>> +    qdev_connect_gpio_out(ds, 0, req);
>>> +
>>> +    /* DMA (Rx) */
>>> +    ack = qdev_get_gpio_in(ds, 1);
>>> +    req = qdev_get_gpio_in(s->pdma, 8);
>>> +    qdev_connect_gpio_out(s->pdma, 8, ack);
>>> +    qdev_connect_gpio_out(ds, 1, req);
>>> +
>>> +    /* Parallel NOR Flash */
>>> +    dinfo = drive_get_next(IF_PFLASH);
>>> +    if (!pflash_cfi01_register(
>>> +                    A369_NOR_FLASH_ADDR,
>>> +                    NULL,
>>> +                    "a369.pflash",
>>> +                    A369_NOR_FLASH_SIZE,
>>> +                    dinfo ? dinfo->bdrv : NULL,
>>> +                    A369_NOR_FLASH_SECT_SIZE,
>>> +                    A369_NOR_FLASH_SIZE / A369_NOR_FLASH_SECT_SIZE,
>>> +                    2, 0x0001, 0x227E, 0x2101, 0x0, 0)) {
>>> +        hw_error("qemu: Error registering flash memory.\n");
>>> +        exit(1);
>>> +    }
>>> +}
>>> +
>>> +static void
>>> +a369_board_reset(void *opaque)
>>> +{
>>> +    a369_state *s = opaque;
>>> +
>>> +    if (s->ddrc.msr) {
>>> +        s->ddrc.mcr = 0;
>>> +        s->ddrc.msr = 0;
>>> +        if (s->ahbc.cr) {
>>> +            /* AHB remapped */
>>> +            sysbus_mmio_map(SYS_BUS_DEVICE(s->rom), 0, 0x00000000);
>>> +            memory_region_del_subregion(s->as, s->ram);
>>> +        } else {
>>> +            /* AHB is not yet remapped, but SDRAM is ready */
>>> +            memory_region_del_subregion(s->as, s->ram_alias);
>>> +        }
>>> +        s->ahbc.cr = 0;
>>> +    }
>>> +
>>> +    cpu_reset(CPU(s->cpu));
>>> +}
>>> +
>>> +static void
>>> +a369_board_init(QEMUMachineInitArgs *args)
>>> +{
>>> +    struct arm_boot_info *bi = NULL;
>>> +    a369_state *s = g_new(a369_state, 1);
>>> +
>>> +    s->as = get_system_memory();
>>> +    s->ram  = g_new(MemoryRegion, 1);
>>> +    s->sram = g_new(MemoryRegion, 1);
>>> +
>>> +    /* CPU */
>>> +    if (!args->cpu_model) {
>>> +        args->cpu_model = "fa626te";
>>> +    }
>>> +
>>> +    s->cpu = cpu_arm_init(args->cpu_model);
>>> +    if (!s->cpu) {
>>> +        fprintf(stderr, "Unable to find CPU definition\n");
>>> +        exit(1);
>>> +    }
>>> +
>>> +    /* A369 supports upto 1GB ram space */
>>> +    if (args->ram_size > 0x40000000) {
>>> +        args->ram_size = 0x40000000;
>>> +    }
>>> +
>>> +    printf("qemu: faraday a369 with %dMB ram.\n", args->ram_size >> 20);
>>> +
>>> +    /* Embedded ROM Init */
>>> +    s->rom = qdev_create(NULL, "rom");
>>> +    qdev_prop_set_uint32(s->rom, "size", 8192);
>>> +    qdev_init_nofail(s->rom);
>>> +
>>> +    /* Embedded RAM Init */
>>> +    memory_region_init_ram(s->sram, "a369.sram", 0x4000);
>>> +    vmstate_register_ram_global(s->sram);
>>> +    memory_region_add_subregion(s->as, 0xA0000000, s->sram);
>>> +
>>> +    /* RAM Init */
>>> +    memory_region_init_ram(s->ram, "a369.ram", args->ram_size);
>>> +    vmstate_register_ram_global(s->ram);
>>> +
>>> +    a369_device_init(s);
>>> +    qemu_register_reset(a369_board_reset, s);
>>> +
>>> +    if (args->kernel_filename) {
>>> +        bi = g_new0(struct arm_boot_info, 1);
>>> +
>>> +        /* RAM Address Binding */
>>> +        memory_region_add_subregion(s->as, 0x00000000, s->ram);
>>> +
>>> +        /* Boot Info */
>>> +        bi->ram_size = args->ram_size;
>>> +        bi->kernel_filename = args->kernel_filename;
>>> +        bi->kernel_cmdline = args->kernel_cmdline;
>>> +        bi->initrd_filename = args->initrd_filename;
>>> +        bi->board_id = 0xa369;
>>> +        arm_load_kernel(s->cpu, bi);
>>> +    } else {
>>> +        /* ROM Address Binding */
>>> +        sysbus_mmio_map(SYS_BUS_DEVICE(s->rom), 0, 0x00000000);
>>> +        /* Partial RAM (before ahb remapped) Address Binding */
>>> +        s->ram_alias = g_new(MemoryRegion, 1);
>>> +        memory_region_init_alias(s->ram_alias, "a369.ram_alias",
>>> +                                 s->ram,
>>> +                                 0,
>>> +                                 MIN(0x10000000, args->ram_size));
>>> +    }
>>
>>
>>
>> Sorry, could you please explain what will happen when we do not specify -kernel argument?

My bad, I forget to answer to this question at previous mail.
In A36x SoC platform, we used to remap SDRAM to 0x00000000 before running
any software (except bootstrap code, of course.)

Let's take A369 for exmaple:

1. While system bootup, the memory mapping is layouted as this:

        ROM:  0x00000000
    SDRAM:  0x10000000
      SRAM:  0xA0000000

   and the bootstrap code is configured as

              TEXT/RO-DATA@0x00000000
   BSS/RW-DATA/STACK@0xA0000000

2. When bootstrap finished its work, the software (u-boot/linux/wince)
    would be loaded into 0x10000000, and before transffering control to
    the loaded software, the bootstrap would remap the AHB slave 4 (SDRAM) and
    slave 6(ROM). See the example bellow:

    Before AHB remap:

        ROM:  0x00000000   (bootstrap code)
    SDRAM:  0x10000000   (software: u-boot/linux/wince)
      SRAM:  0xA0000000   (bootsreap bss/data/stack)

   After AHB remap:

        ROM:  0x40000000   (bootstrap code)
    SDRAM:  0x00000000   (software: u-boot/linux/wince)
      SRAM:  0xA0000000   (bootsreap bss/data/stack)

    When the AHB remapping process is finished, bootstrap code would
    move pc to 0x00000000 and the general software holds the entire resources.

   You might ask why should A36x reamp the AHB slaves?
   It's primary used for processors wihout MMU. Because ARM core expects
   exception table to be stored at either 0x00000000 or 0xffff0000.
   Without AHB remap, these no-mmu processor would never be able to support
   interrupts. And after days past, the AHB remap becomes a general
design in Faraday.

>>
>>
>>
>>
>>> +}
>>> +
>>> +static QEMUMachine a369_machine = {
>>> +    .name = "a369",
>>> +    .desc = "Faraday A369 (fa626te)",
>>> +    .init = a369_board_init,
>>> +};
>>> +
>>> +static void
>>> +a369_machine_init(void)
>>> +{
>>> +    qemu_register_machine(&a369_machine);
>>> +}
>>> +
>>> +machine_init(a369_machine_init);
>>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>>> index 6d049e7..c7bb10e 100644
>>> --- a/hw/arm/Makefile.objs
>>> +++ b/hw/arm/Makefile.objs
>>> @@ -1,4 +1,10 @@
>>>   obj-y = integratorcp.o versatilepb.o arm_pic.o
>>> +obj-y += a360.o a369.o \
>>> +                               rom.o ftdmac020.o ftapbbrg020.o \
>>> +                               ftintc020.o fttmr010.o ftpwmtmr010.o \
>>> +                               ftspi020.o ftssp010.o fti2c010.o \
>>> +                               ftrtc011.o ftwdt010.o ftmac110.o ftgmac100.o ftlcdc200.o \
>>> +                               fttsc010.o ftkbc010.o ftnandc021.o ftsdc010.o
>>
>>
>> No such files exist at this point, you should add them here one by one in a corresponding patch.
>> And tabs should be replaced with spaces.
>
> It looks like that I really have to split up each of one by one.
> That's good to me, because I don't have any common sense about patch process,
> Each component in one patch would be much more easier to me.
>
>>
>>
>>
>>>   obj-y += arm_boot.o
>>>   obj-y += xilinx_zynq.o zynq_slcr.o
>>>   obj-y += xilinx_spips.o
>>> diff --git a/hw/faraday.h b/hw/faraday.h
>>> new file mode 100644
>>> index 0000000..f4fe0cc
>>> --- /dev/null
>>> +++ b/hw/faraday.h
>>
>>
>>
>> None of three function prototyped in this file exists at this point, I think you should add this file later in the patch set.
>
> Same issue.
> I'm studying patchwork now, because I don't want to ack like a idiot,
> I'll send out the new patch when I 'though' I'm ready.
> It might take few days to me, I just have patchwork installed in my PC today.
>
>>
>>
>>
>>> @@ -0,0 +1,21 @@
>>> +/*
>>> + * Faraday SoC platform support.
>>> + *
>>> + * Copyright (c) 2013 Faraday Technology
>>> + * Written by Kuo-Jung Su <dantesu@gmail.com>
>>> + *
>>> + * This code is licensed under the GNU GPL v2.
>>> + */
>>> +#ifndef FARADAY_H
>>> +#define FARADAY_H
>>> +
>>> +/* ftintc020.c */
>>> +qemu_irq *ftintc020_init(hwaddr base, ARMCPU *cpu);
>>> +
>>> +/* ftgmac100.c */
>>> +void ftgmac100_init(NICInfo *, uint32_t, qemu_irq);
>>> +
>>> +/* ftmac110.c */
>>> +void ftmac110_init(NICInfo *, uint32_t, qemu_irq);
>>> +
>>> +#endif
>>>
>>
>>
>> --
>> Mitsyanko Igor
>> ASWG, Moscow R&D center, Samsung Electronics
>> email: i.mitsyanko@samsung.com
>
>
>
>
> --
> Best wishes,
> Kuo-Jung Su



--
Best wishes,
Kuo-Jung Su

Andreas Färber Feb. 1, 2013, 8:32 a.m. UTC | #5

Hi,

Am 01.02.2013 02:39, schrieb Kuo-Jung Su:
> 2013/2/1 Igor Mitsyanko <i.mitsyanko@samsung.com>
>>
>> On 01/25/2013 12:19 PM, Kuo-Jung Su wrote:
>>>
>>> +/* Board init. */
>>> +static void
>>> +a360_device_init(a360_state *s)
>>> +{
>>> +    qemu_irq *pic;
>>> +    qemu_irq ack, req;
>>> +    qemu_irq cs_line;
>>> +    DeviceState *ds;
>>> +    int i, done_nic = 0, nr_flash = 1;
>>> +    SSIBus *spi;
>>> +    DeviceState *fl;
>>> +
>>> +    /* Interrupt Controller */
>>> +    pic = ftintc020_init(0x98800000, s->cpu);
>>
>>
>>
>> You haven't introduced this interrupt controller yet, patches should be arranged in such an order that they at least wouldn't break a build.
>> Same goes for ftintc020_init and ftgmac100_init.
> 
> 
> I thought that's why patch set is designed for.
> And I susposed to split up each component and send out patches one by one?
[...]
>>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>>> index 6d049e7..c7bb10e 100644
>>> --- a/hw/arm/Makefile.objs
>>> +++ b/hw/arm/Makefile.objs
>>> @@ -1,4 +1,10 @@
>>>   obj-y = integratorcp.o versatilepb.o arm_pic.o
>>> +obj-y += a360.o a369.o \
>>> +                               rom.o ftdmac020.o ftapbbrg020.o \
>>> +                               ftintc020.o fttmr010.o ftpwmtmr010.o \
>>> +                               ftspi020.o ftssp010.o fti2c010.o \
>>> +                               ftrtc011.o ftwdt010.o ftmac110.o ftgmac100.o ftlcdc200.o \
>>> +                               fttsc010.o ftkbc010.o ftnandc021.o ftsdc010.o
>>
>>
>> No such files exist at this point, you should add them here one by one in a corresponding patch.
>> And tabs should be replaced with spaces.
> 
> It looks like that I really have to split up each of one by one.
> That's good to me, because I don't have any common sense about patch process,
> Each component in one patch would be much more easier to me.
> 
>>>   obj-y += arm_boot.o
>>>   obj-y += xilinx_zynq.o zynq_slcr.o
>>>   obj-y += xilinx_spips.o
>>> diff --git a/hw/faraday.h b/hw/faraday.h
>>> new file mode 100644
>>> index 0000000..f4fe0cc
>>> --- /dev/null
>>> +++ b/hw/faraday.h
>>
>>
>>
>> None of three function prototyped in this file exists at this point, I think you should add this file later in the patch set.
> 
> Same issue.
> I'm studying patchwork now, because I don't want to [act] like a idiot,
> I'll send out the new patch when I 'though' I'm ready.

The criteria is that every patch must be compilable on its own, we say
'bisectable' because it allows to work with git-bisect command for error
searching. Someone who is debugging an x86 problem might land on an arm
a36x commit and otherwise not be able to compile.

You can compare my Tegra patchset here (not polished yet):
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/tegra

You can add a stub version of your machine and step by step add devices
to it until it is complete. My Tegra2 SoC modeling with a QOM object is
not final yet but might serve as design inspiration. I.e., all A36x SoC
devices should have their state structs either in their own header or in
a SoC-specific header, so that they can be embedded as fields within a
SoC state struct - be it within your patchset or as a later step.

Regards,
Andreas

Kuo-Jung Su Feb. 1, 2013, 8:57 a.m. UTC | #6

Hi Andreas:

Thanks for the information, and sorry for the mess I've done.
I'll one-by-one re-send all the patches.

However because most of my patches are new files,
should I send-out the patches with detail change log?

For example:

[PATCH] dumb timer
... [PATCH v2 0/2] dumb timer (Cover letter)
    [PATCH v2 1/2] dumb timer (The one in Patch V1)
    [PATCH v2 2/2] dumb timer: coding style update (Change log for V2)
...... [PATCH v3 0/2] dumb timer (Cover letter)
       [PATCH v3 1/2] dumb timer (The merged file in Patch V1 & v2)
       [PATCH v3 2/2] dumb timer: bug fix (Change log for V3)


Best Wishes
Dante

2013/2/1 Andreas Färber <afaerber@suse.de>:
> Hi,
>
> Am 01.02.2013 02:39, schrieb Kuo-Jung Su:
>> 2013/2/1 Igor Mitsyanko <i.mitsyanko@samsung.com>
>>>
>>> On 01/25/2013 12:19 PM, Kuo-Jung Su wrote:
>>>>
>>>> +/* Board init. */
>>>> +static void
>>>> +a360_device_init(a360_state *s)
>>>> +{
>>>> +    qemu_irq *pic;
>>>> +    qemu_irq ack, req;
>>>> +    qemu_irq cs_line;
>>>> +    DeviceState *ds;
>>>> +    int i, done_nic = 0, nr_flash = 1;
>>>> +    SSIBus *spi;
>>>> +    DeviceState *fl;
>>>> +
>>>> +    /* Interrupt Controller */
>>>> +    pic = ftintc020_init(0x98800000, s->cpu);
>>>
>>>
>>>
>>> You haven't introduced this interrupt controller yet, patches should be arranged in such an order that they at least wouldn't break a build.
>>> Same goes for ftintc020_init and ftgmac100_init.
>>
>>
>> I thought that's why patch set is designed for.
>> And I susposed to split up each component and send out patches one by one?
> [...]
>>>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>>>> index 6d049e7..c7bb10e 100644
>>>> --- a/hw/arm/Makefile.objs
>>>> +++ b/hw/arm/Makefile.objs
>>>> @@ -1,4 +1,10 @@
>>>>   obj-y = integratorcp.o versatilepb.o arm_pic.o
>>>> +obj-y += a360.o a369.o \
>>>> +                               rom.o ftdmac020.o ftapbbrg020.o \
>>>> +                               ftintc020.o fttmr010.o ftpwmtmr010.o \
>>>> +                               ftspi020.o ftssp010.o fti2c010.o \
>>>> +                               ftrtc011.o ftwdt010.o ftmac110.o ftgmac100.o ftlcdc200.o \
>>>> +                               fttsc010.o ftkbc010.o ftnandc021.o ftsdc010.o
>>>
>>>
>>> No such files exist at this point, you should add them here one by one in a corresponding patch.
>>> And tabs should be replaced with spaces.
>>
>> It looks like that I really have to split up each of one by one.
>> That's good to me, because I don't have any common sense about patch process,
>> Each component in one patch would be much more easier to me.
>>
>>>>   obj-y += arm_boot.o
>>>>   obj-y += xilinx_zynq.o zynq_slcr.o
>>>>   obj-y += xilinx_spips.o
>>>> diff --git a/hw/faraday.h b/hw/faraday.h
>>>> new file mode 100644
>>>> index 0000000..f4fe0cc
>>>> --- /dev/null
>>>> +++ b/hw/faraday.h
>>>
>>>
>>>
>>> None of three function prototyped in this file exists at this point, I think you should add this file later in the patch set.
>>
>> Same issue.
>> I'm studying patchwork now, because I don't want to [act] like a idiot,
>> I'll send out the new patch when I 'though' I'm ready.
>
> The criteria is that every patch must be compilable on its own, we say
> 'bisectable' because it allows to work with git-bisect command for error
> searching. Someone who is debugging an x86 problem might land on an arm
> a36x commit and otherwise not be able to compile.
>
> You can compare my Tegra patchset here (not polished yet):
> http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/tegra
>
> You can add a stub version of your machine and step by step add devices
> to it until it is complete. My Tegra2 SoC modeling with a QOM object is
> not final yet but might serve as design inspiration. I.e., all A36x SoC
> devices should have their state structs either in their own header or in
> a SoC-specific header, so that they can be embedded as fields within a
> SoC state struct - be it within your patchset or as a later step.
>
> 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

Andreas Färber Feb. 1, 2013, 9:47 a.m. UTC | #7

嗨 國榮,

Am 01.02.2013 09:57, schrieb Kuo-Jung Su:
> Thanks for the information, and sorry for the mess I've done.

You don't need to apologize for every review comment you get. It's
meant for improvements of results, not personal. :)

> I'll one-by-one re-send all the patches.
> 
> However because most of my patches are new files,
> should I send-out the patches with detail change log?
> 
> For example:
> 
> [PATCH] dumb timer
> ... [PATCH v2 0/2] dumb timer (Cover letter)
>     [PATCH v2 1/2] dumb timer (The one in Patch V1)
>     [PATCH v2 2/2] dumb timer: coding style update (Change log for V2)
> ...... [PATCH v3 0/2] dumb timer (Cover letter)
>        [PATCH v3 1/2] dumb timer (The merged file in Patch V1 & v2)
>        [PATCH v3 2/2] dumb timer: bug fix (Change log for V3)

No, no, no. What you should do is just something like:

[PATCH v3 0/x] Add Faraday A36x SoC platform support
    [PATCH v3 1/x] arm: Add Faraday A360 and A369 machines
    [PATCH v3 2/x] faraday_a36x: Add FT... timer
    ...

* v3 cover letter contains a change log going back to v1.
* v3 is not a reply to v2 (no --in-reply-to). This aids a threaded mail
display for reviewing and avoids an old version getting reviewed or applied.
* 1+/x are replies to 0/x (usually automatically by git-send-email).
That helps keep the patches together and in the right order.
* Bug fixes of your own code do not go separate (only if you were fixing
existing code from qemu.git). There's no need to introduce bugs and then
to fix them.
* Adding a stub machine in 1/x has the advantage that the patch is much
smaller and easier to review than first adding all devices and then
adding a machine that uses all of them. And each device being added in
(1+n)/x can be tested (system not fully working of course). I.e., the
machine will grow in functionality patch by patch.
* Maybe you can order EHCI last due to the refactoring work involved?

To aid with the requested reordering and squashing of bug fixes into
patches, `git rebase -i` and `git checkout -p` may be of help to you.

Regards,
Andreas

Kuo-Jung Su Feb. 5, 2013, 1:25 a.m. UTC | #8

2013/2/1 Andreas Färber <afaerber@suse.de>:
> 嗨 國榮,
>
> Am 01.02.2013 09:57, schrieb Kuo-Jung Su:
>> Thanks for the information, and sorry for the mess I've done.
>
> You don't need to apologize for every review comment you get. It's
> meant for improvements of results, not personal. :)
>

Thanks. It's so kind of you for helping me building up
the common sense about patch process.

>> I'll one-by-one re-send all the patches.
>>
>> However because most of my patches are new files,
>> should I send-out the patches with detail change log?
>>
>> For example:
>>
>> [PATCH] dumb timer
>> ... [PATCH v2 0/2] dumb timer (Cover letter)
>>     [PATCH v2 1/2] dumb timer (The one in Patch V1)
>>     [PATCH v2 2/2] dumb timer: coding style update (Change log for V2)
>> ...... [PATCH v3 0/2] dumb timer (Cover letter)
>>        [PATCH v3 1/2] dumb timer (The merged file in Patch V1 & v2)
>>        [PATCH v3 2/2] dumb timer: bug fix (Change log for V3)
>
> No, no, no. What you should do is just something like:
>
> [PATCH v3 0/x] Add Faraday A36x SoC platform support
>     [PATCH v3 1/x] arm: Add Faraday A360 and A369 machines
>     [PATCH v3 2/x] faraday_a36x: Add FT... timer
>     ...
>
> * v3 cover letter contains a change log going back to v1.
> * v3 is not a reply to v2 (no --in-reply-to). This aids a threaded mail
> display for reviewing and avoids an old version getting reviewed or applied.
> * 1+/x are replies to 0/x (usually automatically by git-send-email).
> That helps keep the patches together and in the right order.
> * Bug fixes of your own code do not go separate (only if you were fixing
> existing code from qemu.git). There's no need to introduce bugs and then
> to fix them.

Thanks, now I understand what am I supposed to do.

> * Adding a stub machine in 1/x has the advantage that the patch is much
> smaller and easier to review than first adding all devices and then
> adding a machine that uses all of them. And each device being added in
> (1+n)/x can be tested (system not fully working of course). I.e., the
> machine will grow in functionality patch by patch.

Thanks, I'm now re-formating the patch set.

> * Maybe you can order EHCI last due to the refactoring work involved?

I plan to again work on FUSBH200 - EHCI later when the A36x patch set
have been applied.

>
> To aid with the requested reordering and squashing of bug fixes into
> patches, `git rebase -i` and `git checkout -p` may be of help to you.
>

'git rebase' is simply too complicated to me.
I'm now using 'git cherry-pick' and 'git branch' to re-format the patches.

> 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



--
Best wishes,
Kuo-Jung Su

[v2,01/20] arm: add Faraday a36x SoC platform support

Commit Message

Comments

Patch