Generic Support for Motorola i.MX architecture

Well, six minutes ;)

I've temporarily put the patch here:

http://www.pengutronix.de/software/u-boot/u-boot-imx1.diff

Sidenote: it is difficult to audit this patch for trailing whitespace as the patch context contains that much of it that you don't see the needle in the haystack =8-)

Changelog entry:

* Patch by Sascha Hauer, 27 Jun 2004:
Split mx1ads stuff into generic imx and platform parts Add support for Synertronixx scb9328, Viasys FlowScreen 2

Wolfgang, do you prefer the code being split into separate pieces for the single tasks?

Robert

 Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hornemannstra?e 12,  31137 Hildesheim, Germany
    Phone: +49-5121-28619-0 |  Fax: +49-5121-28619-4

No, this is actually trivial. Use the toolbox:

	bash$ egrep '^\+.*[^I ]$' u-boot-imx1.diff  | wc -l
	     21

There are 21 cases where you insert trailing whitespace.

There are 39 cases where you use space instead of TAB for indentation.

There are 9 cases where you use C++ comments in non-C++ files.

There are 5 cases where you use DOS '\r\n' line feeds

There is one case where you add more than 2 empty lines to a source file

There are 4 cases where you add trailing empty lines to source files

Please cleanup and resubmit.

It seems the patch does more than just this. For example, I see that it modifies the code for interrupt handling for all ARM9 systems?

Please make sure the description is complete.

If the patch is clean this is not necessary.

Best regards,

Wolfgang Denk

Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
Microsoft Compatibility:
     your old Windows 3.11 application crash exactly as the new ones.

Hi Wolfgang,

I removed the trailing white spaces, dos line endings, etc from the patch. Ming-Len Wu sent me a patch for mx1ads boards, which I included into the patch. I can't test mx1ads support, but he says it works.

You can download the patch here:

http://www.pengutronix.de/software/u-boot/u-boot-imx1-20040628-1.diff

Best regards,

Sascha Hauer

Dear Sasha,

Added, thanks.

I understand that this patch obsoletes all your previous patches, i. e. the following messages:

06/17 Sascha Hauer       [U-Boot-Users] [PATCH] Motorola i.MX support (1/2)<<--HlL+5n6rz5pIUxb
06/17 Sascha Hauer       [U-Boot-Users] [PATCH] Motorola i.MX support (2/2)<<--cmJC7u66zC7hs+8
06/18 Sascha Hauer       Re: [U-Boot-Users] [PATCH] Motorola i.MX support (1/2)<<I've just had
06/19 Robert Schwebel    Re: [U-Boot-Users] [PATCH] Motorola i.MX support (1/2)<<Hi, On Fri, J
06/19 Sascha Hauer       Re: [U-Boot-Users] [PATCH] Motorola i.MX support (1/2)<<On Sat, Jun 1
06/21 Robert Schwebel    Re: [U-Boot-Users] [PATCH] Motorola i.MX support (1/2)<<--SO98HVl1bnM
06/21 Steven Scholz      Re: [U-Boot-Users] [PATCH] Motorola i.MX support (1/2)<<Robert Schweb
06/21 Robert Schwebel    Re: [U-Boot-Users] [PATCH] Motorola i.MX support (1/2)<<On Mon, Jun 2

Please confirm.

Technical problems:

* In include/configs/mx1ads.h etc. you have this:

  #define        CFG_HZ                  3686400

This is BROKEN. CFG_HZ is the number of timer ticks per second. You don't want to process more than 3 millions of interrupts per sec. Please don't mis-use CFG_HZ for things it was not intended for.

Formal problems:

* Entries for MAKEALL and MAINTAINERS missing. Please submit new patch!

* Would you please keep lists sorted, i. e. don't mix ARM boards inbetween PowerPC systems, and sort the names in ascending order?

* Please use plain '#' comments in Makefiles etc. Your

	#/*
	#*
	...
	#*
	#*/

is ugly.

* Please do not add trailing white space (board/mx1fs2/memsetup.S, include/configs/mx1ads.h)

* Please do not add trailing empty lines.

* Please don't modify a file if all you change is deleting an empty line.

* Don't use 3 or more consecutive empty lines.

* Please don't include opject files multiple times (cpu.o in cpu/arm920t/Makefile)

Best regards,

Wolfgang Denk

Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: [email protected]
He who hesitates is not only lost, but miles from the next exit.

Now that this patch finally got into CVS I raise the following question again:

Does it make sense to put processor specific peripheral code into cpu/arm920t directory (like s3c24x0_serial.c or imx_interrupts.c or usb code)!?

Although the AT91RM9200 is based on a ARM9 it has it's own directory.

I understand that copying the same code again and again won't make sense.

A while ago I suggested to create cpu/imx, cpu/s3c24x0 etc. and put all the cpu specific stuff in there.

To avoid copying the arm9 generic code one could do:

1.) cpu/at91rm9200/Makefile:

     OBJS    = ../arm920t/interrupts.o ../arm920t/cpu.o \
               serial.o at91rm9200_ether.o at45.o

     start.S has to be a link "start.S -> ../arm920t/start.S" since

     START= ../arm920t/start.o

     would not work due to dependencies in the main makefile.

2.) Or creating (by Makefile) links to the generic sources:

     LINKS = start.S interrupts.c cpu.c

     $(LINKS)
         ln -s ../arm920t/$@ $@    (oder s.?.)

Comments?

Steven Scholz
imc Measurement & Control

Yes, it makes sense, in so far as the cpu/<processor> directory is explicitely intended to hold all code that is specific to the <processor> CPU.

It might make sense to add an additionallevel of directories, i. e. turn s3c24x0_* or imx_* into s3c24x0/* and imx/* resp.

Which is IMHO a bad thing.

Indeed.

Agreed. Please submit a patch.

ARM9 generic code should stay in cpu/arm920t/

No.

No.

Both methods don't look really attractive to me. If ther eis common code, it shall remain in the common directory.

Best regards,

Wolfgang Denk

Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
Any sufficiently advanced technology is indistinguishable from magic.
Clarke's Third Law       - _Profiles of the Future_ (1962; rev. 1973)
                  ``Hazards of Prophecy: The Failure of Imagination''

But how else could we solve this?
If we leave the common arm920t code in cpu/arm920t and put the specific stuff in - let's say - cpu/s3c24x0...

The main makefile only takes one cpu type. How can we achieve that the code in cpu/arm920t _and_ in cpu/s3c24x0 is built?

Steven Scholz

As I wrote:

| It might make sense to add an additional level of directories, i. e. | turn s3c24x0_* or imx_* into s3c24x0/* and imx/* resp.

No. Not cpu/s3c24x0/ - cpu/arm920t/s3c24x0/

By just having a cpu/arm920t/ directory with subdirectories?

Best regards,

Wolfgang Denk

Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-4596-87  Fax: (+49)-8142-4596-88  Email: wd at denx.de
A failure will not appear until a unit has passed final inspection.

Ah! Now I see! Thanks!

Steven Scholz

Would you agree that we should put peripheral drivers like

	s3c24x0_serial.c
	usb_ohci.c (s3c24x0)
	at91rm9200_ether.c
	at91rm9200_serial.c

into the drivers/ directory. But stuff like

	imx_interrupts.c
	imx_speed.c
	s3c24x0_interrupts.c
	s3c24x0_speed.c
	at91rm9200/interrupts.c

belongs into the (new) processor specific subdirs under cpu/

Steven Scholz