[Cbe-oss-dev] [RFC/PATCH] adding support for direct MBX interrupt on Axon based platform.

[Jean-Christophe Dubois]

On Friday 18 May 2007 17:54:58 Arnd Bergmann wrote:
> On Friday 18 May 2007, Jean-Christophe Dubois wrote:
> > On Friday 18 May 2007 16:09:19 Arnd Bergmann wrote:
> > > Hi JC,
> > >
> > > Thanks for sending out this patch, I know people have been waiting
> > > for it. I never understood how it's wired and how it should be
> > > in the device-tree, this makes things much clearer.
> > >
> > > I think the mpic device node actually should be changed to point to
> > > the c3po as its parent, but that would mean that you can't boot
> > > an old linux kernel on a new device tree, because the code to
> > > handle c3po is missing there.
> > >
> > > Is the priority the only way you can distinguish mpic interrupts
> > > from non-mpic ones?
> >
> > The programmable parts for a C3PO interrupt are: class, destination node,
> > destination thread on the node and priority. The SLOF firmware just
> > change the prirority for the 4 possible interrupts generated by the C3PO
> > (MPIC machine check = 0, MPIC critical = 4, direct MBX = 8, MPIC normal =
> > c).
> >
> > According to the Cell manual, class = 2 is correct for external
> > interrupts. Class 0 and 1 are reserved for internal SPU/DMA
> > error/translation cases. 3 is reserved.
>
> What about the source unit number? Since the mailbox is obviously a
> different entity from the mpic, you should be able to program it to a
> different source node/unit.

The Axon doesn't set it. From the Cell perspective the interrupt comes from 
one particular IOIF, that's it. The Cell hardware/code knows the unit that 
generated the interrupt. Either IOIF0 (unit = 0) or IOIF1 (unit = 0xb). So 
what I am programming on the C3PO doesn't show on the Cell. This is just so 
that the interrupt packet is routed to the correct location. All interrupt 
packets from the C3PO (MPIC or MBX) will come from the same "unit". You can't 
choose this. What could differ are class and priority. You could also decide 
to target the other PPU thread (all MPIC interrupts to one thread, MBX 
interrupt to the other thread).

> I wouldn't really mind using a different class here if that's the only
> way we can really tell, even if that is not in the spirit of the CBE
> architecture.

Well that might be a solution but I didn't want to go this way because it was 
diverging from the CBE architecture document. Now if we go this route, we 
will have to make sure there is no collision with some already valid 
interrupts. I guess the unit should set the interrupt apart already.

We could also factorise somehow the interrupt priority in the hw interrupt 
number. But I guess that would make a lot of changes.

> > > Please open a bug report against the device tree when there is
> > > something missing. It's much easier to fix it in the right place than
> > > to keep ugly hacks around.
> > >
> > > Since CAB is now an official product, I wonder how such an obvious bug
> > > could slip through QA testing...
> >
> > Well I asked the question at the time the device tree was submited to us
> > but I was told there was no consensus in the firmware team on how to
> > handle the MBX direct interrupt because it was not a usual MPIC and
> > therefore it was intentionaly left out.
>
> If you have the name of the person that gave you such an answer, please
> tell me in a private mail!
>
> If it's not in the device tree, we don't use it. The people that are
> responsible for the device tree should know that.
>
> > > This is the wrong place for it, the IIC code should not need to know
> > > about specific external interrupt controllers.
> >
> > Well sorry about it but this is the only place where I can check
> > priority.
>
> That can be changed if necessary. If we have another cascaded interrupt
> controller (mpic behind c3p0), the c3p0 code should check the actual
> source. If we can differentiate by class, then we don't even need that, as
> the number will be different to start with.
>
> > > This is wrong, the hw_int and iic_int numbers should come from the
> > > "interrupts" and "interrupt-parent" property of the device node you are
> > > looking at.
> >
> > Well there is none for the direct MBX interrupt, so I had to make one. I
> > am hardcoding it to 128 (MPIC external interrupts) + 4 (MPIC IPIs) + 4
> > (MPIC timers) = 136. Then I have to distinguish between IOIF0 (CAB case)
> > and IOIF1 (Malta case).
>
> But the number of the mailbox interrupt is in a different domain from the
> mpic interrupt, as the interrupts don't go to the mpic at all.

True and the interrupt doesn't go through the MPIC at all. And just to make 
things clear the MBX direct interrupt is not handled by the MPIC code at 
all ...

> You can't 
> just make up numbers like that, we've been through that pain before on
> the QS20. The interrupt number needs to be allocated by the
> irq_of_parse_and_map() function now.

Well obviously, I can't use this function for the direct MBX and my driver 
code has to account for this when dealing with MBX direct interrupt. All 
other interrupts are allocated with the the normal irq_of_parse_and_map() 
method when run on the CAB or Malta blade.

I agree that I should avoid to do this but I nedded something that work as the 
MPIC indirect MBX interrupt is broken on Axon 1.1. Only direct interrupts are 
reliable and it is not part of the device tree.

>
> 	Arnd <><