From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34802) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aC0tP-0000lK-Jn for qemu-devel@nongnu.org; Thu, 24 Dec 2015 03:05:01 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aC0tO-0003Uk-3O for qemu-devel@nongnu.org; Thu, 24 Dec 2015 03:04:59 -0500 Received: from [59.151.112.132] (port=42111 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aC0tN-0003Rx-EY for qemu-devel@nongnu.org; Thu, 24 Dec 2015 03:04:58 -0500 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> <20151224082243-mutt-send-email-mst@redhat.com> From: Cao jin Message-ID: <567B94A1.2040804@cn.fujitsu.com> Date: Thu, 24 Dec 2015 14:45:53 +0800 MIME-Version: 1.0 In-Reply-To: <20151224082243-mutt-send-email-mst@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit 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: "Michael S. Tsirkin" Cc: qemu-devel@nongnu.org, armbru@redhat.com On 12/24/2015 02:24 PM, Michael S. Tsirkin wrote: > 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? > Really really sorry about my carelessness, I just find compiling fail before this mail, really sorry about such kind of silly mistake. I know I have made many silly mistakes before, ashamed of these. I am working hard to avoid these silly mistake later. Again, really sorry, and really thanks the time you spend on this. Btw, I just thought maybe this patch could be splitted into 2 independent patches, just like v2 I sent. >>>> 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 >> > > > > . > -- Yours Sincerely, Cao Jin