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

Submitted by Alexander Graf on Dec. 12, 2012, 2:09 p.m.

Details

Message ID 1355321398-23393-4-git-send-email-agraf@suse.de
State New
Headers show

Commit Message

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(-)

Comments

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 hide | download patch | download mbox

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;
 }