Re: [U-Boot] [PATCH 1/2] LPC2468 support

24 Mar 2009

Dear Remco Poelstra,
In message 49C8BE7A.10504@duran-audio.com you wrote:
...
This patch includes support for the LPC2468 processor from NXP.
The example board will follow when this patch is OK.
Such a comment does not belong into the commit message. Please mode it
below the "---" line.
...
Signed-off-by: Remco Poelstra remco.poelstra+u-boot@duran-audio.com
diff -upNr u-boot-orig/cpu/arm720t/interrupts.c u-boot-cleanup/cpu/arm720t/interrupts.c

--- u-boot-orig/cpu/arm720t/interrupts.c	2009-03-18 00:42:12.000000000 +0100
+++ u-boot-cleanup/cpu/arm720t/interrupts.c	2009-03-24 11:48:50.000000000 +0100
@@ -29,7 +29,11 @@
  #include <common.h>
  #include <clps7111.h>
  #include <asm/proc-armv/ptrace.h>
+#if defined(CONFIG_LPC2468)
+#include <asm/arch/immap.h>
+#else
  #include <asm/hardware.h>
+#endif
Is there no way we can do without such a #ifdef here?
...
#ifndef CONFIG_NETARM
  /* we always count down the max. */
@@ -40,6 +44,11 @@
  #ifdef CONFIG_LPC2292
  #undef READ_TIMER
  #define READ_TIMER (0xFFFFFFFF - GET32(T0TC))
+#elif defined(CONFIG_LPC2468)
+#undef TIMER_LOAD_VAL
+#define TIMER_LOAD_VAL 0
+#undef READ_TIMER
+#define READ_TIMER (0xFFFFFFFF - 0xE0004008)
NAK. When you have to #unifdef existing variable definitions, then
ther eis something fundamentally wrong. Please fix this problem at the
cause, i. e. where the wroing values are defined.
...
#endif
#else
@@ -80,6 +89,14 @@ void do_irq (struct pt_regs *pt_regs)
      pfnct = (void (*)(void))VICVectAddr;
  (*pfnct)();

+#elif defined(CONFIG_LPC2468)

void (*pfnct) (void);
vic_2468_t *vic = &(((immap_t *)CONFIG_SYS_IMMAP)->ahb.vic);

pfnct = (void (*)(void))(&(vic->vicaddr));

(*pfnct) ();

Is there no way to combine this code with the one for the LPC2292? It
doesn't look that different to me...
...
#else
  #error do_irq() not defined for this CPU type
  #endif
@@ -112,6 +129,9 @@ static ulong lastdec;
int interrupt_init (void)
  {
+#if defined(CONFIG_LPC2468)

timer_2468_t *timer0=&(((immap_t *)CONFIG_SYS_IMMAP)->apb.timer0);

+#endif
#if defined(CONFIG_NETARM)
   /* disable all interrupts */
@@ -185,6 +205,13 @@ int interrupt_init (void)
   PUT32(T0MCR, 0);
   PUT32(T0TC, 0);
   PUT32(T0TCR, 1);	/* enable timer0 */
+#elif defined(CONFIG_LPC2468)

PUT32 (&(timer0->ir), 0);		/*disable all timer0 interupts */
PUT32 (&(timer0->tcr), 0);		/*disable timer0 */
PUT32 (&(timer0->pr), CFG_SYS_CLK_FREQ / CONFIG_SYS_HZ - 1);
PUT32 (&(timer0->mcr), 0);
PUT32 (&(timer0->tc), 0);
PUT32 (&(timer0->tcr), 1);

Again: Is there no way to combine this code with the one for the
LPC2292? It doesn't look that different to me...
...
...
--- u-boot-orig/cpu/arm720t/lpc24xx/Makefile	1970-01-01 01:00:00.000000000 +0100
+++ u-boot-cleanup/cpu/arm720t/lpc24xx/Makefile	2009-03-19 10:56:53.000000000 +0100
...
...
+$(SOBJS):

$(CC) $(AFLAGS) -march=armv4t -c -o $(SOBJS) iap_entry.S

Such compile options hsould probably be set globally, not just for
this single source file?
...
...
+int serial_tstc (void)
+{

uart_2468_t *uart0=&(((immap_t *)CONFIG_SYS_IMMAP)->apb.uart0);

return (GET8 (&(uart0->lsr)) & 1);

+}



#endif

Get rid of all these empty lines, please.
...
diff -upNr u-boot-orig/cpu/arm720t/start.S u-boot-cleanup/cpu/arm720t/start.S
--- u-boot-orig/cpu/arm720t/start.S	2009-03-18 00:42:12.000000000 +0100
+++ u-boot-cleanup/cpu/arm720t/start.S	2009-03-24 11:52:35.000000000 +0100
@@ -127,7 +127,7 @@ reset:
   bl	cpu_init_crit
  #endif
-#ifdef CONFIG_LPC2292
+#if defined(CONFIG_LPC2292) || defined(CONFIG_LPC2468)
Is there no way to combine this code with the one for the LPC2292?
...
bl	lowlevel_init
  #endif
@@ -368,6 +368,10 @@ lock_loop:
   ldr	r0, VPBDIV_ADR
   mov	r1, #0x01	/* VPB clock is same as process clock */
   str	r1, [r0]
+#elif defined(CONFIG_LPC2468)

ldr r0, =0x40008000 /*0x40000000 is internal SRAM, 0x4000FFFF is end of SRAM*/

Line too long.
...

mov sp,r0
sub sl,sp,#0x2000

Indentation by TAB, please.
...
#else
  #error No cpu_init_crit() defined for current CPU type
  #endif
@@ -383,7 +387,7 @@ lock_loop:
   str	r1, [r0]
  #endif
-#ifndef CONFIG_LPC2292
+#if	!defined(CONFIG_LPC2292) && !defined(CONFIG_LPC2468)
Is there no way to combine this code with the one for the LPC2292?
...
mov	ip, lr
   /*
    * before relocating, we have to setup RAM timing
@@ -601,7 +605,7 @@ reset_cpu:

on external peripherals such as watchdog timers, etc. */

#elif defined(CONFIG_INTEGRATOR) && defined(CONFIG_ARCH_INTEGRATOR)
   /* No specific reset actions for IntegratorAP/CM720T as yet */
-#elif defined(CONFIG_LPC2292)
+#elif defined(CONFIG_LPC2292) || defined(CONFIG_LPC2468)
Is there no way to combine this code with the one for the LPC2292?
...
.align	5
  .globl reset_cpu
  reset_cpu:
diff -upNr u-boot-orig/include/asm-arm/arch-lpc24xx/hardware.h u-boot-cleanup/include/asm-arm/arch-lpc24xx/hardware.h
--- u-boot-orig/include/asm-arm/arch-lpc24xx/hardware.h	1970-01-01 01:00:00.000000000 +0100
+++ u-boot-cleanup/include/asm-arm/arch-lpc24xx/hardware.h	2009-03-24 11:54:44.000000000 +0100
@@ -0,0 +1,32 @@
+#ifndef __ASM_ARCH_HARDWARE_H
+#define __ASM_ARCH_HARDWARE_H



+/*


Copyright (c) 2004	Cucy Systems (http://www.cucy.com)



Curt Brune curt@cucy.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


*/


+#if defined(CONFIG_LPC2468)
+#else
+#error No hardware file defined for this configuration
+#endif



+#endif /* __ASM_ARCH_HARDWARE_H */
Ummm... What exactly is this file needed for?
...
diff -upNr u-boot-orig/include/asm-arm/arch-lpc24xx/immap.h u-boot-cleanup/include/asm-arm/arch-lpc24xx/immap.h
--- u-boot-orig/include/asm-arm/arch-lpc24xx/immap.h	1970-01-01 01:00:00.000000000 +0100
+++ u-boot-cleanup/include/asm-arm/arch-lpc24xx/immap.h	2009-03-24 11:54:10.000000000 +0100
@@ -0,0 +1,362 @@
+/*


(C) Copyright 2009 Duran Audio B.V.







LPC2468 Internal Memory Map







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






*/


+#ifndef __LPC24XX_IMMAP_H
+#define __LPC24XX_IMMAP_H



+#include <asm/types.h>
+#include <config.h>



+/* Macros for reading/writing registers */
+#define PUT8(reg, value) (*(volatile unsigned char*)(reg) = (value))
+#define PUT16(reg, value) (*(volatile unsigned short*)(reg) = (value))
+#define PUT32(reg, value) (*(volatile unsigned int*)(reg) = (value))
+#define GET8(reg) (*(volatile unsigned char*)(reg))
+#define GET16(reg) (*(volatile unsigned short*)(reg))
+#define GET32(reg) (*(volatile unsigned int*)(reg))
Do you clain these are proper accessor functions for your processor?
...
+#define BFS32(reg,value) (*(volatile unsigned int*)(reg) |= (value))
+#define BFC32(reg, value) (*(volatile unsigned int*)(reg) &= (~value))



+typedef struct watchdog_2468 {

u8 fixme[0x4000];

+} watchdog2468_t;
Indentation by TAB, please - here and everywhere else.
...
diff -upNr u-boot-orig/include/flash.h u-boot-cleanup/include/flash.h
--- u-boot-orig/include/flash.h	2009-03-18 00:42:12.000000000 +0100
+++ u-boot-cleanup/include/flash.h	2009-03-24 11:56:34.000000000 +0100
@@ -339,7 +339,7 @@ extern flash_info_t *flash_get_info(ulon
  #define TOSH_ID_FVT160	0xC2		/* TC58FVT160 ID (16 M, top )		*/
  #define TOSH_ID_FVB160	0x43		/* TC58FVT160 ID (16 M, bottom )	*/
  #define PHILIPS_LPC2292 0x0401FF13  /* LPC2292 internal FLASH			*/



+#define PHILIPS_LPC2468 0x0603FF35  /* LPC2468 internal FLASH			*/
  /*-----------------------------------------------------------------------

Internal FLASH identification codes


Please keep the empty line here.
Best regards,
Wolfgang Denk
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
panic: can't find /

    