Dear Remco Poelstra,
In message <49c8be7a.10...@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-b...@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 <c...@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: w...@denx.de
panic: can't find /
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
