[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 at 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 at 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 at 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 at aribaud.net>
> >>>> Signed-off-by: Jeroen Hofstee <jeroen at 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.