[U-Boot] [PATCH] ARM:asm:io.h use static inline

[U-Boot] [PATCH] ARM:asm:io.h use static inline

[Jeroen Hofstee]

When compiling u-boot with W=1 the extern inline void for
read* is likely causing the most noise. gcc / clang will
warn there is never a actual declaration for these functions.
Instead of declaring these extern make them static inline so
it is actually declared.

cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl>
---
 arch/arm/include/asm/io.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index 9f35fd6..7b6189b 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -75,7 +75,7 @@ static inline phys_addr_t virt_to_phys(void * vaddr)
 #define __arch_putw(v,a)		(*(volatile unsigned short *)(a) = (v))
 #define __arch_putl(v,a)		(*(volatile unsigned int *)(a) = (v))
 
-extern inline void __raw_writesb(unsigned long addr, const void *data,
+static inline void __raw_writesb(unsigned long addr, const void *data,
 				 int bytelen)
 {
 	uint8_t *buf = (uint8_t *)data;
@@ -83,7 +83,7 @@ extern inline void __raw_writesb(unsigned long addr, const void *data,
 		__arch_putb(*buf++, addr);
 }
 
-extern inline void __raw_writesw(unsigned long addr, const void *data,
+static inline void __raw_writesw(unsigned long addr, const void *data,
 				 int wordlen)
 {
 	uint16_t *buf = (uint16_t *)data;
@@ -91,7 +91,7 @@ extern inline void __raw_writesw(unsigned long addr, const void *data,
 		__arch_putw(*buf++, addr);
 }
 
-extern inline void __raw_writesl(unsigned long addr, const void *data,
+static inline void __raw_writesl(unsigned long addr, const void *data,
 				 int longlen)
 {
 	uint32_t *buf = (uint32_t *)data;
@@ -99,21 +99,21 @@ extern inline void __raw_writesl(unsigned long addr, const void *data,
 		__arch_putl(*buf++, addr);
 }
 
-extern inline void __raw_readsb(unsigned long addr, void *data, int bytelen)
+static inline void __raw_readsb(unsigned long addr, void *data, int bytelen)
 {
 	uint8_t *buf = (uint8_t *)data;
 	while(bytelen--)
 		*buf++ = __arch_getb(addr);
 }
 
-extern inline void __raw_readsw(unsigned long addr, void *data, int wordlen)
+static inline void __raw_readsw(unsigned long addr, void *data, int wordlen)
 {
 	uint16_t *buf = (uint16_t *)data;
 	while(wordlen--)
 		*buf++ = __arch_getw(addr);
 }
 
-extern inline void __raw_readsl(unsigned long addr, void *data, int longlen)
+static inline void __raw_readsl(unsigned long addr, void *data, int longlen)
 {
 	uint32_t *buf = (uint32_t *)data;
 	while(longlen--)
-- 
1.8.3.2

[U-Boot] [PATCH] ARM:asm:io.h use static inline

[Albert ARIBAUD]

Hi Jeroen,

On Sun, 22 Jun 2014 23:10:39 +0200, Jeroen Hofstee
<jeroen@myspectrum.nl> wrote:

> When compiling u-boot with W=1 the extern inline void for
> read* is likely causing the most noise. gcc / clang will
> warn there is never a actual declaration for these functions.
> Instead of declaring these extern make them static inline so
> it is actually declared.
> 
> cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
> Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl>
> ---

Ok, so the obvious question: what makes you decide to switch to 'static
inline' rather than provide the extern versions that 'extern static'
calls for?

(see e.g. http://elinux.org/Extern_Vs_Static_Inline for an example
discussion on 'static inline' vs 'extern inline'. Note: this link is not
intended as an expression of my stance on the issue, as I don't have
any.)

Amicalement,
-- 
Albert.

[U-Boot] [PATCH] ARM:asm:io.h use static inline

[Jeroen Hofstee]

Hello Albert,

On za, 2014-07-05 at 11:13 +0200, Albert ARIBAUD wrote:
> Hi Jeroen,
> 
> On Sun, 22 Jun 2014 23:10:39 +0200, Jeroen Hofstee
> <jeroen@myspectrum.nl> wrote:
> 
> > When compiling u-boot with W=1 the extern inline void for
> > read* is likely causing the most noise. gcc / clang will
> > warn there is never a actual declaration for these functions.
> > Instead of declaring these extern make them static inline so
> > it is actually declared.
> > 
> > cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
> > Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl>
> > ---
> 
> Ok, so the obvious question: what makes you decide to switch to 'static
> inline' rather than provide the extern versions that 'extern static'
> calls for?

Assuming your question is, why didn't you just add the prototypes instead?

Well if we wanted to be brave gnu99 citizens we should provide the
prototypes, the extern inline version and a separated definition in
case the compiler fails / is not in the mood to inline the function.
This quite fragile / some housekeeping. Furthermore it is gnu specific
and likely fails with gcc -std=c99 as well.

Making them static inline there is always a single definition and it
is up to the compiler to either inline it or make it a static function
by it self. Since we were already relying on the compiler to inline
it (at least I am unaware that there are non inline version around),
this boils down to the same thing, but without warnings.

And... I can likely drop this one as well[1], although I haven't
checked yet. linux does the same btw for __LINUX_ARM_ARCH__ >= 6.

I can check binary size if that something you wonder about...

Regards,
Jeroen

[1] https://github.com/jhofstee/u-boot/commit/5cd261fecc5397bf5abef82f6a781d8b04992654

[U-Boot] [PATCH] ARM:asm:io.h use static inline

[Albert ARIBAUD]

Hi Jeroen,

(sorry for the near-duplicate, and see question at end)

On Sat, 05 Jul 2014 13:36:47 +0200, Jeroen Hofstee
<jeroen@myspectrum.nl> wrote:

> Hello Albert,
> 
> On za, 2014-07-05 at 11:13 +0200, Albert ARIBAUD wrote:
> > Hi Jeroen,
> > 
> > On Sun, 22 Jun 2014 23:10:39 +0200, Jeroen Hofstee
> > <jeroen@myspectrum.nl> wrote:
> > 
> > > When compiling u-boot with W=1 the extern inline void for
> > > read* is likely causing the most noise. gcc / clang will
> > > warn there is never a actual declaration for these functions.
> > > Instead of declaring these extern make them static inline so
> > > it is actually declared.
> > > 
> > > cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
> > > Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl>
> > > ---
> > 
> > Ok, so the obvious question: what makes you decide to switch to 'static
> > inline' rather than provide the extern versions that 'extern static'
> > calls for?
> 
> Assuming your question is, why didn't you just add the prototypes instead?

It was more along the lines of "were you aware that you had a choice
there?"

> Well if we wanted to be brave gnu99 citizens we should provide the
> prototypes, the extern inline version and a separated definition in
> case the compiler fails / is not in the mood to inline the function.
> This quite fragile / some housekeeping. Furthermore it is gnu specific
> and likely fails with gcc -std=c99 as well.
> 
> Making them static inline there is always a single definition and it
> is up to the compiler to either inline it or make it a static function
> by it self. Since we were already relying on the compiler to inline
> it (at least I am unaware that there are non inline version around),
> this boils down to the same thing, but without warnings.
> 
> And... I can likely drop this one as well[1], although I haven't
> checked yet. linux does the same btw for __LINUX_ARM_ARCH__ >= 6.
> 
> I can check binary size if that something you wonder about...

As I said, I have no stance on whether 'static inline' or 'extern
inline' was better / more appropriate / other (please specify). I just
wanted to make sure that you had considered both possibilities before
choosing one of them. I am now assured that you have, so all is fine.

> Regards,
> Jeroen
> 
> [1] https://github.com/jhofstee/u-boot/commit/5cd261fecc5397bf5abef82f6a781d8b04992654

Which board do you get warnings for?

Amicalement,
-- 
Albert.

[U-Boot] [PATCH] ARM:asm:io.h use static inline

[Jeroen Hofstee]

Hello Albert,

On 05-07-14 15:21, Albert ARIBAUD wrote:
> Hi Jeroen,
>
> (sorry for the near-duplicate, and see question at end)
>
> On Sat, 05 Jul 2014 13:36:47 +0200, Jeroen Hofstee
> <jeroen@myspectrum.nl> wrote:
>
>> Hello Albert,
>>
>> On za, 2014-07-05 at 11:13 +0200, Albert ARIBAUD wrote:
>>> Hi Jeroen,
>>>
>>> On Sun, 22 Jun 2014 23:10:39 +0200, Jeroen Hofstee
>>> <jeroen@myspectrum.nl> wrote:
>>>
>>>> When compiling u-boot with W=1 the extern inline void for
>>>> read* is likely causing the most noise. gcc / clang will
>>>> warn there is never a actual declaration for these functions.
>>>> Instead of declaring these extern make them static inline so
>>>> it is actually declared.
>>>>
>>>> cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
>>>> Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl>
>>>> ---
>>> Ok, so the obvious question: what makes you decide to switch to 'static
>>> inline' rather than provide the extern versions that 'extern static'
>>> calls for?
>> Assuming your question is, why didn't you just add the prototypes instead?
> It was more along the lines of "were you aware that you had a choice
> there?"
yes I was aware of that.
>> Well if we wanted to be brave gnu99 citizens we should provide the
>> prototypes, the extern inline version and a separated definition in
>> case the compiler fails / is not in the mood to inline the function.
>> This quite fragile / some housekeeping. Furthermore it is gnu specific
>> and likely fails with gcc -std=c99 as well.
>>
>> Making them static inline there is always a single definition and it
>> is up to the compiler to either inline it or make it a static function
>> by it self. Since we were already relying on the compiler to inline
>> it (at least I am unaware that there are non inline version around),
>> this boils down to the same thing, but without warnings.
>>
>> And... I can likely drop this one as well[1], although I haven't
>> checked yet. linux does the same btw for __LINUX_ARM_ARCH__ >= 6.
>>
>> I can check binary size if that something you wonder about...
> As I said, I have no stance on whether 'static inline' or 'extern
> inline' was better / more appropriate / other (please specify). I just
> wanted to make sure that you had considered both possibilities before
> choosing one of them. I am now assured that you have, so all is fine.
ok
>> Regards,
>> Jeroen
>>
>> [1] https://github.com/jhofstee/u-boot/commit/5cd261fecc5397bf5abef82f6a781d8b04992654
> Which board do you get warnings for?
Basically any, but as the commit message says, it is just for removing
warnings when building with `make W=1` or clang (which actually errors,
without the other patch). There is no hurry in applying it. But e.g.
for the twister I am sure you will see them, with the additional warnings
enabled.

Regards,
Jeroen

[U-Boot] [PATCH] ARM:asm:io.h use static inline

[Albert ARIBAUD]

Hi Jeroen,

On Sat, 05 Jul 2014 15:30:54 +0200, Jeroen Hofstee
<jeroen@myspectrum.nl> wrote:

> Hello Albert,
> 
> On 05-07-14 15:21, Albert ARIBAUD wrote:
> > Hi Jeroen,
> >
> > (sorry for the near-duplicate, and see question at end)
> >
> > On Sat, 05 Jul 2014 13:36:47 +0200, Jeroen Hofstee
> > <jeroen@myspectrum.nl> wrote:
> >
> >> Hello Albert,
> >>
> >> On za, 2014-07-05 at 11:13 +0200, Albert ARIBAUD wrote:
> >>> Hi Jeroen,
> >>>
> >>> On Sun, 22 Jun 2014 23:10:39 +0200, Jeroen Hofstee
> >>> <jeroen@myspectrum.nl> wrote:
> >>>
> >>>> When compiling u-boot with W=1 the extern inline void for
> >>>> read* is likely causing the most noise. gcc / clang will
> >>>> warn there is never a actual declaration for these functions.
> >>>> Instead of declaring these extern make them static inline so
> >>>> it is actually declared.
> >>>>
> >>>> cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
> >>>> Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl>
> >>>> ---
> >>> Ok, so the obvious question: what makes you decide to switch to 'static
> >>> inline' rather than provide the extern versions that 'extern static'
> >>> calls for?
> >> Assuming your question is, why didn't you just add the prototypes instead?
> > It was more along the lines of "were you aware that you had a choice
> > there?"
> yes I was aware of that.
> >> Well if we wanted to be brave gnu99 citizens we should provide the
> >> prototypes, the extern inline version and a separated definition in
> >> case the compiler fails / is not in the mood to inline the function.
> >> This quite fragile / some housekeeping. Furthermore it is gnu specific
> >> and likely fails with gcc -std=c99 as well.
> >>
> >> Making them static inline there is always a single definition and it
> >> is up to the compiler to either inline it or make it a static function
> >> by it self. Since we were already relying on the compiler to inline
> >> it (at least I am unaware that there are non inline version around),
> >> this boils down to the same thing, but without warnings.
> >>
> >> And... I can likely drop this one as well[1], although I haven't
> >> checked yet. linux does the same btw for __LINUX_ARM_ARCH__ >= 6.
> >>
> >> I can check binary size if that something you wonder about...
> > As I said, I have no stance on whether 'static inline' or 'extern
> > inline' was better / more appropriate / other (please specify). I just
> > wanted to make sure that you had considered both possibilities before
> > choosing one of them. I am now assured that you have, so all is fine.
> ok
> >> Regards,
> >> Jeroen
> >>
> >> [1] https://github.com/jhofstee/u-boot/commit/5cd261fecc5397bf5abef82f6a781d8b04992654
> > Which board do you get warnings for?
> Basically any, but as the commit message says, it is just for removing
> warnings when building with `make W=1` or clang (which actually errors,
> without the other patch). There is no hurry in applying it. But e.g.
> for the twister I am sure you will see them, with the additional warnings
> enabled.

Ok, seeing that it is not needed for customary builds, I'll defer this
until after 2014.07.

> Regards,
> Jeroen

Amicalement,
-- 
Albert.

[U-Boot] ARM:asm:io.h use static inline

[Tom Rini]

On Sun, Jun 22, 2014 at 11:10:39PM +0200, Jeroen Hofstee wrote:

> When compiling u-boot with W=1 the extern inline void for
> read* is likely causing the most noise. gcc / clang will
> warn there is never a actual declaration for these functions.
> Instead of declaring these extern make them static inline so
> it is actually declared.
> 
> cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
> Signed-off-by: Jeroen Hofstee <jeroen@myspectrum.nl>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140830/80444ad4/attachment.pgp>