Skip to contentSkip to conversation

timer/m48t59: QOM'ify

PATCH In review Hervé Poussineau 3 participants 7 messages Jan 19, 2015 – Feb 5, 2015

lore
Share & tools
lore
#0 lore raw

cover timer/m48t59: QOM'ify

Hi,

This patchset QOM'ifies the m48t59 timer.
Patch 1 allows the creation of the m4859 ISA device from the command line. Patch 2 QOM'ifies the device.
Patches 3 add a Nvram interface to m48txx devices, so that callers can detect the device and use it without knowing m48txx internals. Patch 4 is using this new Nvram interface and cleaning the m48t59 header.

Patches 3 and 4 are not needed for m48t59 QOM'ification, but are in my opinion a nice cleanup.

Hervé Poussineau (4):
m48t59: move ISA ports/memory regions registration to QOM constructor
m48t59: register a QOM type for each nvram type we support
m48t59: add a Nvram interface
m48t59: let init functions return a Nvram object

 hw/ppc/ppc.c              |  161 ----------------------
 hw/ppc/prep.c             |  161 ++++++++++++++++++++--
 hw/sparc/sun4m.c          |    8 +-
 hw/sparc64/sun4u.c        |   10 +-
 hw/timer/m48t59.c         |  336 ++++++++++++++++++++++++++++++++-------------
 include/hw/timer/m48t59.h |   60 ++++----
 6 files changed, 429 insertions(+), 307 deletions(-)
Signature · 1 line
1.7.10.4
#1 lore raw

PATCH 1/4 m48t59: move ISA ports/memory regions registration to QOM constructor

-device m48t59_isa can now be used to create a fully functional nvram.

---
 hw/timer/m48t59.c |   25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)
hw/timer/m48t59.c +13 -12
index 31509d5..967a313 100644
@@ -658,10 +658,9 @@ M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
     d = SYSBUS_M48T59(dev);
     state = &d->state;
     sysbus_connect_irq(s, 0, IRQ);
-    memory_region_init_io(&d->io, OBJECT(d), &m48t59_io_ops, state,
-                          "m48t59", 4);
     if (io_base != 0) {
-        memory_region_add_subregion(get_system_io(), io_base, &d->io);
+        memory_region_add_subregion(get_system_io(), io_base,
+                                    sysbus_mmio_get_region(dev, 1));
     }
     if (mem_base != 0) {
         sysbus_mmio_map(s, 0, mem_base);
@@ -687,11 +686,6 @@ M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
     d = ISA_M48T59(isadev);
     s = &d->state;
 
-    memory_region_init_io(&d->io, OBJECT(d), &m48t59_io_ops, s, "m48t59", 4);
-    if (io_base != 0) {
-        isa_register_ioport(isadev, &d->io, io_base);
-    }
-
     return s;
 }
 
@@ -715,19 +709,28 @@ static void m48t59_isa_realize(DeviceState *dev, Error **errp)
 
     isa_init_irq(isadev, &s->IRQ, 8);
     m48t59_realize_common(s, errp);
+    memory_region_init_io(&d->io, OBJECT(dev), &m48t59_io_ops, s, "m48t59", 4);
+    if (s->io_base != 0) {
+        isa_register_ioport(isadev, &d->io, s->io_base);
+    }
+
+    return 0;
 }
 
 static int m48t59_init1(SysBusDevice *dev)
 {
     M48t59SysBusState *d = SYSBUS_M48T59(dev);
+    Object *o = OBJECT(dev);
     M48t59State *s = &d->state;
     Error *err = NULL;
 
     sysbus_init_irq(dev, &s->IRQ);
 
-    memory_region_init_io(&s->iomem, OBJECT(d), &nvram_ops, s,
-                          "m48t59.nvram", s->size);
+    memory_region_init_io(&s->iomem, o, &nvram_ops, s, "m48t59.nvram",
+                          s->size);
+    memory_region_init_io(&d->io, o, &m48t59_io_ops, s, "m48t59", 4);
     sysbus_init_mmio(dev, &s->iomem);
+    sysbus_init_mmio(dev, &d->io);
     m48t59_realize_common(s, &err);
     if (err != NULL) {
         error_free(err);
@@ -751,8 +754,6 @@ static void m48t59_isa_class_init(ObjectClass *klass, void *data)
     dc->realize = m48t59_isa_realize;
     dc->reset = m48t59_reset_isa;
     dc->props = m48t59_isa_properties;
-    /* Reason: needs to be wired up by m48t59_init_isa() */
-    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo m48t59_isa_info = {
Signature · 1 line
1.7.10.4
#2 lore raw

PATCH 2/4 m48t59: register a QOM type for each nvram type we support

In reply to Hervé Poussineau

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.

Rename type to 'isa-*' (and 'sysbus-*') to do like other devices ISA devices (isa-ide, isa-parallel, isa-serial...)

---
 hw/timer/m48t59.c |  246 ++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 169 insertions(+), 77 deletions(-)
hw/timer/m48t59.c +169 -77
index 967a313..c46b63c 100644
@@ -2,6 +2,7 @@
  * QEMU M48T59 and M48T08 NVRAM emulation for PPC PREP and Sparc platforms
  *
  * Copyright (c) 2003-2005, 2007 Jocelyn Mayer
+ * Copyright (c) 2013 Hervé Poussineau
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -37,12 +38,35 @@
 #define NVRAM_PRINTF(fmt, ...) do { } while (0)
 #endif
 
+#define TYPE_M48TXX_SYS_BUS "sysbus-m48txx"
+#define M48TXX_SYS_BUS_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(M48txxSysBusDeviceClass, (obj), TYPE_M48TXX_SYS_BUS)
+#define M48TXX_SYS_BUS_CLASS(klass) \
+    OBJECT_CLASS_CHECK(M48txxSysBusDeviceClass, (klass), TYPE_M48TXX_SYS_BUS)
+#define M48TXX_SYS_BUS(obj) \
+    OBJECT_CHECK(M48txxSysBusState, (obj), TYPE_M48TXX_SYS_BUS)
+
+#define TYPE_M48TXX_ISA "isa-m48txx"
+#define M48TXX_ISA_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(M48txxISADeviceClass, (obj), TYPE_M48TXX_ISA)
+#define M48TXX_ISA_CLASS(klass) \
+    OBJECT_CLASS_CHECK(M48txxISADeviceClass, (klass), TYPE_M48TXX_ISA)
+#define M48TXX_ISA(obj) \
+    OBJECT_CHECK(M48txxISAState, (obj), TYPE_M48TXX_ISA)
+
 /*
  * The M48T02, M48T08 and M48T59 chips are very similar. The newer '59 has
  * alarm and a watchdog timer and related control registers. In the
  * 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 +78,6 @@ struct M48t59State {
     /* Hardware parameters */
     qemu_irq IRQ;
     MemoryRegion iomem;
-    uint32_t io_base;
     uint32_t size;
     /* RTC management */
     time_t   time_offset;
@@ -72,26 +95,45 @@ struct M48t59State {
     uint8_t  lock;
 };
 
-#define TYPE_ISA_M48T59 "m48t59_isa"
-#define ISA_M48T59(obj) \
-    OBJECT_CHECK(M48t59ISAState, (obj), TYPE_ISA_M48T59)
-
-typedef struct M48t59ISAState {
+typedef struct M48txxISAState {
     ISADevice parent_obj;
-
     M48t59State state;
+    uint32_t io_base;
     MemoryRegion io;
-} M48t59ISAState;
+} M48txxISAState;
 
-#define SYSBUS_M48T59(obj) \
-    OBJECT_CHECK(M48t59SysBusState, (obj), TYPE_SYSBUS_M48T59)
+typedef struct M48txxISADeviceClass {
+    ISADeviceClass parent_class;
+    M48txxInfo info;
+} M48txxISADeviceClass;
 
-typedef struct M48t59SysBusState {
+typedef struct M48txxSysBusState {
     SysBusDevice parent_obj;
-
     M48t59State state;
     MemoryRegion io;
-} M48t59SysBusState;
+} M48txxSysBusState;
+
+typedef struct M48txxSysBusDeviceClass {
+    SysBusDeviceClass parent_class;
+    M48txxInfo info;
+} M48txxSysBusDeviceClass;
+
+static M48txxInfo m48txx_info[] = {
+    {
+        .sysbus_name = "sysbus-m48t02",
+        .model = 2,
+        .size = 0x800,
+    },{
+        .sysbus_name = "sysbus-m48t08",
+        .model = 8,
+        .size = 0x2000,
+    },{
+        .isa_name = "isa-m48t59",
+        .model = 59,
+        .size = 0x2000,
+    }
+};
+
 
 /* Fake timer functions */
 
@@ -616,7 +658,7 @@ static void m48t59_reset_common(M48t59State *NVRAM)
 
 static void m48t59_reset_isa(DeviceState *d)
 {
-    M48t59ISAState *isa = ISA_M48T59(d);
+    M48txxISAState *isa = M48TXX_ISA(d);
     M48t59State *NVRAM = &isa->state;
 
     m48t59_reset_common(NVRAM);
@@ -624,7 +666,7 @@ static void m48t59_reset_isa(DeviceState *d)
 
 static void m48t59_reset_sysbus(DeviceState *d)
 {
-    M48t59SysBusState *sys = SYSBUS_M48T59(d);
+    M48txxSysBusState *sys = M48TXX_SYS_BUS(d);
     M48t59State *NVRAM = &sys->state;
 
     m48t59_reset_common(NVRAM);
@@ -646,47 +688,59 @@ M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
 {
     DeviceState *dev;
     SysBusDevice *s;
-    M48t59SysBusState *d;
+    M48txxSysBusState *d;
     M48t59State *state;
+    int i;
 
-    dev = qdev_create(NULL, TYPE_SYSBUS_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 = SYSBUS_M48T59(dev);
-    state = &d->state;
-    sysbus_connect_irq(s, 0, IRQ);
-    if (io_base != 0) {
-        memory_region_add_subregion(get_system_io(), io_base,
-                                    sysbus_mmio_get_region(dev, 1));
-    }
-    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 = M48TXX_SYS_BUS(s);
+        state = &d->state;
+        sysbus_connect_irq(s, 0, IRQ);
+        if (io_base != 0) {
+            memory_region_add_subregion(get_system_io(), io_base,
+                                        sysbus_mmio_get_region(s, 1));
+        }
+        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,
                              int model)
 {
-    M48t59ISAState *d;
-    ISADevice *isadev;
     DeviceState *dev;
-    M48t59State *s;
-
-    isadev = isa_create(bus, TYPE_ISA_M48T59);
-    dev = DEVICE(isadev);
-    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);
-    d = ISA_M48T59(isadev);
-    s = &d->state;
-
-    return s;
+    int i;
+
+    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 = DEVICE(isa_create(bus, m48txx_info[i].isa_name));
+        qdev_prop_set_uint32(dev, "iobase", io_base);
+        qdev_init_nofail(dev);
+        return &M48TXX_ISA(dev)->state;
+    }
+
+    assert(false);
+    return NULL;
 }
 
 static void m48t59_realize_common(M48t59State *s, Error **errp)
@@ -703,27 +757,31 @@ static void m48t59_realize_common(M48t59State *s, Error **errp)
 
 static void m48t59_isa_realize(DeviceState *dev, Error **errp)
 {
+    M48txxISADeviceClass *u = M48TXX_ISA_GET_CLASS(dev);
     ISADevice *isadev = ISA_DEVICE(dev);
-    M48t59ISAState *d = ISA_M48T59(dev);
+    M48txxISAState *d = M48TXX_ISA(dev);
     M48t59State *s = &d->state;
 
+    s->model = u->info.model;
+    s->size = u->info.size;
     isa_init_irq(isadev, &s->IRQ, 8);
     m48t59_realize_common(s, errp);
     memory_region_init_io(&d->io, OBJECT(dev), &m48t59_io_ops, s, "m48t59", 4);
-    if (s->io_base != 0) {
-        isa_register_ioport(isadev, &d->io, s->io_base);
+    if (d->io_base != 0) {
+        isa_register_ioport(isadev, &d->io, d->io_base);
     }
-
-    return 0;
 }
 
 static int m48t59_init1(SysBusDevice *dev)
 {
-    M48t59SysBusState *d = SYSBUS_M48T59(dev);
+    M48txxSysBusDeviceClass *u = M48TXX_SYS_BUS_GET_CLASS(dev);
+    M48txxSysBusState *d = M48TXX_SYS_BUS(dev);
     Object *o = OBJECT(dev);
     M48t59State *s = &d->state;
     Error *err = NULL;
 
+    s->model = u->info.model;
+    s->size = u->info.size;
     sysbus_init_irq(dev, &s->IRQ);
 
     memory_region_init_io(&s->iomem, o, &nvram_ops, s, "m48t59.nvram",
@@ -741,13 +799,11 @@ static int m48t59_init1(SysBusDevice *dev)
 }
 
 static Property m48t59_isa_properties[] = {
-    DEFINE_PROP_UINT32("size",    M48t59ISAState, state.size,    -1),
-    DEFINE_PROP_UINT32("model",   M48t59ISAState, state.model,   -1),
-    DEFINE_PROP_UINT32("io_base", M48t59ISAState, state.io_base,  0),
+    DEFINE_PROP_UINT32("iobase", M48txxISAState, io_base, 0x74),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static void m48t59_isa_class_init(ObjectClass *klass, void *data)
+static void m48txx_isa_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
@@ -756,41 +812,77 @@ static void m48t59_isa_class_init(ObjectClass *klass, void *data)
     dc->props = m48t59_isa_properties;
 }
 
-static const TypeInfo m48t59_isa_info = {
-    .name          = TYPE_ISA_M48T59,
-    .parent        = TYPE_ISA_DEVICE,
-    .instance_size = sizeof(M48t59ISAState),
-    .class_init    = m48t59_isa_class_init,
-};
+static void m48txx_isa_concrete_class_init(ObjectClass *klass, void *data)
+{
+    M48txxISADeviceClass *u = M48TXX_ISA_CLASS(klass);
+    M48txxInfo *info = data;
 
-static Property m48t59_properties[] = {
-    DEFINE_PROP_UINT32("size",    M48t59SysBusState, state.size,    -1),
-    DEFINE_PROP_UINT32("model",   M48t59SysBusState, state.model,   -1),
-    DEFINE_PROP_UINT32("io_base", M48t59SysBusState, state.io_base,  0),
-    DEFINE_PROP_END_OF_LIST(),
-};
+    u->info = *info;
+}
 
-static void m48t59_class_init(ObjectClass *klass, void *data)
+static void m48txx_sysbus_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
     k->init = m48t59_init1;
     dc->reset = m48t59_reset_sysbus;
-    dc->props = m48t59_properties;
 }
 
-static const TypeInfo m48t59_info = {
-    .name          = TYPE_SYSBUS_M48T59,
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(M48t59SysBusState),
-    .class_init    = m48t59_class_init,
+static void m48txx_sysbus_concrete_class_init(ObjectClass *klass, void *data)
+{
+    M48txxSysBusDeviceClass *u = M48TXX_SYS_BUS_CLASS(klass);
+    M48txxInfo *info = data;
+
+    u->info = *info;
+}
+
+static const TypeInfo m48txx_sysbus_type_info = {
+    .name = TYPE_M48TXX_SYS_BUS,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(M48txxSysBusState),
+    .abstract = true,
+    .class_init = m48txx_sysbus_class_init,
+};
+
+static const TypeInfo m48txx_isa_type_info = {
+    .name = TYPE_M48TXX_ISA,
+    .parent = TYPE_ISA_DEVICE,
+    .instance_size = sizeof(M48txxISAState),
+    .abstract = true,
+    .class_init = m48txx_isa_class_init,
 };
 
 static void m48t59_register_types(void)
 {
-    type_register_static(&m48t59_info);
-    type_register_static(&m48t59_isa_info);
+    TypeInfo sysbus_type_info = {
+        .parent = TYPE_M48TXX_SYS_BUS,
+        .class_size = sizeof(M48txxSysBusDeviceClass),
+        .class_init = m48txx_sysbus_concrete_class_init,
+    };
+    TypeInfo isa_type_info = {
+        .parent = TYPE_M48TXX_ISA,
+        .class_size = sizeof(M48txxISADeviceClass),
+        .class_init = m48txx_isa_concrete_class_init,
+    };
+    int i;
+
+    type_register_static(&m48txx_sysbus_type_info);
+    type_register_static(&m48txx_isa_type_info);
+
+    for (i = 0; i < ARRAY_SIZE(m48txx_info); i++) {
+        if (m48txx_info[i].sysbus_name) {
+            sysbus_type_info.name = m48txx_info[i].sysbus_name;
+            sysbus_type_info.class_data = &m48txx_info[i];
+            type_register(&sysbus_type_info);
+        }
+
+        if (m48txx_info[i].isa_name) {
+            isa_type_info.name = m48txx_info[i].isa_name;
+            isa_type_info.class_data = &m48txx_info[i];
+            type_register(&isa_type_info);
+        }
+    }
 }
 
 type_init(m48t59_register_types)
Signature · 1 line
1.7.10.4
#3 lore raw

PATCH 3/4 m48t59: add a Nvram interface

In reply to Hervé Poussineau
---
 hw/timer/m48t59.c         |   59 +++++++++++++++++++++++++++++++++++++++++++++
 include/hw/timer/m48t59.h |   24 ++++++++++++++++++
 2 files changed, 83 insertions(+)
hw/timer/m48t59.c +59 -0
index c46b63c..c5e74ce 100644
@@ -798,6 +798,24 @@ static int m48t59_init1(SysBusDevice *dev)
     return 0;
 }
 
+static uint32_t m48txx_isa_read(Nvram *obj, uint32_t addr)
+{
+    M48txxISAState *d = M48TXX_ISA(obj);
+    return m48t59_read(&d->state, addr);
+}
+
+static void m48txx_isa_write(Nvram *obj, uint32_t addr, uint32_t val)
+{
+    M48txxISAState *d = M48TXX_ISA(obj);
+    m48t59_write(&d->state, addr, val);
+}
+
+static void m48txx_isa_toggle_lock(Nvram *obj, int lock)
+{
+    M48txxISAState *d = M48TXX_ISA(obj);
+    m48t59_toggle_lock(&d->state, lock);
+}
+
 static Property m48t59_isa_properties[] = {
     DEFINE_PROP_UINT32("iobase", M48txxISAState, io_base, 0x74),
     DEFINE_PROP_END_OF_LIST(),
@@ -806,10 +824,14 @@ static Property m48t59_isa_properties[] = {
 static void m48txx_isa_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    NvramClass *nc = NVRAM_CLASS(klass);
 
     dc->realize = m48t59_isa_realize;
     dc->reset = m48t59_reset_isa;
     dc->props = m48t59_isa_properties;
+    nc->read = m48txx_isa_read;
+    nc->write = m48txx_isa_write;
+    nc->toggle_lock = m48txx_isa_toggle_lock;
 }
 
 static void m48txx_isa_concrete_class_init(ObjectClass *klass, void *data)
@@ -820,13 +842,35 @@ static void m48txx_isa_concrete_class_init(ObjectClass *klass, void *data)
     u->info = *info;
 }
 
+static uint32_t m48txx_sysbus_read(Nvram *obj, uint32_t addr)
+{
+    M48txxSysBusState *d = M48TXX_SYS_BUS(obj);
+    return m48t59_read(&d->state, addr);
+}
+
+static void m48txx_sysbus_write(Nvram *obj, uint32_t addr, uint32_t val)
+{
+    M48txxSysBusState *d = M48TXX_SYS_BUS(obj);
+    m48t59_write(&d->state, addr, val);
+}
+
+static void m48txx_sysbus_toggle_lock(Nvram *obj, int lock)
+{
+    M48txxSysBusState *d = M48TXX_SYS_BUS(obj);
+    m48t59_toggle_lock(&d->state, lock);
+}
+
 static void m48txx_sysbus_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+    NvramClass *nc = NVRAM_CLASS(klass);
 
     k->init = m48t59_init1;
     dc->reset = m48t59_reset_sysbus;
+    nc->read = m48txx_sysbus_read;
+    nc->write = m48txx_sysbus_write;
+    nc->toggle_lock = m48txx_sysbus_toggle_lock;
 }
 
 static void m48txx_sysbus_concrete_class_init(ObjectClass *klass, void *data)
@@ -837,12 +881,22 @@ static void m48txx_sysbus_concrete_class_init(ObjectClass *klass, void *data)
     u->info = *info;
 }
 
+static const TypeInfo nvram_info = {
+    .name = TYPE_NVRAM,
+    .parent = TYPE_INTERFACE,
+    .class_size = sizeof(NvramClass),
+};
+
 static const TypeInfo m48txx_sysbus_type_info = {
     .name = TYPE_M48TXX_SYS_BUS,
     .parent = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(M48txxSysBusState),
     .abstract = true,
     .class_init = m48txx_sysbus_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_NVRAM },
+        { }
+    }
 };
 
 static const TypeInfo m48txx_isa_type_info = {
@@ -851,6 +905,10 @@ static const TypeInfo m48txx_isa_type_info = {
     .instance_size = sizeof(M48txxISAState),
     .abstract = true,
     .class_init = m48txx_isa_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_NVRAM },
+        { }
+    }
 };
 
 static void m48t59_register_types(void)
@@ -867,6 +925,7 @@ static void m48t59_register_types(void)
     };
     int i;
 
+    type_register_static(&nvram_info);
     type_register_static(&m48txx_sysbus_type_info);
     type_register_static(&m48txx_isa_type_info);
 
include/hw/timer/m48t59.h +24 -0
index 8217522..ddbbda2 100644
@@ -1,6 +1,9 @@
 #ifndef NVRAM_H
 #define NVRAM_H
 
+#include "qemu-common.h"
+#include "qom/object.h"
+
 /* NVRAM helpers */
 typedef uint32_t (*nvram_read_t)(void *private, uint32_t addr);
 typedef void (*nvram_write_t)(void *private, uint32_t addr, uint32_t val);
@@ -34,4 +37,25 @@ M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
 M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
                          uint32_t io_base, uint16_t size, int type);
 
+#define TYPE_NVRAM "nvram"
+
+#define NVRAM_CLASS(klass) \
+    OBJECT_CLASS_CHECK(NvramClass, (klass), TYPE_NVRAM)
+#define NVRAM_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(NvramClass, (obj), TYPE_NVRAM)
+#define NVRAM(obj) \
+    INTERFACE_CHECK(Nvram, (obj), TYPE_NVRAM)
+
+typedef struct Nvram {
+    Object parent;
+} Nvram;
+
+typedef struct NvramClass {
+    InterfaceClass parent;
+
+    uint32_t (*read)(Nvram *obj, uint32_t addr);
+    void (*write)(Nvram *obj, uint32_t addr, uint32_t val);
+    void (*toggle_lock)(Nvram *obj, int lock);
+} NvramClass;
+
 #endif /* !NVRAM_H */
Signature · 1 line
1.7.10.4
#4 lore raw

PATCH 4/4 m48t59: let init functions return a Nvram object

In reply to Hervé Poussineau

Remove left-overs from header file.
Move some functions only used by PReP to hw/ppc/prep.c

---
 hw/ppc/ppc.c              |  161 ---------------------------------------------
 hw/ppc/prep.c             |  161 +++++++++++++++++++++++++++++++++++++++++----
 hw/sparc/sun4m.c          |    8 ++-
 hw/sparc64/sun4u.c        |   10 +--
 hw/timer/m48t59.c         |   30 ++++-----
 include/hw/timer/m48t59.h |   38 ++---------
 6 files changed, 177 insertions(+), 231 deletions(-)
hw/ppc/ppc.c +0 -161
index 5ce565d..99db56c 100644
@@ -1318,167 +1318,6 @@ void PPC_debug_write (void *opaque, uint32_t addr, uint32_t val)
     }
 }
 
-/*****************************************************************************/
-/* NVRAM helpers */
-static inline uint32_t nvram_read (nvram_t *nvram, uint32_t addr)
-{
-    return (*nvram->read_fn)(nvram->opaque, addr);
-}
-
-static inline void nvram_write (nvram_t *nvram, uint32_t addr, uint32_t val)
-{
-    (*nvram->write_fn)(nvram->opaque, addr, val);
-}
-
-static void NVRAM_set_byte(nvram_t *nvram, uint32_t addr, uint8_t value)
-{
-    nvram_write(nvram, addr, value);
-}
-
-static uint8_t NVRAM_get_byte(nvram_t *nvram, uint32_t addr)
-{
-    return nvram_read(nvram, addr);
-}
-
-static void NVRAM_set_word(nvram_t *nvram, uint32_t addr, uint16_t value)
-{
-    nvram_write(nvram, addr, value >> 8);
-    nvram_write(nvram, addr + 1, value & 0xFF);
-}
-
-static uint16_t NVRAM_get_word(nvram_t *nvram, uint32_t addr)
-{
-    uint16_t tmp;
-
-    tmp = nvram_read(nvram, addr) << 8;
-    tmp |= nvram_read(nvram, addr + 1);
-
-    return tmp;
-}
-
-static void NVRAM_set_lword(nvram_t *nvram, uint32_t addr, uint32_t value)
-{
-    nvram_write(nvram, addr, value >> 24);
-    nvram_write(nvram, addr + 1, (value >> 16) & 0xFF);
-    nvram_write(nvram, addr + 2, (value >> 8) & 0xFF);
-    nvram_write(nvram, addr + 3, value & 0xFF);
-}
-
-uint32_t NVRAM_get_lword (nvram_t *nvram, uint32_t addr)
-{
-    uint32_t tmp;
-
-    tmp = nvram_read(nvram, addr) << 24;
-    tmp |= nvram_read(nvram, addr + 1) << 16;
-    tmp |= nvram_read(nvram, addr + 2) << 8;
-    tmp |= nvram_read(nvram, addr + 3);
-
-    return tmp;
-}
-
-static void NVRAM_set_string(nvram_t *nvram, uint32_t addr, const char *str,
-                             uint32_t max)
-{
-    int i;
-
-    for (i = 0; i < max && str[i] != '\0'; i++) {
-        nvram_write(nvram, addr + i, str[i]);
-    }
-    nvram_write(nvram, addr + i, str[i]);
-    nvram_write(nvram, addr + max - 1, '\0');
-}
-
-int NVRAM_get_string (nvram_t *nvram, uint8_t *dst, uint16_t addr, int max)
-{
-    int i;
-
-    memset(dst, 0, max);
-    for (i = 0; i < max; i++) {
-        dst[i] = NVRAM_get_byte(nvram, addr + i);
-        if (dst[i] == '\0')
-            break;
-    }
-
-    return i;
-}
-
-static uint16_t NVRAM_crc_update (uint16_t prev, uint16_t value)
-{
-    uint16_t tmp;
-    uint16_t pd, pd1, pd2;
-
-    tmp = prev >> 8;
-    pd = prev ^ value;
-    pd1 = pd & 0x000F;
-    pd2 = ((pd >> 4) & 0x000F) ^ pd1;
-    tmp ^= (pd1 << 3) | (pd1 << 8);
-    tmp ^= pd2 | (pd2 << 7) | (pd2 << 12);
-
-    return tmp;
-}
-
-static uint16_t NVRAM_compute_crc (nvram_t *nvram, uint32_t start, uint32_t count)
-{
-    uint32_t i;
-    uint16_t crc = 0xFFFF;
-    int odd;
-
-    odd = count & 1;
-    count &= ~1;
-    for (i = 0; i != count; i++) {
-        crc = NVRAM_crc_update(crc, NVRAM_get_word(nvram, start + i));
-    }
-    if (odd) {
-        crc = NVRAM_crc_update(crc, NVRAM_get_byte(nvram, start + i) << 8);
-    }
-
-    return crc;
-}
-
-#define CMDLINE_ADDR 0x017ff000
-
-int PPC_NVRAM_set_params (nvram_t *nvram, uint16_t NVRAM_size,
-                          const char *arch,
-                          uint32_t RAM_size, int boot_device,
-                          uint32_t kernel_image, uint32_t kernel_size,
-                          const char *cmdline,
-                          uint32_t initrd_image, uint32_t initrd_size,
-                          uint32_t NVRAM_image,
-                          int width, int height, int depth)
-{
-    uint16_t crc;
-
-    /* Set parameters for Open Hack'Ware BIOS */
-    NVRAM_set_string(nvram, 0x00, "QEMU_BIOS", 16);
-    NVRAM_set_lword(nvram,  0x10, 0x00000002); /* structure v2 */
-    NVRAM_set_word(nvram,   0x14, NVRAM_size);
-    NVRAM_set_string(nvram, 0x20, arch, 16);
-    NVRAM_set_lword(nvram,  0x30, RAM_size);
-    NVRAM_set_byte(nvram,   0x34, boot_device);
-    NVRAM_set_lword(nvram,  0x38, kernel_image);
-    NVRAM_set_lword(nvram,  0x3C, kernel_size);
-    if (cmdline) {
-        /* XXX: put the cmdline in NVRAM too ? */
-        pstrcpy_targphys("cmdline", CMDLINE_ADDR, RAM_size - CMDLINE_ADDR, cmdline);
-        NVRAM_set_lword(nvram,  0x40, CMDLINE_ADDR);
-        NVRAM_set_lword(nvram,  0x44, strlen(cmdline));
-    } else {
-        NVRAM_set_lword(nvram,  0x40, 0);
-        NVRAM_set_lword(nvram,  0x44, 0);
-    }
-    NVRAM_set_lword(nvram,  0x48, initrd_image);
-    NVRAM_set_lword(nvram,  0x4C, initrd_size);
-    NVRAM_set_lword(nvram,  0x50, NVRAM_image);
-
-    NVRAM_set_word(nvram,   0x54, width);
-    NVRAM_set_word(nvram,   0x56, height);
-    NVRAM_set_word(nvram,   0x58, depth);
-    crc = NVRAM_compute_crc(nvram, 0x00, 0xF8);
-    NVRAM_set_word(nvram,   0xFC, crc);
-
-    return 0;
-}
-
 /* CPU device-tree ID helpers */
 int ppc_get_vcpu_dt_id(PowerPCCPU *cpu)
 {
hw/ppc/prep.c +150 -11

194 lines not on this page · raw message

hw/sparc/sun4m.c +5 -3
index ecd9dc1..5ff7c5b 100644
@@ -127,7 +127,7 @@ static void fw_cfg_boot_set(void *opaque, const char *boot_device,
     fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
 }
 
-static void nvram_init(M48t59State *nvram, uint8_t *macaddr,
+static void nvram_init(Nvram *nvram, uint8_t *macaddr,
                        const char *cmdline, const char *boot_devices,
                        ram_addr_t RAM_size, uint32_t kernel_size,
                        int width, int height, int depth,
@@ -137,6 +137,7 @@ static void nvram_init(M48t59State *nvram, uint8_t *macaddr,
     uint32_t start, end;
     uint8_t image[0x1ff0];
     struct OpenBIOS_nvpart_v1 *part_header;
+    NvramClass *k = NVRAM_GET_CLASS(nvram);
 
     memset(image, '\0', sizeof(image));
 
@@ -170,8 +171,9 @@ static void nvram_init(M48t59State *nvram, uint8_t *macaddr,
     Sun_init_header((struct Sun_nvram *)&image[0x1fd8], macaddr,
                     nvram_machine_id);
 
-    for (i = 0; i < sizeof(image); i++)
-        m48t59_write(nvram, i, image[i]);
+    for (i = 0; i < sizeof(image); i++) {
+        (k->write)(nvram, i, image[i]);
+    }
 }
 
 static DeviceState *slavio_intctl;
hw/sparc64/sun4u.c +6 -4
index 3ff5bd8..0cb6a06 100644
@@ -130,7 +130,7 @@ static void fw_cfg_boot_set(void *opaque, const char *boot_device,
     fw_cfg_add_i16(opaque, FW_CFG_BOOT_DEVICE, boot_device[0]);
 }
 
-static int sun4u_NVRAM_set_params(M48t59State *nvram, uint16_t NVRAM_size,
+static int sun4u_NVRAM_set_params(Nvram *nvram, uint16_t NVRAM_size,
                                   const char *arch, ram_addr_t RAM_size,
                                   const char *boot_devices,
                                   uint32_t kernel_image, uint32_t kernel_size,
@@ -144,6 +144,7 @@ static int sun4u_NVRAM_set_params(M48t59State *nvram, uint16_t NVRAM_size,
     uint32_t start, end;
     uint8_t image[0x1ff0];
     struct OpenBIOS_nvpart_v1 *part_header;
+    NvramClass *k = NVRAM_GET_CLASS(nvram);
 
     memset(image, '\0', sizeof(image));
 
@@ -176,8 +177,9 @@ static int sun4u_NVRAM_set_params(M48t59State *nvram, uint16_t NVRAM_size,
 
     Sun_init_header((struct Sun_nvram *)&image[0x1fd8], macaddr, 0x80);
 
-    for (i = 0; i < sizeof(image); i++)
-        m48t59_write(nvram, i, image[i]);
+    for (i = 0; i < sizeof(image); i++) {
+        (k->write)(nvram, i, image[i]);
+    }
 
     return 0;
 }
@@ -817,7 +819,7 @@ static void sun4uv_init(MemoryRegion *address_space_mem,
                         const struct hwdef *hwdef)
 {
     SPARCCPU *cpu;
-    M48t59State *nvram;
+    Nvram *nvram;
     unsigned int i;
     uint64_t initrd_addr, initrd_size, kernel_addr, kernel_size, kernel_entry;
     PCIBus *pci_bus, *pci_bus2, *pci_bus3;
hw/timer/m48t59.c +11 -19
index c5e74ce..08f0d57 100644
@@ -74,7 +74,7 @@ typedef struct M48txxInfo {
  * http://www.st.com/stonline/products/literature/od/7001/m48t59y.pdf
  */
 
-struct M48t59State {
+typedef struct M48t59State {
     /* Hardware parameters */
     qemu_irq IRQ;
     MemoryRegion iomem;
@@ -93,7 +93,7 @@ struct M48t59State {
     /* NVRAM storage */
     uint16_t addr;
     uint8_t  lock;
-};
+} M48t59State;
 
 typedef struct M48txxISAState {
     ISADevice parent_obj;
@@ -240,9 +240,8 @@ static void set_up_watchdog(M48t59State *NVRAM, uint8_t value)
 }
 
 /* Direct access to NVRAM */
-void m48t59_write (void *opaque, uint32_t addr, uint32_t val)
+static void m48t59_write(M48t59State *NVRAM, uint32_t addr, uint32_t val)
 {
-    M48t59State *NVRAM = opaque;
     struct tm tm;
     int tmp;
 
@@ -410,9 +409,8 @@ void m48t59_write (void *opaque, uint32_t addr, uint32_t val)
     }
 }
 
-uint32_t m48t59_read (void *opaque, uint32_t addr)
+static uint32_t m48t59_read(M48t59State *NVRAM, uint32_t addr)
 {
-    M48t59State *NVRAM = opaque;
     struct tm tm;
     uint32_t retval = 0xFF;
 
@@ -519,10 +517,8 @@ uint32_t m48t59_read (void *opaque, uint32_t addr)
     return retval;
 }
 
-void m48t59_toggle_lock (void *opaque, int lock)
+static void m48t59_toggle_lock(M48t59State *NVRAM, int lock)
 {
-    M48t59State *NVRAM = opaque;
-
     NVRAM->lock ^= 1 << lock;
 }
 
@@ -683,13 +679,11 @@ static const MemoryRegionOps m48t59_io_ops = {
 };
 
 /* Initialisation routine */
-M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
-                         uint32_t io_base, uint16_t size, int model)
+Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
+                   uint32_t io_base, uint16_t size, int model)
 {
     DeviceState *dev;
     SysBusDevice *s;
-    M48txxSysBusState *d;
-    M48t59State *state;
     int i;
 
     for (i = 0; i < ARRAY_SIZE(m48txx_info); i++) {
@@ -702,8 +696,6 @@ M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
         dev = qdev_create(NULL, m48txx_info[i].sysbus_name);
         qdev_init_nofail(dev);
         s = SYS_BUS_DEVICE(dev);
-        d = M48TXX_SYS_BUS(s);
-        state = &d->state;
         sysbus_connect_irq(s, 0, IRQ);
         if (io_base != 0) {
             memory_region_add_subregion(get_system_io(), io_base,
@@ -713,15 +705,15 @@ M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
             sysbus_mmio_map(s, 0, mem_base);
         }
 
-        return state;
+        return NVRAM(s);
     }
 
     assert(false);
     return NULL;
 }
 
-M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
-                             int model)
+Nvram *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
+                       int model)
 {
     DeviceState *dev;
     int i;
@@ -736,7 +728,7 @@ M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
         dev = DEVICE(isa_create(bus, m48txx_info[i].isa_name));
         qdev_prop_set_uint32(dev, "iobase", io_base);
         qdev_init_nofail(dev);
-        return &M48TXX_ISA(dev)->state;
+        return NVRAM(dev);
     }
 
     assert(false);
include/hw/timer/m48t59.h +5 -33
index ddbbda2..cf80d20 100644
@@ -4,39 +4,6 @@
 #include "qemu-common.h"
 #include "qom/object.h"
 
-/* NVRAM helpers */
-typedef uint32_t (*nvram_read_t)(void *private, uint32_t addr);
-typedef void (*nvram_write_t)(void *private, uint32_t addr, uint32_t val);
-typedef struct nvram_t {
-    void *opaque;
-    nvram_read_t read_fn;
-    nvram_write_t write_fn;
-} nvram_t;
-
-uint32_t NVRAM_get_lword (nvram_t *nvram, uint32_t addr);
-int NVRAM_get_string (nvram_t *nvram, uint8_t *dst, uint16_t addr, int max);
-
-int PPC_NVRAM_set_params (nvram_t *nvram, uint16_t NVRAM_size,
-                          const char *arch,
-                          uint32_t RAM_size, int boot_device,
-                          uint32_t kernel_image, uint32_t kernel_size,
-                          const char *cmdline,
-                          uint32_t initrd_image, uint32_t initrd_size,
-                          uint32_t NVRAM_image,
-                          int width, int height, int depth);
-
-#define TYPE_SYSBUS_M48T59 "m48t59"
-
-typedef struct M48t59State M48t59State;
-
-void m48t59_write (void *private, uint32_t addr, uint32_t val);
-uint32_t m48t59_read (void *private, uint32_t addr);
-void m48t59_toggle_lock (void *private, int lock);
-M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
-                             int type);
-M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
-                         uint32_t io_base, uint16_t size, int type);
-
 #define TYPE_NVRAM "nvram"
 
 #define NVRAM_CLASS(klass) \
@@ -58,4 +25,9 @@ typedef struct NvramClass {
     void (*toggle_lock)(Nvram *obj, int lock);
 } NvramClass;
 
+Nvram *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
+                       int type);
+Nvram *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
+                   uint32_t io_base, uint16_t size, int type);
+
 #endif /* !NVRAM_H */
Signature · 1 line
1.7.10.4
Tested-by on the v1 series
#5 lore raw
In reply to Hervé Poussineau
Quoted 18 lines · Hervé Poussineau

Hi,

This patchset QOM'ifies the m48t59 timer.
Patch 1 allows the creation of the m4859 ISA device from the command line. Patch 2 QOM'ifies the device.
Patches 3 add a Nvram interface to m48txx devices, so that callers can detect the device and use it without knowing m48txx internals. Patch 4 is using this new Nvram interface and cleaning the m48t59 header.

Patches 3 and 4 are not needed for m48t59 QOM'ification, but are in my opinion a nice cleanup.

Hervé Poussineau (4):
m48t59: move ISA ports/memory regions registration to QOM constructor
m48t59: register a QOM type for each nvram type we support
m48t59: add a Nvram interface
m48t59: let init functions return a Nvram object

Quoted 7 lines
 hw/ppc/ppc.c              |  161 ----------------------
 hw/ppc/prep.c             |  161 ++++++++++++++++++++--
 hw/sparc/sun4m.c          |    8 +-
 hw/sparc64/sun4u.c        |   10 +-
 hw/timer/m48t59.c         |  336 ++++++++++++++++++++++++++++++++-------------
 include/hw/timer/m48t59.h |   60 ++++----
 6 files changed, 429 insertions(+), 307 deletions(-)

I've tested this in conjunction with a rebased version of my sun4u NVRAM changes for SPARC32 and SPARC64 and it looks good so:

I'll post a new version of my sun4u NVRAM changes rebased on your two patchsets shortly.

ATB,

Mark.

#6 lore raw
Quoted 28 lines · Mark Cave-Ayland
Quoted 18 lines · Hervé Poussineau

Hi,

This patchset QOM'ifies the m48t59 timer.
Patch 1 allows the creation of the m4859 ISA device from the command line. Patch 2 QOM'ifies the device.
Patches 3 add a Nvram interface to m48txx devices, so that callers can detect the device and use it without knowing m48txx internals. Patch 4 is using this new Nvram interface and cleaning the m48t59 header.

Patches 3 and 4 are not needed for m48t59 QOM'ification, but are in my opinion a nice cleanup.

Hervé Poussineau (4):
m48t59: move ISA ports/memory regions registration to QOM constructor
m48t59: register a QOM type for each nvram type we support
m48t59: add a Nvram interface
m48t59: let init functions return a Nvram object

Quoted 7 lines
 hw/ppc/ppc.c              |  161 ----------------------
 hw/ppc/prep.c             |  161 ++++++++++++++++++++--
 hw/sparc/sun4m.c          |    8 +-
 hw/sparc64/sun4u.c        |   10 +-
 hw/timer/m48t59.c         |  336 ++++++++++++++++++++++++++++++++-------------
 include/hw/timer/m48t59.h |   60 ++++----
 6 files changed, 429 insertions(+), 307 deletions(-)
Quoted 7 lines

I've tested this in conjunction with a rebased version of my sun4u NVRAM changes for SPARC32 and SPARC64 and it looks good so:

I'll post a new version of my sun4u NVRAM changes rebased on your two patchsets shortly.

Andreas, I suppose this one should go via your tree? It's quite prep specific.

Alex