Re: [Qemu-devel] [RFC 6/7] pci: Set phb->bus inside pci_bus_new()

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3593 bytes --]

On Mon, Apr 17, 2017 at 06:59:15PM -0300, Eduardo Habkost wrote:
> Every single caller of pci_bus_new() saves the return value inside
> phb->bus. Do that inside pci_bus_new() to avoid code duplication and
> make it harder to break.
> 
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/pci-bridge/pci_expander_bridge.c | 2 --
>  hw/pci-host/piix.c                  | 1 -
>  hw/pci-host/q35.c                   | 6 +++---
>  hw/pci/pci.c                        | 2 +-
>  4 files changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> index 39d29d2230..8344ca1cc8 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -245,8 +245,6 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>      bus->address_space_io = dev->bus->address_space_io;
>      bus->map_irq = pxb_map_irq_fn;
>  
> -    phb->bus = bus;
> -
>      pxb_register_bus(dev, bus, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 91fec05b38..818e4979d8 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -342,7 +342,6 @@ PCIBus *i440fx_init(const char *host_type, const char *pci_type,
>      s = PCI_HOST_BRIDGE(dev);
>      b = pci_bus_new(s, NULL, pci_address_space,
>                      address_space_io, 0, TYPE_PCI_BUS);
> -    s->bus = b;
>      object_property_add_child(qdev_get_machine(), "i440fx", OBJECT(dev), NULL);
>      qdev_init_nofail(dev);
>  
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index 860b47a1ba..5b41412075 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -49,9 +49,9 @@ static void q35_host_realize(DeviceState *dev, Error **errp)
>      sysbus_add_io(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, &pci->data_mem);
>      sysbus_init_ioports(sbd, MCH_HOST_BRIDGE_CONFIG_DATA, 4);
>  
> -    pci->bus = pci_bus_new(pci, "pcie.0",
> -                           s->mch.pci_address_space, s->mch.address_space_io,
> -                           0, TYPE_PCIE_BUS);
> +    pci_bus_new(pci, "pcie.0",
> +                s->mch.pci_address_space, s->mch.address_space_io,
> +                0, TYPE_PCIE_BUS);
>      PC_MACHINE(qdev_get_machine())->bus = pci->bus;
>      qdev_set_parent_bus(DEVICE(&s->mch), BUS(pci->bus));
>      qdev_init_nofail(DEVICE(&s->mch));
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index d3adf806e5..486aeb7514 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -408,6 +408,7 @@ PCIBus *pci_bus_new(PCIHostState *phb, const char *name,
>  
>      bus = PCI_BUS(qbus_create(typename, DEVICE(phb), name));
>      pci_bus_init(bus, phb, address_space_mem, address_space_io, devfn_min);
> +    phb->bus = bus;
>      return bus;
>  }
>  
> @@ -433,7 +434,6 @@ void pci_register_bus(PCIHostState *phb, const char *name,
>      bus = pci_bus_new(phb, name, address_space_mem,
>                        address_space_io, devfn_min, typename);
>      pci_bus_irqs(bus, set_irq, map_irq, irq_opaque, nirq);
> -    phb->bus = bus;
>  }
>  
>  int pci_bus_num(PCIBus *s)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]