Subject: Re: [PATCH 1/2] Add MPC52xx Interrupt controller support for ARCH=powerpc

From: Benjamin Herrenschmidt <[email-masked]>
Date: Tue, 31 Oct 2006 09:31:27 +1100

On Mon, 2006-10-30 at 16:25 -0600, Kumar Gala wrote:
> On Oct 29, 2006, at 5:10 PM, Nicolas DET wrote:
> 
> > This patch add MPC52xx Interrupt controller for ARCH=powerpc.
> >
> > It includes the main code in arch/powerpc/sysdev/ ad well as an  
> > header file in
> > include/asm-powerpc.
> >
> > Signed-off-by: Nicolas DET <[email-masked]>
> 
> Can you see if you can figure out how to inline patches with your  
> mailer, its really difficult to comment on issues w/an attachment.
> 
> +/* MBAR position */
> +#define MPC52xx_MBAR		0xf0000000	/* Phys address */
> +#define MPC52xx_MBAR_VIRT	0xf0000000	/* Virt address */
> +#define MPC52xx_MBAR_SIZE	0x00010000
> +
> +#define MPC52xx_PA(x)		((phys_addr_t)(MPC52xx_MBAR + (x)))
> +#define MPC52xx_VA(x)		((void __iomem *)(MPC52xx_MBAR_VIRT + (x)))
> 
> This should be handled dynamically (pulled from the device tree), I  
> doubt MBAR will be at the same location for all boards.

The physical address should indeed come from the DT. The virtual address
should be assigned dynamically by ioremap anyway. What are thse _PA/_VA
used for ?

> * can you split out the interrupt controller header info into a  
> mpc52xx_pic.h [mpc52xx_intr, MPC52xx_IRQ_...]

Good idea.

Ben