qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v3 03/20] arm: add Faraday FTAHBC020 support


From: Kuo-Jung Su
Subject: Re: [Qemu-devel] [PATCH v3 03/20] arm: add Faraday FTAHBC020 support
Date: Mon, 18 Feb 2013 17:00:24 +0800

2013/2/17 Peter Crosthwaite <address@hidden>:
> Hi Dante,
>
> On Wed, Feb 6, 2013 at 7:45 PM, Kuo-Jung Su <address@hidden> wrote:
>> From: Kuo-Jung Su <address@hidden>
>>
>> It's used to perform AHB remap and also QEMU RAM initialization
>> when SDRAM is initialized before AHB remap process activated.
>>
>> Signed-off-by: Kuo-Jung Su <address@hidden>
>> ---
>>  hw/arm/Makefile.objs  |    1 +
>>  hw/arm/faraday_a369.c |    6 ++
>>  hw/arm/ftahbc020.c    |  185 
>> +++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 192 insertions(+)
>>  create mode 100644 hw/arm/ftahbc020.c
>>
>> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
>> index 02d1a7b..5825c63 100644
>> --- a/hw/arm/Makefile.objs
>> +++ b/hw/arm/Makefile.objs
>> @@ -35,3 +35,4 @@ obj-$(CONFIG_FDT) += ../device_tree.o
>>  obj-y := $(addprefix ../,$(obj-y))
>>  obj-y += faraday_a360.o faraday_a360_pmu.o
>>  obj-y += faraday_a369.o faraday_a369_scu.o faraday_a369_keypad.o
>> +obj-y += ftahbc020.o
>> diff --git a/hw/arm/faraday_a369.c b/hw/arm/faraday_a369.c
>> index e32dc7f..ae6c445 100644
>> --- a/hw/arm/faraday_a369.c
>> +++ b/hw/arm/faraday_a369.c
>> @@ -54,6 +54,12 @@ a369_device_init(A369State *s)
>>
>>      /* ftkbc010 */
>>      sysbus_create_simple("a369.keypad", 0x92f00000, NULL);
>> +
>> +    /* ftahbc020 */
>> +    s->ahbc = qdev_create(NULL, "ftahbc020");
>> +    qdev_prop_set_ptr(s->ahbc, "mach", s);
>> +    qdev_init_nofail(s->ahbc);
>> +    sysbus_mmio_map(SYS_BUS_DEVICE(s->ahbc), 0, 0x94000000);
>>  }
>>
>>  static void
>> diff --git a/hw/arm/ftahbc020.c b/hw/arm/ftahbc020.c
>> new file mode 100644
>> index 0000000..d68676c
>> --- /dev/null
>> +++ b/hw/arm/ftahbc020.c
>> @@ -0,0 +1,185 @@
>> +/*
>> + * Faraday AHB 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 <ui/console.h>
>> +#include <qemu/timer.h>
>> +#include <sysemu/sysemu.h>
>> +
>> +#include "faraday.h"
>> +
>> +#define REG_SLAVE(n)    (n * 4) /* Slave device config (base & size) */
>> +#define REG_PRIR        0x80    /* Priority register */
>> +#define REG_IDLECR      0x84    /* IDLE count register */
>> +#define REG_CR          0x88    /* Control register */
>> +#define REG_REVR        0x8c    /* Revision register */
>> +
>> +#define TYPE_FTAHBC020  "ftahbc020"
>> +
>> +typedef struct Ftahbc020State {
>> +    SysBusDevice busdev;
>> +    MemoryRegion iomem;
>> +    void *mach;
>> +
>> +    /* HW register cache */
>> +    uint32_t slave4;
>> +    uint32_t slave6;
>> +    uint32_t cr;
>> +} Ftahbc020State;
>> +
>> +#define FTAHBC020(obj) \
>> +    OBJECT_CHECK(Ftahbc020State, obj, TYPE_FTAHBC020)
>> +
>> +static uint64_t
>> +ftahbc020_mem_read(void *opaque, hwaddr addr, unsigned size)
>> +{
>> +    Ftahbc020State *s = FTAHBC020(opaque);
>> +    uint64_t ret = 0;
>> +
>> +    switch (addr) {
>> +    case REG_SLAVE(4):
>> +        ret = s->slave4;
>> +        break;
>> +    case REG_SLAVE(6):
>> +        ret = s->slave6;
>> +        break;
>> +    case REG_CR:
>> +        ret = s->cr;
>> +        break;
>> +    case REG_REVR:
>> +        ret = 0x00010301;
>> +        break;
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +static void
>> +ftahbc020_mem_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
>> +{
>> +    Ftahbc020State *s = FTAHBC020(opaque);
>> +    FaradayMachState *mach = s->mach;
>
> I think this breaks the device model and QOM abstractions. You device
> has this ad-hoc total machine visibility through these structs. I
> think it would be better to use links from device to device that
> better reflect you machines interconnect.
>

I don't know much about QOM, so I don't really understand what you mean.
Are we talking about the following APIs?

object_property_set_link()
object_get_link_property()
object_set_link_property()

However it looks to me that 'QEMUMachine *' is not an 'Object *',
so I don't know how to create the links between 'QEMUMachine *' (a36x)
and 'DeviceState *' (ftddrii030 and ftabhc020)

>> +    uint32_t base;
>> +
>> +    if (!mach) {
>> +        hw_error("ftahbc020: mach is not yet registered!\n");
>> +        exit(1);
>> +    }
>> +
>> +    switch (addr) {
>> +    case REG_CR:
>> +        s->cr = (uint32_t)val;
>> +        if (!mach->ahb_remapped && (s->cr & 0x01)) {
>> +            /* Remap AHB slave 4 (ROM) & slave 6 (RAM) */
>> +            /* 1. Remap ROM to (0x00000000 + size of RAM) */
>> +            base = (1 << ((s->slave6 >> 16) & 0x0f)) << 20;
>> +            sysbus_mmio_map(SYS_BUS_DEVICE(mach->rom), 0, base);
>> +            /* 2. Update slave4(ROM) & slave6(RAM) */
>> +            s->slave4 = base | (s->slave4 & 0x000fffff);
>> +            s->slave6 = s->slave6 & 0x000fffff;
>> +            /* 3. Update SDRAM map if it has been initialized. */
>> +            if (mach->ddr_inited) {
>> +                memory_region_del_subregion(mach->as, mach->ram_alias);
>> +                memory_region_add_subregion(mach->as, 0, mach->ram);
>> +            }
>> +            mach->ahb_remapped = 1;
>
> Strange, is the device only capable of one-shot remapping or is this
> just a limitation of this device due to usage of sysbus_mmio_map?
>

First of all, sysbus_mmio_map is not the root cause here.

The AHB remap could only turn on/off for slave 4 and slave 6.
And the software which activates the remap process, must be carefully
designed to hold the entire critical section into one single 32 bytes cache line
or simply lockup by i-cache lockup instructions.
And I've been told that the DDR initial sequence shall never be
invoked more than once.

So yes, the device is only capable of one-shot remapping.

>> +        }
>> +        break;
>> +    }
>> +}
>> +
>> +static const MemoryRegionOps ftahbc020_mem_ops = {
>> +    .read  = ftahbc020_mem_read,
>> +    .write = ftahbc020_mem_write,
>> +    .endianness = DEVICE_LITTLE_ENDIAN,
>> +};
>> +
>> +static void ftahbc020_reset(DeviceState *ds)
>> +{
>> +    SysBusDevice *busdev = SYS_BUS_DEVICE(ds);
>> +    Ftahbc020State *s = FTAHBC020(FROM_SYSBUS(Ftahbc020State, busdev));
>> +    FaradayMachState *mach = s->mach;
>> +
>> +    if (!mach) {
>> +        hw_error("ftahbc020: mach is not yet initialized!\n");
>> +        exit(1);
>> +    }
>> +
>> +    if (mach->ahb_remapped) {
>> +        sysbus_mmio_map(SYS_BUS_DEVICE(mach->rom), 0, 0x00000000);
>> +        mach->ahb_remapped = 0;
>> +    }
>> +
>> +    s->cr = 0;
>> +    s->slave4 = mach->ahb_slave4;
>> +    s->slave6 = mach->ahb_slave6;
>> +
>> +    if (s->slave4 == 0 || s->slave6 == 0) {
>> +        hw_error("ftahbc020: slave 4 or 6 is not yet initialized!\n");
>> +        exit(1);
>> +    }
>> +}
>> +
>> +static int ftahbc020_init(SysBusDevice *dev)
>> +{
>> +    Ftahbc020State *s = FTAHBC020(FROM_SYSBUS(Ftahbc020State, dev));
>> +
>> +    memory_region_init_io(&s->iomem,
>> +                          &ftahbc020_mem_ops,
>> +                          s,
>> +                          TYPE_FTAHBC020,
>> +                          0x1000);
>> +    sysbus_init_mmio(dev, &s->iomem);
>> +    return 0;
>> +}
>> +
>> +static Property ftahbc020_properties[] = {
>> +    DEFINE_PROP_PTR("mach", Ftahbc020State, mach),
>
> DEFINE_PROP_PTR is deprecated and should not be used in new devices.
> QOM links are the replacement.
>

Got it, thanks.
But.... example please :P
I don't what to do right now.

> Regards,
> Peter
>
>> +    DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static const VMStateDescription vmstate_ftahbc020 = {
>> +    .name = TYPE_FTAHBC020,
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .minimum_version_id_old = 1,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_UINT32(cr, Ftahbc020State),
>> +        VMSTATE_END_OF_LIST(),
>> +    }
>> +};
>> +
>> +static void ftahbc020_class_init(ObjectClass *klass, void *data)
>> +{
>> +    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +    k->init   = ftahbc020_init;
>> +    dc->desc  = TYPE_FTAHBC020;
>> +    dc->vmsd  = &vmstate_ftahbc020;
>> +    dc->props = ftahbc020_properties;
>> +    dc->reset = ftahbc020_reset;
>> +    dc->no_user = 1;
>> +}
>> +
>> +static const TypeInfo ftahbc020_info = {
>> +    .name          = TYPE_FTAHBC020,
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(Ftahbc020State),
>> +    .class_init    = ftahbc020_class_init,
>> +};
>> +
>> +static void ftahbc020_register_types(void)
>> +{
>> +    type_register_static(&ftahbc020_info);
>> +}
>> +
>> +type_init(ftahbc020_register_types)
>> --
>> 1.7.9.5
>>
>>



--
Best wishes,
Kuo-Jung Su



reply via email to

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