| Message ID | 1448874044-23808-1-git-send-email-caoj.fnst@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
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 <caoj.fnst@cn.fujitsu.com> 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 > >
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 <caoj.fnst@cn.fujitsu.com> > > 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 >> >> > > > . >
Hi mst friendly ping again... 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 <caoj.fnst@cn.fujitsu.com> >> >> 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 >>> >>> >> >> >> . >> >
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. > 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 <caoj.fnst@cn.fujitsu.com> > >> > >>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 >
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. >> 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 <caoj.fnst@cn.fujitsu.com> >>>> >>>> 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 >> > > > . >
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 <caoj.fnst@cn.fujitsu.com> > >>>> > >>>>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 >
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 <caoj.fnst@cn.fujitsu.com> >>>>>> >>>>>> 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 >> > > > > . >
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);
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 <caoj.fnst@cn.fujitsu.com> --- 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().