On Sat, Apr 20, 2013 at 12:39 PM, Blue Swirl <blauwir...@gmail.com> wrote: > On Sat, Apr 20, 2013 at 9:56 AM, Artyom Tarasenko <atar4q...@gmail.com> wrote: >> On Sat, Apr 20, 2013 at 11:34 AM, Blue Swirl <blauwir...@gmail.com> wrote: >>> On Sun, Apr 14, 2013 at 9:41 PM, Artyom Tarasenko <atar4q...@gmail.com> >>> wrote: >>>> On Sun, Apr 14, 2013 at 10:05 AM, Hervé Poussineau <hpous...@reactos.org> >>>> 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 <hpous...@reactos.org> >>>>> --- >>>>> 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
