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

[Wolfgang Denk]

Dear Remco Poelstra,

In message <49C25F75.3080906 at duran-audio.com> you wrote:
> Wolfgang Denk schreef:
> >> +/* Vectored Interrupt Controller (VIC) */
> >> +#define VIC_BASE_ADDR	0xFFFFF000
> >> +#define VICIRQStatus   (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x000))
> >> +#define VICFIQStatus   (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x004))
> >> +#define VICRawIntr     (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x008))
> >> +#define VICIntSelect   (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x00C))
> >> +#define VICIntEnable   (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x010))
> >> +#define VICIntEnClr    (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x014))
> >> +#define VICSoftInt     (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x018))
> >> +#define VICSoftIntClr  (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x01C))
> >> +#define VICProtection  (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x020))
> >> +#define VICSWPrioMask  (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x024))
> > 
> > Please do not use offset lists, but define a proper C data  structure
> > instead.  And  never  ever  access  device  regiters  through  simple
> > volatile pointers. Use proper accessor functions. Here and elsewhere.
> 
> 
> All examples I checked use the same syntax for defining registers, so I left it
> in the code. If that is a problem, can you indicate an example which does The Right Thing (tm)?

See for example the structure declarations in include/asm-ppc/*immap*

> I think the patch now matches the other criteria. The patch is a bit bigger, since
> other code did not follow the Coding Styles either. I used indent to fix my code and it fixed the other code as well.

I hope you didn't dop this in a single step. Codin style cleanup and
adding new stuff must be done separately.

> The second patch will follow when this patch is OK.
> 
> Regards,
> 
> Remco Poelstra
> 
> ----
> --- u-boot-orig/cpu/arm720t/cpu.c	2009-03-18 00:42:12.000000000 +0100
> +++ u-boot/cpu/arm720t/cpu.c	2009-03-19 16:00:04.000000000 +0100
> @@ -41,7 +41,9 @@ int cpu_init (void)
>   	 * setup up stacks if necessary
>   	 */
>   #ifdef CONFIG_USE_IRQ
> -	IRQ_STACK_START = _armboot_start - CONFIG_SYS_MALLOC_LEN - CONFIG_SYS_GBL_DATA_SIZE - 4;
> +	IRQ_STACK_START =
> +	    _armboot_start - CONFIG_SYS_MALLOC_LEN - CONFIG_SYS_GBL_DATA_SIZE -
> +	    4;
>   	FIQ_STACK_START = IRQ_STACK_START - CONFIG_STACKSIZE_IRQ;
>   #endif
>   	return 0;
> @@ -63,17 +65,17 @@ int cleanup_before_linux (void)
>   	disable_interrupts ();
> 
>   	/* turn off I-cache */
> -	asm ("mrc p15, 0, %0, c1, c0, 0":"=r" (i));
> +      asm ("mrc p15, 0, %0, c1, c0, 0":"=r" (i));
>   	i &= ~0x1000;
> -	asm ("mcr p15, 0, %0, c1, c0, 0": :"r" (i));
> +      asm ("mcr p15, 0, %0, c1, c0, 0": :"r" (i));

What sort of patch is this? There is no commit message, no explanation
what it is, no SoB, ... ?

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
"Deliver yesterday, code today, think tomorrow."