From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45443) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBzK9-0008V3-6t for qemu-devel@nongnu.org; Thu, 24 Dec 2015 01:24:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aBzK6-0007jr-0o for qemu-devel@nongnu.org; Thu, 24 Dec 2015 01:24:29 -0500 Received: from mx1.redhat.com ([209.132.183.28]:38637) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBzK5-0007jj-EW for qemu-devel@nongnu.org; Thu, 24 Dec 2015 01:24:25 -0500 Date: Thu, 24 Dec 2015 08:24:22 +0200 From: "Michael S. Tsirkin" Message-ID: <20151224082243-mutt-send-email-mst@redhat.com> References: <1448874044-23808-1-git-send-email-caoj.fnst@cn.fujitsu.com> <20151130111843-mutt-send-email-mst@redhat.com> <56721589.7000602@cn.fujitsu.com> <567A6101.1020001@cn.fujitsu.com> <20151223153726-mutt-send-email-mst@redhat.com> <567B68D4.9060604@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <567B68D4.9060604@cn.fujitsu.com> Subject: Re: [Qemu-devel] [PATCH] change type of pci_bridge_initfn() to void List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cao jin Cc: qemu-devel@nongnu.org, armbru@redhat.com 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 > >>>> > >>>>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 >