[U-Boot] [PATCH] usb: use noinline define

[U-Boot] [PATCH] usb: use noinline define

[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.

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;
-- 
1.7.8.5

[U-Boot] [PATCH] usb: use noinline define

[Marek Vasut]

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

[U-Boot] [PATCH] usb: use noinline define

[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 ?

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

[U-Boot] [PATCH] usb: use noinline define

[Marek Vasut]

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

[U-Boot] [PATCH] usb: use noinline define

[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).

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

[U-Boot] [PATCH] usb: use noinline define

[Marek Vasut]

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

[U-Boot] [PATCH] usb: use noinline define

[Mike Frysinger]

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