Re: [Qemu-devel] [PATCH v3 01/20] arm: add Faraday a360 SoC platform support

[Kuo-Jung Su]

2013/2/8 Igor Mitsyanko <address@hidden>:
> On 02/06/2013 01:45 PM, Kuo-Jung Su wrote:
>> From: Kuo-Jung Su<address@hidden>
>>
>> The Faraday A360 EVB is a Faraday SoC platform evaluation board used for
>> Faraday IP functional verification based on the well-known ARM AMBA 2.0
>> architecture.
>>
>> Signed-off-by: Kuo-Jung Su<address@hidden>
>> ---
>>   hw/arm/Makefile.objs      |    1 +
>>   hw/arm/faraday.h          |   52 ++++++++++++++++++++
>>   hw/arm/faraday_a360.c     |  118 
>> +++++++++++++++++++++++++++++++++++++++++++++
>>   hw/arm/faraday_a360_pmu.c |  102 +++++++++++++++++++++++++++++++++++++++
>>   4 files changed, 273 insertions(+)
>>   create mode 100644 hw/arm/faraday.h
>>   create mode 100644 hw/arm/faraday_a360.c
>>   create mode 100644 hw/arm/faraday_a360_pmu.c
>>
>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>> index 6d049e7..59d7023 100644
>> --- a/hw/arm/Makefile.objs
>> +++ b/hw/arm/Makefile.objs
>> @@ -33,3 +33,4 @@ obj-y += kzm.o
>>   obj-$(CONFIG_FDT) += ../device_tree.o
>>
>>   obj-y := $(addprefix ../,$(obj-y))
>> +obj-y += faraday_a360.o faraday_a360_pmu.o
>> diff --git a/hw/arm/faraday.h b/hw/arm/faraday.h
>> new file mode 100644
>> index 0000000..25bb056
>> --- /dev/null
>> +++ b/hw/arm/faraday.h
>> @@ -0,0 +1,52 @@
>> +/*
>> + * Faraday SoC platform support.
>> + *
>> + * Copyright (c) 2013 Faraday Technology
>> + * Written by Kuo-Jung Su<address@hidden>
>> + *
>> + * This code is licensed under the GNU GPL v2.
>> + */
>> +#ifndef HW_ARM_FARADAY_H
>> +#define HW_ARM_FARADAY_H
>> +
>> +#include <hw/flash.h>
>
> Its not a system header, you could just use "" instead of <>.
>

Got it, thanks

>> +
>> +#ifdef DEBUG_FARADAY
>> +#define DPRINTF(fmt, ...) \
>> +    do { printf("faraday: " fmt , ## __VA_ARGS__); } while (0)
>> +#else
>> +#define DPRINTF(fmt, ...) \
>> +    do { } while (0)
>> +#endif
>> +
>> +typedef struct FaradayMachState {
>> +    ARMCPU       *cpu;
>> +    DeviceState  *scu;
>> +    DeviceState  *ahbc;
>> +    DeviceState  *ddrc;
>> +    DeviceState  *hdma[2];  /* AHB DMA */
>> +    DeviceState  *pdma[2];  /* APB DMA */
>> +    i2c_bus      *i2c[2];
>> +
>> +    MemoryRegion *as;
>> +    MemoryRegion *ram;
>> +    MemoryRegion *ram_alias;
>> +    pflash_t     *rom;
>> +    MemoryRegion *sram;
>> +
>> +    uint32_t ahb_slave4;    /* AHB slave 4 default value */
>> +    uint32_t ahb_slave6;    /* AHB slave 6 default value */
>> +    uint32_t ahb_remapped:1;
>> +    uint32_t ddr_inited:1;
>> +} FaradayMachState;
>> +
>> +/* ftintc020.c */
>> +qemu_irq *ftintc020_init(hwaddr base, ARMCPU *cpu);
>> +
>> +/* ftgmac100.c */
>> +void ftgmac100_init(NICInfo *nd, uint32_t base, qemu_irq irq);
>> +
>> +/* ftmac110.c */
>> +void ftmac110_init(NICInfo *nd, uint32_t base, qemu_irq irq);
>
> I understand that its not important, but it doesn't look pretty to have
> declarations without definitions. Maybe it'd be better
> to add these declaration later, together with function implementations?
>
>

Got it, thanks

>> +
>> +#endif
>> diff --git a/hw/arm/faraday_a360.c b/hw/arm/faraday_a360.c
>> new file mode 100644
>> index 0000000..a81b6fb
>> --- /dev/null
>> +++ b/hw/arm/faraday_a360.c
>> @@ -0,0 +1,118 @@
>> +/*
>> + * Faraday A360 Evalution Board
>> + *
>> + * Copyright (c) 2012 Faraday Technology
>> + * Written by Dante Su<address@hidden>
>> + *
>> + * This code is licensed under GNU GPL v2+.
>> + */
>> +
>> +#include <hw/sysbus.h>
>> +#include <hw/arm-misc.h>
>> +#include <hw/devices.h>
>> +#include <hw/i2c.h>
>> +#include <hw/boards.h>
>> +#include <hw/flash.h>
>> +#include <hw/serial.h>
>> +#include <hw/ssi.h>
>> +#include <net/net.h>
>> +#include <sysemu/sysemu.h>
>> +#include <sysemu/blockdev.h>
>> +#include <exec/address-spaces.h>
>> +
>> +#include "faraday.h"
>> +
>> +typedef FaradayMachState    A360State;
>> +
>> +/* Board init. */
>> +static void
>> +a360_device_init(A360State *s)
>> +{
>> +    /* Serial (FTUART010 which is 16550A compatible) */
>> +    if (serial_hds[0]) {
>> +        serial_mm_init(s->as,
>> +                       0x98200000,
>> +                       2,
>> +                       NULL,
>> +                       18432000 / 16,
>> +                       serial_hds[0],
>> +                       DEVICE_LITTLE_ENDIAN);
>> +    }
>> +    if (serial_hds[1]) {
>> +        serial_mm_init(s->as,
>> +                       0x98300000,
>> +                       2,
>> +                       NULL,
>> +                       18432000 / 16,
>> +                       serial_hds[1],
>> +                       DEVICE_LITTLE_ENDIAN);
>> +    }
>> +
>> +    /* pmu */
>> +    sysbus_create_simple("a360.pmu", 0x98100000, NULL);
>> +}
>> +
>> +static void
>> +a360_board_init(QEMUMachineInitArgs *args)
>> +{
>> +    struct arm_boot_info *bi = NULL;
>> +    A360State *s = g_new(A360State, 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) {
>> +        args->cpu_model = "arm926";
>> +        s->cpu = cpu_arm_init(args->cpu_model);
>> +        if (!s->cpu) {
>> +            hw_error("a360: Unable to find CPU definition\n");
>> +            exit(1);
>> +        }
>> +    }
>> +
>> +    /* A360 supports upto 1GB ram space */
>> +    if (args->ram_size > 0x40000000) {
>> +        args->ram_size = 0x40000000;
>> +    }
>> +
>> +    /* RAM Init */
>> +    memory_region_init_ram(s->ram, "a360.ram", args->ram_size);
>> +    vmstate_register_ram_global(s->ram);
>> +
>> +    a360_device_init(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,
>> +    DEFAULT_MACHINE_OPTIONS,
>> +};
>> +
>> +static void
>> +a360_machine_init(void)
>> +{
>> +    qemu_register_machine(&a360_machine);
>> +}
>> +
>> +machine_init(a360_machine_init);
>> diff --git a/hw/arm/faraday_a360_pmu.c b/hw/arm/faraday_a360_pmu.c
>> new file mode 100644
>> index 0000000..dc8b749
>> --- /dev/null
>> +++ b/hw/arm/faraday_a360_pmu.c
>> @@ -0,0 +1,102 @@
>> +/*
>> + * Faraday A360 PMU
>> + *
>> + * Copyright (c) 2012 Faraday Technology
>> + * Written by Dante Su<address@hidden>
>> + *
>> + * This code is licensed under GNU GPL v2+
>> + */
>> +
>> +#include <hw/hw.h>
>> +#include <hw/sysbus.h>
>> +#include <hw/devices.h>
>> +#include <ui/console.h>
>> +#include <qemu/timer.h>
>> +#include <sysemu/sysemu.h>
>> +
>> +#include "faraday.h"
>> +
>> +#define REG_PDLLCR      0x30   /* PLL/DLL control register */
>> +
>> +#define TYPE_A360PMU    "a360.pmu"
>> +
>> +typedef struct A360PMUState {
>> +    SysBusDevice busdev;
>> +    MemoryRegion iomem;
>> +} A360PMUState;
>> +
>> +#define A360PMU(obj) \
>> +    OBJECT_CHECK(A360PMUState, obj, TYPE_A360PMU)
>> +
>> +static uint64_t
>> +a360pmu_mem_read(void *opaque, hwaddr addr, unsigned size)
>> +{
>> +    uint64_t ret = 0;
>> +
>> +    switch (addr) {
>> +    case REG_PDLLCR:
>> +        ret = 27 << 3;  /* PLL = 27 */
>> +        break;
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +static void
>> +a360pmu_mem_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
>> +{
>> +}
>> +
>
> So, how does this memory behave on real hardware? Is it actually read as
> zero (RZ/WI) at every offset except
> at clock distribution control register? If not, I think you should model
> it accordingly, by allocating a chunk of memory
> 0x1000 bytes length.
>
>

Yes, I've cheated here, I only add something those are necessary to my firmware.
I'll finish the missing parts later.

>> +static const MemoryRegionOps a360pmu_mem_ops = {
>> +    .read  = a360pmu_mem_read,
>> +    .write = a360pmu_mem_write,
>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>> +};
>> +
>> +static int a360pmu_init(SysBusDevice *dev)
>> +{
>> +    A360PMUState *s = A360PMU(FROM_SYSBUS(A360PMUState, dev));
>
> FROM_SYSBUS seems unnecessary here, you're casting to (Object *) in A360PMU
>   anyway.
>
>

Got it, thanks

>> +
>> +    memory_region_init_io(&s->iomem,
>> +                          &a360pmu_mem_ops,
>> +                          s,
>> +                          TYPE_A360PMU,
>> +                          0x1000);
>> +    sysbus_init_mmio(dev, &s->iomem);
>> +    return 0;
>> +}
>> +
>> +static const VMStateDescription vmstate_a360pmu = {
>> +    .name = TYPE_A360PMU,
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .minimum_version_id_old = 1,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_END_OF_LIST(),
>> +    }
>> +};
>> +
>> +static void a360pmu_class_init(ObjectClass *klass, void *data)
>> +{
>> +    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +    k->init   = a360pmu_init;
>> +    dc->desc  = TYPE_A360PMU;
>> +    dc->vmsd  = &vmstate_a360pmu;
>> +    dc->no_user = 1;
>> +}
>> +
>> +static const TypeInfo a360pmu_info = {
>> +    .name          = TYPE_A360PMU,
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(A360PMUState),
>> +    .class_init    = a360pmu_class_init,
>> +};
>> +
>> +static void a360pmu_register_types(void)
>> +{
>> +    type_register_static(&a360pmu_info);
>> +}
>> +
>> +type_init(a360pmu_register_types)



-- 
Best wishes,
Kuo-Jung Su