[U-Boot] [PATCH] Add support for LPC2468 from NXP

[Wolfgang Denk <wd@denx.de>]

Dear Remco Poelstra,

In message <1274098916-1805-1-git-send-email-remco.poelstra@duran-audio.com> you wrote:
> Add Support for LPC2468 from NXP
> 
> Basic startup code
> Internal flash is uspported (for environment storage)
...
> --- a/Makefile
> +++ b/Makefile
> @@ -3148,6 +3148,9 @@ evb4510_config :	unconfig
>  lpc2292sodimm_config:	unconfig
>  	@$(MKCONFIG) $(@:_config=) arm arm720t lpc2292sodimm NULL lpc2292
>  
> +LPC2468_config:		unconfig
> +	@$(MKCONFIG) $(@:_config=) arm arm720t LPC2468  NULL lpc24xx

Is there any specific reason for not using locwer case names, so
lpc22xx and lpc24xx are somewhat similar?

> --- a/arch/arm/cpu/arm720t/cpu.c
> +++ b/arch/arm/cpu/arm720t/cpu.c
> @@ -63,7 +63,9 @@ 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)

Please sort list.

> diff --git a/arch/arm/cpu/arm720t/interrupts.c b/arch/arm/cpu/arm720t/interrupts.c
> index eb8d425..2ef7101 100644
> --- a/arch/arm/cpu/arm720t/interrupts.c
> +++ b/arch/arm/cpu/arm720t/interrupts.c
...
> @@ -80,6 +82,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) ();

Please unify with code for the LPC2292 and get rid of the #ifdef.

>  int timer_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 */
>  	IRQEN = 0;
> @@ -191,6 +205,13 @@ int timer_init (void)
>  	PUT32(T0MCR, 0);
>  	PUT32(T0TC, 0);
>  	PUT32(T0TCR, 1);	/* enable timer0 */
> +#elif defined(CONFIG_LPC2468)
> +	writel (0, &(timer0->ir));		/*disable all timer0 interupts */
> +	writel (0, &(timer0->tcr));		/*disable timer0 */
> +	writel (CFG_SYS_CLK_FREQ / CONFIG_SYS_HZ - 1, &(timer0->pr));
> +	writel (0, &(timer0->mcr));
> +	writel (0, &(timer0->tc));
> +	writel (1, &(timer0->tcr));

Same here.

> -#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)

Please sort list.

>  void reset_timer_masked (void)
>  {
>  	/* reset time */
> +#if defined(CONFIG_LPC2468)
> +	timer_2468_t *timer0=&(((immap_t *)CONFIG_SYS_IMMAP)->apb.timer0);
> +	lastdec = readl(&(timer0->tc));
> +#else
>  	lastdec = READ_TIMER;
> +#endif
>  	timestamp = 0;
>  }

I think we can avoid this #ifdef as well?

>  ulong get_timer_masked (void)
>  {
> +#if defined(CONFIG_LPC2468)
> +	timer_2468_t *timer0=&(((immap_t *)CONFIG_SYS_IMMAP)->apb.timer0);
> +	ulong now = readl(&(timer0->tc));
> +	
> +	if (lastdec <= now) {
> +		/* normal mode */
> +		timestamp += now - lastdec;
> +	} else {
> +		/* we have an overflow ... */
> +		timestamp += now + TIMER_LOAD_VAL - lastdec;
> +	}
> +#else
>  	ulong now = READ_TIMER;
>  
>  	if (lastdec >= now) {
> @@ -261,6 +300,8 @@ ulong get_timer_masked (void)
>  		/* we have an overflow ... */
>  		timestamp += lastdec + TIMER_LOAD_VAL - now;
>  	}
> +#endif

Ditto here.

> diff --git a/arch/arm/cpu/arm720t/lpc24xx/flash.c b/arch/arm/cpu/arm720t/lpc24xx/flash.c
> new file mode 100644
> index 0000000..963bf6e
> --- /dev/null
> +++ b/arch/arm/cpu/arm720t/lpc24xx/flash.c

This looks as if it were mostly a verbatim copy of
"arch/arm/cpu/arm720t/lpc2292/flash.c" - please unify or at least
factor out common parts.

> diff --git a/arch/arm/cpu/arm720t/lpc24xx/iap_entry.S b/arch/arm/cpu/arm720t/lpc24xx/iap_entry.S
> new file mode 100644
> index 0000000..c31d519
> --- /dev/null
> +++ b/arch/arm/cpu/arm720t/lpc24xx/iap_entry.S
> @@ -0,0 +1,7 @@
> +IAP_ADDRESS:	.word	0x7FFFFFF1
> +
> +.globl iap_entry
> +iap_entry:
> +	ldr	r2, IAP_ADDRESS
> +	bx	r2
> +	mov	pc, lr

Verbatim copy of arch/arm/cpu/arm720t/lpc2292/iap_entry.S - please
unify.

...
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-lpc24xx/hardware.h
> @@ -0,0 +1,32 @@
> +#ifndef __ASM_ARCH_HARDWARE_H
> +#define __ASM_ARCH_HARDWARE_H
> +
> +/*
...
> + */
> +
> +#if defined(CONFIG_LPC2468)
> +#else
> +#error No hardware file defined for this configuration
> +#endif
> +
> +#endif /* __ASM_ARCH_HARDWARE_H */

Do we really need such an empty file?


> diff --git a/arch/arm/include/asm/arch-lpc24xx/immap.h b/arch/arm/include/asm/arch-lpc24xx/immap.h
> new file mode 100644
> index 0000000..02efb5f
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-lpc24xx/immap.h


As far as I can tell large parts of this look extremely similar to
the definitions in arch/arm/include/asm/arch-lpc2292/lpc2292_registers.h

Can we unify these and get rid of
arch/arm/include/asm/arch-lpc2292/lpc2292_registers.h, then?

...
> +typedef struct ssp1_2468 {
> +	u8 fixme[0x4000];
> +} ssp1_2468_t;
> +
> +typedef struct adc_2468 {
> +	u8 fixme[0x4000];
> +} adc_2468_t;
> +
> +typedef struct can_accept_ram_2468 {
> +	u8 fixme[0x4000];
> +} can_accept_ram_2468_t;
> +
> +typedef struct can_accept_filter_2468 {
> +	u8 fixme[0x4000];
> +} can_accept_filter_2468_t;
> +
> +typedef struct can_common_2468 {
> +	u8 fixme[0x4000];
> +} can_common_2468_t;
> +
> +typedef struct can1_2468 {
> +	u8 fixme[0x4000];
> +} can1_2468_t;
> +
> +typedef struct can2_2468 {
> +	u8 fixme[0x4000];
> +} can2_2468_t;
> +
> +typedef struct i2c1_2468 {
> +	u8 fixme[0x4000];
> +} i2c1_2468_t;
...


Do we _really_ need all this?

> diff --git a/arch/arm/lib/eabi_compat.c b/arch/arm/lib/eabi_compat.c
> index 86eacf1..eb3e26d 100644
> --- a/arch/arm/lib/eabi_compat.c
> +++ b/arch/arm/lib/eabi_compat.c
> @@ -16,3 +16,8 @@ int raise (int signum)
>  	printf("raise: Signal # %d caught\n", signum);
>  	return 0;
>  }
> +
> +/* Dummy function to avoid linker complaints */
> +void __aeabi_unwind_cpp_pr0(void)
> +{
> +};

Bogus change. Please drop.

> diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
> index c731bfb..05fd6c9 100644
> --- a/drivers/serial/Makefile
> +++ b/drivers/serial/Makefile
> @@ -43,6 +43,7 @@ COBJS-$(CONFIG_IMX_SERIAL) += serial_imx.o
>  COBJS-$(CONFIG_IXP_SERIAL) += serial_ixp.o
>  COBJS-$(CONFIG_KS8695_SERIAL) += serial_ks8695.o
>  COBJS-$(CONFIG_LPC2292_SERIAL) += serial_lpc2292.o
> +COBJS-$(CONFIG_LPC2468_SERIAL) += serial_lpc2468.o
>  COBJS-$(CONFIG_LH7A40X_SERIAL) += serial_lh7a40x.o
>  COBJS-$(CONFIG_MAX3100_SERIAL) += serial_max3100.o
>  COBJS-$(CONFIG_MXC_UART) += serial_mxc.o
> 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


Can this be unified with "drivers/serial/serial_lpc2292.c" ?


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
I have never understood the female capacity to avoid a direct  answer
to any question.
	-- Spock, "This Side of Paradise", stardate 3417.3