| Message ID | 1333601093-21481-1-git-send-email-vapier@gentoo.org |
|---|---|
| State | Accepted |
| Commit | 66cf64107b891d1cc8112ff76b5687195af2f5b0 |
| Delegated to: | Marek Vasut |
| Headers | show |
Dear Mike Frysinger, > 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? > Signed-off-by: Mike Frysinger <vapier@gentoo.org> > --- > common/usb.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/common/usb.c b/common/usb.c > index 1ec30bc..71b4b2b 100644 > --- a/common/usb.c > +++ b/common/usb.c > @@ -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; Best regards, Marek Vasut
On Thursday 05 April 2012 06:13:21 Marek Vasut wrote: > Dear Mike Frysinger, > > 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 ? > 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
Dear Mike Frysinger, > On Thursday 05 April 2012 06:13:21 Marek Vasut wrote: > > Dear Mike Frysinger, > > > > > 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? > > 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. > -mike Best regards, Marek Vasut
On Thursday 05 April 2012 15:19:11 Marek Vasut wrote: > Dear Mike Frysinger, > > > On Thursday 05 April 2012 06:13:21 Marek Vasut wrote: > > > Dear Mike Frysinger, > > > > > > > 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? 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)) > > > 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
Dear Mike Frysinger, > On Thursday 05 April 2012 15:19:11 Marek Vasut wrote: > > Dear Mike Frysinger, > > > > > On Thursday 05 April 2012 06:13:21 Marek Vasut wrote: > > > > Dear Mike Frysinger, > > > > > > > > > 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? > > 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. > if you really want to be pedantic, you should be using: > __attribute__((__noinline__)) > instead of: > __attribute__((noinline)) Good point > > > > 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. Ok, will queue this for -next then. > -mike Best regards, Marek Vasut
On Thursday 05 April 2012 16:24:14 Marek Vasut wrote: > Dear Mike Frysinger, > > On Thursday 05 April 2012 15:19:11 Marek Vasut wrote: > > > Dear Mike Frysinger, > > > > On Thursday 05 April 2012 06:13:21 Marek Vasut wrote: > > > > > Dear Mike Frysinger, > > > > > > 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? > > > > 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
diff --git a/common/usb.c b/common/usb.c index 1ec30bc..71b4b2b 100644 --- a/common/usb.c +++ b/common/usb.c @@ -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;
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 <vapier@gentoo.org> --- common/usb.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)