Re: [U-Boot-Users] [PATCH] CPU PPC440x5 on Virtex5 FX

Hello Stefan

I will regenerate the patches and send then again. I will be out 'till Monay.

Best Regards and thanks for your comments

On Fri, Jul 11, 2008 at 2:22 PM, Stefan Roese sr@denx.de wrote:

Ricardo,

On Friday 11 July 2008, Ricardo Ribalda Delgado wrote:

...

This patchs gives support for the embbedded ppc440 on the Virtex5 FPGAs

Thanks.

Some general comments:

Please rebase your 4xx related patches on the "next" branch of my u-boot-ppc4xx repository. I just pushed some bigger rework/cleanup patchsets that are destined for the next merge window.

And please add "ppc4xx: " to the beginning of the patch subject and add me to CC on 4xx related patches.

More comments below...

...

---

cpu/ppc4xx/4xx_enet.c | 3 +- cpu/ppc4xx/4xx_uart.c | 3 + cpu/ppc4xx/cpu.c | 4 + cpu/ppc4xx/gpio.c | 5 + cpu/ppc4xx/interrupts.c | 238 ++++++++++++++++++++++++++++++++++++------- cpu/ppc4xx/miiphy.c | 5 + cpu/ppc4xx/speed.c | 6 +- include/asm-ppc/processor.h | 3 +- net/eth.c | 3 +- 9 files changed, 228 insertions(+), 42 deletions(-)

diff --git a/cpu/ppc4xx/4xx_enet.c b/cpu/ppc4xx/4xx_enet.c index 4e863dc..cf26237 100644 --- a/cpu/ppc4xx/4xx_enet.c +++ b/cpu/ppc4xx/4xx_enet.c @@ -97,7 +97,8 @@

• network support enabled.
• Remark: CONFIG_405 describes Xilinx PPC405 FPGA without EMAC

controller! */ -#if defined(CONFIG_CMD_NET) && !defined(CONFIG_405) && !defined(CONFIG_IOP480) +#if defined(CONFIG_CMD_NET) && !defined(CONFIG_405) && !defined(CONFIG_IOP480) \ + && !defined(CONFIG_440_VIRTEX5)

#if !(defined(CONFIG_MII) || defined(CONFIG_CMD_MII)) #error "CONFIG_MII has to be defined!" diff --git a/cpu/ppc4xx/4xx_uart.c b/cpu/ppc4xx/4xx_uart.c index a7587d4..37d3dbc 100644 --- a/cpu/ppc4xx/4xx_uart.c +++ b/cpu/ppc4xx/4xx_uart.c @@ -48,6 +48,7 @@ #include <watchdog.h> #include <asm/ppc4xx-intvec.h>

+#if !defined(CONFIG_440_VIRTEX5)

You add this "#if !defined(CONFIG_440_VIRTEX5)" to many of the cpu/ppc4xx files with this patch. I don't like this. We should let the Makefile handle this. Perhaps something like this:

ifndef $(CONFIG_440_VIRTEX5) COBJS += 4xx_enet.o .. endif

...

#ifdef CONFIG_SERIAL_MULTI #include <serial.h> #endif @@ -873,3 +874,5 @@ int serial_tstc(void) #endif /* CONFIG_SERIAL_MULTI */

#endif /* CONFIG_405GP || CONFIG_405CR */

+#endif diff --git a/cpu/ppc4xx/cpu.c b/cpu/ppc4xx/cpu.c index 39f439d..f510cdd 100644 --- a/cpu/ppc4xx/cpu.c +++ b/cpu/ppc4xx/cpu.c @@ -508,6 +508,10 @@ int checkcpu (void) puts("GT Rev. A"); strcpy(addstr, "Security/Kasumi support"); break;

• case PVR_VIRTEX5:
  

•         printf(" VIRTEX5");
  

•         break;
  
  default:
          printf (" UNKNOWN (PVR=%08x)", pvr);
  

diff --git a/cpu/ppc4xx/gpio.c b/cpu/ppc4xx/gpio.c index df99f53..59159ee 100644 --- a/cpu/ppc4xx/gpio.c +++ b/cpu/ppc4xx/gpio.c @@ -26,6 +26,9 @@ #include <asm/io.h> #include <asm/gpio.h>

+#if !defined(CONFIG_440_VIRTEX5)

Again, move this to the Makefile instead.

...

#if defined(CFG_4xx_GPIO_TABLE) gpio_param_s const gpio_tab[GPIO_GROUP_MAX][GPIO_MAX] = CFG_4xx_GPIO_TABLE; #endif @@ -253,3 +256,5 @@ void gpio_set_chip_configuration(void) } } #endif /* CFG_4xx_GPIO_TABLE */

+#endif diff --git a/cpu/ppc4xx/interrupts.c b/cpu/ppc4xx/interrupts.c index 8620e2b..4648f52 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@sandburst.com

• • (C) Copyright 2008 (PPC440X05 port for Virtex 5 FX)
• • Ricardo Ribalda, Universidad Autonoma de Madrid, ricardo.ribalda@uam.es
• • Work supported by Qtechnology (htpp://qtec.com)
• • See file CREDITS for list of people who contributed to this
  • project.

@@ -41,6 +45,7 @@ DECLARE_GLOBAL_DATA_PTR; /*

• Define the number of UIC's

*/ +#if !defined(CONFIG_440_VIRTEX5) #if defined(CONFIG_440SPE) || \ defined(CONFIG_460EX) || defined(CONFIG_460GT) #define UIC_MAX 4 @@ -54,17 +59,23 @@ DECLARE_GLOBAL_DATA_PTR; #else #define UIC_MAX 1 #endif

+#else +void xilinx_pic_enable(void); +#endif /*

• CPM interrupt vector functions.

*/ -struct irq_action { +struct irq_action {

You seem to have run Lindent on this file too, correct? Please don't do this when you also add and/or fix something. This makes it hard to see the "real" changes. Only run Lindent on newly created files.

...

  interrupt_handler_t *handler;
  void *arg;
  int count;

};

+#if !defined(CONFIG_440_VIRTEX5) static struct irq_action irq_vecs[UIC_MAX * 32]; +#else +static struct irq_action irq_vecs[XPAR_INTC_MAX_NUM_INTR_INPUTS]; +#endif

u32 get_dcr(u16); void set_dcr(u16, u32); @@ -81,27 +92,25 @@ static __inline__ void set_evpr(unsigned long val) asm volatile("mtspr 0x03f,%0" : : "r" (val)); }

-#else /* !defined(CONFIG_440) */ +#else /* !defined(CONFIG_440) */

static __inline__ void set_pit(unsigned long val) { asm volatile("mtpit %0" : : "r" (val)); }

static __inline__ void set_tcr(unsigned long val) { asm volatile("mttcr %0" : : "r" (val)); }

static __inline__ void set_evpr(unsigned long val) { asm volatile("mtevpr %0" : : "r" (val)); } -#endif /* defined(CONFIG_440 */ +#endif /* defined(CONFIG_440 */

-int interrupt_init_cpu (unsigned *decrementer_count) +int interrupt_init_cpu(unsigned *decrementer_count) { int vec; unsigned long val; @@ -112,7 +121,11 @@ int interrupt_init_cpu (unsigned *decrementer_count) /* * Mark all irqs as free */ +#if !defined (CONFIG_440_VIRTEX5) for (vec = 0; vec < (UIC_MAX * 32); vec++) { +#else

• for (vec = 0; vec < XPAR_INTC_MAX_NUM_INTR_INPUTS; vec++) {
  

+#endif irq_vecs[vec].handler = NULL; irq_vecs[vec].arg = NULL; irq_vecs[vec].count = 0; @@ -123,19 +136,19 @@ int interrupt_init_cpu (unsigned *decrementer_count) * Init PIT */ #if defined(CONFIG_440)

• val = mfspr( tcr );
  

• val &= (~0x04400000);           /* clear DIS & ARE */
  

• mtspr( tcr, val );
  

• mtspr( dec, 0 );                /* Prevent exception after TSR clear*/
  

• mtspr( decar, 0 );              /* clear reload */
  

• mtspr( tsr, 0x08000000 );       /* clear DEC status */
  

• val = gd->bd->bi_intfreq/1000;  /* 1 msec */
  

• mtspr( decar, val );            /* Set auto-reload value */
  

• mtspr( dec, val );              /* Set inital val */
  

• val = mfspr(tcr);
  

• val &= (~0x04400000);   /* clear DIS & ARE */
  

• mtspr(tcr, val);
  

• mtspr(dec, 0);          /* Prevent exception after TSR clear */
  

• mtspr(decar, 0);        /* clear reload */
  

• mtspr(tsr, 0x08000000); /* clear DEC status */
  

• val = gd->bd->bi_intfreq / 1000;        /* 1 msec */
  

• mtspr(decar, val);      /* Set auto-reload value */
  

• mtspr(dec, val);        /* Set inital val */
  

#else set_pit(gd->bd->bi_intfreq / 1000); #endif -#endif /* CONFIG_4xx */ +#endif /* CONFIG_4xx */

#ifdef CONFIG_ADCIOP /* @@ -156,6 +169,8 @@ int interrupt_init_cpu (unsigned *decrementer_count) */ set_evpr(0x00000000);

+#if !defined (CONFIG_440_VIRTEX5)

#if !defined(CONFIG_440GX) #if (UIC_MAX > 1) /* Install the UIC1 handlers */ @@ -170,7 +185,7 @@ int interrupt_init_cpu (unsigned *decrementer_count) irq_install_handler(VECNUM_UIC3NC, uic_cascade_interrupt, 0); irq_install_handler(VECNUM_UIC3C, uic_cascade_interrupt, 0); #endif -#else /* !defined(CONFIG_440GX) */ +#else /* !defined(CONFIG_440GX) */ /* Take the GX out of compatibility mode * Travis Sawyer, 9 Mar 2004 * NOTE: 440gx user manual inconsistency here @@ -179,18 +194,27 @@ int interrupt_init_cpu (unsigned *decrementer_count) */ mfsdr(sdr_mfr, val); val &= ~0x10000000;

• mtsdr(sdr_mfr,val);
  

• mtsdr(sdr_mfr, val);
  
  /* Enable UIC interrupts via UIC Base Enable Register */
  mtdcr(uicb0sr, UICB0_ALL);
  mtdcr(uicb0er, 0x54000000);
  /* None are critical */
  mtdcr(uicb0cr, 0);
  

-#endif /* !defined(CONFIG_440GX) */ +#endif /* !defined(CONFIG_440GX) */

+#else

• /*
  

•  * Enable xilinx pic
  

•  */
  

• xilinx_pic_enable();
  

+#endif

  return (0);

}

+#if !defined(CONFIG_440_VIRTEX5) /* Handler for UIC interrupt */ static void uic_interrupt(u32 uic_base, int vec_base) { @@ -214,7 +238,7 @@ static void uic_interrupt(u32 uic_base, int vec_base)

                  if (irq_vecs[vec].handler != NULL) {
                          /* call isr */

•                         (*irq_vecs[vec].handler)(irq_vecs[vec].arg);
  

•                         (*irq_vecs[vec].handler) (irq_vecs[vec].arg);
                  } else {
                          set_dcr(uic_base + UIC_ER,
                                  get_dcr(uic_base + UIC_ER) &
  

@@ -227,7 +251,8 @@ static void uic_interrupt(u32 uic_base, int vec_base) * After servicing the interrupt, we have to remove the * status indicator */

•                 set_dcr(uic_base + UIC_SR, (0x80000000 >> (vec & 0x1f)));
  

•                 set_dcr(uic_base + UIC_SR,
  

•                         (0x80000000 >> (vec & 0x1f)));
          }
  
          /*
  

@@ -254,10 +279,10 @@ static void uic_cascade_interrupt(void *para) #define UIC_BMSR uic0msr #define UIC_BSR uic0sr #endif -#else /* CONFIG_440 */ +#else /* CONFIG_440 */ #define UIC_BMSR uicmsr #define UIC_BSR uicsr -#endif /* CONFIG_440 */ +#endif /* CONFIG_440 */

/*

• Handle external interrupts

@@ -294,9 +319,9 @@ void external_interrupt(struct pt_regs *regs) if ((UICB0_UIC0CI & uic_msr) || (UICB0_UIC0NCI & uic_msr)) uic_interrupt(UIC0_DCR_BASE, 0); #endif -#else /* CONFIG_440 */ +#else /* CONFIG_440 */ uic_interrupt(UIC0_DCR_BASE, 0); -#endif /* CONFIG_440 */ +#endif /* CONFIG_440 */

  mtdcr(UIC_BSR, uic_msr);

@@ -313,7 +338,8 @@ void irq_install_handler(int vec, interrupt_handler_t * handler, void *arg) /* * Print warning when replacing with a different irq vector */

• if ((irq_vecs[vec].handler != NULL) && (irq_vecs[vec].handler !=
  

handler)) { + if ((irq_vecs[vec].handler != NULL)

•     && (irq_vecs[vec].handler != handler)) {
          printf("Interrupt vector %d: handler 0x%x replacing 0x%x\n",
                 vec, (uint) handler, (uint) irq_vecs[vec].handler);
  }
  

@@ -339,7 +365,7 @@ void irq_install_handler(int vec, interrupt_handler_t * handler, void *arg) debug("Install interrupt for vector %d ==> %p\n", vec, handler); }

-void irq_free_handler (int vec) +void irq_free_handler(int vec) { int i;

@@ -365,28 +391,166 @@ void irq_free_handler (int vec) irq_vecs[vec].handler = NULL; irq_vecs[vec].arg = NULL; } +#else

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

This seems to be a completely different interrupt handler than used on IBM/AMCC PPC4xx. Please don't add this code to this file but to a new file. Does this Xilinx interrupt controller have a name? I'll probably rename the current interrupt.c to uic.c soon.

...

+#define intc_out_be32(addr, mask) out32((addr), (mask)) +#define intc_in_be32(addr) in32((addr))

Please don't define new accessor macros here. Use the existing ones, this makes the code better readable.

And use out_be32()/in_be32() instead of out32()/in32().

...

+void xilinx_pic_enable(void) +{

• printf("Xilinx PIC at 0x%8x\n", intc);
  

• /*
  

•  * Disable all external interrupts until they are
  

•  * explicitly requested.
  

•  */
  

• intc_out_be32(intc + IER, 0);
  

• /* Acknowledge any pending interrupts just in case. */
  

• intc_out_be32(intc + IAR, ~(u32) 0);
  

• /* Turn on the Master Enable. */
  

• intc_out_be32(intc + MER, 0x3UL);
  

• return;
  

+}

+int xilinx_pic_irq_get(void) +{

• u32 irq;
  

• irq = intc_in_be32(intc + 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);
  

+}

+static void xilinx_pic_irq_enable(unsigned int irq) +{

• unsigned long mask = (0x00000001 << (irq & 31));
  

• debug("enable: %d\n", irq);
  

• intc_out_be32(intc + SIE, mask);
  

+}

+static void xilinx_pic_irq_disable(unsigned int irq) +{

• unsigned long mask = (0x00000001 << (irq & 31));
  

• debug("disable: %d\n", irq);
  

• intc_out_be32(intc + CIE, mask);
  

+}

+static void xilinx_pic_irq_ack(unsigned int irq) +{

• unsigned long mask = (0x00000001 << (irq & 31));
  

• debug("ack: %d\n", irq);
  

• intc_out_be32(intc + IAR, mask);
  

+}

+static void xilinx_pic_interrupt(void) +{

• int irq;
  

• irq = xilinx_pic_irq_get();
  

• if (irq < 0)
  

•         return;
  

• irq_vecs[irq].count++;
  

• if (irq_vecs[irq].handler != NULL) {
  

•         /* call isr */
  

•         (*irq_vecs[irq].handler) (irq_vecs[irq].arg);
  

• } else {
  

•         xilinx_pic_irq_disable(irq);
  

•         printf("Masking bogus interrupt %d\n", irq);
  

• }
  

• xilinx_pic_irq_ack(irq);
  

• return;
  

+}

+void external_interrupt(struct pt_regs *regs) +{

• xilinx_pic_interrupt();
  

+}

+/*

• • Install and free a interrupt handler.
• */

+void irq_install_handler(int vec, interrupt_handler_t * handler, void *arg) +{

• /*
  

•  * Print warning when replacing with a different irq vector
  

•  */
  

• if ((irq_vecs[vec].handler != NULL)
  

•     && (irq_vecs[vec].handler != handler)) {
  

•         printf("Interrupt vector %d: handler 0x%x replacing 0x%x\n",
  

•                vec, (uint) handler, (uint) irq_vecs[vec].handler);
  

• }
  

• irq_vecs[vec].handler = handler;
  

• irq_vecs[vec].arg = arg;
  

• xilinx_pic_irq_enable(vec);
  

• debug("Install interrupt for vector %d ==> %p\n", vec, handler);
  

+}

+void irq_free_handler(int vec) +{

• debug("Free interrupt for vector %d ==> %p\n",
  

•       vec, irq_vecs[vec].handler);
  

• xilinx_pic_irq_disable(vec);
  

• irq_vecs[vec].handler = NULL;
  

• irq_vecs[vec].arg = NULL;
  

+}

+#endif

-void timer_interrupt_cpu (struct pt_regs *regs) +void timer_interrupt_cpu(struct pt_regs *regs) { /* nothing to do here */ return; }

#if defined(CONFIG_CMD_IRQ) -int do_irqinfo(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) +int do_irqinfo(cmd_tbl_t * cmdtp, int flag, int argc, char *argv[]) { int vec;

• printf ("Interrupt-Information:\n");
  

• printf ("Nr  Routine   Arg       Count\n");
  

• printf("Interrupt-Information:\n");
  

• printf("Nr  Routine   Arg       Count\n");
  
  

+#if !defined (CONFIG_440_VIRTEX5) for (vec = 0; vec < (UIC_MAX * 32); vec++) { +#else

• for (vec = 0; vec < XPAR_INTC_MAX_NUM_INTR_INPUTS; vec++) {
  

+#endif if (irq_vecs[vec].handler != NULL) {

•                 printf ("%02d  %08lx  %08lx  %d\n",
  

•                         vec,
  

•                         (ulong)irq_vecs[vec].handler,
  

•                         (ulong)irq_vecs[vec].arg,
  

•                         irq_vecs[vec].count);
  

•                 printf("%02d  %08lx  %08lx  %d\n",
  

•                        vec,
  

•                        (ulong) irq_vecs[vec].handler,
  

•                        (ulong) irq_vecs[vec].arg, irq_vecs[vec].count);
          }
  }
  
  

diff --git a/cpu/ppc4xx/miiphy.c b/cpu/ppc4xx/miiphy.c index c882720..172fbcd 100644 --- a/cpu/ppc4xx/miiphy.c +++ b/cpu/ppc4xx/miiphy.c @@ -43,6 +43,9 @@ #include <405_mal.h> #include <miiphy.h>

+#if !defined(CONFIG_440_VIRTEX5)

-> Makefile.

...

#if !defined(CONFIG_PHY_CLK_FREQ) #define CONFIG_PHY_CLK_FREQ 0 #endif @@ -331,3 +334,5 @@ int emac4xx_miiphy_write (char *devname, unsigned char addr, unsigned char reg, { return emac_miiphy_command(addr, reg, EMAC_STACR_WRITE, value); }

+#endif diff --git a/cpu/ppc4xx/speed.c b/cpu/ppc4xx/speed.c index 34bd721..d1681e4 100644 --- a/cpu/ppc4xx/speed.c +++ b/cpu/ppc4xx/speed.c @@ -415,7 +415,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_440_VIRTEX5)
  

void get_sys_info (sys_info_t * sysInfo) { unsigned long strp0; @@ -448,6 +449,8 @@ void get_sys_info (sys_info_t * sysInfo) sysInfo->freqUART = sysInfo->freqPLB; } #else

+#if !defined(CONFIG_440_VIRTEX5) void get_sys_info (sys_info_t * sysInfo) { unsigned long strp0; @@ -534,6 +537,7 @@ void get_sys_info (sys_info_t * sysInfo) }

#endif +#endif

#if defined(CONFIG_YUCCA) unsigned long determine_sysper(void) diff --git a/include/asm-ppc/processor.h b/include/asm-ppc/processor.h index 10fd478..88f193a 100644 --- a/include/asm-ppc/processor.h +++ b/include/asm-ppc/processor.h @@ -827,13 +827,12 @@ #define PVR_7400 0x000C0000 #define PVR_7410 0x800C0000 #define PVR_7450 0x80000000

#define PVR_85xx 0x80200000 #define PVR_85xx_REV1 (PVR_85xx | 0x0010) #define PVR_85xx_REV2 (PVR_85xx | 0x0020)

Please don't remove those empty lines here.

...

#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

diff --git a/net/eth.c b/net/eth.c index 7fc9aee..54b59b8 100644 --- a/net/eth.c +++ b/net/eth.c @@ -621,7 +621,8 @@ int eth_initialize(bd_t *bis) at91rm9200_miiphy_initialize(bis); #endif #if defined(CONFIG_4xx) && !defined(CONFIG_IOP480) \

• && !defined(CONFIG_AP1000) && !defined(CONFIG_405)
  

• && !defined(CONFIG_AP1000) && !defined(CONFIG_405) \
  

• && !defined(CONFIG_440_VIRTEX5)
  emac4xx_miiphy_initialize(bis);
  

#endif #if defined(CONFIG_MCF52x2)

Please send this eth.c patch as a separate patch. It need to go through the networking custodian.

Please fix and resubmit. Thanks.

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@denx.de =====================================================================