Patchwork [05/11] cs4231a: Convert to MemoryRegion.

login
register
mail settings
Submitter Richard Henderson
Date Aug. 10, 2011, 10:28 p.m.
Message ID <1313015300-23920-6-git-send-email-rth@twiddle.net>
Download mbox | patch
Permalink /patch/109473/
State New
Headers show

Comments

Richard Henderson - Aug. 10, 2011, 10:28 p.m.
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 hw/cs4231a.c |   38 +++++++++++++++++++-------------------
 1 files changed, 19 insertions(+), 19 deletions(-)
Avi Kivity - Aug. 11, 2011, 6:22 a.m.
On 08/11/2011 01:28 AM, Richard Henderson wrote:
>       }
>   };
>
> +static const MemoryRegionOps cs_ioport_ops = {
> +    .read = cs_read,
> +    .write = cs_write,
> +    .impl = {
> +        .min_access_size = 1,
> +        .max_access_size = 1,
> +    }
> +};

Should be .valid, not .impl.

.impl means the implementation only supports this access size; emulate 
other sizes by using a supported size (e.g. crack a 4-byte access into 4 
1-byte accesses).  .valid means what the device actually supports; if 
the guest uses another size something bus-defined happens.

> +
>   static int cs4231a_initfn (ISADevice *dev)
>   {
>       CSState *s = DO_UPCAST (CSState, dev, dev);
> -    int i;
>
>       isa_init_irq (dev,&s->pic, s->irq);
>
> -    for (i = 0; i<  4; i++) {
> -        isa_init_ioport(dev, i);
> -        register_ioport_write (s->port + i, 1, 1, cs_write, s);
> -        register_ioport_read (s->port + i, 1, 1, cs_read, s);
> -    }
> +    memory_region_init_io(&s->ioports,&cs_ioport_ops, s, "cs4231a", 4);
> +    isa_register_ioport(dev,&s->ioports, s->port);
>
>       DMA_register_channel (s->dma, cs_dma_read, s);
>
Avi Kivity - Aug. 11, 2011, 6:27 a.m.
On 08/11/2011 09:22 AM, Avi Kivity wrote:
> On 08/11/2011 01:28 AM, Richard Henderson wrote:
>>       }
>>   };
>>
>> +static const MemoryRegionOps cs_ioport_ops = {
>> +    .read = cs_read,
>> +    .write = cs_write,
>> +    .impl = {
>> +        .min_access_size = 1,
>> +        .max_access_size = 1,
>> +    }
>> +};
>
> Should be .valid, not .impl.
>
> .impl means the implementation only supports this access size; emulate 
> other sizes by using a supported size (e.g. crack a 4-byte access into 
> 4 1-byte accesses).  .valid means what the device actually supports; 
> if the guest uses another size something bus-defined happens.

Oh, the standard qemu implementation auto-cracks, so .impl is correct.

Patch

diff --git a/hw/cs4231a.c b/hw/cs4231a.c
index 598f032..e16665e 100644
--- a/hw/cs4231a.c
+++ b/hw/cs4231a.c
@@ -59,6 +59,7 @@  static struct {
 typedef struct CSState {
     ISADevice dev;
     QEMUSoundCard card;
+    MemoryRegion ioports;
     qemu_irq pic;
     uint32_t regs[CS_REGS];
     uint8_t dregs[CS_DREGS];
@@ -74,14 +75,6 @@  typedef struct CSState {
     int16_t *tab;
 } CSState;
 
-#define IO_READ_PROTO(name)                             \
-    static uint32_t name (void *opaque, uint32_t addr)
-
-#define IO_WRITE_PROTO(name)                                            \
-    static void name (void *opaque, uint32_t addr, uint32_t val)
-
-#define GET_SADDR(addr) (addr & 3)
-
 #define MODE2 (1 << 6)
 #define MCE (1 << 6)
 #define PMCE (1 << 4)
@@ -353,12 +346,12 @@  static void cs_reset_voices (CSState *s, uint32_t val)
     }
 }
 
-IO_READ_PROTO (cs_read)
+static uint64_t cs_read(void *opaque, target_phys_addr_t addr, unsigned size)
 {
     CSState *s = opaque;
     uint32_t saddr, iaddr, ret;
 
-    saddr = GET_SADDR (addr);
+    saddr = addr;
     iaddr = ~0U;
 
     switch (saddr) {
@@ -390,12 +383,14 @@  IO_READ_PROTO (cs_read)
     return ret;
 }
 
-IO_WRITE_PROTO (cs_write)
+static void cs_write(void *opaque, target_phys_addr_t addr,
+                     uint64_t val64, unsigned size)
 {
     CSState *s = opaque;
-    uint32_t saddr, iaddr;
+    uint32_t saddr, iaddr, val;
 
-    saddr = GET_SADDR (addr);
+    saddr = addr;
+    val = val64;
 
     switch (saddr) {
     case Index_Address:
@@ -637,18 +632,23 @@  static const VMStateDescription vmstate_cs4231a = {
     }
 };
 
+static const MemoryRegionOps cs_ioport_ops = {
+    .read = cs_read,
+    .write = cs_write,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    }
+};
+
 static int cs4231a_initfn (ISADevice *dev)
 {
     CSState *s = DO_UPCAST (CSState, dev, dev);
-    int i;
 
     isa_init_irq (dev, &s->pic, s->irq);
 
-    for (i = 0; i < 4; i++) {
-        isa_init_ioport(dev, i);
-        register_ioport_write (s->port + i, 1, 1, cs_write, s);
-        register_ioport_read (s->port + i, 1, 1, cs_read, s);
-    }
+    memory_region_init_io(&s->ioports, &cs_ioport_ops, s, "cs4231a", 4);
+    isa_register_ioport(dev, &s->ioports, s->port);
 
     DMA_register_channel (s->dma, cs_dma_read, s);