qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Isaku Yamahata <yamahata@valinux.co.jp>
Cc: blauwirbel@gmail.com, qemu-devel@nongnu.org,
	Gerd Hoffmann <kraxel@redhat.com>
Subject: [Qemu-devel] Re: [PATCH 1/2] pci/bridge: allocate PCIBus dynamically for PCIBridge.
Date: Thu, 8 Jul 2010 17:04:32 +0300	[thread overview]
Message-ID: <20100708140432.GB31905@redhat.com> (raw)
In-Reply-To: <20100707023858.GB30653@valinux.co.jp>

On Wed, Jul 07, 2010 at 11:38:58AM +0900, Isaku Yamahata wrote:
> But you claim it's only for root bus, not for secondary bus.

It is currently, isn't it?

> Now I realized why you've rejected such patches so far.
> Then, you also mean the current pci_register_secondary_bus() is broken.

Sorry about being dense, what is broken?

> I also think it's broken. So how do we want to fix it?
> My idea is as follows.
> 
> - introduce something like pci_secondary_bus_new()
>   (pci_sec_bus_new() for short?) for secondary bus. 
>   fix pci_register_secondary_bus() with it.
> 
> - introduce something like pci_host_bus_new() (or pci_root_bus_new()?)
>   for pci host bus which is more generic than pci_bus_new().
>   It's for
>   - to avoid confusion.

IMHO the confusion comes from the fact we have too
many functions that do almost, but not quite, the same
thing, and the function names do not say anything.

We have a ton of 5 line functions with names like
_allocate_inplace, _new, _register, _simple

>   - 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?

-- 
MST

  parent reply	other threads:[~2010-07-08 14:09 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-02  2:30 [Qemu-devel] [PATCH 0/2] pci: split out bridge code into pci_bridge Isaku Yamahata
2010-07-02  2:30 ` [Qemu-devel] [PATCH 1/2] pci/bridge: allocate PCIBus dynamically for PCIBridge Isaku Yamahata
2010-07-06 12:18   ` [Qemu-devel] " Michael S. Tsirkin
2010-07-07  2:38     ` Isaku Yamahata
2010-07-07 11:47       ` Michael S. Tsirkin
2010-07-08  6:39         ` Isaku Yamahata
2010-07-08 14:04       ` Michael S. Tsirkin [this message]
2010-07-08 15:43         ` Isaku Yamahata
2010-07-08 16:49           ` Michael S. Tsirkin
2010-07-09  2:07             ` Isaku Yamahata
2010-07-16  1:46             ` Isaku Yamahata
2010-07-16  7:35               ` Michael S. Tsirkin
2010-07-02  2:30 ` [Qemu-devel] [PATCH 2/2] pci/bridge: split out pci bridge code into pci_bridge.c from pci.c Isaku Yamahata

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100708140432.GB31905@redhat.com \
    --to=mst@redhat.com \
    --cc=blauwirbel@gmail.com \
    --cc=kraxel@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=yamahata@valinux.co.jp \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).