[48/49] hw: remove pio_addr_t

Commit Message

Paolo Bonzini March 16, 2016, 10:46 a.m. UTC

Using uint32_t is enough and avoids the need to include ioport.h everywhere.

Signed-off-by: Paolo Bonzini <[e-mail]>
---
 hw/core/sysbus.c      |  4 ++--
 include/exec/ioport.h | 15 ++++++---------
 include/hw/sysbus.h   |  4 ++--
 ioport.c              | 12 ++++++------
 xen-hvm.c             |  8 ++++----
 5 files changed, 20 insertions(+), 23 deletions(-)

Comments

Peter Maydell March 16, 2016, 11:16 a.m. UTC | #1

On 16 March 2016 at 10:46, Paolo Bonzini <[e-mail]> wrote:
> Using uint32_t is enough and avoids the need to include ioport.h everywhere.
>
> Signed-off-by: Paolo Bonzini <[e-mail]>

I'm not hugely convinced by this patch -- I think it's nice
to have a typedef that indicates that you're dealing with an
IO port address (and not some other kind of address or number).

thanks
-- PMM

Paolo Bonzini March 16, 2016, 11:21 a.m. UTC | #2

On 16/03/2016 12:16, Peter Maydell wrote:
>> > Using uint32_t is enough and avoids the need to include ioport.h everywhere.
>> >
>> > Signed-off-by: Paolo Bonzini <[e-mail]>
> I'm not hugely convinced by this patch -- I think it's nice
> to have a typedef that indicates that you're dealing with an
> IO port address (and not some other kind of address or number).

In theory I agree, but in practice it's just a burden.  It's almost
unused---on one hand that's also because these days I/O ports are simply
accessed through the address space, on the other hand even portio_*
functions use it.

cpu_{in,out}[bwl] are almost unused; monitor.c and xen-hvm.c could use
address_space_read/write directly, since they have an integer size at
hand.  This leaves qtest as the only user of those functions, so we
might as well inline them there.

The only interesting use of pio_addr_t thus is include/hw/sysbus.h.  I
guess I could move it there, but I don't see much benefit in that either.

Paolo

Patch

diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index a7dbe2b..c0f560b 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -190,9 +190,9 @@  MemoryRegion *sysbus_mmio_get_region(SysBusDevice *dev, int n)
     return dev->mmio[n].memory;
 }
 
-void sysbus_init_ioports(SysBusDevice *dev, pio_addr_t ioport, pio_addr_t size)
+void sysbus_init_ioports(SysBusDevice *dev, uint32_t ioport, uint32_t size)
 {
-    pio_addr_t i;
+    uint32_t i;
 
     for (i = 0; i < size; i++) {
         assert(dev->num_pio < QDEV_MAX_PIO);
diff --git a/include/exec/ioport.h b/include/exec/ioport.h
index 3bd6722..6a9639c 100644
--- a/include/exec/ioport.h
+++ b/include/exec/ioport.h
@@ -28,9 +28,6 @@ 
 #include "qom/object.h"
 #include "exec/memory.h"
 
-typedef uint32_t pio_addr_t;
-#define FMT_pioaddr     PRIx32
-
 #define MAX_IOPORTS     (64 * 1024)
 #define IOPORTS_MASK    (MAX_IOPORTS - 1)
 
@@ -49,12 +46,12 @@  typedef struct MemoryRegionPortio {
 extern const MemoryRegionOps unassigned_io_ops;
 #endif
 
-void cpu_outb(pio_addr_t addr, uint8_t val);
-void cpu_outw(pio_addr_t addr, uint16_t val);
-void cpu_outl(pio_addr_t addr, uint32_t val);
-uint8_t cpu_inb(pio_addr_t addr);
-uint16_t cpu_inw(pio_addr_t addr);
-uint32_t cpu_inl(pio_addr_t addr);
+void cpu_outb(uint32_t addr, uint8_t val);
+void cpu_outw(uint32_t addr, uint16_t val);
+void cpu_outl(uint32_t addr, uint32_t val);
+uint8_t cpu_inb(uint32_t addr);
+uint16_t cpu_inw(uint32_t addr);
+uint32_t cpu_inl(uint32_t addr);
 
 typedef struct PortioList {
     const struct MemoryRegionPortio *ports;
diff --git a/include/hw/sysbus.h b/include/hw/sysbus.h
index cc1dba4..a495937 100644
--- a/include/hw/sysbus.h
+++ b/include/hw/sysbus.h
@@ -72,7 +72,7 @@  struct SysBusDevice {
         MemoryRegion *memory;
     } mmio[QDEV_MAX_MMIO];
     int num_pio;
-    pio_addr_t pio[QDEV_MAX_PIO];
+    uint32_t pio[QDEV_MAX_PIO];
 };
 
 typedef int FindSysbusDeviceFunc(SysBusDevice *sbdev, void *opaque);
@@ -81,7 +81,7 @@  void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion *memory);
 MemoryRegion *sysbus_mmio_get_region(SysBusDevice *dev, int n);
 void sysbus_init_irq(SysBusDevice *dev, qemu_irq *p);
 void sysbus_pass_irq(SysBusDevice *dev, SysBusDevice *target);
-void sysbus_init_ioports(SysBusDevice *dev, pio_addr_t ioport, pio_addr_t size);
+void sysbus_init_ioports(SysBusDevice *dev, uint32_t ioport, uint32_t size);
 
 
 bool sysbus_has_irq(SysBusDevice *dev, int n);
diff --git a/ioport.c b/ioport.c
index 901a997..94e08ab 100644
--- a/ioport.c
+++ b/ioport.c
@@ -55,14 +55,14 @@  const MemoryRegionOps unassigned_io_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-void cpu_outb(pio_addr_t addr, uint8_t val)
+void cpu_outb(uint32_t addr, uint8_t val)
 {
     trace_cpu_out(addr, 'b', val);
     address_space_write(&address_space_io, addr, MEMTXATTRS_UNSPECIFIED,
                         &val, 1);
 }
 
-void cpu_outw(pio_addr_t addr, uint16_t val)
+void cpu_outw(uint32_t addr, uint16_t val)
 {
     uint8_t buf[2];
 
@@ -72,7 +72,7 @@  void cpu_outw(pio_addr_t addr, uint16_t val)
                         buf, 2);
 }
 
-void cpu_outl(pio_addr_t addr, uint32_t val)
+void cpu_outl(uint32_t addr, uint32_t val)
 {
     uint8_t buf[4];
 
@@ -82,7 +82,7 @@  void cpu_outl(pio_addr_t addr, uint32_t val)
                         buf, 4);
 }
 
-uint8_t cpu_inb(pio_addr_t addr)
+uint8_t cpu_inb(uint32_t addr)
 {
     uint8_t val;
 
@@ -92,7 +92,7 @@  uint8_t cpu_inb(pio_addr_t addr)
     return val;
 }
 
-uint16_t cpu_inw(pio_addr_t addr)
+uint16_t cpu_inw(uint32_t addr)
 {
     uint8_t buf[2];
     uint16_t val;
@@ -103,7 +103,7 @@  uint16_t cpu_inw(pio_addr_t addr)
     return val;
 }
 
-uint32_t cpu_inl(pio_addr_t addr)
+uint32_t cpu_inl(uint32_t addr)
 {
     uint8_t buf[4];
     uint32_t val;
diff --git a/xen-hvm.c b/xen-hvm.c
index 039680a..76dd76f 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -725,7 +725,7 @@  static ioreq_t *cpu_get_ioreq(XenIOState *state)
     return NULL;
 }
 
-static uint32_t do_inp(pio_addr_t addr, unsigned long size)
+static uint32_t do_inp(uint32_t addr, unsigned long size)
 {
     switch (size) {
         case 1:
@@ -735,11 +735,11 @@  static uint32_t do_inp(pio_addr_t addr, unsigned long size)
         case 4:
             return cpu_inl(addr);
         default:
-            hw_error("inp: bad size: %04"FMT_pioaddr" %lx", addr, size);
+            hw_error("inp: bad size: %04x %lx", addr, size);
     }
 }
 
-static void do_outp(pio_addr_t addr,
+static void do_outp(uint32_t addr,
         unsigned long size, uint32_t val)
 {
     switch (size) {
@@ -750,7 +750,7 @@  static void do_outp(pio_addr_t addr,
         case 4:
             return cpu_outl(addr, val);
         default:
-            hw_error("outp: bad size: %04"FMT_pioaddr" %lx", addr, size);
+            hw_error("outp: bad size: %04x %lx", addr, size);
     }
 }