Skip to contentSkip to conversation

usb: use noinline define

PATCH New Mike Frysinger 2 participants 7 messages Apr 5, 2012

lore
Share & tools
lore

Mike Frysinger

author
#0 lore raw

PATCH 1/1 usb: use noinline define

Building usb for Blackfin boards fails as we get linux/compiler.h included which expands the "noinline" inside of the attribute and we get attribute(attribute(noinline)).

Explicitly use the helper define to avoid this.

Signed-off-by Mike Frysinger <[email protected]>
---
 common/usb.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
common/usb.c +2 -1
index 1ec30bc..71b4b2b 100644
@@ -47,6 +47,7 @@
 #include <common.h>
 #include <command.h>
 #include <asm/processor.h>
+#include <linux/compiler.h>
 #include <linux/ctype.h>
 #include <asm/byteorder.h>
 #include <asm/unaligned.h>

@@ -261,7 +262,7 @@ int usb_maxpacket(struct usb_device *dev, unsigned long pipe)
  *
  * NOTE: Similar behaviour was observed with GCC4.6 on ARMv5.
  */
-static void  __attribute__((noinline))
+static void noinline
 usb_set_maxpacket_ep(struct usb_device *dev, int if_idx, int ep_idx)
 {
 	int b;
Signature · 1 line
1.7.8.5
#1 lore raw

PATCH 1/1 usb: use noinline define

Dear Mike Frysinger,

Quoted 5 lines

Building usb for Blackfin boards fails as we get linux/compiler.h included which expands the "noinline" inside of the attribute and we get attribute(attribute(noinline)).

Explicitly use the helper define to avoid this.

Ain't compiler.h broken then? Btw. is this a fix I should push to .04 release?

Quoted 21 lines
Signed-off-by Mike Frysinger <[email protected]>
---
 common/usb.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
common/usb.c +1 -0
index 1ec30bc..71b4b2b 100644
@@ -47,6 +47,7 @@
 #include <common.h>
 #include <command.h>
 #include <asm/processor.h>
+#include <linux/compiler.h>
 #include <linux/ctype.h>
 #include <asm/byteorder.h>
 #include <asm/unaligned.h>

@@ -261,7 +262,7 @@ int usb_maxpacket(struct usb_device *dev, unsigned long

pipe) *
* NOTE: Similar behaviour was observed with GCC4.6 on ARMv5. */

common/usb.c int usb_maxpacket(struct usb_device *dev, unsigned long pipe) · line 268

Quoted 5 lines
common/usb.c +1 -1
-static void  __attribute__((noinline))
+static void noinline
 usb_set_maxpacket_ep(struct usb_device *dev, int if_idx, int ep_idx)
 {
 	int b;

Best regards,
Marek Vasut

author
#2 lore raw

PATCH 1/1 usb: use noinline define

Quoted 8 lines · On Thursday 05 April 2012 06:13:21 Marek Vasut wrote:

Dear Mike Frysinger,

Quoted 5 lines

Building usb for Blackfin boards fails as we get linux/compiler.h included which expands the "noinline" inside of the attribute and we get attribute(attribute(noinline)).

Explicitly use the helper define to avoid this.

Ain't compiler.h broken then?

no ... why would it be ?

Quoted 1 line

Btw. is this a fix I should push to .04 release?

i looked closer at how exactly it's failing, and i think waiting for the next merge window is OK if you want
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120405/617efd36/attachment.pgp>

#3 lore raw

PATCH 1/1 usb: use noinline define

Dear Mike Frysinger,

Quoted 7 lines · On Thursday 05 April 2012 06:13:21 Marek Vasut wrote:
Quoted 6 lines

Dear Mike Frysinger,

Quoted 4 lines

Building usb for Blackfin boards fails as we get linux/compiler.h included which expands the "noinline" inside of the attribute and we get attribute(attribute(noinline)).

Quoted 5 lines
Quoted 3 lines
Quoted 1 line

Explicitly use the helper define to avoid this.

Ain't compiler.h broken then?

no ... why would it be ?

Because it colides with gcc stuff?

Quoted 4 lines
Quoted 1 line

Btw. is this a fix I should push to .04 release?

i looked closer at how exactly it's failing, and i think waiting for the next merge window is OK if you want

I'm fine either way as I don't observe this. On the other hand, I'd be inclined to push this into current release if you're 100% sure about the fix and it fixes real issue for some people.

Quoted 1 line

-mike

Best regards,
Marek Vasut

author
#4 lore raw

PATCH 1/1 usb: use noinline define

Quoted 9 lines · On Thursday 05 April 2012 15:19:11 Marek Vasut wrote:

Dear Mike Frysinger,

Quoted 7 lines · On Thursday 05 April 2012 06:13:21 Marek Vasut wrote:
Quoted 6 lines

Dear Mike Frysinger,

Quoted 4 lines

Building usb for Blackfin boards fails as we get linux/compiler.h included which expands the "noinline" inside of the attribute and we get attribute(attribute(noinline)).

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

Explicitly use the helper define to avoid this.

Ain't compiler.h broken then?

no ... why would it be ?

Because it colides with gcc stuff?

it provides shortcuts so you don't have to go grubbing into __attribute__ syntax, and it does so in a way that supports multiple gcc versions and compilers (although the latter isn't generally a realistic use case for us).

if you really want to be pedantic, you should be using:

	__attribute__((__noinline__))
instead of:
	__attribute__((noinline))
Quoted 8 lines
Quoted 4 lines
Quoted 1 line

Btw. is this a fix I should push to .04 release?

i looked closer at how exactly it's failing, and i think waiting for the next merge window is OK if you want

I'm fine either way as I don't observe this. On the other hand, I'd be inclined to push this into current release if you're 100% sure about the fix and it fixes real issue for some people.

in general, i wouldn't mind seeing common.h including linux/compiler.h and just having everyone use these shortcuts instead of open coding the attribute. i'm sure that would break more code (like this common/usb.c) in the short term, but it'd be much nicer imo long term.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120405/420fa943/attachment.pgp>

#5 lore raw

PATCH 1/1 usb: use noinline define

Dear Mike Frysinger,

Quoted 14 lines · On Thursday 05 April 2012 15:19:11 Marek Vasut wrote:
Quoted 13 lines

Dear Mike Frysinger,

Quoted 11 lines · On Thursday 05 April 2012 06:13:21 Marek Vasut wrote:
Quoted 9 lines

Dear Mike Frysinger,

Quoted 5 lines

Building usb for Blackfin boards fails as we get linux/compiler.h included which expands the "noinline" inside of the attribute and we get attribute(attribute(noinline)).

Explicitly use the helper define to avoid this.

Ain't compiler.h broken then?

Quoted 8 lines
Quoted 3 lines
Quoted 1 line

no ... why would it be ?

Because it colides with gcc stuff?

it provides shortcuts so you don't have to go grubbing into __attribute__ syntax, and it does so in a way that supports multiple gcc versions and compilers (although the latter isn't generally a realistic use case for us).

U-Boot/LLVM or U-Boot/VC ? ;-)

We'll have to support LLVM eventually anyway.

Quoted 4 lines

if you really want to be pedantic, you should be using:

	__attribute__((__noinline__))
instead of:
	__attribute__((noinline))

Good point

Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
Quoted 1 line

Btw. is this a fix I should push to .04 release?

i looked closer at how exactly it's failing, and i think waiting for the next merge window is OK if you want

Quoted 8 lines
Quoted 3 lines

I'm fine either way as I don't observe this. On the other hand, I'd be inclined to push this into current release if you're 100% sure about the fix and it fixes real issue for some people.

in general, i wouldn't mind seeing common.h including linux/compiler.h and just having everyone use these shortcuts instead of open coding the attribute. i'm sure that would break more code (like this common/usb.c) in the short term, but it'd be much nicer imo long term.

Ok, will queue this for -next then.

Quoted 1 line

-mike

Best regards,
Marek Vasut

author
#6 lore raw

PATCH 1/1 usb: use noinline define

Quoted 17 lines · On Thursday 05 April 2012 16:24:14 Marek Vasut wrote:

Dear Mike Frysinger,

Quoted 16 lines · On Thursday 05 April 2012 15:19:11 Marek Vasut wrote:
Quoted 14 lines

Dear Mike Frysinger,

Quoted 11 lines · On Thursday 05 April 2012 06:13:21 Marek Vasut wrote:
Quoted 8 lines

Dear Mike Frysinger,

Quoted 5 lines

Building usb for Blackfin boards fails as we get linux/compiler.h included which expands the "noinline" inside of the attribute and we get attribute(attribute(noinline)).

Explicitly use the helper define to avoid this.

Ain't compiler.h broken then?

no ... why would it be ?

Because it colides with gcc stuff?

Quoted 8 lines
Quoted 4 lines

it provides shortcuts so you don't have to go grubbing into __attribute__ syntax, and it does so in a way that supports multiple gcc versions and compilers (although the latter isn't generally a realistic use case for us).

U-Boot/LLVM or U-Boot/VC ? ;-)

We'll have to support LLVM eventually anyway.

true ... people want to use llvm/clang for some reason (who cares about code generation!?). linux/compiler.h will make that much easier for us.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120405/6044bcec/attachment.pgp>