qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v8 05/24] hw/arm: add FTDDRII030 DDRII controlle


From: Kuo-Jung Su
Subject: Re: [Qemu-devel] [PATCH v8 05/24] hw/arm: add FTDDRII030 DDRII controller support
Date: Mon, 18 Mar 2013 09:12:29 +0800

2013/3/16 Peter Crosthwaite <address@hidden>:
> Hi Kuo-Jung,
>
> On Fri, Mar 15, 2013 at 11:13 PM, Kuo-Jung Su <address@hidden> wrote:
>> From: Kuo-Jung Su <address@hidden>
>>
>> The FTDDRII030 is a DDRII SDRAM controller which is responsible for
>> SDRAM initialization.
>> In QEMU we emulate only the SDRAM enable function.
>>
>> Signed-off-by: Kuo-Jung Su <address@hidden>
>> ---
>>  hw/arm/Makefile.objs      |    1 +
>>  hw/arm/faraday_a369_soc.c |    9 +++
>>  hw/arm/ftddrii030.c       |  183 
>> +++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 193 insertions(+)
>>  create mode 100644 hw/arm/ftddrii030.c
>>
>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>> index af36b01..0bbf838 100644
>> --- a/hw/arm/Makefile.objs
>> +++ b/hw/arm/Makefile.objs
>> @@ -39,3 +39,4 @@ obj-y += faraday_a369.o faraday_a369_soc.o 
>> faraday_a369_scu.o \
>>              faraday_a369_kpd.o
>>  obj-y += ftintc020.o
>>  obj-y += ftahbc020.o
>> +obj-y += ftddrii030.o
>> diff --git a/hw/arm/faraday_a369_soc.c b/hw/arm/faraday_a369_soc.c
>> index 01b4395..e8a63bb 100644
>> --- a/hw/arm/faraday_a369_soc.c
>> +++ b/hw/arm/faraday_a369_soc.c
>> @@ -158,6 +158,15 @@ a369soc_device_init(FaradaySoCState *s)
>>          fprintf(stderr, "a369soc: Unable to set soc link for FTAHBC020\n");
>>          abort();
>>      }
>> +
>> +    /* ftddrii030 */
>> +    ds = sysbus_create_simple("ftddrii030", 0x93100000, NULL);
>> +    s->ddrc = ds;
>> +    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/arm/ftddrii030.c b/hw/arm/ftddrii030.c
>> new file mode 100644
>> index 0000000..90a5842
>> --- /dev/null
>> +++ b/hw/arm/ftddrii030.c
>> @@ -0,0 +1,183 @@
>> +/*
>> + * Faraday DDRII controller
>> + *
>> + * 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 "sysemu/sysemu.h"
>> +
>> +#include "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)  /* DDR2 initial is completed */
>> +#define MSR_CMD_MRS         BIT(0)  /* start MRS command */
>> +
>> +#define CFG_REGSIZE         (0x50 / 4)
>> +
>> +#define TYPE_FTDDRII030     "ftddrii030"
>> +
>> +typedef struct Ftddrii030State {
>> +    SysBusDevice busdev;
>> +    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);
>> +    uint64_t ret = 0;
>> +
>> +    if (s->soc->ddr_inited) {
>> +        DDR_REG32(s, REG_MSR) |= MSR_INIT_OK;
>> +    }
>> +
>> +    switch (addr) {
>> +    case REG_MCR ... (CFG_REGSIZE - 1) * 4:
>> +        ret = s->regs[addr / 4];
>> +        break;
>> +    case REG_REVR:
>> +        ret = 0x100;    /* rev. = 0.1.0 */
>> +        break;
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +            "ftddrii030: undefined memory address@hidden" 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);
>> +
>> +    switch (addr) {
>> +    case REG_MCR:
>> +        DDR_REG32(s, REG_MCR) = (uint32_t)val & 0xffff;
>> +        break;
>> +    case REG_MSR:
>> +        val = (val & 0x3f) | (DDR_REG32(s, REG_MSR) & MSR_INIT_OK);
>> +        if (!s->soc->ddr_inited && (val & MSR_CMD_MRS)) {
>> +            val &= ~MSR_CMD_MRS;
>> +            val |= MSR_INIT_OK;
>> +            memory_region_add_subregion(s->soc->as,
>> +                                        s->soc->ram_base,
>> +                                        s->soc->ram);
>
> I feel like this is overstepping the bounds of the device. Its
> modifying the internals of the parent device (the SoC itself). AFAICT,
> this device does not need awareness of where the RAM is to live in the
> address map, thats the responsibility of the machine model. It might
> be cleaner to model the actual RAM as a second sysbus memory region
> then leave it up the machine model to decide where in the address map
> it should live. This device just adds/removes the ram from the second
> region without knowing where it lives and the machine model maps the
> RAM to its actual location. Keeps .as .ram_base and .ram private to
> the SoC device.
>

Thanks for the comments,
I'll try to implement a cleaner model in the way suggested in the
above comments.

>> +            s->soc->ddr_inited = true;
>
> I'm still trying to figure out the physical analogue of this. Is there
> a genuine hardware linkage from the DDR controller to other devices
> that says "hey i'm inited" or is this faking firmware activity? In the
> former case, this ddr_inited should be a GPIO from DDR controller to
> whatever devices care. In the latter case, its trickier, and we should
> discuss bootloader based solutions to get your tiny little bit of
> firmware in, without having to model non-existent hardware.
>

Thanks for the comments.
It's the 1st one, it's used to inform the FTAHBC020 of the presence of
DRAM in QEMU model. And thus it could be replaced by a GPIO
implementation.
However in this way, it would slightly disobey the real hardware design.
I'll see if I could find a better way or not. If no luck, I'll try the
GPIO way as you suggested.

Kuo-Jung

> Regards,
> Peter
>
>> +        }
>> +        DDR_REG32(s, REG_MSR) = (uint32_t)val;
>> +        break;
>> +    case 0x08 ... (CFG_REGSIZE - 1) * 4: /* DDRII Timing, ECC ...etc. */
>> +        s->regs[addr / 4] = (uint32_t)val;
>> +        break;
>> +    default:
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +            "ftddrii030: undefined memory address@hidden" 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();
>> +    }
>> +
>> +    if (s->soc->ddr_inited && !s->soc->bi) {
>> +        memory_region_del_subregion(s->soc->as, s->soc->ram);
>> +        s->soc->ddr_inited = false;
>> +    }
>> +
>> +    memset(s->regs, 0, sizeof(s->regs));
>> +}
>> +
>> +static void ftddrii030_realize(DeviceState *dev, Error **errp)
>> +{
>> +    Ftddrii030State *s = FTDDRII030(dev);
>> +
>> +    memory_region_init_io(&s->iomem,
>> +                          &mmio_ops,
>> +                          s,
>> +                          TYPE_FTDDRII030,
>> +                          0x1000);
>> +    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
>> +
>> +    object_property_add_link(OBJECT(dev),
>> +                             "soc",
>> +                             TYPE_FARADAY_SOC,
>> +                             (Object **) &s->soc,
>> +                             errp);
>> +}
>> +
>> +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_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),
>> +    .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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]