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

[Wolfgang Denk]

Dear Remco Poelstra,

In message <4C064504.3010508 at duran-audio.com> you wrote:
> 
> >> +	pfnct = (void (*)(void))(&(vic->vicaddr));
> >> +
> >> +	(*pfnct) ();
> >>      
> > Please unify with code for the LPC2292 and get rid of the #ifdef.
> >    
> 
> This is not possible. I do understand that there is a lot of similarity,
> but I was asked to use C structures to access registers, but the lpc22xx
> code uses direct access. I cannot convert the lpc22xx code, since I don't
> have access to a board and it's a very error-prone process. I think it would
> be better if the current lpc22xx maintainer converts the lpc22xx code to 
> use
> C structures as well. Unfortunatly, the same holds for most of the code 
> merge/factor out
> comments below.

As you can see from MAINTAINERS file, the LPC2292 is an orphaned
board; please unify the code as I asked you, even if you can only
compile-test the LPC2292 configuration.

> > Ditto here.
> 
> This one is different. The lpc2468 uses an upward counting timer,
> while the other ARM's seem to use a downward counting timer (as
> far as I could judge from the code).

What about the LPC2292?

> >> +++ 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.
> >    
> Can you give a hint on how I should accomplish that? Copy the
> iap_netry.S to the parent directiry?

If the code is generic to all arm720t systems, then moving it to the
parent dir world be ok; otherwise it might make sense to put the
common code into arch/arm/cpu/arm720t/lpc2xxx or so.

> >> +#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?
> >    
> 
> Yes, start.S needs this file, but since my code uses C structures, it's 
> empty.

Maybe start.S can be fixed so it does not need this file?

> > Do we _really_ need all this?
> 
> Yes and no. Not all H/W is used. That may well change in
> the future as more peripherals are supported. I do also believe

As long as nobody uses this, the code makes no sense, especially as
it's only place holders. Drop it.


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
Witch!  Witch!  They'll burn ya!
	-- Hag, "Tomorrow is Yesterday", stardate unknown