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

[Arnd Bergmann]

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?

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...

> 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.

> +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.

> +		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.

	Arnd <><