
Dear Ilya,
in message 8496f91a0704080420m1e72900cn70116c3dc7973b2b@mail.gmail.com you wrote:
This patch adds support for MC68360 based board for u-boot-1.1.6. Mail me if you have any questions :)
Please correct me if I'm wrong, but MC68360 is not the name of a board, but of the Freescale (resp. ex Motorola) QICC CPU. There are many different boards using this processor, so you will have to chose a more descriptive board name.
Also, U-Boot 1.1.6 is outdated. Please submit your patch against a recent version of U-Boot (i. e. top of tree in git repository, or at least release 1.2.0).
Then, there are some coding style issues with your patch (indentation not by TAB, indentation not by multiple of 8 columns, trailing white space, C++ comments, too long lines, etc.) as well as other formal issues (missing Copyright entries, missing license headers, missing Signed-off-by: line).
It seems you use a private flash driver for a device that looks as if it was CFI conformant. Please explain why you cannot use the CFI driver instead.
I also wonder if you really need a new ethernet driver (cpu/mc68360/enet.c) and a new serial driver (cpu/mc68360/serial.c) - IIRC the CPM on the 360 is mostly compatible (with very few minor deviations that can be handled easily) to the CPM on the PowerQICC I (= MPC8xx) processors. I think most of this could could (and should) be shared? The same holds for some other files like for example include/asm-m68k/arch-mc68360/commproc.h which is highly redundand with the MPC8xx commproc.h
Some of your files contain version ID stuff like here:
include/asm-m68k/arch-mc68360/mc68360_enet.h:
*********************************** * $Id: m68360_enet.h,v 1.1.1.1 2001/05/18 17:10:11 hamilton Exp $ ***********************************
Please remove this.
I see some "#if 0" blocks in your code; please remove these.
I see:
#define CONFIG_BOOTFILE vmlinuz
This looks broken to me. U-Boot does not boot a vmlinuz file, but U-Boot images created by mkimage.
#define CFG_BAUDRATE_TABLE { 19200 }
This is *very* restrictive. I recommend to support all commonly used baudrates instead (i. e. at least the range from 9600 through 115200).
#define CFG_HZ (unsigned long int)32768000
This is broken. CFG_HZ is required to be 1000 (i. e. millisecond ticks).
#define CFG_SDRAM_SIZE 0x00800000
Please don't do that. U-Boot style is to allow auto-adjustment to the actual RAM size.
#define CFG_MALLOC_LEN (5*1024*1024)
You have 8 MB of RAM and reserve 5 MB for malloc()? This seems broken to me.
You comment out relevant parts of lib_m68k/board.c - you must not do that!
lib_m68k/time.c - your "implementation" of udelay() is broken. Please replace by a working version.
Please clean up these issues and resubmit.
Thanks.
Best regards,
Wolfgang Denk