Re: [PATCH/RFC] powerpc: Add MPC5200 Interrupt Controller support.

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

My only remaining comment is what I said earlier about the way you
divided the interrupt in levels.

In fact, I've finally dug into the doc and found out that the situation
is even less clear :)

The whole point of this division is to avoid special cases and properly
deal with the separation in different HW registers. We have basically 4
register sets controlling irqs:

 - intr->ctrl
 - intr->main_mask
 - intr->per_mask
 - sdma (different device)

Now, your current setup ends up defining a "level" and 5 irq_chips !!!

 - intr->ctrl bit 11 has it's own chip
 - intr->ctrl other bits or intr->main_mask (2 different chips)
 - intr->per_mask a chip 
 - sdma a chip

which seems to totally gross since the first one is basically "main" irq
0, and has no reason whatsoever to be kept separate from "main". Which
would give us something much more sensible with 4 levels and 4 chips:

 - intr->ctrl based interrupts (crit and main, crit being just main 0)
 - intr->main_mask
 - intr->per_mask
 - sdma

In that case, you have a nice 1 level == 1 irq_chip == one register set.

Now, in addition to that, we have another issue I haven't spotted
before, but it might be worth considering:

Do we actually want the sdma interrupts handled there ? Because if you
look closely, the SDMA is basically a cascaded interrupt controller. It
hangs of per interrupt 0 :)

Thus we could simply remove the code for it from that driver and
implement it as as separate controller with a separate interrupt domain.
I know you won't like that idea because it means having a different
interrupt tree but it's worth having the discussion.

Ben.