[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v2 3/7] m48t59: register a QOM type for each nvram
From: |
Artyom Tarasenko |
Subject: |
Re: [Qemu-devel] [RFC v2 3/7] m48t59: register a QOM type for each nvram type we support |
Date: |
Sat, 27 Apr 2013 08:55:59 +0200 |
On Sat, Apr 20, 2013 at 12:39 PM, Blue Swirl <address@hidden> wrote:
> On Sat, Apr 20, 2013 at 9:56 AM, Artyom Tarasenko <address@hidden> wrote:
>> On Sat, Apr 20, 2013 at 11:34 AM, Blue Swirl <address@hidden> wrote:
>>> On Sun, Apr 14, 2013 at 9:41 PM, Artyom Tarasenko <address@hidden> wrote:
>>>> On Sun, Apr 14, 2013 at 10:05 AM, Hervé Poussineau <address@hidden> wrote:
>>>>> As m48t59 devices can only be created with m48t59_init() or
>>>>> m48t59_init_isa(),
>>>>> we know exactly which nvram types are required. Register only those three
>>>>> types.
>>>>> Remove .model and .size properties as they can be infered from nvram name.
>>>>> Remove .io_base ISA address port as m48t59_init_isa() is always called
>>>>> with ioport 0x74.
>>>>
>>>> While this it indeed how it's currently called, this is wrong for the
>>>> sun4u emulation.
>>>> The isa (ebus) variant of the sun4u m48t59_init_isa() should be called
>>>> with a mem_base, not io_base.
>>>
>>> Why? I don't see much difference between EBUS and ISA and with the
>>> memory API, the difference between PIO and MMIO is almost nonexistent
>>> anyway.
>>
>> Can you elaborate? Do you mean we just need to change the io_base?
>
> Why wouldn't that work?
Because the PIO variant registers just 4 ports, whereas MMIO registers
the whole device (0x2000 bytes).
The sun4u machines use a slightly different modification of the ISA Mostek chip.
It has MMIO, 1968 as a base year and no IRQ line. Since it matches our m48t08,
I'll send a patch fixing it.
>>> But it should be possible to change the base to match real HW, whatever it
>>> is:
>>> http://git.kernel.org/cgit/linux/kernel/git/davem/prtconfs.git/tree/ultra5#n273
>>
>> Yes, I know where it is supposed to be, I'm just asking how to achieve
>> this best with our current tooling.
>>
>>> So NACK for the original patch.
>>>
>>>> Do you think it should be implemented as another device type?
>>>>
>>>>
>>>>> Signed-off-by: Hervé Poussineau <address@hidden>
>>>>> ---
>>>>> hw/timer/m48t59.c | 187
>>>>> ++++++++++++++++++++++++++++++++++++-----------------
>>>>> 1 file changed, 126 insertions(+), 61 deletions(-)
>>>>>
>>>>> diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
>>>>> index 41022f2..29ec462 100644
>>>>> --- a/hw/timer/m48t59.c
>>>>> +++ b/hw/timer/m48t59.c
>>>>> @@ -43,6 +43,13 @@
>>>>> * PPC platform there is also a nvram lock function.
>>>>> */
>>>>>
>>>>> +typedef struct M48txxInfo {
>>>>> + const char *isa_name;
>>>>> + const char *sysbus_name;
>>>>> + uint32_t model; /* 2 = m48t02, 8 = m48t08, 59 = m48t59 */
>>>>> + uint32_t size;
>>>>> +} M48txxInfo;
>>>>> +
>>>>> /*
>>>>> * Chipset docs:
>>>>> * http://www.st.com/stonline/products/literature/ds/2410/m48t02.pdf
>>>>> @@ -54,7 +61,6 @@ struct M48t59State {
>>>>> /* Hardware parameters */
>>>>> qemu_irq IRQ;
>>>>> MemoryRegion iomem;
>>>>> - uint32_t io_base;
>>>>> uint32_t size;
>>>>> /* RTC management */
>>>>> time_t time_offset;
>>>>> @@ -78,12 +84,39 @@ typedef struct M48t59ISAState {
>>>>> MemoryRegion io;
>>>>> } M48t59ISAState;
>>>>>
>>>>> +typedef struct M48txxISADeviceClass {
>>>>> + ISADeviceClass parent_class;
>>>>> + M48txxInfo info;
>>>>> +} M48txxISADeviceClass;
>>>>> +
>>>>> typedef struct M48t59SysBusState {
>>>>> SysBusDevice busdev;
>>>>> M48t59State state;
>>>>> MemoryRegion io;
>>>>> } M48t59SysBusState;
>>>>>
>>>>> +typedef struct M48txxSysBusDeviceClass {
>>>>> + SysBusDeviceClass parent_class;
>>>>> + M48txxInfo info;
>>>>> +} M48txxSysBusDeviceClass;
>>>>> +
>>>>> +static M48txxInfo m48txx_info[] = {
>>>>> + {
>>>>> + .sysbus_name = "m48t02",
>>>>> + .model = 2,
>>>>> + .size = 0x800,
>>>>> + },{
>>>>> + .sysbus_name = "m48t08",
>>>>> + .model = 8,
>>>>> + .size = 0x2000,
>>>>> + },{
>>>>> + .isa_name = "m48t59_isa",
>>>>> + .model = 59,
>>>>> + .size = 0x2000,
>>>>> + }
>>>>> +};
>>>>> +
>>>>> +
>>>>> /* Fake timer functions */
>>>>>
>>>>> /* Alarm management */
>>>>> @@ -640,25 +673,34 @@ M48t59State *m48t59_init(qemu_irq IRQ, hwaddr
>>>>> mem_base,
>>>>> SysBusDevice *s;
>>>>> M48t59SysBusState *d;
>>>>> M48t59State *state;
>>>>> + int i;
>>>>>
>>>>> - dev = qdev_create(NULL, "m48t59");
>>>>> - qdev_prop_set_uint32(dev, "model", model);
>>>>> - qdev_prop_set_uint32(dev, "size", size);
>>>>> - qdev_prop_set_uint32(dev, "io_base", io_base);
>>>>> - qdev_init_nofail(dev);
>>>>> - s = SYS_BUS_DEVICE(dev);
>>>>> - d = FROM_SYSBUS(M48t59SysBusState, s);
>>>>> - state = &d->state;
>>>>> - sysbus_connect_irq(s, 0, IRQ);
>>>>> - memory_region_init_io(&d->io, &m48t59_io_ops, state, "m48t59", 4);
>>>>> - if (io_base != 0) {
>>>>> - memory_region_add_subregion(get_system_io(), io_base, &d->io);
>>>>> - }
>>>>> - if (mem_base != 0) {
>>>>> - sysbus_mmio_map(s, 0, mem_base);
>>>>> + for (i = 0; i < ARRAY_SIZE(m48txx_info); i++) {
>>>>> + if (!m48txx_info[i].sysbus_name ||
>>>>> + m48txx_info[i].size != size ||
>>>>> + m48txx_info[i].model != model) {
>>>>> + continue;
>>>>> + }
>>>>> +
>>>>> + dev = qdev_create(NULL, m48txx_info[i].sysbus_name);
>>>>> + qdev_init_nofail(dev);
>>>>> + s = SYS_BUS_DEVICE(dev);
>>>>> + d = FROM_SYSBUS(M48t59SysBusState, s);
>>>>> + state = &d->state;
>>>>> + sysbus_connect_irq(s, 0, IRQ);
>>>>> + memory_region_init_io(&d->io, &m48t59_io_ops, state, "m48t59",
>>>>> 4);
>>>>> + if (io_base != 0) {
>>>>> + memory_region_add_subregion(get_system_io(), io_base,
>>>>> &d->io);
>>>>> + }
>>>>> + if (mem_base != 0) {
>>>>> + sysbus_mmio_map(s, 0, mem_base);
>>>>> + }
>>>>> +
>>>>> + return state;
>>>>> }
>>>>>
>>>>> - return state;
>>>>> + assert(false);
>>>>> + return NULL;
>>>>> }
>>>>>
>>>>> M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t
>>>>> size,
>>>>> @@ -667,16 +709,27 @@ M48t59State *m48t59_init_isa(ISABus *bus, uint32_t
>>>>> io_base, uint16_t size,
>>>>> M48t59ISAState *d;
>>>>> ISADevice *dev;
>>>>> M48t59State *s;
>>>>> + int i;
>>>>> +
>>>>> + assert(io_base == 0x74);
>>>>> +
>>>>> + for (i = 0; i < ARRAY_SIZE(m48txx_info); i++) {
>>>>> + if (!m48txx_info[i].isa_name ||
>>>>> + m48txx_info[i].size != size ||
>>>>> + m48txx_info[i].model != model) {
>>>>> + continue;
>>>>> + }
>>>>>
>>>>> - dev = isa_create(bus, "m48t59_isa");
>>>>> - qdev_prop_set_uint32(&dev->qdev, "model", model);
>>>>> - qdev_prop_set_uint32(&dev->qdev, "size", size);
>>>>> - qdev_prop_set_uint32(&dev->qdev, "io_base", io_base);
>>>>> - qdev_init_nofail(&dev->qdev);
>>>>> - d = DO_UPCAST(M48t59ISAState, busdev, dev);
>>>>> - s = &d->state;
>>>>> + dev = isa_create(bus, m48txx_info[i].isa_name);
>>>>> + qdev_init_nofail(&dev->qdev);
>>>>> + d = DO_UPCAST(M48t59ISAState, busdev, dev);
>>>>> + s = &d->state;
>>>>>
>>>>> - return s;
>>>>> + return s;
>>>>> + }
>>>>> +
>>>>> + assert(false);
>>>>> + return NULL;
>>>>> }
>>>>>
>>>>> static void m48t59_init_common(M48t59State *s)
>>>>> @@ -693,24 +746,32 @@ static void m48t59_init_common(M48t59State *s)
>>>>>
>>>>> static int m48t59_init_isa1(ISADevice *dev)
>>>>> {
>>>>> + ISADeviceClass *ic = ISA_DEVICE_GET_CLASS(dev);
>>>>> + M48txxISADeviceClass *u = container_of(ic, M48txxISADeviceClass,
>>>>> + parent_class);
>>>>> M48t59ISAState *d = DO_UPCAST(M48t59ISAState, busdev, dev);
>>>>> M48t59State *s = &d->state;
>>>>>
>>>>> + s->model = u->info.model;
>>>>> + s->size = u->info.size;
>>>>> isa_init_irq(dev, &s->IRQ, 8);
>>>>> m48t59_init_common(s);
>>>>> memory_region_init_io(&d->io, &m48t59_io_ops, s, "m48t59", 4);
>>>>> - if (s->io_base != 0) {
>>>>> - isa_register_ioport(dev, &d->io, s->io_base);
>>>>> - }
>>>>> + isa_register_ioport(dev, &d->io, 0x74);
>>>>>
>>>>> return 0;
>>>>> }
>>>>>
>>>>> static int m48t59_init1(SysBusDevice *dev)
>>>>> {
>>>>> + SysBusDeviceClass *k = SYS_BUS_DEVICE_GET_CLASS(dev);
>>>>> + M48txxSysBusDeviceClass *u = container_of(k, M48txxSysBusDeviceClass,
>>>>> + parent_class);
>>>>> M48t59SysBusState *d = FROM_SYSBUS(M48t59SysBusState, dev);
>>>>> M48t59State *s = &d->state;
>>>>>
>>>>> + s->model = u->info.model;
>>>>> + s->size = u->info.size;
>>>>> sysbus_init_irq(dev, &s->IRQ);
>>>>>
>>>>> memory_region_init_io(&s->iomem, &nvram_ops, s, "m48t59.nvram",
>>>>> s->size);
>>>>> @@ -720,58 +781,62 @@ static int m48t59_init1(SysBusDevice *dev)
>>>>> return 0;
>>>>> }
>>>>>
>>>>> -static Property m48t59_isa_properties[] = {
>>>>> - DEFINE_PROP_UINT32("size", M48t59ISAState, state.size, -1),
>>>>> - DEFINE_PROP_UINT32("model", M48t59ISAState, state.model, -1),
>>>>> - DEFINE_PROP_HEX32( "io_base", M48t59ISAState, state.io_base, 0),
>>>>> - DEFINE_PROP_END_OF_LIST(),
>>>>> -};
>>>>> -
>>>>> -static void m48t59_init_class_isa1(ObjectClass *klass, void *data)
>>>>> +static void m48t59_isa_class_init(ObjectClass *klass, void *data)
>>>>> {
>>>>> DeviceClass *dc = DEVICE_CLASS(klass);
>>>>> ISADeviceClass *ic = ISA_DEVICE_CLASS(klass);
>>>>> + M48txxISADeviceClass *u = container_of(ic, M48txxISADeviceClass,
>>>>> + parent_class);
>>>>> + M48txxInfo *info = data;
>>>>> +
>>>>> ic->init = m48t59_init_isa1;
>>>>> dc->no_user = 1;
>>>>> dc->reset = m48t59_reset_isa;
>>>>> - dc->props = m48t59_isa_properties;
>>>>> + u->info = *info;
>>>>> }
>>>>>
>>>>> -static const TypeInfo m48t59_isa_info = {
>>>>> - .name = "m48t59_isa",
>>>>> - .parent = TYPE_ISA_DEVICE,
>>>>> - .instance_size = sizeof(M48t59ISAState),
>>>>> - .class_init = m48t59_init_class_isa1,
>>>>> -};
>>>>> -
>>>>> -static Property m48t59_properties[] = {
>>>>> - DEFINE_PROP_UINT32("size", M48t59SysBusState, state.size, -1),
>>>>> - DEFINE_PROP_UINT32("model", M48t59SysBusState, state.model, -1),
>>>>> - DEFINE_PROP_HEX32( "io_base", M48t59SysBusState, state.io_base, 0),
>>>>> - DEFINE_PROP_END_OF_LIST(),
>>>>> -};
>>>>> -
>>>>> static void m48t59_class_init(ObjectClass *klass, void *data)
>>>>> {
>>>>> DeviceClass *dc = DEVICE_CLASS(klass);
>>>>> SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>>>>> + M48txxSysBusDeviceClass *u = container_of(k, M48txxSysBusDeviceClass,
>>>>> + parent_class);
>>>>> + M48txxInfo *info = data;
>>>>>
>>>>> k->init = m48t59_init1;
>>>>> dc->reset = m48t59_reset_sysbus;
>>>>> - dc->props = m48t59_properties;
>>>>> + u->info = *info;
>>>>> }
>>>>>
>>>>> -static const TypeInfo m48t59_info = {
>>>>> - .name = "m48t59",
>>>>> - .parent = TYPE_SYS_BUS_DEVICE,
>>>>> - .instance_size = sizeof(M48t59SysBusState),
>>>>> - .class_init = m48t59_class_init,
>>>>> -};
>>>>> -
>>>>> static void m48t59_register_types(void)
>>>>> {
>>>>> - type_register_static(&m48t59_info);
>>>>> - type_register_static(&m48t59_isa_info);
>>>>> + TypeInfo m48txx_type_info = {
>>>>> + .parent = TYPE_SYS_BUS_DEVICE,
>>>>> + .instance_size = sizeof(M48t59SysBusState),
>>>>> + .class_size = sizeof(M48txxSysBusDeviceClass),
>>>>> + .class_init = m48t59_class_init,
>>>>> + };
>>>>> + TypeInfo m48txx_isa_type_info = {
>>>>> + .parent = TYPE_ISA_DEVICE,
>>>>> + .instance_size = sizeof(M48t59ISAState),
>>>>> + .class_size = sizeof(M48txxISADeviceClass),
>>>>> + .class_init = m48t59_isa_class_init,
>>>>> + };
>>>>> + int i;
>>>>> +
>>>>> + for (i = 0; i < ARRAY_SIZE(m48txx_info); i++) {
>>>>> + if (m48txx_info[i].sysbus_name) {
>>>>> + m48txx_type_info.name = m48txx_info[i].sysbus_name;
>>>>> + m48txx_type_info.class_data = &m48txx_info[i];
>>>>> + type_register(&m48txx_type_info);
>>>>> + }
>>>>> +
>>>>> + if (m48txx_info[i].isa_name) {
>>>>> + m48txx_isa_type_info.name = m48txx_info[i].isa_name;
>>>>> + m48txx_isa_type_info.class_data = &m48txx_info[i];
>>>>> + type_register(&m48txx_isa_type_info);
>>>>> + }
>>>>> + }
>>>>> }
>>>>>
>>>>> type_init(m48t59_register_types)
>>>>> --
>>>>> 1.7.10.4
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Regards,
>>>> Artyom Tarasenko
>>>>
>>>> linux/sparc and solaris/sparc under qemu blog:
>>>> http://tyom.blogspot.com/search/label/qemu
>>
>>
>>
>> --
>> Regards,
>> Artyom Tarasenko
>>
>> linux/sparc and solaris/sparc under qemu blog:
>> http://tyom.blogspot.com/search/label/qemu
--
Regards,
Artyom Tarasenko
linux/sparc and solaris/sparc under qemu blog:
http://tyom.blogspot.com/search/label/qemu
[Qemu-devel] [RFC v2 4/7] m48t59: use DeviceState in public functions, Hervé Poussineau, 2013/04/14
[Qemu-devel] [RFC v2 5/7] prep: add IBM RS/6000 7248 (43p) machine emulation, Hervé Poussineau, 2013/04/14
[Qemu-devel] [RFC v2 6/7] prep: QOM'ify System I/O, Hervé Poussineau, 2013/04/14
[Qemu-devel] [RFC v2 7/7] m48t59: hack(?) to make it work on IBM 43p, Hervé Poussineau, 2013/04/14