From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=60011 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OYKN8-0005bk-IF for qemu-devel@nongnu.org; Mon, 12 Jul 2010 10:52:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OYKN4-0004Yq-E7 for qemu-devel@nongnu.org; Mon, 12 Jul 2010 10:52:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54007) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OYKN3-0004Yc-UY for qemu-devel@nongnu.org; Mon, 12 Jul 2010 10:52:38 -0400 Date: Mon, 12 Jul 2010 17:47:20 +0300 From: "Michael S. Tsirkin" Message-ID: <20100712144720.GA5005@redhat.com> References: <20100712121000.GA31649@redhat.com> <20100712132824.GC31689@valinux.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100712132824.GC31689@valinux.co.jp> Subject: [Qemu-devel] Re: [PATCH v2 5/5] pci_bridge: introduce pci bridge library. List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Isaku Yamahata Cc: blauwirbel@gmail.com, qemu-devel@nongnu.org On Mon, Jul 12, 2010 at 10:28:24PM +0900, Isaku Yamahata wrote: > On Mon, Jul 12, 2010 at 03:10:00PM +0300, Michael S. Tsirkin wrote: > > On Mon, Jul 12, 2010 at 07:36:44PM +0900, Isaku Yamahata wrote: > > > diff --git a/hw/pci_bridge.h b/hw/pci_bridge.h > > > index ddb2c82..4697c7a 100644 > > > --- a/hw/pci_bridge.h > > > +++ b/hw/pci_bridge.h > > > @@ -29,13 +29,27 @@ > > > #include "pci.h" > > > > > > PCIDevice *pci_bridge_get_device(PCIBus *bus); > > > +PCIBus *pci_bridge_get_sec_bus(PCIBridge *br); > > > > > > -pcibus_t pci_bridge_get_base(PCIDevice *bridge, uint8_t type); > > > -pcibus_t pci_bridge_get_limit(PCIDevice *bridge, uint8_t type); > > > +pcibus_t pci_bridge_get_base(const PCIDevice *bridge, uint8_t type); > > > +pcibus_t pci_bridge_get_limit(const PCIDevice *bridge, uint8_t type); > > > > > > -PCIBus *pci_bridge_init(PCIBus *bus, int devfn, bool multifunction, > > > - uint16_t vid, uint16_t did, > > > - pci_map_irq_fn map_irq, const char *name); > > > +void pci_bridge_write_config(PCIDevice *d, > > > + uint32_t address, uint32_t val, int len); > > > +void pci_bridge_reset_reg(PCIDevice *dev); > > > +void pci_bridge_reset(DeviceState *qdev); > > > + > > > +int pci_bridge_initfn(PCIDevice *pci_dev); > > > +int pci_bridge_exitfn(PCIDevice *pci_dev); > > > + > > > +void pci_bridge_qdev_register(PCIDeviceInfo *info); > > > + > > > +PCIBridge *pci_bridge_create(PCIBus *bus, int devfn, bool multifunction, > > > + pci_map_irq_fn map_irq, > > > + const char *name, const char *bus_name); > > > +PCIBridge *pci_bridge_create_simple(PCIBus *bus, int devfn, bool multifunction, > > > + pci_map_irq_fn map_irq, > > > + const char *name, const char *bus_name); > > > > The APIs leave much to be desired. _simple and regular are same? What does _register do? > > > > We really should just use qdev: Can't we use pci_qdev_register_many and > > pci_create to create the bridge? Long term, all pci_create variants > > should go and get replaced with qdev_create. > > If struct PCIBridge is exported, those three can be eliminated. > I think it is okay to export struct PCIBridge, but it would not > be a good idea to export PCIBus which is embedded in PCIBridge::sec_bus. > > So how should we go? > - export both PCIBus and PCIBridge. > > - make PCIBridge::sec_bus pointer, and export PCIBridge. > And kill register, create, create_simple. > v1 patch. Although you rejected it, I suppose you didn't see > this issue. > > - introduce wrapper functions to convert types. > This patch. > > - better alternatives? Let's export and see where this leads us. > -- > yamahata