[Qemu-devel] [PATCH v2 0/5] pxa2xx: minor bugfixes, updates to QOM, etc

[Qemu-devel] [PATCH v2 0/5] pxa2xx: minor bugfixes, updates to QOM, etc

[Peter Maydell]

These patches update some ancient PXA2xx devices to use
QOM, VMState and new MemoryRegionOps APIs. The main motivation
here was to get rid of a few of uses of legacy APIs
like register_savevm() and old_mmio, in the hope that eventually
we can manage to get rid of them completely...

v1->v2 changes:
 * patch 2: add missing sysbus_init_irq calls
 * patch 4: use ARRAY_SIZE rather than sizeof

Peter Maydell (5):
  hw/arm/pxa2xx: Mark coprocessor registers as ARM_CP_IO
  hw/arm/pxa2xx: Convert pxa2xx-fir to QOM and VMState
  hw/arm/pxa2xx: Add reset method for pxa2xx_ssp
  hw/arm/pxa2xx: Convert pxa2xx-ssp to VMState
  hw/sd/pxa2xx_mmci: Stop using old_mmio in MemoryRegionOps

 hw/arm/pxa2xx.c     | 247 +++++++++++++++++++++++++++++-----------------------
 hw/arm/pxa2xx_pic.c |   2 +-
 hw/sd/pxa2xx_mmci.c |  68 ++-------------
 3 files changed, 145 insertions(+), 172 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v2 1/5] hw/arm/pxa2xx: Mark coprocessor registers as ARM_CP_IO

[Peter Maydell]

The pxa2xx custom coprocessor registers in cp6 and cp14 do device
accesses, so mark the non-constant regs as ARM_CP_IO so that
icount works correctly and doesn't abort.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 hw/arm/pxa2xx.c     | 8 ++++----
 hw/arm/pxa2xx_pic.c | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
index f921a56..8123f05 100644
--- a/hw/arm/pxa2xx.c
+++ b/hw/arm/pxa2xx.c
@@ -334,10 +334,10 @@ static uint64_t pxa2xx_cpccnt_read(CPUARMState *env, const ARMCPRegInfo *ri)
 static const ARMCPRegInfo pxa_cp_reginfo[] = {
     /* cp14 crm==1: perf registers */
     { .name = "CPPMNC", .cp = 14, .crn = 0, .crm = 1, .opc1 = 0, .opc2 = 0,
-      .access = PL1_RW,
+      .access = PL1_RW, .type = ARM_CP_IO,
       .readfn = pxa2xx_cppmnc_read, .writefn = pxa2xx_cppmnc_write },
     { .name = "CPCCNT", .cp = 14, .crn = 1, .crm = 1, .opc1 = 0, .opc2 = 0,
-      .access = PL1_RW,
+      .access = PL1_RW, .type = ARM_CP_IO,
       .readfn = pxa2xx_cpccnt_read, .writefn = arm_cp_write_ignore },
     { .name = "CPINTEN", .cp = 14, .crn = 4, .crm = 1, .opc1 = 0, .opc2 = 0,
       .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
@@ -356,11 +356,11 @@ static const ARMCPRegInfo pxa_cp_reginfo[] = {
       .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
     /* cp14 crn==6: CLKCFG */
     { .name = "CLKCFG", .cp = 14, .crn = 6, .crm = 0, .opc1 = 0, .opc2 = 0,
-      .access = PL1_RW,
+      .access = PL1_RW, .type = ARM_CP_IO,
       .readfn = pxa2xx_clkcfg_read, .writefn = pxa2xx_clkcfg_write },
     /* cp14 crn==7: PWRMODE */
     { .name = "PWRMODE", .cp = 14, .crn = 7, .crm = 0, .opc1 = 0, .opc2 = 0,
-      .access = PL1_RW,
+      .access = PL1_RW, .type = ARM_CP_IO,
       .readfn = arm_cp_read_zero, .writefn = pxa2xx_pwrmode_write },
     REGINFO_SENTINEL
 };
diff --git a/hw/arm/pxa2xx_pic.c b/hw/arm/pxa2xx_pic.c
index 9cfc714..d41ac93 100644
--- a/hw/arm/pxa2xx_pic.c
+++ b/hw/arm/pxa2xx_pic.c
@@ -232,7 +232,7 @@ static void pxa2xx_pic_cp_write(CPUARMState *env, const ARMCPRegInfo *ri,
 
 #define REGINFO_FOR_PIC_CP(NAME, CRN) \
     { .name = NAME, .cp = 6, .crn = CRN, .crm = 0, .opc1 = 0, .opc2 = 0, \
-      .access = PL1_RW, \
+      .access = PL1_RW, .type = ARM_CP_IO, \
       .readfn = pxa2xx_pic_cp_read, .writefn = pxa2xx_pic_cp_write }
 
 static const ARMCPRegInfo pxa_pic_cp_reginfo[] = {
-- 
1.9.1

[Qemu-devel] [PATCH v2 2/5] hw/arm/pxa2xx: Convert pxa2xx-fir to QOM and VMState

[Peter Maydell]

Convert the pxa2xx-fir device to QOM, including using a
VMState for its migration info.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/pxa2xx.c | 140 ++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 85 insertions(+), 55 deletions(-)

diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
index 8123f05..6f80496 100644
--- a/hw/arm/pxa2xx.c
+++ b/hw/arm/pxa2xx.c
@@ -1759,24 +1759,33 @@ static PXA2xxI2SState *pxa2xx_i2s_init(MemoryRegion *sysmem,
 }
 
 /* PXA Fast Infra-red Communications Port */
+#define TYPE_PXA2XX_FIR "pxa2xx-fir"
+#define PXA2XX_FIR(obj) OBJECT_CHECK(PXA2xxFIrState, (obj), TYPE_PXA2XX_FIR)
+
 struct PXA2xxFIrState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
     MemoryRegion iomem;
     qemu_irq irq;
     qemu_irq rx_dma;
     qemu_irq tx_dma;
-    int enable;
+    uint32_t enable;
     CharDriverState *chr;
 
     uint8_t control[3];
     uint8_t status[2];
 
-    int rx_len;
-    int rx_start;
+    uint32_t rx_len;
+    uint32_t rx_start;
     uint8_t rx_fifo[64];
 };
 
-static void pxa2xx_fir_reset(PXA2xxFIrState *s)
+static void pxa2xx_fir_reset(DeviceState *d)
 {
+    PXA2xxFIrState *s = PXA2XX_FIR(d);
+
     s->control[0] = 0x00;
     s->control[1] = 0x00;
     s->control[2] = 0x00;
@@ -1953,73 +1962,94 @@ static void pxa2xx_fir_event(void *opaque, int event)
 {
 }
 
-static void pxa2xx_fir_save(QEMUFile *f, void *opaque)
+static void pxa2xx_fir_instance_init(Object *obj)
 {
-    PXA2xxFIrState *s = (PXA2xxFIrState *) opaque;
-    int i;
-
-    qemu_put_be32(f, s->enable);
-
-    qemu_put_8s(f, &s->control[0]);
-    qemu_put_8s(f, &s->control[1]);
-    qemu_put_8s(f, &s->control[2]);
-    qemu_put_8s(f, &s->status[0]);
-    qemu_put_8s(f, &s->status[1]);
+    PXA2xxFIrState *s = PXA2XX_FIR(obj);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 
-    qemu_put_byte(f, s->rx_len);
-    for (i = 0; i < s->rx_len; i ++)
-        qemu_put_byte(f, s->rx_fifo[(s->rx_start + i) & 63]);
+    memory_region_init_io(&s->iomem, NULL, &pxa2xx_fir_ops, s,
+                          "pxa2xx-fir", 0x1000);
+    sysbus_init_mmio(sbd, &s->iomem);
+    sysbus_init_irq(sbd, &s->irq);
+    sysbus_init_irq(sbd, &s->rx_dma);
+    sysbus_init_irq(sbd, &s->tx_dma);
 }
 
-static int pxa2xx_fir_load(QEMUFile *f, void *opaque, int version_id)
+static void pxa2xx_fir_realize(DeviceState *dev, Error **errp)
 {
-    PXA2xxFIrState *s = (PXA2xxFIrState *) opaque;
-    int i;
+    PXA2xxFIrState *s = PXA2XX_FIR(dev);
 
-    s->enable = qemu_get_be32(f);
-
-    qemu_get_8s(f, &s->control[0]);
-    qemu_get_8s(f, &s->control[1]);
-    qemu_get_8s(f, &s->control[2]);
-    qemu_get_8s(f, &s->status[0]);
-    qemu_get_8s(f, &s->status[1]);
+    if (s->chr) {
+        qemu_chr_fe_claim_no_fail(s->chr);
+        qemu_chr_add_handlers(s->chr, pxa2xx_fir_is_empty,
+                        pxa2xx_fir_rx, pxa2xx_fir_event, s);
+    }
+}
 
-    s->rx_len = qemu_get_byte(f);
-    s->rx_start = 0;
-    for (i = 0; i < s->rx_len; i ++)
-        s->rx_fifo[i] = qemu_get_byte(f);
+static bool pxa2xx_fir_vmstate_validate(void *opaque, int version_id)
+{
+    PXA2xxFIrState *s = opaque;
 
-    return 0;
+    return s->rx_start < sizeof(s->rx_fifo);
 }
 
-static PXA2xxFIrState *pxa2xx_fir_init(MemoryRegion *sysmem,
-                hwaddr base,
-                qemu_irq irq, qemu_irq rx_dma, qemu_irq tx_dma,
-                CharDriverState *chr)
-{
-    PXA2xxFIrState *s = (PXA2xxFIrState *)
-            g_malloc0(sizeof(PXA2xxFIrState));
+static const VMStateDescription pxa2xx_fir_vmsd = {
+    .name = "pxa2xx-fir",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(enable, PXA2xxFIrState),
+        VMSTATE_UINT8_ARRAY(control, PXA2xxFIrState, 3),
+        VMSTATE_UINT8_ARRAY(status, PXA2xxFIrState, 2),
+        VMSTATE_UINT32(rx_len, PXA2xxFIrState),
+        VMSTATE_UINT32(rx_start, PXA2xxFIrState),
+        VMSTATE_VALIDATE("fifo is 64 bytes", pxa2xx_fir_vmstate_validate),
+        VMSTATE_UINT8_ARRAY(rx_fifo, PXA2xxFIrState, 64),
+        VMSTATE_END_OF_LIST()
+    }
+};
 
-    s->irq = irq;
-    s->rx_dma = rx_dma;
-    s->tx_dma = tx_dma;
-    s->chr = chr;
+static Property pxa2xx_fir_properties[] = {
+    DEFINE_PROP_CHR("chardev", PXA2xxFIrState, chr),
+    DEFINE_PROP_END_OF_LIST(),
+};
 
-    pxa2xx_fir_reset(s);
+static void pxa2xx_fir_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
 
-    memory_region_init_io(&s->iomem, NULL, &pxa2xx_fir_ops, s, "pxa2xx-fir", 0x1000);
-    memory_region_add_subregion(sysmem, base, &s->iomem);
+    dc->realize = pxa2xx_fir_realize;
+    dc->vmsd = &pxa2xx_fir_vmsd;
+    dc->props = pxa2xx_fir_properties;
+    dc->reset = pxa2xx_fir_reset;
+}
 
-    if (chr) {
-        qemu_chr_fe_claim_no_fail(chr);
-        qemu_chr_add_handlers(chr, pxa2xx_fir_is_empty,
-                        pxa2xx_fir_rx, pxa2xx_fir_event, s);
-    }
+static const TypeInfo pxa2xx_fir_info = {
+    .name = TYPE_PXA2XX_FIR,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(PXA2xxFIrState),
+    .class_init = pxa2xx_fir_class_init,
+    .instance_init = pxa2xx_fir_instance_init,
+};
 
-    register_savevm(NULL, "pxa2xx_fir", 0, 0, pxa2xx_fir_save,
-                    pxa2xx_fir_load, s);
+static PXA2xxFIrState *pxa2xx_fir_init(MemoryRegion *sysmem,
+                                       hwaddr base,
+                                       qemu_irq irq, qemu_irq rx_dma,
+                                       qemu_irq tx_dma,
+                                       CharDriverState *chr)
+{
+    DeviceState *dev;
+    SysBusDevice *sbd;
 
-    return s;
+    dev = qdev_create(NULL, TYPE_PXA2XX_FIR);
+    qdev_prop_set_chr(dev, "chardev", chr);
+    qdev_init_nofail(dev);
+    sbd = SYS_BUS_DEVICE(dev);
+    sysbus_mmio_map(sbd, 0, base);
+    sysbus_connect_irq(sbd, 0, irq);
+    sysbus_connect_irq(sbd, 1, rx_dma);
+    sysbus_connect_irq(sbd, 2, tx_dma);
+    return PXA2XX_FIR(dev);
 }
 
 static void pxa2xx_reset(void *opaque, int line, int level)
-- 
1.9.1

[Qemu-devel] [PATCH v2 3/5] hw/arm/pxa2xx: Add reset method for pxa2xx_ssp

[Peter Maydell]

The pxa2xx_ssp device was missing a reset method; add one.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Peter Crosthwaite <peter..crosthwaite@xilinx.com>
---
 hw/arm/pxa2xx.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
index 6f80496..59bc0a6 100644
--- a/hw/arm/pxa2xx.c
+++ b/hw/arm/pxa2xx.c
@@ -756,6 +756,22 @@ static int pxa2xx_ssp_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
+static void pxa2xx_ssp_reset(DeviceState *d)
+{
+    PXA2xxSSPState *s = PXA2XX_SSP(d);
+
+    s->enable = 0;
+    s->sscr[0] = s->sscr[1] = 0;
+    s->sspsp = 0;
+    s->ssto = 0;
+    s->ssitr = 0;
+    s->sssr = 0;
+    s->sstsa = 0;
+    s->ssrsa = 0;
+    s->ssacd = 0;
+    s->rx_start = s->rx_level = 0;
+}
+
 static int pxa2xx_ssp_init(SysBusDevice *sbd)
 {
     DeviceState *dev = DEVICE(sbd);
@@ -2336,8 +2352,10 @@ PXA2xxState *pxa255_init(MemoryRegion *address_space, unsigned int sdram_size)
 static void pxa2xx_ssp_class_init(ObjectClass *klass, void *data)
 {
     SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
 
     sdc->init = pxa2xx_ssp_init;
+    dc->reset = pxa2xx_ssp_reset;
 }
 
 static const TypeInfo pxa2xx_ssp_info = {
-- 
1.9.1

[Qemu-devel] [PATCH v2 4/5] hw/arm/pxa2xx: Convert pxa2xx-ssp to VMState

[Peter Maydell]

The pxa2xx-ssp device is already a QOM device but is still
using the old-style register_savevm(); convert to VMState.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 hw/arm/pxa2xx.c | 91 +++++++++++++++++++++------------------------------------
 1 file changed, 34 insertions(+), 57 deletions(-)

diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
index 59bc0a6..31a1044 100644
--- a/hw/arm/pxa2xx.c
+++ b/hw/arm/pxa2xx.c
@@ -457,7 +457,7 @@ typedef struct {
 
     MemoryRegion iomem;
     qemu_irq irq;
-    int enable;
+    uint32_t enable;
     SSIBus *bus;
 
     uint32_t sscr[2];
@@ -470,10 +470,39 @@ typedef struct {
     uint8_t ssacd;
 
     uint32_t rx_fifo[16];
-    int rx_level;
-    int rx_start;
+    uint32_t rx_level;
+    uint32_t rx_start;
 } PXA2xxSSPState;
 
+static bool pxa2xx_ssp_vmstate_validate(void *opaque, int version_id)
+{
+    PXA2xxSSPState *s = opaque;
+
+    return s->rx_start < sizeof(s->rx_fifo);
+}
+
+static const VMStateDescription vmstate_pxa2xx_ssp = {
+    .name = "pxa2xx-ssp",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(enable, PXA2xxSSPState),
+        VMSTATE_UINT32_ARRAY(sscr, PXA2xxSSPState, 2),
+        VMSTATE_UINT32(sspsp, PXA2xxSSPState),
+        VMSTATE_UINT32(ssto, PXA2xxSSPState),
+        VMSTATE_UINT32(ssitr, PXA2xxSSPState),
+        VMSTATE_UINT32(sssr, PXA2xxSSPState),
+        VMSTATE_UINT8(sstsa, PXA2xxSSPState),
+        VMSTATE_UINT8(ssrsa, PXA2xxSSPState),
+        VMSTATE_UINT8(ssacd, PXA2xxSSPState),
+        VMSTATE_UINT32(rx_level, PXA2xxSSPState),
+        VMSTATE_UINT32(rx_start, PXA2xxSSPState),
+        VMSTATE_VALIDATE("fifo is 16 bytes", pxa2xx_ssp_vmstate_validate),
+        VMSTATE_UINT32_ARRAY(rx_fifo, PXA2xxSSPState, 16),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 #define SSCR0	0x00	/* SSP Control register 0 */
 #define SSCR1	0x04	/* SSP Control register 1 */
 #define SSSR	0x08	/* SSP Status register */
@@ -705,57 +734,6 @@ static const MemoryRegionOps pxa2xx_ssp_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void pxa2xx_ssp_save(QEMUFile *f, void *opaque)
-{
-    PXA2xxSSPState *s = (PXA2xxSSPState *) opaque;
-    int i;
-
-    qemu_put_be32(f, s->enable);
-
-    qemu_put_be32s(f, &s->sscr[0]);
-    qemu_put_be32s(f, &s->sscr[1]);
-    qemu_put_be32s(f, &s->sspsp);
-    qemu_put_be32s(f, &s->ssto);
-    qemu_put_be32s(f, &s->ssitr);
-    qemu_put_be32s(f, &s->sssr);
-    qemu_put_8s(f, &s->sstsa);
-    qemu_put_8s(f, &s->ssrsa);
-    qemu_put_8s(f, &s->ssacd);
-
-    qemu_put_byte(f, s->rx_level);
-    for (i = 0; i < s->rx_level; i ++)
-        qemu_put_byte(f, s->rx_fifo[(s->rx_start + i) & 0xf]);
-}
-
-static int pxa2xx_ssp_load(QEMUFile *f, void *opaque, int version_id)
-{
-    PXA2xxSSPState *s = (PXA2xxSSPState *) opaque;
-    int i, v;
-
-    s->enable = qemu_get_be32(f);
-
-    qemu_get_be32s(f, &s->sscr[0]);
-    qemu_get_be32s(f, &s->sscr[1]);
-    qemu_get_be32s(f, &s->sspsp);
-    qemu_get_be32s(f, &s->ssto);
-    qemu_get_be32s(f, &s->ssitr);
-    qemu_get_be32s(f, &s->sssr);
-    qemu_get_8s(f, &s->sstsa);
-    qemu_get_8s(f, &s->ssrsa);
-    qemu_get_8s(f, &s->ssacd);
-
-    v = qemu_get_byte(f);
-    if (v < 0 || v > ARRAY_SIZE(s->rx_fifo)) {
-        return -EINVAL;
-    }
-    s->rx_level = v;
-    s->rx_start = 0;
-    for (i = 0; i < s->rx_level; i ++)
-        s->rx_fifo[i] = qemu_get_byte(f);
-
-    return 0;
-}
-
 static void pxa2xx_ssp_reset(DeviceState *d)
 {
     PXA2xxSSPState *s = PXA2XX_SSP(d);
@@ -782,8 +760,6 @@ static int pxa2xx_ssp_init(SysBusDevice *sbd)
     memory_region_init_io(&s->iomem, OBJECT(s), &pxa2xx_ssp_ops, s,
                           "pxa2xx-ssp", 0x1000);
     sysbus_init_mmio(sbd, &s->iomem);
-    register_savevm(dev, "pxa2xx_ssp", -1, 0,
-                    pxa2xx_ssp_save, pxa2xx_ssp_load, s);
 
     s->bus = ssi_create_bus(dev, "ssi");
     return 0;
@@ -2006,7 +1982,7 @@ static bool pxa2xx_fir_vmstate_validate(void *opaque, int version_id)
 {
     PXA2xxFIrState *s = opaque;
 
-    return s->rx_start < sizeof(s->rx_fifo);
+    return s->rx_start < ARRAY_SIZE(s->rx_fifo);
 }
 
 static const VMStateDescription pxa2xx_fir_vmsd = {
@@ -2356,6 +2332,7 @@ static void pxa2xx_ssp_class_init(ObjectClass *klass, void *data)
 
     sdc->init = pxa2xx_ssp_init;
     dc->reset = pxa2xx_ssp_reset;
+    dc->vmsd = &vmstate_pxa2xx_ssp;
 }
 
 static const TypeInfo pxa2xx_ssp_info = {
-- 
1.9.1

[Qemu-devel] [PATCH v2 5/5] hw/sd/pxa2xx_mmci: Stop using old_mmio in MemoryRegionOps

[Peter Maydell]

Update the pxa2xx_mmci device to stop using the old_mmio read
and write callbacks in its MemoryRegionOps. This actually
simplifies the code because the separate byte/halfword/word
access functions were all calling into a single function to
do the work anyway.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 hw/sd/pxa2xx_mmci.c | 68 +++++++----------------------------------------------
 1 file changed, 8 insertions(+), 60 deletions(-)

diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c
index ac3ab39..d1fe6d5 100644
--- a/hw/sd/pxa2xx_mmci.c
+++ b/hw/sd/pxa2xx_mmci.c
@@ -48,7 +48,6 @@ struct PXA2xxMMCIState {
     int resp_len;
 
     int cmdreq;
-    int ac_width;
 };
 
 #define MMC_STRPCL	0x00	/* MMC Clock Start/Stop register */
@@ -215,7 +214,7 @@ static void pxa2xx_mmci_wakequeues(PXA2xxMMCIState *s)
     pxa2xx_mmci_fifo_update(s);
 }
 
-static uint32_t pxa2xx_mmci_read(void *opaque, hwaddr offset)
+static uint64_t pxa2xx_mmci_read(void *opaque, hwaddr offset, unsigned size)
 {
     PXA2xxMMCIState *s = (PXA2xxMMCIState *) opaque;
     uint32_t ret;
@@ -257,8 +256,8 @@ static uint32_t pxa2xx_mmci_read(void *opaque, hwaddr offset)
         return 0;
     case MMC_RXFIFO:
         ret = 0;
-        while (s->ac_width -- && s->rx_len) {
-            ret |= s->rx_fifo[s->rx_start ++] << (s->ac_width << 3);
+        while (size-- && s->rx_len) {
+            ret |= s->rx_fifo[s->rx_start++] << (size << 3);
             s->rx_start &= 0x1f;
             s->rx_len --;
         }
@@ -277,7 +276,7 @@ static uint32_t pxa2xx_mmci_read(void *opaque, hwaddr offset)
 }
 
 static void pxa2xx_mmci_write(void *opaque,
-                hwaddr offset, uint32_t value)
+                              hwaddr offset, uint64_t value, unsigned size)
 {
     PXA2xxMMCIState *s = (PXA2xxMMCIState *) opaque;
 
@@ -370,9 +369,9 @@ static void pxa2xx_mmci_write(void *opaque,
         break;
 
     case MMC_TXFIFO:
-        while (s->ac_width -- && s->tx_len < 0x20)
+        while (size-- && s->tx_len < 0x20)
             s->tx_fifo[(s->tx_start + (s->tx_len ++)) & 0x1f] =
-                    (value >> (s->ac_width << 3)) & 0xff;
+                    (value >> (size << 3)) & 0xff;
         s->intreq &= ~INT_TXFIFO_REQ;
         pxa2xx_mmci_fifo_update(s);
         break;
@@ -386,60 +385,9 @@ static void pxa2xx_mmci_write(void *opaque,
     }
 }
 
-static uint32_t pxa2xx_mmci_readb(void *opaque, hwaddr offset)
-{
-    PXA2xxMMCIState *s = (PXA2xxMMCIState *) opaque;
-    s->ac_width = 1;
-    return pxa2xx_mmci_read(opaque, offset);
-}
-
-static uint32_t pxa2xx_mmci_readh(void *opaque, hwaddr offset)
-{
-    PXA2xxMMCIState *s = (PXA2xxMMCIState *) opaque;
-    s->ac_width = 2;
-    return pxa2xx_mmci_read(opaque, offset);
-}
-
-static uint32_t pxa2xx_mmci_readw(void *opaque, hwaddr offset)
-{
-    PXA2xxMMCIState *s = (PXA2xxMMCIState *) opaque;
-    s->ac_width = 4;
-    return pxa2xx_mmci_read(opaque, offset);
-}
-
-static void pxa2xx_mmci_writeb(void *opaque,
-                hwaddr offset, uint32_t value)
-{
-    PXA2xxMMCIState *s = (PXA2xxMMCIState *) opaque;
-    s->ac_width = 1;
-    pxa2xx_mmci_write(opaque, offset, value);
-}
-
-static void pxa2xx_mmci_writeh(void *opaque,
-                hwaddr offset, uint32_t value)
-{
-    PXA2xxMMCIState *s = (PXA2xxMMCIState *) opaque;
-    s->ac_width = 2;
-    pxa2xx_mmci_write(opaque, offset, value);
-}
-
-static void pxa2xx_mmci_writew(void *opaque,
-                hwaddr offset, uint32_t value)
-{
-    PXA2xxMMCIState *s = (PXA2xxMMCIState *) opaque;
-    s->ac_width = 4;
-    pxa2xx_mmci_write(opaque, offset, value);
-}
-
 static const MemoryRegionOps pxa2xx_mmci_ops = {
-    .old_mmio = {
-        .read = { pxa2xx_mmci_readb,
-                  pxa2xx_mmci_readh,
-                  pxa2xx_mmci_readw, },
-        .write = { pxa2xx_mmci_writeb,
-                   pxa2xx_mmci_writeh,
-                   pxa2xx_mmci_writew, },
-    },
+    .read = pxa2xx_mmci_read,
+    .write = pxa2xx_mmci_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2 2/5] hw/arm/pxa2xx: Convert pxa2xx-fir to QOM and VMState

[Peter Crosthwaite]

On Fri, Jun 12, 2015 at 7:06 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> Convert the pxa2xx-fir device to QOM, including using a
> VMState for its migration info.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

> ---
>  hw/arm/pxa2xx.c | 140 ++++++++++++++++++++++++++++++++++----------------------
>  1 file changed, 85 insertions(+), 55 deletions(-)
>
> diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
> index 8123f05..6f80496 100644
> --- a/hw/arm/pxa2xx.c
> +++ b/hw/arm/pxa2xx.c
> @@ -1759,24 +1759,33 @@ static PXA2xxI2SState *pxa2xx_i2s_init(MemoryRegion *sysmem,
>  }
>
>  /* PXA Fast Infra-red Communications Port */
> +#define TYPE_PXA2XX_FIR "pxa2xx-fir"
> +#define PXA2XX_FIR(obj) OBJECT_CHECK(PXA2xxFIrState, (obj), TYPE_PXA2XX_FIR)
> +
>  struct PXA2xxFIrState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
>      MemoryRegion iomem;
>      qemu_irq irq;
>      qemu_irq rx_dma;
>      qemu_irq tx_dma;
> -    int enable;
> +    uint32_t enable;
>      CharDriverState *chr;
>
>      uint8_t control[3];
>      uint8_t status[2];
>
> -    int rx_len;
> -    int rx_start;
> +    uint32_t rx_len;
> +    uint32_t rx_start;
>      uint8_t rx_fifo[64];
>  };
>
> -static void pxa2xx_fir_reset(PXA2xxFIrState *s)
> +static void pxa2xx_fir_reset(DeviceState *d)
>  {
> +    PXA2xxFIrState *s = PXA2XX_FIR(d);
> +
>      s->control[0] = 0x00;
>      s->control[1] = 0x00;
>      s->control[2] = 0x00;
> @@ -1953,73 +1962,94 @@ static void pxa2xx_fir_event(void *opaque, int event)
>  {
>  }
>
> -static void pxa2xx_fir_save(QEMUFile *f, void *opaque)
> +static void pxa2xx_fir_instance_init(Object *obj)
>  {
> -    PXA2xxFIrState *s = (PXA2xxFIrState *) opaque;
> -    int i;
> -
> -    qemu_put_be32(f, s->enable);
> -
> -    qemu_put_8s(f, &s->control[0]);
> -    qemu_put_8s(f, &s->control[1]);
> -    qemu_put_8s(f, &s->control[2]);
> -    qemu_put_8s(f, &s->status[0]);
> -    qemu_put_8s(f, &s->status[1]);
> +    PXA2xxFIrState *s = PXA2XX_FIR(obj);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>
> -    qemu_put_byte(f, s->rx_len);
> -    for (i = 0; i < s->rx_len; i ++)
> -        qemu_put_byte(f, s->rx_fifo[(s->rx_start + i) & 63]);
> +    memory_region_init_io(&s->iomem, NULL, &pxa2xx_fir_ops, s,
> +                          "pxa2xx-fir", 0x1000);
> +    sysbus_init_mmio(sbd, &s->iomem);
> +    sysbus_init_irq(sbd, &s->irq);
> +    sysbus_init_irq(sbd, &s->rx_dma);
> +    sysbus_init_irq(sbd, &s->tx_dma);
>  }
>
> -static int pxa2xx_fir_load(QEMUFile *f, void *opaque, int version_id)
> +static void pxa2xx_fir_realize(DeviceState *dev, Error **errp)
>  {
> -    PXA2xxFIrState *s = (PXA2xxFIrState *) opaque;
> -    int i;
> +    PXA2xxFIrState *s = PXA2XX_FIR(dev);
>
> -    s->enable = qemu_get_be32(f);
> -
> -    qemu_get_8s(f, &s->control[0]);
> -    qemu_get_8s(f, &s->control[1]);
> -    qemu_get_8s(f, &s->control[2]);
> -    qemu_get_8s(f, &s->status[0]);
> -    qemu_get_8s(f, &s->status[1]);
> +    if (s->chr) {
> +        qemu_chr_fe_claim_no_fail(s->chr);
> +        qemu_chr_add_handlers(s->chr, pxa2xx_fir_is_empty,
> +                        pxa2xx_fir_rx, pxa2xx_fir_event, s);
> +    }
> +}
>
> -    s->rx_len = qemu_get_byte(f);
> -    s->rx_start = 0;
> -    for (i = 0; i < s->rx_len; i ++)
> -        s->rx_fifo[i] = qemu_get_byte(f);
> +static bool pxa2xx_fir_vmstate_validate(void *opaque, int version_id)
> +{
> +    PXA2xxFIrState *s = opaque;
>
> -    return 0;
> +    return s->rx_start < sizeof(s->rx_fifo);
>  }
>
> -static PXA2xxFIrState *pxa2xx_fir_init(MemoryRegion *sysmem,
> -                hwaddr base,
> -                qemu_irq irq, qemu_irq rx_dma, qemu_irq tx_dma,
> -                CharDriverState *chr)
> -{
> -    PXA2xxFIrState *s = (PXA2xxFIrState *)
> -            g_malloc0(sizeof(PXA2xxFIrState));
> +static const VMStateDescription pxa2xx_fir_vmsd = {
> +    .name = "pxa2xx-fir",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(enable, PXA2xxFIrState),
> +        VMSTATE_UINT8_ARRAY(control, PXA2xxFIrState, 3),
> +        VMSTATE_UINT8_ARRAY(status, PXA2xxFIrState, 2),
> +        VMSTATE_UINT32(rx_len, PXA2xxFIrState),
> +        VMSTATE_UINT32(rx_start, PXA2xxFIrState),
> +        VMSTATE_VALIDATE("fifo is 64 bytes", pxa2xx_fir_vmstate_validate),
> +        VMSTATE_UINT8_ARRAY(rx_fifo, PXA2xxFIrState, 64),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
>
> -    s->irq = irq;
> -    s->rx_dma = rx_dma;
> -    s->tx_dma = tx_dma;
> -    s->chr = chr;
> +static Property pxa2xx_fir_properties[] = {
> +    DEFINE_PROP_CHR("chardev", PXA2xxFIrState, chr),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
>
> -    pxa2xx_fir_reset(s);
> +static void pxa2xx_fir_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
>
> -    memory_region_init_io(&s->iomem, NULL, &pxa2xx_fir_ops, s, "pxa2xx-fir", 0x1000);
> -    memory_region_add_subregion(sysmem, base, &s->iomem);
> +    dc->realize = pxa2xx_fir_realize;
> +    dc->vmsd = &pxa2xx_fir_vmsd;
> +    dc->props = pxa2xx_fir_properties;
> +    dc->reset = pxa2xx_fir_reset;
> +}
>
> -    if (chr) {
> -        qemu_chr_fe_claim_no_fail(chr);
> -        qemu_chr_add_handlers(chr, pxa2xx_fir_is_empty,
> -                        pxa2xx_fir_rx, pxa2xx_fir_event, s);
> -    }
> +static const TypeInfo pxa2xx_fir_info = {
> +    .name = TYPE_PXA2XX_FIR,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(PXA2xxFIrState),
> +    .class_init = pxa2xx_fir_class_init,
> +    .instance_init = pxa2xx_fir_instance_init,
> +};
>
> -    register_savevm(NULL, "pxa2xx_fir", 0, 0, pxa2xx_fir_save,
> -                    pxa2xx_fir_load, s);
> +static PXA2xxFIrState *pxa2xx_fir_init(MemoryRegion *sysmem,
> +                                       hwaddr base,
> +                                       qemu_irq irq, qemu_irq rx_dma,
> +                                       qemu_irq tx_dma,
> +                                       CharDriverState *chr)
> +{
> +    DeviceState *dev;
> +    SysBusDevice *sbd;
>
> -    return s;
> +    dev = qdev_create(NULL, TYPE_PXA2XX_FIR);
> +    qdev_prop_set_chr(dev, "chardev", chr);
> +    qdev_init_nofail(dev);
> +    sbd = SYS_BUS_DEVICE(dev);
> +    sysbus_mmio_map(sbd, 0, base);
> +    sysbus_connect_irq(sbd, 0, irq);
> +    sysbus_connect_irq(sbd, 1, rx_dma);
> +    sysbus_connect_irq(sbd, 2, tx_dma);
> +    return PXA2XX_FIR(dev);
>  }
>
>  static void pxa2xx_reset(void *opaque, int line, int level)
> --
> 1.9.1
>
>

Re: [Qemu-devel] [PATCH v2 2/5] hw/arm/pxa2xx: Convert pxa2xx-fir to QOM and VMState

[Peter Maydell]

On 12 June 2015 at 15:06, Peter Maydell <peter.maydell@linaro.org> wrote:
> Convert the pxa2xx-fir device to QOM, including using a
> VMState for its migration info.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/pxa2xx.c | 140 ++++++++++++++++++++++++++++++++++----------------------
>  1 file changed, 85 insertions(+), 55 deletions(-)
>
> diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
> index 8123f05..6f80496 100644
> --- a/hw/arm/pxa2xx.c
> +++ b/hw/arm/pxa2xx.c
> @@ -1759,24 +1759,33 @@ static PXA2xxI2SState *pxa2xx_i2s_init(MemoryRegion *sysmem,
>  }
>
>  /* PXA Fast Infra-red Communications Port */
> +#define TYPE_PXA2XX_FIR "pxa2xx-fir"
> +#define PXA2XX_FIR(obj) OBJECT_CHECK(PXA2xxFIrState, (obj), TYPE_PXA2XX_FIR)
> +
>  struct PXA2xxFIrState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
>      MemoryRegion iomem;
>      qemu_irq irq;
>      qemu_irq rx_dma;
>      qemu_irq tx_dma;
> -    int enable;
> +    uint32_t enable;
>      CharDriverState *chr;
>
>      uint8_t control[3];
>      uint8_t status[2];
>
> -    int rx_len;
> -    int rx_start;
> +    uint32_t rx_len;
> +    uint32_t rx_start;
>      uint8_t rx_fifo[64];
>  };
>
> -static void pxa2xx_fir_reset(PXA2xxFIrState *s)
> +static void pxa2xx_fir_reset(DeviceState *d)
>  {
> +    PXA2xxFIrState *s = PXA2XX_FIR(d);
> +
>      s->control[0] = 0x00;
>      s->control[1] = 0x00;
>      s->control[2] = 0x00;
> @@ -1953,73 +1962,94 @@ static void pxa2xx_fir_event(void *opaque, int event)
>  {
>  }
>
> -static void pxa2xx_fir_save(QEMUFile *f, void *opaque)
> +static void pxa2xx_fir_instance_init(Object *obj)
>  {
> -    PXA2xxFIrState *s = (PXA2xxFIrState *) opaque;
> -    int i;
> -
> -    qemu_put_be32(f, s->enable);
> -
> -    qemu_put_8s(f, &s->control[0]);
> -    qemu_put_8s(f, &s->control[1]);
> -    qemu_put_8s(f, &s->control[2]);
> -    qemu_put_8s(f, &s->status[0]);
> -    qemu_put_8s(f, &s->status[1]);
> +    PXA2xxFIrState *s = PXA2XX_FIR(obj);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
>
> -    qemu_put_byte(f, s->rx_len);
> -    for (i = 0; i < s->rx_len; i ++)
> -        qemu_put_byte(f, s->rx_fifo[(s->rx_start + i) & 63]);
> +    memory_region_init_io(&s->iomem, NULL, &pxa2xx_fir_ops, s,
> +                          "pxa2xx-fir", 0x1000);
> +    sysbus_init_mmio(sbd, &s->iomem);
> +    sysbus_init_irq(sbd, &s->irq);
> +    sysbus_init_irq(sbd, &s->rx_dma);
> +    sysbus_init_irq(sbd, &s->tx_dma);
>  }
>
> -static int pxa2xx_fir_load(QEMUFile *f, void *opaque, int version_id)
> +static void pxa2xx_fir_realize(DeviceState *dev, Error **errp)
>  {
> -    PXA2xxFIrState *s = (PXA2xxFIrState *) opaque;
> -    int i;
> +    PXA2xxFIrState *s = PXA2XX_FIR(dev);
>
> -    s->enable = qemu_get_be32(f);
> -
> -    qemu_get_8s(f, &s->control[0]);
> -    qemu_get_8s(f, &s->control[1]);
> -    qemu_get_8s(f, &s->control[2]);
> -    qemu_get_8s(f, &s->status[0]);
> -    qemu_get_8s(f, &s->status[1]);
> +    if (s->chr) {
> +        qemu_chr_fe_claim_no_fail(s->chr);
> +        qemu_chr_add_handlers(s->chr, pxa2xx_fir_is_empty,
> +                        pxa2xx_fir_rx, pxa2xx_fir_event, s);
> +    }
> +}
>
> -    s->rx_len = qemu_get_byte(f);
> -    s->rx_start = 0;
> -    for (i = 0; i < s->rx_len; i ++)
> -        s->rx_fifo[i] = qemu_get_byte(f);
> +static bool pxa2xx_fir_vmstate_validate(void *opaque, int version_id)
> +{
> +    PXA2xxFIrState *s = opaque;
>
> -    return 0;
> +    return s->rx_start < sizeof(s->rx_fifo);
>  }
>
> -static PXA2xxFIrState *pxa2xx_fir_init(MemoryRegion *sysmem,
> -                hwaddr base,
> -                qemu_irq irq, qemu_irq rx_dma, qemu_irq tx_dma,
> -                CharDriverState *chr)
> -{
> -    PXA2xxFIrState *s = (PXA2xxFIrState *)
> -            g_malloc0(sizeof(PXA2xxFIrState));
> +static const VMStateDescription pxa2xx_fir_vmsd = {
> +    .name = "pxa2xx-fir",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(enable, PXA2xxFIrState),
> +        VMSTATE_UINT8_ARRAY(control, PXA2xxFIrState, 3),
> +        VMSTATE_UINT8_ARRAY(status, PXA2xxFIrState, 2),
> +        VMSTATE_UINT32(rx_len, PXA2xxFIrState),
> +        VMSTATE_UINT32(rx_start, PXA2xxFIrState),
> +        VMSTATE_VALIDATE("fifo is 64 bytes", pxa2xx_fir_vmstate_validate),
> +        VMSTATE_UINT8_ARRAY(rx_fifo, PXA2xxFIrState, 64),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
>
> -    s->irq = irq;
> -    s->rx_dma = rx_dma;
> -    s->tx_dma = tx_dma;
> -    s->chr = chr;
> +static Property pxa2xx_fir_properties[] = {
> +    DEFINE_PROP_CHR("chardev", PXA2xxFIrState, chr),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
>
> -    pxa2xx_fir_reset(s);
> +static void pxa2xx_fir_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
>
> -    memory_region_init_io(&s->iomem, NULL, &pxa2xx_fir_ops, s, "pxa2xx-fir", 0x1000);
> -    memory_region_add_subregion(sysmem, base, &s->iomem);
> +    dc->realize = pxa2xx_fir_realize;
> +    dc->vmsd = &pxa2xx_fir_vmsd;
> +    dc->props = pxa2xx_fir_properties;
> +    dc->reset = pxa2xx_fir_reset;
> +}
>
> -    if (chr) {
> -        qemu_chr_fe_claim_no_fail(chr);
> -        qemu_chr_add_handlers(chr, pxa2xx_fir_is_empty,
> -                        pxa2xx_fir_rx, pxa2xx_fir_event, s);
> -    }
> +static const TypeInfo pxa2xx_fir_info = {
> +    .name = TYPE_PXA2XX_FIR,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(PXA2xxFIrState),
> +    .class_init = pxa2xx_fir_class_init,
> +    .instance_init = pxa2xx_fir_instance_init,
> +};

Oops, forgot the type_register_static():

--- a/hw/arm/pxa2xx.c
+++ b/hw/arm/pxa2xx.c
@@ -2348,6 +2348,7 @@ static void pxa2xx_register_types(void)
     type_register_static(&pxa2xx_ssp_info);
     type_register_static(&pxa2xx_i2c_info);
     type_register_static(&pxa2xx_rtc_sysbus_info);
+    type_register_static(&pxa2xx_fir_info);
 }

 type_init(pxa2xx_register_types)

Will fold in and apply to target-arm.next.

-- PMM