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
