Cleanup cpu/arm920t

Hi there,

Since the cpu/ directory gets clobbered with peripheral driver code I started cleaning up cpu/arm920t.

Idea is that all SoC that are based on ARM9(20T) should share cpu/arm920t.

* Patch by Steven Scholz, 06 Aug 2004:
- Cleanup cpu/arm920t directory by moving peripheral driver code out of cpu/arm920t/ into drivers/ and creating subdirectories cpu/arm920t/imx and cpu/arm920t/s3c24x0 for processor specific code
- rename cpu/arm920t/usb_ohci.c to drivers/s3c24x0_usb_ohci.c
- remove cpu/arm920t/usb_ohci.h (duplicate code)
- rename cpu/mpc5xxx/usb_ohci.h to drivers/usb_ohci.h so everybody can use it (maybe we can merge cpu/arm920t/usb_ohci.c and cpu/mpc5xxx/usb_ohci.c someday)

Since the patch is quite big I'll send it to Wolfgang directly. If it gets applied I'd like to do the same with cpu/at91rm9200. That is moving code for serial and ethernet into drivers/ and the rest into arm920t/ and arm920t/at91rm9200.

The patch does not break the build process for ARM9. I tested "./MAKEALL ARM9"

Thanks for comments.

Steven Scholz

Dear Steven,

Thanks for the effort, but I disagree with some parts of the patch.

OK for the general idea, but...

I disagree with this. drivers/ is for device drivers that are _common_ among architectures. It makes absolutely no sense to mode processor specific code like the s3c24x0 USB drivers to this directory.

Please move this into cpu/arm920t/s3c24x0/ instead.

Duplicate to what?

Be careful. Do you really think this is a good idea? The 52xx USB code suffers from the endinaess problem of the 52xx. I'm not sure if this is a good base for common code.

Please fix above issues and resubmit.

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
No question is too silly to ask. Of course, some  questions  are  too
silly to to answer...  - L. Wall & R. L. Schwartz, _Programming Perl_

Dear Wolfgang,

Accepted. I found some processor specific driver code in drivers/ (like s3c24x0_i2c.c s3c4510b_eth.c s3c4510b_eth.h s3c4510b_uart.c s3c4510b_uart.h) so I thought it becomes "common practice" to put peripheral driver code for uart or ethernet (although it might be processor specific) into drivers/.

Duplicate to cpu/mpc5xxx/usb_ohci.h.

The only difference I found is

+#if defined(CONFIG_MPC5200)

+       __u16   pad1;                   /* set to 0 on each frame_no change */
+       __u16   frame_no;               /* current frame number */
+#else
         __u16   frame_no;               /* current frame number */
         __u16   pad1;                   /* set to 0 on each frame_no change */

+#endif

Thanks,

Steven Scholz

Hi guys,

as I've just started to merge the code for Cirrus Logic EDB93xx, I'll follow these new rules. So I'll introduce the directory cpu/arm920t/ep93xx.

llandre

DAVE Electronics System House - R&D Department

web:   http://www.dave-tech.it
email: r&d2 at dave-tech.it

Thanks.

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
Cigarette, n.: A fire at one end, a fool at the other, and a  bit  of
tobacco in between.

Yes - this has sneaked in because there was no better solution. Given the planned restructuring this belongs to cpu/XXX/ as well.

No, I don't want to have this. drivers/ is for common (processor independent) conde.

Agreed - so please use the MPC5200 version as base, not the other one.

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
Hiring experienced unix people is  like  a  built-in  filter  against
idiots. Hiring experienced NT people provides no such guarantee.
            -- Miguel Cruz in WgL96.349$CC.122704 at typhoon2.ba-dsg.net

1) What about the Makefile in cpu/arm920t? Is it sufficient to change OBJS like this?

OBJS    = cpu.o \
           imx_generic.o imx_interrupts.o imx_speed.o \
           interrupts.o \
           s3c24x0_interrupts.o s3c24x0_serial.o s3c24x0_speed.o \
           usb_ohci.o \
           ep93xx/ep93xx_interrupts.o \
           ep93xx/ep93xx_net.o \
           ep93xx/ep93xx_serial.o \
           ep93xx/ep93xx_speed.o

2) Should the new processor-specific files keep the header (ep93xx_ in the previous example) in the name?
As a new directory has been introduced, it is clearly redundant.

Regards,

llandre

DAVE Electronics System House - R&D Department

web:   http://www.dave-tech.it
email: r&d2 at dave-tech.it

No. Imagine what the Makefile would look like if we had 50 different systems with 5...10 files and drivers each.

No. There is no need for that, as the subdirectory name already presents this information.

Indeed.

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
You don't have to stay up nights to succeed; you have to  stay  awake
days.

Ehm.
So you want a seperate Makefile in each subdir?
Any hints how this could be done?

Thanks!

--
Steven

Yes, of course. What else?

Ummm... what? Adding a Makefile per subdirectory? Sounds trivial to me... We do this all the time in all U-Boot directories. Or what exactly is your problem?

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
I had the rare misfortune of being one of the first people to try and
implement a PL/1 compiler.                             -- T. Cheatham

I thought of something like cpu/arm920t/imx/libimx.a and cpu/arm920t/s3c24x0/s3c24x0.a. But I don't want to add them the the main Makefile.
Now I am wondering how I can put several sublibs together to one cpu/$(CPU)/lib$(CPU).a ...

Maybe just linking the objects in cpu/arm920t/<SoC>/ to one big object file...

But I am to stupid for writing Makefiles... :-(

Steven Scholz

OK.

NO. Not the toplevel makefile. But maybe cpu/*/Makefile ?

By running "ar" ?

No, please don't.

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
Is a computer language with goto's totally Wirth-less?

My first attempt to do something like

$(LIB): $(OBJS)

         $(AR) crv $@ $(OBJS) imx/libimx.a

resulted in some error

UNDEF_SYM=`arm_920TDI-objdump -x lib_generic/libgeneric.a board/mx1ads/libmx1ads.a cpu/arm920t/libarm920t.a lib_arm/libarm.a fs/cramfs/libcramfs.a fs/fat/libfat.a fs/fdos/libfdos.a fs/jffs2/libjffs2.a fs/reiserfs/libreiserfs.a net/libnet.a disk/libdisk.a rtc/librtc.a dtt/libdtt.a drivers/libdrivers.a drivers/sk98lin/libsk98lin.a post/libpost.a post/cpu/libcpu.a common/libcommon.a |sed -n -e

's/.*\(__u_boot_cmd_.*\)/-u\1/p'|sort|uniq`;\
         arm_920TDI-ld -Bstatic -T 

/home/scholz/Projekte/HiResDisplay/u-boot.work/board/mx1ads/u-boot.lds -Ttext 0x08400000 $UNDEF_SYM cpu/arm920t/start.o \
--start-group lib_generic/libgeneric.a
board/mx1ads/libmx1ads.a cpu/arm920t/libarm920t.a lib_arm/libarm.a fs/cramfs/libcramfs.a fs/fat/libfat.a fs/fdos/libfdos.a fs/jffs2/libjffs2.a fs/reiserfs/libreiserfs.a net/libnet.a disk/libdisk.a rtc/librtc.a dtt/libdtt.a drivers/libdrivers.a drivers/sk98lin/libsk98lin.a post/libpost.a post/cpu/libcpu.a common/libcommon.a --no-warn-mismatch -L /opt/eldk/usr/lib/gcc-lib/arm-linux/2.95.4 -lgcc --end-group \
-Map u-boot.map -o u-boot
arm_920TDI-objdump: libimx.a: File format not recognized

Although coming to think about it

arm_920TDI-ar crv libimx.a imx_generic.o imx_interrupts.o imx_serial.o imx_speed.o
a - imx_generic.o
a - imx_interrupts.o
a - imx_serial.o
a - imx_speed.o
make[2]: Leaving directory `/home/scholz/Projekte/HiResDisplay/u-boot.work/cpu/arm920t/imx' arm_920TDI-ar crv libarm920t.a cpu.o interrupts.o imx/libimx.a a - cpu.o
a - interrupts.o
a - imx/libimx.a
make[1]: Leaving directory `/home/scholz/Projekte/HiResDisplay/u-boot.work/cpu/arm920t'

looks quit good, doesn't it? I thought

arm_920TDI-ar crv libarm920t.a cpu.o interrupts.o imx/libimx.a

would not work, but maybe the error is somewhere else...

--

Steven

Hi llandre,

Did you have any success with your Cirrus Logic EDB93xx and cpu/arm920t/ep93xx???

My first attempt to do something like

$(LIB): $(OBJS)

	$(AR) crv $@ $(OBJS) imx/libimx.a

in cpu/arm920t/Makefile resulted in some error

My cpu/arm920t/imx/Makefile looks like

#########################################################################
include $(TOPDIR)/config.mk

LIB = libimx.a

OBJS = imx_generic.o imx_interrupts.o \
imx_serial.o imx_speed.o

all: .depend $(LIB)

$(LIB): $(OBJS)

         $(AR) crv $@ $(OBJS)

.depend: Makefile $(OBJS:.o=.c)

                 $(CC) -M $(CFLAGS) $(OBJS:.o=.c) > $@

sinclude .depend
#########################################################################

which is pretty much like cpu/arm920t/Makefile...

But I suspect that creating an archive with another archive like

	$(AR) crv $@ $(OBJS) imx/libimx.a

goes wrong...

"arm_920TDI-nm imx/libimx.a" works fine. But "arm_920TDI-nm libarm920t.a" brings
...
00000188 T do_undefined_instruction
00000000 T enable_interrupts
U memcpy
U panic
U printf
U reset_cpu
00000048 T show_regs
arm_920TDI-nm: libimx.a: File format not recognized

--
Steven

No :(
I was just writing you an email when I received yours.
I tried to change both cpu/arm920t/Makefile and cpu/arm920t/ep93xx/Makefile but I was not able even to build the sources ...

After getting your email I changed the two Makefiles as follows:

##################################################################
# cpu/arm920t/Makefile

include $(TOPDIR)/config.mk

LIB = lib$(CPU).a

# Processor-specific subdirs
SUBDIRS = ep93xx
.PHONY : $(SUBDIRS)

START = start.o

OBJS    = cpu.o \
           imx_generic.o imx_interrupts.o imx_speed.o \
           interrupts.o \
           s3c24x0_interrupts.o s3c24x0_serial.o s3c24x0_speed.o \
           usb_ohci.o

all: .depend $(START) $(LIB) $(SUBDIRS)

$(LIB): $(OBJS)

         $(AR) crv $@ $(OBJS)

$(SUBDIRS):

                 $(MAKE) -C $@ all

#########################################################################

.depend: Makefile $(START:.o=.S) $(OBJS:.o=.c)

                 $(CC) -M $(CFLAGS) $(START:.o=.S) $(OBJS:.o=.c) > $@
                 @for dir in $(SUBDIRS) ; do $(MAKE) -C $$dir .depend ; 

done

sinclude .depend

#########################################################################

##################################################################
# cpu/arm920t/ep93xx/Makefile

include $(TOPDIR)/config.mk

LIB = ../lib$(CPU).a

OBJS = eth.o ep93xx_interrupts.o \
serial.o speed.o

all: .depend $(LIB)

$(LIB): $(OBJS)

         $(AR) crv $@ $(OBJS)

.depend: Makefile $(OBJS:.o=.c)

         $(CC) -M $(CFLAGS) $(OBJS:.o=.c) > $@

sinclude .depend

The build has been completed successfully.
According to you is this solution ok? Wolfang?

llandre

DAVE Electronics System House - R&D Department

web:   http://www.dave-tech.it
email: r&d2 at dave-tech.it

Looks OK to me.

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
If you're not part of the solution, you're part of the problem.

But this means "ar crv libarm920t.a" is called several times.

We have to be careful then since the option "crv" would overwrite existing members of the archive. Right?
I think that's why llandre used ep93xx_interrupts.o instead of interrupts.o. So maybe the SoC name in the files is not redundand...

On the other hand this could be used to explicitly _overwrite_ the generic stuff in (let's say) cpu/arm920t/interrupt.o by (let's say) cpu/arm920t/imx/interrupt.o. But I am afraid code is duplicated by people again...

--
Steven

Or maybe use "ar cqv lib$(CPU).a" in the subdirs... ?

--
Steven

Yes, do this if you want to spend many hours hunting for problems caused by bad (stale) archive members. [Think of incremental builds.]

No, you really DON'T want to do this.

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
Why don't you have a Linux partition installed so you can be  working
in  a  programmer-friendly environment instead of a keep-gates'-bank-
account-happy one? :-)                            -- Tom Christiansen

Right.

It is redundand, just packing all the files into a single archive file is not possible.

I don't think this is a good idea.

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
Q: Why do PCs have a reset button on the front?
A: Because they are expected to run Microsoft operating systems.

I think so.

Yes, you are right.
In fact the processor-specific files should have different names rispect to the ones in
cpu/arm920t.

llandre

DAVE Electronics System House - R&D Department

web:   http://www.dave-tech.it
email: r&d2 at dave-tech.it

Why?

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
The software required `Windows 95 or better', so I installed Linux.

I use as an example the two files cpu/arm920t/interrupts.c and cpu/arm920t/ep93xx/interrupts.c.
In this case make produces the following error (both with "ar cqv" and "ar crv"):

make[1]: Leaving directory `/home/llandre/devel/zephyr/sw/u-boot/u-boot/common'
UNDEF_SYM=`arm-linux-objdump -x lib_generic/libgeneric.a
board/edb93xx/libedb93xx.a cpu/arm920t/libarm920t.a lib_arm/libarm.a
fs/cramfs/libcramfs.a fs/fat/libfat.a fs/fdos/libfdos.a fs/jffs2/libjffs2.a
fs/reiserfs/libreiserfs.a net/libnet.a disk/libdisk.a rtc/librtc.a
dtt/libdtt.a drivers/libdrivers.a drivers/sk98lin/libsk98lin.a
post/libpost.a post/cpu/libcpu.a common/libcommon.a |sed -n -e
's/.*\(__u_boot_cmd_.*\)/-u\1/p'|sort|uniq`;\
arm-linux-ld -Bstatic -T
/home/llandre/devel/zephyr/sw/u-boot/u-boot/board/edb93xx/u-boot.lds -Ttext
0xfffe0000 $UNDEF_SYM cpu/arm920t/start.o \
--start-group lib_generic/libgeneric.a
board/edb93xx/libedb93xx.a cpu/arm920t/libarm920t.a lib_arm/libarm.a
fs/cramfs/libcramfs.a fs/fat/libfat.a fs/fdos/libfdos.a fs/jffs2/libjffs2.a
fs/reiserfs/libreiserfs.a net/libnet.a disk/libdisk.a rtc/librtc.a
dtt/libdtt.a drivers/libdrivers.a drivers/sk98lin/libsk98lin.a
post/libpost.a post/cpu/libcpu.a common/libcommon.a --no-warn-mismatch -L /usr/local/arm/3.3.2/lib/gcc-lib/arm-linux/3.3.2 -lgcc --end-group \
-Map u-boot.map -o u-boot
cpu/arm920t/start.o(.text+0x154):/home/llandre/devel/zephyr/sw/u-boot/u-boot/cpu/arm920t/start.S:376:
undefined reference to `do_undefined_instruction' cpu/arm920t/start.o(.text+0x1b4):/home/llandre/devel/zephyr/sw/u-boot/u-boot/cpu/arm920t/start.S:382:
undefined reference to `do_software_interrupt' cpu/arm920t/start.o(.text+0x214):/home/llandre/devel/zephyr/sw/u-boot/u-boot/cpu/arm920t/start.S:388:
undefined reference to `do_prefetch_abort' cpu/arm920t/start.o(.text+0x274):/home/llandre/devel/zephyr/sw/u-boot/u-boot/cpu/arm920t/start.S:394:
undefined reference to `do_data_abort' cpu/arm920t/start.o(.text+0x2d4):/home/llandre/devel/zephyr/sw/u-boot/u-boot/cpu/arm920t/start.S:400:
undefined reference to `do_not_used' cpu/arm920t/start.o(.text+0x334):/home/llandre/devel/zephyr/sw/u-boot/u-boot/cpu/arm920t/start.S:425:
undefined reference to `do_irq' cpu/arm920t/start.o(.text+0x394):/home/llandre/devel/zephyr/sw/u-boot/u-boot/cpu/arm920t/start.S:431:
undefined reference to `do_fiq'
lib_arm/libarm.a(board.o)(.text+0x2d4): In function `start_armboot':
/home/llandre/devel/zephyr/sw/u-boot/u-boot/lib_arm/board.c:318: undefined
reference to `enable_interrupts'
common/libcommon.a(cmd_bootm.o)(.text+0x264): In function `do_bootm':
/home/llandre/devel/zephyr/sw/u-boot/u-boot/common/cmd_bootm.c:288:
undefined reference to `disable_interrupts'common/libcommon.a(cmd_bootm.o)(.text+0x32c):/home/llandre/devel/zephyr/sw/u-boot/u-boot/common/cmd_bootm.c:356:
undefined reference to `enable_interrupts' common/libcommon.a(cmd_bootm.o)(.text+0x370):/home/llandre/devel/zephyr/sw/u-boot/u-boot/common/cmd_bootm.c:367:
undefined reference to `enable_interrupts' common/libcommon.a(cmd_bootm.o)(.text+0x3f4):/home/llandre/devel/zephyr/sw/u-boot/u-boot/common/cmd_bootm.c:387:
undefined reference to `enable_interrupts'
board/edb93xx/libedb93xx.a(flash.o)(.text+0x2f0): In function `flash_erase':
/home/llandre/devel/zephyr/sw/u-boot/u-boot/board/edb93xx/flash.c:174:
undefined reference to `disable_interrupts' board/edb93xx/libedb93xx.a(flash.o)(.text+0x3e0):/home/llandre/devel/zephyr/sw/u-boot/u-boot/board/edb93xx/flash.c:219:
undefined reference to `enable_interrupts'
board/edb93xx/libedb93xx.a(flash.o)(.text+0x440): In function `write_word':
/home/llandre/devel/zephyr/sw/u-boot/u-boot/board/edb93xx/flash.c:251:
undefined reference to `disable_interrupts' board/edb93xx/libedb93xx.a(flash.o)(.text+0x52c):/home/llandre/devel/zephyr/sw/u-boot/u-boot/board/edb93xx/flash.c:302:
undefined reference to `enable_interrupts'
cpu/arm920t/libarm920t.a(cpu.o)(.text+0x54): In function
`cleanup_before_linux':
/home/llandre/devel/zephyr/sw/u-boot/u-boot/cpu/arm920t/cpu.c:112:
undefined reference to `disable_interrupts'
cpu/arm920t/libarm920t.a(cpu.o)(.text+0x7c): In function `do_reset':
/home/llandre/devel/zephyr/sw/u-boot/u-boot/cpu/arm920t/cpu.c:129:
undefined reference to `disable_interrupts'
make: *** [u-boot] Error 1

llandre

DAVE Electronics System House - R&D Department

web:   http://www.dave-tech.it
email: r&d2 at dave-tech.it

Don't try to put different object files with the same file name into the same archive file, then. Use per-directory archives, for example.

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
If a man had a child who'd gone  anti-social,  killed  perhaps,  he'd
still tend to protect that child.
	-- McCoy, "The Ultimate Computer", stardate 4731.3

Ok. arm/cpu/arm920t/ep93xx/Makefile looks like this:

##############################################################
include $(TOPDIR)/config.mk

LIB = libep93xx.a

OBJS = eth.o interrupts.o \
serial.o speed.o

all: .depend $(LIB)

$(LIB): $(OBJS)

         $(AR) crv $@ $(OBJS)

.depend: Makefile $(OBJS:.o=.c)

         $(CC) -M $(CFLAGS) $(OBJS:.o=.c) > $@

sinclude .depend
##############################################################

and I changed main Makefile like this:

##############################################################
...
ifeq ($(CPU),mpc85xx)
OBJS += cpu/$(CPU)/resetvec.o
endif

LIBS = lib_generic/libgeneric.a
LIBS += board/$(BOARDDIR)/lib$(BOARD).a
LIBS += cpu/$(CPU)/lib$(CPU).a
ifeq ($(CPU),arm920t)
LIBS += cpu/$(CPU)/ep93xx/libep93xx.a
endif
LIBS += lib_$(ARCH)/lib$(ARCH).a

LIBS += fs/cramfs/libcramfs.a fs/fat/libfat.a fs/fdos/libfdos.a fs/jffs2/libjffs2.a \
fs/reiserfs/libreiserfs.a
...
##############################################################

llandre

DAVE Electronics System House - R&D Department

web:   http://www.dave-tech.it
email: r&d2 at dave-tech.it

Hmm.
But it seems that I can't add one archive to another archive as I describe in an earlier mail, i.e.

ar crv limbimx.a foo.o bar.o
ar crv libarm920t.a foo1.o bar1.o imx/libimx.a

And I don't want to add an entry for every posible SoC to the main makefile... ? I have to admit that I don't know much about "ar". Maybe you could share some of your wisdom... ;-)

Steven Scholz

No, this makes no sense to me. Why would you want to do that?

Then maybe add a Make variable (probably by some config.mk file) so that we always can poull in the correct library.

man ar? info ar?

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
Calm down, it's *__only* ones and zeroes.

Cause I thought it would be nice to have the arm920t generic stuff _and_ the arm920t based SoC stuff in _one_ archive.

? :-(

--
Steven

Hmm ... do you mean to define in cpu/arm920t/Makefile the variable SUBDIRS and use
the same technique used by the main Makefile?

llandre

DAVE Electronics System House - R&D Department

web:   http://www.dave-tech.it
email: r&d2 at dave-tech.it

Something like that, yes.

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
By the way, ALL software projects are done by iterative  prototyping.
Some companies call their prototypes "releases", that's all.