Re: [PATCH] General CHRP/MPC5K2 platform support patch

[Sylvain Munaut <tnt@246tNt.com>]

Hi Nicolas,

Here is a few comments. I'm not very familiar with the new irq stuff so
others
might have more insights.


Sylvain

> --- a/arch/powerpc/sysdev/mpc52xx_pic.c	1970-01-01 01:00:00.000000000 +0100
> +++ b/arch/powerpc/sysdev/mpc52xx_pic.c	2006-10-27 15:58:29.000000000 +0200
> @@ -0,0 +1,414 @@
> +/*
> + * arch/powerpc/sysdev/mpc52xx_pic.c
>   
Looks like jdl is right, we apparently don't do that any more ... So
let's not ;)
> + * Based on (well, mostly copied from) the code from the 2.4 kernel by
> + * Dale Farnsworth <dfarnsworth@mvista.com> and Kent Borg.
>   
That can be removed ... We can't blame Dale anymore if it doesn't work ;)

> +
> +static void mpc52xx_ic_mask_and_ack(unsigned int irq)
> +{
> +	mpc52xx_ic_mask(irq);
> +	mpc52xx_ic_ack(irq);
> +}
>   
>From kernel/irq/chip.c that's done automatically if mask_and_ack is NULL.

> +
> +static struct irq_chip mpc52xx_irqchip = {
> +	.typename = " MPC52xx  ",
> +	.mask = mpc52xx_ic_mask,
> +	.unmask = mpc52xx_ic_unmask,
> +	.mask_ack = mpc52xx_ic_mask_and_ack,
> +};
Is it useful to implement set_type for IRQ[0-3] ? (Just asking ...)

> +	for (i = 0; i < NR_IRQS; i++) {
> +		irq_desc[i].chip = &mpc52xx_irqchip;
> +		irq_desc[i].status = IRQ_LEVEL;
> +
> +	}
>   
All LEVEL ?

> +
> +	/*
> +	 * As last step, add an irq host to translate the real
> +	 * hw irq information provided by the ofw to linux virq
> +	 */
> +
> +	mpc52xx_irqhost =
> +	    irq_alloc_host(IRQ_HOST_MAP_LINEAR, NR_IRQS, &mpc52xx_irqhost_ops,
> +			   -1);
> +}
>   
NR_IRQS ? Might be time to do something better.



> diff -uprN a/include/asm-powerpc/mpc52xx.h b/include/asm-powerpc/mpc52xx.h
> --- a/include/asm-powerpc/mpc52xx.h	1970-01-01 01:00:00.000000000 +0100
> +++ b/include/asm-powerpc/mpc52xx.h	2006-10-27 15:51:55.000000000 +0200
> @@ -0,0 +1,414 @@
> +/*
> + * include/asm-ppc/mpc52xx.h
> + * 
> + * Prototypes, etc. for the Freescale MPC52xx embedded cpu chips
> + * May need to be cleaned as the port goes on ...
> + *
> + *
> + * Maintainer : Sylvain Munaut <tnt@246tNt.com>
> + *
> + * Originally written by Dale Farnsworth <dfarnsworth@mvista.com> 
> + * for the 2.4 kernel.
>   
Again, remove all that (just leave the first line of the description)

> +
> +/* ======================================================================== */
> +/* IRQ mapping                                                              */
> +/* ======================================================================== */
> +
> +#define MPC52xx_IRQ_L1_CRIT	0
> +#define MPC52xx_IRQ_L1_MAIN	1
> +#define MPC52xx_IRQ_L1_PERP	2
> +#define MPC52xx_IRQ_L1_SDMA	3
> +
> +#define MPC52xx_IRQ_L1_OFFSET  (6)
> +#define MPC52xx_IRQ_L1_MASK    (0xc0)
> +
> +#define MPC52xx_IRQ_L2_OFFSET  (0)
> +#define MPC52xx_IRQ_L2_MASK    (0x3f)
>   
As benh suggested on IRC, a L1 offset of 8 might be better for
readability of the
hw irq numbers.