Patchwork [v9,05/24] hw/arm: add FTDDRII030 DDRII controller support

login
register
mail settings
Submitter Kuo-Jung Su
Date March 25, 2013, 12:09 p.m.
Message ID <1364213400-10266-6-git-send-email-dantesu@gmail.com>
Download mbox | patch
Permalink /patch/230688/
State New
Headers show

Comments

Kuo-Jung Su - March 25, 2013, 12:09 p.m.
From: Kuo-Jung Su <dantesu@faraday-tech.com>

The FTDDRII030 is a DDRII SDRAM controller which is responsible for
SDRAM initialization.

Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
---
 hw/arm/Makefile.objs    |    2 +-
 hw/arm/ftplat_a369soc.c |    8 ++
 hw/ftddrii030.c         |  192 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 201 insertions(+), 1 deletion(-)
 create mode 100644 hw/ftddrii030.c
Peter Crosthwaite - March 28, 2013, 12:09 a.m.
Hi Kuo-Jung,

On Mon, Mar 25, 2013 at 10:09 PM, Kuo-Jung Su <dantesu@gmail.com> wrote:
> From: Kuo-Jung Su <dantesu@faraday-tech.com>
>
> The FTDDRII030 is a DDRII SDRAM controller which is responsible for
> SDRAM initialization.
>
> Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
> ---
>  hw/arm/Makefile.objs    |    2 +-
>  hw/arm/ftplat_a369soc.c |    8 ++
>  hw/ftddrii030.c         |  192 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 201 insertions(+), 1 deletion(-)
>  create mode 100644 hw/ftddrii030.c
>
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index b2fa20f..e774962 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -24,7 +24,7 @@ obj-y += framebuffer.o
>  obj-y += strongarm.o
>  obj-y += imx_serial.o imx_ccm.o imx_timer.o imx_avic.o
>  obj-$(CONFIG_KVM) += kvm/arm_gic.o
> -obj-y += ftintc020.o ftahbc020.o
> +obj-y += ftintc020.o ftahbc020.o ftddrii030.o
>
>  obj-y := $(addprefix ../,$(obj-y))
>
> diff --git a/hw/arm/ftplat_a369soc.c b/hw/arm/ftplat_a369soc.c
> index 7f222cb..b2da582 100644
> --- a/hw/arm/ftplat_a369soc.c
> +++ b/hw/arm/ftplat_a369soc.c
> @@ -129,6 +129,14 @@ static void a369soc_chip_init(FaradaySoCState *s)
>          fprintf(stderr, "a369soc: Unable to set soc link for FTAHBC020\n");
>          abort();
>      }
> +
> +    /* ftddrii030 */
> +    ds = sysbus_create_simple("ftddrii030", 0x93100000, NULL);
> +    object_property_set_link(OBJECT(ds), OBJECT(s), "soc", &local_errp);
> +    if (local_errp) {
> +        fprintf(stderr, "a369soc: Unable to set soc link for FTDDRII030\n");
> +        abort();
> +    }
>  }
>
>  static void a369soc_realize(DeviceState *dev, Error **errp)
> diff --git a/hw/ftddrii030.c b/hw/ftddrii030.c
> new file mode 100644
> index 0000000..158db1f
> --- /dev/null
> +++ b/hw/ftddrii030.c
> @@ -0,0 +1,192 @@
> +/*
> + * Faraday DDRII controller
> + *
> + * Copyright (c) 2012 Faraday Technology
> + * Written by Dante Su <dantesu@faraday-tech.com>
> + *
> + * This code is licensed under GNU GPL v2+
> + */
> +
> +#include "hw/hw.h"
> +#include "hw/sysbus.h"
> +#include "hw/devices.h"
> +#include "sysemu/sysemu.h"
> +
> +#include "hw/faraday.h"
> +
> +#define REG_MCR             0x00    /* memory configuration register */
> +#define REG_MSR             0x04    /* memory status register */
> +#define REG_REVR            0x50    /* revision register */
> +
> +#define MSR_INIT_OK         BIT(8)  /* initialization finished */
> +#define MSR_CMD_MRS         BIT(0)  /* start MRS command (init. seq.) */
> +
> +#define CFG_REGSIZE         (REG_REVR / 4)
> +
> +#define TYPE_FTDDRII030     "ftddrii030"
> +
> +typedef struct Ftddrii030State {
> +    /*< private >*/
> +    SysBusDevice parent;
> +
> +    /*< public >*/
> +    MemoryRegion iomem;
> +
> +    FaradaySoCState *soc;
> +

This new implementation still has many of the same encapsulation
issues discussed in v8. If you go with the suggestion myself and Peter
came up with you could make this device completely self contained - it
should have no awareness that it is part of the Faraday SoC. If you
model as a second MemoryRegion you can push all the Faraday specific
foo up to the SoC. Interdiff would go something like this:

-    FaradaySoCState *soc;
+    MemoryRegion ram;

> +    /* HW register cache */
> +    uint32_t regs[CFG_REGSIZE];
> +} Ftddrii030State;
> +
> +#define FTDDRII030(obj) \
> +    OBJECT_CHECK(Ftddrii030State, obj, TYPE_FTDDRII030)
> +
> +#define DDR_REG32(s, off) \
> +    ((s)->regs[(off) / 4])
> +
> +static uint64_t
> +ftddrii030_mem_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +    Ftddrii030State *s = FTDDRII030(opaque);
> +    FaradaySoCState *soc = s->soc;
> +    uint64_t ret = 0;
> +
> +    if (soc->ram_visible) {
> +        DDR_REG32(s, REG_MSR) |= MSR_INIT_OK;
> +    } else {
> +        DDR_REG32(s, REG_MSR) &= ~MSR_INIT_OK;
> +    }
> +
> +    switch (addr) {
> +    case REG_MCR ... REG_REVR - 4:
> +        ret = DDR_REG32(s, addr);
> +        break;
> +    case REG_REVR:
> +        ret = 0x100;    /* rev. = 0.1.0 */
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +            "ftddrii030: undefined memory access@%#" HWADDR_PRIx "\n", addr);
> +        break;
> +    }
> +
> +    return ret;
> +}
> +
> +static void
> +ftddrii030_mem_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
> +{
> +    Ftddrii030State *s = FTDDRII030(opaque);
> +    FaradaySoCState *soc = s->soc;
> +
> +    switch (addr) {
> +    case REG_MCR:
> +        DDR_REG32(s, addr) = (uint32_t)val & 0xffff;
> +        break;
> +    case REG_MSR:
> +        if (!soc->ram_visible && (val & MSR_CMD_MRS)) {
> +            val &= ~MSR_CMD_MRS;
> +            faraday_soc_ram_setup(soc, true);

This function (added earlier in this series) then becomes local to
this device model and the memory_region_add_subregion() happens with
the new ram Memory region instead. AFAICT that function only requires
the ram size which strikes me as something that should be a QOM
property to the device.

> +        }
> +        DDR_REG32(s, addr) = (uint32_t)val;
> +        break;
> +    /* SDRAM Timing, ECC ...etc. */
> +    case REG_MSR + 4 ... REG_REVR - 4:
> +        DDR_REG32(s, addr) = (uint32_t)val;
> +        break;
> +    case REG_REVR:
> +        break;
> +    default:
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +            "ftddrii030: undefined memory access@%#" HWADDR_PRIx "\n", addr);
> +        break;
> +    }
> +}
> +
> +static const MemoryRegionOps mmio_ops = {
> +    .read  = ftddrii030_mem_read,
> +    .write = ftddrii030_mem_write,
> +    .endianness = DEVICE_LITTLE_ENDIAN,
> +    .valid = {
> +        .min_access_size = 4,
> +        .max_access_size = 4,
> +    }
> +};
> +
> +static void ftddrii030_reset(DeviceState *ds)
> +{
> +    Ftddrii030State *s = FTDDRII030(SYS_BUS_DEVICE(ds));
> +    Error *local_errp = NULL;
> +
> +    s->soc = FARADAY_SOC(object_property_get_link(OBJECT(s),
> +                                                  "soc",
> +                                                  &local_errp));
> +    if (local_errp) {
> +        fprintf(stderr, "ftahbc020: Unable to get soc link\n");
> +        abort();
> +    }
> +
> +    faraday_soc_ram_setup(s->soc, false);
> +    memset(s->regs, 0, sizeof(s->regs));
> +}
> +
> +static void ftddrii030_realize(DeviceState *dev, Error **errp)
> +{
> +    Ftddrii030State *s = FTDDRII030(dev);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +
> +    memory_region_init_io(&s->iomem,
> +                          &mmio_ops,
> +                          s,
> +                          TYPE_FTDDRII030,
> +                          0x1000);
> +    sysbus_init_mmio(sbd, &s->iomem);

Init the new ram memory region here.

I could use a second opinion on all this QOM stuff though - you are on
the bleeding edge with the QOM SoC stuff. I suggest giving it some
list time for others to reply before a remake.

Regards,
Peter

> +}
> +
> +static const VMStateDescription vmstate_ftddrii030 = {
> +    .name = TYPE_FTDDRII030,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32_ARRAY(regs, Ftddrii030State, CFG_REGSIZE),
> +        VMSTATE_END_OF_LIST(),
> +    }
> +};
> +
> +static void ftddrii030_instance_init(Object *obj)
> +{
> +    Ftddrii030State *s = FTDDRII030(obj);
> +
> +    object_property_add_link(obj,
> +                             "soc",
> +                             TYPE_FARADAY_SOC,
> +                             (Object **) &s->soc,
> +                             NULL);
> +}
> +
> +static void ftddrii030_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->desc  = TYPE_FTDDRII030;
> +    dc->vmsd  = &vmstate_ftddrii030;
> +    dc->reset = ftddrii030_reset;
> +    dc->realize = ftddrii030_realize;
> +    dc->no_user = 1;
> +}
> +
> +static const TypeInfo ftddrii030_info = {
> +    .name          = TYPE_FTDDRII030,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(Ftddrii030State),
> +    .instance_init = ftddrii030_instance_init,
> +    .class_init    = ftddrii030_class_init,
> +};
> +
> +static void ftddrii030_register_types(void)
> +{
> +    type_register_static(&ftddrii030_info);
> +}
> +
> +type_init(ftddrii030_register_types)
> --
> 1.7.9.5
>
>
Kuo-Jung Su - March 28, 2013, 3:24 a.m.
2013/3/28 Peter Crosthwaite <peter.crosthwaite@xilinx.com>:
> Hi Kuo-Jung,
>
> On Mon, Mar 25, 2013 at 10:09 PM, Kuo-Jung Su <dantesu@gmail.com> wrote:
>> From: Kuo-Jung Su <dantesu@faraday-tech.com>
>>
>> The FTDDRII030 is a DDRII SDRAM controller which is responsible for
>> SDRAM initialization.
>>
>> Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
>> ---
>>  hw/arm/Makefile.objs    |    2 +-
>>  hw/arm/ftplat_a369soc.c |    8 ++
>>  hw/ftddrii030.c         |  192 +++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 201 insertions(+), 1 deletion(-)
>>  create mode 100644 hw/ftddrii030.c
>>
>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>> index b2fa20f..e774962 100644
>> --- a/hw/arm/Makefile.objs
>> +++ b/hw/arm/Makefile.objs
>> @@ -24,7 +24,7 @@ obj-y += framebuffer.o
>>  obj-y += strongarm.o
>>  obj-y += imx_serial.o imx_ccm.o imx_timer.o imx_avic.o
>>  obj-$(CONFIG_KVM) += kvm/arm_gic.o
>> -obj-y += ftintc020.o ftahbc020.o
>> +obj-y += ftintc020.o ftahbc020.o ftddrii030.o
>>
>>  obj-y := $(addprefix ../,$(obj-y))
>>
>> diff --git a/hw/arm/ftplat_a369soc.c b/hw/arm/ftplat_a369soc.c
>> index 7f222cb..b2da582 100644
>> --- a/hw/arm/ftplat_a369soc.c
>> +++ b/hw/arm/ftplat_a369soc.c
>> @@ -129,6 +129,14 @@ static void a369soc_chip_init(FaradaySoCState *s)
>>          fprintf(stderr, "a369soc: Unable to set soc link for FTAHBC020\n");
>>          abort();
>>      }
>> +
>> +    /* ftddrii030 */
>> +    ds = sysbus_create_simple("ftddrii030", 0x93100000, NULL);
>> +    object_property_set_link(OBJECT(ds), OBJECT(s), "soc", &local_errp);
>> +    if (local_errp) {
>> +        fprintf(stderr, "a369soc: Unable to set soc link for FTDDRII030\n");
>> +        abort();
>> +    }
>>  }
>>
>>  static void a369soc_realize(DeviceState *dev, Error **errp)
>> diff --git a/hw/ftddrii030.c b/hw/ftddrii030.c
>> new file mode 100644
>> index 0000000..158db1f
>> --- /dev/null
>> +++ b/hw/ftddrii030.c
>> @@ -0,0 +1,192 @@
>> +/*
>> + * Faraday DDRII controller
>> + *
>> + * Copyright (c) 2012 Faraday Technology
>> + * Written by Dante Su <dantesu@faraday-tech.com>
>> + *
>> + * This code is licensed under GNU GPL v2+
>> + */
>> +
>> +#include "hw/hw.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/devices.h"
>> +#include "sysemu/sysemu.h"
>> +
>> +#include "hw/faraday.h"
>> +
>> +#define REG_MCR             0x00    /* memory configuration register */
>> +#define REG_MSR             0x04    /* memory status register */
>> +#define REG_REVR            0x50    /* revision register */
>> +
>> +#define MSR_INIT_OK         BIT(8)  /* initialization finished */
>> +#define MSR_CMD_MRS         BIT(0)  /* start MRS command (init. seq.) */
>> +
>> +#define CFG_REGSIZE         (REG_REVR / 4)
>> +
>> +#define TYPE_FTDDRII030     "ftddrii030"
>> +
>> +typedef struct Ftddrii030State {
>> +    /*< private >*/
>> +    SysBusDevice parent;
>> +
>> +    /*< public >*/
>> +    MemoryRegion iomem;
>> +
>> +    FaradaySoCState *soc;
>> +
>
> This new implementation still has many of the same encapsulation
> issues discussed in v8. If you go with the suggestion myself and Peter
> came up with you could make this device completely self contained - it
> should have no awareness that it is part of the Faraday SoC. If you
> model as a second MemoryRegion you can push all the Faraday specific
> foo up to the SoC. Interdiff would go something like this:
>
> -    FaradaySoCState *soc;
> +    MemoryRegion ram;
>

Sorry, but I think it might not work in that way.
Because there is an AHB remap function is Faraday SoC,
which would alter the base address of both ROM and RAM.
So the ram instance should never be a local variable to FTDDRII030 only.

The first thing solution spring up in my mind is to add a QOM link in
FTDDRII030, and then pass the ram instance as a QOM link from device model.
However it turns out that it's also not possible, since the ram
instance (MemoryRegion) is not a Object *.

So I turn out to implement both the 'AHB remap' and 'RAM
initialization' as common routines
to Faraday SoC, and also create a remappable memory region for both ROM and RAM.

>> +    /* HW register cache */
>> +    uint32_t regs[CFG_REGSIZE];
>> +} Ftddrii030State;
>> +
>> +#define FTDDRII030(obj) \
>> +    OBJECT_CHECK(Ftddrii030State, obj, TYPE_FTDDRII030)
>> +
>> +#define DDR_REG32(s, off) \
>> +    ((s)->regs[(off) / 4])
>> +
>> +static uint64_t
>> +ftddrii030_mem_read(void *opaque, hwaddr addr, unsigned size)
>> +{
>> +    Ftddrii030State *s = FTDDRII030(opaque);
>> +    FaradaySoCState *soc = s->soc;
>> +    uint64_t ret = 0;
>> +
>> +    if (soc->ram_visible) {
>> +        DDR_REG32(s, REG_MSR) |= MSR_INIT_OK;
>> +    } else {
>> +        DDR_REG32(s, REG_MSR) &= ~MSR_INIT_OK;
>> +    }
>> +
>> +    switch (addr) {
>> +    case REG_MCR ... REG_REVR - 4:
>> +        ret = DDR_REG32(s, addr);
>> +        break;
>> +    case REG_REVR:
>> +        ret = 0x100;    /* rev. = 0.1.0 */
>> +        break;
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +            "ftddrii030: undefined memory access@%#" HWADDR_PRIx "\n", addr);
>> +        break;
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +static void
>> +ftddrii030_mem_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
>> +{
>> +    Ftddrii030State *s = FTDDRII030(opaque);
>> +    FaradaySoCState *soc = s->soc;
>> +
>> +    switch (addr) {
>> +    case REG_MCR:
>> +        DDR_REG32(s, addr) = (uint32_t)val & 0xffff;
>> +        break;
>> +    case REG_MSR:
>> +        if (!soc->ram_visible && (val & MSR_CMD_MRS)) {
>> +            val &= ~MSR_CMD_MRS;
>> +            faraday_soc_ram_setup(soc, true);
>
> This function (added earlier in this series) then becomes local to
> this device model and the memory_region_add_subregion() happens with
> the new ram Memory region instead. AFAICT that function only requires
> the ram size which strikes me as something that should be a QOM
> property to the device.
>

Please see the comments above.

>> +        }
>> +        DDR_REG32(s, addr) = (uint32_t)val;
>> +        break;
>> +    /* SDRAM Timing, ECC ...etc. */
>> +    case REG_MSR + 4 ... REG_REVR - 4:
>> +        DDR_REG32(s, addr) = (uint32_t)val;
>> +        break;
>> +    case REG_REVR:
>> +        break;
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +            "ftddrii030: undefined memory access@%#" HWADDR_PRIx "\n", addr);
>> +        break;
>> +    }
>> +}
>> +
>> +static const MemoryRegionOps mmio_ops = {
>> +    .read  = ftddrii030_mem_read,
>> +    .write = ftddrii030_mem_write,
>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>> +    .valid = {
>> +        .min_access_size = 4,
>> +        .max_access_size = 4,
>> +    }
>> +};
>> +
>> +static void ftddrii030_reset(DeviceState *ds)
>> +{
>> +    Ftddrii030State *s = FTDDRII030(SYS_BUS_DEVICE(ds));
>> +    Error *local_errp = NULL;
>> +
>> +    s->soc = FARADAY_SOC(object_property_get_link(OBJECT(s),
>> +                                                  "soc",
>> +                                                  &local_errp));
>> +    if (local_errp) {
>> +        fprintf(stderr, "ftahbc020: Unable to get soc link\n");
>> +        abort();
>> +    }
>> +
>> +    faraday_soc_ram_setup(s->soc, false);
>> +    memset(s->regs, 0, sizeof(s->regs));
>> +}
>> +
>> +static void ftddrii030_realize(DeviceState *dev, Error **errp)
>> +{
>> +    Ftddrii030State *s = FTDDRII030(dev);
>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>> +
>> +    memory_region_init_io(&s->iomem,
>> +                          &mmio_ops,
>> +                          s,
>> +                          TYPE_FTDDRII030,
>> +                          0x1000);
>> +    sysbus_init_mmio(sbd, &s->iomem);
>
> Init the new ram memory region here.
>

Please see the comments above.

> I could use a second opinion on all this QOM stuff though - you are on
> the bleeding edge with the QOM SoC stuff. I suggest giving it some
> list time for others to reply before a remake.
>

Got it, thanks.

> Regards,
> Peter
>
>> +}
>> +
>> +static const VMStateDescription vmstate_ftddrii030 = {
>> +    .name = TYPE_FTDDRII030,
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .minimum_version_id_old = 1,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT32_ARRAY(regs, Ftddrii030State, CFG_REGSIZE),
>> +        VMSTATE_END_OF_LIST(),
>> +    }
>> +};
>> +
>> +static void ftddrii030_instance_init(Object *obj)
>> +{
>> +    Ftddrii030State *s = FTDDRII030(obj);
>> +
>> +    object_property_add_link(obj,
>> +                             "soc",
>> +                             TYPE_FARADAY_SOC,
>> +                             (Object **) &s->soc,
>> +                             NULL);
>> +}
>> +
>> +static void ftddrii030_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +    dc->desc  = TYPE_FTDDRII030;
>> +    dc->vmsd  = &vmstate_ftddrii030;
>> +    dc->reset = ftddrii030_reset;
>> +    dc->realize = ftddrii030_realize;
>> +    dc->no_user = 1;
>> +}
>> +
>> +static const TypeInfo ftddrii030_info = {
>> +    .name          = TYPE_FTDDRII030,
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(Ftddrii030State),
>> +    .instance_init = ftddrii030_instance_init,
>> +    .class_init    = ftddrii030_class_init,
>> +};
>> +
>> +static void ftddrii030_register_types(void)
>> +{
>> +    type_register_static(&ftddrii030_info);
>> +}
>> +
>> +type_init(ftddrii030_register_types)
>> --
>> 1.7.9.5
>>
>>



--
Best wishes,
Kuo-Jung Su
Peter Crosthwaite - March 28, 2013, 3:58 a.m.
Hi Kuo Jung,

On Thu, Mar 28, 2013 at 1:24 PM, Kuo-Jung Su <dantesu@gmail.com> wrote:
> 2013/3/28 Peter Crosthwaite <peter.crosthwaite@xilinx.com>:
>> Hi Kuo-Jung,
>>
>> On Mon, Mar 25, 2013 at 10:09 PM, Kuo-Jung Su <dantesu@gmail.com> wrote:
>>> From: Kuo-Jung Su <dantesu@faraday-tech.com>
>>>
>>> The FTDDRII030 is a DDRII SDRAM controller which is responsible for
>>> SDRAM initialization.
>>>
>>> Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
>>> ---
>>>  hw/arm/Makefile.objs    |    2 +-
>>>  hw/arm/ftplat_a369soc.c |    8 ++
>>>  hw/ftddrii030.c         |  192 +++++++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 201 insertions(+), 1 deletion(-)
>>>  create mode 100644 hw/ftddrii030.c
>>>
>>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>>> index b2fa20f..e774962 100644
>>> --- a/hw/arm/Makefile.objs
>>> +++ b/hw/arm/Makefile.objs
>>> @@ -24,7 +24,7 @@ obj-y += framebuffer.o
>>>  obj-y += strongarm.o
>>>  obj-y += imx_serial.o imx_ccm.o imx_timer.o imx_avic.o
>>>  obj-$(CONFIG_KVM) += kvm/arm_gic.o
>>> -obj-y += ftintc020.o ftahbc020.o
>>> +obj-y += ftintc020.o ftahbc020.o ftddrii030.o
>>>
>>>  obj-y := $(addprefix ../,$(obj-y))
>>>
>>> diff --git a/hw/arm/ftplat_a369soc.c b/hw/arm/ftplat_a369soc.c
>>> index 7f222cb..b2da582 100644
>>> --- a/hw/arm/ftplat_a369soc.c
>>> +++ b/hw/arm/ftplat_a369soc.c
>>> @@ -129,6 +129,14 @@ static void a369soc_chip_init(FaradaySoCState *s)
>>>          fprintf(stderr, "a369soc: Unable to set soc link for FTAHBC020\n");
>>>          abort();
>>>      }
>>> +
>>> +    /* ftddrii030 */
>>> +    ds = sysbus_create_simple("ftddrii030", 0x93100000, NULL);
>>> +    object_property_set_link(OBJECT(ds), OBJECT(s), "soc", &local_errp);
>>> +    if (local_errp) {
>>> +        fprintf(stderr, "a369soc: Unable to set soc link for FTDDRII030\n");
>>> +        abort();
>>> +    }
>>>  }
>>>
>>>  static void a369soc_realize(DeviceState *dev, Error **errp)
>>> diff --git a/hw/ftddrii030.c b/hw/ftddrii030.c
>>> new file mode 100644
>>> index 0000000..158db1f
>>> --- /dev/null
>>> +++ b/hw/ftddrii030.c
>>> @@ -0,0 +1,192 @@
>>> +/*
>>> + * Faraday DDRII controller
>>> + *
>>> + * Copyright (c) 2012 Faraday Technology
>>> + * Written by Dante Su <dantesu@faraday-tech.com>
>>> + *
>>> + * This code is licensed under GNU GPL v2+
>>> + */
>>> +
>>> +#include "hw/hw.h"
>>> +#include "hw/sysbus.h"
>>> +#include "hw/devices.h"
>>> +#include "sysemu/sysemu.h"
>>> +
>>> +#include "hw/faraday.h"
>>> +
>>> +#define REG_MCR             0x00    /* memory configuration register */
>>> +#define REG_MSR             0x04    /* memory status register */
>>> +#define REG_REVR            0x50    /* revision register */
>>> +
>>> +#define MSR_INIT_OK         BIT(8)  /* initialization finished */
>>> +#define MSR_CMD_MRS         BIT(0)  /* start MRS command (init. seq.) */
>>> +
>>> +#define CFG_REGSIZE         (REG_REVR / 4)
>>> +
>>> +#define TYPE_FTDDRII030     "ftddrii030"
>>> +
>>> +typedef struct Ftddrii030State {
>>> +    /*< private >*/
>>> +    SysBusDevice parent;
>>> +
>>> +    /*< public >*/
>>> +    MemoryRegion iomem;
>>> +
>>> +    FaradaySoCState *soc;
>>> +
>>
>> This new implementation still has many of the same encapsulation
>> issues discussed in v8. If you go with the suggestion myself and Peter
>> came up with you could make this device completely self contained - it
>> should have no awareness that it is part of the Faraday SoC. If you
>> model as a second MemoryRegion you can push all the Faraday specific
>> foo up to the SoC. Interdiff would go something like this:
>>
>> -    FaradaySoCState *soc;
>> +    MemoryRegion ram;
>>
>
> Sorry, but I think it might not work in that way.
> Because there is an AHB remap function is Faraday SoC,
> which would alter the base address of both ROM and RAM.
> So the ram instance should never be a local variable to FTDDRII030 only.
>

The new RAM region would have no awareness of its base address. It is
just a remappable container. The SoC can still do the remapping part.
This device model is then oblivous to the fact that the SoC and other
devices are remapping its base address.

> The first thing solution spring up in my mind is to add a QOM link in
> FTDDRII030, and then pass the ram instance as a QOM link from device model.
> However it turns out that it's also not possible, since the ram
> instance (MemoryRegion) is not a Object *.
>

calling sysbus_mmio_get_region() on the SoC level should do the trick.
If you init the container region as proposed, this function can be
used by the SoC to get a handle to it.

I think we are on similar timezones (+10:00 here) if you want to try
and clarify on the IRC sometime today as well.

Regards,
Peter

> So I turn out to implement both the 'AHB remap' and 'RAM
> initialization' as common routines
> to Faraday SoC, and also create a remappable memory region for both ROM and RAM.
>
>>> +    /* HW register cache */
>>> +    uint32_t regs[CFG_REGSIZE];
>>> +} Ftddrii030State;
>>> +
>>> +#define FTDDRII030(obj) \
>>> +    OBJECT_CHECK(Ftddrii030State, obj, TYPE_FTDDRII030)
>>> +
>>> +#define DDR_REG32(s, off) \
>>> +    ((s)->regs[(off) / 4])
>>> +
>>> +static uint64_t
>>> +ftddrii030_mem_read(void *opaque, hwaddr addr, unsigned size)
>>> +{
>>> +    Ftddrii030State *s = FTDDRII030(opaque);
>>> +    FaradaySoCState *soc = s->soc;
>>> +    uint64_t ret = 0;
>>> +
>>> +    if (soc->ram_visible) {
>>> +        DDR_REG32(s, REG_MSR) |= MSR_INIT_OK;
>>> +    } else {
>>> +        DDR_REG32(s, REG_MSR) &= ~MSR_INIT_OK;
>>> +    }
>>> +
>>> +    switch (addr) {
>>> +    case REG_MCR ... REG_REVR - 4:
>>> +        ret = DDR_REG32(s, addr);
>>> +        break;
>>> +    case REG_REVR:
>>> +        ret = 0x100;    /* rev. = 0.1.0 */
>>> +        break;
>>> +    default:
>>> +        qemu_log_mask(LOG_GUEST_ERROR,
>>> +            "ftddrii030: undefined memory access@%#" HWADDR_PRIx "\n", addr);
>>> +        break;
>>> +    }
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +static void
>>> +ftddrii030_mem_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
>>> +{
>>> +    Ftddrii030State *s = FTDDRII030(opaque);
>>> +    FaradaySoCState *soc = s->soc;
>>> +
>>> +    switch (addr) {
>>> +    case REG_MCR:
>>> +        DDR_REG32(s, addr) = (uint32_t)val & 0xffff;
>>> +        break;
>>> +    case REG_MSR:
>>> +        if (!soc->ram_visible && (val & MSR_CMD_MRS)) {
>>> +            val &= ~MSR_CMD_MRS;
>>> +            faraday_soc_ram_setup(soc, true);
>>
>> This function (added earlier in this series) then becomes local to
>> this device model and the memory_region_add_subregion() happens with
>> the new ram Memory region instead. AFAICT that function only requires
>> the ram size which strikes me as something that should be a QOM
>> property to the device.
>>
>
> Please see the comments above.
>
>>> +        }
>>> +        DDR_REG32(s, addr) = (uint32_t)val;
>>> +        break;
>>> +    /* SDRAM Timing, ECC ...etc. */
>>> +    case REG_MSR + 4 ... REG_REVR - 4:
>>> +        DDR_REG32(s, addr) = (uint32_t)val;
>>> +        break;
>>> +    case REG_REVR:
>>> +        break;
>>> +    default:
>>> +        qemu_log_mask(LOG_GUEST_ERROR,
>>> +            "ftddrii030: undefined memory access@%#" HWADDR_PRIx "\n", addr);
>>> +        break;
>>> +    }
>>> +}
>>> +
>>> +static const MemoryRegionOps mmio_ops = {
>>> +    .read  = ftddrii030_mem_read,
>>> +    .write = ftddrii030_mem_write,
>>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>>> +    .valid = {
>>> +        .min_access_size = 4,
>>> +        .max_access_size = 4,
>>> +    }
>>> +};
>>> +
>>> +static void ftddrii030_reset(DeviceState *ds)
>>> +{
>>> +    Ftddrii030State *s = FTDDRII030(SYS_BUS_DEVICE(ds));
>>> +    Error *local_errp = NULL;
>>> +
>>> +    s->soc = FARADAY_SOC(object_property_get_link(OBJECT(s),
>>> +                                                  "soc",
>>> +                                                  &local_errp));
>>> +    if (local_errp) {
>>> +        fprintf(stderr, "ftahbc020: Unable to get soc link\n");
>>> +        abort();
>>> +    }
>>> +
>>> +    faraday_soc_ram_setup(s->soc, false);
>>> +    memset(s->regs, 0, sizeof(s->regs));
>>> +}
>>> +
>>> +static void ftddrii030_realize(DeviceState *dev, Error **errp)
>>> +{
>>> +    Ftddrii030State *s = FTDDRII030(dev);
>>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>> +
>>> +    memory_region_init_io(&s->iomem,
>>> +                          &mmio_ops,
>>> +                          s,
>>> +                          TYPE_FTDDRII030,
>>> +                          0x1000);
>>> +    sysbus_init_mmio(sbd, &s->iomem);
>>
>> Init the new ram memory region here.
>>
>
> Please see the comments above.
>
>> I could use a second opinion on all this QOM stuff though - you are on
>> the bleeding edge with the QOM SoC stuff. I suggest giving it some
>> list time for others to reply before a remake.
>>
>
> Got it, thanks.
>
>> Regards,
>> Peter
>>
>>> +}
>>> +
>>> +static const VMStateDescription vmstate_ftddrii030 = {
>>> +    .name = TYPE_FTDDRII030,
>>> +    .version_id = 1,
>>> +    .minimum_version_id = 1,
>>> +    .minimum_version_id_old = 1,
>>> +    .fields = (VMStateField[]) {
>>> +        VMSTATE_UINT32_ARRAY(regs, Ftddrii030State, CFG_REGSIZE),
>>> +        VMSTATE_END_OF_LIST(),
>>> +    }
>>> +};
>>> +
>>> +static void ftddrii030_instance_init(Object *obj)
>>> +{
>>> +    Ftddrii030State *s = FTDDRII030(obj);
>>> +
>>> +    object_property_add_link(obj,
>>> +                             "soc",
>>> +                             TYPE_FARADAY_SOC,
>>> +                             (Object **) &s->soc,
>>> +                             NULL);
>>> +}
>>> +
>>> +static void ftddrii030_class_init(ObjectClass *klass, void *data)
>>> +{
>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>> +
>>> +    dc->desc  = TYPE_FTDDRII030;
>>> +    dc->vmsd  = &vmstate_ftddrii030;
>>> +    dc->reset = ftddrii030_reset;
>>> +    dc->realize = ftddrii030_realize;
>>> +    dc->no_user = 1;
>>> +}
>>> +
>>> +static const TypeInfo ftddrii030_info = {
>>> +    .name          = TYPE_FTDDRII030,
>>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>>> +    .instance_size = sizeof(Ftddrii030State),
>>> +    .instance_init = ftddrii030_instance_init,
>>> +    .class_init    = ftddrii030_class_init,
>>> +};
>>> +
>>> +static void ftddrii030_register_types(void)
>>> +{
>>> +    type_register_static(&ftddrii030_info);
>>> +}
>>> +
>>> +type_init(ftddrii030_register_types)
>>> --
>>> 1.7.9.5
>>>
>>>
>
>
>
> --
> Best wishes,
> Kuo-Jung Su
>
Kuo-Jung Su - March 28, 2013, 5:28 a.m.
2013/3/28 Peter Crosthwaite <peter.crosthwaite@xilinx.com>:
> Hi Kuo Jung,
>
> On Thu, Mar 28, 2013 at 1:24 PM, Kuo-Jung Su <dantesu@gmail.com> wrote:
>> 2013/3/28 Peter Crosthwaite <peter.crosthwaite@xilinx.com>:
>>> Hi Kuo-Jung,
>>>
>>> On Mon, Mar 25, 2013 at 10:09 PM, Kuo-Jung Su <dantesu@gmail.com> wrote:
>>>> From: Kuo-Jung Su <dantesu@faraday-tech.com>
>>>>
>>>> The FTDDRII030 is a DDRII SDRAM controller which is responsible for
>>>> SDRAM initialization.
>>>>
>>>> Signed-off-by: Kuo-Jung Su <dantesu@faraday-tech.com>
>>>> ---
>>>>  hw/arm/Makefile.objs    |    2 +-
>>>>  hw/arm/ftplat_a369soc.c |    8 ++
>>>>  hw/ftddrii030.c         |  192 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>  3 files changed, 201 insertions(+), 1 deletion(-)
>>>>  create mode 100644 hw/ftddrii030.c
>>>>
>>>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>>>> index b2fa20f..e774962 100644
>>>> --- a/hw/arm/Makefile.objs
>>>> +++ b/hw/arm/Makefile.objs
>>>> @@ -24,7 +24,7 @@ obj-y += framebuffer.o
>>>>  obj-y += strongarm.o
>>>>  obj-y += imx_serial.o imx_ccm.o imx_timer.o imx_avic.o
>>>>  obj-$(CONFIG_KVM) += kvm/arm_gic.o
>>>> -obj-y += ftintc020.o ftahbc020.o
>>>> +obj-y += ftintc020.o ftahbc020.o ftddrii030.o
>>>>
>>>>  obj-y := $(addprefix ../,$(obj-y))
>>>>
>>>> diff --git a/hw/arm/ftplat_a369soc.c b/hw/arm/ftplat_a369soc.c
>>>> index 7f222cb..b2da582 100644
>>>> --- a/hw/arm/ftplat_a369soc.c
>>>> +++ b/hw/arm/ftplat_a369soc.c
>>>> @@ -129,6 +129,14 @@ static void a369soc_chip_init(FaradaySoCState *s)
>>>>          fprintf(stderr, "a369soc: Unable to set soc link for FTAHBC020\n");
>>>>          abort();
>>>>      }
>>>> +
>>>> +    /* ftddrii030 */
>>>> +    ds = sysbus_create_simple("ftddrii030", 0x93100000, NULL);
>>>> +    object_property_set_link(OBJECT(ds), OBJECT(s), "soc", &local_errp);
>>>> +    if (local_errp) {
>>>> +        fprintf(stderr, "a369soc: Unable to set soc link for FTDDRII030\n");
>>>> +        abort();
>>>> +    }
>>>>  }
>>>>
>>>>  static void a369soc_realize(DeviceState *dev, Error **errp)
>>>> diff --git a/hw/ftddrii030.c b/hw/ftddrii030.c
>>>> new file mode 100644
>>>> index 0000000..158db1f
>>>> --- /dev/null
>>>> +++ b/hw/ftddrii030.c
>>>> @@ -0,0 +1,192 @@
>>>> +/*
>>>> + * Faraday DDRII controller
>>>> + *
>>>> + * Copyright (c) 2012 Faraday Technology
>>>> + * Written by Dante Su <dantesu@faraday-tech.com>
>>>> + *
>>>> + * This code is licensed under GNU GPL v2+
>>>> + */
>>>> +
>>>> +#include "hw/hw.h"
>>>> +#include "hw/sysbus.h"
>>>> +#include "hw/devices.h"
>>>> +#include "sysemu/sysemu.h"
>>>> +
>>>> +#include "hw/faraday.h"
>>>> +
>>>> +#define REG_MCR             0x00    /* memory configuration register */
>>>> +#define REG_MSR             0x04    /* memory status register */
>>>> +#define REG_REVR            0x50    /* revision register */
>>>> +
>>>> +#define MSR_INIT_OK         BIT(8)  /* initialization finished */
>>>> +#define MSR_CMD_MRS         BIT(0)  /* start MRS command (init. seq.) */
>>>> +
>>>> +#define CFG_REGSIZE         (REG_REVR / 4)
>>>> +
>>>> +#define TYPE_FTDDRII030     "ftddrii030"
>>>> +
>>>> +typedef struct Ftddrii030State {
>>>> +    /*< private >*/
>>>> +    SysBusDevice parent;
>>>> +
>>>> +    /*< public >*/
>>>> +    MemoryRegion iomem;
>>>> +
>>>> +    FaradaySoCState *soc;
>>>> +
>>>
>>> This new implementation still has many of the same encapsulation
>>> issues discussed in v8. If you go with the suggestion myself and Peter
>>> came up with you could make this device completely self contained - it
>>> should have no awareness that it is part of the Faraday SoC. If you
>>> model as a second MemoryRegion you can push all the Faraday specific
>>> foo up to the SoC. Interdiff would go something like this:
>>>
>>> -    FaradaySoCState *soc;
>>> +    MemoryRegion ram;
>>>
>>
>> Sorry, but I think it might not work in that way.
>> Because there is an AHB remap function is Faraday SoC,
>> which would alter the base address of both ROM and RAM.
>> So the ram instance should never be a local variable to FTDDRII030 only.
>>
>
> The new RAM region would have no awareness of its base address. It is
> just a remappable container. The SoC can still do the remapping part.
> This device model is then oblivous to the fact that the SoC and other
> devices are remapping its base address.
>
>> The first thing solution spring up in my mind is to add a QOM link in
>> FTDDRII030, and then pass the ram instance as a QOM link from device model.
>> However it turns out that it's also not possible, since the ram
>> instance (MemoryRegion) is not a Object *.
>>
>
> calling sysbus_mmio_get_region() on the SoC level should do the trick.
> If you init the container region as proposed, this function can be
> used by the SoC to get a handle to it.
>

It sounds like pretty good idea to me, I think I know what to do now.
Thanks

> I think we are on similar timezones (+10:00 here) if you want to try
> and clarify on the IRC sometime today as well.
>

Thanks for the invitation, but the IRQ is banned in my company...

> Regards,
> Peter
>
>> So I turn out to implement both the 'AHB remap' and 'RAM
>> initialization' as common routines
>> to Faraday SoC, and also create a remappable memory region for both ROM and RAM.
>>
>>>> +    /* HW register cache */
>>>> +    uint32_t regs[CFG_REGSIZE];
>>>> +} Ftddrii030State;
>>>> +
>>>> +#define FTDDRII030(obj) \
>>>> +    OBJECT_CHECK(Ftddrii030State, obj, TYPE_FTDDRII030)
>>>> +
>>>> +#define DDR_REG32(s, off) \
>>>> +    ((s)->regs[(off) / 4])
>>>> +
>>>> +static uint64_t
>>>> +ftddrii030_mem_read(void *opaque, hwaddr addr, unsigned size)
>>>> +{
>>>> +    Ftddrii030State *s = FTDDRII030(opaque);
>>>> +    FaradaySoCState *soc = s->soc;
>>>> +    uint64_t ret = 0;
>>>> +
>>>> +    if (soc->ram_visible) {
>>>> +        DDR_REG32(s, REG_MSR) |= MSR_INIT_OK;
>>>> +    } else {
>>>> +        DDR_REG32(s, REG_MSR) &= ~MSR_INIT_OK;
>>>> +    }
>>>> +
>>>> +    switch (addr) {
>>>> +    case REG_MCR ... REG_REVR - 4:
>>>> +        ret = DDR_REG32(s, addr);
>>>> +        break;
>>>> +    case REG_REVR:
>>>> +        ret = 0x100;    /* rev. = 0.1.0 */
>>>> +        break;
>>>> +    default:
>>>> +        qemu_log_mask(LOG_GUEST_ERROR,
>>>> +            "ftddrii030: undefined memory access@%#" HWADDR_PRIx "\n", addr);
>>>> +        break;
>>>> +    }
>>>> +
>>>> +    return ret;
>>>> +}
>>>> +
>>>> +static void
>>>> +ftddrii030_mem_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
>>>> +{
>>>> +    Ftddrii030State *s = FTDDRII030(opaque);
>>>> +    FaradaySoCState *soc = s->soc;
>>>> +
>>>> +    switch (addr) {
>>>> +    case REG_MCR:
>>>> +        DDR_REG32(s, addr) = (uint32_t)val & 0xffff;
>>>> +        break;
>>>> +    case REG_MSR:
>>>> +        if (!soc->ram_visible && (val & MSR_CMD_MRS)) {
>>>> +            val &= ~MSR_CMD_MRS;
>>>> +            faraday_soc_ram_setup(soc, true);
>>>
>>> This function (added earlier in this series) then becomes local to
>>> this device model and the memory_region_add_subregion() happens with
>>> the new ram Memory region instead. AFAICT that function only requires
>>> the ram size which strikes me as something that should be a QOM
>>> property to the device.
>>>
>>
>> Please see the comments above.
>>
>>>> +        }
>>>> +        DDR_REG32(s, addr) = (uint32_t)val;
>>>> +        break;
>>>> +    /* SDRAM Timing, ECC ...etc. */
>>>> +    case REG_MSR + 4 ... REG_REVR - 4:
>>>> +        DDR_REG32(s, addr) = (uint32_t)val;
>>>> +        break;
>>>> +    case REG_REVR:
>>>> +        break;
>>>> +    default:
>>>> +        qemu_log_mask(LOG_GUEST_ERROR,
>>>> +            "ftddrii030: undefined memory access@%#" HWADDR_PRIx "\n", addr);
>>>> +        break;
>>>> +    }
>>>> +}
>>>> +
>>>> +static const MemoryRegionOps mmio_ops = {
>>>> +    .read  = ftddrii030_mem_read,
>>>> +    .write = ftddrii030_mem_write,
>>>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>>>> +    .valid = {
>>>> +        .min_access_size = 4,
>>>> +        .max_access_size = 4,
>>>> +    }
>>>> +};
>>>> +
>>>> +static void ftddrii030_reset(DeviceState *ds)
>>>> +{
>>>> +    Ftddrii030State *s = FTDDRII030(SYS_BUS_DEVICE(ds));
>>>> +    Error *local_errp = NULL;
>>>> +
>>>> +    s->soc = FARADAY_SOC(object_property_get_link(OBJECT(s),
>>>> +                                                  "soc",
>>>> +                                                  &local_errp));
>>>> +    if (local_errp) {
>>>> +        fprintf(stderr, "ftahbc020: Unable to get soc link\n");
>>>> +        abort();
>>>> +    }
>>>> +
>>>> +    faraday_soc_ram_setup(s->soc, false);
>>>> +    memset(s->regs, 0, sizeof(s->regs));
>>>> +}
>>>> +
>>>> +static void ftddrii030_realize(DeviceState *dev, Error **errp)
>>>> +{
>>>> +    Ftddrii030State *s = FTDDRII030(dev);
>>>> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>>>> +
>>>> +    memory_region_init_io(&s->iomem,
>>>> +                          &mmio_ops,
>>>> +                          s,
>>>> +                          TYPE_FTDDRII030,
>>>> +                          0x1000);
>>>> +    sysbus_init_mmio(sbd, &s->iomem);
>>>
>>> Init the new ram memory region here.
>>>
>>
>> Please see the comments above.
>>
>>> I could use a second opinion on all this QOM stuff though - you are on
>>> the bleeding edge with the QOM SoC stuff. I suggest giving it some
>>> list time for others to reply before a remake.
>>>
>>
>> Got it, thanks.
>>
>>> Regards,
>>> Peter
>>>
>>>> +}
>>>> +
>>>> +static const VMStateDescription vmstate_ftddrii030 = {
>>>> +    .name = TYPE_FTDDRII030,
>>>> +    .version_id = 1,
>>>> +    .minimum_version_id = 1,
>>>> +    .minimum_version_id_old = 1,
>>>> +    .fields = (VMStateField[]) {
>>>> +        VMSTATE_UINT32_ARRAY(regs, Ftddrii030State, CFG_REGSIZE),
>>>> +        VMSTATE_END_OF_LIST(),
>>>> +    }
>>>> +};
>>>> +
>>>> +static void ftddrii030_instance_init(Object *obj)
>>>> +{
>>>> +    Ftddrii030State *s = FTDDRII030(obj);
>>>> +
>>>> +    object_property_add_link(obj,
>>>> +                             "soc",
>>>> +                             TYPE_FARADAY_SOC,
>>>> +                             (Object **) &s->soc,
>>>> +                             NULL);
>>>> +}
>>>> +
>>>> +static void ftddrii030_class_init(ObjectClass *klass, void *data)
>>>> +{
>>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>>> +
>>>> +    dc->desc  = TYPE_FTDDRII030;
>>>> +    dc->vmsd  = &vmstate_ftddrii030;
>>>> +    dc->reset = ftddrii030_reset;
>>>> +    dc->realize = ftddrii030_realize;
>>>> +    dc->no_user = 1;
>>>> +}
>>>> +
>>>> +static const TypeInfo ftddrii030_info = {
>>>> +    .name          = TYPE_FTDDRII030,
>>>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>>>> +    .instance_size = sizeof(Ftddrii030State),
>>>> +    .instance_init = ftddrii030_instance_init,
>>>> +    .class_init    = ftddrii030_class_init,
>>>> +};
>>>> +
>>>> +static void ftddrii030_register_types(void)
>>>> +{
>>>> +    type_register_static(&ftddrii030_info);
>>>> +}
>>>> +
>>>> +type_init(ftddrii030_register_types)
>>>> --
>>>> 1.7.9.5
>>>>
>>>>
>>
>>
>>
>> --
>> Best wishes,
>> Kuo-Jung Su
>>



--
Best wishes,
Kuo-Jung Su

Patch

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index b2fa20f..e774962 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -24,7 +24,7 @@  obj-y += framebuffer.o
 obj-y += strongarm.o
 obj-y += imx_serial.o imx_ccm.o imx_timer.o imx_avic.o
 obj-$(CONFIG_KVM) += kvm/arm_gic.o
-obj-y += ftintc020.o ftahbc020.o
+obj-y += ftintc020.o ftahbc020.o ftddrii030.o
 
 obj-y := $(addprefix ../,$(obj-y))
 
diff --git a/hw/arm/ftplat_a369soc.c b/hw/arm/ftplat_a369soc.c
index 7f222cb..b2da582 100644
--- a/hw/arm/ftplat_a369soc.c
+++ b/hw/arm/ftplat_a369soc.c
@@ -129,6 +129,14 @@  static void a369soc_chip_init(FaradaySoCState *s)
         fprintf(stderr, "a369soc: Unable to set soc link for FTAHBC020\n");
         abort();
     }
+
+    /* ftddrii030 */
+    ds = sysbus_create_simple("ftddrii030", 0x93100000, NULL);
+    object_property_set_link(OBJECT(ds), OBJECT(s), "soc", &local_errp);
+    if (local_errp) {
+        fprintf(stderr, "a369soc: Unable to set soc link for FTDDRII030\n");
+        abort();
+    }
 }
 
 static void a369soc_realize(DeviceState *dev, Error **errp)
diff --git a/hw/ftddrii030.c b/hw/ftddrii030.c
new file mode 100644
index 0000000..158db1f
--- /dev/null
+++ b/hw/ftddrii030.c
@@ -0,0 +1,192 @@ 
+/*
+ * Faraday DDRII controller
+ *
+ * Copyright (c) 2012 Faraday Technology
+ * Written by Dante Su <dantesu@faraday-tech.com>
+ *
+ * This code is licensed under GNU GPL v2+
+ */
+
+#include "hw/hw.h"
+#include "hw/sysbus.h"
+#include "hw/devices.h"
+#include "sysemu/sysemu.h"
+
+#include "hw/faraday.h"
+
+#define REG_MCR             0x00    /* memory configuration register */
+#define REG_MSR             0x04    /* memory status register */
+#define REG_REVR            0x50    /* revision register */
+
+#define MSR_INIT_OK         BIT(8)  /* initialization finished */
+#define MSR_CMD_MRS         BIT(0)  /* start MRS command (init. seq.) */
+
+#define CFG_REGSIZE         (REG_REVR / 4)
+
+#define TYPE_FTDDRII030     "ftddrii030"
+
+typedef struct Ftddrii030State {
+    /*< private >*/
+    SysBusDevice parent;
+
+    /*< public >*/
+    MemoryRegion iomem;
+
+    FaradaySoCState *soc;
+
+    /* HW register cache */
+    uint32_t regs[CFG_REGSIZE];
+} Ftddrii030State;
+
+#define FTDDRII030(obj) \
+    OBJECT_CHECK(Ftddrii030State, obj, TYPE_FTDDRII030)
+
+#define DDR_REG32(s, off) \
+    ((s)->regs[(off) / 4])
+
+static uint64_t
+ftddrii030_mem_read(void *opaque, hwaddr addr, unsigned size)
+{
+    Ftddrii030State *s = FTDDRII030(opaque);
+    FaradaySoCState *soc = s->soc;
+    uint64_t ret = 0;
+
+    if (soc->ram_visible) {
+        DDR_REG32(s, REG_MSR) |= MSR_INIT_OK;
+    } else {
+        DDR_REG32(s, REG_MSR) &= ~MSR_INIT_OK;
+    }
+
+    switch (addr) {
+    case REG_MCR ... REG_REVR - 4:
+        ret = DDR_REG32(s, addr);
+        break;
+    case REG_REVR:
+        ret = 0x100;    /* rev. = 0.1.0 */
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+            "ftddrii030: undefined memory access@%#" HWADDR_PRIx "\n", addr);
+        break;
+    }
+
+    return ret;
+}
+
+static void
+ftddrii030_mem_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
+{
+    Ftddrii030State *s = FTDDRII030(opaque);
+    FaradaySoCState *soc = s->soc;
+
+    switch (addr) {
+    case REG_MCR:
+        DDR_REG32(s, addr) = (uint32_t)val & 0xffff;
+        break;
+    case REG_MSR:
+        if (!soc->ram_visible && (val & MSR_CMD_MRS)) {
+            val &= ~MSR_CMD_MRS;
+            faraday_soc_ram_setup(soc, true);
+        }
+        DDR_REG32(s, addr) = (uint32_t)val;
+        break;
+    /* SDRAM Timing, ECC ...etc. */
+    case REG_MSR + 4 ... REG_REVR - 4:
+        DDR_REG32(s, addr) = (uint32_t)val;
+        break;
+    case REG_REVR:
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR,
+            "ftddrii030: undefined memory access@%#" HWADDR_PRIx "\n", addr);
+        break;
+    }
+}
+
+static const MemoryRegionOps mmio_ops = {
+    .read  = ftddrii030_mem_read,
+    .write = ftddrii030_mem_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    }
+};
+
+static void ftddrii030_reset(DeviceState *ds)
+{
+    Ftddrii030State *s = FTDDRII030(SYS_BUS_DEVICE(ds));
+    Error *local_errp = NULL;
+
+    s->soc = FARADAY_SOC(object_property_get_link(OBJECT(s),
+                                                  "soc",
+                                                  &local_errp));
+    if (local_errp) {
+        fprintf(stderr, "ftahbc020: Unable to get soc link\n");
+        abort();
+    }
+
+    faraday_soc_ram_setup(s->soc, false);
+    memset(s->regs, 0, sizeof(s->regs));
+}
+
+static void ftddrii030_realize(DeviceState *dev, Error **errp)
+{
+    Ftddrii030State *s = FTDDRII030(dev);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+
+    memory_region_init_io(&s->iomem,
+                          &mmio_ops,
+                          s,
+                          TYPE_FTDDRII030,
+                          0x1000);
+    sysbus_init_mmio(sbd, &s->iomem);
+}
+
+static const VMStateDescription vmstate_ftddrii030 = {
+    .name = TYPE_FTDDRII030,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(regs, Ftddrii030State, CFG_REGSIZE),
+        VMSTATE_END_OF_LIST(),
+    }
+};
+
+static void ftddrii030_instance_init(Object *obj)
+{
+    Ftddrii030State *s = FTDDRII030(obj);
+
+    object_property_add_link(obj,
+                             "soc",
+                             TYPE_FARADAY_SOC,
+                             (Object **) &s->soc,
+                             NULL);
+}
+
+static void ftddrii030_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->desc  = TYPE_FTDDRII030;
+    dc->vmsd  = &vmstate_ftddrii030;
+    dc->reset = ftddrii030_reset;
+    dc->realize = ftddrii030_realize;
+    dc->no_user = 1;
+}
+
+static const TypeInfo ftddrii030_info = {
+    .name          = TYPE_FTDDRII030,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(Ftddrii030State),
+    .instance_init = ftddrii030_instance_init,
+    .class_init    = ftddrii030_class_init,
+};
+
+static void ftddrii030_register_types(void)
+{
+    type_register_static(&ftddrii030_info);
+}
+
+type_init(ftddrii030_register_types)