[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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
address@hidden:/home/agraf/release/qemu> git diff HEAD
address@hidden:/home/agraf/release/qemu> git diff HEAD | cat

What does piping through cat get you?


reply via email to

[Prev in Thread] Current Thread [Next in Thread]