[U-Boot-Users] [Patch] Disable icache before call the first line of kernel in do_bootelf().

[U-Boot-Users] [Patch] Disable icache before call the first line of kernel in do_bootelf().

[Sonic Zhang]

On some architecture, it is not save to call the first line of kernel 
without disable the instruction cache. For do_bootm() path, architecture 
specific library code do_bootm_linux() can adress this issue, but 
do_bootelf() is common code. This patch disables icache before call kernel.


Signed-off-by: Sonic Zhang <sonic.zhang@gmail.com>

Index: common/cmd_elf.c
===================================================================
--- common/cmd_elf.c	(revision 871)
+++ common/cmd_elf.c	(working copy)
@@ -65,6 +65,8 @@
 	 */
 	if (dcache_status ())
 		dcache_disable ();
+	if (icache_status ())
+		icache_disable ();
 
 	/* pass cmdline to the kernel. */
 	cmdline = make_command_line();

[U-Boot-Users] [Patch] Disable icache before call the first line of kernel in do_bootelf().

[Wolfgang Denk]

In message <1186552257.15798.6.camel@sevens.analog.com> you wrote:
> On some architecture, it is not save to call the first line of kernel 
> without disable the instruction cache. For do_bootm() path, architecture 
> specific library code do_bootm_linux() can adress this issue, but 
> do_bootelf() is common code. This patch disables icache before call kernel.
> 
> 
> Signed-off-by: Sonic Zhang <sonic.zhang@gmail.com>

Did you test the impact of this change on all other architectures but
the one you are talking about (without ever mentioning it)?

I don't think we want to do this in general. Please make  this code
conditional.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Though a program be but three lines long,
someday it will have to be maintained."
- The Tao of Programming

[U-Boot-Users] [Patch] Disable icache before call the first line of kernel in do_bootelf().

[Michal Simek]

Hi,

>> On some architecture, it is not save to call the first line of kernel 
>> without disable the instruction cache. For do_bootm() path, architecture 
>> specific library code do_bootm_linux() can adress this issue, but 
>> do_bootelf() is common code. This patch disables icache before call kernel.
>> Signed-off-by: Sonic Zhang <sonic.zhang@gmail.com>

>Did you test the impact of this change on all other architectures but
>the one you are talking about (without ever mentioning it)?

>I don't think we want to do this in general. Please make  this code
>conditional.

I met with this problem on PowerQuicc II Pro in bootm command. Disabling icache before call kernel
solved my problem. I suppose the same problem can be in bootelf command.

Best regards,
Michal Simek

[U-Boot-Users] [Patch] Disable icache before call the firstline of kernel in do_bootelf().

[Robin Getz]

On Wed 8 Aug 2007 02:50, Wolfgang Denk pondered:
> In message <1186552257.15798.6.camel@sevens.analog.com> you wrote:
> > On some architecture, it is not save to call the first line of kernel 
> > without disable the instruction cache. For do_bootm() path,
> architecture 
> > specific library code do_bootm_linux() can adress this issue, but 
> > do_bootelf() is common code. This patch disables icache before call
> kernel.
> > 
> > 
> > Signed-off-by: Sonic Zhang <sonic.zhang@gmail.com>
> 
> Did you test the impact of this change on all other architectures but
> the one you are talking about (without ever mentioning it)?
> 
> I don't think we want to do this in general. Please make  this code
> conditional.

I think it is reasonable/prudent to turn off cache in all situations, on all 
architectures.

As an application developer, I don't want an early TLB miss on any 
architecture to cause a vector back to U-Boot's cache handler (which may not 
be there anymore). I would expect to have to write my own, and set it up 
myself if I want to run with cache on.

Instruction and data caches in U-Boot are a great thing, since it decreases 
the time for a kernel or file system to be loaded, decompressed, but once 
U-Boot looses control (the jump to the unknown application), it shouldn't 
expect a return, or the hardware not to be changed by the app.

go, bootm, boote, etc - should flush & turn off cache before the jump to the 
application code (should also turn off what limited interrupts are enabled) 
to be safe.

It could be a fair comment to tell people to add "dcache off; icache off; 
boote" to their scripts, but this slows down boot, since you want the cache 
on while you are doing the elf relocations. What you might want is 
a  "relocate_elf; dcache off; icache off; go $(elf_entry)" which seems worse 
than just forcing it off in all situations. ?

-Robin

[U-Boot-Users] [Patch] Disable icache before call the firstline of kernel in do_bootelf().

[Wolfgang Denk]

In message <200708081147.09111.rgetz@blackfin.uclinux.org> you wrote:
>
> I think it is reasonable/prudent to turn off cache in all situations, on all 
> architectures.

No, it is not. Some architectures /  operating  systems  may  require
cache(s)  being on.  Some  others may not care, except for boot speed
which in turn is something some of the customers do care about.

Finally, I don't want to fix a prblem where non exists.

If we know for sure that certain systems require caches disabeled,
then let's provide them this environment; but please leave all the
rest unchanged.

> Instruction and data caches in U-Boot are a great thing, since it decreases 
> the time for a kernel or file system to be loaded, decompressed, but once 
> U-Boot looses control (the jump to the unknown application), it shouldn't 
> expect a return, or the hardware not to be changed by the app.

There is more than just Linux...

> go, bootm, boote, etc - should flush & turn off cache before the jump to the 
> application code (should also turn off what limited interrupts are enabled) 
> to be safe.

I disagree.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Clothes make the man. Naked people have little  or  no  influence  on
society.                                                 - Mark Twain

[U-Boot-Users] [Patch] Disable icache before call the firstline of kernel in do_bootelf().

[Stefan Roese]

On Wednesday 08 August 2007, Wolfgang Denk wrote:
> In message <200708081147.09111.rgetz@blackfin.uclinux.org> you wrote:
> > I think it is reasonable/prudent to turn off cache in all situations, on
> > all architectures.
>
> No, it is not. Some architectures /  operating  systems  may  require
> cache(s)  being on.  Some  others may not care, except for boot speed
> which in turn is something some of the customers do care about.
>
> Finally, I don't want to fix a prblem where non exists.

I'm not totally sure here, but I remember seeing a problem with booting 
VxWorks which could be related to having the icache still on. Or was it 
QNX/Neutrino? Not sure anymore, too long ago. From my memory, all those other 
OS'es I ever worked with, are much "happier" with interrupts and caches 
disabled upon startup. It could be that we just forgot to disable the icache 
in the initial port of the bootelf command.

Perhaps some users of those other OS'es (VxWorks, QNX...) could speak up and 
suggest which implementation is recommended.

> If we know for sure that certain systems require caches disabeled,
> then let's provide them this environment; but please leave all the
> rest unchanged.
>
> > Instruction and data caches in U-Boot are a great thing, since it
> > decreases the time for a kernel or file system to be loaded,
> > decompressed, but once U-Boot looses control (the jump to the unknown
> > application), it shouldn't expect a return, or the hardware not to be
> > changed by the app.
>
> There is more than just Linux...

Yes. But the same reasons could apply to the other systems too. From my 
understanding we are "on the safe side" with caches disabled.

> > go, bootm, boote, etc - should flush & turn off cache before the jump to
> > the application code (should also turn off what limited interrupts are
> > enabled) to be safe.
>
> I disagree.

I tend to agree with Robin here.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot-Users] [Patch] Disable icache before call the first line of kernel in do_bootelf().

[Sonic Zhang]

Hi Wolfgang Denk,

Thank you for your comment. Yes, this may not be necessary for all arch.
How about add a hook function for each arch?

I find the icache is also disabled before call a binary in common code
do_go(). See following snip. Should these lines be conditional?

Thanks

Sonic

int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
{
        ulong   addr, rc;
        int     rcode = 0;

        if (argc < 2) {
                printf ("Usage:\n%s\n", cmdtp->usage);
                return 1;
        }

        addr = simple_strtoul(argv[1], NULL, 16);
        printf ("## Starting application at 0x%08lX ...\n", addr);

        if (icache_status()){
               icache_disable();
        }
        if (dcache_status()) {
               dcache_disable();
        }

...

[U-Boot-Users] [Patch] Disable icache before call the first line of kernel in do_bootelf().

[Wolfgang Denk]

In message <1186558396.17519.11.camel@sevens.analog.com> you wrote:
> 
> Thank you for your comment. Yes, this may not be necessary for all arch.
> How about add a hook function for each arch?

Using "weak", right.

> I find the icache is also disabled before call a binary in common code
> do_go(). See following snip. Should these lines be conditional?

Ummm...

> int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
> {
>         ulong   addr, rc;
>         int     rcode = 0;
> 
>         if (argc < 2) {
>                 printf ("Usage:\n%s\n", cmdtp->usage);
>                 return 1;
>         }
> 
>         addr = simple_strtoul(argv[1], NULL, 16);
>         printf ("## Starting application at 0x%08lX ...\n", addr);
> 
>         if (icache_status()){
>                icache_disable();
>         }
>         if (dcache_status()) {
>                dcache_disable();
>         }

This is not in the official tree, and it will not make it  there.  It
will probably not even build for many boards.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
There are some things worth dying for.
	-- Kirk, "Errand of Mercy", stardate 3201.7

[U-Boot-Users] [Patch] Disable icache before call the first line of kernel in do_bootelf().

[Mike Frysinger]

On Wednesday 08 August 2007, Wolfgang Denk wrote:
> In message <1186558396.17519.11.camel@sevens.analog.com> you wrote:
> > Thank you for your comment. Yes, this may not be necessary for all arch.
> > How about add a hook function for each arch?
>
> Using "weak", right.

so using weak hooks is OK now ?  i think it'd be good to migrate all of the 
ugly boote/bootm/etc... cruft to external weaks and let arches define their 
own

> > int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
> > {
> >         ulong   addr, rc;
> >         int     rcode = 0;
> >
> >         if (argc < 2) {
> >                 printf ("Usage:\n%s\n", cmdtp->usage);
> >                 return 1;
> >         }
> >
> >         addr = simple_strtoul(argv[1], NULL, 16);
> >         printf ("## Starting application at 0x%08lX ...\n", addr);
> >
> >         if (icache_status()){
> >                icache_disable();
> >         }
> >         if (dcache_status()) {
> >                dcache_disable();
> >         }
>
> This is not in the official tree, and it will not make it  there.  It
> will probably not even build for many boards.

it's called pseudo code ... of course code that has "..." in it wont build :p
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20070813/7d25b842/attachment.pgp 

[U-Boot-Users] [Patch] Disable icache before call the first line of kernel in do_bootelf().

[Wolfgang Denk]

Dear Mike,

in message <200708131052.44472.vapier@gentoo.org> you wrote:
> 
> so using weak hooks is OK now ?  i think it'd be good to migrate all of the

It always has been OK - just nobody bothered to use it. [And I didn't
even know about it when I started working on PPCBoot.]

> ugly boote/bootm/etc... cruft to external weaks and let arches define their
> own

Yes, a few #ifdef's can be eliminated that way.

> > > int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
> > > {
> > >         ulong   addr, rc;
> > >         int     rcode = 0;
> > >
> > >         if (argc < 2) {
> > >                 printf ("Usage:\n%s\n", cmdtp->usage);
> > >                 return 1;
> > >         }
> > >
> > >         addr = simple_strtoul(argv[1], NULL, 16);
> > >         printf ("## Starting application at 0x%08lX ...\n", addr);
> > >
> > >         if (icache_status()){
> > >                icache_disable();
> > >         }
> > >         if (dcache_status()) {
> > >                dcache_disable();
> > >         }
> >
> > This is not in the official tree, and it will not make it  there.  It
> > will probably not even build for many boards.
> 
> it's called pseudo code ... of course code that has "..." in it wont build > :p

The only "..." I see was in the printf() format string, where it
doesn't hurt compilation.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Swap read error.  You lose your mind.

[U-Boot-Users] [Patch] Disable icache before call the first line of kernel in do_bootelf().

[Mike Frysinger]

On Monday 13 August 2007, Wolfgang Denk wrote:
> in message <200708131052.44472.vapier@gentoo.org> you wrote:
> > so using weak hooks is OK now ?  i think it'd be good to migrate all of
> > the
>
> It always has been OK - just nobody bothered to use it. [And I didn't
> even know about it when I started working on PPCBoot.]

the word on the street was weaks were banned due to old busted compilers ... 
but if this is not the case :)

> > ugly boote/bootm/etc... cruft to external weaks and let arches define
> > their own
>
> Yes, a few #ifdef's can be eliminated that way.

ok, we'll look at doing that, thanks !
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20070813/4d5b4c35/attachment.pgp 

[U-Boot-Users] [Patch] Disable icache before call the first line of kernel in do_bootelf().

[Mike Frysinger]

On Monday 13 August 2007, Wolfgang Denk wrote:
> in message <200708131052.44472.vapier@gentoo.org> you wrote:
> > so using weak hooks is OK now ?  i think it'd be good to migrate all of
> > the
>
> It always has been OK - just nobody bothered to use it. [And I didn't
> even know about it when I started working on PPCBoot.]
>
> > ugly boote/bootm/etc... cruft to external weaks and let arches define
> > their own
>
> Yes, a few #ifdef's can be eliminated that way.

unfortunately, using weak symbols and overriding elsewhere doesnt look like 
it's possible currently due to the way ld searches archives.  for example, if 
i do something like:
common/cmd_elf.c:
__attribute__((weak)) do_bootelf_setup() { ... current code ... }
do_bootelf() {
	...
	do_bootelf_setup();
	...

and then i want to override this with a Blackfin version:
lib_blackfin/bootelf_setup.c:
do_bootelf_setup() { ... }

but since the linking process looks like:
ld ... --start-group ... \
	... lib_blackfin/libblackfin.a ... \
	... common/libcommon.a ... \
	--end-group ...
ld will pick the weak symbol provided by libcommon.a even though a strong 
symbol is also available in libblackfin.a :(

so our only realistic options are:
 - create another static archive in common/ for weak symbols and specify it 
just before libcommon.a
 - live with #ifdef's, but minimize the crappy situation by splitting out just 
the relevant code so the main bootelf function stays clean
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080128/a3885df1/attachment.pgp 

[U-Boot-Users] [Patch] Disable icache before call the first line of kernel in do_bootelf().

[Wolfgang Denk]

In message <200801282114.14393.vapier@gentoo.org> you wrote:
>
> unfortunately, using weak symbols and overriding elsewhere doesnt look like 
> it's possible currently due to the way ld searches archives.  for example, if 

???

> ld will pick the weak symbol provided by libcommon.a even though a strong 
> symbol is also available in libblackfin.a :(

That should never happen. What is your toolchain?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
As usual, this being a 1.3.x release, I haven't  even  compiled  this
kernel yet. So if it works, you should be doubly impressed.
      - Linus Torvalds in <199506181536.SAA10638@keos.cs.Helsinki.FI>

[U-Boot-Users] [Patch] Disable icache before call the first line of kernel in do_bootelf().

[Mike Frysinger]

On Tuesday 29 January 2008, Wolfgang Denk wrote:
> In message <200801282114.14393.vapier@gentoo.org> you wrote:
> > unfortunately, using weak symbols and overriding elsewhere doesnt look
> > like it's possible currently due to the way ld searches archives.  for
> > example, if
>
> ???
>
> > ld will pick the weak symbol provided by libcommon.a even though a strong
> > symbol is also available in libblackfin.a :(
>
> That should never happen. What is your toolchain?

read the binutils mailing list.  this is the expected behavior of ld.

http://sourceware.org/ml/binutils/2008-01/msg00301.html
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080129/961d4384/attachment.pgp 

[U-Boot-Users] [Patch] Disable icache before call the first line of kernel in do_bootelf().

[Johannes Stezenbach]

On Tue, Jan 29, 2008 at 07:12:10PM -0500, Mike Frysinger wrote:
> On Tuesday 29 January 2008, Wolfgang Denk wrote:
> > In message <200801282114.14393.vapier@gentoo.org> you wrote:
> > > unfortunately, using weak symbols and overriding elsewhere doesnt look
> > > like it's possible currently due to the way ld searches archives.  for
> > > example, if
> >
> > ???
> >
> > > ld will pick the weak symbol provided by libcommon.a even though a strong
> > > symbol is also available in libblackfin.a :(
> >
> > That should never happen. What is your toolchain?
> 
> read the binutils mailing list.  this is the expected behavior of ld.
> 
> http://sourceware.org/ml/binutils/2008-01/msg00301.html

Overriding weak with strong symbols works just fine, however
in your case ld has no reason to even look at your override,
since it already has a (weak) definition for do_bootelf_setup().

Thus the solution for your problem is to give ld a reason
to pull in your do_bootelf_setup() definition. There are
two ways:

- explicitly list the .o file on the linker command line,
  i.e. add it to $(OBJS) or $(PLATFORM_LIBS) instead
  of libblackfin.a

- put the do_bootelf_setup() definition in a .o file
  along with some other code you know will be pulled in,
  e.g. add it to lib_blackfin/cache.c instead of putting
  it in its own file


HTH,
Johannes

[U-Boot-Users] [Patch] Disable icache before call the first line of kernel in do_bootelf().

[Mike Frysinger]

On Wednesday 30 January 2008, Johannes Stezenbach wrote:
> On Tue, Jan 29, 2008 at 07:12:10PM -0500, Mike Frysinger wrote:
> > On Tuesday 29 January 2008, Wolfgang Denk wrote:
> > > In message <200801282114.14393.vapier@gentoo.org> you wrote:
> > > > unfortunately, using weak symbols and overriding elsewhere doesnt
> > > > look like it's possible currently due to the way ld searches
> > > > archives.  for example, if
> > >
> > > ???
> > >
> > > > ld will pick the weak symbol provided by libcommon.a even though a
> > > > strong symbol is also available in libblackfin.a :(
> > >
> > > That should never happen. What is your toolchain?
> >
> > read the binutils mailing list.  this is the expected behavior of ld.
> >
> > http://sourceware.org/ml/binutils/2008-01/msg00301.html
>
> Overriding weak with strong symbols works just fine, however
> in your case ld has no reason to even look at your override,
> since it already has a (weak) definition for do_bootelf_setup().
>
> Thus the solution for your problem is to give ld a reason
> to pull in your do_bootelf_setup() definition. There are
> two ways:
>
> - explicitly list the .o file on the linker command line,
>   i.e. add it to $(OBJS) or $(PLATFORM_LIBS) instead
>   of libblackfin.a
>
> - put the do_bootelf_setup() definition in a .o file
>   along with some other code you know will be pulled in,
>   e.g. add it to lib_blackfin/cache.c instead of putting
>   it in its own file

thanks ... i had created a dedicated .c file for it of only strong symbols 
which would override the weaks, but as you say, with no other compelling 
reason to look at the object, ld wasnt considering it
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080201/909b988b/attachment.pgp 

[U-Boot-Users] [patch] allow ports to override bootelf behavior

[Mike Frysinger]

This splits the dcache logic out of do_bootelf into a dedicated weak function
called do_bootelf_exec.  This way ports can control the behavior before
executing an ELF image however they like.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
diff --git a/common/cmd_elf.c b/common/cmd_elf.c
index 2eb7453..5689e2c 100644
--- a/common/cmd_elf.c
+++ b/common/cmd_elf.c
@@ -30,6 +30,31 @@ DECLARE_GLOBAL_DATA_PTR;
 int valid_elf_image (unsigned long addr);
 unsigned long load_elf_image (unsigned long addr);
 
+__attribute__((weak))
+unsigned long do_bootelf_exec (ulong (*entry)(int, char *[]), int argc, char *argv[])
+{
+	unsigned long ret;
+
+	/*
+	 * QNX images require the data cache is disabled.
+	 * Data cache is already flushed, so just turn it off.
+	 */
+	int dcache = dcache_status ();
+	if (dcache)
+		dcache_disable ();
+
+	/*
+	 * pass address parameter as argv[0] (aka command name),
+	 * and all remaining args
+	 */
+	ret = entry (argc, argv);
+
+	if (dcache)
+		dcache_enable ();
+
+	return ret;
+}
+
 /* ======================================================================
  * Interpreter command to boot an arbitrary ELF image from memory.
  * ====================================================================== */
@@ -53,18 +78,7 @@ int do_bootelf (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 
 	printf ("## Starting application at 0x%08lx ...\n", addr);
 
-	/*
-	 * QNX images require the data cache is disabled.
-	 * Data cache is already flushed, so just turn it off.
-	 */
-	if (dcache_status ())
-		dcache_disable ();
-
-	/*
-	 * pass address parameter as argv[0] (aka command name),
-	 * and all remaining args
-	 */
-	rc = ((ulong (*)(int, char *[])) addr) (--argc, &argv[1]);
+	rc = do_bootelf_exec ((void *)addr, argc - 1, argv + 1);
 	if (rc != 0)
 		rcode = 1;
 

[U-Boot-Users] [patch] allow ports to override bootelf behavior

[Wolfgang Denk]

In message <200802011045.00200.vapier@gentoo.org> you wrote:
> This splits the dcache logic out of do_bootelf into a dedicated weak function
> called do_bootelf_exec.  This way ports can control the behavior before
> executing an ELF image however they like.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> diff --git a/common/cmd_elf.c b/common/cmd_elf.c

This doesn't fit current code any moe. Please rebase and resubmit.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Even if you aren't in doubt, consider the mental welfare of the  per-
son who has to maintain the code after you, and who will probably put
parens in the wrong place.          - Larry Wall in the perl man page

[U-Boot-Users] [patch] allow ports to override go behavior

[Mike Frysinger]

This splits the arch-specific logic out of do_go() and into a dedicated weak
function called do_go_exec() that lives in cpu directories.  This will need
review from i386/nios people to make sure i didnt break them.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
diff --git a/common/cmd_boot.c b/common/cmd_boot.c
index e68f16f..82a5710 100644
--- a/common/cmd_boot.c
+++ b/common/cmd_boot.c
@@ -28,9 +28,11 @@
 #include <command.h>
 #include <net.h>
 
-#if defined(CONFIG_I386)
-DECLARE_GLOBAL_DATA_PTR;
-#endif
+__attribute__((weak))
+unsigned long do_go_exec (ulong (*entry)(int, char *[]), int argc, char *argv[])
+{
+	return entry (argc, argv);
+}
 
 int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 {
@@ -50,21 +52,7 @@ int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	 * pass address parameter as argv[0] (aka command name),
 	 * and all remaining args
 	 */
-#if defined(CONFIG_I386)
-	/*
-	 * x86 does not use a dedicated register to pass the pointer
-	 * to the global_data
-	 */
-	argv[0] = (char *)gd;
-#endif
-#if !defined(CONFIG_NIOS)
-	rc = ((ulong (*)(int, char *[]))addr) (--argc, &argv[1]);
-#else
-	/*
-	 * Nios function pointers are address >> 1
-	 */
-	rc = ((ulong (*)(int, char *[]))(addr>>1)) (--argc, &argv[1]);
-#endif
+	rc = do_go_exec ((void *)addr, argv - 1, argv + 1);
 	if (rc != 0) rcode = 1;
 
 	printf ("## Application terminated, rc = 0x%lX\n", rc);
diff --git a/lib_i386/board.c b/lib_i386/board.c
index 47fbab4..22191e6 100644
--- a/lib_i386/board.c
+++ b/lib_i386/board.c
@@ -421,3 +421,11 @@ void hang (void)
 	puts ("### ERROR ### Please RESET the board ###\n");
 	for (;;);
 }
+
+unsigned long do_go_exec (ulong (*entry)(int, char *[]), int argc, char *argv[])
+{
+	/*
+	 * Nios function pointers are address >> 1
+	 */
+	return (entry >> 1) (argc, argv);
+}
diff --git a/lib_nios/board.c b/lib_nios/board.c
index 0a0d2e3..cdaf753 100644
--- a/lib_nios/board.c
+++ b/lib_nios/board.c
@@ -190,3 +190,13 @@ void hang (void)
 	puts("### ERROR ### Please reset board ###\n");
 	for (;;);
 }
+
+unsigned long do_go_exec (ulong (*entry)(int, char *[]), int argc, char *argv[])
+{
+	/*
+	 * x86 does not use a dedicated register to pass the pointer
+	 * to the global_data
+	 */
+	argv[-1] = (char *)gd;
+	return entry (argc, argv);
+}

[U-Boot-Users] [patch] allow ports to override go behavior

[Mike Frysinger]

blah, and without fail, i swapped nios/i386
---
This splits the arch-specific logic out of do_go() and into a dedicated weak
function called do_go_exec() that lives in cpu directories.  This will need
review from i386/nios people to make sure i didnt break them.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
diff --git a/common/cmd_boot.c b/common/cmd_boot.c
index e68f16f..82a5710 100644
--- a/common/cmd_boot.c
+++ b/common/cmd_boot.c
@@ -28,9 +28,11 @@
 #include <command.h>
 #include <net.h>
 
-#if defined(CONFIG_I386)
-DECLARE_GLOBAL_DATA_PTR;
-#endif
+__attribute__((weak))
+unsigned long do_go_exec (ulong (*entry)(int, char *[]), int argc, char *argv[])
+{
+	return entry (argc, argv);
+}
 
 int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 {
@@ -50,21 +52,7 @@ int do_go (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
 	 * pass address parameter as argv[0] (aka command name),
 	 * and all remaining args
 	 */
-#if defined(CONFIG_I386)
-	/*
-	 * x86 does not use a dedicated register to pass the pointer
-	 * to the global_data
-	 */
-	argv[0] = (char *)gd;
-#endif
-#if !defined(CONFIG_NIOS)
-	rc = ((ulong (*)(int, char *[]))addr) (--argc, &argv[1]);
-#else
-	/*
-	 * Nios function pointers are address >> 1
-	 */
-	rc = ((ulong (*)(int, char *[]))(addr>>1)) (--argc, &argv[1]);
-#endif
+	rc = do_go_exec ((void *)addr, argv - 1, argv + 1);
 	if (rc != 0) rcode = 1;
 
 	printf ("## Application terminated, rc = 0x%lX\n", rc);
diff --git a/lib_i386/board.c b/lib_i386/board.c
index 47fbab4..36ab6a8 100644
--- a/lib_i386/board.c
+++ b/lib_i386/board.c
@@ -421,3 +421,13 @@ void hang (void)
 	puts ("### ERROR ### Please RESET the board ###\n");
 	for (;;);
 }
+
+unsigned long do_go_exec (ulong (*entry)(int, char *[]), int argc, char *argv[])
+{
+	/*
+	 * x86 does not use a dedicated register to pass the pointer
+	 * to the global_data
+	 */
+	argv[-1] = (char *)gd;
+	return entry (argc, argv);
+}
diff --git a/lib_nios/board.c b/lib_nios/board.c
index 0a0d2e3..3a05e83 100644
--- a/lib_nios/board.c
+++ b/lib_nios/board.c
@@ -190,3 +190,11 @@ void hang (void)
 	puts("### ERROR ### Please reset board ###\n");
 	for (;;);
 }
+
+unsigned long do_go_exec (ulong (*entry)(int, char *[]), int argc, char *argv[])
+{
+	/*
+	 * Nios function pointers are address >> 1
+	 */
+	return (entry >> 1) (argc, argv);
+}

[U-Boot-Users] [patch] allow ports to override go behavior

[Wolfgang Denk]

In message <200802011136.55390.vapier@gentoo.org> you wrote:
> blah, and without fail, i swapped nios/i386
> ---
> This splits the arch-specific logic out of do_go() and into a dedicated weak
> function called do_go_exec() that lives in cpu directories.  This will need
> review from i386/nios people to make sure i didnt break them.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> diff --git a/common/cmd_boot.c b/common/cmd_boot.c

This doesn't fit current code any more. Please rebase and resubmit.

Please make sure to use only ONE "---" line, and place comments that
are supposed NOT to go into the commit message BELOW that line; you
got this swapped here.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A supercomputer is a machine that runs an endless loop in 2 seconds.

[U-Boot-Users] [patch] allow ports to override go behavior

[Wolfgang Denk]

In message <200802011058.34891.vapier@gentoo.org> you wrote:
> This splits the arch-specific logic out of do_go() and into a dedicated weak
> function called do_go_exec() that lives in cpu directories.  This will need
> review from i386/nios people to make sure i didnt break them.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>

This doesn't fit current code any more. Please rebase and resubmit.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Even if you aren't in doubt, consider the mental welfare of the  per-
son who has to maintain the code after you, and who will probably put
parens in the wrong place.          - Larry Wall in the perl man page