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

[Wolfgang Denk]

Dear Remco Poelstra,

In message <49F1A92F.9070804 at duran-audio.com> you wrote:
> This patch includes support for the LPC2468 processor from NXP.

Thanks.

Here a few comments:

> --- a/cpu/arm720t/cpu.c
> +++ b/cpu/arm720t/cpu.c
> @@ -75,7 +75,7 @@ int cleanup_before_linux (void)
>   	/* go to high speed */
>   	IO_SYSCON3 = (IO_SYSCON3 & ~CLKCTL) | CLKCTL_73;
>   #endif
> -#elif defined(CONFIG_NETARM) || defined(CONFIG_S3C4510B) || defined(CONFIG_LPC2292)
> +#elif defined(CONFIG_NETARM) || defined(CONFIG_S3C4510B) || defined(CONFIG_LPC2000)

Line too long.

> diff --git a/cpu/arm720t/interrupts.c b/cpu/arm720t/interrupts.c
> index 39ed345..d7aec27 100644
> --- a/cpu/arm720t/interrupts.c
> +++ b/cpu/arm720t/interrupts.c
> @@ -29,17 +29,26 @@
>   #include <common.h>
>   #include <clps7111.h>
>   #include <asm/proc-armv/ptrace.h>
> +#include <asm/io.h>
> +#if defined(CONFIG_LPC2468)
> +#include <asm/arch/immap.h>
> +#else
>   #include <asm/hardware.h>
> +#endif

Would it be possible to keep a generic ``#include <asm/hardware.h>''
here and add the processor specific stuff there?

>   #ifndef CONFIG_NETARM
> +
> +#if defined(CONFIG_LPC2292)
> +#define TIMER_LOAD_VAL 0xffff
> +#define READ_TIMER (0xFFFFFFFF - GET32(T0TC))
> +#elif defined(CONFIG_LPC2468)
> +#define TIMER_LOAD_VAL 0
> +#define READ_TIMER (0xFFFFFFFF - 0xE0004008)
> +#else

Can we please move this out of a common source file?

> -#if defined(CONFIG_IMPA7) || defined(CONFIG_EP7312) || defined(CONFIG_NETARM) || defined(CONFIG_ARMADILLO) || defined(CONFIG_LPC2292)
> +#if defined(CONFIG_IMPA7) || defined(CONFIG_EP7312) || defined(CONFIG_NETARM) || defined(CONFIG_ARMADILLO) || defined(CONFIG_LPC2000)

Line too long.

> --- a/cpu/arm720t/start.S
> +++ b/cpu/arm720t/start.S
...
> @@ -368,6 +368,11 @@ 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*/
> +    mov sp,r0
> +    sub sl,sp,#0x2000

Indentation not by TAB

> diff --git a/drivers/serial/serial_lpc2468.c b/drivers/serial/serial_lpc2468.c
> new file mode 100644
> index 0000000..4b8f241
> --- /dev/null
> +++ b/drivers/serial/serial_lpc2468.c
> @@ -0,0 +1,119 @@

Do we really need a new file here?


> diff --git a/include/asm-arm/arch-lpc24xx/immap.h b/include/asm-arm/arch-lpc24xx/immap.h
> new file mode 100644
> index 0000000..d37e4f1
> --- /dev/null
> +++ b/include/asm-arm/arch-lpc24xx/immap.h
> @@ -0,0 +1,351 @@
> +/*
> + * (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>
> +
> +typedef struct watchdog_2468 {
> +    u8 fixme[0x4000];
> +} watchdog2468_t;
> +
> +typedef struct timer_2468 {
> +    u32 ir;
> +    u32 tcr;
> +    u32 tc;
> +    u32 pr;
> +    u32 pc;
> +    u32 mcr;
> +    u32 mr0;
> +    u32 mr1;
> +    u32 mr2;
> +    u32 mr3;
> +    u32 ccr;
> +    u32 cr0;
> +    u32 cr1;
> +    u32 cr2;
> +    u32 cr3;
> +    u32 emr;
> +    u32 ctcr;
> +    u8 notused[0x3fbc];
> +} timer_2468_t;

Indentation not by TAB (whole file!).

How about adding some comments to explain what all these fields mean? 
(whole file!)


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 at denx.de
Of course there's no reason for it, it's just our policy.