From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=55338 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OOs9b-0000mq-Bq for qemu-devel@nongnu.org; Wed, 16 Jun 2010 08:55:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OOs2k-0003Rr-Q9 for qemu-devel@nongnu.org; Wed, 16 Jun 2010 08:48:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:27163) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OOs2k-0003Ri-IB for qemu-devel@nongnu.org; Wed, 16 Jun 2010 08:48:34 -0400 Date: Wed, 16 Jun 2010 15:43:41 +0300 From: "Michael S. Tsirkin" Message-ID: <20100616124341.GA5683@redhat.com> References: <20100615091207.GA1365@redhat.com> <20100616022002.GB7932@valinux.co.jp> <20100616085425.GA4637@redhat.com> <20100616094353.GB23748@valinux.co.jp> <20100616111944.GA5661@redhat.com> <20100616113818.GA25458@valinux.co.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100616113818.GA25458@valinux.co.jp> Subject: [Qemu-devel] Re: [PATCH 2/2] pci: don't overwrite pci header type. 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 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