Patchwork [3/5] PPC: E500: Generate dt pci irq map dynamically

login
register
mail settings
Submitter Alexander Graf
Date Dec. 12, 2012, 2:09 p.m.
Message ID <1355321398-23393-4-git-send-email-agraf@suse.de>
Download mbox | patch
Permalink /patch/205536/
State New
Headers show

Comments

Alexander Graf - Dec. 12, 2012, 2:09 p.m.
Today we're hardcoding the PCI interrupt map in the e500 machine file.
Instead, let's write it dynamically so that different machine types
can have different slot properties.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppc/e500.c |   51 +++++++++++++++++++++++++++++++--------------------
 1 files changed, 31 insertions(+), 20 deletions(-)
Scott Wood - Dec. 12, 2012, 6:40 p.m.
On 12/12/2012 08:09:56 AM, Alexander Graf wrote:
> Today we're hardcoding the PCI interrupt map in the e500 machine file.
> Instead, let's write it dynamically so that different machine types
> can have different slot properties.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  hw/ppc/e500.c |   51  
> +++++++++++++++++++++++++++++++--------------------
>  1 files changed, 31 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index 1034f93..ebb6d96 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -66,25 +66,33 @@ struct boot_info
>      uint32_t entry;
>  };
> 
> -static void pci_map_create(void *fdt, uint32_t *pci_map, uint32_t  
> mpic)
> +static uint32_t *pci_map_create(void *fdt, uint32_t mpic, int  
> first_slot,
> +                                int nr_slots, int *len)
>  {
> -    int i;
> -    const uint32_t tmp[] = {
> -                             /* IDSEL 0x11 J17 Slot 1 */
> -                             0x8800, 0x0, 0x0, 0x1, mpic, 0x2, 0x1,
> -                             0x8800, 0x0, 0x0, 0x2, mpic, 0x3, 0x1,
> -                             0x8800, 0x0, 0x0, 0x3, mpic, 0x4, 0x1,
> -                             0x8800, 0x0, 0x0, 0x4, mpic, 0x1, 0x1,
> -
> -                             /* IDSEL 0x12 J16 Slot 2 */
> -                             0x9000, 0x0, 0x0, 0x1, mpic, 0x3, 0x1,
> -                             0x9000, 0x0, 0x0, 0x2, mpic, 0x4, 0x1,
> -                             0x9000, 0x0, 0x0, 0x3, mpic, 0x2, 0x1,
> -                             0x9000, 0x0, 0x0, 0x4, mpic, 0x1, 0x1,
> -                           };
> -    for (i = 0; i < (7 * 8); i++) {
> -        pci_map[i] = cpu_to_be32(tmp[i]);
> +    int i = 0;
> +    int slot;
> +    int pci_irq;
> +    int last_slot = first_slot + nr_slots;
> +    uint32_t *pci_map;
> +
> +    *len = nr_slots * 4 * 7 * sizeof(uint32_t);
> +    pci_map = g_malloc(*len);
> +
> +    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.

-Scott
Alexander Graf - Dec. 12, 2012, 11:38 p.m.
On 12.12.2012, at 19:40, Scott Wood wrote:

> On 12/12/2012 08:09:56 AM, Alexander Graf wrote:
>> Today we're hardcoding the PCI interrupt map in the e500 machine file.
>> Instead, let's write it dynamically so that different machine types
>> can have different slot properties.
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> hw/ppc/e500.c |   51 +++++++++++++++++++++++++++++++--------------------
>> 1 files changed, 31 insertions(+), 20 deletions(-)
>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>> index 1034f93..ebb6d96 100644
>> --- a/hw/ppc/e500.c
>> +++ b/hw/ppc/e500.c
>> @@ -66,25 +66,33 @@ struct boot_info
>>     uint32_t entry;
>> };
>> -static void pci_map_create(void *fdt, uint32_t *pci_map, uint32_t mpic)
>> +static uint32_t *pci_map_create(void *fdt, uint32_t mpic, int first_slot,
>> +                                int nr_slots, int *len)
>> {
>> -    int i;
>> -    const uint32_t tmp[] = {
>> -                             /* IDSEL 0x11 J17 Slot 1 */
>> -                             0x8800, 0x0, 0x0, 0x1, mpic, 0x2, 0x1,
>> -                             0x8800, 0x0, 0x0, 0x2, mpic, 0x3, 0x1,
>> -                             0x8800, 0x0, 0x0, 0x3, mpic, 0x4, 0x1,
>> -                             0x8800, 0x0, 0x0, 0x4, mpic, 0x1, 0x1,
>> -
>> -                             /* IDSEL 0x12 J16 Slot 2 */
>> -                             0x9000, 0x0, 0x0, 0x1, mpic, 0x3, 0x1,
>> -                             0x9000, 0x0, 0x0, 0x2, mpic, 0x4, 0x1,
>> -                             0x9000, 0x0, 0x0, 0x3, mpic, 0x2, 0x1,
>> -                             0x9000, 0x0, 0x0, 0x4, mpic, 0x1, 0x1,
>> -                           };
>> -    for (i = 0; i < (7 * 8); i++) {
>> -        pci_map[i] = cpu_to_be32(tmp[i]);
>> +    int i = 0;
>> +    int slot;
>> +    int pci_irq;
>> +    int last_slot = first_slot + nr_slots;
>> +    uint32_t *pci_map;
>> +
>> +    *len = nr_slots * 4 * 7 * sizeof(uint32_t);
>> +    pci_map = g_malloc(*len);
>> +
>> +    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? :)

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. 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.


Alex
Scott Wood - Dec. 12, 2012, 11:43 p.m.
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.

> 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.

-Scott

Patch

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 1034f93..ebb6d96 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -66,25 +66,33 @@  struct boot_info
     uint32_t entry;
 };
 
-static void pci_map_create(void *fdt, uint32_t *pci_map, uint32_t mpic)
+static uint32_t *pci_map_create(void *fdt, uint32_t mpic, int first_slot,
+                                int nr_slots, int *len)
 {
-    int i;
-    const uint32_t tmp[] = {
-                             /* IDSEL 0x11 J17 Slot 1 */
-                             0x8800, 0x0, 0x0, 0x1, mpic, 0x2, 0x1,
-                             0x8800, 0x0, 0x0, 0x2, mpic, 0x3, 0x1,
-                             0x8800, 0x0, 0x0, 0x3, mpic, 0x4, 0x1,
-                             0x8800, 0x0, 0x0, 0x4, mpic, 0x1, 0x1,
-
-                             /* IDSEL 0x12 J16 Slot 2 */
-                             0x9000, 0x0, 0x0, 0x1, mpic, 0x3, 0x1,
-                             0x9000, 0x0, 0x0, 0x2, mpic, 0x4, 0x1,
-                             0x9000, 0x0, 0x0, 0x3, mpic, 0x2, 0x1,
-                             0x9000, 0x0, 0x0, 0x4, mpic, 0x1, 0x1,
-                           };
-    for (i = 0; i < (7 * 8); i++) {
-        pci_map[i] = cpu_to_be32(tmp[i]);
+    int i = 0;
+    int slot;
+    int pci_irq;
+    int last_slot = first_slot + nr_slots;
+    uint32_t *pci_map;
+
+    *len = nr_slots * 4 * 7 * sizeof(uint32_t);
+    pci_map = g_malloc(*len);
+
+    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);
+        }
     }
+
+    assert((i * sizeof(uint32_t)) == *len);
+
+    return pci_map;
 }
 
 static void dt_serial_create(void *fdt, unsigned long long offset,
@@ -132,7 +140,8 @@  static int ppce500_load_device_tree(CPUPPCState *env,
     char gutil[128];
     char pci[128];
     char msi[128];
-    uint32_t pci_map[7 * 8];
+    uint32_t *pci_map = NULL;
+    int len;
     uint32_t pci_ranges[14] =
         {
             0x2000000, 0x0, 0xc0000000,
@@ -329,8 +338,9 @@  static int ppce500_load_device_tree(CPUPPCState *env,
     qemu_devtree_setprop_string(fdt, pci, "device_type", "pci");
     qemu_devtree_setprop_cells(fdt, pci, "interrupt-map-mask", 0xf800, 0x0,
                                0x0, 0x7);
-    pci_map_create(fdt, pci_map, qemu_devtree_get_phandle(fdt, mpic));
-    qemu_devtree_setprop(fdt, pci, "interrupt-map", pci_map, sizeof(pci_map));
+    pci_map = pci_map_create(fdt, qemu_devtree_get_phandle(fdt, mpic),
+                             0x11, 2, &len);
+    qemu_devtree_setprop(fdt, pci, "interrupt-map", pci_map, len);
     qemu_devtree_setprop_phandle(fdt, pci, "interrupt-parent", mpic);
     qemu_devtree_setprop_cells(fdt, pci, "interrupts", 24, 2);
     qemu_devtree_setprop_cells(fdt, pci, "bus-range", 0, 255);
@@ -364,6 +374,7 @@  done:
     ret = fdt_size;
 
 out:
+    g_free(pci_map);
 
     return ret;
 }