From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=38792 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OX3Bh-0001mp-DM for qemu-devel@nongnu.org; Thu, 08 Jul 2010 22:19:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OX33z-0002GH-K7 for qemu-devel@nongnu.org; Thu, 08 Jul 2010 22:11:40 -0400 Received: from mail.valinux.co.jp ([210.128.90.3]:52451) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OX33z-0002Fz-4Z for qemu-devel@nongnu.org; Thu, 08 Jul 2010 22:11:39 -0400 Date: Fri, 9 Jul 2010 11:07:37 +0900 From: Isaku Yamahata Subject: Re: [Qemu-devel] Re: [PATCH 1/2] pci/bridge: allocate PCIBus dynamically for PCIBridge. Message-ID: <20100709020737.GB22818@valinux.co.jp> References: <8ab6e42be20518d6491e0cd0d5985f7df6912c01.1278037560.git.yamahata@valinux.co.jp> <20100706121852.GA20208@redhat.com> <20100707023858.GB30653@valinux.co.jp> <20100708140432.GB31905@redhat.com> <20100708154318.GA14985@valinux.co.jp> <20100708164934.GB4392@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100708164934.GB4392@redhat.com> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: blauwirbel@gmail.com, qemu-devel@nongnu.org, Gerd Hoffmann On Thu, Jul 08, 2010 at 07:49:35PM +0300, Michael S. Tsirkin wrote: > > > > - to eliminate assumption of pci_bus_new(). > > > > pci_bus_new() assumes that its pci segment is 0. > > > > keep pci_bus_new() as a convenience wrapper of > > > > pci_host_bus_new(segment = 0). Thus we can avoid fixing up > > > > all the caller. > > > > > > We have a single caller, right? I think you mean pci_register_bus? > > > So IIUC, you propose that we add pci_register_host_bus, > > > and make pci_register_bus a compatibility wrapper? > > > Sure, let's just add a comment this is deprecated. > > > > > > I am not sure why do we need an API to deal with secondary bus: > > > it is always a part of the bridge, so all users can and should call > > > pci_bridge_init? > > > > Okay, then how about the following? > > > > For root bus: > > - pci_host_bus_new()/pci_host_bus_new_inplace() > > qbus style api. pci segment must be specified. > > New code should use this. > > I'd prefer a simple _init which works like _inplace. > Allocating memory is simple enough for users. I see. Then - export PCIBus. This is necessary to tell its size. - pci_host_bus_init()/pci_host_bus_cleanup() which don't allocate memory. > > - pci_bus_new() > > qbus style API. > > convenience wrapper for compatibility of > > pci_host_bus_new(pci segment = 0) > > In fact, the only current user piix_pci.c. It's easy to remove it. > > - pci_register_bus() > > old style API. deprecated. > > It has been kept for compatibility so far. > > This will be gradually replaced with pci_host_bus_new() > > Also, let's make these helpers inline: will make it possible > to check code by comparing binary after changes. Agreed. > > > For secondary bus: > > - pci_bridge_init() > > qdev style API. > > New code should use this. > > Well - isn't the way we do this a bit backwards? > I thought the idea was that each device has its own > PCIDeviceInfo qdev structure, instead of the common pci-bridge. > > And then pci_bridge_init (or _setup to avoid reusing existing names) > would be a common function that devices can reuse in their init > functions.. Agreed and in fact I'm having such a patch. Just I think it's next phase of clean up. The patch series creates a library for pci bridge as you suggested before and converts apb_pci.c and dec_pci.c into qdev. > > > - pci_{register, unregister}_secondary_bus(): > > old stype API. deprecated. > > Keep them only for internal use in pci.c > > or they can be easily removed or renamed for qdev style. > > > > For pci device: > > - pci_create() > > qdev style API. > > The transitional function until completion of qdev conversion. > > If the creation of a device tree from config file is implemented, > > this function will be unnecessary. > > > > - pci_create_simple() > > qdev style API. > > convenience function = pci_create() + qdev_init_nofail() > > > > -- > > yamahata > -- yamahata