On Sat, May 21, 2022 at 10:43 AM Mark Cave-Ayland <
mark.cave-ayl...@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 <shen...@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-ayl...@ilande.co.uk>
>
>
> ATB,
>
> Mark.
>
