diff mbox series

[v3,14/18] exec/ioport: Factor portio_list_register() out

Message ID 20230302224058.43315-15-philmd@linaro.org
State New
Headers show
Series hw/ide: Untangle ISA/PCI abuses of ide_init_ioport() | expand

Commit Message

Philippe Mathieu-Daudé March 2, 2023, 10:40 p.m. UTC 
We always follow the same pattern when registering
non-coalesced portio:

  - portio_list_init()
  - portio_list_add()

Factor these 2 operations in a single helper named
portio_list_register(). Since both calls become local
to ioport.c, reduce their scope by declaring them static.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20230207234615.77300-3-philmd@linaro.org>
---
 hw/audio/adlib.c        |  4 ++--
 hw/display/vga.c        |  4 ++--
 hw/dma/i82374.c         |  7 +++----
 hw/ide/ioport.c         |  9 ++++-----
 hw/isa/isa-bus.c        |  5 ++---
 hw/watchdog/wdt_ib700.c |  4 ++--
 include/exec/ioport.h   | 10 ++++------
 softmmu/ioport.c        | 21 ++++++++++++++-------
 8 files changed, 33 insertions(+), 31 deletions(-)

Comments

Mark Cave-Ayland April 26, 2023, 1:28 p.m. UTC | #1 
On 02/03/2023 22:40, Philippe Mathieu-Daudé wrote:

> We always follow the same pattern when registering
> non-coalesced portio:
> 
>    - portio_list_init()
>    - portio_list_add()
> 
> Factor these 2 operations in a single helper named
> portio_list_register(). Since both calls become local
> to ioport.c, reduce their scope by declaring them static.
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Message-Id: <20230207234615.77300-3-philmd@linaro.org>
> ---
>   hw/audio/adlib.c        |  4 ++--
>   hw/display/vga.c        |  4 ++--
>   hw/dma/i82374.c         |  7 +++----
>   hw/ide/ioport.c         |  9 ++++-----
>   hw/isa/isa-bus.c        |  5 ++---
>   hw/watchdog/wdt_ib700.c |  4 ++--
>   include/exec/ioport.h   | 10 ++++------
>   softmmu/ioport.c        | 21 ++++++++++++++-------
>   8 files changed, 33 insertions(+), 31 deletions(-)
> 
> diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c
> index 5f979b1487..cc03c99306 100644
> --- a/hw/audio/adlib.c
> +++ b/hw/audio/adlib.c
> @@ -291,8 +291,8 @@ static void adlib_realizefn (DeviceState *dev, Error **errp)
>   
>       adlib_portio_list[0].offset = s->port;
>       adlib_portio_list[1].offset = s->port + 8;
> -    portio_list_init (&s->port_list, OBJECT(s), adlib_portio_list, s, "adlib");
> -    portio_list_add (&s->port_list, isa_address_space_io(&s->parent_obj), 0);
> +    portio_list_register(&s->port_list, OBJECT(s), adlib_portio_list, s,
> +                         "adlib", isa_address_space_io(&s->parent_obj), 0);
>   }
>   
>   static Property adlib_properties[] = {
> diff --git a/hw/display/vga.c b/hw/display/vga.c
> index 98d644922e..aa899fddc3 100644
> --- a/hw/display/vga.c
> +++ b/hw/display/vga.c
> @@ -2313,7 +2313,7 @@ void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space,
>                                                s, "vga", address_space_io, 0x3b0);
>       }
>       if (vbe_ports) {
> -        portio_list_init(&s->vbe_port_list, obj, vbe_ports, s, "vbe");
> -        portio_list_add(&s->vbe_port_list, address_space_io, 0x1ce);
> +        portio_list_register(&s->vbe_port_list, obj, vbe_ports, s,
> +                             "vbe", address_space_io, 0x1ce);
>       }
>   }
> diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c
> index 63734c22c9..aeca0e8323 100644
> --- a/hw/dma/i82374.c
> +++ b/hw/dma/i82374.c
> @@ -131,10 +131,9 @@ static void i82374_realize(DeviceState *dev, Error **errp)
>       }
>       i8257_dma_init(isa_bus, true);
>   
> -    portio_list_init(&s->port_list, OBJECT(s), i82374_portio_list, s,
> -                     "i82374");
> -    portio_list_add(&s->port_list, isa_address_space_io(&s->parent_obj),
> -                    s->iobase);
> +    portio_list_register(&s->port_list, OBJECT(s), i82374_portio_list, s,
> +                         "i82374", isa_address_space_io(&s->parent_obj),
> +                         s->iobase);
>   
>       memset(s->commands, 0, sizeof(s->commands));
>   }
> diff --git a/hw/ide/ioport.c b/hw/ide/ioport.c
> index ed7957dbae..7a6f29955f 100644
> --- a/hw/ide/ioport.c
> +++ b/hw/ide/ioport.c
> @@ -60,9 +60,8 @@ int ide_bus_init_ioport_isa(IDEBus *bus, ISADevice *dev,
>   void ide_bus_init_ioport(IDEBus *bus, Object *owner, MemoryRegion *io,
>                            int iobase, int iobase2)
>   {
> -    portio_list_init(&bus->portio_list, owner, ide_portio_list, bus, "ide");
> -    portio_list_add(&bus->portio_list, io, iobase);
> -
> -    portio_list_init(&bus->portio2_list, owner, ide_portio2_list, bus, "ide");
> -    portio_list_add(&bus->portio_list, io, iobase2);
> +    portio_list_register(&bus->portio_list, owner, ide_portio_list,
> +                         bus, "ide", io, iobase);
> +    portio_list_register(&bus->portio2_list, owner, ide_portio2_list,
> +                         bus, "ide", io, iobase2);
>   }
> diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
> index 8e3ca3785e..087293108e 100644
> --- a/hw/isa/isa-bus.c
> +++ b/hw/isa/isa-bus.c
> @@ -130,15 +130,14 @@ int isa_register_portio_list(ISADevice *dev,
>                                void *opaque, const char *name)
>   {
>       assert(dev);
> -    assert(piolist && !piolist->owner);
>   
>       /* START is how we should treat DEV, regardless of the actual
>          contents of the portio array.  This is how the old code
>          actually handled e.g. the FDC device.  */
>       isa_init_ioport(dev, start);
>   
> -    portio_list_init(piolist, OBJECT(dev), pio_start, opaque, name);
> -    portio_list_add(piolist, isa_address_space_io(dev), start);
> +    portio_list_register(piolist, OBJECT(dev), pio_start, opaque, name,
> +                         isa_address_space_io(dev), start);
>   
>       return 0;
>   }
> diff --git a/hw/watchdog/wdt_ib700.c b/hw/watchdog/wdt_ib700.c
> index b116c3a3aa..ac4f0be7d8 100644
> --- a/hw/watchdog/wdt_ib700.c
> +++ b/hw/watchdog/wdt_ib700.c
> @@ -115,8 +115,8 @@ static void wdt_ib700_realize(DeviceState *dev, Error **errp)
>   
>       s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, ib700_timer_expired, s);
>   
> -    portio_list_init(&s->port_list, OBJECT(s), wdt_portio_list, s, "ib700");
> -    portio_list_add(&s->port_list, isa_address_space_io(&s->parent_obj), 0);
> +    portio_list_register(&s->port_list, OBJECT(s), wdt_portio_list, s,
> +                         "ib700", isa_address_space_io(&s->parent_obj), 0);
>   }
>   
>   static void wdt_ib700_reset(DeviceState *dev)
> diff --git a/include/exec/ioport.h b/include/exec/ioport.h
> index eb9882a3ee..ca44f269ea 100644
> --- a/include/exec/ioport.h
> +++ b/include/exec/ioport.h
> @@ -62,17 +62,15 @@ typedef struct PortioList {
>       bool flush_coalesced_mmio;
>   } PortioList;
>   
> -void portio_list_init(PortioList *piolist, Object *owner,
> -                      const struct MemoryRegionPortio *callbacks,
> -                      void *opaque, const char *name);
> +void portio_list_register(PortioList *piolist, Object *owner,
> +                          const MemoryRegionPortio *callbacks,
> +                          void *opaque, const char *name,
> +                          MemoryRegion *mr, uint32_t offset);
>   void portio_list_register_flush_coalesced(PortioList *piolist, Object *owner,
>                                             const MemoryRegionPortio *callbacks,
>                                             void *opaque, const char *name,
>                                             MemoryRegion *mr, uint32_t offset);
>   void portio_list_destroy(PortioList *piolist);
> -void portio_list_add(PortioList *piolist,
> -                     struct MemoryRegion *address_space,
> -                     uint32_t addr);
>   void portio_list_del(PortioList *piolist);
>   
>   #endif /* IOPORT_H */
> diff --git a/softmmu/ioport.c b/softmmu/ioport.c
> index be0c920c5c..42d43f8b27 100644
> --- a/softmmu/ioport.c
> +++ b/softmmu/ioport.c
> @@ -113,10 +113,9 @@ uint32_t cpu_inl(uint32_t addr)
>       return val;
>   }
>   
> -void portio_list_init(PortioList *piolist,
> -                      Object *owner,
> -                      const MemoryRegionPortio *callbacks,
> -                      void *opaque, const char *name)
> +static void portio_list_init(PortioList *piolist, Object *owner,
> +                             const MemoryRegionPortio *callbacks,
> +                             void *opaque, const char *name)
>   {
>       unsigned n = 0;
>   
> @@ -246,9 +245,8 @@ static void portio_list_add_1(PortioList *piolist,
>       ++piolist->nr;
>   }
>   
> -void portio_list_add(PortioList *piolist,
> -                     MemoryRegion *address_space,
> -                     uint32_t start)
> +static void portio_list_add(PortioList *piolist, MemoryRegion *address_space,
> +                            uint32_t start)
>   {
>       const MemoryRegionPortio *pio, *pio_start = piolist->ports;
>       unsigned int off_low, off_high, off_last, count;
> @@ -314,3 +312,12 @@ void portio_list_register_flush_coalesced(PortioList *piolist, Object *owner,
>       do_portio_list_register(piolist, owner, callbacks,
>                               opaque, name, mr, offset, true);
>   }
> +
> +void portio_list_register(PortioList *piolist, Object *owner,
> +                          const MemoryRegionPortio *callbacks,
> +                          void *opaque, const char *name,
> +                          MemoryRegion *mr, uint32_t offset)
> +{
> +    do_portio_list_register(piolist, owner, callbacks,
> +                            opaque, name, mr, offset, false);
> +}

Ah, and you beat me to it :)  I'm not 100% sold on the previous patch, but this makes 
sense so:

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.
diff mbox series

Patch

diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c
index 5f979b1487..cc03c99306 100644
--- a/hw/audio/adlib.c
+++ b/hw/audio/adlib.c
@@ -291,8 +291,8 @@  static void adlib_realizefn (DeviceState *dev, Error **errp)
 
     adlib_portio_list[0].offset = s->port;
     adlib_portio_list[1].offset = s->port + 8;
-    portio_list_init (&s->port_list, OBJECT(s), adlib_portio_list, s, "adlib");
-    portio_list_add (&s->port_list, isa_address_space_io(&s->parent_obj), 0);
+    portio_list_register(&s->port_list, OBJECT(s), adlib_portio_list, s,
+                         "adlib", isa_address_space_io(&s->parent_obj), 0);
 }
 
 static Property adlib_properties[] = {
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 98d644922e..aa899fddc3 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -2313,7 +2313,7 @@  void vga_init(VGACommonState *s, Object *obj, MemoryRegion *address_space,
                                              s, "vga", address_space_io, 0x3b0);
     }
     if (vbe_ports) {
-        portio_list_init(&s->vbe_port_list, obj, vbe_ports, s, "vbe");
-        portio_list_add(&s->vbe_port_list, address_space_io, 0x1ce);
+        portio_list_register(&s->vbe_port_list, obj, vbe_ports, s,
+                             "vbe", address_space_io, 0x1ce);
     }
 }
diff --git a/hw/dma/i82374.c b/hw/dma/i82374.c
index 63734c22c9..aeca0e8323 100644
--- a/hw/dma/i82374.c
+++ b/hw/dma/i82374.c
@@ -131,10 +131,9 @@  static void i82374_realize(DeviceState *dev, Error **errp)
     }
     i8257_dma_init(isa_bus, true);
 
-    portio_list_init(&s->port_list, OBJECT(s), i82374_portio_list, s,
-                     "i82374");
-    portio_list_add(&s->port_list, isa_address_space_io(&s->parent_obj),
-                    s->iobase);
+    portio_list_register(&s->port_list, OBJECT(s), i82374_portio_list, s,
+                         "i82374", isa_address_space_io(&s->parent_obj),
+                         s->iobase);
 
     memset(s->commands, 0, sizeof(s->commands));
 }
diff --git a/hw/ide/ioport.c b/hw/ide/ioport.c
index ed7957dbae..7a6f29955f 100644
--- a/hw/ide/ioport.c
+++ b/hw/ide/ioport.c
@@ -60,9 +60,8 @@  int ide_bus_init_ioport_isa(IDEBus *bus, ISADevice *dev,
 void ide_bus_init_ioport(IDEBus *bus, Object *owner, MemoryRegion *io,
                          int iobase, int iobase2)
 {
-    portio_list_init(&bus->portio_list, owner, ide_portio_list, bus, "ide");
-    portio_list_add(&bus->portio_list, io, iobase);
-
-    portio_list_init(&bus->portio2_list, owner, ide_portio2_list, bus, "ide");
-    portio_list_add(&bus->portio_list, io, iobase2);
+    portio_list_register(&bus->portio_list, owner, ide_portio_list,
+                         bus, "ide", io, iobase);
+    portio_list_register(&bus->portio2_list, owner, ide_portio2_list,
+                         bus, "ide", io, iobase2);
 }
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 8e3ca3785e..087293108e 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -130,15 +130,14 @@  int isa_register_portio_list(ISADevice *dev,
                              void *opaque, const char *name)
 {
     assert(dev);
-    assert(piolist && !piolist->owner);
 
     /* START is how we should treat DEV, regardless of the actual
        contents of the portio array.  This is how the old code
        actually handled e.g. the FDC device.  */
     isa_init_ioport(dev, start);
 
-    portio_list_init(piolist, OBJECT(dev), pio_start, opaque, name);
-    portio_list_add(piolist, isa_address_space_io(dev), start);
+    portio_list_register(piolist, OBJECT(dev), pio_start, opaque, name,
+                         isa_address_space_io(dev), start);
 
     return 0;
 }
diff --git a/hw/watchdog/wdt_ib700.c b/hw/watchdog/wdt_ib700.c
index b116c3a3aa..ac4f0be7d8 100644
--- a/hw/watchdog/wdt_ib700.c
+++ b/hw/watchdog/wdt_ib700.c
@@ -115,8 +115,8 @@  static void wdt_ib700_realize(DeviceState *dev, Error **errp)
 
     s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, ib700_timer_expired, s);
 
-    portio_list_init(&s->port_list, OBJECT(s), wdt_portio_list, s, "ib700");
-    portio_list_add(&s->port_list, isa_address_space_io(&s->parent_obj), 0);
+    portio_list_register(&s->port_list, OBJECT(s), wdt_portio_list, s,
+                         "ib700", isa_address_space_io(&s->parent_obj), 0);
 }
 
 static void wdt_ib700_reset(DeviceState *dev)
diff --git a/include/exec/ioport.h b/include/exec/ioport.h
index eb9882a3ee..ca44f269ea 100644
--- a/include/exec/ioport.h
+++ b/include/exec/ioport.h
@@ -62,17 +62,15 @@  typedef struct PortioList {
     bool flush_coalesced_mmio;
 } PortioList;
 
-void portio_list_init(PortioList *piolist, Object *owner,
-                      const struct MemoryRegionPortio *callbacks,
-                      void *opaque, const char *name);
+void portio_list_register(PortioList *piolist, Object *owner,
+                          const MemoryRegionPortio *callbacks,
+                          void *opaque, const char *name,
+                          MemoryRegion *mr, uint32_t offset);
 void portio_list_register_flush_coalesced(PortioList *piolist, Object *owner,
                                           const MemoryRegionPortio *callbacks,
                                           void *opaque, const char *name,
                                           MemoryRegion *mr, uint32_t offset);
 void portio_list_destroy(PortioList *piolist);
-void portio_list_add(PortioList *piolist,
-                     struct MemoryRegion *address_space,
-                     uint32_t addr);
 void portio_list_del(PortioList *piolist);
 
 #endif /* IOPORT_H */
diff --git a/softmmu/ioport.c b/softmmu/ioport.c
index be0c920c5c..42d43f8b27 100644
--- a/softmmu/ioport.c
+++ b/softmmu/ioport.c
@@ -113,10 +113,9 @@  uint32_t cpu_inl(uint32_t addr)
     return val;
 }
 
-void portio_list_init(PortioList *piolist,
-                      Object *owner,
-                      const MemoryRegionPortio *callbacks,
-                      void *opaque, const char *name)
+static void portio_list_init(PortioList *piolist, Object *owner,
+                             const MemoryRegionPortio *callbacks,
+                             void *opaque, const char *name)
 {
     unsigned n = 0;
 
@@ -246,9 +245,8 @@  static void portio_list_add_1(PortioList *piolist,
     ++piolist->nr;
 }
 
-void portio_list_add(PortioList *piolist,
-                     MemoryRegion *address_space,
-                     uint32_t start)
+static void portio_list_add(PortioList *piolist, MemoryRegion *address_space,
+                            uint32_t start)
 {
     const MemoryRegionPortio *pio, *pio_start = piolist->ports;
     unsigned int off_low, off_high, off_last, count;
@@ -314,3 +312,12 @@  void portio_list_register_flush_coalesced(PortioList *piolist, Object *owner,
     do_portio_list_register(piolist, owner, callbacks,
                             opaque, name, mr, offset, true);
 }
+
+void portio_list_register(PortioList *piolist, Object *owner,
+                          const MemoryRegionPortio *callbacks,
+                          void *opaque, const char *name,
+                          MemoryRegion *mr, uint32_t offset)
+{
+    do_portio_list_register(piolist, owner, callbacks,
+                            opaque, name, mr, offset, false);
+}