[RFC PATCH v3 10/31] hw/pxb: Use a type for realizing expanders

Ben Widawsky posted 31 patches 5 years ago
Maintainers: "Dr. David Alan Gilbert" <dgilbert@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, Markus Armbruster <armbru@redhat.com>, Michael Tokarev <mjt@tls.msk.ru>, Sergio Lopez <slp@redhat.com>, David Gibson <david@gibson.dropbear.id.au>, Peter Maydell <peter.maydell@linaro.org>, Greg Kurz <groug@kaod.org>, Richard Henderson <richard.henderson@linaro.org>, Ben Widawsky <ben.widawsky@intel.com>, Laurent Vivier <laurent@vivier.eu>, Thomas Huth <thuth@redhat.com>, Eric Blake <eblake@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Laurent Vivier <lvivier@redhat.com>
[RFC PATCH v3 10/31] hw/pxb: Use a type for realizing expanders
Posted by Ben Widawsky 5 years ago
This opens up the possibility for more types of expanders (other than
PCI and PCIe). We'll need this to create a CXL expander.

Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
 hw/pci-bridge/pci_expander_bridge.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index aedded1064..232b7ce305 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -24,6 +24,8 @@
 #include "hw/boards.h"
 #include "qom/object.h"
 
+enum BusType { PCI, PCIE };
+
 #define TYPE_PXB_BUS "pxb-bus"
 typedef struct PXBBus PXBBus;
 DECLARE_INSTANCE_CHECKER(PXBBus, PXB_BUS,
@@ -214,7 +216,8 @@ static gint pxb_compare(gconstpointer a, gconstpointer b)
            0;
 }
 
-static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
+static void pxb_dev_realize_common(PCIDevice *dev, enum BusType type,
+                                   Error **errp)
 {
     PXBDev *pxb = convert_to_pxb(dev);
     DeviceState *ds, *bds = NULL;
@@ -239,7 +242,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
     }
 
     ds = qdev_new(TYPE_PXB_HOST);
-    if (pcie) {
+    if (type == PCIE) {
         bus = pci_root_bus_new(ds, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
     } else {
         bus = pci_root_bus_new(ds, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS);
@@ -287,7 +290,7 @@ static void pxb_dev_realize(PCIDevice *dev, Error **errp)
         return;
     }
 
-    pxb_dev_realize_common(dev, false, errp);
+    pxb_dev_realize_common(dev, PCI, errp);
 }
 
 static void pxb_dev_exitfn(PCIDevice *pci_dev)
@@ -339,7 +342,7 @@ static void pxb_pcie_dev_realize(PCIDevice *dev, Error **errp)
         return;
     }
 
-    pxb_dev_realize_common(dev, true, errp);
+    pxb_dev_realize_common(dev, PCIE, errp);
 }
 
 static void pxb_pcie_dev_class_init(ObjectClass *klass, void *data)
-- 
2.30.0


Re: [RFC PATCH v3 10/31] hw/pxb: Use a type for realizing expanders
Posted by Jonathan Cameron 5 years ago
On Mon, 1 Feb 2021 16:59:27 -0800
Ben Widawsky <ben.widawsky@intel.com> wrote:

> This opens up the possibility for more types of expanders (other than
> PCI and PCIe). We'll need this to create a CXL expander.
> 
> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
Minor suggestion inline but nothing important if you don't want to change it.

Jonathan

> ---
>  hw/pci-bridge/pci_expander_bridge.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> index aedded1064..232b7ce305 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -24,6 +24,8 @@
>  #include "hw/boards.h"
>  #include "qom/object.h"
>  
> +enum BusType { PCI, PCIE };
> +
>  #define TYPE_PXB_BUS "pxb-bus"
>  typedef struct PXBBus PXBBus;
>  DECLARE_INSTANCE_CHECKER(PXBBus, PXB_BUS,
> @@ -214,7 +216,8 @@ static gint pxb_compare(gconstpointer a, gconstpointer b)
>             0;
>  }
>  
> -static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
> +static void pxb_dev_realize_common(PCIDevice *dev, enum BusType type,
> +                                   Error **errp)
>  {
>      PXBDev *pxb = convert_to_pxb(dev);
>      DeviceState *ds, *bds = NULL;
> @@ -239,7 +242,7 @@ static void pxb_dev_realize_common(PCIDevice *dev, bool pcie, Error **errp)
>      }
>  
>      ds = qdev_new(TYPE_PXB_HOST);
> -    if (pcie) {
> +    if (type == PCIE) {

I'd make this a switch statement now given we are about to the 3 entries and may well
get more in the future.

>          bus = pci_root_bus_new(ds, dev_name, NULL, NULL, 0, TYPE_PXB_PCIE_BUS);
>      } else {
>          bus = pci_root_bus_new(ds, "pxb-internal", NULL, NULL, 0, TYPE_PXB_BUS);
> @@ -287,7 +290,7 @@ static void pxb_dev_realize(PCIDevice *dev, Error **errp)
>          return;
>      }
>  
> -    pxb_dev_realize_common(dev, false, errp);
> +    pxb_dev_realize_common(dev, PCI, errp);
>  }
>  
>  static void pxb_dev_exitfn(PCIDevice *pci_dev)
> @@ -339,7 +342,7 @@ static void pxb_pcie_dev_realize(PCIDevice *dev, Error **errp)
>          return;
>      }
>  
> -    pxb_dev_realize_common(dev, true, errp);
> +    pxb_dev_realize_common(dev, PCIE, errp);
>  }
>  
>  static void pxb_pcie_dev_class_init(ObjectClass *klass, void *data)