Re: [Qemu-devel] [PATCH] change type of pci_bridge_initfn() to void

[Michael S. Tsirkin]

On Thu, Dec 24, 2015 at 11:39:00AM +0800, Cao jin wrote:
> Hi mst
> 
> On 12/23/2015 09:38 PM, Michael S. Tsirkin wrote:
> >On Wed, Dec 23, 2015 at 04:53:21PM +0800, Cao jin wrote:
> >>Hi mst
> >>friendly ping again...
> >
> >This does not work since then this function can not be
> >used as an init callback, and this is how
> >dec uses it.
> >
> 
> thanks very much for your time:) I was planning to separate supporting
> function & device convert to realize() into different patches, but seems it
> is not suitable for this case. then, I will put "dec: convert to realize"
> into this patch.

OK but how exactly did you test your patch that
a build break was missed?
Could you include some info about testing done
when you post patches?

> >>On 12/17/2015 09:53 AM, Cao jin wrote:
> >>>Ping
> >>>
> >>>On 11/30/2015 05:19 PM, Michael S. Tsirkin wrote:
> >>>>On Mon, Nov 30, 2015 at 05:00:44PM +0800, Cao jin wrote:
> >>>>>It always return 0(success), change its type to void, and modify its
> >>>>>caller.
> >>>>>Doing this can reduce a error path of its caller, and it is also good
> >>>>>when
> >>>>>convert init() to realize()
> >>>>>
> >>>>>Signed-off-by: Cao jin <address@hidden>
> >>>>
> >>>>Sounds good, but pls remember to ping me after 2.5 is out.
> >>>>
> >>>>>---
> >>>>>  hw/pci-bridge/i82801b11.c          | 5 +----
> >>>>>  hw/pci-bridge/ioh3420.c            | 6 +-----
> >>>>>  hw/pci-bridge/pci_bridge_dev.c     | 8 +++-----
> >>>>>  hw/pci-bridge/xio3130_downstream.c | 6 +-----
> >>>>>  hw/pci-bridge/xio3130_upstream.c   | 6 +-----
> >>>>>  hw/pci-host/apb.c                  | 5 +----
> >>>>>  hw/pci/pci_bridge.c                | 3 +--
> >>>>>  include/hw/pci/pci_bridge.h        | 2 +-
> >>>>>  8 files changed, 10 insertions(+), 31 deletions(-)
> >>>>>
> >>>>>leave DEC 21154 PCI-PCI bridge unchanged because it is better to
> >>>>>handle it
> >>>>>when convert init() to realize().
> >>>>>
> >>>>>diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c
> >>>>>index 7e79bc0..b21bc2c 100644
> >>>>>--- a/hw/pci-bridge/i82801b11.c
> >>>>>+++ b/hw/pci-bridge/i82801b11.c
> >>>>>@@ -61,10 +61,7 @@ static int i82801b11_bridge_initfn(PCIDevice *d)
> >>>>>  {
> >>>>>      int rc;
> >>>>>
> >>>>>-    rc = pci_bridge_initfn(d, TYPE_PCI_BUS);
> >>>>>-    if (rc < 0) {
> >>>>>-        return rc;
> >>>>>-    }
> >>>>>+    pci_bridge_initfn(d, TYPE_PCI_BUS);
> >>>>>
> >>>>>      rc = pci_bridge_ssvid_init(d, I82801ba_SSVID_OFFSET,
> >>>>>                                 I82801ba_SSVID_SVID,
> >>>>>I82801ba_SSVID_SSID);
> >>>>>diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
> >>>>>index cce2fdd..eead195 100644
> >>>>>--- a/hw/pci-bridge/ioh3420.c
> >>>>>+++ b/hw/pci-bridge/ioh3420.c
> >>>>>@@ -97,11 +97,7 @@ static int ioh3420_initfn(PCIDevice *d)
> >>>>>      PCIESlot *s = PCIE_SLOT(d);
> >>>>>      int rc;
> >>>>>
> >>>>>-    rc = pci_bridge_initfn(d, TYPE_PCIE_BUS);
> >>>>>-    if (rc < 0) {
> >>>>>-        return rc;
> >>>>>-    }
> >>>>>-
> >>>>>+    pci_bridge_initfn(d, TYPE_PCIE_BUS);
> >>>>>      pcie_port_init_reg(d);
> >>>>>
> >>>>>      rc = pci_bridge_ssvid_init(d, IOH_EP_SSVID_OFFSET,
> >>>>>diff --git a/hw/pci-bridge/pci_bridge_dev.c
> >>>>>b/hw/pci-bridge/pci_bridge_dev.c
> >>>>>index 26aded9..bc3e1b7 100644
> >>>>>--- a/hw/pci-bridge/pci_bridge_dev.c
> >>>>>+++ b/hw/pci-bridge/pci_bridge_dev.c
> >>>>>@@ -52,10 +52,8 @@ static int pci_bridge_dev_initfn(PCIDevice *dev)
> >>>>>      PCIBridgeDev *bridge_dev = PCI_BRIDGE_DEV(dev);
> >>>>>      int err;
> >>>>>
> >>>>>-    err = pci_bridge_initfn(dev, TYPE_PCI_BUS);
> >>>>>-    if (err) {
> >>>>>-        goto bridge_error;
> >>>>>-    }
> >>>>>+    pci_bridge_initfn(dev, TYPE_PCI_BUS);
> >>>>>+
> >>>>>      if (bridge_dev->flags & (1 << PCI_BRIDGE_DEV_F_SHPC_REQ)) {
> >>>>>          dev->config[PCI_INTERRUPT_PIN] = 0x1;
> >>>>>          memory_region_init(&bridge_dev->bar, OBJECT(dev), "shpc-bar",
> >>>>>@@ -94,7 +92,7 @@ slotid_error:
> >>>>>      }
> >>>>>  shpc_error:
> >>>>>      pci_bridge_exitfn(dev);
> >>>>>-bridge_error:
> >>>>>+
> >>>>>      return err;
> >>>>>  }
> >>>>>
> >>>>>diff --git a/hw/pci-bridge/xio3130_downstream.c
> >>>>>b/hw/pci-bridge/xio3130_downstream.c
> >>>>>index 86b7970..012daf3 100644
> >>>>>--- a/hw/pci-bridge/xio3130_downstream.c
> >>>>>+++ b/hw/pci-bridge/xio3130_downstream.c
> >>>>>@@ -61,11 +61,7 @@ static int xio3130_downstream_initfn(PCIDevice *d)
> >>>>>      PCIESlot *s = PCIE_SLOT(d);
> >>>>>      int rc;
> >>>>>
> >>>>>-    rc = pci_bridge_initfn(d, TYPE_PCIE_BUS);
> >>>>>-    if (rc < 0) {
> >>>>>-        return rc;
> >>>>>-    }
> >>>>>-
> >>>>>+    pci_bridge_initfn(d, TYPE_PCIE_BUS);
> >>>>>      pcie_port_init_reg(d);
> >>>>>
> >>>>>      rc = msi_init(d, XIO3130_MSI_OFFSET, XIO3130_MSI_NR_VECTOR,
> >>>>>diff --git a/hw/pci-bridge/xio3130_upstream.c
> >>>>>b/hw/pci-bridge/xio3130_upstream.c
> >>>>>index eada582..434c8fd 100644
> >>>>>--- a/hw/pci-bridge/xio3130_upstream.c
> >>>>>+++ b/hw/pci-bridge/xio3130_upstream.c
> >>>>>@@ -56,11 +56,7 @@ static int xio3130_upstream_initfn(PCIDevice *d)
> >>>>>      PCIEPort *p = PCIE_PORT(d);
> >>>>>      int rc;
> >>>>>
> >>>>>-    rc = pci_bridge_initfn(d, TYPE_PCIE_BUS);
> >>>>>-    if (rc < 0) {
> >>>>>-        return rc;
> >>>>>-    }
> >>>>>-
> >>>>>+    pci_bridge_initfn(d, TYPE_PCIE_BUS);
> >>>>>      pcie_port_init_reg(d);
> >>>>>
> >>>>>      rc = msi_init(d, XIO3130_MSI_OFFSET, XIO3130_MSI_NR_VECTOR,
> >>>>>diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
> >>>>>index 599768e..e9117b9 100644
> >>>>>--- a/hw/pci-host/apb.c
> >>>>>+++ b/hw/pci-host/apb.c
> >>>>>@@ -636,10 +636,7 @@ static int apb_pci_bridge_initfn(PCIDevice *dev)
> >>>>>  {
> >>>>>      int rc;
> >>>>>
> >>>>>-    rc = pci_bridge_initfn(dev, TYPE_PCI_BUS);
> >>>>>-    if (rc < 0) {
> >>>>>-        return rc;
> >>>>>-    }
> >>>>>+    pci_bridge_initfn(dev, TYPE_PCI_BUS);
> >>>>>
> >>>>>      /*
> >>>>>       * command register:
> >>>>>diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
> >>>>>index 40c97b1..5c30795 100644
> >>>>>--- a/hw/pci/pci_bridge.c
> >>>>>+++ b/hw/pci/pci_bridge.c
> >>>>>@@ -332,7 +332,7 @@ void pci_bridge_reset(DeviceState *qdev)
> >>>>>  }
> >>>>>
> >>>>>  /* default qdev initialization function for PCI-to-PCI bridge */
> >>>>>-int pci_bridge_initfn(PCIDevice *dev, const char *typename)
> >>>>>+void pci_bridge_initfn(PCIDevice *dev, const char *typename)
> >>>>>  {
> >>>>>      PCIBus *parent = dev->bus;
> >>>>>      PCIBridge *br = PCI_BRIDGE(dev);
> >>>>>@@ -378,7 +378,6 @@ int pci_bridge_initfn(PCIDevice *dev, const char
> >>>>>*typename)
> >>>>>      br->windows = pci_bridge_region_init(br);
> >>>>>      QLIST_INIT(&sec_bus->child);
> >>>>>      QLIST_INSERT_HEAD(&parent->child, sec_bus, sibling);
> >>>>>-    return 0;
> >>>>>  }
> >>>>>
> >>>>>  /* default qdev clean up function for PCI-to-PCI bridge */
> >>>>>diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
> >>>>>index 93b621c..ed4aff6 100644
> >>>>>--- a/include/hw/pci/pci_bridge.h
> >>>>>+++ b/include/hw/pci/pci_bridge.h
> >>>>>@@ -48,7 +48,7 @@ void pci_bridge_disable_base_limit(PCIDevice *dev);
> >>>>>  void pci_bridge_reset_reg(PCIDevice *dev);
> >>>>>  void pci_bridge_reset(DeviceState *qdev);
> >>>>>
> >>>>>-int pci_bridge_initfn(PCIDevice *pci_dev, const char *typename);
> >>>>>+void pci_bridge_initfn(PCIDevice *pci_dev, const char *typename);
> >>>>>  void pci_bridge_exitfn(PCIDevice *pci_dev);
> >>>>>
> >>>>>
> >>>>>--
> >>>>>2.1.0
> >>>>>
> >>>>>
> >>>>
> >>>>
> >>>>.
> >>>>
> >>>
> >>
> >>--
> >>Yours Sincerely,
> >>
> >>Cao Jin
> >>
> >
> >
> >.
> >
> 
> -- 
> Yours Sincerely,
> 
> Cao Jin
>