All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] ARM LED weak symbols
@ 2009-11-04  0:00 Tom Rix
  2009-11-04  0:00 ` [U-Boot] [PATCH 1/2] ARM Conditionally compile board LED functions Tom Rix
  0 siblings, 1 reply; 21+ messages in thread
From: Tom Rix @ 2009-11-04  0:00 UTC (permalink / raw)
  To: u-boot


As discussed earlier today

http://lists.denx.de/pipermail/u-boot/2009-November/063711.html

This is the patch to conditionally compile the ARM board LED
functions.

It was regression tested with MAKEALL arm using the 
Code Sourcery 2009 compiler and my roll-your-own gcc-4.4.2

Both patches were runtested with the Code Sourcery compiler on Zoom2.

Since this is a common patch, I would like to see it
tested on other boards (hint).  

I will wait till tomorrow for comments and then push it
to arm/next.

Tom
 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [U-Boot] [PATCH 1/2] ARM Conditionally compile board LED functions
  2009-11-04  0:00 [U-Boot] ARM LED weak symbols Tom Rix
@ 2009-11-04  0:00 ` Tom Rix
  2009-11-04  0:00   ` [U-Boot] [PATCH 2/2] ARM: fix build error with gcc-4.4.2 about inline function declared weak Tom Rix
  2009-11-05 20:24   ` [U-Boot] [PATCH 1/2] ARM Conditionally compile board LED functions Wolfgang Denk
  0 siblings, 2 replies; 21+ messages in thread
From: Tom Rix @ 2009-11-04  0:00 UTC (permalink / raw)
  To: u-boot

The ARM board LED functions are defined as weak.
They add a size overhead if they are not used.

Now they are only defined if CONFIG_STATUS_LED is also defined.

The arm920t and arm926ejs _start function calls these LED functions

	bl	coloured_LED_init
	bl	red_LED_on

In general, what happens is they call into the weak
stubs.  Only if the cpu/board provides an overriding
function do these calls cause anything meaningful to
happen.

Now this noop case is removed and these LED functions are excuted
only when CONFIG_STATUS_LED is defined

Signed-off-by: Tom Rix <Tom.Rix@windriver.com>
---
 cpu/arm920t/start.S   |    4 ++--
 cpu/arm926ejs/start.S |    4 ++--
 lib_arm/board.c       |    2 ++
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/cpu/arm920t/start.S b/cpu/arm920t/start.S
index 114427a..e05ebf3 100644
--- a/cpu/arm920t/start.S
+++ b/cpu/arm920t/start.S
@@ -113,10 +113,10 @@ start_code:
 	bic	r0, r0, #0x1f
 	orr	r0, r0, #0xd3
 	msr	cpsr, r0
-
+#ifdef CONFIG_STATUS_LED
 	bl	coloured_LED_init
 	bl	red_LED_on
-
+#endif
 #if	defined(CONFIG_AT91RM9200DK) || defined(CONFIG_AT91RM9200EK)
 	/*
 	 * relocate exception table
diff --git a/cpu/arm926ejs/start.S b/cpu/arm926ejs/start.S
index 4421b6a..117ffb1 100644
--- a/cpu/arm926ejs/start.S
+++ b/cpu/arm926ejs/start.S
@@ -183,10 +183,10 @@ clbss_l:str	r2, [r0]		/* clear loop...                    */
 	add	r0, r0, #4
 	cmp	r0, r1
 	ble	clbss_l
-
+#ifdef CONFIG_STATUS_LED
 	bl coloured_LED_init
 	bl red_LED_on
-
+#endif
 	ldr	pc, _start_armboot
 
 _start_armboot:
diff --git a/lib_arm/board.c b/lib_arm/board.c
index 5e3d7f6..28b15da 100644
--- a/lib_arm/board.c
+++ b/lib_arm/board.c
@@ -87,6 +87,7 @@ extern void rtl8019_get_enetaddr (uchar * addr);
 #endif
 
 
+#ifdef CONFIG_STATUS_LED
 /************************************************************************
  * Coloured LED functionality
  ************************************************************************
@@ -110,6 +111,7 @@ void inline __blue_LED_on(void) {}
 void inline blue_LED_on(void)__attribute__((weak, alias("__blue_LED_on")));
 void inline __blue_LED_off(void) {}
 void inline blue_LED_off(void)__attribute__((weak, alias("__blue_LED_off")));
+#endif
 
 /************************************************************************
  * Init Utilities							*
-- 
1.6.0.6

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [U-Boot] [PATCH 2/2] ARM: fix build error with gcc-4.4.2 about inline function declared weak
  2009-11-04  0:00 ` [U-Boot] [PATCH 1/2] ARM Conditionally compile board LED functions Tom Rix
@ 2009-11-04  0:00   ` Tom Rix
  2009-11-04  0:34     ` [U-Boot] multiple partitions in mtdparts myuboot at fastmail.fm
  2009-11-05 20:19     ` [U-Boot] [PATCH 2/2] ARM: fix build error with gcc-4.4.2 about inline function declared weak Wolfgang Denk
  2009-11-05 20:24   ` [U-Boot] [PATCH 1/2] ARM Conditionally compile board LED functions Wolfgang Denk
  1 sibling, 2 replies; 21+ messages in thread
From: Tom Rix @ 2009-11-04  0:00 UTC (permalink / raw)
  To: u-boot

From: Abdoulaye Walsimou Gaye <walsimou@walsimou.com>

This patch fix build error with gcc-4.4.2 about inline function
declared weak, see below:
board.c:96: error: inline function 'coloured_LED_init' cannot be declared weak
board.c:98: error: inline function 'red_LED_on' cannot be declared weak
board.c:100: error: inline function 'red_LED_off' cannot be declared weak
board.c:102: error: inline function 'green_LED_on' cannot be declared weak
board.c:104: error: inline function 'green_LED_off' cannot be declared weak
board.c:106: error: inline function 'yellow_LED_on' cannot be declared weak
board.c:108: error: inline function 'yellow_LED_off' cannot be declared weak
board.c:110: error: inline function 'blue_LED_on' cannot be declared weak
board.c:112: error: inline function 'blue_LED_off' cannot be declared weak
make[1]: *** [board.o] Error 1

Signed-off-by: Abdoulaye Walsimou Gaye <walsimou@walsimou.com>
---
 lib_arm/board.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/lib_arm/board.c b/lib_arm/board.c
index 28b15da..2665093 100644
--- a/lib_arm/board.c
+++ b/lib_arm/board.c
@@ -94,23 +94,23 @@ extern void rtl8019_get_enetaddr (uchar * addr);
  * May be supplied by boards if desired
  */
 void inline __coloured_LED_init (void) {}
-void inline coloured_LED_init (void) __attribute__((weak, alias("__coloured_LED_init")));
+void coloured_LED_init(void)__attribute__((weak, alias("__coloured_LED_init")));
 void inline __red_LED_on (void) {}
-void inline red_LED_on (void) __attribute__((weak, alias("__red_LED_on")));
+void red_LED_on(void) __attribute__((weak, alias("__red_LED_on")));
 void inline __red_LED_off(void) {}
-void inline red_LED_off(void)	     __attribute__((weak, alias("__red_LED_off")));
+void red_LED_off(void)__attribute__((weak, alias("__red_LED_off")));
 void inline __green_LED_on(void) {}
-void inline green_LED_on(void) __attribute__((weak, alias("__green_LED_on")));
+void green_LED_on(void) __attribute__((weak, alias("__green_LED_on")));
 void inline __green_LED_off(void) {}
-void inline green_LED_off(void)__attribute__((weak, alias("__green_LED_off")));
+void green_LED_off(void)__attribute__((weak, alias("__green_LED_off")));
 void inline __yellow_LED_on(void) {}
-void inline yellow_LED_on(void)__attribute__((weak, alias("__yellow_LED_on")));
+void yellow_LED_on(void)__attribute__((weak, alias("__yellow_LED_on")));
 void inline __yellow_LED_off(void) {}
-void inline yellow_LED_off(void)__attribute__((weak, alias("__yellow_LED_off")));
+void yellow_LED_off(void)__attribute__((weak, alias("__yellow_LED_off")));
 void inline __blue_LED_on(void) {}
-void inline blue_LED_on(void)__attribute__((weak, alias("__blue_LED_on")));
+void blue_LED_on(void)__attribute__((weak, alias("__blue_LED_on")));
 void inline __blue_LED_off(void) {}
-void inline blue_LED_off(void)__attribute__((weak, alias("__blue_LED_off")));
+void blue_LED_off(void)__attribute__((weak, alias("__blue_LED_off")));
 #endif
 
 /************************************************************************
-- 
1.6.0.6

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [U-Boot] multiple partitions in mtdparts
  2009-11-04  0:00   ` [U-Boot] [PATCH 2/2] ARM: fix build error with gcc-4.4.2 about inline function declared weak Tom Rix
@ 2009-11-04  0:34     ` myuboot at fastmail.fm
  2009-11-04  7:14       ` Dieter Kiermaier
  2009-11-05 20:19     ` [U-Boot] [PATCH 2/2] ARM: fix build error with gcc-4.4.2 about inline function declared weak Wolfgang Denk
  1 sibling, 1 reply; 21+ messages in thread
From: myuboot at fastmail.fm @ 2009-11-04  0:34 UTC (permalink / raw)
  To: u-boot

Hi, 

I am trying to define multiple partitions using mtdparts variable. But I
can't get it set automatically right.

I defined the set_bootargs as the following:
set_bootargs=setenv bootargs root=/dev/mtdblock2 rootfs=cramfs,jffs2
mtdparts=ph
ysmap-flash.0:256k(u-boot),256k(u-boot-env),7936k(root1),7936k(root2);physmap-fl
ash.1:16384k(data) console=ttyS0,115200n8

But when I run set_bootargs, I got the following error - 
# run set_bootargs
Unknown command 'physmap-flash.1:16384k(data)' - try 'help'
Can someone help me with the correct format on mtdparts? thanks a lot.


====================================================
mtdparts=mtdparts=physmap-flash.0:256k(u-boot),256k(u-boot-env),7936k(root1),793
6k(root2);physmap-flash.1:16384k(data)
boot=run set_bootargs;chpart nor0,${boot_device};fsload
boot/uImage;bootm
mtdids=nor0=physmap-flash.0,nor1=physmap-flash.1
partition=nor0,2
mtddevnum=2
mtddevname=root1
set_bootargs=setenv bootargs root=/dev/mtdblock2 rootfs=cramfs,jffs2
mtdparts=ph
ysmap-flash.0:256k(u-boot),256k(u-boot-env),7936k(root1),7936k(root2);physmap-fl
ash.1:16384k(data) console=ttyS0,115200n8
bootargs=root=/dev/mtdblock2 rootfs=cramfs,jffs2
mtdparts=physmap-flash.0:256k(u
-boot),256k(u-boot-env),7936k(root1),7936k(root2)

Environment size: 847/262140 bytes
# run set_bootargs
Unknown command 'physmap-flash.1:16384k(data)' - try 'help'
#

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [U-Boot] multiple partitions in mtdparts
  2009-11-04  0:34     ` [U-Boot] multiple partitions in mtdparts myuboot at fastmail.fm
@ 2009-11-04  7:14       ` Dieter Kiermaier
  2009-11-04 15:35         ` myuboot at fastmail.fm
  0 siblings, 1 reply; 21+ messages in thread
From: Dieter Kiermaier @ 2009-11-04  7:14 UTC (permalink / raw)
  To: u-boot

Am Mittwoch 04 November 2009 01:34:53 schrieb myuboot at fastmail.fm:
> Hi, 
> 
> I am trying to define multiple partitions using mtdparts variable. But I
> can't get it set automatically right.
> 
> I defined the set_bootargs as the following:
> set_bootargs=setenv bootargs root=/dev/mtdblock2 rootfs=cramfs,jffs2
> mtdparts=ph
> ysmap-flash.0:256k(u-boot),256k(u-boot-env),7936k(root1),7936k(root2);physmap-fl
> ash.1:16384k(data) console=ttyS0,115200n8
> 
> But when I run set_bootargs, I got the following error - 
> # run set_bootargs
> Unknown command 'physmap-flash.1:16384k(data)' - try 'help'
> Can someone help me with the correct format on mtdparts? thanks a lot.
> 

I don't know the exact format but it looks like the ";" between (root2);physmap-flash1 causes your error?

Dieter

> 
> ====================================================
> mtdparts=mtdparts=physmap-flash.0:256k(u-boot),256k(u-boot-env),7936k(root1),793
> 6k(root2);physmap-flash.1:16384k(data)
> boot=run set_bootargs;chpart nor0,${boot_device};fsload
> boot/uImage;bootm
> mtdids=nor0=physmap-flash.0,nor1=physmap-flash.1
> partition=nor0,2
> mtddevnum=2
> mtddevname=root1
> set_bootargs=setenv bootargs root=/dev/mtdblock2 rootfs=cramfs,jffs2
> mtdparts=ph
> ysmap-flash.0:256k(u-boot),256k(u-boot-env),7936k(root1),7936k(root2);physmap-fl
> ash.1:16384k(data) console=ttyS0,115200n8
> bootargs=root=/dev/mtdblock2 rootfs=cramfs,jffs2
> mtdparts=physmap-flash.0:256k(u
> -boot),256k(u-boot-env),7936k(root1),7936k(root2)
> 
> Environment size: 847/262140 bytes
> # run set_bootargs
> Unknown command 'physmap-flash.1:16384k(data)' - try 'help'
> #
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [U-Boot] multiple partitions in mtdparts
  2009-11-04  7:14       ` Dieter Kiermaier
@ 2009-11-04 15:35         ` myuboot at fastmail.fm
  2009-11-04 15:57           ` Wolfgang Denk
  0 siblings, 1 reply; 21+ messages in thread
From: myuboot at fastmail.fm @ 2009-11-04 15:35 UTC (permalink / raw)
  To: u-boot

I thought that is a legal definition according to u-boot document
section "5.9.3.5. mtdparts - define a Linux compatible MTD partition
scheme" at http://www.denx.de/wiki/view/DULG/UBootCmdGroupFlash

mtdparts=mtdparts=<mtd-def>[;<mtd-def>...]

Thanks.

On Wed, 04 Nov 2009 08:14 +0100, "Dieter Kiermaier"
<dk-arm-linux@gmx.de> wrote:
> Am Mittwoch 04 November 2009 01:34:53 schrieb myuboot at fastmail.fm:
> > Hi, 
> > 
> > I am trying to define multiple partitions using mtdparts variable. But I
> > can't get it set automatically right.
> > 
> > I defined the set_bootargs as the following:
> > set_bootargs=setenv bootargs root=/dev/mtdblock2 rootfs=cramfs,jffs2
> > mtdparts=ph
> > ysmap-flash.0:256k(u-boot),256k(u-boot-env),7936k(root1),7936k(root2);physmap-fl
> > ash.1:16384k(data) console=ttyS0,115200n8
> > 
> > But when I run set_bootargs, I got the following error - 
> > # run set_bootargs
> > Unknown command 'physmap-flash.1:16384k(data)' - try 'help'
> > Can someone help me with the correct format on mtdparts? thanks a lot.
> > 
> 
> I don't know the exact format but it looks like the ";" between
> (root2);physmap-flash1 causes your error?
> 
> Dieter
> 
> > 
> > ====================================================
> > mtdparts=mtdparts=physmap-flash.0:256k(u-boot),256k(u-boot-env),7936k(root1),793
> > 6k(root2);physmap-flash.1:16384k(data)
> > boot=run set_bootargs;chpart nor0,${boot_device};fsload
> > boot/uImage;bootm
> > mtdids=nor0=physmap-flash.0,nor1=physmap-flash.1
> > partition=nor0,2
> > mtddevnum=2
> > mtddevname=root1
> > set_bootargs=setenv bootargs root=/dev/mtdblock2 rootfs=cramfs,jffs2
> > mtdparts=ph
> > ysmap-flash.0:256k(u-boot),256k(u-boot-env),7936k(root1),7936k(root2);physmap-fl
> > ash.1:16384k(data) console=ttyS0,115200n8
> > bootargs=root=/dev/mtdblock2 rootfs=cramfs,jffs2
> > mtdparts=physmap-flash.0:256k(u
> > -boot),256k(u-boot-env),7936k(root1),7936k(root2)
> > 
> > Environment size: 847/262140 bytes
> > # run set_bootargs
> > Unknown command 'physmap-flash.1:16384k(data)' - try 'help'
> > #
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
> > 
> 
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [U-Boot] multiple partitions in mtdparts
  2009-11-04 15:35         ` myuboot at fastmail.fm
@ 2009-11-04 15:57           ` Wolfgang Denk
  0 siblings, 0 replies; 21+ messages in thread
From: Wolfgang Denk @ 2009-11-04 15:57 UTC (permalink / raw)
  To: u-boot

Dear myuboot at fastmail.fm,

how about providing a realname?

And please read http://www.netmeister.org/news/learn2quote.html

Do not top post / full quote!


In message <1257348926.15792.1343548151@webmail.messagingengine.com> you wrote:
> I thought that is a legal definition according to u-boot document
> section "5.9.3.5. mtdparts - define a Linux compatible MTD partition
> scheme" at http://www.denx.de/wiki/view/DULG/UBootCmdGroupFlash
> 
> mtdparts=mtdparts=<mtd-def>[;<mtd-def>...]

Yes, it is. But the shell is interpreting the semicolon so you have to
escape 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
All men should freely use those seven words which have the  power  to
make any marriage run smoothly: You know dear, you may be right.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [U-Boot] [PATCH 2/2] ARM: fix build error with gcc-4.4.2 about inline function declared weak
  2009-11-04  0:00   ` [U-Boot] [PATCH 2/2] ARM: fix build error with gcc-4.4.2 about inline function declared weak Tom Rix
  2009-11-04  0:34     ` [U-Boot] multiple partitions in mtdparts myuboot at fastmail.fm
@ 2009-11-05 20:19     ` Wolfgang Denk
  2009-11-05 20:39       ` Tom
  1 sibling, 1 reply; 21+ messages in thread
From: Wolfgang Denk @ 2009-11-05 20:19 UTC (permalink / raw)
  To: u-boot

Dear Tom Rix,

In message <1257292804-10612-3-git-send-email-Tom.Rix@windriver.com> you wrote:
> From: Abdoulaye Walsimou Gaye <walsimou@walsimou.com>
> 
> This patch fix build error with gcc-4.4.2 about inline function
> declared weak, see below:
> board.c:96: error: inline function 'coloured_LED_init' cannot be declared weak
> board.c:98: error: inline function 'red_LED_on' cannot be declared weak
> board.c:100: error: inline function 'red_LED_off' cannot be declared weak
> board.c:102: error: inline function 'green_LED_on' cannot be declared weak
> board.c:104: error: inline function 'green_LED_off' cannot be declared weak
> board.c:106: error: inline function 'yellow_LED_on' cannot be declared weak
> board.c:108: error: inline function 'yellow_LED_off' cannot be declared weak
> board.c:110: error: inline function 'blue_LED_on' cannot be declared weak
> board.c:112: error: inline function 'blue_LED_off' cannot be declared weak
> make[1]: *** [board.o] Error 1
> 
> Signed-off-by: Abdoulaye Walsimou Gaye <walsimou@walsimou.com>

What is this? Are you sure this patch is properly attributed? Was
Abdoulaye Walsimou Gaye really the first to submit this patch? 

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
Quote from a recent meeting:   "We are going to continue having these
meetings everyday until I find out why no work is getting done."

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [U-Boot] [PATCH 1/2] ARM Conditionally compile board LED functions
  2009-11-04  0:00 ` [U-Boot] [PATCH 1/2] ARM Conditionally compile board LED functions Tom Rix
  2009-11-04  0:00   ` [U-Boot] [PATCH 2/2] ARM: fix build error with gcc-4.4.2 about inline function declared weak Tom Rix
@ 2009-11-05 20:24   ` Wolfgang Denk
  2009-11-05 20:47     ` Tom
  1 sibling, 1 reply; 21+ messages in thread
From: Wolfgang Denk @ 2009-11-05 20:24 UTC (permalink / raw)
  To: u-boot

Dear Tom Rix,

In message <1257292804-10612-2-git-send-email-Tom.Rix@windriver.com> you wrote:
> The ARM board LED functions are defined as weak.
> They add a size overhead if they are not used.
> 
> Now they are only defined if CONFIG_STATUS_LED is also defined.
> 
> The arm920t and arm926ejs _start function calls these LED functions
> 
> 	bl	coloured_LED_init
> 	bl	red_LED_on
> 
> In general, what happens is they call into the weak
> stubs.  Only if the cpu/board provides an overriding
> function do these calls cause anything meaningful to
> happen.
> 
> Now this noop case is removed and these LED functions are excuted
> only when CONFIG_STATUS_LED is defined

I don't get it. We use "weak" to avoid #ifdef's, and you insert new
#ifdef's?   That seems to be the wrong approach to me.

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
"What if" is a trademark of Hewlett Packard, so stop using it in your
sentences without permission, or risk being sued.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [U-Boot] [PATCH 2/2] ARM: fix build error with gcc-4.4.2 about inline function declared weak
  2009-11-05 20:19     ` [U-Boot] [PATCH 2/2] ARM: fix build error with gcc-4.4.2 about inline function declared weak Wolfgang Denk
@ 2009-11-05 20:39       ` Tom
  2009-11-05 22:38         ` Wolfgang Denk
  0 siblings, 1 reply; 21+ messages in thread
From: Tom @ 2009-11-05 20:39 UTC (permalink / raw)
  To: u-boot

Wolfgang Denk wrote:
> Dear Tom Rix,
> 
> In message <1257292804-10612-3-git-send-email-Tom.Rix@windriver.com> you wrote:
>> From: Abdoulaye Walsimou Gaye <walsimou@walsimou.com>
>>
>> This patch fix build error with gcc-4.4.2 about inline function
>> declared weak, see below:
>> board.c:96: error: inline function 'coloured_LED_init' cannot be declared weak
>> board.c:98: error: inline function 'red_LED_on' cannot be declared weak
>> board.c:100: error: inline function 'red_LED_off' cannot be declared weak
>> board.c:102: error: inline function 'green_LED_on' cannot be declared weak
>> board.c:104: error: inline function 'green_LED_off' cannot be declared weak
>> board.c:106: error: inline function 'yellow_LED_on' cannot be declared weak
>> board.c:108: error: inline function 'yellow_LED_off' cannot be declared weak
>> board.c:110: error: inline function 'blue_LED_on' cannot be declared weak
>> board.c:112: error: inline function 'blue_LED_off' cannot be declared weak
>> make[1]: *** [board.o] Error 1
>>
>> Signed-off-by: Abdoulaye Walsimou Gaye <walsimou@walsimou.com>
> 
> What is this? Are you sure this patch is properly attributed? Was
> Abdoulaye Walsimou Gaye really the first to submit this patch? 
> 
> Best regards,
> 
> Wolfgang Denk
> 
Would you like the older one ?
As you said, they looked the same.

Tom

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [U-Boot] [PATCH 1/2] ARM Conditionally compile board LED functions
  2009-11-05 20:24   ` [U-Boot] [PATCH 1/2] ARM Conditionally compile board LED functions Wolfgang Denk
@ 2009-11-05 20:47     ` Tom
  2009-11-05 22:43       ` Wolfgang Denk
  0 siblings, 1 reply; 21+ messages in thread
From: Tom @ 2009-11-05 20:47 UTC (permalink / raw)
  To: u-boot

Wolfgang Denk wrote:
> Dear Tom Rix,
> 
> In message <1257292804-10612-2-git-send-email-Tom.Rix@windriver.com> you wrote:
>> The ARM board LED functions are defined as weak.
>> They add a size overhead if they are not used.
>>
>> Now they are only defined if CONFIG_STATUS_LED is also defined.
>>
>> The arm920t and arm926ejs _start function calls these LED functions
>>
>> 	bl	coloured_LED_init
>> 	bl	red_LED_on
>>
>> In general, what happens is they call into the weak
>> stubs.  Only if the cpu/board provides an overriding
>> function do these calls cause anything meaningful to
>> happen.
>>
>> Now this noop case is removed and these LED functions are excuted
>> only when CONFIG_STATUS_LED is defined
> 
> I don't get it. We use "weak" to avoid #ifdef's, and you insert new
> #ifdef's?   That seems to be the wrong approach to me.
> 

The arguments for using weak are getting weak :P

Using weak is less relevant with the #ifdef's
The benefit now is that boards that use the led functions do
not have to define all of them.

I am open to ideas on how to kill weak off completely.

Has a general led driver layer ever been proposed ?
Something to convert status led for a mixture of #defines and weak
symbols to something that had a register function and some
file_ops ?

Tom

Tom

> Best regards,
> 
> Wolfgang Denk
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [U-Boot] [PATCH 2/2] ARM: fix build error with gcc-4.4.2 about inline function declared weak
  2009-11-05 20:39       ` Tom
@ 2009-11-05 22:38         ` Wolfgang Denk
  2009-11-10 19:34           ` Tom
  2009-11-11 16:53           ` [U-Boot] [PATCH 2/2] ARM: fix build error with gcc-4.4.2 about inline function declared weak Gaye Abdoulaye Walsimou
  0 siblings, 2 replies; 21+ messages in thread
From: Wolfgang Denk @ 2009-11-05 22:38 UTC (permalink / raw)
  To: u-boot

Dear Tom,

In message <4AF33815.4030101@windriver.com> you wrote:
>
> >> Signed-off-by: Abdoulaye Walsimou Gaye <walsimou@walsimou.com>
> > 
> > What is this? Are you sure this patch is properly attributed? Was
> > Abdoulaye Walsimou Gaye really the first to submit this patch? 
> > 
> > Best regards,
> > 
> > Wolfgang Denk
> > 
> Would you like the older one ?
> As you said, they looked the same.

It does not matter what I like. What matters is who is to be credited
for this patch, and in my opinion this has to be the first to submit
it.

You don't get any credits any more today for inventing a light bulb,
either.

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
Stupidity, like virtue, is its own reward.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [U-Boot] [PATCH 1/2] ARM Conditionally compile board LED functions
  2009-11-05 20:47     ` Tom
@ 2009-11-05 22:43       ` Wolfgang Denk
  2009-11-05 23:33         ` Tom
  2009-11-12 15:17         ` Alessandro Rubini
  0 siblings, 2 replies; 21+ messages in thread
From: Wolfgang Denk @ 2009-11-05 22:43 UTC (permalink / raw)
  To: u-boot

Dear Tom,

In message <4AF339E1.9060809@windriver.com> you wrote:
>
> The arguments for using weak are getting weak :P

:-P

> Using weak is less relevant with the #ifdef's

But it's the wrong direction your heading. We should get rid of
#ifdef's, not add new ones.

With #ifdef's, you have different versions of the code, which
increases the multitude of configurations that actually need to be
tested. With weak, you have one version of the code only. 

> The benefit now is that boards that use the led functions do
> not have to define all of them.

That's just an indication of a broken implementation.

Normally you would provide the weak functions in a central place,
where any board configuration which wants can overwrite them, or not.

> I am open to ideas on how to kill weak off completely.

You got it wrong.

We want to kill off the #ifdef's.

> Has a general led driver layer ever been proposed ?
> Something to convert status led for a mixture of #defines and weak
> symbols to something that had a register function and some
> file_ops ?

We use status LEDs on many boards, without real issues. It's only AT91
which suffers from this mess.

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 is nothing new under the sun, but there are lots of old things
we don't know yet."                                  - Ambrose Bierce

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [U-Boot] [PATCH 1/2] ARM Conditionally compile board LED functions
  2009-11-05 22:43       ` Wolfgang Denk
@ 2009-11-05 23:33         ` Tom
  2009-11-12 15:17         ` Alessandro Rubini
  1 sibling, 0 replies; 21+ messages in thread
From: Tom @ 2009-11-05 23:33 UTC (permalink / raw)
  To: u-boot

Wolfgang Denk wrote:
> Dear Tom,
> 
> In message <4AF339E1.9060809@windriver.com> you wrote:
>> The arguments for using weak are getting weak :P
> 
> :-P
> 
>> Using weak is less relevant with the #ifdef's
> 
> But it's the wrong direction your heading. We should get rid of
> #ifdef's, not add new ones.
> 
> With #ifdef's, you have different versions of the code, which
> increases the multitude of configurations that actually need to be
> tested. With weak, you have one version of the code only. 
> 

To use the status led api, you have to define CONFIG_STATUS_LED anyway.
I did not think this added to the configuration space.

>> The benefit now is that boards that use the led functions do
>> not have to define all of them.
> 
> That's just an indication of a broken implementation.
> 
> Normally you would provide the weak functions in a central place,
> where any board configuration which wants can overwrite them, or not.
> 
>> I am open to ideas on how to kill weak off completely.
> 
> You got it wrong.
> 
> We want to kill off the #ifdef's.
> 

My vector is obviously pointing in the wrong direction..

>> Has a general led driver layer ever been proposed ?
>> Something to convert status led for a mixture of #defines and weak
>> symbols to something that had a register function and some
>> file_ops ?
> 
> We use status LEDs on many boards, without real issues. It's only AT91
> which suffers from this mess.
> 

I withdraw this patch.
I will rethink this and come up with something better.

Tom

> Best regards,
> 
> Wolfgang Denk
> 

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [U-Boot] [PATCH 2/2] ARM: fix build error with gcc-4.4.2 about inline function declared weak
  2009-11-05 22:38         ` Wolfgang Denk
@ 2009-11-10 19:34           ` Tom
  2009-11-10 22:45             ` Wolfgang Denk
  2009-11-11 16:53           ` [U-Boot] [PATCH 2/2] ARM: fix build error with gcc-4.4.2 about inline function declared weak Gaye Abdoulaye Walsimou
  1 sibling, 1 reply; 21+ messages in thread
From: Tom @ 2009-11-10 19:34 UTC (permalink / raw)
  To: u-boot

Wolfgang Denk wrote:
> Dear Tom,
> 
> In message <4AF33815.4030101@windriver.com> you wrote:
>>>> Signed-off-by: Abdoulaye Walsimou Gaye <walsimou@walsimou.com>
>>> What is this? Are you sure this patch is properly attributed? Was
>>> Abdoulaye Walsimou Gaye really the first to submit this patch? 
>>>
>>> Best regards,
>>>
>>> Wolfgang Denk
>>>
>> Would you like the older one ?
>> As you said, they looked the same.
> 
> It does not matter what I like. What matters is who is to be credited
> for this patch, and in my opinion this has to be the first to submit
> it.
> 
> You don't get any credits any more today for inventing a light bulb,
> either.
> 
> Best regards,
> 
> Wolfgang Denk
> 

I have pushed the earlier patch to arm/next.
Tom


Author: Ron Lee <ron@debian.org>
Date:   Wed Aug 5 20:14:01 2009 +0200

     ARM Don't inline weak symbols

     ------------------------------------------------------------------------

     GCC 4.4 complains about this now.

     Signed-off-by: Ron Lee <ron@debian.org>

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [U-Boot] [PATCH 2/2] ARM: fix build error with gcc-4.4.2 about inline function declared weak
  2009-11-10 19:34           ` Tom
@ 2009-11-10 22:45             ` Wolfgang Denk
  2009-11-12  0:43               ` [U-Boot] ARM pull request Tom
  0 siblings, 1 reply; 21+ messages in thread
From: Wolfgang Denk @ 2009-11-10 22:45 UTC (permalink / raw)
  To: u-boot

Dear Tom,

In message <4AF9C04F.1020509@windriver.com> you wrote:
>
> I have pushed the earlier patch to arm/next.

Thnaks, but this being a clear bug fix I think we should include this
in the upcoming release?

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
I wish I had a bronze torc for every user who didn't read the manual.
                             - Terry Pratchett, _The Light Fantastic_

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [U-Boot] [PATCH 2/2] ARM: fix build error with gcc-4.4.2 about inline function declared weak
  2009-11-05 22:38         ` Wolfgang Denk
  2009-11-10 19:34           ` Tom
@ 2009-11-11 16:53           ` Gaye Abdoulaye Walsimou
  1 sibling, 0 replies; 21+ messages in thread
From: Gaye Abdoulaye Walsimou @ 2009-11-11 16:53 UTC (permalink / raw)
  To: u-boot

Wolfgang Denk wrote:
> Dear Tom,
>
> In message <4AF33815.4030101@windriver.com> you wrote:
>   
>>>> Signed-off-by: Abdoulaye Walsimou Gaye <walsimou@walsimou.com>
>>>>         
>>> What is this? Are you sure this patch is properly attributed? Was
>>> Abdoulaye Walsimou Gaye really the first to submit this patch? 
>>>
>>> Best regards,
>>>
>>> Wolfgang Denk
>>>
>>>       
>> Would you like the older one ?
>> As you said, they looked the same.
>>     
>
> It does not matter what I like. What matters is who is to be credited
> for this patch, and in my opinion this has to be the first to submit
> it.
>
> You don't get any credits any more today for inventing a light bulb,
> either.
>
> Best regards,
>
> Wolfgang Denk
>
>   
Hello all,
May be having tools such as patchwork will avoid people to submit the
same patch.
Recently linux-mips[1] adopted patchwork and it seems useful (for
submitters and maintainers).

Best regards,

[1] http://www.linux-mips.org/archives/linux-mips/2009-11/msg00273.html

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [U-Boot] ARM pull request
  2009-11-10 22:45             ` Wolfgang Denk
@ 2009-11-12  0:43               ` Tom
  2009-11-15 21:39                 ` Wolfgang Denk
  0 siblings, 1 reply; 21+ messages in thread
From: Tom @ 2009-11-12  0:43 UTC (permalink / raw)
  To: u-boot

Wolfgang,

I have cherry picked the weak sym change into arm/master-sync
This is the pull request.
Tom


The following changes since commit 0f365273a6c210e0d82f6dca3994be5283e6bf82:
   Wolfgang Denk (1):
         Merge branch 'master-sync' of git://git.denx.de/u-boot-arm

are available in the git repository at:

   git://git.denx.de/u-boot-arm master-sync

Ron Lee (1):
       ARM Don't inline weak symbols

  lib_arm/board.c |   18 +++++++++---------
  1 files changed, 9 insertions(+), 9 deletions(-)

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [U-Boot] [PATCH 1/2] ARM Conditionally compile board LED functions
  2009-11-05 22:43       ` Wolfgang Denk
  2009-11-05 23:33         ` Tom
@ 2009-11-12 15:17         ` Alessandro Rubini
  2009-11-12 15:59           ` Tom
  1 sibling, 1 reply; 21+ messages in thread
From: Alessandro Rubini @ 2009-11-12 15:17 UTC (permalink / raw)
  To: u-boot

> I withdraw this patch.
> I will rethink this and come up with something better.

I agree weak is better than ifdef. But the led situation on ARM isn't
really pleasant when you look in lib_arm/board.c .

When I proposed a simplification of board.c back on Jul 22 ("[RFC]
arm/board.c: avoid ifdef using weak default functions", I noticed the
led approach and thought it would need cleanup (for example, by moving
out of board.c to led.c or something).  However, the patch was
rejected by JC as he has initcalls as work in progress.

Since we still missing the initcalls (as missing JC), could that patch
be reconsidered? I can rebase if there's any interest.

/alessandro

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [U-Boot] [PATCH 1/2] ARM Conditionally compile board LED functions
  2009-11-12 15:17         ` Alessandro Rubini
@ 2009-11-12 15:59           ` Tom
  0 siblings, 0 replies; 21+ messages in thread
From: Tom @ 2009-11-12 15:59 UTC (permalink / raw)
  To: u-boot

Alessandro Rubini wrote:
>> I withdraw this patch.
>> I will rethink this and come up with something better.
> 
> I agree weak is better than ifdef. But the led situation on ARM isn't
> really pleasant when you look in lib_arm/board.c .
> 
> When I proposed a simplification of board.c back on Jul 22 ("[RFC]
> arm/board.c: avoid ifdef using weak default functions", I noticed the
> led approach and thought it would need cleanup (for example, by moving
> out of board.c to led.c or something).  However, the patch was
> rejected by JC as he has initcalls as work in progress.
> 
> Since we still missing the initcalls (as missing JC), could that patch
> be reconsidered? I can rebase if there's any interest.
>

Yes I am interested.
Please rebase the RFC patch.

Here is the link to the old RFC
http://lists.denx.de/pipermail/u-boot/2009-July/057273.html

Thanks,
Tom


> /alessandro

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [U-Boot] ARM pull request
  2009-11-12  0:43               ` [U-Boot] ARM pull request Tom
@ 2009-11-15 21:39                 ` Wolfgang Denk
  0 siblings, 0 replies; 21+ messages in thread
From: Wolfgang Denk @ 2009-11-15 21:39 UTC (permalink / raw)
  To: u-boot

Dear Tom,

In message <4AFB5A23.7030606@windriver.com> you wrote:
> 
> I have cherry picked the weak sym change into arm/master-sync
> This is the pull request.
> Tom
> 
> 
> The following changes since commit 0f365273a6c210e0d82f6dca3994be5283e6bf82:
>    Wolfgang Denk (1):
>          Merge branch 'master-sync' of git://git.denx.de/u-boot-arm
> 
> are available in the git repository at:
> 
>    git://git.denx.de/u-boot-arm master-sync
> 
> Ron Lee (1):
>        ARM Don't inline weak symbols
> 
>   lib_arm/board.c |   18 +++++++++---------
>   1 files changed, 9 insertions(+), 9 deletions(-)

Applied, 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
In the beginning, I was made. I didn't ask to be made. No one consul-
ted with me or considered my feelings  in  this  matter.  But  if  it
brought  some  passing fancy to some lowly humans as they haphazardly
pranced their way through life's mournful jungle, then so be it.
- Marvin the Paranoid Android

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2009-11-15 21:39 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-04  0:00 [U-Boot] ARM LED weak symbols Tom Rix
2009-11-04  0:00 ` [U-Boot] [PATCH 1/2] ARM Conditionally compile board LED functions Tom Rix
2009-11-04  0:00   ` [U-Boot] [PATCH 2/2] ARM: fix build error with gcc-4.4.2 about inline function declared weak Tom Rix
2009-11-04  0:34     ` [U-Boot] multiple partitions in mtdparts myuboot at fastmail.fm
2009-11-04  7:14       ` Dieter Kiermaier
2009-11-04 15:35         ` myuboot at fastmail.fm
2009-11-04 15:57           ` Wolfgang Denk
2009-11-05 20:19     ` [U-Boot] [PATCH 2/2] ARM: fix build error with gcc-4.4.2 about inline function declared weak Wolfgang Denk
2009-11-05 20:39       ` Tom
2009-11-05 22:38         ` Wolfgang Denk
2009-11-10 19:34           ` Tom
2009-11-10 22:45             ` Wolfgang Denk
2009-11-12  0:43               ` [U-Boot] ARM pull request Tom
2009-11-15 21:39                 ` Wolfgang Denk
2009-11-11 16:53           ` [U-Boot] [PATCH 2/2] ARM: fix build error with gcc-4.4.2 about inline function declared weak Gaye Abdoulaye Walsimou
2009-11-05 20:24   ` [U-Boot] [PATCH 1/2] ARM Conditionally compile board LED functions Wolfgang Denk
2009-11-05 20:47     ` Tom
2009-11-05 22:43       ` Wolfgang Denk
2009-11-05 23:33         ` Tom
2009-11-12 15:17         ` Alessandro Rubini
2009-11-12 15:59           ` Tom

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.