| Message ID | 1365926760-5803-4-git-send-email-hpoussin@reactos.org |
|---|---|
| State | New |
| Headers | show |
On Sun, Apr 14, 2013 at 10:05 AM, Hervé Poussineau <hpoussin@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 <hpoussin@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
Artyom Tarasenko a écrit : > On Sun, Apr 14, 2013 at 10:05 AM, Hervé Poussineau <hpoussin@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? > I don't know EBUS, but I think it should be implemented either as a completly new bus (1), or as a child of the ISA bus type (2). For 1), you'll need to add a another device type to be plugged on EBUS. For 2), I let experts answer :) In all cases, maybe the m48t59_init() wrapper is what you need? You can already give him a membase. Otherwise, you can maybe use sysbus_create_simple("m48t59"), get the resulting MemoryRegion from the device, and add it in whatever MemoryRegion. Hervé
On Sun, Apr 14, 2013 at 9:41 PM, Artyom Tarasenko <atar4qemu@gmail.com> wrote: > On Sun, Apr 14, 2013 at 10:05 AM, Hervé Poussineau <hpoussin@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. 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 So NACK for the original patch. > Do you think it should be implemented as another device type? > > >> Signed-off-by: Hervé Poussineau <hpoussin@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
On Sat, Apr 20, 2013 at 11:34 AM, Blue Swirl <blauwirbel@gmail.com> wrote: > On Sun, Apr 14, 2013 at 9:41 PM, Artyom Tarasenko <atar4qemu@gmail.com> wrote: >> On Sun, Apr 14, 2013 at 10:05 AM, Hervé Poussineau <hpoussin@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? > 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 <hpoussin@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
On Sat, Apr 20, 2013 at 9:56 AM, Artyom Tarasenko <atar4qemu@gmail.com> wrote: > On Sat, Apr 20, 2013 at 11:34 AM, Blue Swirl <blauwirbel@gmail.com> wrote: >> On Sun, Apr 14, 2013 at 9:41 PM, Artyom Tarasenko <atar4qemu@gmail.com> wrote: >>> On Sun, Apr 14, 2013 at 10:05 AM, Hervé Poussineau <hpoussin@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? >> 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 <hpoussin@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
On Sat, Apr 20, 2013 at 12:39 PM, Blue Swirl <blauwirbel@gmail.com> wrote: > On Sat, Apr 20, 2013 at 9:56 AM, Artyom Tarasenko <atar4qemu@gmail.com> wrote: >> On Sat, Apr 20, 2013 at 11:34 AM, Blue Swirl <blauwirbel@gmail.com> wrote: >>> On Sun, Apr 14, 2013 at 9:41 PM, Artyom Tarasenko <atar4qemu@gmail.com> wrote: >>>> On Sun, Apr 14, 2013 at 10:05 AM, Hervé Poussineau <hpoussin@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 <hpoussin@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
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)
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. Signed-off-by: Hervé Poussineau <hpoussin@reactos.org> --- hw/timer/m48t59.c | 187 ++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 126 insertions(+), 61 deletions(-)