[U-Boot-Users] Re: [uClinux-dev] [Announcement] U-Boot for Motorola M5272C3 and M5282EVB
Bernhard Kuhn
bkuhn at metrowerks.com
Mon Dec 8 22:34:30 CET 2003
Hi Wolfgang!
Thanks for your time on reviewing the u-boot patch for
Coldfire - i can imagine it was quite a lot of work ...
I have to admit that the coldfire u-boot port was a
quick-hack intended as prototype that at least was
usefull for me :-)
Unforntunatly i'm not involved in any Coldfire related
development at the moment and pretty busy with other
projects, so i won't have the time right now to sort
out the issue you have discovered.
But i will at least try to provide some usefull
comments for each of the items listed below, so that
other Coldfire/u-boot users can start cleaning things
up, so that the patch will be free of any licencing
or technical issues. In this context, please feel
free to redistribute this mail to whomever you think
it might be usefull for.
One general note: the whole lot of "PPCisms" are deriving
from the fact that i copied and renamed several PPC
files from the u-boot tree and modified them until they
did the job.
Although certain files are even holding PPC assembly
instructions, or other obviously PPC related routines,
the bootloader will still compile and work (IMHO
pretty well) for the coldfire because the appropriate
code will not be compiled (ifdef'ed out) or is just
dead code in the resulting binary.
Wolfgang Denk wrote:
>
> OK, we will leave the "pre-bootloader" out of the U-Boot tree. Please
> set up a FTP server or such where Coldfire users can get it from.
I guess the best place for doing so would be www.uclinux.org
(where i still have an account - not accessible at the moment,
but that's different story)
> Please provide a doc/README.Coldfire which explains this procedure.
I will add appropriate notes to the existing coldfire specfic
installation instruction when the cleanups have been finished.
> I am afraid your patch is far from being usable:
That's a question of definition! :-)
To be serious: as the file name of the patch indicates, it
was intended to be applied for u-boot-0.4.0. I didn't have
done anything on it since then ...
> * "examples/syscall.S" does not exist any more; I omitted this part
> of the patch
Fine for me! i actually never tried to start anything als than linux :-)
> * "include/asm-m68k/8xx_immap.h" does not make any sense to me - why
> do you copy a MPC8xx specific file to the "include/asm-m68k/"
> header tree? Omitted.
The situation is that the FEC is almost the same one as
used by MPC8xx, so the related structures and definitions
are pretty usefull in 8xx_immap.h. We could rename the
file to something like mcf5xxx_fec.h and throw out any
non-FEC related stuff (iirc, non of the other structures
and definitions are used). I guess the "most clean"
way would be to put the FEC driver into the "drivers"
subdirectory since the FEC actually became an archtecture
independend device. But doing so would probably imply
a lot of work, because it would probably break all MPC8xx[x]
implementations for quite some time.
> * "include/asm-m68k/arch-coldfire/dummy.h" is effectively an empty
> file which is nowhere referenced. Omitted.
IIRC, the situation is that the Makefile (as taken from the
PPC tree) likes to create a link to a platform dependend
sub-directory. Unfortnualty, an empty sub-directoy does
not survive in a patch file and such won't be created when
applying the patch - then the Makefile will stop operation
because of a missing directory. I could have modified the
Makefile to not take care about different platforms, but
i thought that in near or far future, somebody may like to
port u-boot to other M68k(-like) architectures such as
Amiga, Mac or Cris, and it may be necessary to provide
architecture specific files. So at least the "structural"
requirements are met this way ...
> * "include/asm-m68k/bitops.h" reads: "Bit string operations on the
> ppc" -- on the PPC??? Actually the file does contain PowerPC
> assembler instructions which will never work on M68K. Omitted.
Actually it does work, because the __INLINE definitions (containing
the ppc-assembly code) are not used: the "pure C" inlined prototypes
at the end of the file are used, instead. But true: this stuff
should be deleted to avoid confusions like this one.
> * "include/asm-m68k/byteorder.h" contains more PowerPC assembler
> instructions which will never work on M68K. Omitted.
Same thing: most of the definitions are not activly used in
the coldfire port - needs cleanup to avoid confusion.
> * "include/asm-m68k/cache.h" is another verbatim copy of a PowerPC
> file which makes no sense for M68K. Omitted.
dito
> * "include/asm-m68k/io.h" is effectively an empty file - please
> verify that this is OK!
That's more or less ok: lot of the *.h files exist because
they are included by the ppc-derived *.c files and i wanted
to maintain the given structure as close as possible so that
it is possible to easily add related features if necessary.
That's the reason why lot of the header files seem to have
non-sense content, but u-boot for coldfire will still compile
and work. In the current case, inb()/outb() do not need to be
defined simply because all register accesses are directly memory
referenced for coldfire (*(volatile ...)).
> * "include/asm-m68k/m5272.h" and "include/asm-m68k/m5282.h" contain this:
> (C) Copyright 2001, Key Technology (http://www.keyww.com)
> To become part of U-Boot these files must be available under GPL.
> If you can guarantee that we may use these files under GPL please fix
> the file headers.
Interessting thing! I'm glad that you stumbled of this
issue: i have copied over the m5272.h file from the COLILO
("coldfire linux loader" which is the pre-bootloader for
M5272C3). COLILO is distributed under the terms of the
GPL, so i didn't took too much care about the file headers.
We might check if a newer release of COLILO has a GPLed
header in this file. Otherwise we need to check (together
with the author of COLILO) back with the current copyright
holder. The m5282.h is just a copy of the m5272.h file
with the approprate changes for 5282 (changes done by me).
> * "include/asm-m68k/mcftimer.h" and "include/asm-m68k/mcfuart.h"
> contain this:
> (C) Copyright 1999-2002, Greg Ungerer (gerg at snapgear.com)
> (C) Copyright 2000, Lineo Inc. (www.lineo.com)
> To become part of U-Boot these files must be available under GPL.
> If you can guarantee that we may use these files under GPL please fix
> the file headers.
This is a similar issue: that file was taken from the
uClinux kernel source tree from include/asm-m68knommu/mcfuart.h,
and guess what: even the latest release of the kernel
"linux-2.6.0-test11" still holds that header!! (somebody
should inform the LKML) - it should be easy to sort out that
issue, i guess, unless the companies that bought snapgear and
lineo are trying to go the SCO way :-)
> * "include/asm-m68k/mmu.h" contains (as a comment says): "PowerPC
> memory management structures" which makes no sense for M68K.
> Omitted.
True, but just as like as for the other files: maybe somebody
may like to port u-boot for MMU-full Amigas (BTW.: there is
an MMU-full coldfire coming, soon). Then at least we are
using a defined "infrastructure". But certainly agreed: we
need to get rid of any "PPCisms" in these files. The files
could IMHO stay, even if empty, who cares?
> * "include/asm-m68k/processor.h" is a PowerPC file which makes no
> sense for M68K. Omitted.
dito
> * "include/configs/M5272C3.h" and "include/configs/M5282EVB.h"
> contain neither a copyright notice nor a GPL header.
> Pleae fix.
M5272C3.h is IIRC a copy of TQM860L.h, so it should
actually include your standard header with a minor
modification:
/*
* (C) Copyright 2000, 2001, 2002
* Wolfgang Denk, DENX Software Engineering, wd at denx.de.
* Coldfire support contributed by Bernhard Kuhn, bkuhn at metrowerks.com
*
* See file CREDITS for list of people who contributed to this
* project.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License as
* published by the Free Software Foundation; either version 2 of
* the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 59 Temple Place, Suite 330, Boston,
* MA 02111-1307 USA
*/
The same header can be applied to M5282EVB.h since it is
derived from M5272C3.h
> * "lib_m68k/cache.c" is effectively an empty file. Omitted.
same "structural maintainance idea" as for certain *.h files ...
> * "lib_m68k/config.mk" defines "-D__linux__ -D__uClinux__", but both
> is most probably wrong for U-Boot. Omitted.
I'm not sure about that: some of the header files
are simply copied over from the uClinux kernel (such
as mcfuart.h) and iirc, these files need these definitions
in order to be compilable. But true, for a clean solution
this issue should be fixed in the appropriate files, rather
than applying this quick-hack.
> * "lib_m68k/ctype.c" contains the same cose as already present in
> "lib_generic/ctype.c". Omitted.
Then we should use the generic one, right? :-)
> * "lib_m68k/kgdb.c" is a PowerPC file which makes no sense for
> M68K. Omitted.
Again, copied over from the ppc tree at the beginning of
the development and doesn't provide any usefull functionality
at the moment, except for being a place-holder for future use.
> * "lib_m68k/ldiv.c" contains the same cose as already present in
> "lib_generic/ldiv.c". Omitted.
> * "lib_m68k/Makefile" needs major cleanup (see above). Omitted.
> * "lib_m68k/ticks.S" is a PowerPC file which makes no sense for M68K.
> Omitted.
Ok, needs cleanup.
> * "lib_m68k/time.c" contains a dummy implementation of
> init_timebase() [which probably makes no sense on M68K at all] -
> please verify that this is OK!
We can probably delete it, unless this function may be
usefull for other m68k-ports (which i don't know)
> * All files in the "utils/coldfire/M5272C3/bdm/" directory are either
> missing a copyright notice and a GPL header, or they contain
proprietary
> copyright notices. Omitted.
We can make this files part of the pre-loader distribution(s),
or seperate them otherwise - these files are not vital for
the compilation process.
> * "cpu/coldfire/cpu_init.c" contains verbatim copy of a MPC8xx
> specific file which makes no sense for M68K. Omitted.
IIRC, only cpu_init_r() is called.
> * "cpu/coldfire/fec.h" is effectively an empty file - please
> verify that this is OK!
the actual fec register defintions are in 8xx_immap.h
> * "cpu/coldfire/interrupts.c" fails to implement
> irq_install_handler() / irq_free_handler() / enable_interrupts()
> and disable_interrupts() [and probably more] functions - please
> verify that this is OK!
The current implementation of u-boot for coldfire is not
using any interrupts. Status registers are pooled.
> * "cpu/coldfire/kgdb.S" is a verbatim copy of a MPC8xx specific file
> which makes no sense for M68K. Omitted.
> * "cpu/coldfire/kgdb.c" is broken / missing. Omitted.
again, legacy code :-) needs cleanup.
> * "cpu/coldfire/speed.c" contains only dummy definitions for critical
> functions get_gclk_freq() and get_bus_freq() - please verify that
> this is OK!
To my knowladge, for the EVBs, these frequencies can only be
altered by changing the crystals, so it's probably ok to
not taking care about core and bus frequency for now.
> I have added the remaining stuff which is obviously not usable in
> this form. Please help to clean up and add the missing parts.
As indicated above: i can't afford to spend time on the cleanup work,
right now, sorry. However, if somebody is volunteering to do the
necessary cleanups, then i will try to assist her or him as good
as possible.
best regards
Bernhard
More information about the U-Boot
mailing list