qemu-ppc
[Top][All Lists]

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

 From: Scott Wood Subject: Re: [Qemu-ppc] [PATCH 3/5] PPC: E500: Generate dt pci irq map dynamically Date: Wed, 12 Dec 2012 18:20:59 -0600

```On 12/12/2012 06:04:11 PM, Alexander Graf wrote:
```
```
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.
```
```
Sigh. I don't understand the hostility to device tree generation, to the point of enforcing unnatural code grouping and possibly even duplication.
```
```
```But this one might be an exception, because it's not a generic device.
```
```
```
So what happens when we do have a generic device? Duplicate the code in every machine that uses it, or have a parallel "hw/device_dt.c" (or maybe some better hidden place) to factor out common code while (sort of) complying with Anthony's mandate? :-P
```
```
>> 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.
```
```
If we do anything like that, it should probably be to iterate over the devices that actually exist and add interrupt-map entries only for those.
```
```
```So you're indicating you'd like the below patch?
```
```
I think you pasted a bit more than one patch, but yes.

```
```Do you think it's worth the additional code for a simple + and & 3?
```
```
```
It's not about duplicating "+ and & 3" so much as having only one place where the relationship is defined, in case someone wants to alter it (e.g. for adding some other board where the mapping is done differently).
```
```
```address@hidden:/home/agraf/release/qemu> git add hw/ppce500_pci.h
```