[U-Boot-Users] [PATCH] FreeScale MC68360 support

[Matvejchikov Ilya]

2007/4/9, Wolfgang Denk <wd at denx.de>:
>
> Dear Ilya,
>
> in message <8496f91a0704080420m1e72900cn70116c3dc7973b2b at 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.


Yes, you are right. MC68360 is the name of the CPU. But the board I'm
working
at right now is my own design. That's why I can't do it.

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).


Ooops, I was not aware of it. As far as I know, there was no official
information about it.

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).


Ok. Could you tell me where I can get exact information about it?

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.


This is because I have old non-CFI flash chips :(

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


I don't know this processors family well enough, but I see what I can do
with it....

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.


ok

I see some "#if 0" blocks in your code; please remove these.


ok

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.


Yes, this name is incorrect... Let it be uImage.

#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).


ok

#define CFG_HZ                         (unsigned long int)32768000
>
> This is broken. CFG_HZ is required to be 1000 (i. e. millisecond
> ticks).


okkk..

#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.


Why not? I really have it working :)

You comment out relevant parts of lib_m68k/board.c - you must not do
> that!


Could you tell me the reason why I should not do it?

lib_m68k/time.c - your "implementation" of udelay() is broken. Please
> replace by a working version.


I'll try.

Please clean up these issues and resubmit.
>
>
I'm grateful to you for your help and for what you are doing :)

I will take into consideration your advice and recommendations. And I'll try
to correct my patch.

Best regards,
Matvejchikov Ilya.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.denx.de/pipermail/u-boot/attachments/20070411/2ac7b8c7/attachment.htm