[Qemu-devel] Re: [PATCH 2/2] pci: don't overwrite pci header type.

[Michael S. Tsirkin]

On Wed, Jun 16, 2010 at 08:38:18PM +0900, Isaku Yamahata wrote:
> On Wed, Jun 16, 2010 at 02:19:44PM +0300, Michael S. Tsirkin wrote:
> > > > > Apb host bridge specifies PCI_HEADER_TYPE_BRIDGE in PCIDeviceInfo,
> > > > > on the other hand pbc_pci_host_init() sets the register
> > > > > to PCI_HEADER_TYPE_NORMAL.
> > > > > To be honest I don't know why it does so, but that is what Blue wants.
> > > > 
> > > > BTW I think it would be prettier to have is_bridge instead of 
> > > > header_type
> > > > as a qdev property. Agree?
> > > 
> > > The spec version 3.0 defines three header types.
> > > 0:normal device, 1:pci-to-pci bridge, 2:card bus bridge
> > > So I'd like the name a bit more generic than is_bridge.
> > > Any suggestion?
> > 
> > Could we just have functions that set up header for
> > each type, such as
> > pci_init_normal_header()
> > pci_init_p2p_bridge_header()
> > pci_init_cardbus_header()
> 
> I see. You mean device specific initialization function should
> call one of them. Then header_type property will be dropped.
> I'll split pci p2p bridge related functions into a file
> at first.
> Then introduce helper functions.

Just to clarify what I meant:
the common pci spec implementation should be in pci.c,
any platform that supports pci will need it.
What I think we want to move to pc_pci_bridge.c or such
is this:
static PCIDeviceInfo bridge_info = {
    .qdev.name    = "pci-bridge",
    .qdev.size    = sizeof(PCIBridge),
    .init         = pci_bridge_initfn,
    .exit         = pci_bridge_exitfn,
    .config_write = pci_bridge_write_config, 
    .header_type  = PCI_HEADER_TYPE_BRIDGE,
    .qdev.props   = (Property[]) {
        DEFINE_PROP_HEX32("vendorid", PCIBridge, vid, 0),
        DEFINE_PROP_HEX32("deviceid", PCIBridge, did, 0),
        DEFINE_PROP_END_OF_LIST(),
    }
};

Because if I understand correctly, this is not "the bridge",
it's just a pci bridge that PC has, but it is currently
instanciated even on platforms where it's unused.
This way we can avoid linking it on these platforms.

But I think the bridge header setup is common
so it should be implemented in a set of
common functions and stay in pci.c, then all bridges
can call these functions.

> -- 
> yamahata