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

Jean-Christophe PLAGNIOL-VILLARD plagnioj at jcrosoft.com
Wed Mar 25 22:43:44 CET 2009


> >> --- 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?
> 
> No, thumb code is less efficient in terms of performance, but this 
> single file needs thumb code. See LPC2292.
what is the Difference?

until a real big gap please do not use thumb
> 
> >> 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?
> 
> I'm sorry, it is combined in this case, no?
> 
> >>   #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?
> 
> Same here.
> 
> > 
> >>   	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?
> 
> Same here.
> 
> > 
> >>   	.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 at 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?
> 
> I don't need it, but start.S wants to include it. See my comment about 
> the #ifdef's. Other architectures left it empty too, so it seemed the 
> best option to me.
> 
> >> +/* 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? 
> 
> Yes I do. They are straight from the LPC2292 code, so once they were 
> considered OK. I checked out the the write{s,l,b} functions in asm/io.h, 
> but although they look similar, for some reason they simply don't work. 
> Given the similarities between the write{s,l,b} and the PUT* functions, 
> what is the problem with those? Furthermore, the ARM architecture 
> doesn't use any kind of special instructions for accessing registers, 
> everything is memory mapped.
please use readx/writex
> 
> 
> I do understand that you want the best code for U-boot, but I do not 
> entirely agree on all points. Certainly when I look at the code already 
> in place in U-boot.
I'm preparing a patch series to clean the arm720t to seprate it as arch
and avoid this borring #ifdef

Best Regards,
J.


More information about the U-Boot mailing list