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

[Jean-Christophe Dubois]

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.

> On Friday 18 May 2007, Jean-Christophe Dubois wrote:
> > This patch allows Linux to support the Mailbox direct interrupt that is
> > left out in the actual SLOF device tree.
> >
> > This patch has been tested on the CAB and the Malta blade with 2.6.20 and
> > 2.6.21 kernels.
>
> 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.

> > Index: linux-2.6.20/arch/powerpc/platforms/cell/interrupt.c
> > ===================================================================
> > --- linux-2.6.20.orig/arch/powerpc/platforms/cell/interrupt.c	2007-02-04
> > 19:44:54.000000000 +0100
> > +++ linux-2.6.20/arch/powerpc/platforms/cell/interrupt.c	2007-03-28
> > 16:35:17.000000000 +0200
> > @@ -64,6 +64,22 @@
> >  	unsigned char node = bits.source >> 4;
> >  	unsigned char class = bits.class & 3;
> >
> > +	/*
> > +	 * This is a special case for the Axon direct interrupt.
> > +	 * The direct interrupt is characterized by a priority set to 8
> > +	 * But the priority is not used to generate the hwnum
> > +	 * So here is a little hack.
> > +	 */
> > +	if ((unit == 0x0 || unit == 0xb) && (class == 2)
> > +	    && ((bits.prio>>4)  == 8)) {
> > +		/*
> > +		 * in order to discriminate the direct MBX we change the
> > +		 * unit for an unused value:
> > +		 * 0xc for IOIF0 and 0xd for IOIF1
> > +		 */
> > +		unit = unit?0xd:0xc;
> > +	}
> > +
> >  	/* Decode IPIs */
> >  	if (bits.flags & CBE_IIC_IRQ_IPI)
> >  		return IIC_IRQ_TYPE_IPI | (bits.prio >> 4);
>
> 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.

> > +static void c3po_mask(unsigned int irq)
> > +{
> > +}
> > +
> > +static void c3po_unmask(unsigned int irq)
> > +{
> > +}
> > +
> > +static void c3po_eoi(unsigned int irq)
> > +{
> > +}
> > +
> > +static void c3po_ack(unsigned int irq)
> > +{
> > +}
> > +
> > +static struct irq_chip c3po_chip = {
> > +	.typename = " AXON-C3PO",
> > +	.mask = c3po_mask,
> > +	.unmask = c3po_unmask,
> > +	.ack = c3po_ack,
> > +	.eoi = c3po_eoi,
> > +};
> > +
> > +static void c3po_irq_cascade(unsigned int irq, struct irq_desc *desc)
> > +{
> > +	int hw_int = (int)desc->handler_data;
> > +	unsigned int virq = irq_find_mapping(NULL, hw_int);
> > +
> > +	if (virq != NO_IRQ) {
> > +		generic_handle_irq(virq);
> > +	}
> > +
> > +	desc->chip->eoi(irq);
> > +}
> > +
> >  static void __init mpic_init_IRQ(void)
> >  {
> >  	struct device_node *dn;
> > @@ -134,12 +170,66 @@
> >  	}
> >  }
> >
> > +static void __init c3po_init_IRQ(void)
> > +{
> > +	struct device_node *dn;
> > +	// We need a way to find out the cell/node to which the c3po is
> > qttqched +	int node = 0;
> > +
> > +	for (dn = NULL;
> > +	     (dn = of_find_node_by_name(dn, "c3po"));node++) {
> > +		unsigned int virq;
> > +		int hw_int;
> > +		int iic_int;
> > +		u32 *prop;
> > +		int len;
>
> You should check the "compatible" property to make sure you are on one of
> the known machines. Just looking for a device named "c3po" is a bit
> too vague.

OK even if today c3po is quite unique to the Axon. I'll add it.

> > +		if (pdn) {
> > +			prop = (u32*) get_property(pdn, "ioif",&len);
> > +			if ( prop && len == 4 )
> > +				ioif = *prop;
> > +
> > +			of_node_put(pdn);
> > +		}
> > +
> > +		hw_int = (node * 256) + 136 + ioif;
> > +		iic_int = (node << IIC_IRQ_NODE_SHIFT) |
> > +			      (2 << IIC_IRQ_CLASS_SHIFT) |
> > +			      (0xc + ioif);
> > +
> > +		printk(KERN_INFO "c3po: ioif=%d, hw_int=%d, iic_int=%d \n",
> > +				ioif, hw_int, iic_int);
>
> 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).

JC

> 	Arnd <><