[U-Boot-Users] take 3: [PATCH](ARM-specific) New ARM 1176 CPU and board for Energizer SoC
wd at denx.de
Tue Mar 27 11:22:17 CEST 2007
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/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
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
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.
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.
More information about the U-Boot