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

[Peter Maydell]

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
>> the
>> 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 */
>>     ppc460ex_pcie_init(env);
>> -    /* 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][22],
>> +                                uic[1][0], 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
-- PMM