[U-Boot-Users] ppc4xx: [PATCH] CPU PPC440x5 on Virtex5 FX (new version)

Stefan Roese
2008-07-17T06:24:56Z

On Wednesday 16 July 2008, Ricardo Ribalda Delgado wrote:
> Previous patch did not compile on some boards
>
>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@uam.es>
> ---
> -This patchs gives support for the embbedded ppc440
> on the Virtex5 FPGAs
>
> -interrupts.c divided in uic.c and interrupts.c
>
> -xilinx_irq.c for xilinx interrupt controller

Jon already commented about the swapped commit text/comments issue. Apart from
that please find some more comments below:

> cpu/ppc4xx/Makefile | 13 +++
> cpu/ppc4xx/cpu.c | 8 ++
> cpu/ppc4xx/interrupts.c | 178 ++++++------------------------------
> cpu/ppc4xx/speed.c | 6 +-
> cpu/ppc4xx/uic.c | 210
> +++++++++++++++++++++++++++++++++++++++++++ cpu/ppc4xx/xilinx_irq.c |
> 111 +++++++++++++++++++++++
> include/asm-ppc/interrupt.h | 37 ++++++++
> include/asm-ppc/processor.h | 2 +
> 8 files changed, 416 insertions(+), 149 deletions(-)
> create mode 100644 cpu/ppc4xx/uic.c
> create mode 100644 cpu/ppc4xx/xilinx_irq.c
> create mode 100644 include/asm-ppc/interrupt.h
>
> diff --git a/cpu/ppc4xx/Makefile b/cpu/ppc4xx/Makefile
> index 800bb41..9d918c9 100644
> --- a/cpu/ppc4xx/Makefile
> +++ b/cpu/ppc4xx/Makefile
> @@ -35,10 +35,14 @@ SOBJS += kgdb.o
> COBJS := 40x_spd_sdram.o
> COBJS += 44x_spd_ddr.o
> COBJS += 44x_spd_ddr2.o
> +ifndef CONFIG_XILINX_440
> COBJS += 4xx_enet.o
> +endif
> COBJS += 4xx_pci.o
> COBJS += 4xx_pcie.o
> +ifndef CONFIG_XILINX_440
> COBJS += 4xx_uart.o
> +endif
> COBJS += bedbug_405.o
> COBJS += commproc.o
> COBJS += cpu.o
> @@ -47,11 +51,20 @@ COBJS += denali_data_eye.o
> COBJS += denali_spd_ddr2.o
> COBJS += ecc.o
> COBJS += fdt.o
> +ifndef CONFIG_XILINX_440
> COBJS += gpio.o
> +endif
> COBJS += i2c.o
> COBJS += interrupts.o
> +ifndef CONFIG_XILINX_440
> +COBJS += uic.o
> +else
> +COBJS += xilinx_irq.o
> +endif
> COBJS += iop480_uart.o
> +ifndef CONFIG_XILINX_440
> COBJS += miiphy.o
> +endif

Even though we usually sort the files alphabetically, these
multiple "ifndef's" look very ugly. Perhaps we should change this here to a
single "ifndef" with all the non-Xilinx files in it.

> COBJS += ndfc.o
> COBJS += sdram.o
> COBJS += speed.o
> diff --git a/cpu/ppc4xx/cpu.c b/cpu/ppc4xx/cpu.c
> index ef32bc6..775ed01 100644
> --- a/cpu/ppc4xx/cpu.c
> +++ b/cpu/ppc4xx/cpu.c
> @@ -279,7 +279,11 @@ int checkcpu (void)
>
> get_sys_info(&sys_info);
>
> +#if defined(CONFIG_XILINX_440)
> + puts("IBM PowerPC 4");
> +#else
> puts("AMCC PowerPC 4");
> +#endif
>
> #if defined(CONFIG_405GP) || defined(CONFIG_405CR) || \
> defined(CONFIG_405EP) || defined(CONFIG_405EZ) || \
> @@ -541,6 +545,10 @@ int checkcpu (void)
> puts("GX Rev. A");
> strcpy(addstr, "No Security support");
> break;
> +
> + case PVR_VIRTEX5:
> + puts(" VIRTEX5");
> + break;

Is the CPU really a VIRTEX5 or a PPC440 variant? I think the latter. Perhaps
something like "PPC440x5"?

> default:
> printf (" UNKNOWN (PVR=%08x)", pvr);
> diff --git a/cpu/ppc4xx/interrupts.c b/cpu/ppc4xx/interrupts.c
> index 8215dc6..58d1d81 100644
> --- a/cpu/ppc4xx/interrupts.c
> +++ b/cpu/ppc4xx/interrupts.c
> @@ -8,6 +8,10 @@
> * (C) Copyright 2003 (440GX port)
> * Travis B. Sawyer, Sandburst Corporation, tsawyer at sandburst.com
> *
> + * (C) Copyright 2008 (PPC440X05 port for Virtex 5 FX)
> + * Ricardo Ribalda-Universidad Autonoma de Madrid-ricardo.ribalda at uam.es
> + * Work supported by Qtechnology (htpp://qtec.com)
> + *
> * See file CREDITS for list of people who contributed to this
> * project.
> *
> @@ -31,23 +35,11 @@
> #include <watchdog.h>
> #include <command.h>
> #include <asm/processor.h>
> +#include <asm/interrupt.h>
> #include <ppc4xx.h>
> #include <ppc_asm.tmpl>
> #include <commproc.h>
>
> -#if (UIC_MAX > 3)
> -#define UICB0_ALL (UIC_MASK(VECNUM_UIC1CI) | UIC_MASK(VECNUM_UIC1NCI) | \
> - UIC_MASK(VECNUM_UIC2CI) | UIC_MASK(VECNUM_UIC2NCI) | \
> - UIC_MASK(VECNUM_UIC3CI) | UIC_MASK(VECNUM_UIC3NCI))
> -#elif (UIC_MAX > 2)
> -#define UICB0_ALL (UIC_MASK(VECNUM_UIC1CI) | UIC_MASK(VECNUM_UIC1NCI) | \
> - UIC_MASK(VECNUM_UIC2CI) | UIC_MASK(VECNUM_UIC2NCI))
> -#elif (UIC_MAX > 1)
> -#define UICB0_ALL (UIC_MASK(VECNUM_UIC1CI) | UIC_MASK(VECNUM_UIC1NCI))
> -#else
> -#define UICB0_ALL 0
> -#endif
> -
> DECLARE_GLOBAL_DATA_PTR;
>
> /*
> @@ -58,11 +50,7 @@ struct irq_action {
> void *arg;
> int count;
> };
> -
> -static struct irq_action irq_vecs[UIC_MAX * 32];
> -
> -u32 get_dcr(u16);
> -void set_dcr(u16, u32);
> +static struct irq_action irq_vecs[IRQ_MAX];
>
> #if defined(CONFIG_440)
>
> @@ -103,7 +91,7 @@ int interrupt_init_cpu (unsigned *decrementer_count)
> /*
> * Mark all irqs as free
> */
> - for (vec = 0; vec < (UIC_MAX * 32); vec++) {
> + for (vec = 0; vec < IRQ_MAX; vec++) {
> irq_vecs[vec].handler = NULL;
> irq_vecs[vec].arg = NULL;
> irq_vecs[vec].count = 0;
> @@ -147,110 +135,36 @@ int interrupt_init_cpu (unsigned *decrementer_count)
> */
> set_evpr(0x00000000);
>
> -#if (UIC_MAX > 1)
> - /* Install the UIC1 handlers */
> - irq_install_handler(VECNUM_UIC1NCI, (void *)(void *)external_interrupt,
> 0); - irq_install_handler(VECNUM_UIC1CI, (void *)(void
> *)external_interrupt, 0); -#endif
> -#if (UIC_MAX > 2)
> - irq_install_handler(VECNUM_UIC2NCI, (void *)(void *)external_interrupt,
> 0); - irq_install_handler(VECNUM_UIC2CI, (void *)(void
> *)external_interrupt, 0); -#endif
> -#if (UIC_MAX > 3)
> - irq_install_handler(VECNUM_UIC3NCI, (void *)(void *)external_interrupt,
> 0); - irq_install_handler(VECNUM_UIC3CI, (void *)(void
> *)external_interrupt, 0); -#endif
> + /*
> + *Call uic or xilinx_irq pic_enable
> + */
> + pic_enable();
>
> return (0);
> }
>
> -/* Handler for UIC interrupt */
> -static void uic_interrupt(u32 uic_base, int vec_base)
> +void timer_interrupt_cpu(struct pt_regs *regs)
> {
> - u32 uic_msr;
> - u32 msr_shift;
> - int vec;
> -
> - /*
> - * Read masked interrupt status register to determine interrupt source
> - */
> - uic_msr = get_dcr(uic_base + UIC_MSR);
> - msr_shift = uic_msr;
> - vec = vec_base;
> -
> - while (msr_shift != 0) {
> - if (msr_shift & 0x80000000) {
> - /*
> - * Increment irq counter (for debug purpose only)
> - */
> - irq_vecs[vec].count++;
> -
> - if (irq_vecs[vec].handler != NULL) {
> - /* call isr */
> - (*irq_vecs[vec].handler)(irq_vecs[vec].arg);
> - } else {
> - set_dcr(uic_base + UIC_ER,
> - get_dcr(uic_base + UIC_ER) & ~UIC_MASK(vec));
> - printf("Masking bogus interrupt vector %d"
> - " (UIC_BASE=0x%x)\n", vec, uic_base);
> - }
> -
> - /*
> - * After servicing the interrupt, we have to remove the
> - * status indicator
> - */
> - set_dcr(uic_base + UIC_SR, UIC_MASK(vec));
> - }
> -
> - /*
> - * Shift msr to next position and increment vector
> - */
> - msr_shift <<= 1;
> - vec++;
> - }
> + /* nothing to do here */
> + return;
> }
>
> -/*
> - * Handle external interrupts
> - */
> -void external_interrupt(struct pt_regs *regs)
> +void interrupt_run_handler(int vec)
> {
> - u32 uic_msr;
> -
> - /*
> - * Read masked interrupt status register to determine interrupt source
> - */
> - uic_msr = mfdcr(uic0msr);
> -
> -#if (UIC_MAX > 1)
> - if ((UIC_MASK(VECNUM_UIC1CI) & uic_msr) ||
> - (UIC_MASK(VECNUM_UIC1NCI) & uic_msr))
> - uic_interrupt(UIC1_DCR_BASE, 32);
> -#endif
> -
> -#if (UIC_MAX > 2)
> - if ((UIC_MASK(VECNUM_UIC2CI) & uic_msr) ||
> - (UIC_MASK(VECNUM_UIC2NCI) & uic_msr))
> - uic_interrupt(UIC2_DCR_BASE, 64);
> -#endif
> -
> -#if (UIC_MAX > 3)
> - if ((UIC_MASK(VECNUM_UIC3CI) & uic_msr) ||
> - (UIC_MASK(VECNUM_UIC3NCI) & uic_msr))
> - uic_interrupt(UIC3_DCR_BASE, 96);
> -#endif
> -
> - if (uic_msr & ~(UICB0_ALL))
> - uic_interrupt(UIC0_DCR_BASE, 0);
> -
> - mtdcr(uic0sr, uic_msr);
> + irq_vecs[vec].count++;
> +
> + if (irq_vecs[vec].handler != NULL) {
> + /* call isr */
> + (*irq_vecs[vec].handler) (irq_vecs[vec].arg);
> + } else {
> + pic_irq_disable(vec);
> + printf("Masking bogus interrupt vector %d\n", vec);
> + }
>
> + pic_irq_ack(vec);
> return;
> }
>
> -/*
> - * Install and free a interrupt handler.
> - */
> void irq_install_handler(int vec, interrupt_handler_t * handler, void
> *arg) {
> /*
> @@ -263,51 +177,19 @@ void irq_install_handler(int vec, interrupt_handler_t
> * handler, void *arg) irq_vecs[vec].handler = handler;
> irq_vecs[vec].arg = arg;
>
> - if ((vec >= 0) && (vec < 32))
> - mtdcr(uicer, mfdcr(uicer) | UIC_MASK(vec));
> -#if (UIC_MAX > 1)
> - else if ((vec >= 32) && (vec < 64))
> - mtdcr(uic1er, mfdcr(uic1er) | UIC_MASK(vec));
> -#endif
> -#if (UIC_MAX > 2)
> - else if ((vec >= 64) && (vec < 96))
> - mtdcr(uic2er, mfdcr(uic2er) | UIC_MASK(vec));
> -#endif
> -#if (UIC_MAX > 3)
> - else if (vec >= 96)
> - mtdcr(uic3er, mfdcr(uic3er) | UIC_MASK(vec));
> -#endif
> -
> - debug("Install interrupt for vector %d ==> %p\n", vec, handler);
> + pic_irq_enable(vec);
> + return;
> }
>
> -void irq_free_handler (int vec)
> +void irq_free_handler(int vec)
> {
> debug("Free interrupt for vector %d ==> %p\n",
> vec, irq_vecs[vec].handler);
>
> - if ((vec >= 0) && (vec < 32))
> - mtdcr(uicer, mfdcr(uicer) & ~UIC_MASK(vec));
> -#if (UIC_MAX > 1)
> - else if ((vec >= 32) && (vec < 64))
> - mtdcr(uic1er, mfdcr(uic1er) & ~UIC_MASK(vec));
> -#endif
> -#if (UIC_MAX > 2)
> - else if ((vec >= 64) && (vec < 96))
> - mtdcr(uic2er, mfdcr(uic2er) & ~UIC_MASK(vec));
> -#endif
> -#if (UIC_MAX > 3)
> - else if (vec >= 96)
> - mtdcr(uic3er, mfdcr(uic3er) & ~UIC_MASK(vec));
> -#endif
> + pic_irq_disable(vec);
>
> irq_vecs[vec].handler = NULL;
> irq_vecs[vec].arg = NULL;
> -}
> -
> -void timer_interrupt_cpu (struct pt_regs *regs)
> -{
> - /* nothing to do here */
> return;
> }
>
> @@ -319,7 +201,7 @@ int do_irqinfo(cmd_tbl_t *cmdtp, int flag, int argc,
> char *argv[]) printf ("Interrupt-Information:\n");
> printf ("Nr Routine Arg Count\n");
>
> - for (vec = 0; vec < (UIC_MAX * 32); vec++) {
> + for (vec = 0; vec < IRQ_MAX; vec++) {
> if (irq_vecs[vec].handler != NULL) {
> printf ("%02d %08lx %08lx %d\n",
> vec,
> diff --git a/cpu/ppc4xx/speed.c b/cpu/ppc4xx/speed.c
> index b86b6de..d21bd82 100644
> --- a/cpu/ppc4xx/speed.c
> +++ b/cpu/ppc4xx/speed.c
> @@ -416,7 +416,8 @@ ulong get_PCI_freq (void)
> return sys_info.freqPCI;
> }
>
> -#elif !defined(CONFIG_440GX) && !defined(CONFIG_440SP) &&
> !defined(CONFIG_440SPE) +#elif !defined(CONFIG_440GX) &&
> !defined(CONFIG_440SP) && !defined(CONFIG_440SPE) \ + &&
> !defined(CONFIG_XILINX_440)
> void get_sys_info (sys_info_t * sysInfo)
> {
> unsigned long strp0;
> @@ -449,6 +450,8 @@ void get_sys_info (sys_info_t * sysInfo)
> sysInfo->freqUART = sysInfo->freqPLB;
> }
> #else
> +
> +#if !defined(CONFIG_XILINX_440)
> void get_sys_info (sys_info_t * sysInfo)
> {
> unsigned long strp0;
> @@ -535,6 +538,7 @@ void get_sys_info (sys_info_t * sysInfo)
> }
>
> #endif
> +#endif /* CONFIG_XILINX_440 */
>
> #if defined(CONFIG_YUCCA)
> unsigned long determine_sysper(void)
> diff --git a/cpu/ppc4xx/uic.c b/cpu/ppc4xx/uic.c
> new file mode 100644
> index 0000000..102b67a
> --- /dev/null
> +++ b/cpu/ppc4xx/uic.c
> @@ -0,0 +1,210 @@
> +/*
> + * (C) Copyright 2000-2002
> + * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
> + *
> + * (C) Copyright 2002 (440 port)
> + * Scott McNutt, Artesyn Communication Producs, smcnutt at artsyncp.com
> + *
> + * (C) Copyright 2003 (440GX port)
> + * Travis B. Sawyer, Sandburst Corporation, tsawyer at sandburst.com
> + *
> + * (C) Copyright 2008 (PPC440X05 port for Virtex 5 FX)
> + * Ricardo Ribalda-Universidad Autonoma de Madrid-ricardo.ribalda at uam.es
> + * Work supported by Qtechnology (htpp://qtec.com)
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#include <common.h>
> +#include <watchdog.h>
> +#include <command.h>
> +#include <asm/processor.h>
> +#include <asm/interrupt.h>
> +#include <ppc4xx.h>
> +#include <ppc_asm.tmpl>
> +#include <commproc.h>
> +
> +#if (UIC_MAX > 3)
> +#define UICB0_ALL (UIC_MASK(VECNUM_UIC1CI) | UIC_MASK(VECNUM_UIC1NCI) | \
> + UIC_MASK(VECNUM_UIC2CI) | UIC_MASK(VECNUM_UIC2NCI) | \
> + UIC_MASK(VECNUM_UIC3CI) | UIC_MASK(VECNUM_UIC3NCI))
> +#elif (UIC_MAX > 2)
> +#define UICB0_ALL (UIC_MASK(VECNUM_UIC1CI) | UIC_MASK(VECNUM_UIC1NCI) | \
> + UIC_MASK(VECNUM_UIC2CI) | UIC_MASK(VECNUM_UIC2NCI))
> +#elif (UIC_MAX > 1)
> +#define UICB0_ALL (UIC_MASK(VECNUM_UIC1CI) | UIC_MASK(VECNUM_UIC1NCI))
> +#else
> +#define UICB0_ALL 0
> +#endif
> +
> +u32 get_dcr(u16);
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +void pic_enable(void)
> +{
> +
> +#if (UIC_MAX > 1)
> + /* Install the UIC1 handlers */
> + irq_install_handler(VECNUM_UIC1NCI, (void *)(void *)external_interrupt,
> + 0);
> + irq_install_handler(VECNUM_UIC1CI, (void *)(void *)external_interrupt,
> + 0);
> +#endif
> +#if (UIC_MAX > 2)
> + irq_install_handler(VECNUM_UIC2NCI, (void *)(void *)external_interrupt,
> + 0);
> + irq_install_handler(VECNUM_UIC2CI, (void *)(void *)external_interrupt,
> + 0);
> +#endif
> +#if (UIC_MAX > 3)
> + irq_install_handler(VECNUM_UIC3NCI, (void *)(void *)external_interrupt,
> + 0);
> + irq_install_handler(VECNUM_UIC3CI, (void *)(void *)external_interrupt,
> + 0);
> +#endif
> +
> +}
> +
> +

Nitpick: Only one empty line please.

> +/* Handler for UIC interrupt */
> +static void uic_interrupt(u32 uic_base, int vec_base)
> +{
> + u32 uic_msr;
> + u32 msr_shift;
> + int vec;
> +
> + /*
> + * Read masked interrupt status register to determine interrupt source
> + */
> + uic_msr = get_dcr(uic_base + UIC_MSR);
> + msr_shift = uic_msr;
> + vec = vec_base;
> +
> + while (msr_shift != 0) {
> + if (msr_shift & 0x80000000)
> + interrupt_run_handler(vec);
> + /*
> + * Shift msr to next position and increment vector
> + */
> + msr_shift <<= 1;
> + vec++;
> + }
> +}
> +
> +/*
> + * Handle external interrupts
> + */
> +void external_interrupt(struct pt_regs *regs)
> +{
> + u32 uic_msr;
> +
> + /*
> + * Read masked interrupt status register to determine interrupt source
> + */
> + uic_msr = mfdcr(uic0msr);
> +
> +#if (UIC_MAX > 1)
> + if ((UIC_MASK(VECNUM_UIC1CI) & uic_msr) ||
> + (UIC_MASK(VECNUM_UIC1NCI) & uic_msr))
> + uic_interrupt(UIC1_DCR_BASE, 32);
> +#endif
> +
> +#if (UIC_MAX > 2)
> + if ((UIC_MASK(VECNUM_UIC2CI) & uic_msr) ||
> + (UIC_MASK(VECNUM_UIC2NCI) & uic_msr))
> + uic_interrupt(UIC2_DCR_BASE, 64);
> +#endif
> +
> +#if (UIC_MAX > 3)
> + if ((UIC_MASK(VECNUM_UIC3CI) & uic_msr) ||
> + (UIC_MASK(VECNUM_UIC3NCI) & uic_msr))
> + uic_interrupt(UIC3_DCR_BASE, 96);
> +#endif
> +
> + if (uic_msr & ~(UICB0_ALL))
> + uic_interrupt(UIC0_DCR_BASE, 0);
> +
> + mtdcr(uic0sr, uic_msr);
> +
> + return;
> +}
> +
> +void pic_irq_ack(unsigned int vec)
> +{
> +
> + if ((vec >= 0) && (vec < 32))
> + mtdcr(uicsr, UIC_MASK(vec));
> +#if (UIC_MAX > 1)
> + else if ((vec >= 32) && (vec < 64))
> + mtdcr(uic1sr, UIC_MASK(vec));
> +#endif
> +#if (UIC_MAX > 2)
> + else if ((vec >= 64) && (vec < 96))
> + mtdcr(uic2sr, UIC_MASK(vec));
> +#endif
> +#if (UIC_MAX > 3)
> + else if (vec >= 96)
> + mtdcr(uic3sr, UIC_MASK(vec));
> +#endif
> +}
> +
> +/*
> + * Install and free a interrupt handler.
> + */
> +void pic_irq_enable(unsigned int vec)
> +{
> +
> + if ((vec >= 0) && (vec < 32))
> + mtdcr(uicer, mfdcr(uicer) | UIC_MASK(vec));
> +#if (UIC_MAX > 1)
> + else if ((vec >= 32) && (vec < 64))
> + mtdcr(uic1er, mfdcr(uic1er) | UIC_MASK(vec));
> +#endif
> +#if (UIC_MAX > 2)
> + else if ((vec >= 64) && (vec < 96))
> + mtdcr(uic2er, mfdcr(uic2er) | UIC_MASK(vec));
> +#endif
> +#if (UIC_MAX > 3)
> + else if (vec >= 96)
> + mtdcr(uic3er, mfdcr(uic3er) | UIC_MASK(vec));
> +#endif
> +
> + debug("Install interrupt for vector %d ==> %p\n", vec, handler);
> +}
> +
> +void pic_irq_disable(unsigned int vec)
> +{
> +
> + if ((vec >= 0) && (vec < 32))
> + mtdcr(uicer, mfdcr(uicer) & ~UIC_MASK(vec));
> +#if (UIC_MAX > 1)
> + else if ((vec >= 32) && (vec < 64))
> + mtdcr(uic1er, mfdcr(uic1er) & ~UIC_MASK(vec));
> +#endif
> +#if (UIC_MAX > 2)
> + else if ((vec >= 64) && (vec < 96))
> + mtdcr(uic2er, mfdcr(uic2er) & ~UIC_MASK(vec));
> +#endif
> +#if (UIC_MAX > 3)
> + else if (vec >= 96)
> + mtdcr(uic3er, mfdcr(uic3er) & ~UIC_MASK(vec));
> +#endif
> +
> +}
> diff --git a/cpu/ppc4xx/xilinx_irq.c b/cpu/ppc4xx/xilinx_irq.c
> new file mode 100644
> index 0000000..f2dbcc8
> --- /dev/null
> +++ b/cpu/ppc4xx/xilinx_irq.c
> @@ -0,0 +1,111 @@
> +/*
> + * (C) Copyright 2008
> + * Ricado Ribalda-Universidad Autonoma de Madrid-ricardo.ribalda at uam.es
> + * This work has been supported by: QTechnology http://qtec.com/
> + * Based on interrupts.c Wolfgang Denk-DENX Software
> Engineering-wd at denx.de + * This program is free software: you can
> redistribute it and/or modify + * it under the terms of the GNU General
> Public License as published by + * the Free Software Foundation, either
> version 2 of the License, or + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> +*/
> +#include <common.h>
> +#include <watchdog.h>
> +#include <command.h>
> +#include <asm/processor.h>
> +#include <asm/interrupt.h>
> +#include <ppc4xx.h>
> +#include <ppc_asm.tmpl>
> +#include <commproc.h>
> +#include <asm/io.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;

Do you really need this in this file?

> +
> +#define intc XPAR_INTC_0_BASEADDR
> +#define ISR (u32*)(intc+(0*4)) /* Interrupt Status Register */
> +#define IPR (u32*)(intc+(1*4)) /* Interrupt Pending Register */
> +#define IER (u32*)(intc+(2*4)) /* Interrupt Enable Register */
> +#define IAR (u32*)(intc+(3*4)) /* Interrupt Acknowledge Register */
> +#define SIE (u32*)(intc+(4*4)) /* Set Interrupt Enable bits */
> +#define CIE (u32*)(intc+(5*4)) /* Clear Interrupt Enable bits */
> +#define IVR (u32*)(intc+(6*4)) /* Interrupt Vector Register */
> +#define MER (u32*)(intc+(7*4)) /* Master Enable Register */

Don't put the casts into the defines. They belong in the code.

> +#define IRQ_MASK(irq) (1<<(irq&0x1f))

Conding-style: Missing spaces.

> +void pic_enable(void)
> +{
> + printf("Xilinx PIC at 0x%8x\n", intc);

You probably what a debug() here.

> +
> + /*
> + * Disable all external interrupts until they are
> + * explicitly requested.
> + */
> + out_be32(IER, 0);
> +
> + /* Acknowledge any pending interrupts just in case. */
> + out_be32(IAR, ~(u32) 0);

I personally prefer to use 0xffffffff here.

> + /* Turn on the Master Enable. */
> + out_be32(MER, 0x3UL);
> +
> + return;
> +}
> +
> +int xilinx_pic_irq_get(void)
> +{
> + u32 irq;
> + irq = in_be32(IVR);
> +
> + /* If no interrupt is pending then all bits of the IVR are set to 1. As
> + * the IVR is as many bits wide as numbers of inputs are available.
> + * Therefore, if all bits of the IVR are set to one, its content will
> + * be bigger than XPAR_INTC_MAX_NUM_INTR_INPUTS.
> + */
> + if (irq >= XPAR_INTC_MAX_NUM_INTR_INPUTS)
> + irq = -1; /* report no pending interrupt. */
> +
> + debug("get_irq: %d\n", irq);
> + return (irq);
> +}
> +
> +void pic_irq_enable(unsigned int irq)
> +{
> + unsigned long mask = IRQ_MASK(irq);
> + debug("enable: %d\n", irq);
> + out_be32(SIE, mask);
> +}

I suggest you decide for one type here and not use "unsigned int"
and "unsigned long". I use u32 now if possible. This is for the whole file.

> +
> +void pic_irq_disable(unsigned int irq)
> +{
> + unsigned long mask = IRQ_MASK(irq);
> + debug("disable: %d\n", irq);
> + out_be32(CIE, mask);
> +}
> +
> +void pic_irq_ack(unsigned int irq)
> +{
> + unsigned long mask = IRQ_MASK(irq);
> + debug("ack: %d\n", irq);
> + out_be32(IAR, mask);
> +}
> +
> +void external_interrupt(struct pt_regs *regs)
> +{
> + int irq;
> +
> + irq = xilinx_pic_irq_get();
> + if (irq < 0)
> + return;
> +
> + interrupt_run_handler(irq);
> +
> + return;
> +}
> diff --git a/include/asm-ppc/interrupt.h b/include/asm-ppc/interrupt.h
> new file mode 100644
> index 0000000..6c84a84
> --- /dev/null
> +++ b/include/asm-ppc/interrupt.h
> @@ -0,0 +1,37 @@
> +/*
> + * (C) Copyright 2008
> + * Ricado Ribalda-Universidad Autonoma de Madrid-ricardo.ribalda at uam.es
> + * This work has been supported by: QTechnology http://qtec.com/
> + * Based on interrupts.c Wolfgang Denk-DENX Software
> Engineering-wd at denx.de + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> +*/
> +#ifndef INTERRUPT_H
> +#define INTERRUPT_H
> +
> +
> +#if defined(CONFIG_440_VIRTEX5)
> +#define IRQ_MAX XPAR_INTC_MAX_NUM_INTR_INPUTS
> +#else
> +#define IRQ_MAX UIC_MAX * 32
> +#endif

Please move those defines into some interrupt controller specific headers. For
uic version this is include/asm-ppc/ppc4xx-uic.h.

> +
> +void pic_enable(void);
> +void pic_irq_enable(unsigned int irq);
> +void pic_irq_disable(unsigned int irq);
> +void pic_irq_ack(unsigned int irq);
> +void external_interrupt(struct pt_regs *regs);
> +void interrupt_run_handler(int vec);
> +
> +#endif
> diff --git a/include/asm-ppc/processor.h b/include/asm-ppc/processor.h
> index 6e134c3..5501244 100644
> --- a/include/asm-ppc/processor.h
> +++ b/include/asm-ppc/processor.h
> @@ -839,6 +839,8 @@
> #define PVR_86xx 0x80040000
> #define PVR_86xx_REV1 (PVR_86xx | 0x0010)
>
> +#define PVR_VIRTEX5 0x7ff21912
> +
> /*
> * For the 8xx processors, all of them report the same PVR family for
> * the PowerPC core. The various versions of these processors must be

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office at denx.de
=====================================================================