[U-Boot] show_boot_progess @ ppc not working

[U-Boot] show_boot_progess @ ppc not working

[Andre Schwarz]

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

[U-Boot] show_boot_progess @ ppc not working

[Heiko Schocher]

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

[U-Boot] show_boot_progess @ ppc not working

[Andre Schwarz]

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

[U-Boot] show_boot_progess @ ppc not working

[Heiko Schocher]

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!
-- 
1.5.6.1

[U-Boot] show_boot_progess @ ppc not working

[Andre Schwarz]

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

[U-Boot] show_boot_progess @ ppc not working

[Graeme Russ]

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

[U-Boot] show_boot_progess @ ppc not working

[Andre Schwarz]

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

[U-Boot] show_boot_progess @ ppc not working

[André Schwarz]

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

[U-Boot] show_boot_progess @ ppc not working

[Heiko Schocher]

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
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] show_boot_progess @ ppc not working

[André Schwarz]

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

[U-Boot] show_boot_progess @ ppc not working

[Heiko Schocher]

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
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] show_boot_progess @ ppc not working

[Wolfgang Denk]

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

-- 
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
Well, the way I see it, logic is only a way of being ignorant by num-
bers.                                 - Terry Pratchett, _Small Gods_

[U-Boot] show_boot_progess @ ppc not working

[Heiko Schocher]

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
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] show_boot_progess @ ppc not working

[Wolfgang Denk]

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

-- 
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
"Where humor is concerned there are no standards -- no  one  can  say
what is good or bad, although you can be sure that everyone will.
- John Kenneth Galbraith

[U-Boot] show_boot_progess @ ppc not working

[Heiko Schocher]

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
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany