diff mbox series

[6/6] hw/isa/piix{3,4}: Inline and remove create() functions

Message ID 20220513175445.89616-7-shentey@gmail.com (mailing list archive)
State New, archived
Headers show
Series QOM'ify PIIX southbridge creation | expand

Commit Message

Bernhard Beschow May 13, 2022, 5:54 p.m. UTC 
During the previous changesets the create() functions became trivial
wrappers around more generic functions. Modernize the code.

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/i386/pc_piix.c             |  6 +++++-
 hw/isa/piix3.c                | 16 ----------------
 hw/isa/piix4.c                | 10 ----------
 hw/mips/malta.c               |  3 ++-
 include/hw/southbridge/piix.h |  6 ++----
 5 files changed, 9 insertions(+), 32 deletions(-)

Comments

Mark Cave-Ayland May 21, 2022, 8:43 a.m. UTC | #1 
On 13/05/2022 18:54, Bernhard Beschow wrote:

> During the previous changesets the create() functions became trivial
> wrappers around more generic functions. Modernize the code.
> 
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/i386/pc_piix.c             |  6 +++++-
>   hw/isa/piix3.c                | 16 ----------------
>   hw/isa/piix4.c                | 10 ----------
>   hw/mips/malta.c               |  3 ++-
>   include/hw/southbridge/piix.h |  6 ++----
>   5 files changed, 9 insertions(+), 32 deletions(-)
> 
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 47932448fd..82c7941958 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -196,6 +196,9 @@ static void pc_init1(MachineState *machine,
>   
>       if (pcmc->pci_enabled) {
>           PIIX3State *piix3;
> +        PCIDevice *pci_dev;
> +        const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
> +                                         : TYPE_PIIX3_DEVICE;
>   
>           pci_bus = i440fx_init(host_type,
>                                 pci_type,
> @@ -206,7 +209,8 @@ static void pc_init1(MachineState *machine,
>                                 pci_memory, ram_memory);
>           pcms->bus = pci_bus;
>   
> -        piix3 = piix3_create(pci_bus);
> +        pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
> +        piix3 = PIIX3_PCI_DEVICE(pci_dev);
>           piix3->pic = x86ms->gsi;
>           piix3_devfn = piix3->dev.devfn;
>           isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
> diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
> index 6eacb22dd0..01c376b39a 100644
> --- a/hw/isa/piix3.c
> +++ b/hw/isa/piix3.c
> @@ -36,9 +36,6 @@
>   
>   #define XEN_PIIX_NUM_PIRQS      128ULL
>   
> -#define TYPE_PIIX3_DEVICE "PIIX3"
> -#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
> -
>   static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
>   {
>       qemu_set_irq(piix3->pic[pic_irq],
> @@ -402,16 +399,3 @@ static void piix3_register_types(void)
>   }
>   
>   type_init(piix3_register_types)
> -
> -PIIX3State *piix3_create(PCIBus *pci_bus)
> -{
> -    PIIX3State *piix3;
> -    PCIDevice *pci_dev;
> -    const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
> -                                     : TYPE_PIIX3_DEVICE;
> -
> -    pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
> -    piix3 = PIIX3_PCI_DEVICE(pci_dev);
> -
> -    return piix3;
> -}
> diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
> index 852e5c4db1..a70063bc77 100644
> --- a/hw/isa/piix4.c
> +++ b/hw/isa/piix4.c
> @@ -300,13 +300,3 @@ static void piix4_register_types(void)
>   }
>   
>   type_init(piix4_register_types)
> -
> -PCIDevice *piix4_create(PCIBus *pci_bus)
> -{
> -    PCIDevice *pci;
> -
> -    pci = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0), true,
> -                                          TYPE_PIIX4_PCI_DEVICE);
> -
> -    return pci;
> -}
> diff --git a/hw/mips/malta.c b/hw/mips/malta.c
> index d4bd3549d0..57b5eddc74 100644
> --- a/hw/mips/malta.c
> +++ b/hw/mips/malta.c
> @@ -1400,7 +1400,8 @@ void mips_malta_init(MachineState *machine)
>       empty_slot_init("GT64120", 0, 0x20000000);
>   
>       /* Southbridge */
> -    piix4 = piix4_create(pci_bus);
> +    piix4 = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0), true,
> +                                            TYPE_PIIX4_PCI_DEVICE);
>       dev = DEVICE(piix4);
>       isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
>       smbus = piix4_pm_init(pci_bus, piix4->devfn + 3, 0x1100,
> diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
> index bea3b44551..2d55dbdef7 100644
> --- a/include/hw/southbridge/piix.h
> +++ b/include/hw/southbridge/piix.h
> @@ -70,10 +70,8 @@ typedef struct PIIXState PIIX3State;
>   DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE,
>                            TYPE_PIIX3_PCI_DEVICE)
>   
> +#define TYPE_PIIX3_DEVICE "PIIX3"
> +#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
>   #define TYPE_PIIX4_PCI_DEVICE "piix4-isa"

I think it would make sense for the movement of these types to be included in patch 1 
in a single place.

> -PIIX3State *piix3_create(PCIBus *pci_bus);
> -
> -PCIDevice *piix4_create(PCIBus *pci_bus);
> -
>   #endif

Otherwise:

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


ATB,

Mark.
Bernhard Beschow May 22, 2022, 9:09 a.m. UTC | #2 
On Sat, May 21, 2022 at 10:43 AM Mark Cave-Ayland <
mark.cave-ayland@ilande.co.uk> wrote:

> On 13/05/2022 18:54, Bernhard Beschow wrote:
>
> > During the previous changesets the create() functions became trivial
> > wrappers around more generic functions. Modernize the code.
> >
> > Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> > ---
> >   hw/i386/pc_piix.c             |  6 +++++-
> >   hw/isa/piix3.c                | 16 ----------------
> >   hw/isa/piix4.c                | 10 ----------
> >   hw/mips/malta.c               |  3 ++-
> >   include/hw/southbridge/piix.h |  6 ++----
> >   5 files changed, 9 insertions(+), 32 deletions(-)
> >
> > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > index 47932448fd..82c7941958 100644
> > --- a/hw/i386/pc_piix.c
> > +++ b/hw/i386/pc_piix.c
> > @@ -196,6 +196,9 @@ static void pc_init1(MachineState *machine,
> >
> >       if (pcmc->pci_enabled) {
> >           PIIX3State *piix3;
> > +        PCIDevice *pci_dev;
> > +        const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
> > +                                         : TYPE_PIIX3_DEVICE;
> >
> >           pci_bus = i440fx_init(host_type,
> >                                 pci_type,
> > @@ -206,7 +209,8 @@ static void pc_init1(MachineState *machine,
> >                                 pci_memory, ram_memory);
> >           pcms->bus = pci_bus;
> >
> > -        piix3 = piix3_create(pci_bus);
> > +        pci_dev = pci_create_simple_multifunction(pci_bus, -1, true,
> type);
> > +        piix3 = PIIX3_PCI_DEVICE(pci_dev);
> >           piix3->pic = x86ms->gsi;
> >           piix3_devfn = piix3->dev.devfn;
> >           isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
> > diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
> > index 6eacb22dd0..01c376b39a 100644
> > --- a/hw/isa/piix3.c
> > +++ b/hw/isa/piix3.c
> > @@ -36,9 +36,6 @@
> >
> >   #define XEN_PIIX_NUM_PIRQS      128ULL
> >
> > -#define TYPE_PIIX3_DEVICE "PIIX3"
> > -#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
> > -
> >   static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
> >   {
> >       qemu_set_irq(piix3->pic[pic_irq],
> > @@ -402,16 +399,3 @@ static void piix3_register_types(void)
> >   }
> >
> >   type_init(piix3_register_types)
> > -
> > -PIIX3State *piix3_create(PCIBus *pci_bus)
> > -{
> > -    PIIX3State *piix3;
> > -    PCIDevice *pci_dev;
> > -    const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
> > -                                     : TYPE_PIIX3_DEVICE;
> > -
> > -    pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
> > -    piix3 = PIIX3_PCI_DEVICE(pci_dev);
> > -
> > -    return piix3;
> > -}
> > diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
> > index 852e5c4db1..a70063bc77 100644
> > --- a/hw/isa/piix4.c
> > +++ b/hw/isa/piix4.c
> > @@ -300,13 +300,3 @@ static void piix4_register_types(void)
> >   }
> >
> >   type_init(piix4_register_types)
> > -
> > -PCIDevice *piix4_create(PCIBus *pci_bus)
> > -{
> > -    PCIDevice *pci;
> > -
> > -    pci = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0),
> true,
> > -                                          TYPE_PIIX4_PCI_DEVICE);
> > -
> > -    return pci;
> > -}
> > diff --git a/hw/mips/malta.c b/hw/mips/malta.c
> > index d4bd3549d0..57b5eddc74 100644
> > --- a/hw/mips/malta.c
> > +++ b/hw/mips/malta.c
> > @@ -1400,7 +1400,8 @@ void mips_malta_init(MachineState *machine)
> >       empty_slot_init("GT64120", 0, 0x20000000);
> >
> >       /* Southbridge */
> > -    piix4 = piix4_create(pci_bus);
> > +    piix4 = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0),
> true,
> > +                                            TYPE_PIIX4_PCI_DEVICE);
> >       dev = DEVICE(piix4);
> >       isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
> >       smbus = piix4_pm_init(pci_bus, piix4->devfn + 3, 0x1100,
> > diff --git a/include/hw/southbridge/piix.h
> b/include/hw/southbridge/piix.h
> > index bea3b44551..2d55dbdef7 100644
> > --- a/include/hw/southbridge/piix.h
> > +++ b/include/hw/southbridge/piix.h
> > @@ -70,10 +70,8 @@ typedef struct PIIXState PIIX3State;
> >   DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE,
> >                            TYPE_PIIX3_PCI_DEVICE)
> >
> > +#define TYPE_PIIX3_DEVICE "PIIX3"
> > +#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
> >   #define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
>
> I think it would make sense for the movement of these types to be included
> in patch 1
> in a single place.
>

This makes sense indeed. I'll change it.

>
> > -PIIX3State *piix3_create(PCIBus *pci_bus);
> > -
> > -PCIDevice *piix4_create(PCIBus *pci_bus);
> > -
> >   #endif
>
> Otherwise:
>
> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>
>
> ATB,
>
> Mark.
>
diff mbox series

Patch

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 47932448fd..82c7941958 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -196,6 +196,9 @@  static void pc_init1(MachineState *machine,
 
     if (pcmc->pci_enabled) {
         PIIX3State *piix3;
+        PCIDevice *pci_dev;
+        const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
+                                         : TYPE_PIIX3_DEVICE;
 
         pci_bus = i440fx_init(host_type,
                               pci_type,
@@ -206,7 +209,8 @@  static void pc_init1(MachineState *machine,
                               pci_memory, ram_memory);
         pcms->bus = pci_bus;
 
-        piix3 = piix3_create(pci_bus);
+        pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
+        piix3 = PIIX3_PCI_DEVICE(pci_dev);
         piix3->pic = x86ms->gsi;
         piix3_devfn = piix3->dev.devfn;
         isa_bus = ISA_BUS(qdev_get_child_bus(DEVICE(piix3), "isa.0"));
diff --git a/hw/isa/piix3.c b/hw/isa/piix3.c
index 6eacb22dd0..01c376b39a 100644
--- a/hw/isa/piix3.c
+++ b/hw/isa/piix3.c
@@ -36,9 +36,6 @@ 
 
 #define XEN_PIIX_NUM_PIRQS      128ULL
 
-#define TYPE_PIIX3_DEVICE "PIIX3"
-#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
-
 static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
 {
     qemu_set_irq(piix3->pic[pic_irq],
@@ -402,16 +399,3 @@  static void piix3_register_types(void)
 }
 
 type_init(piix3_register_types)
-
-PIIX3State *piix3_create(PCIBus *pci_bus)
-{
-    PIIX3State *piix3;
-    PCIDevice *pci_dev;
-    const char *type = xen_enabled() ? TYPE_PIIX3_XEN_DEVICE
-                                     : TYPE_PIIX3_DEVICE;
-
-    pci_dev = pci_create_simple_multifunction(pci_bus, -1, true, type);
-    piix3 = PIIX3_PCI_DEVICE(pci_dev);
-
-    return piix3;
-}
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 852e5c4db1..a70063bc77 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -300,13 +300,3 @@  static void piix4_register_types(void)
 }
 
 type_init(piix4_register_types)
-
-PCIDevice *piix4_create(PCIBus *pci_bus)
-{
-    PCIDevice *pci;
-
-    pci = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0), true,
-                                          TYPE_PIIX4_PCI_DEVICE);
-
-    return pci;
-}
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index d4bd3549d0..57b5eddc74 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -1400,7 +1400,8 @@  void mips_malta_init(MachineState *machine)
     empty_slot_init("GT64120", 0, 0x20000000);
 
     /* Southbridge */
-    piix4 = piix4_create(pci_bus);
+    piix4 = pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0), true,
+                                            TYPE_PIIX4_PCI_DEVICE);
     dev = DEVICE(piix4);
     isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
     smbus = piix4_pm_init(pci_bus, piix4->devfn + 3, 0x1100,
diff --git a/include/hw/southbridge/piix.h b/include/hw/southbridge/piix.h
index bea3b44551..2d55dbdef7 100644
--- a/include/hw/southbridge/piix.h
+++ b/include/hw/southbridge/piix.h
@@ -70,10 +70,8 @@  typedef struct PIIXState PIIX3State;
 DECLARE_INSTANCE_CHECKER(PIIX3State, PIIX3_PCI_DEVICE,
                          TYPE_PIIX3_PCI_DEVICE)
 
+#define TYPE_PIIX3_DEVICE "PIIX3"
+#define TYPE_PIIX3_XEN_DEVICE "PIIX3-xen"
 #define TYPE_PIIX4_PCI_DEVICE "piix4-isa"
 
-PIIX3State *piix3_create(PCIBus *pci_bus);
-
-PCIDevice *piix4_create(PCIBus *pci_bus);
-
 #endif