[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connecti

From: BALATON Zoltan
Subject: Re: [Qemu-ppc] [Qemu-devel] [PATCH] sam460ex: Fix PCI interrupt connections
Date: Tue, 31 Jul 2018 01:00:46 +0200 (CEST)
User-agent: Alpine 2.21 (BSF 202 2017-01-01)

On Mon, 30 Jul 2018, Peter Maydell wrote:
On 30 July 2018 at 12:06, BALATON Zoltan <address@hidden> wrote:
On Mon, 30 Jul 2018, Sebastian Bauer wrote:

The four interrupts of the PCI bus are connected to the same UIC pin on
real Sam460ex. Evidence for this can be found in the UBoot source for the
Sam460ex in the Sam460ex.c file where PCI_INTERRUPT_LINE in written. This
change brings the connection in line with this.

This fixes the problem that can be observed when adding further PCI cards
that get their interrupt rotated to other interrupts than PCI INT A. In
particular, the bug was observed and verified to be fixed (after this
change) with an additional OHCI PCI card.

Signed-off-by: Sebastian Bauer <address@hidden>
hw/ppc/sam460ex.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 0999efcc1e..b2b22f280d 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -515,9 +515,9 @@ static void sam460ex_init(MachineState *machine)

    /* PCI bus */
-    /* FIXME: is this correct? */
+    /* All PCI ints are connected to the same UIC pin (cf. UBoot source)
    dev = sysbus_create_varargs("ppc440-pcix-host", 0xc0ec00000,
-                                uic[1][0], uic[1][20], uic[1][21],
+                                uic[1][0], uic[1][0], uic[1][0],

I don't understand QOM. Does this really work? It will ultimately do

qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 0, uic[1][0]);
qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 1, uic[1][0]);
qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 2, uic[1][0]);
qdev_connect_gpio_out_named(dev, SYSBUS_DEVICE_GPIO_IRQ, 3, uic[1][0]);

You are correct; this will not do the intended thing. If you
want to wire up multiple outputs which are logically ORed
together into a single input, you need to instantiate an
OR gate for that (we have the TYPE_OR_IRQ for this).

Thanks for confirming it. However after reading hw/pci/pci.c I still don't see where these properties are used at all. It looks like it will just call the map_irq and set_irq functions specified in pci_register_root_bus passing them the irq array given in opaque and these will change irq lines directly. So I think we could just change the map function to map everything to the first line and then the other lines don't matter at all. We could even get rid of them and change ppc440_pcix to have a single line as used in the sam460ex (which is the only board using this model currently so we can add more complexity when/if anything else needs it to be different). Is there anything why this simple solution would not work that justifies adding more complexity now?


reply via email to

[Prev in Thread] Current Thread [Next in Thread]