From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:40196) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Rl6cV-0003WP-V8 for qemu-devel@nongnu.org; Wed, 11 Jan 2012 17:26:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Rl6cQ-0003kC-6e for qemu-devel@nongnu.org; Wed, 11 Jan 2012 17:26:11 -0500 Message-ID: <4F0E0C13.60008@web.de> Date: Wed, 11 Jan 2012 23:24:19 +0100 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1325894809-17322-1-git-send-email-andreas.faerber@web.de> <1325894809-17322-3-git-send-email-andreas.faerber@web.de> <4BFAF924-2CB2-45DB-9DB4-1148B5A741E7@suse.de> In-Reply-To: <4BFAF924-2CB2-45DB-9DB4-1148B5A741E7@suse.de> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 2/3] prep: Add Raven PCI host SysBus device List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: =?ISO-8859-1?Q?Herv=E9_Poussineau?= , Anthony Liguori , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, "Michael S. Tsirkin" Am 11.01.2012 23:12, schrieb Alexander Graf: > > On 07.01.2012, at 01:06, Andreas Färber wrote: > >> For now, focus on qdev'ification and leave PIC IRQs unchanged. >> >> Signed-off-by: Andreas Färber >> Cc: Hervé Poussineau >> Cc: Michael S. Tsirkin >> Cc: Anthony Liguori >> --- >> hw/prep_pci.c | 41 +++++++++++++++++++++++++++++++---------- >> 1 files changed, 31 insertions(+), 10 deletions(-) >> >> diff --git a/hw/prep_pci.c b/hw/prep_pci.c >> index 741b273..2ff6b8c 100644 >> --- a/hw/prep_pci.c >> +++ b/hw/prep_pci.c >> @@ -114,31 +114,43 @@ PCIBus *pci_prep_init(qemu_irq *pic, >> MemoryRegion *address_space_mem, >> MemoryRegion *address_space_io) >> { > > I'm not sure this is the best way to do this. For e500, we just create the host bridge explicitly in the board file: > > /* PCI */ > dev = sysbus_create_varargs("e500-pcihost", MPC8544_PCI_REGS_BASE, > mpic[pci_irq_nrs[0]], mpic[pci_irq_nrs[1]], > mpic[pci_irq_nrs[2]], mpic[pci_irq_nrs[3]], > NULL); > pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0"); > if (!pci_bus) > printf("couldn't create PCI controller!\n"); > > and that's all the interaction there is between the pci host code and the board code. No calling into functions. The way you're doing it now, the board still needs to call into prep_pci.c which doesn't sound too appealing to me :). That's a TODO for a later patch. As you can see, those lines were not introduced in this series. For the PCI-ISA bridge, we need to get rid of qemu_irq *pic anyway - that's what the commit message refers to. Should clarify that, thanks. >> + DeviceState *dev; >> PREPPCIState *s; >> >> - s = g_malloc0(sizeof(PREPPCIState)); >> - s->bus = pci_register_bus(NULL, "pci", >> + dev = qdev_create(NULL, "raven-pcihost"); >> + s = FROM_SYSBUS(PREPPCIState, sysbus_from_qdev(dev)); >> + s->address_space = address_space_mem; >> + s->bus = pci_register_bus(&s->busdev.qdev, "pci", >> prep_set_irq, prep_map_irq, pic, >> address_space_mem, >> address_space_io, >> 0, 4); > > This should be happening in the host bridge init code. Take a look at e500_pcihost_initfn() in hw/ppce500_pci.c. I don't see how that could work for PReP: prep_set_irq and prep_map_irq need the IRQs allocated by the i8259 on the upcoming i82378 PCI-ISA bridge, which as a PCIDevice needs the PCI host bridge set up already... To allow for board-specific setup (prep vs. 40p) v2 uses pci_bus_new() there and uses pci_bus_irqs() on the board. :) Andreas