[Qemu-devel] Re: [PATCH v2 5/5] pci_bridge: introduce pci bridge library.

["Michael S. Tsirkin" <mst@redhat.com>]

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.

>  
>  #endif  /* QEMU_PCI_BRIDGE_H */
>  /*
> diff --git a/hw/pci_internals.h b/hw/pci_internals.h
> index fa844ab..6502f83 100644
> --- a/hw/pci_internals.h
> +++ b/hw/pci_internals.h
> @@ -30,11 +30,13 @@ struct PCIBus {
>      int *irq_count;
>  };
>  
> -typedef struct {
> +struct PCIBridge {
>      PCIDevice dev;
> +
> +    /* private member */
>      PCIBus sec_bus;
> -    uint32_t vid;
> -    uint32_t did;
> -} PCIBridge;
> +    pci_map_irq_fn map_irq;
> +    const char *bus_name;
> +};
>  
>  #endif /* QEMU_PCI_INTERNALS_H */
> diff --git a/qemu-common.h b/qemu-common.h
> index 3fb2f0b..d735235 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -219,6 +219,7 @@ typedef struct PCIHostState PCIHostState;
>  typedef struct PCIExpressHost PCIExpressHost;
>  typedef struct PCIBus PCIBus;
>  typedef struct PCIDevice PCIDevice;
> +typedef struct PCIBridge PCIBridge;
>  typedef struct SerialState SerialState;
>  typedef struct IRQState *qemu_irq;
>  typedef struct PCMCIACardState PCMCIACardState;
> -- 
> 1.7.1.1