[U-Boot] show_boot_progess @ ppc not working

All,
can someone tell me why the board specific function "void show_boot_progress(int arg)" is no longer called (at least on MPC5200).
Of course this line is present in the board config :
#define CONFIG_SHOW_BOOT_PROGRESS 1
What have I missed ?
regards, André
MATRIX VISION GmbH, Talstraße 16, DE-71570 Oppenweiler - Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschäftsführer: Gerhard Thullner, Werner Armingeon, Uwe Furtner

Hello Andre,
Andre Schwarz schrieb:
All,
can someone tell me why the board specific function "void show_boot_progress(int arg)" is no longer called (at least on MPC5200).
Of course this line is present in the board config :
#define CONFIG_SHOW_BOOT_PROGRESS 1
What have I missed ?
The show_boot_progress () function was changed in a weak function, see commit:
make show_boot_progress () weak.
author Heiko Schocher hs@pollux.denx.de Fri, 13 Jul 2007 07:54:17 +0000 (09:54 +0200) committer Heiko Schocher hs@pollux.denx.de Fri, 13 Jul 2007 07:54:17 +0000 (09:54 +0200) commit fad63407154f46246ce80d53a9c669a44362ac67
For what board exactly is it not working?
Maybe a problem with your compiler?
bye Heiko

Heiko Schocher schrieb:
Hello Andre,
Andre Schwarz schrieb:
All,
can someone tell me why the board specific function "void show_boot_progress(int arg)" is no longer called (at least on MPC5200).
Of course this line is present in the board config :
#define CONFIG_SHOW_BOOT_PROGRESS 1
What have I missed ?
The show_boot_progress () function was changed in a weak function, see commit:
make show_boot_progress () weak.
author Heiko Schocher hs@pollux.denx.de Fri, 13 Jul 2007 07:54:17 +0000 (09:54 +0200) committer Heiko Schocher hs@pollux.denx.de Fri, 13 Jul 2007 07:54:17 +0000 (09:54 +0200) commit fad63407154f46246ce80d53a9c669a44362ac67
For what board exactly is it not working?
mvBC-P (5200 based) -> board/matrix_vision/mvbc_p
Maybe a problem with your compiler?
Don't know - it's ELDK-4.2 :
ppc_6xx-gcc -v gives :
Reading specs from /home/eldk-4.2/usr/bin/../lib/gcc/powerpc-linux/4.2.2/specs Target: powerpc-linux Configured with: /opt/eldk/build/ppc-2008-04-01/work/usr/src/denx/BUILD/crosstool-0.43/build/gcc-4.2.2-glibc-20070515T2025-eldk/powerpc-linux/gcc-4.2.2/configure --target=powerpc-linux --host=i686-host_pc-linux-gnu --prefix=/var/tmp/eldk.UZpAG7/usr/crosstool/gcc-4.2.2-glibc-20070515T2025-eldk/powerpc-linux --disable-hosted-libstdcxx --with-headers=/var/tmp/eldk.UZpAG7/usr/crosstool/gcc-4.2.2-glibc-20070515T2025-eldk/powerpc-linux/powerpc-linux/include --with-local-prefix=/var/tmp/eldk.UZpAG7/usr/crosstool/gcc-4.2.2-glibc-20070515T2025-eldk/powerpc-linux/powerpc-linux --disable-nls --enable-threads=posix --enable-symvers=gnu --enable-__cxa_atexit --enable-languages=c,c++,java --enable-shared --enable-c99 --enable-long-long --without-x Thread model: posix gcc version 4.2.2
bye Heiko
MATRIX VISION GmbH, Talstraße 16, DE-71570 Oppenweiler - Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschäftsführer: Gerhard Thullner, Werner Armingeon, Uwe Furtner

Hello Andre,
Andre Schwarz wrote:
All,
can someone tell me why the board specific function "void show_boot_progress(int arg)" is no longer called (at least on MPC5200).
Of course this line is present in the board config :
#define CONFIG_SHOW_BOOT_PROGRESS 1
What have I missed ?
Nothing! Good catch. I tried it on a mpc8xx and on a mpx82xx based board and it also didnt worked :-(
Can you try the following patch? (This patch solved it on my boards ...)
thanks, Heiko
[PATCH] all platforms: make show_boot_progress () again working.
Signed-off-by: Heiko Schocher hs@denx.de --- include/common.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/common.h b/include/common.h index 5968036..6583f00 100644 --- a/include/common.h +++ b/include/common.h @@ -693,7 +693,7 @@ int pcmcia_init (void); /* * Board-specific Platform code can reimplement show_boot_progress () if needed */ -void __attribute__((weak)) show_boot_progress (int val); +void show_boot_progress (int val);
#ifdef CONFIG_INIT_CRITICAL #error CONFIG_INIT_CRITICAL is deprecated!

Heiko Schocher schrieb:
Hello Andre,
Andre Schwarz wrote:
All,
can someone tell me why the board specific function "void show_boot_progress(int arg)" is no longer called (at least on MPC5200).
Of course this line is present in the board config :
#define CONFIG_SHOW_BOOT_PROGRESS 1
What have I missed ?
Nothing! Good catch. I tried it on a mpc8xx and on a mpx82xx based board and it also didnt worked :-(
Can you try the following patch? (This patch solved it on my boards ...)
thanks, Heiko
[PATCH] all platforms: make show_boot_progress () again working.
Signed-off-by: Heiko Schocher hs@denx.de
include/common.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/common.h b/include/common.h index 5968036..6583f00 100644 --- a/include/common.h +++ b/include/common.h @@ -693,7 +693,7 @@ int pcmcia_init (void); /*
- Board-specific Platform code can reimplement show_boot_progress ()
if needed */ -void __attribute__((weak)) show_boot_progress (int val); +void show_boot_progress (int val);
#ifdef CONFIG_INIT_CRITICAL #error CONFIG_INIT_CRITICAL is deprecated!
Heiko,
of course this patch makes it work again. But the question is : Why is this specific weak function not replaced by the board specific one ?
This gives me some pain when looking at the other weak functions....
In order to be able to compile u-boot I also (still) have to change <config.h> into "../include/config.h" inside common/env_embedded.c. Otherwise the config file is not taken into account and complains about CFG_ENV_SIZE undeclared.
I've discussed this with Wolfgang already a few weeks ago - he blamed my system configuration. This issues occur on my gentoo and ubuntu system. There's nothing special about this systems (out of the box + ELDK-4.2).
How can we solve this without making a guessing game out of it ?
regards, André
MATRIX VISION GmbH, Talstraße 16, DE-71570 Oppenweiler - Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschäftsführer: Gerhard Thullner, Werner Armingeon, Uwe Furtner

Hi All,
I had some 'fun' with weak functions a little while ago, and I'm wondering if maybe we should fix up a few inconsistencies...
Andre Schwarz wrote:
Heiko Schocher schrieb:
Hello Andre,
Andre Schwarz wrote:
All,
can someone tell me why the board specific function "void show_boot_progress(int arg)" is no longer called (at least on MPC5200).
<snip>
Can you try the following patch? (This patch solved it on my boards ...)
<snip>
-void __attribute__((weak)) show_boot_progress (int val); +void show_boot_progress (int val);
This makes show_boot_progress () consistent with nearly every other weak function (see below)
Interesting that this is the only function defined weak in this way. All other weak functions are defined normally (no attributes) and then declared weak with an alias. Good examples can be found in /include/status_led.h lines 391-396 and /lib_arm/board.c lines 125-138
<snip>
Heiko,
of course this patch makes it work again.
This patch leaves only a handful of inconsistent weak functions:
/include/asm-avr32/arch-at32ap700x/clk.h line 85:
extern void gclk_init(void) __attribute__((weak));
But this is not designed to be overridable - It is designed to not require a declaration - See /cpu/at32ap/cpu.c lines 68-69:
if(gclk_init) gclk_init();
/common/cmd_boot.c lines 32-36: (/common/cmd_elf.c lines 31-54 and /cpu/mips/cpu.c lines 41-43 are same semantics)
__attribute__((weak)) unsigned long do_go_exec (ulong (*entry)(int, char *[]), int argc, char *argv[]) { return entry (argc, argv); }
<snip>
But the question is : Why is this specific weak function not replaced by the board specific one ?
Good question - maybe because this function is inlined? (just a wild guess) - could be any of a number of reasons as far as I can tell.
This gives me some pain when looking at the other weak functions....
Is there any reason _not_ to fix up these last four weak functions so that all weak functions are defined and declared the same way? I can do up a patch if the consensus is to make them consistent.
Regards,
Graeme

Graeme Russ schrieb:
Hi All,
I had some 'fun' with weak functions a little while ago, and I'm wondering if maybe we should fix up a few inconsistencies...
Andre Schwarz wrote:
Heiko Schocher schrieb:
Hello Andre,
Andre Schwarz wrote:
All,
can someone tell me why the board specific function "void show_boot_progress(int arg)" is no longer called (at least on MPC5200).
<snip>
Can you try the following patch? (This patch solved it on my boards ...)
<snip>
-void __attribute__((weak)) show_boot_progress (int val); +void show_boot_progress (int val);
This makes show_boot_progress () consistent with nearly every other weak function (see below)
Interesting that this is the only function defined weak in this way. All other weak functions are defined normally (no attributes) and then declared weak with an alias. Good examples can be found in /include/status_led.h lines 391-396 and /lib_arm/board.c lines 125-138
<snip>
Heiko,
of course this patch makes it work again.
This patch leaves only a handful of inconsistent weak functions:
/include/asm-avr32/arch-at32ap700x/clk.h line 85:
extern void gclk_init(void) __attribute__((weak));
But this is not designed to be overridable - It is designed to not require a declaration - See /cpu/at32ap/cpu.c lines 68-69:
if(gclk_init) gclk_init();
/common/cmd_boot.c lines 32-36: (/common/cmd_elf.c lines 31-54 and /cpu/mips/cpu.c lines 41-43 are same semantics)
__attribute__((weak)) unsigned long do_go_exec (ulong (*entry)(int, char *[]), int argc, char *argv[]) { return entry (argc, argv); }
<snip>
But the question is : Why is this specific weak function not replaced by the board specific one ?
Good question - maybe because this function is inlined? (just a wild guess)
- could be any of a number of reasons as far as I can tell.
This gives me some pain when looking at the other weak functions....
Is there any reason _not_ to fix up these last four weak functions so that all weak functions are defined and declared the same way? I can do up a patch if the consensus is to make them consistent.
Please send a proper patch with explanation. Since I can not explain this effect 100% I'm not able to fix it.
Regards,
Graeme
MATRIX VISION GmbH, Talstraße 16, DE-71570 Oppenweiler - Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschäftsführer: Gerhard Thullner, Werner Armingeon, Uwe Furtner

Heiko,
this issue looks still unfixed and is somewhat annoying.
Im unable to use show_boot_progress with this weak definition in include/common.h.
Simply re-defining the prototype in my local repo is not reasonable.
How did you solve it ?
Regards, André
On Wed, 2008-12-10 at 08:27 +0100, Heiko Schocher wrote:
Hello Andre,
Andre Schwarz wrote:
All,
can someone tell me why the board specific function "void show_boot_progress(int arg)" is no longer called (at least on MPC5200).
Of course this line is present in the board config :
#define CONFIG_SHOW_BOOT_PROGRESS 1
What have I missed ?
Nothing! Good catch. I tried it on a mpc8xx and on a mpx82xx based board and it also didnt worked :-(
Can you try the following patch? (This patch solved it on my boards ...)
thanks, Heiko
[PATCH] all platforms: make show_boot_progress () again working.
Signed-off-by: Heiko Schocher hs@denx.de
include/common.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/common.h b/include/common.h index 5968036..6583f00 100644 --- a/include/common.h +++ b/include/common.h @@ -693,7 +693,7 @@ int pcmcia_init (void); /*
- Board-specific Platform code can reimplement show_boot_progress ()
if needed */ -void __attribute__((weak)) show_boot_progress (int val); +void show_boot_progress (int val);
#ifdef CONFIG_INIT_CRITICAL #error CONFIG_INIT_CRITICAL is deprecated!
MATRIX VISION GmbH, Talstra�e 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Gesch�ftsf�hrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Hans-Joachim Reich

Hello André,
sorry for the late reply, I was on vacation ...
André Schwarz wrote:
this issue looks still unfixed and is somewhat annoying.
Seems that this fix didn;t find the way in mainline.
Im unable to use show_boot_progress with this weak definition in include/common.h.
Simply re-defining the prototype in my local repo is not reasonable.
Hmm... as I can remember, the patch I posted solved it on my boards on which I tried it ...
How did you solve it ?
With the Patch I posted ... I try it with actual code tomorrow again ...
bye Heiko

On Sun, 2009-06-28 at 11:09 +0200, Heiko Schocher wrote:
Hello André,
sorry for the late reply, I was on vacation ...
André Schwarz wrote:
this issue looks still unfixed and is somewhat annoying.
Seems that this fix didn;t find the way in mainline.
What fix ? removing the _weak_ attribute ?
Im unable to use show_boot_progress with this weak definition in include/common.h.
Simply re-defining the prototype in my local repo is not reasonable.
Hmm... as I can remember, the patch I posted solved it on my boards on which I tried it ...
removing the weak definition works for me ... but of course I don't want to need an extra patch. It should simply work out-of-the-box.
How did you solve it ?
With the Patch I posted ... I try it with actual code tomorrow again ...
yes, please.
Regards, André
bye Heiko
MATRIX VISION GmbH, Talstra�e 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Gesch�ftsf�hrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Hans-Joachim Reich

Hello André,
André Schwarz wrote:
On Sun, 2009-06-28 at 11:09 +0200, Heiko Schocher wrote:
Hello André,
sorry for the late reply, I was on vacation ...
André Schwarz wrote:
this issue looks still unfixed and is somewhat annoying.
Seems that this fix didn;t find the way in mainline.
What fix ? removing the _weak_ attribute ?
Yep.
Im unable to use show_boot_progress with this weak definition in include/common.h.
Simply re-defining the prototype in my local repo is not reasonable.
Hmm... as I can remember, the patch I posted solved it on my boards on which I tried it ...
removing the weak definition works for me ... but of course I don't want to need an extra patch. It should simply work out-of-the-box.
So, the only problem is/was, that this patch didn;t found his way in mainline ... Wolfgang, could you please pick up this patch, when you are back from vacation?
Thanks
bye Heiko

Dear Heiko,
In message 4A48948D.7010806@denx.de you wrote:
removing the weak definition works for me ... but of course I don't want to need an extra patch. It should simply work out-of-the-box.
So, the only problem is/was, that this patch didn;t found his way in mainline ... Wolfgang, could you please pick up this patch, when you are back from vacation?
Ummm... seems I lost traces of this patch. Do you have a reference for me (date and subject, or URL to ML archive or similar)?
Thanks.
Best regards,
Wolfgang Denk

Hello Wolfgang,
Wolfgang Denk wrote:
In message 4A48948D.7010806@denx.de you wrote:
removing the weak definition works for me ... but of course I don't want to need an extra patch. It should simply work out-of-the-box.
So, the only problem is/was, that this patch didn;t found his way in mainline ... Wolfgang, could you please pick up this patch, when you are back from vacation?
Ummm... seems I lost traces of this patch. Do you have a reference for me (date and subject, or URL to ML archive or similar)?
http://lists.denx.de/pipermail/u-boot/2008-December/044556.html
bye Heiko

Dear Heiko Schocher,
In message 493F6F45.5060205@invitel.hu you wrote:
[PATCH] all platforms: make show_boot_progress () again working.
Signed-off-by: Heiko Schocher hs@denx.de
Applied, but...
include/common.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/common.h b/include/common.h index 5968036..6583f00 100644 --- a/include/common.h +++ b/include/common.h @@ -693,7 +693,7 @@ int pcmcia_init (void); /*
- Board-specific Platform code can reimplement show_boot_progress ()
if needed
^^^^^^^^^^^^^^^^^
... your patch was line-wrapped and I had to manually apply it.
Best regards,
Wolfgang Denk

Hello Wolfgang,
Wolfgang Denk wrote:
In message 493F6F45.5060205@invitel.hu you wrote:
[PATCH] all platforms: make show_boot_progress () again working.
Signed-off-by: Heiko Schocher hs@denx.de
Applied, but...
include/common.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/common.h b/include/common.h index 5968036..6583f00 100644 --- a/include/common.h +++ b/include/common.h @@ -693,7 +693,7 @@ int pcmcia_init (void); /*
- Board-specific Platform code can reimplement show_boot_progress ()
if needed
^^^^^^^^^^^^^^^^^
... your patch was line-wrapped and I had to manually apply it.
Ups, sorry. Seems at this time I made this patch, I didn;t had setup correct my mailer. Thanks for fixing.
bye Heiko
participants (6)
-
Andre Schwarz
-
André Schwarz
-
Graeme Russ
-
Heiko Schocher
-
Heiko Schocher
-
Wolfgang Denk