[U-Boot-Users] take 3: [PATCH](ARM-specific) New ARM 1176 CPU and board for Energizer SoC

[Wolfgang Denk]

Dear Peter,

in message <000001c7704a$a44cb5d0$821ba8c0 at Emea.Arm.com> you wrote:
> 
> The following patch has been added to the u-boot-arm repository 
> as branch 20070326_ene.

For reviewing it would be easier (for me) if you  could  update  your
tree from the "master" so the code differences are better visible.

> CHANGELOG: * New ARM 1176 CPU and board for Energizer SoC
> A new board and cpu definition for Energizer SoC is added. 
> The Energizer SoC. is an ARM 1176 CPU core with NXP periphereals connected
> on the AXI bus. 
> 
> Signed-off-by: Jean-Paul Saman <jean-paul.saman at nxp.com>


This needs a lot of cleanup.

First, there are coding style violations (C++ comments in
include/asm-arm/arch-arm1176/sys_proto.h and
include/configs/energizer.h, indentation not by TAB in
board/energizer/lowlevel_init.S, trailing empty lines in
cpu/arm1176/config.mk and cpu/arm1176/interrupts.c, very long lines in
include/configs/energizer.h, and maybe more).

MAKEALL: the "LIST_ARM11" should be kept in sorted order.

Makefile: the "ARM1176 Systems" entry must not preceed the generic
"ARM' entry.

README: indentation like this:

	...
	  - arm920t     Files specific to ARM 920 CPUs
	    - at91rm9200 Files specific to Atmel AT91RM9200 CPU
	    - imx       Files specific to Freescale MC9328 i.MX CPUs
	    - s3c24x0   Files specific to Samsung S3C24X0 CPUs
	...

	used to represent the real directory structure, i. e. it
	showed that there is a directory cpu/arm920t/at91rm9200/;
	the changes drop this information; please revert.

include/asm-arm/mach-types.h: instead of updating just single machine
entries, the whole file should be updated.


All Makefiles are  broken  as  they  don't  support  building  in  an
external directory. This must be fixed.

board/energizer/energizer.c  has  delays  based  on  trivial,   empty
counted  loops.  This does not work - the compiler will optimize away
this code. Please fix.

cpu/arm1176/cpu.c implements cp_delay() based on a counted loop which
will be eliminated by the optimizer. Please fix.

include/asm-arm/arch-arm1176/energizer.h - is this board-specific
code? Why is this in a public directory then?

include/asm-arm/arch-arm1176/sys_info.h - this also seems very  board
specific   and   should  probably  be  moved  out  of  common  header
directories.

include/asm-arm/arch-arm1176/sys_proto.h - ditto.

The C code uses a lot of __name  functions  like  __raw_writel().  In
ANSI C, these are reserved identifiers. Is there any good reason here
to use these?

Most files need an update of the Copyright information.

The code still uses the SZ_xxxK macros which are  just  being  phased
out; please replace these.


NAK for the current state.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Einstein argued that there must be simplified explanations of nature,
because God is not capricious or arbitrary. No  such  faith  comforts
the software engineer.                             - Fred Brooks, Jr.