Re: [Qemu-ppc] [PATCH 3/5] PPC: E500: Generate dt pci irq map dynamically

[Alexander Graf]

On 13.12.2012, at 00:43, Scott Wood wrote:

> On 12/12/2012 05:38:32 PM, Alexander Graf wrote:
>> On 12.12.2012, at 19:40, Scott Wood wrote:
>> > On 12/12/2012 08:09:56 AM, Alexander Graf wrote:
>> >> +    for (slot = first_slot; slot < last_slot; slot++) {
>> >> +        for (pci_irq = 0; pci_irq < 4; pci_irq++) {
>> >> +            pci_map[i++] = cpu_to_be32(slot << 11);
>> >> +            pci_map[i++] = cpu_to_be32(0x0);
>> >> +            pci_map[i++] = cpu_to_be32(0x0);
>> >> +            pci_map[i++] = cpu_to_be32(pci_irq + 1);
>> >> +            pci_map[i++] = cpu_to_be32(mpic);
>> >> +            pci_map[i++] = cpu_to_be32(((pci_irq + slot) % 4) + 1);
>> >> +            pci_map[i++] = cpu_to_be32(0x1);
>> >> +        }
>> >>     }
>> >
>> > It would be nice if the slot-to-IRQ calculation were done in only one 
>> > place rather than duplicated here.
>> Sure, what exactly would you suggest to do? :)
> 
> Have a common function to calculate the IRQ given the slot number, and call 
> that both from here and from mpc85xx_pci_map_irq().
> 
>> We can move the whole function to ppce500_pci.c.
>> We could export the function(slot, pci_irq) through the header of 
>> ppce500_pci.c.
> 
> Either works, though I'd lean towards moving this function into ppce500_pci.c.

Well, I'm not sure Anthony would be happy about that. He wanted to keep device 
tree generation inside the machine files. But this one might be an exception, 
because it's not a generic device.

> 
>> We could also try and traverse the pci bus to find the function that is 
>> actually called to convert irq numbers internally, so we potentially support 
>> other pci host controllers.
> 
> Not sure what you mean here.

We could call bus->map_irq(...) with an artificially created PCIDevice struct 
;). But that's pretty hacky.

So you're indicating you'd like the below patch? Do you think it's worth the 
additional code for a simple + and & 3?


Alex

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.cindex 564f654..af6b671 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -34,6 +34,7 @@
 #include "hw/sysbus.h"
 #include "exec-memory.h"
 #include "host-utils.h"
+#include "hw/ppce500_pci.h"
 
 #define BINARY_DEVICE_TREE_FILE    "mpc8544ds.dtb"
 #define UIMAGE_LOAD_BASE           0
@@ -72,6 +73,7 @@ static uint32_t *pci_map_create(void *fdt, uint32_t mpic, int 
first_slot,
     int i = 0;
     int slot;
     int pci_irq;
+    int host_irq;
     int last_slot = first_slot + nr_slots;
     uint32_t *pci_map;
 
@@ -85,7 +87,8 @@ static uint32_t *pci_map_create(void *fdt, uint32_t mpic, int 
first_slot,
             pci_map[i++] = cpu_to_be32(0x0);
             pci_map[i++] = cpu_to_be32(pci_irq + 1);
             pci_map[i++] = cpu_to_be32(mpic);
-            pci_map[i++] = cpu_to_be32(((pci_irq + slot) % 4) + 1);
+            host_irq = ppce500_pci_map_irq_slot(slot, pci_irq);
+            pci_map[i++] = cpu_to_be32(host_irq + 1);
             pci_map[i++] = cpu_to_be32(0x1);
         }
     }
diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
index 561a776..09e3507 100644
--- a/hw/ppce500_pci.c
+++ b/hw/ppce500_pci.c
@@ -19,6 +19,7 @@
 #include "pci.h"
 #include "pci_host.h"
 #include "bswap.h"
+#include "ppce500_pci.h"
 
 #ifdef DEBUG_PCI
 #define pci_debug(fmt, ...) fprintf(stderr, fmt, ## __VA_ARGS__)
@@ -256,7 +257,7 @@ static int mpc85xx_pci_map_irq(PCIDevice *pci_dev, int 
irq_num)
     int devno = pci_dev->devfn >> 3;
     int ret;
 
-    ret = (irq_num + devno) % 4;
+    ret = ppce500_pci_map_irq_slot(devno, irq_num);
 
     pci_debug("%s: devfn %x irq %d -> %d  devno:%x\n", __func__,
            pci_dev->devfn, irq_num, ret, devno);
address@hidden:/home/agraf/release/qemu> git add hw/ppce500_pci.h
address@hidden:/home/agraf/release/qemu> git diff HEAD
address@hidden:/home/agraf/release/qemu> git diff HEAD | cat
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 564f654..af6b671 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -34,6 +34,7 @@
 #include "hw/sysbus.h"
 #include "exec-memory.h"
 #include "host-utils.h"
+#include "hw/ppce500_pci.h"
 
 #define BINARY_DEVICE_TREE_FILE    "mpc8544ds.dtb"
 #define UIMAGE_LOAD_BASE           0
@@ -72,6 +73,7 @@ static uint32_t *pci_map_create(void *fdt, uint32_t mpic, int 
first_slot,
     int i = 0;
     int slot;
     int pci_irq;
+    int host_irq;
     int last_slot = first_slot + nr_slots;
     uint32_t *pci_map;
 
@@ -85,7 +87,8 @@ static uint32_t *pci_map_create(void *fdt, uint32_t mpic, int 
first_slot,
             pci_map[i++] = cpu_to_be32(0x0);
             pci_map[i++] = cpu_to_be32(pci_irq + 1);
             pci_map[i++] = cpu_to_be32(mpic);
-            pci_map[i++] = cpu_to_be32(((pci_irq + slot) % 4) + 1);
+            host_irq = ppce500_pci_map_irq_slot(slot, pci_irq);
+            pci_map[i++] = cpu_to_be32(host_irq + 1);
             pci_map[i++] = cpu_to_be32(0x1);
         }
     }
diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
index 561a776..09e3507 100644
--- a/hw/ppce500_pci.c
+++ b/hw/ppce500_pci.c
@@ -19,6 +19,7 @@
 #include "pci.h"
 #include "pci_host.h"
 #include "bswap.h"
+#include "ppce500_pci.h"
 
 #ifdef DEBUG_PCI
 #define pci_debug(fmt, ...) fprintf(stderr, fmt, ## __VA_ARGS__)
@@ -256,7 +257,7 @@ static int mpc85xx_pci_map_irq(PCIDevice *pci_dev, int 
irq_num)
     int devno = pci_dev->devfn >> 3;
     int ret;
 
-    ret = (irq_num + devno) % 4;
+    ret = ppce500_pci_map_irq_slot(devno, irq_num);
 
     pci_debug("%s: devfn %x irq %d -> %d  devno:%x\n", __func__,
            pci_dev->devfn, irq_num, ret, devno);
diff --git a/hw/ppce500_pci.h b/hw/ppce500_pci.h
new file mode 100644
index 0000000..61f773e
--- /dev/null
+++ b/hw/ppce500_pci.h
@@ -0,0 +1,9 @@
+#ifndef PPCE500_PCI_H
+#define PPCE500_PCI_H
+
+static inline int ppce500_pci_map_irq_slot(int devno, int irq_num)
+{
+    return (devno + irq_num) % 4;
+}
+
+#endif