Re: [PATCH 3/3] sam460ex: Clean up irq mapping
On 12/25/20 3:07 PM, BALATON Zoltan wrote:
> Avoid mapping multiple interrupts to the same irq. Instead map them to
> the 4 PCI interrupts and use an or-gate in the board to connect them
> to the interrupt controller. This does not fix any known problem but
> does not seem to cause a new problem either and may be cleaner at least.
>
> Signed-off-by: BALATON Zoltan
Tested-by: Guenter Roeck
> ---
> hw/ppc/Kconfig | 1 +
> hw/ppc/ppc440_pcix.c | 28 ++--
> hw/ppc/sam460ex.c| 16 +---
> 3 files changed, 28 insertions(+), 17 deletions(-)
>
> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
> index 5893f80909..fabdb1a96f 100644
> --- a/hw/ppc/Kconfig
> +++ b/hw/ppc/Kconfig
> @@ -58,6 +58,7 @@ config SAM460EX
> select PFLASH_CFI01
> select IDE_SII3112
> select M41T80
> +select OR_IRQ
> select PPC440
> select SM501
> select SMBUS_EEPROM
> diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
> index ee952314c8..504decbbc2 100644
> --- a/hw/ppc/ppc440_pcix.c
> +++ b/hw/ppc/ppc440_pcix.c
> @@ -57,8 +57,8 @@ struct PPC440PCIXState {
> PCIDevice *dev;
> struct PLBOutMap pom[PPC440_PCIX_NR_POMS];
> struct PLBInMap pim[PPC440_PCIX_NR_PIMS];
> +qemu_irq irq[PCI_NUM_PINS];
> uint32_t sts;
> -qemu_irq irq;
> AddressSpace bm_as;
> MemoryRegion bm;
>
> @@ -415,24 +415,20 @@ static void ppc440_pcix_reset(DeviceState *dev)
> s->sts = 0;
> }
>
> -/* All pins from each slot are tied to a single board IRQ.
> - * This may need further refactoring for other boards. */
> static int ppc440_pcix_map_irq(PCIDevice *pci_dev, int irq_num)
> {
> -trace_ppc440_pcix_map_irq(pci_dev->devfn, irq_num, 0);
> -return 0;
> +int n = (irq_num + PCI_SLOT(pci_dev->devfn)) % PCI_NUM_PINS;
> +
> +trace_ppc440_pcix_map_irq(pci_dev->devfn, irq_num, n);
> +return n;
> }
>
> static void ppc440_pcix_set_irq(void *opaque, int irq_num, int level)
> {
> -qemu_irq *pci_irq = opaque;
> +qemu_irq *pci_irqs = opaque;
>
> trace_ppc440_pcix_set_irq(irq_num);
> -if (irq_num < 0) {
> -error_report("%s: PCI irq %d", __func__, irq_num);
> -return;
> -}
> -qemu_set_irq(*pci_irq, level);
> +qemu_set_irq(pci_irqs[irq_num], level);
> }
>
> static AddressSpace *ppc440_pcix_set_iommu(PCIBus *b, void *opaque, int
> devfn)
> @@ -472,15 +468,19 @@ static void ppc440_pcix_realize(DeviceState *dev, Error
> **errp)
> SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> PPC440PCIXState *s;
> PCIHostState *h;
> +int i;
>
> h = PCI_HOST_BRIDGE(dev);
> s = PPC440_PCIX_HOST_BRIDGE(dev);
>
> -sysbus_init_irq(sbd, >irq);
> +for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
> +sysbus_init_irq(sbd, >irq[i]);
> +}
> memory_region_init(>busmem, OBJECT(dev), "pci bus memory",
> UINT64_MAX);
> h->bus = pci_register_root_bus(dev, NULL, ppc440_pcix_set_irq,
> - ppc440_pcix_map_irq, >irq, >busmem,
> - get_system_io(), PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS);
> + ppc440_pcix_map_irq, s->irq, >busmem,
> + get_system_io(), PCI_DEVFN(0, 0),
> ARRAY_SIZE(s->irq),
> + TYPE_PCI_BUS);
>
> s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0),
> "ppc4xx-host-bridge");
>
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index 14e6583eb0..59b19fbca1 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -33,6 +33,7 @@
> #include "sysemu/qtest.h"
> #include "sysemu/reset.h"
> #include "hw/sysbus.h"
> +#include "hw/or-irq.h"
> #include "hw/char/serial.h"
> #include "hw/i2c/ppc4xx_i2c.h"
> #include "hw/i2c/smbus_eeprom.h"
> @@ -292,7 +293,7 @@ static void sam460ex_init(MachineState *machine)
> SysBusDevice *sbdev;
> struct boot_info *boot_info;
> uint8_t *spd_data;
> -int success;
> +int i, success;
>
> cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
> env = >env;
> @@ -382,13 +383,22 @@ static void sam460ex_init(MachineState *machine)
>
> /* PCI bus */
> ppc460ex_pcie_init(env);
> -/* All PCI irqs are connected to the same UIC pin (cf. UBoot source) */
> -dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec0, uic[1][0]);
> +dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec0, NULL);
> pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
> if (!pci_bus) {
> error_report("couldn't create PCI controller!");
> exit(1);
> }
> +/* All PCI irqs are connected to the same UIC pin (cf. UBoot source) */
> +sbdev = SYS_BUS_DEVICE(dev);
> +dev = qdev_new(TYPE_OR_IRQ);
> +object_property_set_int(OBJECT(dev), "num-lines", PCI_NUM_PINS,
> +_fatal);
> +qdev_realize_and_unref(dev, NULL, _fatal);
> +for (i = 0; i < PCI_NUM_PINS; i++) {
> +sysbus_connect_irq(sbdev, i,
[PATCH 3/3] sam460ex: Clean up irq mapping
Avoid mapping multiple interrupts to the same irq. Instead map them to
the 4 PCI interrupts and use an or-gate in the board to connect them
to the interrupt controller. This does not fix any known problem but
does not seem to cause a new problem either and may be cleaner at least.
Signed-off-by: BALATON Zoltan
---
hw/ppc/Kconfig | 1 +
hw/ppc/ppc440_pcix.c | 28 ++--
hw/ppc/sam460ex.c| 16 +---
3 files changed, 28 insertions(+), 17 deletions(-)
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index 5893f80909..fabdb1a96f 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -58,6 +58,7 @@ config SAM460EX
select PFLASH_CFI01
select IDE_SII3112
select M41T80
+select OR_IRQ
select PPC440
select SM501
select SMBUS_EEPROM
diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
index ee952314c8..504decbbc2 100644
--- a/hw/ppc/ppc440_pcix.c
+++ b/hw/ppc/ppc440_pcix.c
@@ -57,8 +57,8 @@ struct PPC440PCIXState {
PCIDevice *dev;
struct PLBOutMap pom[PPC440_PCIX_NR_POMS];
struct PLBInMap pim[PPC440_PCIX_NR_PIMS];
+qemu_irq irq[PCI_NUM_PINS];
uint32_t sts;
-qemu_irq irq;
AddressSpace bm_as;
MemoryRegion bm;
@@ -415,24 +415,20 @@ static void ppc440_pcix_reset(DeviceState *dev)
s->sts = 0;
}
-/* All pins from each slot are tied to a single board IRQ.
- * This may need further refactoring for other boards. */
static int ppc440_pcix_map_irq(PCIDevice *pci_dev, int irq_num)
{
-trace_ppc440_pcix_map_irq(pci_dev->devfn, irq_num, 0);
-return 0;
+int n = (irq_num + PCI_SLOT(pci_dev->devfn)) % PCI_NUM_PINS;
+
+trace_ppc440_pcix_map_irq(pci_dev->devfn, irq_num, n);
+return n;
}
static void ppc440_pcix_set_irq(void *opaque, int irq_num, int level)
{
-qemu_irq *pci_irq = opaque;
+qemu_irq *pci_irqs = opaque;
trace_ppc440_pcix_set_irq(irq_num);
-if (irq_num < 0) {
-error_report("%s: PCI irq %d", __func__, irq_num);
-return;
-}
-qemu_set_irq(*pci_irq, level);
+qemu_set_irq(pci_irqs[irq_num], level);
}
static AddressSpace *ppc440_pcix_set_iommu(PCIBus *b, void *opaque, int devfn)
@@ -472,15 +468,19 @@ static void ppc440_pcix_realize(DeviceState *dev, Error
**errp)
SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
PPC440PCIXState *s;
PCIHostState *h;
+int i;
h = PCI_HOST_BRIDGE(dev);
s = PPC440_PCIX_HOST_BRIDGE(dev);
-sysbus_init_irq(sbd, >irq);
+for (i = 0; i < ARRAY_SIZE(s->irq); i++) {
+sysbus_init_irq(sbd, >irq[i]);
+}
memory_region_init(>busmem, OBJECT(dev), "pci bus memory", UINT64_MAX);
h->bus = pci_register_root_bus(dev, NULL, ppc440_pcix_set_irq,
- ppc440_pcix_map_irq, >irq, >busmem,
- get_system_io(), PCI_DEVFN(0, 0), 1, TYPE_PCI_BUS);
+ ppc440_pcix_map_irq, s->irq, >busmem,
+ get_system_io(), PCI_DEVFN(0, 0), ARRAY_SIZE(s->irq),
+ TYPE_PCI_BUS);
s->dev = pci_create_simple(h->bus, PCI_DEVFN(0, 0), "ppc4xx-host-bridge");
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 14e6583eb0..59b19fbca1 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -33,6 +33,7 @@
#include "sysemu/qtest.h"
#include "sysemu/reset.h"
#include "hw/sysbus.h"
+#include "hw/or-irq.h"
#include "hw/char/serial.h"
#include "hw/i2c/ppc4xx_i2c.h"
#include "hw/i2c/smbus_eeprom.h"
@@ -292,7 +293,7 @@ static void sam460ex_init(MachineState *machine)
SysBusDevice *sbdev;
struct boot_info *boot_info;
uint8_t *spd_data;
-int success;
+int i, success;
cpu = POWERPC_CPU(cpu_create(machine->cpu_type));
env = >env;
@@ -382,13 +383,22 @@ static void sam460ex_init(MachineState *machine)
/* PCI bus */
ppc460ex_pcie_init(env);
-/* All PCI irqs are connected to the same UIC pin (cf. UBoot source) */
-dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec0, uic[1][0]);
+dev = sysbus_create_simple("ppc440-pcix-host", 0xc0ec0, NULL);
pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci.0");
if (!pci_bus) {
error_report("couldn't create PCI controller!");
exit(1);
}
+/* All PCI irqs are connected to the same UIC pin (cf. UBoot source) */
+sbdev = SYS_BUS_DEVICE(dev);
+dev = qdev_new(TYPE_OR_IRQ);
+object_property_set_int(OBJECT(dev), "num-lines", PCI_NUM_PINS,
+_fatal);
+qdev_realize_and_unref(dev, NULL, _fatal);
+for (i = 0; i < PCI_NUM_PINS; i++) {
+sysbus_connect_irq(sbdev, i, qdev_get_gpio_in(dev, i));
+}
+qdev_connect_gpio_out(dev, 0, uic[1][0]);
memory_region_init_alias(isa, NULL, "isa_mmio", get_system_io(),
0, 0x1);
memory_region_add_subregion(get_system_memory(), 0xc0800, isa);
--
2.21.3
