Skip to contentSkip to conversation

omap3: beagle: Fix build warning

PATCH New Sanjeev Premi 7 participants 12 messages Sep 5, 2011 – Sep 7, 2011

lore
Share & tools
lore

Sanjeev Premi

author
#0 lore raw

PATCH 1/1 omap3: beagle: Fix build warning

This patch fixes the warning dure to recent changes to the board configuration:
cmd_i2c.o cmd_i2c.c -c
cmd_i2c.c:109:1: warning: missing braces around initializer cmd_i2c.c:109:1: warning: (near initialization for 'i2c_no_probes[0]')

Signed-off-by Sanjeev Premi <[email protected]>
Cc Jason Kridner <[email protected]>

---

Patch was compile tested against the latest u-boot-arm.git at:
58c583b : net: Check network device driver name

 include/configs/omap3_beagle.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
include/configs/omap3_beagle.h +1 -1
index 18c6deb..a891246 100644
@@ -118,7 +118,7 @@
 #define CONFIG_I2C_MULTI_BUS		1
 
 /* Probe all devices */
-#define CONFIG_SYS_I2C_NOPROBES		{0x0, 0x0}
+#define CONFIG_SYS_I2C_NOPROBES		{{0x0, 0x0}}
 
 /* USB */
 #define CONFIG_MUSB_UDC			1
Signature · 1 line
1.7.2.2
#1 lore raw

PATCH 1/1 omap3: beagle: Fix build warning

Hi Sanjeev,

Le 05/09/2011 12:25, Sanjeev Premi a ?crit :

Quoted 21 lines

This patch fixes the warning dure to recent changes to the board configuration:
cmd_i2c.o cmd_i2c.c -c
cmd_i2c.c:109:1: warning: missing braces around initializer cmd_i2c.c:109:1: warning: (near initialization for 'i2c_no_probes[0]')

Signed-off-by Sanjeev Premi<[email protected]>
Cc Jason Kridner<[email protected]>

---

Patch was compile tested against the latest u-boot-arm.git at:
58c583b : net: Check network device driver name

  include/configs/omap3_beagle.h |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
include/configs/omap3_beagle.h +0 -0
index 18c6deb..a891246 100644

include/configs/omap3_beagle.h line 124

Quoted 8 lines
include/configs/omap3_beagle.h +1 -1
@@ -118,7 +118,7 @@
  #define CONFIG_I2C_MULTI_BUS		1
 
  /* Probe all devices */
-#define CONFIG_SYS_I2C_NOPROBES		{0x0, 0x0}
+#define CONFIG_SYS_I2C_NOPROBES		{{0x0, 0x0}}
 
  /* USB */
  #define CONFIG_MUSB_UDC			1

Won't all board configs which use CONFIG_SYS_I2C_NOPROBES suffer from the same bug? I would hate to see an endless trickle of individual board config patches, and would much prefer a single patch to fix all boards in one go if you are willing to do it -- yes, it would touch boards that you cannot even test, but I'd say the risk is next to zero.

Wolfgang, Heiko, your opinion?

Amicalement,

Signature · 1 line
Albert.
#2 lore raw

PATCH 1/1 omap3: beagle: Fix build warning

Quoted 39 lines

-----Original Message-----
From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
Sent: Monday, September 05, 2011 5:06 PM
To: Premi, Sanjeev
Cc: u-boot at lists.denx.de; Heiko Schocher; Wolfgang Denk
Subject: Re: [U-Boot] [PATCH] omap3: beagle: Fix build warning

Hi Sanjeev,

Le 05/09/2011 12:25, Sanjeev Premi a ?crit :

Quoted 5 lines

This patch fixes the warning dure to recent changes to the board configuration:
cmd_i2c.o cmd_i2c.c -c
cmd_i2c.c:109:1: warning: missing braces around initializer cmd_i2c.c:109:1: warning: (near initialization for

'i2c_no_probes[0]')

Quoted 5 lines
Signed-off-by Sanjeev Premi<[email protected]>
Cc Jason Kridner<[email protected]>

---

Review

Quoted 7 lines

Patch was compile tested against the latest u-boot-arm.git at:
58c583b : net: Check network device driver name

  include/configs/omap3_beagle.h |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
(quoted diff) +0 -0

b/include/configs/omap3_beagle.h

Quoted 10 lines

index 18c6deb..a891246 100644

include/configs/omap3_beagle.h +1 -1
@@ -118,7 +118,7 @@
  #define CONFIG_I2C_MULTI_BUS		1
 
  /* Probe all devices */
-#define CONFIG_SYS_I2C_NOPROBES		{0x0, 0x0}
+#define CONFIG_SYS_I2C_NOPROBES		{{0x0, 0x0}}
Quoted 8 lines

include/configs/omap3_beagle.h line 124

Quoted 2 lines
include/configs/omap3_beagle.h +0 -0
  /* USB */
  #define CONFIG_MUSB_UDC			1

Won't all board configs which use CONFIG_SYS_I2C_NOPROBES suffer from the same bug? I would hate to see an endless trickle of individual board
config patches, and would much prefer a single patch to fix all boards

[sp] Actually, not all boards suffer from this issue.
But yes, I can make a single patch and submit it in next couple of hours.

Quoted 10 lines

in one go if you are willing to do it -- yes, it would touch boards that
you cannot even test, but I'd say the risk is next to zero.

Wolfgang, Heiko, your opinion?

Amicalement,

Signature · 1 line
Albert.
#3 lore raw

PATCH 1/1 omap3: beagle: Fix build warning

Hello Sanjeev,

Quoted 46 lines · Sanjeev
Quoted 38 lines

-----Original Message-----
From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
Sent: Monday, September 05, 2011 5:06 PM
To: Premi, Sanjeev
Cc: u-boot at lists.denx.de; Heiko Schocher; Wolfgang Denk
Subject: Re: [U-Boot] [PATCH] omap3: beagle: Fix build warning

Hi Sanjeev,

Le 05/09/2011 12:25, Sanjeev Premi a ?crit :

Quoted 5 lines

This patch fixes the warning dure to recent changes to the board configuration:
cmd_i2c.o cmd_i2c.c -c
cmd_i2c.c:109:1: warning: missing braces around initializer cmd_i2c.c:109:1: warning: (near initialization for

'i2c_no_probes[0]')

Review

Quoted 11 lines
Signed-off-by Sanjeev Premi<[email protected]>
Cc Jason Kridner<[email protected]>

---

Patch was compile tested against the latest u-boot-arm.git at:
58c583b : net: Check network device driver name

  include/configs/omap3_beagle.h |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
(quoted diff) +0 -0

b/include/configs/omap3_beagle.h

Quoted 10 lines

index 18c6deb..a891246 100644

include/configs/omap3_beagle.h +1 -1
@@ -118,7 +118,7 @@
  #define CONFIG_I2C_MULTI_BUS		1
 
  /* Probe all devices */
-#define CONFIG_SYS_I2C_NOPROBES		{0x0, 0x0}
+#define CONFIG_SYS_I2C_NOPROBES		{{0x0, 0x0}}
Quoted 7 lines

include/configs/omap3_beagle.h line 124

Quoted 2 lines
include/configs/omap3_beagle.h +0 -0
  /* USB */
  #define CONFIG_MUSB_UDC			1

Won't all board configs which use CONFIG_SYS_I2C_NOPROBES suffer from the same bug? I would hate to see an endless trickle of individual board
config patches, and would much prefer a single patch to fix all boards

Quoted 3 lines

[sp] Actually, not all boards suffer from this issue.
But yes, I can make a single patch and submit it in next couple of hours.

Isn;t this issue introduced just from commit

author Jason Kridner <[email protected]>
Sat, 23 Jul 2011 04:42:44 +0000 (23:42 -0500)
committer Albert ARIBAUD <[email protected]>
Sun, 4 Sep 2011 09:36:21 +0000 (11:36 +0200)
commit f74fc4ae6d6257ecdbc0049f6aa2e96212207f05

so ~1day old ... Hmm, I think, this is just a single board bugfix, as this warning only raises, if CONFIG_I2C_MULTI_BUS and CONFIG_SYS_I2C_NOPROBES is defined ... as introduced for the beagle board through above commit ... other boards should be clean, as I tend to do a MAKEALL after applying patches from ML ... but if you find time and can check this, it would be nice!

bye,
Heiko

Signature · 2 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
#4 lore raw

PATCH 1/1 omap3: beagle: Fix build warning

Quoted 7 lines

-----Original Message-----
From: Heiko Schocher [mailto:hs at denx.de]
Sent: Monday, September 05, 2011 7:01 PM
To: Premi, Sanjeev
Cc: Albert ARIBAUD; u-boot at lists.denx.de; Wolfgang Denk
Subject: Re: [U-Boot] [PATCH] omap3: beagle: Fix build warning

Quoted 10 lines

Hello Sanjeev,

Quoted 7 lines · Sanjeev
Quoted 7 lines

-----Original Message-----
From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net]
Sent: Monday, September 05, 2011 5:06 PM
To: Premi, Sanjeev
Cc: u-boot at lists.denx.de; Heiko Schocher; Wolfgang Denk
Subject: Re: [U-Boot] [PATCH] omap3: beagle: Fix build warning

[snip]...[snip]

Quoted 30 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
include/configs/omap3_beagle.h +0 -0
@@ -118,7 +118,7 @@
  #define CONFIG_I2C_MULTI_BUS		1
Quoted 7 lines
Quoted 7 lines

include/configs/omap3_beagle.h line 124

Quoted 6 lines
include/configs/omap3_beagle.h +1 -1
  /* Probe all devices */
-#define CONFIG_SYS_I2C_NOPROBES		{0x0, 0x0}
+#define CONFIG_SYS_I2C_NOPROBES		{{0x0, 0x0}}
 
  /* USB */
  #define CONFIG_MUSB_UDC			1

Won't all board configs which use CONFIG_SYS_I2C_NOPROBES

suffer from

Quoted 8 lines
Quoted 4 lines

the same bug? I would hate to see an endless trickle of individual board
config patches, and would much prefer a single patch to fix all boards

[sp] Actually, not all boards suffer from this issue.
But yes, I can make a single patch and submit it in next couple of hours.

Isn;t this issue introduced just from commit

author Jason Kridner <[email protected]>
Sat, 23 Jul 2011 04:42:44 +0000 (23:42 -0500)
committer Albert ARIBAUD <[email protected]>
Sun, 4 Sep 2011 09:36:21 +0000 (11:36 +0200)
commit f74fc4ae6d6257ecdbc0049f6aa2e96212207f05

Quoted 5 lines

so ~1day old ... Hmm, I think, this is just a single board bugfix, as this warning only raises, if CONFIG_I2C_MULTI_BUS and CONFIG_SYS_I2C_NOPROBES is defined ... as introduced for the beagle board through above commit ... other boards should be clean, as I tend to do a MAKEALL after applying patches from ML ... but if you find

[sp] Yes, you are right about the issue, and hence I posted it as
single patch against beagleboard only. The description includes a bash script that I used to "blindly" extent the fix to other boards.

I guess, MAKEALL succeeds because the appropriate config options may not be enabled for the boards.

Will dig on a few cases (of the ones changed) as to why the build succeeds.

Quoted 1 line

time and can check this, it would be nice!

[sp] I only have a arm codesourcery toolchain installed. Would be able
to check only ARM boards (of the 31) impacted by the updated patch.

Can do by tomorrow...

Quoted 7 lines

bye,
Heiko

Signature · 2 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
#5 lore raw

PATCH 1/1 omap3: beagle: Fix build warning

In reply to Sanjeev Premi
Quoted 8 lines · Sanjeev Premi

This patch fixes the warning dure to recent changes to the board configuration:
cmd_i2c.o cmd_i2c.c -c
cmd_i2c.c:109:1: warning: missing braces around initializer cmd_i2c.c:109:1: warning: (near initialization for 'i2c_no_probes[0]')

Signed-off-by Sanjeev Premi <[email protected]>
Cc Jason Kridner <[email protected]>
Acked-by Jason Kridner <[email protected]>

I confirmed that this fixes the above warning and builds and operates properly. As far as other platforms needing this type of patch, it seems there is a challenge in speaking for all of them, so I hope this patch can be applied.

Quoted 25 lines

---

?Patch was compile tested against the latest u-boot-arm.git at:
?58c583b : net: Check network device driver name

?include/configs/omap3_beagle.h | ? ?2 +-
?1 files changed, 1 insertions(+), 1 deletions(-)

include/configs/omap3_beagle.h +0 -0
index 18c6deb..a891246 100644
@@ -118,7 +118,7 @@

?#define CONFIG_I2C_MULTI_BUS ? ? ? ? ? 1

?/* Probe all devices */

include/configs/omap3_beagle.h +1 -1
-#define CONFIG_SYS_I2C_NOPROBES ? ? ? ? ? ? ? ?{0x0, 0x0}
+#define CONFIG_SYS_I2C_NOPROBES ? ? ? ? ? ? ? ?{{0x0, 0x0}}

?/* USB */
?#define CONFIG_MUSB_UDC ? ? ? ? ? ? ? ? ? ? ? ?1
--
1.7.2.2

#6 lore raw

PATCH 1/1 omap3: beagle: Fix build warning

In reply to Sanjeev Premi

(Cc:ing Dirk for the non-patch-related error)

Hi Sanjeev,

Le 05/09/2011 12:25, Sanjeev Premi a ?crit :

Quoted 9 lines

This patch fixes the warning dure to recent changes to the board configuration:
cmd_i2c.o cmd_i2c.c -c
cmd_i2c.c:109:1: warning: missing braces around initializer cmd_i2c.c:109:1: warning: (near initialization for 'i2c_no_probes[0]')

Signed-off-by Sanjeev Premi<[email protected]>
Cc Jason Kridner<[email protected]>

---

Applied to u-boot-arm/master as it does fix the warning.

Note however that there is an error, independent from this patch, in building this board with ELDK42 and CS 2011q1 :

Configuring for omap3_beagle board...
beagle.c:532: warning: initialization from incompatible pointer type
led.c: In function '__led_toggle':
led.c:62: warning: implicit declaration of function 'omap_get_gpio_dataout'
board/ti/beagle/libbeagle.o: In function `__led_toggle':
/home/uboot/src/u-boot-arm/board/ti/beagle/led.c:62: undefined reference
to `omap_get_gpio_dataout'
arm-linux-ld: BFD (GNU Binutils) 2.17.90.20070806 assertion fail /opt/eldk/build/arm-2008-11-24/work/usr/src/denx/BUILD/crosstool-0.43/build/gcc-4.2.2-glibc-20070515T2025-eldk/arm-linux-gnueabi/binutils-2.17.90/bfd/elf32-arm.c:8886
arm-linux-ld: BFD (GNU Binutils) 2.17.90.20070806 assertion fail /opt/eldk/build/arm-2008-11-24/work/usr/src/denx/BUILD/crosstool-0.43/build/gcc-4.2.2-glibc-20070515T2025-eldk/arm-linux-gnueabi/binutils-2.17.90/bfd/elf32-arm.c:9117

(foillows a linker segmentation error)

Anyone can reproduce and tell what the issue is?

Amicalement,

Signature · 1 line
Albert.
#7 lore raw

PATCH 1/1 omap3: beagle: Fix build warning

Quoted 2 lines · Albert ARIBAUD

(Cc:ing Dirk for the non-patch-related error)

Hi Albert,

Quoted 15 lines

Note however that there is an error, independent from this patch, in building this board with ELDK42 and CS 2011q1 :

Configuring for omap3_beagle board...
beagle.c:532: warning: initialization from incompatible pointer type
led.c: In function '__led_toggle':
led.c:62: warning: implicit declaration of function 'omap_get_gpio_dataout'
board/ti/beagle/libbeagle.o: In function `__led_toggle':
/home/uboot/src/u-boot-arm/board/ti/beagle/led.c:62: undefined reference
to `omap_get_gpio_dataout'
arm-linux-ld: BFD (GNU Binutils) 2.17.90.20070806 assertion fail /opt/eldk/build/arm-2008-11-24/work/usr/src/denx/BUILD/crosstool-0.43/build/gcc-4.2.2-glibc-20070515T2025-eldk/arm-linux-gnueabi/binutils-2.17.90/bfd/elf32-arm.c:8886
arm-linux-ld: BFD (GNU Binutils) 2.17.90.20070806 assertion fail /opt/eldk/build/arm-2008-11-24/work/usr/src/denx/BUILD/crosstool-0.43/build/gcc-4.2.2-glibc-20070515T2025-eldk/arm-linux-gnueabi/binutils-2.17.90/bfd/elf32-arm.c:9117

Quoted 3 lines

(foillows a linker segmentation error)

Anyone can reproduce and tell what the issue is?

I can reproduce it. IMHO this issue is introduced with the following commit:

commit b8bc8973a1830bb92e7a9bf3356dc209afb2f4e8

Author: Joel A Fernandes <[email protected]>
Date:   Thu Aug 11 23:16:53 2011 -0500

There is no omap_get_gpio_dataout() actually in u-boot, but it is called to get the value of the LED:

	state = omap_get_gpio_dataout(toggle_gpio);

Even if we had this function, it sounds odd to read the status of a LED (or generally from a GPIO set to output), because we should already know which value we have written before. Instead of reading from hardware should we not save the state of the LED in a variable ?

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
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
=====================================================================
#8 lore raw

PATCH 1/1 omap3: beagle: Fix build warning

Quoted 23 lines

-----Original Message-----
From: Stefano Babic [mailto:sbabic at denx.de]
Sent: Wednesday, September 07, 2011 1:11 PM
To: Albert ARIBAUD
Cc: Premi, Sanjeev; u-boot at lists.denx.de; Dirk Behme; agnel.joel at gmail.com
Subject: Re: [U-Boot] [PATCH] omap3: beagle: Fix build warning

Quoted 2 lines · Albert ARIBAUD

(Cc:ing Dirk for the non-patch-related error)

Hi Albert,

Quoted 1 line

Note however that there is an error, independent from this

patch, in

Quoted 6 lines

building this board with ELDK42 and CS 2011q1 :

Configuring for omap3_beagle board...
beagle.c:532: warning: initialization from incompatible pointer type led.c: In function '__led_toggle':
led.c:62: warning: implicit declaration of function

'omap_get_gpio_dataout'

Quoted 8 lines
Quoted 2 lines

board/ti/beagle/libbeagle.o: In function `__led_toggle':
/home/uboot/src/u-boot-arm/board/ti/beagle/led.c:62:

undefined reference

Quoted 3 lines

to `omap_get_gpio_dataout'
arm-linux-ld: BFD (GNU Binutils) 2.17.90.20070806 assertion fail

/opt/eldk/build/arm-2008-11-24/work/usr/src/denx/BUILD/crossto ol-0.43/build/gcc-4.2.2-glibc-20070515T2025-eldk/arm-linux-gnu

eabi/binutils-2.17.90/bfd/elf32-arm.c:8886

Quoted 4 lines
Quoted 2 lines

arm-linux-ld: BFD (GNU Binutils) 2.17.90.20070806 assertion fail

/opt/eldk/build/arm-2008-11-24/work/usr/src/denx/BUILD/crossto ol-0.43/build/gcc-4.2.2-glibc-20070515T2025-eldk/arm-linux-gnu

eabi/binutils-2.17.90/bfd/elf32-arm.c:9117

Quoted 8 lines
Quoted 4 lines

(foillows a linker segmentation error)

Anyone can reproduce and tell what the issue is?

I can reproduce it. IMHO this issue is introduced with the following commit:

Quoted 8 lines

commit b8bc8973a1830bb92e7a9bf3356dc209afb2f4e8

Author: Joel A Fernandes <[email protected]>
Date:   Thu Aug 11 23:16:53 2011 -0500

There is no omap_get_gpio_dataout() actually in u-boot, but it is called
to get the value of the LED:

	state = omap_get_gpio_dataout(toggle_gpio);

[sp] I reported the missing function few days ago:

     http://marc.info/?l=u-boot&m=131522045310324&w=2

~sanjeev

Quoted 18 lines

Even if we had this function, it sounds odd to read the status of a LED
(or generally from a GPIO set to output), because we should already know
which value we have written before. Instead of reading from hardware should we not save the state of the LED in a variable ?

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
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
=====================================================================
#9 lore raw

PATCH 1/1 omap3: beagle: Fix build warning

cc ing Sandeep as the commit apparently comes from the TI tree.

Le 07/09/2011 10:47, Premi, Sanjeev a ?crit :

Quoted 54 lines
Quoted 23 lines

-----Original Message-----
From: Stefano Babic [mailto:sbabic at denx.de]
Sent: Wednesday, September 07, 2011 1:11 PM
To: Albert ARIBAUD
Cc: Premi, Sanjeev; u-boot at lists.denx.de; Dirk Behme; agnel.joel at gmail.com
Subject: Re: [U-Boot] [PATCH] omap3: beagle: Fix build warning

Quoted 2 lines · Albert ARIBAUD

(Cc:ing Dirk for the non-patch-related error)

Hi Albert,

Quoted 1 line

Note however that there is an error, independent from this

patch, in

Quoted 6 lines

building this board with ELDK42 and CS 2011q1 :

Configuring for omap3_beagle board...
beagle.c:532: warning: initialization from incompatible pointer type led.c: In function '__led_toggle':
led.c:62: warning: implicit declaration of function

'omap_get_gpio_dataout'

Quoted 8 lines
Quoted 2 lines

board/ti/beagle/libbeagle.o: In function `__led_toggle':
/home/uboot/src/u-boot-arm/board/ti/beagle/led.c:62:

undefined reference

Quoted 3 lines

to `omap_get_gpio_dataout'
arm-linux-ld: BFD (GNU Binutils) 2.17.90.20070806 assertion fail

/opt/eldk/build/arm-2008-11-24/work/usr/src/denx/BUILD/crossto ol-0.43/build/gcc-4.2.2-glibc-20070515T2025-eldk/arm-linux-gnu

eabi/binutils-2.17.90/bfd/elf32-arm.c:8886

Quoted 4 lines
Quoted 2 lines

arm-linux-ld: BFD (GNU Binutils) 2.17.90.20070806 assertion fail

/opt/eldk/build/arm-2008-11-24/work/usr/src/denx/BUILD/crossto ol-0.43/build/gcc-4.2.2-glibc-20070515T2025-eldk/arm-linux-gnu

eabi/binutils-2.17.90/bfd/elf32-arm.c:9117

Quoted 8 lines
Quoted 4 lines

(foillows a linker segmentation error)

Anyone can reproduce and tell what the issue is?

I can reproduce it. IMHO this issue is introduced with the following commit:

Quoted 8 lines

commit b8bc8973a1830bb92e7a9bf3356dc209afb2f4e8

Author: Joel A Fernandes<[email protected]>
Date:   Thu Aug 11 23:16:53 2011 -0500

There is no omap_get_gpio_dataout() actually in u-boot, but it is called
to get the value of the LED:

	state = omap_get_gpio_dataout(toggle_gpio);
Quoted 4 lines

[sp] I reported the missing function few days ago:

      http://marc.info/?l=u-boot&m=131522045310324&w=2

~sanjeev

Apologies for not noticing.

Quoted 6 lines
Quoted 6 lines

Even if we had this function, it sounds odd to read the status of a LED
(or generally from a GPIO set to output), because we should already know
which value we have written before. Instead of reading from hardware should we not save the state of the LED in a variable ?

Actually, this is not that weird. All GPIOs I have dealt with can provide the value of their output, and I don't see what added value there is in storing their value in a RAM variable also.

What worries me, though, is that this commit is obviously dependent on other code changes that we don't have. Joel, can you help?

Quoted 2 lines
Quoted 2 lines

Best regards,
Stefano Babic

Amicalement,

Signature · 1 line
Albert.
#10 lore raw

PATCH 1/1 omap3: beagle: Fix build warning

Quoted 71 lines

Le 07/09/2011 10:47, Premi, Sanjeev a ?crit :

Quoted 54 lines
Quoted 23 lines

-----Original Message-----
From: Stefano Babic [mailto:sbabic at denx.de]
Sent: Wednesday, September 07, 2011 1:11 PM
To: Albert ARIBAUD
Cc: Premi, Sanjeev; u-boot at lists.denx.de; Dirk Behme; agnel.joel at gmail.com
Subject: Re: [U-Boot] [PATCH] omap3: beagle: Fix build warning

Quoted 2 lines · Albert ARIBAUD

(Cc:ing Dirk for the non-patch-related error)

Hi Albert,

Quoted 1 line

Note however that there is an error, independent from this

patch, in

Quoted 6 lines

building this board with ELDK42 and CS 2011q1 :

Configuring for omap3_beagle board...
beagle.c:532: warning: initialization from incompatible pointer type led.c: In function '__led_toggle':
led.c:62: warning: implicit declaration of function

'omap_get_gpio_dataout'

Quoted 8 lines
Quoted 2 lines

board/ti/beagle/libbeagle.o: In function `__led_toggle':
/home/uboot/src/u-boot-arm/board/ti/beagle/led.c:62:

undefined reference

Quoted 3 lines

to `omap_get_gpio_dataout'
arm-linux-ld: BFD (GNU Binutils) 2.17.90.20070806 assertion fail

/opt/eldk/build/arm-2008-11-24/work/usr/src/denx/BUILD/crossto ol-0.43/build/gcc-4.2.2-glibc-20070515T2025-eldk/arm-linux-gnu

eabi/binutils-2.17.90/bfd/elf32-arm.c:8886

Quoted 4 lines
Quoted 2 lines

arm-linux-ld: BFD (GNU Binutils) 2.17.90.20070806 assertion fail

/opt/eldk/build/arm-2008-11-24/work/usr/src/denx/BUILD/crossto ol-0.43/build/gcc-4.2.2-glibc-20070515T2025-eldk/arm-linux-gnu

eabi/binutils-2.17.90/bfd/elf32-arm.c:9117

Quoted 8 lines
Quoted 4 lines

(foillows a linker segmentation error)

Anyone can reproduce and tell what the issue is?

I can reproduce it. IMHO this issue is introduced with the following commit:

Quoted 8 lines

commit b8bc8973a1830bb92e7a9bf3356dc209afb2f4e8

Author: Joel A Fernandes<[email protected]>
Date:   Thu Aug 11 23:16:53 2011 -0500

There is no omap_get_gpio_dataout() actually in u-boot, but it is called
to get the value of the LED:

	state = omap_get_gpio_dataout(toggle_gpio);
Quoted 4 lines

[sp] I reported the missing function few days ago:

      http://marc.info/?l=u-boot&m=131522045310324&w=2

~sanjeev

Apologies for not noticing.

Quoted 6 lines
Quoted 6 lines

Even if we had this function, it sounds odd to read the status of a LED
(or generally from a GPIO set to output), because we should already know
which value we have written before. Instead of reading from hardware should we not save the state of the LED in a variable ?

Quoted 6 lines

Actually, this is not that weird. All GPIOs I have dealt with can provide the value of their output, and I don't see what added value there is in storing their value in a RAM variable also.

What worries me, though, is that this commit is obviously dependent on other code changes that we don't have. Joel, can you help?

Albert,

The patch is in the TI tree.

We were having some issues w.r.t the GPIO.

Patches for this have been sent to the list. I was waiting for some testing to see whether the GPIO patches actually work.

Some issues seem to be present after rebasing with mainline but they don't seem to be GPIO related.

I'll send a pull request soon.

Regards,
Sandeep

#11 lore raw

PATCH 1/1 omap3: beagle: Fix build warning

In reply to Albert ARIBAUD
Quoted 82 lines · Albert ARIBAUD
cc ing Sandeep as the commit apparently comes from the TI tree.

Le 07/09/2011 10:47, Premi, Sanjeev a ?crit :

Quoted 65 lines
Quoted 32 lines

-----Original Message-----
From: Stefano Babic [mailto:sbabic at denx.de]
Sent: Wednesday, September 07, 2011 1:11 PM
To: Albert ARIBAUD
Cc: Premi, Sanjeev; u-boot at lists.denx.de; Dirk Behme; agnel.joel at gmail.com
Subject: Re: [U-Boot] [PATCH] omap3: beagle: Fix build warning

Quoted 3 lines · Albert ARIBAUD

(Cc:ing Dirk for the non-patch-related error)

Hi Albert,

Quoted 1 line

Note however that there is an error, independent from this

patch, in

Quoted 7 lines

building this board with ELDK42 and CS 2011q1 :

Configuring for omap3_beagle board...
beagle.c:532: warning: initialization from incompatible pointer type led.c: In function '__led_toggle':
led.c:62: warning: implicit declaration of function

'omap_get_gpio_dataout'

Quoted 3 lines

board/ti/beagle/libbeagle.o: In function `__led_toggle':
/home/uboot/src/u-boot-arm/board/ti/beagle/led.c:62:

Quoted 7 lines

undefined reference

Quoted 4 lines

to `omap_get_gpio_dataout'
arm-linux-ld: BFD (GNU Binutils) 2.17.90.20070806 assertion fail

/opt/eldk/build/arm-2008-11-24/work/usr/src/denx/BUILD/crossto ol-0.43/build/gcc-4.2.2-glibc-20070515T2025-eldk/arm-linux-gnu

eabi/binutils-2.17.90/bfd/elf32-arm.c:8886

Quoted 5 lines
Quoted 3 lines

arm-linux-ld: BFD (GNU Binutils) 2.17.90.20070806 assertion fail

/opt/eldk/build/arm-2008-11-24/work/usr/src/denx/BUILD/crossto ol-0.43/build/gcc-4.2.2-glibc-20070515T2025-eldk/arm-linux-gnu

eabi/binutils-2.17.90/bfd/elf32-arm.c:9117

Quoted 8 lines
Quoted 4 lines

(foillows a linker segmentation error)

Anyone can reproduce and tell what the issue is?

I can reproduce it. IMHO this issue is introduced with the following commit:

Quoted 8 lines

commit b8bc8973a1830bb92e7a9bf3356dc209afb2f4e8
Author: Joel A Fernandes<[email protected]>
Date: ? Thu Aug 11 23:16:53 2011 -0500

There is no omap_get_gpio_dataout() actually in u-boot, but it is called
to get the value of the LED:
? ? ? ?state = omap_get_gpio_dataout(toggle_gpio);

Quoted 4 lines

[sp] I reported the missing function few days ago:
? ? ?http://marc.info/?l=u-boot&m=131522045310324&w=2

~sanjeev

Apologies for not noticing.

Quoted 6 lines
Quoted 6 lines

Even if we had this function, it sounds odd to read the status of a LED
(or generally from a GPIO set to output), because we should already know
which value we have written before. Instead of reading from hardware should we not save the state of the LED in a variable ?

Quoted 3 lines

Actually, this is not that weird. All GPIOs I have dealt with can provide the value of their output, and I don't see what added value there is in storing their value in a RAM variable also.

True, this was discussed here hence the patch:
http://lists.denx.de/pipermail/u-boot/2011-May/093068.html

Quoted 3 lines

What worries me, though, is that this commit is obviously dependent on other code changes that we don't have. Joel, can you help?

I hope from Sandeep's response, it is clarified that the new patch is not yet in explaining the undefined reference. Let me know if you need any other help from me. Thanks,
Joel