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

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

[Matvejchikov Ilya]

Good day!

This patch adds support for MC68360 based board for u-boot-1.1.6. Mail me if
you have any questions :)

Best regards,
Matvejchikov Ilya.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.denx.de/pipermail/u-boot/attachments/20070408/3f74b315/attachment.htm 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: u-boot-1.1.6-mc68360-i0.patch.gz
Type: application/x-gzip
Size: 28186 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20070408/3f74b315/attachment.bin 

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

[Wolfgang Denk]

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

-- 
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
Include the success of others in your dreams for your own success.

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

[Matvejchikov Ilya]

2007/4/9, Wolfgang Denk <wd@denx.de>:
>
> 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.


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 

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

[Wolfgang Denk]

In message <8496f91a0704111258g7bd22c55m43d0e17799185b69@mail.gmail.com> you wrote:
>
> > 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.

Can't do it? You cannot give your own board a name? Why not?!?

> > least release 1.2.0).
> 
> Ooops, I was not aware of it. As far as I know, there was no official
> information about it.

Yes, there was.

> 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?

Start at http://www.denx.de/wiki/UBoot, and make sure to read at
lkeast the README.

> #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 cannot load a Linux kernel and a ramdisk on such a system, while
there are still ~5 MB of malloc space unused.

> 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?

Because lib_m68k/board.c is common code  which  you  share  with  all
other m68k boards. You must not mess with this.

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

Thanks.

> ------=_Part_4889_10802702.1176321492201
> Content-Type: text/html; charset=ISO-8859-1
> Content-Transfer-Encoding: 7bit
> Content-Disposition: inline

But please NEVER post HTML here again!

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
The idea of male and female are universal constants.
	-- Kirk, "Metamorphosis", stardate 3219.8

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

[patrick.mathes]

Was the MC68360 patch ever released?
-- 
View this message in context: http://www.nabble.com/-PATCH--FreeScale-MC68360-support-tf3543268.html#a10288952
Sent from the Uboot - Users mailing list archive at Nabble.com.