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. 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