1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v4 00/11] pci: hotplug handler reworks
  4. View patch

[Qemu-devel] [PATCH v4 09/11] pci: Reuse pci-bridge hotplug handler handlers for pcie-pci-bridge

David Hildenbrand posted 11 patches 6 years, 10 months ago
[Qemu-devel] [PATCH v4 09/11] pci: Reuse pci-bridge hotplug handler handlers for pcie-pci-bridge
Posted by David Hildenbrand 6 years, 10 months ago 
These functions are essentially the same, we only have to use
object_get_typename() for reporting errors. So let's share the
implementation of hotplug handler callbacks.

Suggested-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/pci-bridge/pci_bridge_dev.c  | 12 ++++++------
 hw/pci-bridge/pcie_pci_bridge.c | 30 ++----------------------------
 include/hw/pci/pci_bridge.h     |  4 ++++
 3 files changed, 12 insertions(+), 34 deletions(-)

diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
index e1df9a52ac..fa0be13ac4 100644
--- a/hw/pci-bridge/pci_bridge_dev.c
+++ b/hw/pci-bridge/pci_bridge_dev.c
@@ -206,27 +206,27 @@ static const VMStateDescription pci_bridge_dev_vmstate = {
     }
 };
 
-static void pci_bridge_dev_plug_cb(HotplugHandler *hotplug_dev,
-                                   DeviceState *dev, Error **errp)
+void pci_bridge_dev_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
+                            Error **errp)
 {
     PCIDevice *pci_hotplug_dev = PCI_DEVICE(hotplug_dev);
 
     if (!shpc_present(pci_hotplug_dev)) {
         error_setg(errp, "standard hotplug controller has been disabled for "
-                   "this %s", TYPE_PCI_BRIDGE_DEV);
+                   "this %s", object_get_typename(OBJECT(hotplug_dev)));
         return;
     }
     shpc_device_plug_cb(hotplug_dev, dev, errp);
 }
 
-static void pci_bridge_dev_unplug_request_cb(HotplugHandler *hotplug_dev,
-                                             DeviceState *dev, Error **errp)
+void pci_bridge_dev_unplug_request_cb(HotplugHandler *hotplug_dev,
+                                      DeviceState *dev, Error **errp)
 {
     PCIDevice *pci_hotplug_dev = PCI_DEVICE(hotplug_dev);
 
     if (!shpc_present(pci_hotplug_dev)) {
         error_setg(errp, "standard hotplug controller has been disabled for "
-                   "this %s", TYPE_PCI_BRIDGE_DEV);
+                   "this %s", object_get_typename(OBJECT(hotplug_dev)));
         return;
     }
     shpc_device_unplug_request_cb(hotplug_dev, dev, errp);
diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c
index c634353b06..0ffea680d5 100644
--- a/hw/pci-bridge/pcie_pci_bridge.c
+++ b/hw/pci-bridge/pcie_pci_bridge.c
@@ -137,32 +137,6 @@ static const VMStateDescription pcie_pci_bridge_dev_vmstate = {
         }
 };
 
-static void pcie_pci_bridge_plug_cb(HotplugHandler *hotplug_dev,
-                                    DeviceState *dev, Error **errp)
-{
-    PCIDevice *pci_hotplug_dev = PCI_DEVICE(hotplug_dev);
-
-    if (!shpc_present(pci_hotplug_dev)) {
-        error_setg(errp, "standard hotplug controller has been disabled for "
-                   "this %s", TYPE_PCIE_PCI_BRIDGE_DEV);
-        return;
-    }
-    shpc_device_plug_cb(hotplug_dev, dev, errp);
-}
-
-static void pcie_pci_bridge_unplug_request_cb(HotplugHandler *hotplug_dev,
-                                              DeviceState *dev, Error **errp)
-{
-    PCIDevice *pci_hotplug_dev = PCI_DEVICE(hotplug_dev);
-
-    if (!shpc_present(pci_hotplug_dev)) {
-        error_setg(errp, "standard hotplug controller has been disabled for "
-                   "this %s", TYPE_PCIE_PCI_BRIDGE_DEV);
-        return;
-    }
-    shpc_device_unplug_request_cb(hotplug_dev, dev, errp);
-}
-
 static void pcie_pci_bridge_class_init(ObjectClass *klass, void *data)
 {
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
@@ -179,8 +153,8 @@ static void pcie_pci_bridge_class_init(ObjectClass *klass, void *data)
     dc->props = pcie_pci_bridge_dev_properties;
     dc->reset = &pcie_pci_bridge_reset;
     set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
-    hc->plug = pcie_pci_bridge_plug_cb;
-    hc->unplug_request = pcie_pci_bridge_unplug_request_cb;
+    hc->plug = pci_bridge_dev_plug_cb;
+    hc->unplug_request = pci_bridge_dev_unplug_request_cb;
 }
 
 static const TypeInfo pcie_pci_bridge_info = {
diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
index cdff7edfd1..6e37c7551a 100644
--- a/include/hw/pci/pci_bridge.h
+++ b/include/hw/pci/pci_bridge.h
@@ -99,6 +99,10 @@ void pci_bridge_reset(DeviceState *qdev);
 void pci_bridge_initfn(PCIDevice *pci_dev, const char *typename);
 void pci_bridge_exitfn(PCIDevice *pci_dev);
 
+void pci_bridge_dev_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
+                            Error **errp);
+void pci_bridge_dev_unplug_request_cb(HotplugHandler *hotplug_dev,
+                                      DeviceState *dev, Error **errp);
 
 /*
  * before qdev initialization(qdev_init()), this function sets bus_name and
-- 
2.17.2
Re: [Qemu-devel] [PATCH v4 09/11] pci: Reuse pci-bridge hotplug handler handlers for pcie-pci-bridge
Posted by David Gibson 6 years, 10 months ago 
On Wed, Dec 12, 2018 at 10:16:21AM +0100, David Hildenbrand wrote:
> These functions are essentially the same, we only have to use
> object_get_typename() for reporting errors. So let's share the
> implementation of hotplug handler callbacks.
> 
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>

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

> ---
>  hw/pci-bridge/pci_bridge_dev.c  | 12 ++++++------
>  hw/pci-bridge/pcie_pci_bridge.c | 30 ++----------------------------
>  include/hw/pci/pci_bridge.h     |  4 ++++
>  3 files changed, 12 insertions(+), 34 deletions(-)
> 
> diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
> index e1df9a52ac..fa0be13ac4 100644
> --- a/hw/pci-bridge/pci_bridge_dev.c
> +++ b/hw/pci-bridge/pci_bridge_dev.c
> @@ -206,27 +206,27 @@ static const VMStateDescription pci_bridge_dev_vmstate = {
>      }
>  };
>  
> -static void pci_bridge_dev_plug_cb(HotplugHandler *hotplug_dev,
> -                                   DeviceState *dev, Error **errp)
> +void pci_bridge_dev_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> +                            Error **errp)
>  {
>      PCIDevice *pci_hotplug_dev = PCI_DEVICE(hotplug_dev);
>  
>      if (!shpc_present(pci_hotplug_dev)) {
>          error_setg(errp, "standard hotplug controller has been disabled for "
> -                   "this %s", TYPE_PCI_BRIDGE_DEV);
> +                   "this %s", object_get_typename(OBJECT(hotplug_dev)));
>          return;
>      }
>      shpc_device_plug_cb(hotplug_dev, dev, errp);
>  }
>  
> -static void pci_bridge_dev_unplug_request_cb(HotplugHandler *hotplug_dev,
> -                                             DeviceState *dev, Error **errp)
> +void pci_bridge_dev_unplug_request_cb(HotplugHandler *hotplug_dev,
> +                                      DeviceState *dev, Error **errp)
>  {
>      PCIDevice *pci_hotplug_dev = PCI_DEVICE(hotplug_dev);
>  
>      if (!shpc_present(pci_hotplug_dev)) {
>          error_setg(errp, "standard hotplug controller has been disabled for "
> -                   "this %s", TYPE_PCI_BRIDGE_DEV);
> +                   "this %s", object_get_typename(OBJECT(hotplug_dev)));
>          return;
>      }
>      shpc_device_unplug_request_cb(hotplug_dev, dev, errp);
> diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c
> index c634353b06..0ffea680d5 100644
> --- a/hw/pci-bridge/pcie_pci_bridge.c
> +++ b/hw/pci-bridge/pcie_pci_bridge.c
> @@ -137,32 +137,6 @@ static const VMStateDescription pcie_pci_bridge_dev_vmstate = {
>          }
>  };
>  
> -static void pcie_pci_bridge_plug_cb(HotplugHandler *hotplug_dev,
> -                                    DeviceState *dev, Error **errp)
> -{
> -    PCIDevice *pci_hotplug_dev = PCI_DEVICE(hotplug_dev);
> -
> -    if (!shpc_present(pci_hotplug_dev)) {
> -        error_setg(errp, "standard hotplug controller has been disabled for "
> -                   "this %s", TYPE_PCIE_PCI_BRIDGE_DEV);
> -        return;
> -    }
> -    shpc_device_plug_cb(hotplug_dev, dev, errp);
> -}
> -
> -static void pcie_pci_bridge_unplug_request_cb(HotplugHandler *hotplug_dev,
> -                                              DeviceState *dev, Error **errp)
> -{
> -    PCIDevice *pci_hotplug_dev = PCI_DEVICE(hotplug_dev);
> -
> -    if (!shpc_present(pci_hotplug_dev)) {
> -        error_setg(errp, "standard hotplug controller has been disabled for "
> -                   "this %s", TYPE_PCIE_PCI_BRIDGE_DEV);
> -        return;
> -    }
> -    shpc_device_unplug_request_cb(hotplug_dev, dev, errp);
> -}
> -
>  static void pcie_pci_bridge_class_init(ObjectClass *klass, void *data)
>  {
>      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
> @@ -179,8 +153,8 @@ static void pcie_pci_bridge_class_init(ObjectClass *klass, void *data)
>      dc->props = pcie_pci_bridge_dev_properties;
>      dc->reset = &pcie_pci_bridge_reset;
>      set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> -    hc->plug = pcie_pci_bridge_plug_cb;
> -    hc->unplug_request = pcie_pci_bridge_unplug_request_cb;
> +    hc->plug = pci_bridge_dev_plug_cb;
> +    hc->unplug_request = pci_bridge_dev_unplug_request_cb;
>  }
>  
>  static const TypeInfo pcie_pci_bridge_info = {
> diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
> index cdff7edfd1..6e37c7551a 100644
> --- a/include/hw/pci/pci_bridge.h
> +++ b/include/hw/pci/pci_bridge.h
> @@ -99,6 +99,10 @@ void pci_bridge_reset(DeviceState *qdev);
>  void pci_bridge_initfn(PCIDevice *pci_dev, const char *typename);
>  void pci_bridge_exitfn(PCIDevice *pci_dev);
>  
> +void pci_bridge_dev_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> +                            Error **errp);
> +void pci_bridge_dev_unplug_request_cb(HotplugHandler *hotplug_dev,
> +                                      DeviceState *dev, Error **errp);
>  
>  /*
>   * before qdev initialization(qdev_init()), this function sets bus_name and

-- 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.